Updates in v2:
	- Rebased and fixed a small issue in genpd, see patch3.
	- Re-tested on v6.9-rt5 (PREEMPT_RT enabled)
	- Re-tested on v6.10-rc1 (for regressions, PREEMPT_RT disabled)

The hierarchical PM domain topology and the corresponding domain-idle-states
are currently disabled on a PREEMPT_RT based configuration. The main reason is
because spinlocks are turned into sleepable locks on PREEMPT_RT, which means
genpd and runtime PM can't be use in the atomic idle-path when
selecting/entering an idle-state.

For s2idle/s2ram this is an unnecessary limitation that this series intends to
address. Note that, the support for cpuhotplug is left to future improvements.
More information about this are available in the commit messages.

I have tested this on a Dragonboard 410c.

Kind regards
Ulf Hansson


Ulf Hansson (7):
  pmdomain: core: Enable s2idle for CPU PM domains on PREEMPT_RT
  pmdomain: core: Don't hold the genpd-lock when calling
    dev_pm_domain_set()
  pmdomain: core: Use dev_name() instead of kobject_get_path() in
    debugfs
  cpuidle: psci-domain: Enable system-wide suspend on PREEMPT_RT
  cpuidle: psci: Drop redundant assignment of CPUIDLE_FLAG_RCU_IDLE
  cpuidle: psci: Enable the hierarchical topology for s2ram on
    PREEMPT_RT
  cpuidle: psci: Enable the hierarchical topology for s2idle on
    PREEMPT_RT

 drivers/cpuidle/cpuidle-psci-domain.c | 10 ++--
 drivers/cpuidle/cpuidle-psci.c        | 26 ++++++----
 drivers/pmdomain/core.c               | 75 +++++++++++++++++++--------
 include/linux/pm_domain.h             |  5 +-
 4 files changed, 80 insertions(+), 36 deletions(-)

-- 
2.34.1

+ Raghavendra Kakarla, Sebastian Andrzej Siewior

On Mon, 27 May 2024 at 16:26, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Updates in v2:
>         - Rebased and fixed a small issue in genpd, see patch3.
>         - Re-tested on v6.9-rt5 (PREEMPT_RT enabled)
>         - Re-tested on v6.10-rc1 (for regressions, PREEMPT_RT disabled)
>
> The hierarchical PM domain topology and the corresponding domain-idle-states
> are currently disabled on a PREEMPT_RT based configuration. The main reason is
> because spinlocks are turned into sleepable locks on PREEMPT_RT, which means
> genpd and runtime PM can't be use in the atomic idle-path when
> selecting/entering an idle-state.
>
> For s2idle/s2ram this is an unnecessary limitation that this series intends to
> address. Note that, the support for cpuhotplug is left to future improvements.
> More information about this are available in the commit messages.
>
> I have tested this on a Dragonboard 410c.
>
> Kind regards
> Ulf Hansson
>
>
> Ulf Hansson (7):
>   pmdomain: core: Enable s2idle for CPU PM domains on PREEMPT_RT
>   pmdomain: core: Don't hold the genpd-lock when calling
>     dev_pm_domain_set()
>   pmdomain: core: Use dev_name() instead of kobject_get_path() in
>     debugfs
>   cpuidle: psci-domain: Enable system-wide suspend on PREEMPT_RT
>   cpuidle: psci: Drop redundant assignment of CPUIDLE_FLAG_RCU_IDLE
>   cpuidle: psci: Enable the hierarchical topology for s2ram on
>     PREEMPT_RT
>   cpuidle: psci: Enable the hierarchical topology for s2idle on
>     PREEMPT_RT
>
>  drivers/cpuidle/cpuidle-psci-domain.c | 10 ++--
>  drivers/cpuidle/cpuidle-psci.c        | 26 ++++++----
>  drivers/pmdomain/core.c               | 75 +++++++++++++++++++--------
>  include/linux/pm_domain.h             |  5 +-
>  4 files changed, 80 insertions(+), 36 deletions(-)
>

Just wanted to let you all know that I have queued up this series via
my pmdomain tree. Please let me know if you have any further
comments/suggestions.

Kind regards
Uffe

On 5/27/2024 7:55 PM, Ulf Hansson wrote:
> Updates in v2:
> 	- Rebased and fixed a small issue in genpd, see patch3.
> 	- Re-tested on v6.9-rt5 (PREEMPT_RT enabled)
> 	- Re-tested on v6.10-rc1 (for regressions, PREEMPT_RT disabled)
> 
> The hierarchical PM domain topology and the corresponding domain-idle-states
> are currently disabled on a PREEMPT_RT based configuration. The main reason is
> because spinlocks are turned into sleepable locks on PREEMPT_RT, which means
> genpd and runtime PM can't be use in the atomic idle-path when
> selecting/entering an idle-state.
> 
> For s2idle/s2ram this is an unnecessary limitation that this series intends to
> address. Note that, the support for cpuhotplug is left to future improvements.
> More information about this are available in the commit messages.
> 
> I have tested this on a Dragonboard 410c.
This series is tested on qcm6490 with PREEMPT_RT enabled. For the series,

