Add mt8195 platform specific changes #4725

kuanhsuncheng · 2021-09-07T14:04:42Z

Hi sirs,

This is Allen Cheng from MediaTek.

There are some commits I don't put in the previous pull request.

Most of them will modify the common code in sof.

Some are hw restriction (compile error) and feature(mpu) when we develop mt8195 in chromebook.

If there is any comments, please let me know.

Thanks.

lgirdwood

I think we just need the Kconfig selection of exclusive as this looks like its showing lots of red in CI (probably because it's not cache coherent like S32CLI on Intel DSPs).

src/arch/xtensa/include/arch/atomic.h

kuanhsuncheng

Hi I add CONFIG_XTENSA_EXCLUSIVE in kconfig,

let me know if there is anything needs to be changed, thanks.

src/arch/xtensa/include/arch/atomic.h

kuanhsuncheng · 2021-09-08T07:19:55Z

src/arch/xtensa/include/arch/atomic.h

+	*a += value;
+	return *a;
+#else
+	#error No atomic methods available.


Hi @lgirdwood ,

Do I need to add XCHAL_HAVE_S32C1I?

if we use gcc to compile, it seems compiler will go to "#error No atomic methods available".

Yes. we need

#if CONFIG_LOCK_EXCLUSIVE && __XCC__ && XCHAL_EXCLUSIVE /* do this */ #elif XCHAL_S32C1I /* do that */ #elif CONFIG_NUM_CORES == 1 /* do something else */ #else #error No atomic support for multicore system #endif

See the CI build

warning: the int symbol CORE_COUNT (defined at src/platform/Kconfig:272) has a non-int default MP_NUM_CPUS (undefined) -- MEU_OFFSET=1344 -- Configuring done -- Generating done -- Build files have been written to: /srv/home/jenkins/workspace/sof_generic_build/build_tgl_xcc [ 0%] Built target genconfig -- from /srv/home/jenkins/workspace/sof_generic_build/src/include/sof/lib/dma.h:20, from /srv/home/jenkins/workspace/sof_generic_build/src/platform/intel/cavs/include/cavs/drivers/dw-dma.h:14, from /srv/home/jenkins/workspace/sof_generic_build/src/platform/tigerlake/include/platform/drivers/dw-dma.h:13, from /srv/home/jenkins/workspace/sof_generic_build/src/include/sof/drivers/dw-dma.h:12, from /srv/home/jenkins/workspace/sof_generic_build/src/platform/intel/cavs/lib/dma.c:11: /srv/home/jenkins/workspace/sof_generic_build/src/arch/xtensa/include/arch/atomic.h:69:3: error: #error No atomic methods available. /srv/home/jenkins/workspace/sof_generic_build/src/arch/xtensa/include/arch/atomic.h:106:3: error: #error No atomic methods available. [ 45%] Building C object CMakeFiles/sof.dir/src/platform/intel/cavs/lib/mem_window.c.o [ 45%] Building C object CMakeFiles/sof.dir/src/platform/intel/cavs/platform.c.o make[3]: *** [CMakeFiles/sof.dir/build.make:118: CMakeFiles/sof.dir/src/platform/intel/cavs/lib/dma.c.o] Error 2

Hi Liam,

For gcc-build, there is no XCHAL_HAVE_EXCLUSIVE and XCHAL_HAVE_S32C1I used,

I wonder how it handles those cases before.

That's a good point - then I think its best if we have for atomic.h and spinlock.h a set of functions for EXCLUSIVE and the existing set of functions for S32C11I and a set of functions for UP and non atomic ISA. e.g.

#if XCHAL_HAVE_EXCLUSIVE && CONFIG_XTENSA_EXCLUSIVE && __XCC__ /* All EXCLUSIVE methods here */ static inline int32_t arch_atomic_add(atomic_t *a, int32_t value) { /*reference xtos : xipc_misc.h*/ int32_t result = 0; int32_t current; while (!result) { current = XT_L32EX((int32_t *)a); result = current + value; XT_S32EX(result, (int32_t *)a); XT_GETEX(result); } return current; } #elif XCHAL_S32C1I /* all S32C1I methods here */ static inline int32_t arch_atomic_add(atomic_t *a, int32_t value) { int32_t result, current; __asm__ __volatile__( "1: l32i %1, %2, 0\n" " wsr %1, scompare1\n" " add %0, %1, %3\n" " s32c1i %0, %2, 0\n" " bne %0, %1, 1b\n" : "=&a" (result), "=&a" (current) : "a" (&a->value), "a" (value) : "memory"); return current; } #else /* The xtensa config has no atomic method ISA support - we can use normal integer arithmetic iff UP */ #if CONFIG_NUM_CORES > 1 #error No atomic ISA for SMP configuration #endif /* integer arithmetic methods here */ static inline int32_t arch_atomic_add(atomic_t *a, int32_t value) { *a += value; } #endif

This is simple do to in both files and wont break anything.

lgirdwood · 2021-09-09T08:47:21Z

@andyross fyi - for Zephyr support of xtensa EXCLUSIVE locking and MPU.

kuanhsuncheng

HI I found there is no CONFIG_NUM_CORES macro define.

so I use CONFIG_CORE_COUNT to check the current dsp core number.

And I am not sure the gcc case is ok or not.

let me know if there is anything needs to be changed, thanks.

kuanhsuncheng · 2021-09-09T08:51:35Z

src/arch/xtensa/include/arch/atomic.h

+	*a += value;
+	return *a;
+#else
+	#error No atomic methods available.


Hi Liam,

For gcc-build, there is no XCHAL_HAVE_EXCLUSIVE and XCHAL_HAVE_S32C1I used,

I wonder how it handles those cases before.

src/arch/xtensa/exc-dump.S

src/arch/xtensa/xtos/reset-vector.S

scripts/xtensa-build-all.sh

src/drivers/mediatek/mt8195/CMakeLists.txt

kuanhsuncheng

Update the new changes,

If there is any comments, please let me know.

Thanks.

dbaluta · 2021-09-09T14:10:40Z

We can immediately merge the following patches:

drivers: mtk: remove superfluous variable for mt8195
build: remove unused files from build system
Kconfig: add mt8195 to Kconfig
scripts: Add mt8195 to build script

if you move them to another PR and leave time for others to review the rest of the patches which look to be a little bit more delicate :)

