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

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

[Kevin Hilman (TI)]

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@ti.com>
Signed-off-by: Kevin Hilman (TI) <khilman@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;
+	}
 
 	return 0;
 }

---
base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
change-id: 20260831-topic-lpm-psci-domain-callbacks-a8af0cd7df35

Best regards,
--  
Kevin Hilman (TI) <khilman@baylibre.com>

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

[Ulf Hansson]

On Mon, Aug 31, 2026 at 8:52 PM Kevin Hilman (TI) <khilman@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@ti.com>
> Signed-off-by: Kevin Hilman (TI) <khilman@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.

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.

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?

>
>         return 0;
>  }
>
> ---
> base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
> change-id: 20260831-topic-lpm-psci-domain-callbacks-a8af0cd7df35
>
> Best regards,
> --
> Kevin Hilman (TI) <khilman@baylibre.com>
>

Kind regards
Uffe

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

[Kevin Hilman]

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

> On Mon, Aug 31, 2026 at 8:52 PM Kevin Hilman (TI) <khilman@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@ti.com>
>> Signed-off-by: Kevin Hilman (TI) <khilman@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

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

[Ulf Hansson]

On Tue, Sep 1, 2026 at 8:46 PM Kevin Hilman <khilman@baylibre.com> wrote:
>
> Ulf Hansson <ulf.hansson@oss.qualcomm.com> writes:
>
> > On Mon, Aug 31, 2026 at 8:52 PM Kevin Hilman (TI) <khilman@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@ti.com>
> >> Signed-off-by: Kevin Hilman (TI) <khilman@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.

Right, that's a limitation we decided to start with, simply because we
had no use cases back then.

So, if we would do the runtime PM reference counting when entering a
shallower state for the CPU, it would allow the genpd governor to pick
any of the domain idle states for the shared cluster PM domain. This
doesn't work as is.

The CPU PM domains are structured hierarchically and the child CPU PM
domains that contains only a single CPU, don't have any idle states
that are managed by the genpd governor. Instead those states are
presented to the regular cpuidle governor as per CPU idle 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.

Right, I see your point. So in that case, we need genpd (and its
governor) to know about these per CPU idle states, as that piece of
information is not available today.

>
> 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 totally agree that this problem isn't limited to CPUs. Generic
devices can certainly have support for multiple low power states.

At this point, through runtime PM, we support only runtime suspend
(not counting runtime idle), which means there is only one low power
state available per device. It's only PM domains through genpd that
currently supports multiple low power states.

For a CPU device, it's slightly different though, as we have decided
to model each CPU to have its own CPU PM domain.

>
> 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

Sounds like we have a BoF material to discuss at LPC...

Kind regards
Uffe