src/platform: Add "mtk" Zephyr platform for MediaTek DSPs #9755

andyross · 2024-12-20T20:32:39Z

Unify the MediaTek 8195/86/88 devices into a single Zephyr-based "mtk" platform.

Almost all the device config and core (interrupt, timer) drivers are in upstream Zephyr now. There's almost no per-device code left in SOF.
In fact it's really quite small. Just two sub-300-line C files and some mostly boilerplate headers.
This still builds with ZEPHYR_NATIVE_DRIVERS=n. The pre-existing dummy-dma and afe drivers are used unchanged. The former has an equivalent in upstream Zephyr. The latter will need a port.
The AFE driver is "half Zephyr" though. The C code remains a legacy SOF driver but all the config is source from Zephyr DTS and converted at runtime to the legacy struct.
It's still a IPC3 platform. Kernel support for IPC4 doesn't exist yet.

This PR depends for build and functionality on the following Zephyr PRs, don't merge until they do:

zephyrproject-rtos/zephyr#82993
zephyrproject-rtos/zephyr#83291
zephyrproject-rtos/zephyr#83292

Also open source toolchains for these platforms have been merged to Zephyr sdk-ng, but are not part of a release yet. So (other than 8195, which is already present) development is limited to Cadence tooling for the moment.

andyross · 2024-12-20T20:37:48Z

Also full disclosure: almost all the validation on this code is happening on an unreleased platform. Support for the legacy devices is happening on a time-available basis, and I'm just one guy. :) Right now all build and run Zephyr tests. But 8195 is running a very stale kernel and I can't use factory images, I've managed to brick my 8186 (can probably get it working but have to open it and pull the battery), and 8188 is having some topology trouble even with the old binaries I need to figure out (likely missing a patch or three from a release branch?). But on the focus device it's working great and was playing bad Christmas music for me all morning.

andyross · 2024-12-21T02:13:09Z

Oh, and the checkpatch error[1] is a false positive. It complains about the lack of a space after a comma at the end of this code generation macro. But I add it it complains about a space before a closing paren, heh. Macrobatics confuses everyone, heh.

ERROR: space required after that ',' (ctx:VxB)
#308: FILE: src/platform/mtk/dai.c:115:
+	IF_ENABLED(DT_NODE_HAS_PROP(n, prop), (.prop = DT_PROP(n, prop),))

[1] Lost in a sea of codespell warnings that "AFE" must be a misspelled "SAFE". Alas I don't get to rename the hardware.

eddy1021

some nits

app/boards/mt8186_mt8186_adsp.conf

scripts/xtensa-build-zephyr.py

src/platform/mtk/dai.c

src/platform/mtk/platform.c

lyakh · 2024-12-23T07:38:52Z

src/platform/mtk/tools/mt-dai-gen.c

+#include <stdio.h>
+
+/* DIY assertion, an "assert()" is already defined in platform headers */
+#define CHECK(expr) while (!(expr)) { \


I guess if would do just as well. Usually while is used as #define x do {printf("fail");} while (0) to accept the closing semicolon without side effects, but while () {} doesn't help for that

lyakh · 2024-12-23T07:46:43Z

src/platform/mtk/tools/mt-dai-gen.c

+ *
+ * ch_num is skipped if the stored reg value is negative
+ * quad_ch is skipped if the mask is zero
+ * int_odd: <=0 reg


sorry, not sure, what these mean - did you mean reg <= 0?

Heh. It's clearer if you read it as English and not C: "<=0" is shorthand for an adjectival phrase ("less than or equal to zero") modifying "reg". :)

Really this is just me taking notes while reverse engineering the configuration mechanisms and committing the result without editting. Swapped the order, but the text isn't a spec and the original is weird, so I doubt this will ever be obvious.

lyakh · 2024-12-23T07:47:59Z

src/platform/mtk/tools/mt-dai-gen.c

+
+	int bits = __builtin_ffs(lomask + 1) - 1;
+
+	CHECK(((lomask + 1) & lomask) == 0);       /* must be power of two */


this checks, that lomask + 1 is a power of two?

Yes. Confusing comment. It's validating the the mask value is a proper right-aligned bitmask without holes, e.g. "0xff" is legal, "0xff0" is not.

lyakh · 2024-12-23T08:39:56Z

tools/rimage/config/mt8196.toml

+type = "SRAM"
+base = "0x90000000"
+size = "0x00600000"
+host_offset = "0x0"


in fact we can now preprocess toml with gcc, as we do for Intal ADSP platforms, so getting values from DTS might be possible?

Yeah. Though this particular block (not all the module config) is almost entirely generatable. I guess my gut says what we want is to construct and emit it ourselves from cmake (or a script invoked from there) instead of trying to do it with the preprocessor. But that takes some thought and design.

I've made the point in the past, but SOF seems to have a love affair with weak programming languages (cpp here, also m4, the topology2 changes to alsaconf, frankly alsaconf itself seems to have always been too limited pre-sof, alsabat is sort of in that list too) and then getting in trouble when the requirements expand farther than the syntax can handle. There's no shame in using python from the start. :)

lyakh · 2024-12-23T08:42:30Z

src/platform/mtk/dai.c

+// Author: Andy Ross <[email protected]>
+#include <sof/lib/dai-legacy.h>
+#include <ipc/dai.h>
+#include <sof/drivers/afe-drv.h>


this isn't yet replacing src/platform/mt81*/lib/dai.c, right? But eventually it will?

It is replacing the platform layers. It's still using the legacy driver code in src/drivers/mediatek/afe, but configuring it from DTS.

@andyross but that file isn't deleted yet? Supposedly because both XTOS and Zephyr builds should now be possible

Early versions of the AFE driver were published with large C struct definitions tied to platform headers. Zephyr strongly prefers Devicetree for configuration and not C code. So this is a somewhat klugey C program that builds and runs the AFE platform code on a Linux host CPU, producing valid DTS output that can be included in a board devicetree file in Zephyr. Just run ./build.sh from inside this directory. The only required host software is a working gcc that supports the "-m32" flag. The resulting afe-<platform>.dts files can be included directly in Zephyr board config. Signed-off-by: Andy Ross <[email protected]>

When building under Zephyr, you have to have a log module instantiated (or declare someone else's) if you want to use the component logging macros. Signed-off-by: Andy Ross <[email protected]>

Unify the MediaTek 8195/86/88 devices into a single Zephyr-based "mtk" platform. + Almost all the device config and core (interrupt, timer) drivers are in upstream Zephyr now. There's almost no per-device code left in SOF. + In fact it's really quite small. Just two sub-300-line C files and some mostly boilerplate headers. + This still builds with ZEPHYR_NATIVE_DRIVERS=n. The pre-existing dummy-dma and afe drivers are used unchanged. The former has an equivalent in upstream Zephyr. The latter will need a port. + The AFE driver is "half Zephyr" though. The C code remains a legacy SOF driver but all the config is source from Zephyr DTS and converted at runtime to the legacy struct. + It's still a IPC3 platform. Kernel support for IPC4 doesn't exist yet. Signed-off-by: Andy Ross <[email protected]>

Platform-layer support for the MT8196 Audio DSP. Virtually all the device-dependence is now provided by Zephyr upstream, this is just some boilerplate in a few areas, plus one legacy/to-be-removed MMIO address which isn't captured there. Signed-off-by: Andy Ross <[email protected]>

Add initial M4 topology for the mt8196 DSP Originally by Darren Ye <[email protected]> Signed-off-by: Andy Ross <[email protected]>

Simple address-space-only rimage config. Really it would be nice to source the data here from Zephyr DTS instead of recapitulating numbers we already store elsewhere. Originally by Darren Ye <[email protected]> Signed-off-by: Andy Ross <[email protected]>

kv2019i

Looks great! Some minor notes on possible further simplifications to platform implementation (that may not be possible yet if XTOS drivers are used)

kv2019i · 2024-12-27T11:47:17Z

app/boards/mt8188_mt8188_adsp.conf

+# "MTK" is an member), it can't be selected automatically from other
+# kconfigs, nor expressed as a default.  Don't put anything else here.
+# Board-level config goes in Zephyr (and ideally in DTS).  App-level
+# config goes in prj.conf.


Just a note it is accepted practise use to put vendor specific app settings in these board files. E.g. Intel example #9609 . I do agree anything that can be put in Zephyr board files/DTS or the app prj.conf, should not be put here.

kv2019i · 2024-12-27T11:48:25Z

src/platform/Kconfig

+	select SCHEDULE_DMA_MULTI_CHANNEL
+	select HOST_PTABLE
+	help
+	  Select if your target is a Mediatek DSP.  Builds Zephyr firmware.


This will likely conflict with #9758 . Not a big issue, just need one to go first and then rebase...

kv2019i · 2024-12-27T11:57:42Z

src/platform/mtk/include/platform/lib/clk.h

+#define CLK_CPU(x) (x)
+
+// FIXME: set correctly from mtk_adsp layer!
+#define CLK_MAX_CPU_HZ CONFIG_XTENSA_CCOUNT_HZ


Some related effort to drop these definitions on SOF side #9541
This (CLK_MAX_CPU_HZ) should not be needed in generic SOF code anymore when built for Zephyr (XTOS builds may still needed if used in XTOS drivers).

kv2019i · 2024-12-27T11:58:46Z

src/platform/mtk/include/platform/lib/pm_runtime.h

+
+struct pm_runtime_data;
+
+static inline void platform_pm_runtime_init(struct pm_runtime_data *prd)


With #9495 , lib/pm_runtime.h is no longer needd for Zephyr SOF builds.

kv2019i · 2024-12-27T12:00:00Z

src/platform/mtk/include/platform/lib/dai.h

+ */
+#ifndef _SOF_PLATFORM_MTK_LIB_DAI_H
+#define _SOF_PLATFORM_MTK_LIB_DAI_H
+


With #9653 , this can be dropped if only supporting Zephyr builds.

lyakh · 2025-01-02T07:29:15Z

src/platform/mtk/dai.c

+// Author: Andy Ross <[email protected]>
+#include <sof/lib/dai-legacy.h>
+#include <ipc/dai.h>
+#include <sof/drivers/afe-drv.h>


@andyross but that file isn't deleted yet? Supposedly because both XTOS and Zephyr builds should now be possible

kv2019i · 2025-01-03T11:22:43Z

@andyross let us know if you want to merge this version.

lgirdwood

LGTM @andyross

lgirdwood · 2025-01-06T15:02:07Z

@andyross let us know if you want to merge this version.

Ack, we can take any incremental fixes as they land as we've lots of time now that stable has forked.

andyross · 2025-01-07T16:30:35Z

Yeah, let's merge this absent a -1. Note that it's still waiting on some Zephyr PRs though: zephyrproject-rtos/zephyr#83291 and zephyrproject-rtos/zephyr#83292 (though the second is non-fatal, just a performance fix), and open source builds will need to wait for the next Zephyr SDK release (or else be done with a hand-pulled pre-release of the toolchain).

andyross requested review from akloniex, dabekjakub, kv2019i, lyakh, iuliana-prodan, dbaluta, abonislawski, ranj063, lgirdwood, plbossart, mmaka1 and lbetlej as code owners December 20, 2024 20:32

andyross requested review from eddy1021 and johnylin76 December 22, 2024 13:19

eddy1021 reviewed Dec 23, 2024

View reviewed changes

app/boards/mt8186_mt8186_adsp.conf Show resolved Hide resolved

scripts/xtensa-build-zephyr.py Show resolved Hide resolved

src/platform/mtk/dai.c Outdated Show resolved Hide resolved

src/platform/mtk/platform.c Outdated Show resolved Hide resolved

lyakh reviewed Dec 23, 2024

View reviewed changes

andyross added 6 commits December 24, 2024 05:40

drivers/mediatek/afe: Zephyr logging

3b6df4a

When building under Zephyr, you have to have a log module instantiated (or declare someone else's) if you want to use the component logging macros. Signed-off-by: Andy Ross <[email protected]>

tools/topology1: Add basic mt8196 topology

72861bf

Add initial M4 topology for the mt8196 DSP Originally by Darren Ye <[email protected]> Signed-off-by: Andy Ross <[email protected]>

andyross force-pushed the mtk-platform branch from 61b8a2e to 827ea54 Compare December 24, 2024 13:41

kv2019i mentioned this pull request Dec 27, 2024

Mediatek: mt8196: add platform/core_config/build_config #9758

Merged

kv2019i approved these changes Dec 27, 2024

View reviewed changes

lyakh approved these changes Jan 2, 2025

View reviewed changes

lgirdwood approved these changes Jan 6, 2025

View reviewed changes

lgirdwood merged commit c1df459 into thesofproject:main Jan 8, 2025
45 of 51 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src/platform: Add "mtk" Zephyr platform for MediaTek DSPs #9755

src/platform: Add "mtk" Zephyr platform for MediaTek DSPs #9755

andyross commented Dec 20, 2024

andyross commented Dec 20, 2024

andyross commented Dec 21, 2024

eddy1021 left a comment

lyakh Dec 23, 2024 •

edited

Loading

lyakh Dec 23, 2024

andyross Dec 24, 2024

lyakh Dec 23, 2024

andyross Dec 24, 2024

lyakh Dec 23, 2024

andyross Dec 24, 2024

lyakh Dec 23, 2024

andyross Dec 24, 2024

lyakh Jan 2, 2025

kv2019i left a comment

kv2019i Dec 27, 2024

kv2019i Dec 27, 2024

kv2019i Dec 27, 2024

kv2019i Dec 27, 2024

kv2019i Dec 27, 2024

lyakh Jan 2, 2025

kv2019i commented Jan 3, 2025

lgirdwood left a comment

lgirdwood commented Jan 6, 2025

andyross commented Jan 7, 2025


		int bits = __builtin_ffs(lomask + 1) - 1;

		CHECK(((lomask + 1) & lomask) == 0); /* must be power of two */


		struct pm_runtime_data;

		static inline void platform_pm_runtime_init(struct pm_runtime_data *prd)

src/platform: Add "mtk" Zephyr platform for MediaTek DSPs #9755

src/platform: Add "mtk" Zephyr platform for MediaTek DSPs #9755

Conversation

andyross commented Dec 20, 2024

andyross commented Dec 20, 2024

andyross commented Dec 21, 2024

eddy1021 left a comment

Choose a reason for hiding this comment

lyakh Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kv2019i left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kv2019i commented Jan 3, 2025

lgirdwood left a comment

Choose a reason for hiding this comment

lgirdwood commented Jan 6, 2025

andyross commented Jan 7, 2025

lyakh Dec 23, 2024 •

edited

Loading