From 4998c52ba8272061329648f95576bab59d68ae4f Mon Sep 17 00:00:00 2001 From: Flavio Ceolin Date: Sun, 14 Nov 2021 11:04:56 -0800 Subject: [PATCH] pm: Make pm_power_state_force multicore aware Change pm_power_state_force to receive which cpu the state should be forced. Also, it changed the API behavior to force the given state only when the idle thread for that core is executed. In a multicore environment force arbitrarily a core to suspend is not safe because the kernel cannot infer what that cpu is running and how it impacts the overall system, for example, if it is holding a lock that is required by a thread that is running in another cpu. Signed-off-by: Flavio Ceolin --- include/pm/pm.h | 5 ++- samples/boards/nrf/system_off/src/main.c | 2 +- .../stm32/power_mgmt/stm32wb_ble/src/main.c | 2 +- .../ti/cc13x2_cc26x2/system_off/src/main.c | 2 +- subsys/pm/power.c | 45 +++++++------------ 5 files changed, 23 insertions(+), 33 deletions(-) diff --git a/include/pm/pm.h b/include/pm/pm.h index 5ace3e626c1..e4bdf73747e 100644 --- a/include/pm/pm.h +++ b/include/pm/pm.h @@ -69,14 +69,15 @@ struct pm_notifier { * @brief Force usage of given power state. * * This function overrides decision made by PM policy forcing - * usage of given power state immediately. + * usage of given power state upon next entry of the idle thread. * * @note This function can only run in thread context * + * @param cpu CPU index. * @param info Power state which should be used in the ongoing * suspend operation. */ -void pm_power_state_force(struct pm_state_info info); +bool pm_power_state_force(uint8_t cpu, struct pm_state_info info); /** * @brief Register a power management notifier diff --git a/samples/boards/nrf/system_off/src/main.c b/samples/boards/nrf/system_off/src/main.c index a73f946700a..684f35faa44 100644 --- a/samples/boards/nrf/system_off/src/main.c +++ b/samples/boards/nrf/system_off/src/main.c @@ -90,7 +90,7 @@ void main(void) * controlled delay. Here we need to override that, then * force entry to deep sleep on any delay. */ - pm_power_state_force((struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0}); + pm_power_state_force(0u, (struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0}); printk("ERROR: System off failed\n"); while (true) { diff --git a/samples/boards/stm32/power_mgmt/stm32wb_ble/src/main.c b/samples/boards/stm32/power_mgmt/stm32wb_ble/src/main.c index 1e1fd57ffc7..911c65b08bd 100644 --- a/samples/boards/stm32/power_mgmt/stm32wb_ble/src/main.c +++ b/samples/boards/stm32/power_mgmt/stm32wb_ble/src/main.c @@ -118,5 +118,5 @@ void main(void) printk("Device shutdown\n"); - pm_power_state_force((struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0}); + pm_power_state_force(0u, (struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0}); } diff --git a/samples/boards/ti/cc13x2_cc26x2/system_off/src/main.c b/samples/boards/ti/cc13x2_cc26x2/system_off/src/main.c index 72c8fb8b99e..0c3bdf270fe 100644 --- a/samples/boards/ti/cc13x2_cc26x2/system_off/src/main.c +++ b/samples/boards/ti/cc13x2_cc26x2/system_off/src/main.c @@ -67,7 +67,7 @@ void main(void) /* * Force the SOFT_OFF state. */ - pm_power_state_force((struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0}); + pm_power_state_force(0u, (struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0}); printk("ERROR: System off failed\n"); while (true) { diff --git a/subsys/pm/power.c b/subsys/pm/power.c index 54e75a61442..3e23deb464a 100644 --- a/subsys/pm/power.c +++ b/subsys/pm/power.c @@ -24,6 +24,8 @@ LOG_MODULE_REGISTER(pm, CONFIG_PM_LOG_LEVEL); static bool post_ops_done = true; static sys_slist_t pm_notifiers = SYS_SLIST_STATIC_INIT(&pm_notifiers); static struct pm_state_info z_power_states[CONFIG_MP_NUM_CPUS]; +/* bitmask to check if a power state was forced. */ +static ATOMIC_DEFINE(z_power_states_forced, CONFIG_MP_NUM_CPUS); #ifdef CONFIG_PM_DEVICE static atomic_t z_cpus_active = ATOMIC_INIT(CONFIG_MP_NUM_CPUS); #endif @@ -239,43 +241,24 @@ void pm_system_resume(void) pm_state_notify(false); z_power_states[id] = (struct pm_state_info){PM_STATE_ACTIVE, 0, 0}; + atomic_clear_bit(z_power_states_forced, id); } } -void pm_power_state_force(struct pm_state_info info) +bool pm_power_state_force(uint8_t cpu, struct pm_state_info info) { - uint8_t id = _current_cpu->id; + bool ret = false; __ASSERT(info.state < PM_STATES_LEN, "Invalid power state %d!", info.state); - if (info.state == PM_STATE_ACTIVE) { - return; + + if (!atomic_test_and_set_bit(z_power_states_forced, cpu)) { + z_power_states[cpu] = info; + ret = true; } - (void)arch_irq_lock(); - z_power_states[id] = info; - -#ifdef CONFIG_PM_DEVICE - if (z_power_states[id].state != PM_STATE_RUNTIME_IDLE) { - (void)atomic_sub(&z_cpus_active, 1); - } -#endif - - post_ops_done = false; - pm_state_notify(true); - - k_sched_lock(); - pm_start_timer(); - /* Enter power state */ - pm_state_set(info); - pm_stop_timer(); - - pm_system_resume(); -#ifdef CONFIG_PM_DEVICE - (void)atomic_add(&z_cpus_active, 1); -#endif - k_sched_unlock(); + return ret; } bool pm_system_suspend(int32_t ticks) @@ -283,11 +266,16 @@ bool pm_system_suspend(int32_t ticks) uint8_t id = _current_cpu->id; SYS_PORT_TRACING_FUNC_ENTER(pm, system_suspend, ticks); - z_power_states[id] = pm_policy_next_state(id, ticks); + + if (!atomic_test_and_set_bit(z_power_states_forced, id)) { + z_power_states[id] = pm_policy_next_state(id, ticks); + } + if (z_power_states[id].state == PM_STATE_ACTIVE) { LOG_DBG("No PM operations done."); SYS_PORT_TRACING_FUNC_EXIT(pm, system_suspend, ticks, z_power_states[id].state); + atomic_clear_bit(z_power_states_forced, id); return false; } post_ops_done = false; @@ -320,6 +308,7 @@ bool pm_system_suspend(int32_t ticks) (void)atomic_add(&z_cpus_active, 1); SYS_PORT_TRACING_FUNC_EXIT(pm, system_suspend, ticks, _handle_device_abort(z_power_states[id])); + atomic_clear_bit(z_power_states_forced, id); return false; } }