[DNM]cpu: fix cpu_disable_core for primary core by ranj063 · Pull Request #10036 · thesofproject/sof

ranj063 · 2025-05-29T21:14:20Z

Nodify the logic for DSP D3 transition to invoke the sys_poweroff() call to speed up the transition by performing the suspend/disable actions in the IPC context instead of idle context. This results in a reduction of the D3 transition time from 100ms on the ACE1.5 based MTL laptop to ~1ms.

Depends on zephyr PR: zephyrproject-rtos/zephyr#90821

Copilot

Pull Request Overview

This PR updates the core disable logic to speed up the DSP D3 transition by performing suspend/disable actions in the IPC context and adjusts board configurations for proper poweroff functionality.

Updated cpu_disable_core() to call sys_poweroff() on the primary core.
Modified IPC powerdown sequence to suspend devices before disabling the core.
Added CONFIG_POWEROFF=y to multiple board configuration files.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
zephyr/lib/cpu.c	Updated core disable logic for primary core using sys_poweroff.
src/ipc/ipc-zephyr.c	Adjusted IPC shutdown sequence to trigger device suspend.
app/boards/intel_adsp_cavs25.conf	Enabled poweroff configuration.
app/boards/intel_adsp_ace30_wcl.conf	Enabled poweroff configuration.
app/boards/intel_adsp_ace30_ptl.conf	Enabled poweroff configuration.
app/boards/intel_adsp_ace20_lnl.conf	Enabled poweroff configuration.
app/boards/intel_adsp_ace15_mtpm.conf	Enabled poweroff configuration.

Copilot · 2025-05-29T21:14:59Z

