Build SOF with zephyr on acp_6_0 platform #9578

DINESHKUMARK1 · 2024-10-15T09:34:32Z

Integrate Zephyr support into the SOF build for AMD platforms.

sofci · 2024-10-15T09:36:12Z

Can one of the admins verify this patch?

reply test this please to run this test once

lgirdwood · 2024-10-15T12:27:58Z

test this please

lgirdwood

@DINESHKUMARK1 I think this is almost ready to go, it does need proper formatting and descriptions for all commit messages though. I also assume you have some Zephyr upstream dependencies and there will be a west update to include those ?

src/drivers/amd/common/acp_dma.c

lyakh

empty commit "Update acp_dma.c?" Maybe file flags changed or something? As mentioned in one of the comments, please improve some of your "Update X" commit descriptions, remember that those commits will stay forever in git log

lyakh · 2024-10-16T07:53:00Z

scripts/xtensa-build-zephyr.py

@@ -168,6 +168,12 @@ class PlatformConfig:
 		"imx", "imx95_evk/mimx9596/m7/ddr",
 		"", "", "", ""
 	),
+	"acp_6_0" : PlatformConfig(


As you see above, the convention is to put a line like " platforms" for each vendor, please do the same for AMD to avoid it looking like an NXP platform

Since you have no toolchain yet, move this to extra_platform_configs = for now, it's not ready for platform_configs_all = yet.

lyakh · 2024-10-16T07:56:23Z

zephyr/CMakeLists.txt

+	)
+
+	# Zephyr DMA domain should only be used with zephyr_ll
+	if (NOT(CONFIG_DMA_DOMAIN))


nitpick: maybe easier to use a positive form if (CONFIG_DMA_DOMAIN) and swap the branches below

lyakh · 2024-10-16T07:58:42Z

src/platform/amd/rembrandt/platform.c

 static SHARED_DATA struct timer timer = {
 	.id = TIMER0,
 	.irq = IRQ_NUM_TIMER0,
-};
+	};


the brace placement was correct

lyakh · 2024-10-16T07:59:17Z

src/platform/amd/rembrandt/platform.c

 	sof->platform_timer = &timer;
 	sof->cpu_timers = &timer;
+#endif
+#ifdef __ZEPHYR__


#else and maybe swap to use a positive #ifdef __ZEPHYR__

lyakh · 2024-10-16T08:00:03Z

src/platform/amd/rembrandt/include/platform/lib/memory.h

@@ -172,6 +172,8 @@ static inline void *platform_rfree_prepare(void *ptr)

 #endif /* __PLATFORM_LIB_MEMORY_H__ */

+#endif /* __PLATFORM_LIB_MEMORY_H__ */


lyakh · 2024-10-16T08:02:34Z

src/drivers/amd/rembrandt/acp_dmic_dma.c

+						return 0;
+					tr_err(&acp_dmic_dma_rmb_tr, "timed out for dma start");
+					return -ETIME;
+				}


1 TAB fewer would make this hunk much smaller

missed this one? In fact, can you use sof_cycle_get_64()?

src/platform/amd/rembrandt/include/platform/lib/memory.h

lyakh · 2024-10-16T08:08:30Z

zephyr/include/rtos/interrupt.h

-	defined(CONFIG_ZEPHYR_POSIX) || (defined(CONFIG_IMX) && !defined(CONFIG_IMX8M)) \
-	|| defined(CONFIG_AMD)
+	defined(CONFIG_ZEPHYR_POSIX) || \
+	(defined(CONFIG_IMX) && !defined(CONFIG_IMX8M)) || defined(CONFIG_AMD)


please, make commit titles and preferably bodies a bit more descriptive, at least mention AMD and Zephyr

lyakh · 2024-10-16T08:09:03Z

zephyr/include/rtos/interrupt.h

