mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5 0/4] PM: QoS/pmdomains: support resume latencies for system-wide PM
@ 2026-08-26 22:23 Kevin Hilman (TI)
  2026-08-26 22:23 ` [PATCH v5 1/4] PM / QoS: add flag to indicate latency applies system-wide Kevin Hilman (TI)
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Kevin Hilman (TI) @ 2026-08-26 22:23 UTC (permalink / raw)
  To: Rafael J. Wysocki, linux-pm, Ulf Hansson
  Cc: linux-kernel, Kendall Willis, Abel Vesa

Currently QoS resume latencies are only considered for runtime PM
transitions of pmdomains, which remains the default.

In order to also support QoS resume latencies during system-wide PM,
add a new flag to indicate a resume latency should be used for
system-wide PM *in addition to* runtime PM.

If a user requires a different latency value for system-wide PM
compared to runtime PM, then the runtime PM value can be set for
normal operations, and the system-wide value (and flag) can be set by
during suspend (for example in a drivers ->prepare() hook) and the
runtime PM value can be restored after resume (for example, in a
driver's ->complete() hook.)

To: Rafael J. Wysocki <rafael@kernel.org>
To: Ulf Hansson <ulf.hansson@linaro.org>
To: linux-pm@vger.kernel.org

Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
Changes in v5:
- update copyright year & author in new header
- add Reviewed-by tags from Kendall & Abel
- Link to v4: https://patch.msgid.link/20260819-topic-lpm-pmdomain-device-constraints-v4-0-7ceb095f5474@baylibre.com

Changes in v4:
- added corresponding WRITE_ONCE() to match READ_ONCE()
- rebase to v7.2
- Link to v3: https://patch.msgid.link/20260611-topic-lpm-pmdomain-device-constraints-v3-0-75d69438518b@baylibre.com

Changes in v3:
- rebased to v7.0
- fix PREEMPT_RT: add new helper for lockless read of flags for atomic contexts 
- update kerneldoc for genpd_for_each_child() as requested by Ulf
- Link to v2: https://patch.msgid.link/20260205-topic-lpm-pmdomain-device-constraints-v2-0-61f7be7d35ac@baylibre.com

Changes in v2:
- drop the userspace interface
- add genpd helper to iterate over all devices in domain and child domains
- new flag means latency applies to runtime PM *and* system-wide PM
- Link to v1: https://patch.msgid.link/20260120-topic-lpm-pmdomain-device-constraints-v1-0-108fc4cfafce@baylibre.com

---
Kevin Hilman (TI) (4):
      PM / QoS: add flag to indicate latency applies system-wide
      PM / QoS: add lockless read for flags
      pmdomain: core: add genpd_for_each_child() helper
      pmdomain: add support system-wide resume latency constraints

 drivers/pmdomain/core.c     | 45 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/pmdomain/core.h     | 17 +++++++++++++++++
 drivers/pmdomain/governor.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_qos.h      |  9 +++++++++
 kernel/power/qos.c          |  4 ++--
 5 files changed, 129 insertions(+), 2 deletions(-)
---
base-commit: 237a1c39e8dfd3e1c6f1f023eea37a48ec04cc63
change-id: 20260120-topic-lpm-pmdomain-device-constraints-e5e78ce48502

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 1/4] PM / QoS: add flag to indicate latency applies system-wide
  2026-08-26 22:23 [PATCH v5 0/4] PM: QoS/pmdomains: support resume latencies for system-wide PM Kevin Hilman (TI)
@ 2026-08-26 22:23 ` Kevin Hilman (TI)
  2026-08-26 22:23 ` [PATCH v5 2/4] PM / QoS: add lockless read for flags Kevin Hilman (TI)
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman (TI) @ 2026-08-26 22:23 UTC (permalink / raw)
  To: Rafael J. Wysocki, linux-pm, Ulf Hansson; +Cc: linux-kernel, Kendall Willis

By default, the QoS resume latency currenly only applied to runtime PM
decisions.

Add new PM_QOS_FLAG_LATENCY_SYS flag to indicate that the
resume latency QoS constraint should be applied to system-wide
PM *in addition to* runtime PM.

Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
Reviewed-by: Kendall Willis <k-willis@ti.com>
Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
 include/linux/pm_qos.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 6cea4455f867..aededda52b6b 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -37,6 +37,8 @@ enum pm_qos_flags_status {
 #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT	(-1)
 
 #define PM_QOS_FLAG_NO_POWER_OFF	(1 << 0)
+/* latency value applies to system-wide suspend/s2idle */
+#define PM_QOS_FLAG_LATENCY_SYS		(2 << 0)
 
 enum pm_qos_type {
 	PM_QOS_UNITIALIZED,

-- 
2.47.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 2/4] PM / QoS: add lockless read for flags
  2026-08-26 22:23 [PATCH v5 0/4] PM: QoS/pmdomains: support resume latencies for system-wide PM Kevin Hilman (TI)
  2026-08-26 22:23 ` [PATCH v5 1/4] PM / QoS: add flag to indicate latency applies system-wide Kevin Hilman (TI)