+	/* Primary core will be turned off by the host. This does not return. */
+	if (cpu_is_primary(id)) {
+ cpu_notify_state_entry(PM_STATE_SOFT_OFF);
+ sys_poweroff();


Since sys_poweroff() is not expected to return, consider adding an unreachable marker (e.g., an assertion or a __builtin_unreachable()) immediately after the call to clarify the control flow in case it unexpectedly returns.
Suggested change
sys_poweroff();
sys_poweroff();
__builtin_unreachable();

@ranj063 another good point.

Copilot · 2025-05-29T21:14:59Z

- * IPC thread during idle.
- */
+ /* force device suspend */
+ ipc_device_suspend_handler(NULL, ipc);


The return value of ipc_device_suspend_handler() is not checked. Consider verifying the result and handling any errors to ensure device suspend completes successfully.
Suggested change
ipc_device_suspend_handler(NULL, ipc);
intret=ipc_device_suspend_handler(NULL, ipc);
if (ret<0) {
LOG_ERR("Device suspend failed with error: %d", ret);
/* Handle error appropriately, e.g., abort further operations */
returnSOF_TASK_STATE_ERROR;
}

@ranj063 can we log here ?

lgirdwood · 2025-05-30T09:05:15Z

@abonislawski pls take a look.

abonislawski · 2025-06-04T06:40:51Z

@abonislawski pls take a look.

Good direction and transition time reduction, AFAIK it will be more aligned with Intel reference now.

tmleman

Currently, we can utilize POWEROFF when CONFIG_ADSP_IMR_CONTEXT_SAVE is disabled. This approach makes sense, as it enables us to skip unnecessary power management operations, thus reducing the D3 entry time. However, it is crucial to separate these two cases in the code.

tmleman · 2025-06-05T12:13:07Z

+	if (cpu_is_primary(id)) {
+ cpu_notify_state_entry(PM_STATE_SOFT_OFF);
+ sys_poweroff();
+	}


This flow, in its current implementation, makes sense only in configurations with IMR context save disabled. The current context save implementation expects us to return to the function that was the entry point for power down. Therefore, in this case, we should return from sys_poweroff(). Consequently, we need to reinitialize all devices that were suspended and send a FW ready ipc, essentially implementing the following change:
--- a/zephyr/lib/cpu.c+++ b/zephyr/lib/cpu.c@@ -268,6 +268,8 @@ void cpu_disable_core(int id) if (cpu_is_primary(id)) { cpu_notify_state_entry(PM_STATE_SOFT_OFF); sys_poweroff(); + cpu_notify_state_exit(PM_STATE_SOFT_OFF);+ return; }

ranj063 · 2025-06-09T20:41:26Z

SOFCI TEST

ranj063 · 2025-06-09T20:51:23Z

SOFCI TEST

ranj063 · 2025-06-23T23:26:11Z

SOFCI TEST

ranj063 · 2025-06-24T14:52:04Z

SOFCI TEST

Nodify the logic for DSP D3 transition to invoke the sys_poweroff() call to speed up the transition by performing the suspend/disable actions in the IPC context instead of idle context. This results in a reduction of the D3 transition time from 100ms on the ACE1.5 based MTL laptop to ~1ms. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

test Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

lgirdwood

Some GH actions failing due to disk usage over 14G limit. Unrelated to this PR.

lgirdwood · 2025-06-25T11:59:06Z

- * IPC thread during idle.
- */
+ /* force device suspend */
+ ipc_device_suspend_handler(NULL, ipc);


@ranj063 can we log here ?

lgirdwood · 2025-06-25T11:59:43Z

+	/* Primary core will be turned off by the host. This does not return. */
+	if (cpu_is_primary(id)) {
+ cpu_notify_state_entry(PM_STATE_SOFT_OFF);
+ sys_poweroff();


@ranj063 another good point.

ranj063 · 2025-06-26T19:23:25Z

dropping this PR as #10028 reduces the D3 transition times to just about ~2ms without full context save/restore.

CopilotAI review requested due to automatic review settings May 29, 2025 21:14

ranj063 requested review from bardliao, dbaluta, kv2019i, lbetlej, lgirdwood, marcinszkudlinski, mmaka1, pblaszko, plbossart and tmleman as code owners May 29, 2025 21:14

CopilotAI reviewed May 29, 2025
View reviewed changes

lgirdwood reviewed May 30, 2025
View reviewed changes

Comment threadapp/boards/intel_adsp_ace15_mtpm.conf

ranj063 force-pushed the fix/d3_transition branch from 2d0bf26 to f3f65baCompare June 3, 2025 18:23

tmleman requested changes Jun 5, 2025
View reviewed changes

ranj063 force-pushed the fix/d3_transition branch from f3f65ba to d9d66abCompare June 9, 2025 20:43

ranj063 force-pushed the fix/d3_transition branch from d9d66ab to 2f856ffCompare June 9, 2025 20:56

ranj063 force-pushed the fix/d3_transition branch from 2f856ff to bade59dCompare June 23, 2025 21:05

ranj063 force-pushed the fix/d3_transition branch from bade59d to b5fadfaCompare June 23, 2025 23:27

ranj063 added 2 commits June 24, 2025 09:48

west.yml: Update zephyr to point to PR
9e14383
test Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/d3_transition branch from b5fadfa to 9e14383Compare June 24, 2025 16:48

lgirdwood reviewed Jun 25, 2025
View reviewed changes

ranj063 closed this Jun 26, 2025

-ipc_device_suspend_handler(NULL, ipc);
+intret=ipc_device_suspend_handler(NULL, ipc);
+if (ret<0) {
+LOG_ERR("Device suspend failed with error: %d", ret);
+/* Handle error appropriately, e.g., abort further operations */
+returnSOF_TASK_STATE_ERROR;
+ }

Uh oh!

Conversation

ranj063 commented May 29, 2025

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

CopilotAIMay 29, 2025

Choose a reason for hiding this comment

Uh oh!

lgirdwoodJun 25, 2025

Choose a reason for hiding this comment

Uh oh!

CopilotAIMay 29, 2025

Choose a reason for hiding this comment

Uh oh!

lgirdwoodJun 25, 2025

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented May 30, 2025

Uh oh!

Uh oh!

abonislawski commented Jun 4, 2025

Uh oh!

tmleman left a comment

Choose a reason for hiding this comment

Uh oh!

tmlemanJun 5, 2025

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Jun 9, 2025

Uh oh!

ranj063 commented Jun 9, 2025

Uh oh!

ranj063 commented Jun 23, 2025

Uh oh!

ranj063 commented Jun 24, 2025

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwoodJun 25, 2025

Choose a reason for hiding this comment

Uh oh!

lgirdwoodJun 25, 2025

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Jun 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants