Skip to content

Add a few fixes for Mediatek mt8188 and mt8195 platforms #10137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kv2019i merged 7 commits into from
Sep 3, 2025
Merged

Add a few fixes for Mediatek mt8188 and mt8195 platforms #10137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2e3f214
platform/mtk: Set AFE unused reg to 0 instead of -1.
andrew-mtk Apr 1, 2025
fa92c53
platform/mtk: Add FS timings for MT818X, MT8195
andrew-mtk May 20, 2025
cacbaad
zephyr: alloc: Limit heap size on MTK platforms
andrew-mtk May 27, 2025
85ab536
drivers/mediatek/afe: Fix DMA buffer alignment for MT818X and MT8195
andrew-mtk May 27, 2025
18f80dc
platform: mtk: Add AFE buffer address translation
andrew-mtk Jun 4, 2025
46e5cd9
zephyr: lib/dma.h: Fix build error in dma-legacy.h
andrew-mtk Jul 22, 2025
46515b0
boards: mtk: Fix "dai_common_copy(): nothing to copy" warning
andrew-mtk Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/boards/mt8188_mt8188_adsp.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@
# Board-level config goes in Zephyr (and ideally in DTS). App-level
# config goes in prj.conf.
CONFIG_MTK=y

# Override project default setting so 1 millisecond is exactly
# represented by an integral number of ticks.
CONFIG_SYS_CLOCK_TICKS_PER_SEC=13000
4 changes: 4 additions & 0 deletions app/boards/mt8195_mt8195_adsp.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@
# Board-level config goes in Zephyr (and ideally in DTS). App-level
# config goes in prj.conf.
CONFIG_MTK=y

# Override project default setting so 1 millisecond is exactly
# represented by an integral number of ticks.
CONFIG_SYS_CLOCK_TICKS_PER_SEC=13000
4 changes: 4 additions & 0 deletions src/drivers/mediatek/afe/afe-memif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,11 @@ static int memif_get_attribute(struct dma *dma, uint32_t type, uint32_t *value)
*value = 4;
break;
case DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT:
#if defined(CONFIG_SOC_SERIES_MT818X) || defined(CONFIG_SOC_MT8195)
*value = 64;
#else
*value = 16;
#endif
break;
case DMA_ATTR_BUFFER_PERIOD_COUNT:
*value = 4;
Expand Down
74 changes: 73 additions & 1 deletion src/platform/mtk/dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,25 @@
*/
#if defined(CONFIG_SOC_MT8186)
#define MTK_AFE_BASE 0x11210000
#define SRAM_CPU_START 0x10800000
#elif defined(CONFIG_SOC_SERIES_MT818X)
#define MTK_AFE_BASE 0x10b10000
#define SRAM_CPU_START 0x10d00000
Copy link
Contributor

@andyross andyross Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: maybe "HOST_START" or "AP_START" or "LINUX_START" and not "CPU", which is ambiguous. (Zephyr has "cpus" too).

#elif defined(CONFIG_SOC_MT8195)
#define MTK_AFE_BASE 0x10890000
#define SRAM_CPU_START 0x10840000
#elif defined(CONFIG_SOC_MT8196)
#define MTK_AFE_BASE 0x1a110000
#define SRAM_CPU_START 0x1a210000
#else
#error Unrecognized device
#endif

#define SRAM_ADSP_START DT_REG_ADDR(DT_NODELABEL(sram0))
#define SRAM_SIZE DT_REG_SIZE(DT_NODELABEL(sram0))
#define SRAM_ADSP_END (SRAM_ADSP_START + SRAM_SIZE)
#define SRAM_CPU_END (SRAM_CPU_START + SRAM_SIZE)

