[PATCH] cpuidle: psci: Assign domain callbacks to all CPU idle states

[Kevin Hilman]

Ulf Hansson <ulf.hansson at oss.qualcomm.com> writes:

> On Mon, Aug 31, 2026 at 8:52 PM Kevin Hilman (TI) <khilman at baylibre.com> wrote:
>>
>> Previously, only the deepest CPU idle state had its enter and
>> enter_s2idle callbacks set to the domain-aware implementations. This
>> meant that if a QoS latency constraint excluded the deepest state during
>> s2idle, find_deepest_state() would find no state with enter_s2idle set
>> and skip the domain idle path entirely. Similarly, during normal runtime
>> idle, shallower CPU idle states could not trigger cluster-level domain
>> idle states.
>>
>> Assign both enter_s2idle and enter (non-PREEMPT_RT) to all non-WFI CPU
>> idle states so that the domain-idle-state logic is triggered regardless
>> of which CPU idle state is selected. The genpd governor remains
>> responsible for honouring domain-level latency constraints independently.
>>
>> The enter_s2idle path uses dev_pm_genpd_suspend() which is safe on
>> PREEMPT_RT. The enter path uses pm_runtime_put_sync_suspend() which may
>> sleep and is therefore still excluded on PREEMPT_RT.
>>
>> Suggested-by: Scaria Kochidanadu <s-kochidanadu at ti.com>
>> Signed-off-by: Kevin Hilman (TI) <khilman at baylibre.com>
>> ---
>>  drivers/cpuidle/cpuidle-psci.c | 24 +++++++++++++++++-------
>>  1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
>> index dcf20ea5ef5e..db9aa57c51f5 100644
>> --- a/drivers/cpuidle/cpuidle-psci.c
>> +++ b/drivers/cpuidle/cpuidle-psci.c
>> @@ -250,6 +250,8 @@ static int psci_dt_cpu_init_topology(struct cpuidle_driver *drv,
>>                                      struct psci_cpuidle_data *data,
>>                                      unsigned int state_count, int cpu)
>>  {
>> +       int i;
>> +
>>         /* Currently limit the hierarchical topology to be used in OSI mode. */
>>         if (!psci_has_osi_support())
>>                 return 0;
>> @@ -261,14 +263,22 @@ static int psci_dt_cpu_init_topology(struct cpuidle_driver *drv,
>>         psci_cpuidle_use_syscore = true;
>>
>>         /*
>> -        * Using the deepest state for the CPU to trigger a potential selection
>> -        * of a shared state for the domain, assumes the domain states are all
>> -        * deeper states. On PREEMPT_RT the hierarchical topology is limited to
>> -        * s2ram and s2idle.
>> +        * Assign the domain-aware callbacks to all CPU idle states so that the
>> +        * domain-idle-state logic is triggered regardless of which CPU idle
>> +        * state is selected.
>> +        *
>> +        * For s2idle, enter_s2idle uses dev_pm_genpd_suspend() which is safe
>> +        * on PREEMPT_RT. find_deepest_state() will pick the deepest state
>> +        * whose exit latency fits within the active QoS constraint.
>> +        *
>> +        * For the normal idle path, enter uses pm_runtime_put_sync_suspend()
>> +        * which may sleep and is therefore not used on PREEMPT_RT.
>>          */
>> -       drv->states[state_count - 1].enter_s2idle = psci_enter_s2idle_domain_idle_state;
>> -       if (!IS_ENABLED(CONFIG_PREEMPT_RT))
>> -               drv->states[state_count - 1].enter = psci_enter_domain_idle_state;
>> +       for (i = 1; i < state_count; i++) {
>> +               drv->states[i].enter_s2idle = psci_enter_s2idle_domain_idle_state;
>> +               if (!IS_ENABLED(CONFIG_PREEMPT_RT))
>> +                       drv->states[i].enter = psci_enter_domain_idle_state;
>> +       }
>
> This breaks the current contract for genpd when it tries to select a
> domain idle state for a group of CPUs that shares the same PM domain.

Could you elaborate on what that "current contract" is or point me to
where it's described in more detail.  I understand there was a reason
for this design choice when first implemented, but now that we have
added support for using QoS to constrain the state selection used for
system-wide suspend (including s2idle) this "contract" is really
restrictive, and doesn't allow the domain idle logic to be used at all
for shallower states.

> In principle, if the CPU has a clock gating state (shallow) and a
> power collapse state (deep), it would be sufficient for the CPU to be
> in the clock gating state, while allowing the cluster PM domain
> (through genpd) to enter a domain idle state that corresponds to a
> power collapse state. Depending on the platform of course.

Yes, that is possible in principle, but with an important clarification:

Once all CPUs are in an idle state (either shallow or deep), the domain
idle state logic is entered.  It's then up to the domain (or its
governor) to select the appropriate state(s) for the domain.  If any of
the CPUs are in a shallow state, the domain governor should not allow a
deep state.

In your example, you mention the CPUs in a shallow state but the cluster
domain would pick a deep state.  I would say that's a bug in the
PM domain (or its governor) if it would allow that.

The job of the PM domain is to pick the deepest state that is possible
based on the state of the devices (including CPUs) that are in that
domain.

The same is true today for domains that do not have CPUs.  If you have a
device that runtime suspended, but in a shallow state (e.g. only clock
gated), the PM domain it is in should not hit a deep (power-off) state,
otherwise that device will lose context and not resume properly.

I'm trying to enable that same thing for PM domains with CPUs.

> On the platform you are working, is there a clock gating state (or
> similar) on the cluster PM domain, which is allowed to be entered when
> the corresponding CPUs are in the similar state?

Yes.  Not only the CPUs have shallow and deep states, but the domains
can have shallow and deep states as well.

Kevin