Tested-by: Raghavendra Kakarla <quic_rkakarla@quicinc.com>  # qcm6490 
with PREEMPT_RT enabled

Tested APSS suspend and then SoC power collapse through s2idle and s2ram 
paths.
APSS and soc power collapse stats are incremented.
/sys/kernel/debug/pm_genpd/power-domain-cluster/idle_states
/sys/kernel/debug/qcom_stats/cxsd

Without this series, they are not incremented.

Thanks,
Raghavendra K.
> 
> Kind regards
> Ulf Hansson
> 
> 
> Ulf Hansson (7):
>    pmdomain: core: Enable s2idle for CPU PM domains on PREEMPT_RT
>    pmdomain: core: Don't hold the genpd-lock when calling
>      dev_pm_domain_set()
>    pmdomain: core: Use dev_name() instead of kobject_get_path() in
>      debugfs
>    cpuidle: psci-domain: Enable system-wide suspend on PREEMPT_RT
>    cpuidle: psci: Drop redundant assignment of CPUIDLE_FLAG_RCU_IDLE
>    cpuidle: psci: Enable the hierarchical topology for s2ram on
>      PREEMPT_RT
>    cpuidle: psci: Enable the hierarchical topology for s2idle on
>      PREEMPT_RT
> 
>   drivers/cpuidle/cpuidle-psci-domain.c | 10 ++--
>   drivers/cpuidle/cpuidle-psci.c        | 26 ++++++----
>   drivers/pmdomain/core.c               | 75 +++++++++++++++++++--------
>   include/linux/pm_domain.h             |  5 +-
>   4 files changed, 80 insertions(+), 36 deletions(-)
>

On 2024-05-27 16:25:50 [+0200], Ulf Hansson wrote:
> Updates in v2:
> 	- Rebased and fixed a small issue in genpd, see patch3.
> 	- Re-tested on v6.9-rt5 (PREEMPT_RT enabled)
> 	- Re-tested on v6.10-rc1 (for regressions, PREEMPT_RT disabled)
> 
> The hierarchical PM domain topology and the corresponding domain-idle-states
> are currently disabled on a PREEMPT_RT based configuration. The main reason is
> because spinlocks are turned into sleepable locks on PREEMPT_RT, which means
> genpd and runtime PM can't be use in the atomic idle-path when
> selecting/entering an idle-state.
> 
> For s2idle/s2ram this is an unnecessary limitation that this series intends to
> address. Note that, the support for cpuhotplug is left to future improvements.
> More information about this are available in the commit messages.

I looked at it and it seems limited to pmdomain/core.c, also I don't
know if there is a ->set_performance_state callback set since the one I
checked have mutex_t locking ;)
So if this is needed, then be it. s2ram wouldn't be used in "production"
but in "safe state" so I wouldn't worry too much about latency spikes.
Not sure what it means for the other modes.
I am not to worried for now, please don't let spread more than needed ;)

> Kind regards
> Ulf Hansson

Sebastian

On Mon, 8 Jul 2024 at 15:53, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2024-05-27 16:25:50 [+0200], Ulf Hansson wrote:
> > Updates in v2:
> >       - Rebased and fixed a small issue in genpd, see patch3.
> >       - Re-tested on v6.9-rt5 (PREEMPT_RT enabled)
> >       - Re-tested on v6.10-rc1 (for regressions, PREEMPT_RT disabled)
> >
> > The hierarchical PM domain topology and the corresponding domain-idle-states
> > are currently disabled on a PREEMPT_RT based configuration. The main reason is
> > because spinlocks are turned into sleepable locks on PREEMPT_RT, which means
> > genpd and runtime PM can't be use in the atomic idle-path when
> > selecting/entering an idle-state.
> >
> > For s2idle/s2ram this is an unnecessary limitation that this series intends to
> > address. Note that, the support for cpuhotplug is left to future improvements.
> > More information about this are available in the commit messages.
>
> I looked at it and it seems limited to pmdomain/core.c, also I don't
> know if there is a ->set_performance_state callback set since the one I
> checked have mutex_t locking ;)
> So if this is needed, then be it. s2ram wouldn't be used in "production"
> but in "safe state" so I wouldn't worry too much about latency spikes.
> Not sure what it means for the other modes.
> I am not to worried for now, please don't let spread more than needed ;)

Thanks for taking a look and for providing your thoughts. Can I
consider that as an "ack" for the whole series?

Before I decide to apply this I am awaiting some additional
confirmation from Qcom guys. It's getting late for v6.11, so I may
need to make another re-spin, but let's see.

Kind regards
Uffe

On 2024-07-09 17:31:12 [+0200], Ulf Hansson wrote:
> 
> Thanks for taking a look and for providing your thoughts. Can I
> consider that as an "ack" for the whole series?

Yes.

Sebastian