/* Bitfield register: address, left shift amount, and number of bits */
struct afe_bitfld {
uint32_t reg;
Expand Down Expand Up @@ -70,7 +79,7 @@ struct afe_cfg {
*/
static void cfg_convert(const struct afe_cfg *src, struct mtk_base_memif_data *dst)
{
#define REGCVT(R) (((R) > 0) ? ((R) - MTK_AFE_BASE) : -1)
#define REGCVT(R) (((R) > 0) ? ((R) - MTK_AFE_BASE) : 0)
Copy link
Contributor

@andyross andyross Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW: this isn't the AFE driver's struct though, it's the Zephyr-side intermediate. I chose -1 because in a few spots it looked like a zero had valid semantic meaning. But I was doing the port blind.

Copy link
Contributor Author

@andrew-mtk andrew-mtk Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andyross , I've tried to avoid AFE refactoring that were crashing with a memory violation exception when reg_ofs_base_msb and reg_ofs_end_msb were set to -1

sof/src/platform/mtk/dai.c

Lines 92 to 94 in 800f6ce

dst->reg_ofs_base_msb = REGCVT(src->base.hi);
dst->reg_ofs_cur_msb = REGCVT(src->cur.hi);
dst->reg_ofs_end_msb = REGCVT(src->end.hi);

sof/src/drivers/mediatek/afe/afe-drv.c

Lines 179 to 183 in 800f6ce

/* set start, end, upper 32 bits */
if (memif->data->reg_ofs_base_msb) {
afe_reg_write(afe, memif->data->reg_ofs_base_msb, phys_buf_addr_upper_32);
afe_reg_write(afe, memif->data->reg_ofs_end_msb, phys_buf_addr_upper_32);
}


#define COPYBIT(S, Dr, Ds) do { \
dst->Dr = REGCVT(src->S.reg); \
Expand Down Expand Up @@ -199,10 +208,68 @@ static const struct dai_info mtk_dai_info = {
.num_dai_types = ARRAY_SIZE(mtk_dai_types),
};

#if defined(CONFIG_SOC_SERIES_MT818X) || defined(CONFIG_SOC_MT8195)
static unsigned int mtk_afe2adsp_addr(unsigned int addr)
{
/* CPU -> ADSP address remap */
if ((addr >= SRAM_CPU_START) && (addr < SRAM_CPU_END)) {
addr = SRAM_ADSP_START + (addr - SRAM_CPU_START);
}

return addr;
}

static unsigned int mtk_adsp2afe_addr(unsigned int addr)
{
/* ADSP -> CPU address remap */
if ((addr >= SRAM_ADSP_START) && (addr < SRAM_ADSP_END)) {
addr = SRAM_CPU_START + (addr - SRAM_ADSP_START);
}

return addr;
}
#endif

/* Static table of fs register values. TODO: binary search */
static unsigned int mtk_afe_fs_timing(unsigned int rate)
{
static const struct { int hz, reg; } rate2reg[] = {
#if defined(CONFIG_SOC_MT8188) || defined(CONFIG_SOC_MT8195)
{ 7350, 16 },
{ 8000, 0 },
{ 11025, 17 },
{ 12000, 1 },
{ 14700, 18 },
{ 16000, 2 },
{ 22050, 19 },
{ 24000, 3 },
{ 29400, 20 },
{ 32000, 4 },
{ 44100, 21 },
{ 48000, 5 },
{ 88200, 22 },
{ 96000, 6 },
{ 176400, 23 },
{ 192000, 7 },
{ 352800, 24 },
{ 384000, 8 },
Copy link
Contributor

@andyross andyross Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As these tables grow, it's probably best to migrate them to DTS, or to some better-abstracted representation (like, it looks like most of the divisor values are the same between platforms, but some only have a subset of the available frequencies?)

#elif defined(CONFIG_SOC_MT8186)
{ 8000, 0 },
{ 11025, 1 },
{ 12000, 2 },
{ 16000, 4 },
{ 22050, 5 },
{ 24000, 6 },
{ 32000, 8 },
{ 44100, 9 },
{ 48000, 10 },
{ 88200, 11 },
{ 96000, 12 },
{ 176400, 13 },
{ 192000, 14 },
{ 352800, 7 },
{ 384000, 3 },
#else
{ 8000, 0 },
{ 11025, 1 },
{ 12000, 2 },
Expand All @@ -218,6 +285,7 @@ static unsigned int mtk_afe_fs_timing(unsigned int rate)
{ 192000, 18 },
{ 352800, 21 },
{ 384000, 22 },
#endif
};

for (int i = 0; i < ARRAY_SIZE(rate2reg); i++)
Expand All @@ -242,6 +310,10 @@ struct mtk_base_afe_platform mtk_afe_platform = {
.dais_size = ARRAY_SIZE(mtk_dais),
.afe_fs = mtk_afe_fs,
.irq_fs = mtk_afe_fs_timing,
#if defined(CONFIG_SOC_SERIES_MT818X) || defined(CONFIG_SOC_MT8195)
.afe2adsp_addr = mtk_afe2adsp_addr,
.adsp2afe_addr = mtk_adsp2afe_addr,
#endif
};

int mtk_dai_init(struct sof *sof)
Expand Down
20 changes: 20 additions & 0 deletions zephyr/include/sof/lib/dma.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,11 @@ enum sof_dma_cb_status {
/* Attributes have been ported to Zephyr. This condition is necessary until full support of
* CONFIG_SOF_ZEPHYR_STRICT_HEADERS.
*/
#ifdef CONFIG_ZEPHYR_NATIVE_DRIVERS
Copy link
Contributor

@andyross andyross Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems confusing. This header shouldn't be included if the SOF drivers are in use, no? I suspect the real bug here is upstream header hygiene, some include or another should probably be guarded.

Copy link
Collaborator

@kv2019i kv2019i Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andyross This is ok. We've done a flurry of changes to drop XTOS support at start of the SOF 2.14 cycle and commit ee58c00 went too far.

This is still overloaded in sof/lib/dma.h, but we'll need to keep the ifdefs until everybody has moved to native drivers.

struct sof_dma;
#else
struct dma;
#endif

/**
* \brief Element of SG list (as array item).
Expand Down Expand Up @@ -194,7 +198,11 @@ struct dma_plat_data {
uint32_t period_count;
};

#ifdef CONFIG_ZEPHYR_NATIVE_DRIVERS
struct sof_dma {
#else
struct dma {
#endif
struct dma_plat_data plat_data;
struct k_spinlock lock; /**< locking mechanism */
int sref; /**< simple ref counter, guarded by lock */
Expand All @@ -206,7 +214,11 @@ struct sof_dma {
};

struct dma_chan_data {
#ifdef CONFIG_ZEPHYR_NATIVE_DRIVERS
struct sof_dma *dma;
#else
struct dma *dma;
#endif

uint32_t status;
uint32_t direction;
Expand All @@ -224,14 +236,22 @@ struct dma_chan_data {
};

struct dma_info {
#ifdef CONFIG_ZEPHYR_NATIVE_DRIVERS
struct sof_dma *dma_array;
#else
struct dma *dma_array;
#endif
size_t num_dmas;
};

/* generic DMA DSP <-> Host copier */
struct dma_copy {
struct dma_chan_data *chan;
#ifdef CONFIG_ZEPHYR_NATIVE_DRIVERS
struct sof_dma *dmac;
#else
struct dma *dmac;
#endif
};

struct audio_stream;
Expand Down
6 changes: 6 additions & 0 deletions zephyr/lib/alloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,13 @@ extern char _mtk_adsp_sram_end;
#define SRAM_SIZE DT_REG_SIZE(DT_NODELABEL(sram0))
#define SRAM_END (SRAM_START + SRAM_SIZE)
#define heapmem ((uint8_t *)ALIGN_UP((uintptr_t)&_mtk_adsp_sram_end, PLATFORM_DCACHE_ALIGN))

/* Heap size is limited to 0x7fffU chunk units when CONFIG_SYS_HEAP_SMALL_ONLY is set */
#if defined(CONFIG_SYS_HEAP_SMALL_ONLY)
Copy link
Contributor

@andyross andyross Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly not sure why SOF is setting this. It's a code size thing, intended to reduce .text footprint on devices whose RAM heaps will never be beyond 256kb. But SOF is actually a really large Zephyr app. Probably best to just turn it off.

But doing the clamp here just for safety isn't wrong either, and it allows cleaner use of the suffix of the region for non-heap things.

#define HEAPMEM_SIZE MIN(((uint8_t *)SRAM_END - heapmem), 0x7fff * 8)
#else
#define HEAPMEM_SIZE ((uint8_t *)SRAM_END - heapmem)
#endif /* CONFIG_SYS_HEAP_SMALL_ONLY */

#else

Expand Down
Loading