lgirdwood

@kuanhsuncheng these are all good to merge except the atomic one as it needs an update.

lgirdwood · 2021-09-09T14:53:08Z

src/arch/xtensa/include/arch/atomic.h

@@ -65,6 +105,39 @@ static inline int32_t arch_atomic_sub(atomic_t *a, int32_t value)
 	return current;
 }

+#else
+
+#if CONFIG_CORE_COUNT > 1 && __XCC__


We dont need the __XCC__ here as we are saying that the ISA has no multicore instructions and SMP has been selected. This is not compiler related.

lgirdwood · 2021-09-09T14:56:40Z

src/arch/xtensa/include/arch/spinlock.h

@@ -28,6 +28,18 @@ static inline void arch_spin_lock(spinlock_t *lock)
 {
 	uint32_t result;

+#if XCHAL_HAVE_EXCLUSIVE && CONFIG_XTENSA_EXCLUSIVE


I think it's cleaner if we do the same as atomic.h here i.e.

#if XCHAL_HAVE_EXCLUSIVE && CONFIG_XTENSA_EXCLUSIVE && __XCC__ /* all the EXCLUSIVE functions here */ #elif XCHAL_S321CI /* all the S32 funcs here */ #else /* error check for multicore */ /* all the integer functions here */ #endif

lgirdwood · 2021-09-09T14:59:10Z

src/arch/xtensa/include/arch/atomic.h

+#error No atomic ISA for SMP configuration
+
+#endif
+


Need to put a comment here (and a similar one in spinlock.h) that says
The ISA has no atomic operations so use integer arithmetic on uniprocessor systems. This helps support GCC and qemu emulation of certain targets.

lgirdwood · 2021-09-09T15:01:04Z

src/arch/xtensa/include/arch/atomic.h

+	current = arch_atomic_read(a);
+	result = current - value;
+	arch_atomic_set(a, value);
+	return result;


This should return current value, this way the caller know the value at the addition and the result (in a*).

lgirdwood · 2021-09-09T15:01:17Z

src/arch/xtensa/include/arch/atomic.h

+	current = arch_atomic_read(a);
+	result = current - value;
+	arch_atomic_set(a, value);
+	return result;


Should return current.

kuanhsuncheng · 2021-09-10T02:03:56Z

We can immediately merge the following patches:

drivers: mtk: remove superfluous variable for mt8195

build: remove unused files from build system

Kconfig: add mt8195 to Kconfig

scripts: Add mt8195 to build script

if you move them to another PR and leave time for others to review the rest of the patches which look to be a little bit more delicate :)