@ 2026-08-26 22:23 ` Kevin Hilman (TI)
  2026-09-11 13:59   ` Rafael J. Wysocki (Intel)
  2026-08-26 22:23 ` [PATCH v5 3/4] pmdomain: core: add genpd_for_each_child() helper Kevin Hilman (TI)
  2026-08-26 22:23 ` [PATCH v5 4/4] pmdomain: add support system-wide resume latency constraints Kevin Hilman (TI)
  3 siblings, 1 reply; 10+ messages in thread
From: Kevin Hilman (TI) @ 2026-08-26 22:23 UTC (permalink / raw)
  To: Rafael J. Wysocki, linux-pm, Ulf Hansson; +Cc: linux-kernel, Kendall Willis

Add a lockless read for QoS flags similar to the lockless read for
resume latency (dev_pm_qos_raw_resume_latency) which may be called
from atomic context (e.g. genpd governors running under a raw spinlock
or in the syscore suspend path), where taking that sleeping lock would
be invalid on PREEMPT_RT.

dev_pm_qos_raw_flags() now reads dev->power.qos->flags.effective_flags
with READ_ONCE() so also add corresponding WRITE_ONCE() to the two
effective_flags writes, the same way pm_qos_set_value() already does
for target_value/pm_qos_read_value().

Reviewed-by: Kendall Willis <k-willis@ti.com>
Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
 include/linux/pm_qos.h | 7 +++++++
 kernel/power/qos.c     | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index aededda52b6b..439a9e779d81 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -219,6 +219,12 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
 		PM_QOS_RESUME_LATENCY_NO_CONSTRAINT :
 		pm_qos_read_value(&dev->power.qos->resume_latency);
 }
+
+static inline s32 dev_pm_qos_raw_flags(struct device *dev)
+{
+	return IS_ERR_OR_NULL(dev->power.qos) ?
+		0 : READ_ONCE(dev->power.qos->flags.effective_flags);
+}
 #else
 static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
 							  s32 mask)
@@ -300,6 +306,7 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
 {
 	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
 }
+static inline s32 dev_pm_qos_raw_flags(struct device *dev) { return 0; }
 #endif
 
 static inline int freq_qos_request_active(struct freq_qos_request *req)
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 1944dbeb0d4c..c241da5bccf6 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -159,7 +159,7 @@ static void pm_qos_flags_remove_req(struct pm_qos_flags *pqf,
 	list_for_each_entry(req, &pqf->list, node)
 		val |= req->flags;
 
-	pqf->effective_flags = val;
+	WRITE_ONCE(pqf->effective_flags, val);
 }
 
 /**
@@ -193,7 +193,7 @@ bool pm_qos_update_flags(struct pm_qos_flags *pqf,
 		req->flags = val;
 		INIT_LIST_HEAD(&req->node);
 		list_add_tail(&req->node, &pqf->list);
-		pqf->effective_flags |= val;
+		WRITE_ONCE(pqf->effective_flags, pqf->effective_flags | val);
 		break;
 	default:
 		/* no action */

-- 
2.47.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 3/4] pmdomain: core: add genpd_for_each_child() helper
  2026-08-26 22:23 [PATCH v5 0/4] PM: QoS/pmdomains: support resume latencies for system-wide PM Kevin Hilman (TI)
  2026-08-26 22:23 ` [PATCH v5 1/4] PM / QoS: add flag to indicate latency applies system-wide Kevin Hilman (TI)
  2026-08-26 22:23 ` [PATCH v5 2/4] PM / QoS: add lockless read for flags Kevin Hilman (TI)
