Skip to content

[DNM]cpu: fix cpu_disable_core for primary core #10036

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ranj063 wants to merge 2 commits into from
Closed

[DNM]cpu: fix cpu_disable_core for primary core #10036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a4eefa
cpu: fix cpu_disable_core for primary core
ranj063 May 12, 2025
9e14383
west.yml: Update zephyr to point to PR
ranj063 Jun 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/boards/intel_adsp_ace15_mtpm.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ CONFIG_SYS_CLOCK_TICKS_PER_SEC=12000
CONFIG_ADSP_IDLE_CLOCK_GATING=y
CONFIG_ADSP_IMR_CONTEXT_SAVE=n
CONFIG_PM=y
CONFIG_POWEROFF=y
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_RUNTIME=y
CONFIG_PM_DEVICE_SYSTEM_MANAGED=y
Expand Down
1 change: 1 addition & 0 deletions app/boards/intel_adsp_ace20_lnl.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ CONFIG_SYS_CLOCK_TICKS_PER_SEC=12000
CONFIG_ADSP_IDLE_CLOCK_GATING=y
CONFIG_ADSP_IMR_CONTEXT_SAVE=y
CONFIG_PM=y
CONFIG_HIBERNATE=y
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_POWER_DOMAIN=y
CONFIG_PM_DEVICE_RUNTIME=y
Expand Down
1 change: 1 addition & 0 deletions app/boards/intel_adsp_ace30_ptl.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ CONFIG_SYS_CLOCK_TICKS_PER_SEC=12000
# Zephyr / power settings
CONFIG_ADSP_IMR_CONTEXT_SAVE=y
CONFIG_PM=y
CONFIG_HIBERNATE=y
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_RUNTIME=y
CONFIG_PM_DEVICE_POWER_DOMAIN=y
Expand Down
1 change: 1 addition & 0 deletions app/boards/intel_adsp_ace30_wcl.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ CONFIG_SYS_CLOCK_TICKS_PER_SEC=12000
# Zephyr / power settings
CONFIG_ADSP_IMR_CONTEXT_SAVE=y
CONFIG_PM=y
CONFIG_HIBERNATE=y
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_RUNTIME=y
CONFIG_PM_DEVICE_POWER_DOMAIN=y
Expand Down
1 change: 1 addition & 0 deletions app/boards/intel_adsp_cavs25.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ CONFIG_MM_DRV=y

# Zephyr / power settings
CONFIG_PM=y
CONFIG_POWEROFF=y
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_RUNTIME=y
CONFIG_PM_POLICY_CUSTOM=y
Expand Down
11 changes: 5 additions & 6 deletions src/ipc/ipc-zephyr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,11 @@ enum task_state ipc_platform_do_cmd(struct ipc *ipc)
if (ipc->task_mask & IPC_TASK_POWERDOWN ||
ipc_get()->pm_prepare_D3) {
#if defined(CONFIG_PM)
/**
* @note For primary core this function
* will only force set lower power state
* in power management settings.
* Core will enter D3 state after exit
* IPC thread during idle.
/* force device suspend */
ipc_device_suspend_handler(NULL, ipc);
Copy link

Copilot AI May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
int ret = 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 */
return SOF_TASK_STATE_ERROR;
}

Copilot uses AI. Check for mistakes.
Copy link
Member

@lgirdwood lgirdwood Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 can we log here ?

/*
* When context save is not enabled, this function will never return but if it is
* enabled, this function will return during context restore
*/
cpu_disable_core(PLATFORM_PRIMARY_CORE_ID);
#else
Expand Down
2 changes: 1 addition & 1 deletion west.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ manifest:

- name: zephyr
repo-path: zephyr
revision: 8c6b2c8c88773810d64a47ed3b6206b7d6e8290f
revision: pull/90821/head
remote: zephyrproject

# Import some projects listed in zephyr/west.yml@revision
Expand Down
19 changes: 17 additions & 2 deletions zephyr/lib/cpu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ static uint32_t mic_disable_status;
#include <zephyr/kernel/smp.h>
#include <zephyr/device.h>
#include <zephyr/drivers/mm/mm_drv_intel_adsp_mtl_tlb.h>
#include <zephyr/sys/poweroff.h>
#include <zephyr/sys/hibernate.h>

#if CONFIG_MULTICORE && CONFIG_SMP

Expand Down Expand Up @@ -255,16 +257,29 @@ void cpu_disable_core(int id)
tr_warn(&zephyr_tr, "core %d is already disabled", id);
return;
}

#if defined(CONFIG_PM)
/* TODO: before requesting core shut down check if it's not actively used */
if (!pm_state_force(id, &(struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0})) {
tr_err(&zephyr_tr, "failed to set PM_STATE_SOFT_OFF on core %d", id);
return;
}

/* Primary core will be turn off by the host after it enter SOFT_OFF state */
if (cpu_is_primary(id))
if (cpu_is_primary(id)) {
cpu_notify_state_entry(PM_STATE_SOFT_OFF);
#if defined(CONFIG_POWEROFF)
/* Primary core will be turned off by the host. This does not return. */
sys_poweroff();
Copy link

Copilot AI May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copilot uses AI. Check for mistakes.
Copy link
Member

@lgirdwood lgirdwood Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 another good point.

#elif defined(CONFIG_HIBERNATE)
/*
* Primary core will be turned off by the host. This function returns during
* context restore.
*/
sys_hibernate();
return;
#endif
}


/* Broadcasting interrupts to other cores. */
arch_sched_broadcast_ipi();
Expand Down
Loading