@@ -50,7 +50,7 @@ static inline int interrupt_get_irq(unsigned int irq, const char *cascade)
 {
 #if defined(CONFIG_LIBRARY) || defined(CONFIG_ACE) || defined(CONFIG_CAVS) || \
 	defined(CONFIG_ZEPHYR_POSIX) || \
-	(defined(CONFIG_IMX) && !defined(CONFIG_IMX8M)) || defined(CONFIG_AMD)
+		(defined(CONFIG_IMX) && !defined(CONFIG_IMX8M)) || defined(CONFIG_AMD)


please drop this commit

kv2019i · 2024-10-17T12:45:11Z

I guess this depends on zephyrproject-rtos/zephyr#79796 , but I'm ok to merge separately given this is initial enabling and not covered by any public CI yet (so merging SOF PR first will not break anything).

src/drivers/amd/rembrandt/interrupt.c

marc-hb

Just FYI: you just sent a separate Github notification every time you replied "done". You can very easily batch these notifications with the "Start Review" green button:

https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers

That page has other, unrelated but fantastic advice.

lyakh

Thanks for addressing the comments! Maybe timer code could still be simplified a bit more

lyakh · 2024-10-18T07:29:48Z

src/drivers/amd/rembrandt/acp_dmic_dma.c

+						return 0;
+					tr_err(&acp_dmic_dma_rmb_tr, "timed out for dma start");
+					return -ETIME;
+				}


missed this one? In fact, can you use sof_cycle_get_64()?

Add config file for acp_6_0: amd_acp_6_0_adsp.conf. Signed-off-by: DineshKumar Kalva <[email protected]>

Add support to build sof for acp_6_0 platform. Signed-off-by: DineshKumar Kalva <[email protected]>

…nabling irqsteer with zephyr. For the functions platform_interrupt_clear and platform_interrupt_set, the existing definitions based on arch_interrupt_* do not compile with the xt-clang 2023 toolchain for acp_6_0. Use the Zephyr wrapper implementations for those for now. Signed-off-by: DineshKumar Kalva <[email protected]>

This commit introduces support in the CMakeLists.txt of Zephyr for building SOF for acp_6_0 platform. Signed-off-by: DineshKumar Kalva <[email protected]>

…tform There are a mapping between LOCAL and HOST on platform, then add macro local_to_host, host_to_local. DSP memory region has mapped to host memory region on acp_6_0. Then if we want access the address from host, need to convert the address. Signed-off-by: DineshKumar Kalva <[email protected]>

Replaced sof timer related functions with Zephyr alternative. Signed-off-by: DineshKumar Kalva <[email protected]>

add heap memory allocation for .heap_mem section. Signed-off-by: DineshKumar Kalva <[email protected]>

… zephyr. With Zephyr, SOF uses k_cycle_get_64() API in order to count clock cycles.this API have a 64bit counter (e.g acp_6_0 dsp integration only has xtensa timer which is limited to 32 bits). Signed-off-by: DineshKumar Kalva <[email protected]>

…tform. In order to avoid first level interrupt handling from wrapper.c second level interrupt handling go through interrupt.c define macros to rename the duplicated functions Signed-off-by: DineshKumar Kalva <[email protected]>

… build. Definitions of mm_heap.h interface are not needed in Zephyr builds, added a no-op version for Zephyr. Signed-off-by: DineshKumar Kalva <[email protected]>

Add acp_6_0 toml file to support sof-acp_6_0.ri binary build. Signed-off-by: DineshKumar Kalva <[email protected]>

DINESHKUMARK1 requested review from dabekjakub, kv2019i, lyakh, iuliana-prodan, dbaluta, abonislawski, pblaszko, marcinszkudlinski, LaurentiuM1234, bhiregoudar, sunilkumardommati, lgirdwood, plbossart, mmaka1 and lbetlej as code owners October 15, 2024 09:34

DINESHKUMARK1 changed the title ~~Integrate Zephyr support into the SOF build for AMD platforms.~~ Build SOF with zephyr on acp_6_0 platform Oct 15, 2024

lgirdwood reviewed Oct 15, 2024

View reviewed changes

src/drivers/amd/common/acp_dma.c Show resolved Hide resolved

lyakh reviewed Oct 16, 2024

View reviewed changes

DINESHKUMARK1 force-pushed the main branch 2 times, most recently from c103101 to 3b1de04 Compare October 17, 2024 09:28

kv2019i approved these changes Oct 17, 2024

View reviewed changes

dbaluta reviewed Oct 17, 2024

View reviewed changes

src/drivers/amd/rembrandt/interrupt.c Show resolved Hide resolved

DINESHKUMARK1 force-pushed the main branch from 3b1de04 to 402aaea Compare October 17, 2024 13:30

marc-hb reviewed Oct 17, 2024

View reviewed changes

lyakh reviewed Oct 18, 2024

View reviewed changes

DINESHKUMARK1 added 2 commits October 18, 2024 15:46

app: boards: Add amd_acp_6_0_adsp conf to support acp_6_0 config.

4c01052

Add config file for acp_6_0: amd_acp_6_0_adsp.conf. Signed-off-by: DineshKumar Kalva <[email protected]>

scripts: zephyr: Add support to build sof for acp_6_0 platform.

56d1a08

Add support to build sof for acp_6_0 platform. Signed-off-by: DineshKumar Kalva <[email protected]>

DINESHKUMARK1 added 5 commits October 18, 2024 15:47

zephyr: CMakeLists.txt: Add support for building SOF for acp_6_0.

23065b0

This commit introduces support in the CMakeLists.txt of Zephyr for building SOF for acp_6_0 platform. Signed-off-by: DineshKumar Kalva <[email protected]>

timer: Use Zephyr timer API

86d0ba7

Replaced sof timer related functions with Zephyr alternative. Signed-off-by: DineshKumar Kalva <[email protected]>

zephyr: alloc : add heap memory allocation for .heap_mem section.

39eac69

add heap memory allocation for .heap_mem section. Signed-off-by: DineshKumar Kalva <[email protected]>

DINESHKUMARK1 force-pushed the main branch from 402aaea to 3326b52 Compare October 18, 2024 10:22

DINESHKUMARK1 added 4 commits October 18, 2024 16:00

platform: Definitions of mm_heap.h interface are not needed in Zephyr…

1565b1a

… build. Definitions of mm_heap.h interface are not needed in Zephyr builds, added a no-op version for Zephyr. Signed-off-by: DineshKumar Kalva <[email protected]>

rimage: config: add acp_6_0 toml file to support acp_6_0 build.

04939c4

Add acp_6_0 toml file to support sof-acp_6_0.ri binary build. Signed-off-by: DineshKumar Kalva <[email protected]>

DINESHKUMARK1 force-pushed the main branch from 3326b52 to 04939c4 Compare October 18, 2024 10:30

lgirdwood approved these changes Oct 18, 2024

View reviewed changes

lgirdwood merged commit edd20ae into thesofproject:main Oct 18, 2024
43 of 49 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build SOF with zephyr on acp_6_0 platform #9578

Build SOF with zephyr on acp_6_0 platform #9578

DINESHKUMARK1 commented Oct 15, 2024 •

edited

Loading

sofci commented Oct 15, 2024

lgirdwood commented Oct 15, 2024

lgirdwood left a comment

lyakh left a comment

lyakh Oct 16, 2024

marc-hb Oct 16, 2024

DINESHKUMARK1 Oct 17, 2024

lyakh Oct 16, 2024

DINESHKUMARK1 Oct 17, 2024

lyakh Oct 16, 2024

DINESHKUMARK1 Oct 17, 2024

lyakh Oct 16, 2024

DINESHKUMARK1 Oct 17, 2024

lyakh Oct 16, 2024

DINESHKUMARK1 Oct 17, 2024

lyakh Oct 16, 2024 •

edited

Loading

lyakh Oct 18, 2024

lyakh Oct 16, 2024

lyakh Oct 16, 2024

kv2019i commented Oct 17, 2024

marc-hb left a comment

lyakh left a comment

lyakh Oct 18, 2024

		@@ -172,6 +172,8 @@ static inline void platform_rfree_prepare(void ptr)

		#endif /* __PLATFORM_LIB_MEMORY_H__ */

		#endif /* __PLATFORM_LIB_MEMORY_H__ */

Build SOF with zephyr on acp_6_0 platform #9578

Build SOF with zephyr on acp_6_0 platform #9578

Conversation

DINESHKUMARK1 commented Oct 15, 2024 • edited Loading

sofci commented Oct 15, 2024

lgirdwood commented Oct 15, 2024

lgirdwood left a comment

Choose a reason for hiding this comment

lyakh 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

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

lyakh Oct 16, 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

kv2019i commented Oct 17, 2024

marc-hb left a comment

Choose a reason for hiding this comment

lyakh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DINESHKUMARK1 commented Oct 15, 2024 •

edited

Loading

lyakh Oct 16, 2024 •

edited

Loading