@ 2026-08-26 22:23 ` Kevin Hilman (TI)
  2026-09-16 10:04   ` Ulf Hansson
  2026-08-26 22:23 ` [PATCH v5 4/4] pmdomain: add support system-wide resume latency constraints Kevin Hilman (TI)
  3 siblings, 1 reply; 10+ messages in thread
From: Kevin Hilman (TI) @ 2026-08-26 22:23 UTC (permalink / raw)
  To: Rafael J. Wysocki, linux-pm, Ulf Hansson; +Cc: linux-kernel, Abel Vesa

Add a new internal helper function genpd_for_each_child() that recursively
iterates over all devices in a PM domain and its child domains (subdomains).
This helper is useful for governors and other core PM domain code that needs
to examine or apply operations to all devices within a domain hierarchy.

The function takes a callback that is invoked for each device, and supports
early termination if the callback returns a non-zero value.

The helper is defined in a new internal header drivers/pmdomain/core.h and
implemented in drivers/pmdomain/core.c, making it available to other PM
domain subsystem components.

The first user of this helper is the cpu_system_power_down_ok() governor
function, which uses it to check device QoS latency constraints across the
entire domain hierarchy.

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
 drivers/pmdomain/core.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/pmdomain/core.h | 17 +++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 842c4169e290..ff27369d9a97 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -24,6 +24,8 @@
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
 
+#include "core.h"
+
 /* Provides a unique ID for each genpd device */
 static DEFINE_IDA(genpd_ida);
 
@@ -276,6 +278,49 @@ static void genpd_sd_counter_inc(struct generic_pm_domain *genpd)
 	smp_mb__after_atomic();
 }
 
+/**
+ * genpd_for_each_child - Recursively iterate over all devices
+ *                        in a PM domain and its subdomains.
+ * @genpd: PM domain to iterate over.
+ * @fn: Callback function to invoke for each device.
+ * @data: Data to pass to the callback function.
+ *
+ * This function recursively walks through all devices in the given PM domain
+ * and all devices in its child PM domains (subdomains). For each device found,
+ * the callback function @fn is invoked with the device and @data as arguments.
+ *
+ * Note: this function is inteded for use by the core and governors,
+ * not for pmdomain providers.
+ *
+ * Returns: 0 on success, or the first non-zero value returned by @fn.
+ */
+int genpd_for_each_child(struct generic_pm_domain *genpd,
+			 int (*fn)(struct device *dev, void *data),
+			 void *data)
+{
+	struct pm_domain_data *pdd;
+	struct gpd_link *link;
+	int ret;
+
+	/* First, iterate over all devices in this domain */
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		ret = fn(pdd->dev, data);
+		if (ret)
+			return ret;
+	}
+
+	/* Then, recursively iterate over all child domains (subdomains) */
+	list_for_each_entry(link, &genpd->parent_links, parent_node) {
+		struct generic_pm_domain *child_pd = link->child;
+
+		ret = genpd_for_each_child(child_pd, fn, data);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_DEBUG_FS
 static struct dentry *genpd_debugfs_dir;
 
diff --git a/drivers/pmdomain/core.h b/drivers/pmdomain/core.h
new file mode 100644
index 000000000000..a296134d4005
--- /dev/null
+++ b/drivers/pmdomain/core.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Internal header for PM domain core
+ *
+ * Copyright (C) 2026 Kevin Hilman <khilman@baylibre.com>, Texas Instruments
+ */
+
+#ifndef __PM_DOMAIN_CORE_H__
+#define __PM_DOMAIN_CORE_H__
+
+#include <linux/pm_domain.h>
+
+int genpd_for_each_child(struct generic_pm_domain *genpd,
+			 int (*fn)(struct device *dev, void *data),
+			 void *data);
+
+#endif /* __PM_DOMAIN_CORE_H__ */

-- 
2.47.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 4/4] pmdomain: add support system-wide resume latency constraints
  2026-08-26 22:23 [PATCH v5 0/4] PM: QoS/pmdomains: support resume latencies for system-wide PM Kevin Hilman (TI)
                   ` (2 preceding siblings ...)
  2026-08-26 22:23 ` [PATCH v5 3/4] pmdomain: core: add genpd_for_each_child() helper Kevin Hilman (TI)
@ 2026-08-26 22:23 ` Kevin Hilman (TI)
  2026-09-16 10:43   ` Ulf Hansson
  3 siblings, 1 reply; 10+ messages in thread
From: Kevin Hilman (TI) @ 2026-08-26 22:23 UTC (permalink / raw)
  To: Rafael J. Wysocki, linux-pm, Ulf Hansson
  Cc: linux-kernel, Abel Vesa, Kendall Willis

In addition to checking for CPU latency constraints when checking if
OK to power down a domain, also check for QoS latency constraints in
all devices of a domain and use that in determining the final latency
constraint to use for the domain.

Since cpu_system_power_down_ok() is used for system-wide suspend, the
per-device constratints are only relevant if the LATENCY_SYS QoS flag
is set.

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Kendall Willis <k-willis@ti.com>
Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
 drivers/pmdomain/governor.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/pmdomain/governor.c b/drivers/pmdomain/governor.c
index 96737abbb496..1a85fd375db9 100644
--- a/drivers/pmdomain/governor.c
+++ b/drivers/pmdomain/governor.c
@@ -13,6 +13,8 @@
 #include <linux/cpumask.h>
 #include <linux/ktime.h>
 
+#include "core.h"
+
 static int dev_update_qos_constraint(struct device *dev, void *data)
 {
 	s64 *constraint_ns_p = data;
@@ -425,17 +427,71 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
 	return true;
 }
 
+/**
+ * check_device_qos_latency - Callback to check device QoS latency constraints
+ * @dev: Device to check
+ * @data: Pointer to s32 variable holding minimum latency found so far
+ *
+ * This callback checks if the device has a system-wide resume latency QoS
+ * constraint and updates the minimum latency if this device has a stricter
+ * constraint.
+ *
+ * This runs in atomic context: for a CPU domain the genpd lock is a raw
+ * spinlock and the s2idle path runs in the syscore suspend window with
+ * interrupts disabled.  The lockless dev_pm_qos_raw_*() accessors must
+ * therefore be used here; the locked dev_pm_qos_read_value() /
+ * dev_pm_qos_flags() would take dev->power.lock, which is a sleeping lock
+ * on PREEMPT_RT and must not be acquired in this context.  The values read
+ * are best-effort, which matches the sibling cpu_power_down_ok() governor.
+ *
+ * The system-wide flag is checked first so that devices that have not opted
+ * in only incur a single lockless read.
+ *
+ * Returns: 0 to continue iteration.
+ */
+static int check_device_qos_latency(struct device *dev, void *data)
+{
+	s32 *min_dev_latency = data;
+	s32 dev_latency;
+
+	if (!(dev_pm_qos_raw_flags(dev) & PM_QOS_FLAG_LATENCY_SYS))
+		return 0;
+
+	dev_latency = dev_pm_qos_raw_resume_latency(dev);
+	if (dev_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) {
+		dev_dbg(dev,
+			"has QoS system-wide resume latency=%d\n",
+			dev_latency);
+		if (dev_latency < *min_dev_latency)
+			*min_dev_latency = dev_latency;
+	}
+
+	return 0;
+}
+
 static bool cpu_system_power_down_ok(struct dev_pm_domain *pd)
 {
 	s64 constraint_ns = cpu_wakeup_latency_qos_limit() * NSEC_PER_USEC;
 	struct generic_pm_domain *genpd = pd_to_genpd(pd);
 	int state_idx = genpd->state_count - 1;
+	s32 min_dev_latency = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
+	s64 min_dev_latency_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;
 
 	if (!(genpd->flags & GENPD_FLAG_CPU_DOMAIN)) {
 		genpd->state_idx = state_idx;
 		return true;
 	}
 
+	genpd_for_each_child(genpd, check_device_qos_latency,
+			     &min_dev_latency);
+
+	/* If device latency < CPU wakeup latency, use it instead */
+	if (min_dev_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) {
+		min_dev_latency_ns = min_dev_latency * NSEC_PER_USEC;
+		if (min_dev_latency_ns < constraint_ns)
+			constraint_ns = min_dev_latency_ns;
+	}
+
 	/* Find the deepest state for the latency constraint. */
 	while (state_idx >= 0) {
 		s64 latency_ns = genpd->states[state_idx].power_off_latency_ns +

-- 
2.47.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 2/4] PM / QoS: add lockless read for flags
  2026-08-26 22:23 ` [PATCH v5 2/4] PM / QoS: add lockless read for flags Kevin Hilman (TI)
