| Message ID | 20240527142557.321610-1-ulf.hansson@linaro.org |
|---|---|
| Headers | show |
| Series | pmdomain/cpuidle-psci: Support s2idle/s2ram on PREEMPT_RT | expand |
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 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-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
+ 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
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(-)