2024-05-27 15:05:50

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH v2 0/7] pmdomain/cpuidle-psci: Support s2idle/s2ram on PREEMPT_RT

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



2024-05-27 15:06:00

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH v2 2/7] pmdomain: core: Don't hold the genpd-lock when calling dev_pm_domain_set()

There is no need to hold the genpd-lock, while assigning the
dev->pm_domain. In fact, it becomes a problem on a PREEMPT_RT based
configuration as the genpd-lock may be a raw spinlock, while the lock
acquired through the call to dev_pm_domain_set() is a regular spinlock.

To fix the problem, let's simply move the calls to dev_pm_domain_set()
outside the genpd-lock.

Signed-off-by: Ulf Hansson <[email protected]>
---
Changes in v2:
- None.

---
drivers/pmdomain/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 072e6bdb6ee6..454fccc38df1 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -1736,7 +1736,6 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
genpd_lock(genpd);

genpd_set_cpumask(genpd, gpd_data->cpu);
- dev_pm_domain_set(dev, &genpd->domain);

genpd->device_count++;
if (gd)
@@ -1745,6 +1744,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);

genpd_unlock(genpd);
+ dev_pm_domain_set(dev, &genpd->domain);
out:
if (ret)
genpd_free_dev_data(dev, gpd_data);
@@ -1801,12 +1801,13 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
genpd->gd->max_off_time_changed = true;

genpd_clear_cpumask(genpd, gpd_data->cpu);
- dev_pm_domain_set(dev, NULL);

list_del_init(&pdd->list_node);

genpd_unlock(genpd);

+ dev_pm_domain_set(dev, NULL);
+
if (genpd->detach_dev)
genpd->detach_dev(genpd, dev);

--
2.34.1


2024-05-27 15:07:09

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH v2 5/7] cpuidle: psci: Drop redundant assignment of CPUIDLE_FLAG_RCU_IDLE

When using the hierarchical topology and PSCI OSI-mode we may end up
overriding the deepest idle-state's ->enter|enter_s2idle() callbacks, but
there is no point to also re-assign the CPUIDLE_FLAG_RCU_IDLE for the
idle-state in question, as that has already been set when parsing the
states from DT. See init_state_node().

Signed-off-by: Ulf Hansson <[email protected]>
---
Changes in v2:
- None.
---
drivers/cpuidle/cpuidle-psci.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
index 782030a27703..d82a8bc1b194 100644
--- a/drivers/cpuidle/cpuidle-psci.c
+++ b/drivers/cpuidle/cpuidle-psci.c
@@ -234,7 +234,6 @@ static int psci_dt_cpu_init_topology(struct cpuidle_driver *drv,
* of a shared state for the domain, assumes the domain states are all
* deeper states.
*/
- drv->states[state_count - 1].flags |= CPUIDLE_FLAG_RCU_IDLE;
drv->states[state_count - 1].enter = psci_enter_domain_idle_state;
drv->states[state_count - 1].enter_s2idle = psci_enter_s2idle_domain_idle_state;
psci_cpuidle_use_cpuhp = true;
--
2.34.1


2024-05-27 15:07:10

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH v2 4/7] cpuidle: psci-domain: Enable system-wide suspend on PREEMPT_RT

The domain-idle-states are currently disabled on a PREEMPT_RT based
configuration for the cpuidle-psci-domain. To enable them to be used for
system-wide suspend and in particular during s2idle, let's set the
GENPD_FLAG_RPM_ALWAYS_ON instead of GENPD_FLAG_ALWAYS_ON for the
corresponding genpd provider.

In this way, the runtime PM path remains disabled in genpd for its attached
devices, while powering-on/off the PM domain during system-wide suspend
becomes allowed.

Signed-off-by: Ulf Hansson <[email protected]>
---
Changes in v2:
- None.
---
drivers/cpuidle/cpuidle-psci-domain.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c
index fae958794339..ea28b73ef3fb 100644
--- a/drivers/cpuidle/cpuidle-psci-domain.c
+++ b/drivers/cpuidle/cpuidle-psci-domain.c
@@ -67,12 +67,16 @@ static int psci_pd_init(struct device_node *np, bool use_osi)

/*
* Allow power off when OSI has been successfully enabled.
- * PREEMPT_RT is not yet ready to enter domain idle states.
+ * On a PREEMPT_RT based configuration the domain idle states are
+ * supported, but only during system-wide suspend.
*/
- if (use_osi && !IS_ENABLED(CONFIG_PREEMPT_RT))
+ if (use_osi) {
pd->power_off = psci_pd_power_off;
- else
+ if (IS_ENABLED(CONFIG_PREEMPT_RT))
+ pd->flags |= GENPD_FLAG_RPM_ALWAYS_ON;
+ } else {
pd->flags |= GENPD_FLAG_ALWAYS_ON;
+ }

/* Use governor for CPU PM domains if it has some states to manage. */
pd_gov = pd->states ? &pm_domain_cpu_gov : NULL;
--
2.34.1