hi Daniel,

I agree that, it will be more clear for PR.

I create another PR for MT8196 build of sof.

#4741

For 4725, I leave some specific patches for mt8192.

Thanks for your comment.

This results in a compiler error when xtensa configuration has no IRQ level 5. Make it use core-isa.h. Error: invalid register 'EPC5' for 'rsr' instruction Error: invalid register 'EPS5' for 'rsr' instruction For mt8195, we don't support those instrucations Signed-off-by: YC Hung <[email protected]> Signed-off-by: Allen-KH Cheng <[email protected]>

Override the default MPU setup. This table matches the mt8195 memory map Add xtensa/mpuasm.h, for mpu_write_map opcode Signed-off-by: YC Hung <[email protected]> Signed-off-by: Allen-KH Cheng <[email protected]>

In mp_asm.S, if XCHAL_HAVE_EXCLUSIVE is defined, it will use exclusive instructions, else it will use s32c1i instructions. It supports S32C1I and exclusive instruction in xthal_compare_and_set() API. Refer to xtos-simc-mutex.c, xtos_mutex_p structure is similar to spinlock_t. For dsp design, we cannot use s32c1i intrcutions in mt8195. In order to not affect other platform, add CONFIG_XTENSA_EXCLUSIVE and __XCC__ compile options. Signed-off-by: YC Hung <[email protected]> Signed-off-by: Allen-KH Cheng <[email protected]>

kuanhsuncheng

Hi

I update the new version.

Add spin lock simple implement for for UP

Also move build script patch to other PR.

let me know if there is anything needs to be changed, thanks.

lgirdwood · 2021-09-10T16:09:29Z

CI showing unrelated timeouts on alsabat and suspend unrelated to this PR.

marc-hb · 2024-08-07T17:19:28Z

src/arch/xtensa/include/arch/spinlock.h

+	uint32_t result;
+
+	lock->lock = 0;
+	result = 1;


Unused result variable, removal submitted in #9351

kuanhsuncheng requested review from dbaluta, lbetlej, lgirdwood, marc-hb, mmaka1 and plbossart as code owners September 7, 2021 14:04

lgirdwood reviewed Sep 7, 2021

View reviewed changes

src/arch/xtensa/include/arch/atomic.h Show resolved Hide resolved

src/arch/xtensa/include/arch/atomic.h Outdated Show resolved Hide resolved

kuanhsuncheng force-pushed the mt8195-dev branch from faa3bf1 to d40e5e0 Compare September 8, 2021 07:06

kuanhsuncheng commented Sep 8, 2021

View reviewed changes

kuanhsuncheng force-pushed the mt8195-dev branch 5 times, most recently from d64e1c7 to 7d1cac9 Compare September 9, 2021 08:30

