From 0c836554b215464d1b96c5eca27fc58191ee365b Mon Sep 17 00:00:00 2001 From: Boyan Karatotev <boyan.karatotev@arm.com> Date: Mon, 30 Sep 2024 11:31:55 +0100 Subject: [PATCH] refactor(psci): don't use PSCI_INVALID_PWR_LVL to signal OFF state The target_pwrlvl field in the psci cpu data struct only stores the highest power domain that a CPU_SUSPEND call affected, and is used to resume those same domains on warm reset. If the cpu is otherwise OFF (never turned on or CPU_OFF), then this needs to be the highest power level because we don't know the highest level that will be off. So skip the invalidation and always keep the field to the maximum value. During suspend the field will be lowered to the appropriate value and then put back after wakeup. Also, do that in the suspend to standby path as well as it will have been written before the sleep and it might end up incorrect. Change-Id: I614272ec387e1d83023c94700780a0f538a9a6b6 Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com> --- include/lib/psci/psci.h | 2 +- lib/psci/psci_common.c | 6 +----- lib/psci/psci_setup.c | 6 +++--- lib/psci/psci_suspend.c | 9 ++++++--- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/include/lib/psci/psci.h b/include/lib/psci/psci.h index c40f955f0..f12a4d62c 100644 --- a/include/lib/psci/psci.h +++ b/include/lib/psci/psci.h @@ -302,7 +302,7 @@ typedef struct psci_cpu_data { /* * Highest power level which takes part in a power management - * operation. + * operation. May be lower while the core is in suspend state. */ unsigned int target_pwrlvl; diff --git a/lib/psci/psci_common.c b/lib/psci/psci_common.c index 3f6b89f2b..5c7993bf2 100644 --- a/lib/psci/psci_common.c +++ b/lib/psci/psci_common.c @@ -260,13 +260,9 @@ static unsigned int get_power_on_target_pwrlvl(void) /* * Assume that this cpu was suspended and retrieve its target power - * level. If it is invalid then it could only have been turned off - * earlier. PLAT_MAX_PWR_LVL will be the highest power level a - * cpu can be turned off to. + * level. If it wasn't, the cpu is off so this will be PLAT_MAX_PWR_LVL. */ pwrlvl = psci_get_suspend_pwrlvl(); - if (pwrlvl == PSCI_INVALID_PWR_LVL) - pwrlvl = PLAT_MAX_PWR_LVL; assert(pwrlvl < PSCI_INVALID_PWR_LVL); return pwrlvl; } diff --git a/lib/psci/psci_setup.c b/lib/psci/psci_setup.c index 6bf1ff456..1afc91808 100644 --- a/lib/psci/psci_setup.c +++ b/lib/psci/psci_setup.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2020, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2013-2024, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -68,8 +68,8 @@ static void __init psci_init_pwr_domain_node(uint16_t node_idx, /* Set the Affinity Info for the cores as OFF */ svc_cpu_data->aff_info_state = AFF_STATE_OFF; - /* Invalidate the suspend level for the cpu */ - svc_cpu_data->target_pwrlvl = PSCI_INVALID_PWR_LVL; + /* Default to the highest power level when the cpu is not suspending */ + svc_cpu_data->target_pwrlvl = PLAT_MAX_PWR_LVL; /* Set the power state to OFF state */ svc_cpu_data->local_state = PLAT_MAX_OFF_STATE; diff --git a/lib/psci/psci_suspend.c b/lib/psci/psci_suspend.c index cb12b8351..300466672 100644 --- a/lib/psci/psci_suspend.c +++ b/lib/psci/psci_suspend.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2022, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2013-2024, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -54,6 +54,9 @@ static void psci_suspend_to_standby_finisher(unsigned int cpu_idx, */ psci_plat_pm_ops->pwr_domain_suspend_finish(&state_info); + /* This loses its meaning when not suspending, reset so it's correct for OFF */ + psci_set_suspend_pwrlvl(PLAT_MAX_PWR_LVL); + /* * Set the requested and target state of this CPU and all the higher * power domain levels for this CPU to run. @@ -363,8 +366,8 @@ void psci_cpu_suspend_finish(unsigned int cpu_idx, const psci_power_state_t *sta psci_spd_pm->svc_suspend_finish(max_off_lvl); } - /* Invalidate the suspend level for the cpu */ - psci_set_suspend_pwrlvl(PSCI_INVALID_PWR_LVL); + /* This loses its meaning when not suspending, reset so it's correct for OFF */ + psci_set_suspend_pwrlvl(PLAT_MAX_PWR_LVL); PUBLISH_EVENT(psci_suspend_pwrdown_finish); }