@ 2026-09-11 13:59   ` Rafael J. Wysocki (Intel)
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-09-11 13:59 UTC (permalink / raw)
  To: Kevin Hilman (TI)
  Cc: Rafael J. Wysocki, linux-pm, Ulf Hansson, linux-kernel, Kendall Willis

On Thu, Aug 27, 2026 at 12:23 AM Kevin Hilman (TI) <khilman@baylibre.com> wrote:
>
> Add a lockless read for QoS flags similar to the lockless read for
> resume latency (dev_pm_qos_raw_resume_latency) which may be called
> from atomic context (e.g. genpd governors running under a raw spinlock
> or in the syscore suspend path), where taking that sleeping lock would
> be invalid on PREEMPT_RT.
>
> dev_pm_qos_raw_flags() now reads dev->power.qos->flags.effective_flags
> with READ_ONCE() so also add corresponding WRITE_ONCE() to the two
> effective_flags writes, the same way pm_qos_set_value() already does
> for target_value/pm_qos_read_value().
>
> Reviewed-by: Kendall Willis <k-willis@ti.com>
> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>

Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

and please route this one as suitable.

And sorry for the delay.

> ---
>  include/linux/pm_qos.h | 7 +++++++
>  kernel/power/qos.c     | 4 ++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index aededda52b6b..439a9e779d81 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -219,6 +219,12 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>                 PM_QOS_RESUME_LATENCY_NO_CONSTRAINT :
>                 pm_qos_read_value(&dev->power.qos->resume_latency);
>  }
> +
> +static inline s32 dev_pm_qos_raw_flags(struct device *dev)
> +{
> +       return IS_ERR_OR_NULL(dev->power.qos) ?
> +               0 : READ_ONCE(dev->power.qos->flags.effective_flags);
> +}
>  #else
>  static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
>                                                           s32 mask)
> @@ -300,6 +306,7 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>  {
>         return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>  }
> +static inline s32 dev_pm_qos_raw_flags(struct device *dev) { return 0; }
>  #endif
>
>  static inline int freq_qos_request_active(struct freq_qos_request *req)
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 1944dbeb0d4c..c241da5bccf6 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -159,7 +159,7 @@ static void pm_qos_flags_remove_req(struct pm_qos_flags *pqf,
>         list_for_each_entry(req, &pqf->list, node)
>                 val |= req->flags;
>
> -       pqf->effective_flags = val;
> +       WRITE_ONCE(pqf->effective_flags, val);
>  }
>
>  /**
> @@ -193,7 +193,7 @@ bool pm_qos_update_flags(struct pm_qos_flags *pqf,
>                 req->flags = val;
>                 INIT_LIST_HEAD(&req->node);
>                 list_add_tail(&req->node, &pqf->list);
> -               pqf->effective_flags |= val;
> +               WRITE_ONCE(pqf->effective_flags, pqf->effective_flags | val);
>                 break;
>         default:
>                 /* no action */
>
> --
> 2.47.3
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 3/4] pmdomain: core: add genpd_for_each_child() helper
  2026-08-26 22:23 ` [PATCH v5 3/4] pmdomain: core: add genpd_for_each_child() helper Kevin Hilman (TI)
@ 2026-09-16 10:04   ` Ulf Hansson
  2026-09-24 18:51     ` Kevin Hilman
  0 siblings, 1 reply; 10+ messages in thread
