-
Notifications
You must be signed in to change notification settings - Fork 349
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.
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
Conversation
|
Can one of the admins verify this patch?
|
MTK AFE driver check registers for 0 instead of -1 when not in use. Signed-off-by: Andrew Perepech <andrew.perepech@mediatek.com>
Add sampling rate Hz to reg value conversion tables for MT8186, MT8188 and MT8195 Signed-off-by: Andrew Perepech <andrew.perepech@mediatek.com>
MTK platforms are built with CONFIG_SYS_HEAP_SMALL_ONLY. If heap size is greater than 0x7fff chunk units, it throws an assert at boot. Signed-off-by: Andrew Perepech <andrew.perepech@mediatek.com>
DMA buffer alignment for MT818X and MT8195 must be set to 0x40 Signed-off-by: Andrew Perepech <andrew.perepech@mediatek.com>
Add AFE buffer address translation between CPU and ADSP for MT818X and MT8195 platforms. AFE buffer is allocated in heap located in ADSP SRAM. SRAM base address is not the same for CPU and ADSP address space. AFE hardware accept buffer base address in CPU address space only. Signed-off-by: Andrew Perepech <andrew.perepech@mediatek.com>
Reinstate 'struct dma' data type for platforms compiled with CONFIG_ZEPHYR_NATIVE_DRIVERS undefined. Signed-off-by: Andrew Perepech <andrew.perepech@mediatek.com>
Override CONFIG_SYS_CLOCK_TICKS_PER_SEC project default setting so 1 millisecond is exactly represented by an integral number of ticks. This fixes warning: <wrn> dai_comp: comp:1.2 dai_common_copy(): nothing to copy Signed-off-by: Andrew Perepech <andrew.perepech@mediatek.com>
817c3a2 to
46515b0
Compare
|
Adding @andyross |
|
test this please |
andyross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes & nitpicks, but nothing fatal. So nice to see vendor support here!
| 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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); | |
| } |
| { 176400, 23 }, | ||
| { 192000, 7 }, | ||
| { 352800, 24 }, | ||
| { 384000, 8 }, |
There was a problem hiding this comment.
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?)
| #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) |
There was a problem hiding this comment.
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 SRAM_CPU_START 0x10800000 | ||
| #elif defined(CONFIG_SOC_SERIES_MT818X) | ||
| #define MTK_AFE_BASE 0x10b10000 | ||
| #define SRAM_CPU_START 0x10d00000 |
There was a problem hiding this comment.
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).
| /* Attributes have been ported to Zephyr. This condition is necessary until full support of | ||
| * CONFIG_SOF_ZEPHYR_STRICT_HEADERS. | ||
| */ | ||
| #ifdef CONFIG_ZEPHYR_NATIVE_DRIVERS |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /* Attributes have been ported to Zephyr. This condition is necessary until full support of | ||
| * CONFIG_SOF_ZEPHYR_STRICT_HEADERS. | ||
| */ | ||
| #ifdef CONFIG_ZEPHYR_NATIVE_DRIVERS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@lrudyX can you check, quickbuild seems to be stuck? |
|
@andyross, is there anything else needs to be done for this MR? I'm planning to submit MR that add support for Genio 350 platform that depend on this. |
|
Sorry for delay @andrew-mtk , it seems the Intel CI got stuck. Given this is so delayed, let me go and merge. |
Added a few fixes to existing Mediatek platforms in preparation to port MT8365 to Zephyr