[FEATURE] stop using the internal kernel API on the SOF side when enabling secondary cores.

[tmleman]

SOF should use the public zephyr API for power management of CPU cores. The currently used functions are internal kernel functions and should not be used from within the application. The code in file zephyr/lib/cpu.c needs to be refactored.

• soc: intel_adsp: call framework callback function for restore zephyrproject-rtos/zephyr#65394

• kernel: introduce k_smp_cpu_start() and k_smp_cpu_resume() zephyrproject-rtos/zephyr#64755

• zephyr: use k_smp_cpu_start()/k_smp_cpu_resume() for secondary core power up zephyrproject-rtos/sof#32

EDIT updated 4th Dec with latest tracking.

[kv2019i]

FYI @nashif

[lgirdwood]

@tmleman @kv2019i ping, any update ?

[RanderWang]

@lgirdwood @kv2019i @tmleman I investigate this issue. Let's first review our last state:

kv2019i Apr 28, 2023

Given this is an open-coded of z_smp_start_cpu() (see comment above), I don't fully understand why we have a different implementation here, and if this is correct, why this is not needed in the Zephyr kernel/smp.c version. Wouldn't the same problem be there? Or is this specific to a configuration where state is saved to persistant memory? If yes, this would need to be ifdef'ed out so this applies only to applicable platforms.

tmleman May 8, 2023

I don't know how zephyr is handling it but right now it's not working properly. After secondary core exits D3 it will repeat the path that led him to this state. Power will not be shut off so eventually it will return to the Idle state but this is not correct behavior. Maybe zephyr doesn't support situation in which each core can be turned off and on multiple times.

My conclusion:
we define a private version of z_smp_start_cpu for two reasons:
(1) we need a point to initialize sof environment for secondary core after it boots up. Now we do it by our own callback function secondary_init which is called immediately after core boot up. For z_smp_start_cpu, it uses its own function: smp_init_top and we can't redefine it. Actually we can initialize sof environment in callback function :cpu_notify_state_entry or in primary core boot up stage since these initialization only use get_cpu_id() to init some data array then we can only init one time

(2) z_smp_start_cpu will initialized the cpu context for each core power-up stage, so our context save can't work. I agree with tmleman, current zephyr implementation doesn't support our design. But what's the gain of our design ? I find sys_cache_data_flush_and_invd_all and xthal_window_spill. these two operations maybe consume more time than skipping z_init_cpu. Booting from clean environment may be less bug. To use z_smp_start_cpu, we need to first disable context saving &restore for SOFT_OFF state and then do effort to debug it. It is very a struggling decision.

[RanderWang]

@dcpleung are you working on this feature ? (zephyrproject-rtos/zephyr#64755) If so, can you take over it ? Thanks!

[nashif]

@dcpleung are you working on this feature ? (zephyrproject-rtos/zephyr#64755) If so, can you take over it ? Thanks!

@RanderWang @dcpleung is working on the cleanup on the zephyr side of things and provided a patch to be applied on SOF: zephyrproject-rtos#32. So it would be great if you can collaborate on this and help test the change from an SOF perspective.

[dcpleung]

I don't have a test rig to verify that the changes work correctly on all supported platforms, especially where secondary cores are going into power saving mode and then being brought back up. It would be great if you can test this.

[RanderWang]

great! let's work together

[RanderWang]

latest status:
zephyrproject-rtos/zephyr#65394
zephyrproject-rtos/zephyr#64755
zephyrproject-rtos#32

[lgirdwood]

@kv2019i probably a v2.9 unless you think it can land soon ?

[kv2019i]

Ack, this was very close (the PR set passes most tests, but a few iterations are still needed). So I'll push to v2.9, we can consider a cherry-pick if needed.

[kv2019i]

This feature has been implemented with Zephyr parts merged and SOF parts in review (latest attempt #8823 ), but due to regressions elsewhere (tracked in #8818 ), the PRs have not been merged to SOF main and are not ready for SOF2.9. Moving to 2.10.

[kv2019i]

This is now done and merged to SOF main.