From: Ulf Hansson @ 2026-09-16 10:04 UTC (permalink / raw)
  To: Kevin Hilman (TI)
  Cc: Rafael J. Wysocki, linux-pm, Ulf Hansson, linux-kernel, Abel Vesa

On Thu, Aug 27, 2026 at 12:23 AM Kevin Hilman (TI) <khilman@baylibre.com> wrote:
>
> Add a new internal helper function genpd_for_each_child() that recursively
> iterates over all devices in a PM domain and its child domains (subdomains).
> This helper is useful for governors and other core PM domain code that needs
> to examine or apply operations to all devices within a domain hierarchy.
>
> The function takes a callback that is invoked for each device, and supports
> early termination if the callback returns a non-zero value.
>
> The helper is defined in a new internal header drivers/pmdomain/core.h and
> implemented in drivers/pmdomain/core.c, making it available to other PM
> domain subsystem components.
>
> The first user of this helper is the cpu_system_power_down_ok() governor
> function, which uses it to check device QoS latency constraints across the
> entire domain hierarchy.
>
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
> ---
>  drivers/pmdomain/core.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/pmdomain/core.h | 17 +++++++++++++++++
>  2 files changed, 62 insertions(+)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 842c4169e290..ff27369d9a97 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -24,6 +24,8 @@
>  #include <linux/cpu.h>
>  #include <linux/debugfs.h>
>
> +#include "core.h"
> +
>  /* Provides a unique ID for each genpd device */
>  static DEFINE_IDA(genpd_ida);
>
> @@ -276,6 +278,49 @@ static void genpd_sd_counter_inc(struct generic_pm_domain *genpd)
>         smp_mb__after_atomic();
>  }
>
> +/**
> + * genpd_for_each_child - Recursively iterate over all devices
> + *                        in a PM domain and its subdomains.
> + * @genpd: PM domain to iterate over.
> + * @fn: Callback function to invoke for each device.
> + * @data: Data to pass to the callback function.
> + *
> + * This function recursively walks through all devices in the given PM domain
> + * and all devices in its child PM domains (subdomains). For each device found,
> + * the callback function @fn is invoked with the device and @data as arguments.
> + *
> + * Note: this function is inteded for use by the core and governors,
> + * not for pmdomain providers.
> + *
> + * Returns: 0 on success, or the first non-zero value returned by @fn.
> + */
> +int genpd_for_each_child(struct generic_pm_domain *genpd,
> +                        int (*fn)(struct device *dev, void *data),
> +                        void *data)
> +{
> +       struct pm_domain_data *pdd;
> +       struct gpd_link *link;
> +       int ret;
> +
> +       /* First, iterate over all devices in this domain */
> +       list_for_each_entry(pdd, &genpd->dev_list, list_node) {
> +               ret = fn(pdd->dev, data);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       /* Then, recursively iterate over all child domains (subdomains) */
> +       list_for_each_entry(link, &genpd->parent_links, parent_node) {
> +               struct generic_pm_domain *child_pd = link->child;
> +
> +               ret = genpd_for_each_child(child_pd, fn, data);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
>  #ifdef CONFIG_DEBUG_FS
>  static struct dentry *genpd_debugfs_dir;
>
> diff --git a/drivers/pmdomain/core.h b/drivers/pmdomain/core.h
> new file mode 100644
> index 000000000000..a296134d4005
> --- /dev/null
> +++ b/drivers/pmdomain/core.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Internal header for PM domain core
> + *
> + * Copyright (C) 2026 Kevin Hilman <khilman@baylibre.com>, Texas Instruments
> + */
> +
> +#ifndef __PM_DOMAIN_CORE_H__
> +#define __PM_DOMAIN_CORE_H__
> +
> +#include <linux/pm_domain.h>

I don't think this is needed. Instead please make forward declaration
of the structs we need. Like:

struct device;
struct generic_pm_domain;

> +
> +int genpd_for_each_child(struct generic_pm_domain *genpd,
> +                        int (*fn)(struct device *dev, void *data),
> +                        void *data);
> +
> +#endif /* __PM_DOMAIN_CORE_H__ */
>
> --
> 2.47.3
>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 4/4] pmdomain: add support system-wide resume latency constraints
  2026-08-26 22:23 ` [PATCH v5 4/4] pmdomain: add support system-wide resume latency constraints Kevin Hilman (TI)
@ 2026-09-16 10:43   ` Ulf Hansson
  2026-09-24 19:13     ` Kevin Hilman
  0 siblings, 1 reply; 10+ messages in thread
From: Ulf Hansson @ 2026-09-16 10:43 UTC (permalink / raw)
  To: Kevin Hilman (TI)
  Cc: Rafael J. Wysocki, linux-pm, Ulf Hansson, linux-kernel,
	Abel Vesa, Kendall Willis

On Thu, Aug 27, 2026 at 12:23 AM Kevin Hilman (TI) <khilman@baylibre.com> wrote:
>
> In addition to checking for CPU latency constraints when checking if
> OK to power down a domain, also check for QoS latency constraints in
> all devices of a domain and use that in determining the final latency
> constraint to use for the domain.
>
> Since cpu_system_power_down_ok() is used for system-wide suspend, the
> per-device constratints are only relevant if the LATENCY_SYS QoS flag
> is set.

cpu_system_power_down_ok() is especially used for genpd's that have
the GENPD_FLAG_CPU_DOMAIN bit set (cpuidle-psci-domain and
cpuidle-riscv-sbi).

In other words, this has no effect on other types of PM domains that
are managed by genpd. Are you planning on adding that on top or this
is sufficient for your use cases?

>
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Reviewed-by: Kendall Willis <k-willis@ti.com>
> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
> ---
>  drivers/pmdomain/governor.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>
> diff --git a/drivers/pmdomain/governor.c b/drivers/pmdomain/governor.c
> index 96737abbb496..1a85fd375db9 100644
> --- a/drivers/pmdomain/governor.c
> +++ b/drivers/pmdomain/governor.c
> @@ -13,6 +13,8 @@
>  #include <linux/cpumask.h>
>  #include <linux/ktime.h>
>
> +#include "core.h"
> +
>  static int dev_update_qos_constraint(struct device *dev, void *data)
>  {
>         s64 *constraint_ns_p = data;
> @@ -425,17 +427,71 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
>         return true;
>  }
>
> +/**
> + * check_device_qos_latency - Callback to check device QoS latency constraints
> + * @dev: Device to check
> + * @data: Pointer to s32 variable holding minimum latency found so far
> + *
> + * This callback checks if the device has a system-wide resume latency QoS
> + * constraint and updates the minimum latency if this device has a stricter
> + * constraint.
> + *
> + * This runs in atomic context: for a CPU domain the genpd lock is a raw
> + * spinlock and the s2idle path runs in the syscore suspend window with
> + * interrupts disabled.  The lockless dev_pm_qos_raw_*() accessors must
> + * therefore be used here; the locked dev_pm_qos_read_value() /
> + * dev_pm_qos_flags() would take dev->power.lock, which is a sleeping lock
> + * on PREEMPT_RT and must not be acquired in this context.  The values read
> + * are best-effort, which matches the sibling cpu_power_down_ok() governor.

This is a bit too much in my opinion, please leave out the parts
concerning the syscore/atomic/lockless parts.

If we want that information to be described (I guess it would make
sense), I suggest we add that along with cpu_system_power_down_ok()
instead as it better belongs there.

> + *
> + * The system-wide flag is checked first so that devices that have not opted
> + * in only incur a single lockless read.
> + *
> + * Returns: 0 to continue iteration.
> + */
> +static int check_device_qos_latency(struct device *dev, void *data)
> +{
> +       s32 *min_dev_latency = data;
> +       s32 dev_latency;
> +
> +       if (!(dev_pm_qos_raw_flags(dev) & PM_QOS_FLAG_LATENCY_SYS))
> +               return 0;
> +
> +       dev_latency = dev_pm_qos_raw_resume_latency(dev);
> +       if (dev_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) {
> +               dev_dbg(dev,
> +                       "has QoS system-wide resume latency=%d\n",
> +                       dev_latency);

Do we really need a dev_dbg() here? Leftover from debugging?

> +               if (dev_latency < *min_dev_latency)
> +                       *min_dev_latency = dev_latency;
> +       }
> +
> +       return 0;
> +}
> +
>  static bool cpu_system_power_down_ok(struct dev_pm_domain *pd)
>  {
>         s64 constraint_ns = cpu_wakeup_latency_qos_limit() * NSEC_PER_USEC;
>         struct generic_pm_domain *genpd = pd_to_genpd(pd);
>         int state_idx = genpd->state_count - 1;
> +       s32 min_dev_latency = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> +       s64 min_dev_latency_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;

We don't need to assign a default value for min_dev_latency_ns.

>
>         if (!(genpd->flags & GENPD_FLAG_CPU_DOMAIN)) {
>                 genpd->state_idx = state_idx;
>                 return true;
>         }
>
> +       genpd_for_each_child(genpd, check_device_qos_latency,
> +                            &min_dev_latency);
> +
> +       /* If device latency < CPU wakeup latency, use it instead */
> +       if (min_dev_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) {
> +               min_dev_latency_ns = min_dev_latency * NSEC_PER_USEC;
> +               if (min_dev_latency_ns < constraint_ns)
> +                       constraint_ns = min_dev_latency_ns;
> +       }
> +
>         /* Find the deepest state for the latency constraint. */
>         while (state_idx >= 0) {
>                 s64 latency_ns = genpd->states[state_idx].power_off_latency_ns +
>
> --
> 2.47.3
>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 3/4] pmdomain: core: add genpd_for_each_child() helper
  2026-09-16 10:04   ` Ulf Hansson
@ 2026-09-24 18:51     ` Kevin Hilman
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2026-09-24 18:51 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rafael J. Wysocki, linux-pm, Ulf Hansson, linux-kernel, Abel Vesa

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

> On Thu, Aug 27, 2026 at 12:23 AM Kevin Hilman (TI) <khilman@baylibre.com> wrote:
>>
>> Add a new internal helper function genpd_for_each_child() that recursively
>> iterates over all devices in a PM domain and its child domains (subdomains).
>> This helper is useful for governors and other core PM domain code that needs
>> to examine or apply operations to all devices within a domain hierarchy.
>>
>> The function takes a callback that is invoked for each device, and supports
>> early termination if the callback returns a non-zero value.
>>
>> The helper is defined in a new internal header drivers/pmdomain/core.h and
>> implemented in drivers/pmdomain/core.c, making it available to other PM
>> domain subsystem components.
>>
>> The first user of this helper is the cpu_system_power_down_ok() governor
>> function, which uses it to check device QoS latency constraints across the
>> entire domain hierarchy.
>>
>> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
>> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
>> ---
>>  drivers/pmdomain/core.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/pmdomain/core.h | 17 +++++++++++++++++
>>  2 files changed, 62 insertions(+)
>>
>> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
>> index 842c4169e290..ff27369d9a97 100644
>> --- a/drivers/pmdomain/core.c
>> +++ b/drivers/pmdomain/core.c
>> @@ -24,6 +24,8 @@
>>  #include <linux/cpu.h>
>>  #include <linux/debugfs.h>
>>
>> +#include "core.h"
>> +
>>  /* Provides a unique ID for each genpd device */
>>  static DEFINE_IDA(genpd_ida);
>>
>> @@ -276,6 +278,49 @@ static void genpd_sd_counter_inc(struct generic_pm_domain *genpd)
>>         smp_mb__after_atomic();
>>  }
>>
>> +/**
>> + * genpd_for_each_child - Recursively iterate over all devices
>> + *                        in a PM domain and its subdomains.
>> + * @genpd: PM domain to iterate over.
>> + * @fn: Callback function to invoke for each device.
>> + * @data: Data to pass to the callback function.
>> + *
>> + * This function recursively walks through all devices in the given PM domain
>> + * and all devices in its child PM domains (subdomains). For each device found,
>> + * the callback function @fn is invoked with the device and @data as arguments.
>> + *
>> + * Note: this function is inteded for use by the core and governors,
>> + * not for pmdomain providers.
>> + *
>> + * Returns: 0 on success, or the first non-zero value returned by @fn.
>> + */
>> +int genpd_for_each_child(struct generic_pm_domain *genpd,
>> +                        int (*fn)(struct device *dev, void *data),
>> +                        void *data)
>> +{
>> +       struct pm_domain_data *pdd;
>> +       struct gpd_link *link;
>> +       int ret;
>> +
>> +       /* First, iterate over all devices in this domain */
>> +       list_for_each_entry(pdd, &genpd->dev_list, list_node) {
>> +               ret = fn(pdd->dev, data);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       /* Then, recursively iterate over all child domains (subdomains) */
>> +       list_for_each_entry(link, &genpd->parent_links, parent_node) {
>> +               struct generic_pm_domain *child_pd = link->child;
>> +
>> +               ret = genpd_for_each_child(child_pd, fn, data);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  #ifdef CONFIG_DEBUG_FS
>>  static struct dentry *genpd_debugfs_dir;
>>
>> diff --git a/drivers/pmdomain/core.h b/drivers/pmdomain/core.h
>> new file mode 100644
>> index 000000000000..a296134d4005
>> --- /dev/null
>> +++ b/drivers/pmdomain/core.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Internal header for PM domain core
>> + *
>> + * Copyright (C) 2026 Kevin Hilman <khilman@baylibre.com>, Texas Instruments
>> + */
>> +
>> +#ifndef __PM_DOMAIN_CORE_H__
>> +#define __PM_DOMAIN_CORE_H__
>> +
>> +#include <linux/pm_domain.h>
>
> I don't think this is needed. Instead please make forward declaration
> of the structs we need. Like:
>
> struct device;
> struct generic_pm_domain;

OK.

Kevin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 4/4] pmdomain: add support system-wide resume latency constraints
  2026-09-16 10:43   ` Ulf Hansson
@ 2026-09-24 19:13     ` Kevin Hilman
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2026-09-24 19:13 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rafael J. Wysocki, linux-pm, Ulf Hansson, linux-kernel,
	Abel Vesa, Kendall Willis

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

> On Thu, Aug 27, 2026 at 12:23 AM Kevin Hilman (TI) <khilman@baylibre.com> wrote:
>>
>> In addition to checking for CPU latency constraints when checking if
>> OK to power down a domain, also check for QoS latency constraints in
>> all devices of a domain and use that in determining the final latency
>> constraint to use for the domain.
>>
>> Since cpu_system_power_down_ok() is used for system-wide suspend, the
>> per-device constratints are only relevant if the LATENCY_SYS QoS flag
>> is set.
>
> cpu_system_power_down_ok() is especially used for genpd's that have
> the GENPD_FLAG_CPU_DOMAIN bit set (cpuidle-psci-domain and
> cpuidle-riscv-sbi).
>
> In other words, this has no effect on other types of PM domains that
> are managed by genpd. Are you planning on adding that on top or this
> is sufficient for your use cases?

This is sufficient for my use cases.

>> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
>> Reviewed-by: Kendall Willis <k-willis@ti.com>
>> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
>> ---
>>  drivers/pmdomain/governor.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>
>> diff --git a/drivers/pmdomain/governor.c b/drivers/pmdomain/governor.c
>> index 96737abbb496..1a85fd375db9 100644
>> --- a/drivers/pmdomain/governor.c
>> +++ b/drivers/pmdomain/governor.c
>> @@ -13,6 +13,8 @@
>>  #include <linux/cpumask.h>
>>  #include <linux/ktime.h>
>>
>> +#include "core.h"
>> +
>>  static int dev_update_qos_constraint(struct device *dev, void *data)
>>  {
>>         s64 *constraint_ns_p = data;
>> @@ -425,17 +427,71 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
>>         return true;
>>  }
>>
>> +/**
>> + * check_device_qos_latency - Callback to check device QoS latency constraints
>> + * @dev: Device to check
>> + * @data: Pointer to s32 variable holding minimum latency found so far
>> + *
>> + * This callback checks if the device has a system-wide resume latency QoS
>> + * constraint and updates the minimum latency if this device has a stricter
>> + * constraint.
>> + *
>> + * This runs in atomic context: for a CPU domain the genpd lock is a raw
>> + * spinlock and the s2idle path runs in the syscore suspend window with
>> + * interrupts disabled.  The lockless dev_pm_qos_raw_*() accessors must
>> + * therefore be used here; the locked dev_pm_qos_read_value() /
>> + * dev_pm_qos_flags() would take dev->power.lock, which is a sleeping lock
>> + * on PREEMPT_RT and must not be acquired in this context.  The values read
>> + * are best-effort, which matches the sibling cpu_power_down_ok() governor.
>
> This is a bit too much in my opinion, please leave out the parts
> concerning the syscore/atomic/lockless parts.
> If we want that information to be described (I guess it would make
> sense), I suggest we add that along with cpu_system_power_down_ok()
> instead as it better belongs there.

OK, sounds good.  I'll move it.

>> + *
>> + * The system-wide flag is checked first so that devices that have not opted
>> + * in only incur a single lockless read.
>> + *
>> + * Returns: 0 to continue iteration.
>> + */
>> +static int check_device_qos_latency(struct device *dev, void *data)
>> +{
>> +       s32 *min_dev_latency = data;
>> +       s32 dev_latency;
>> +
>> +       if (!(dev_pm_qos_raw_flags(dev) & PM_QOS_FLAG_LATENCY_SYS))
>> +               return 0;
>> +
>> +       dev_latency = dev_pm_qos_raw_resume_latency(dev);
>> +       if (dev_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) {
>> +               dev_dbg(dev,
>> +                       "has QoS system-wide resume latency=%d\n",
>> +                       dev_latency);
>
> Do we really need a dev_dbg() here? Leftover from debugging?

Not needed, debug leftover.

>> +               if (dev_latency < *min_dev_latency)
>> +                       *min_dev_latency = dev_latency;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  static bool cpu_system_power_down_ok(struct dev_pm_domain *pd)
>>  {
>>         s64 constraint_ns = cpu_wakeup_latency_qos_limit() * NSEC_PER_USEC;
>>         struct generic_pm_domain *genpd = pd_to_genpd(pd);
>>         int state_idx = genpd->state_count - 1;
>> +       s32 min_dev_latency = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>> +       s64 min_dev_latency_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;
>
> We don't need to assign a default value for min_dev_latency_ns.

OK.

>>
>>         if (!(genpd->flags & GENPD_FLAG_CPU_DOMAIN)) {
>>                 genpd->state_idx = state_idx;
>>                 return true;
>>         }
>>
>> +       genpd_for_each_child(genpd, check_device_qos_latency,
>> +                            &min_dev_latency);
>> +
>> +       /* If device latency < CPU wakeup latency, use it instead */
>> +       if (min_dev_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) {
>> +               min_dev_latency_ns = min_dev_latency * NSEC_PER_USEC;
>> +               if (min_dev_latency_ns < constraint_ns)
>> +                       constraint_ns = min_dev_latency_ns;
>> +       }
>> +
>>         /* Find the deepest state for the latency constraint. */
>>         while (state_idx >= 0) {
>>                 s64 latency_ns = genpd->states[state_idx].power_off_latency_ns +
>>
>> --
>> 2.47.3
>>

Kevin

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-09-24 19:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-26 22:23 [PATCH v5 0/4] PM: QoS/pmdomains: support resume latencies for system-wide PM Kevin Hilman (TI)
2026-08-26 22:23 ` [PATCH v5 1/4] PM / QoS: add flag to indicate latency applies system-wide Kevin Hilman (TI)
2026-08-26 22:23 ` [PATCH v5 2/4] PM / QoS: add lockless read for flags Kevin Hilman (TI)
2026-09-11 13:59   ` Rafael J. Wysocki (Intel)
2026-08-26 22:23 ` [PATCH v5 3/4] pmdomain: core: add genpd_for_each_child() helper Kevin Hilman (TI)
2026-09-16 10:04   ` Ulf Hansson
2026-09-24 18:51     ` Kevin Hilman
2026-08-26 22:23 ` [PATCH v5 4/4] pmdomain: add support system-wide resume latency constraints Kevin Hilman (TI)
2026-09-16 10:43   ` Ulf Hansson
2026-09-24 19:13     ` Kevin Hilman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®