kuanhsuncheng commented Sep 9, 2021

View reviewed changes

dbaluta reviewed Sep 9, 2021

View reviewed changes

src/arch/xtensa/exc-dump.S Show resolved Hide resolved

src/arch/xtensa/xtos/reset-vector.S Show resolved Hide resolved

scripts/xtensa-build-all.sh Outdated Show resolved Hide resolved

src/drivers/mediatek/mt8195/CMakeLists.txt Outdated Show resolved Hide resolved

kuanhsuncheng force-pushed the mt8195-dev branch from 7d1cac9 to e35fbdc Compare September 9, 2021 13:08

kuanhsuncheng commented Sep 9, 2021

View reviewed changes

lgirdwood reviewed Sep 9, 2021

View reviewed changes

kuanhsuncheng added 2 commits September 10, 2021 13:40

xtensa: add mt8195 MPU setup

f80d344

Override the default MPU setup. This table matches the mt8195 memory map Add xtensa/mpuasm.h, for mpu_write_map opcode Signed-off-by: YC Hung <[email protected]> Signed-off-by: Allen-KH Cheng <[email protected]>

kuanhsuncheng force-pushed the mt8195-dev branch from e35fbdc to 6b9cd84 Compare September 10, 2021 05:48

kuanhsuncheng force-pushed the mt8195-dev branch from 6b9cd84 to 62e8dfe Compare September 10, 2021 06:15

kuanhsuncheng commented Sep 10, 2021

View reviewed changes

lgirdwood approved these changes Sep 10, 2021

View reviewed changes

lgirdwood merged commit 83093da into thesofproject:main Sep 10, 2021

kuanhsuncheng deleted the mt8195-dev branch September 13, 2021 09:09

marc-hb reviewed Aug 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mt8195 platform specific changes #4725

Add mt8195 platform specific changes #4725

kuanhsuncheng commented Sep 7, 2021

lgirdwood left a comment

kuanhsuncheng left a comment

kuanhsuncheng Sep 8, 2021 •

edited

Loading

lgirdwood Sep 8, 2021

lgirdwood Sep 8, 2021

kuanhsuncheng Sep 9, 2021

lgirdwood Sep 9, 2021 •

edited

Loading

lgirdwood commented Sep 9, 2021

kuanhsuncheng left a comment •

edited

Loading

kuanhsuncheng Sep 9, 2021

kuanhsuncheng left a comment

dbaluta commented Sep 9, 2021

lgirdwood left a comment

lgirdwood Sep 9, 2021

lgirdwood Sep 9, 2021

lgirdwood Sep 9, 2021

lgirdwood Sep 9, 2021

lgirdwood Sep 9, 2021

kuanhsuncheng commented Sep 10, 2021 •

edited

Loading

kuanhsuncheng left a comment

lgirdwood commented Sep 10, 2021

marc-hb Aug 7, 2024

Add mt8195 platform specific changes #4725

Add mt8195 platform specific changes #4725

Conversation

kuanhsuncheng commented Sep 7, 2021

lgirdwood left a comment

Choose a reason for hiding this comment

kuanhsuncheng left a comment

Choose a reason for hiding this comment

kuanhsuncheng Sep 8, 2021 • 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

lgirdwood Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

lgirdwood commented Sep 9, 2021

kuanhsuncheng left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuanhsuncheng left a comment

Choose a reason for hiding this comment

dbaluta commented Sep 9, 2021

lgirdwood 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

kuanhsuncheng commented Sep 10, 2021 • edited Loading

kuanhsuncheng left a comment

Choose a reason for hiding this comment

lgirdwood commented Sep 10, 2021

Choose a reason for hiding this comment

kuanhsuncheng Sep 8, 2021 •

edited

Loading

lgirdwood Sep 9, 2021 •

edited

Loading

kuanhsuncheng left a comment •

edited

Loading

kuanhsuncheng commented Sep 10, 2021 •

edited

Loading