* [PATCH v2] pmdomain: governor: Consider CPU latency tolerance from pm_domain_cpu_gov
@ 2025-07-09 8:30 Maulik Shah
2025-07-09 11:30 ` Ulf Hansson
0 siblings, 1 reply; 2+ messages in thread
From: Maulik Shah @ 2025-07-09 8:30 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-pm, linux-kernel, linux-arm-msm, Maulik Shah
pm_domain_cpu_gov is selecting a cluster idle state but does not consider
latency tolerance of child CPUs. This results in deeper cluster idle state
whose latency does not meet latency tolerance requirement.
Select deeper idle state only if global and device latency tolerance of all
child CPUs meet.
Test results on SM8750 with 300 usec PM-QoS on CPU0 which is less than
domain idle state entry (2150) + exit (1983) usec latency mentioned in
devicetree, demonstrate the issue.
# echo 300 > /sys/devices/system/cpu/cpu0/power/pm_qos_resume_latency_us
Before: (Usage is incrementing)
======
# cat /sys/kernel/debug/pm_genpd/power-domain-cluster0/idle_states
State Time Spent(ms) Usage Rejected Above Below
S0 29817 537 8 270 0
# cat /sys/kernel/debug/pm_genpd/power-domain-cluster0/idle_states
State Time Spent(ms) Usage Rejected Above Below
S0 30348 542 8 271 0
After: (Usage is not incrementing due to latency tolerance)
======
# cat /sys/kernel/debug/pm_genpd/power-domain-cluster0/idle_states
State Time Spent(ms) Usage Rejected Above Below
S0 39319 626 14 307 0
# cat /sys/kernel/debug/pm_genpd/power-domain-cluster0/idle_states
State Time Spent(ms) Usage Rejected Above Below
S0 39319 626 14 307 0
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
Changes in v2:
- Rename device pointer to cpu_dev
- Replace dev_pm_qos_read_value() with dev_pm_qos_raw_resume_latency()
- Link to v1: https://lore.kernel.org/all/20250708-pmdomain_qos-v1-1-7c502f4c901a@oss.qualcomm.com
---
drivers/pmdomain/governor.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/pmdomain/governor.c b/drivers/pmdomain/governor.c
index c1e148657c873a6b5b4d9c0f058d54cb020c56e2..39359811a93047b36443a1b9583962726f24b88b 100644
--- a/drivers/pmdomain/governor.c
+++ b/drivers/pmdomain/governor.c
@@ -8,6 +8,7 @@
#include <linux/pm_domain.h>
#include <linux/pm_qos.h>
#include <linux/hrtimer.h>
+#include <linux/cpu.h>
#include <linux/cpuidle.h>
#include <linux/cpumask.h>
#include <linux/ktime.h>
@@ -349,6 +350,8 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
struct cpuidle_device *dev;
ktime_t domain_wakeup, next_hrtimer;
ktime_t now = ktime_get();
+ struct device *cpu_dev;
+ s64 cpu_constraint, global_constraint;
s64 idle_duration_ns;
int cpu, i;
@@ -359,6 +362,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
if (!(genpd->flags & GENPD_FLAG_CPU_DOMAIN))
return true;
+ global_constraint = cpu_latency_qos_limit();
/*
* Find the next wakeup for any of the online CPUs within the PM domain
* and its subdomains. Note, we only need the genpd->cpus, as it already
@@ -372,8 +376,16 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
if (ktime_before(next_hrtimer, domain_wakeup))
domain_wakeup = next_hrtimer;
}
+
+ cpu_dev = get_cpu_device(cpu);
+ if (cpu_dev) {
+ cpu_constraint = dev_pm_qos_raw_resume_latency(cpu_dev);
+ if (cpu_constraint < global_constraint)
+ global_constraint = cpu_constraint;
+ }
}
+ global_constraint *= NSEC_PER_USEC;
/* The minimum idle duration is from now - until the next wakeup. */
idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
if (idle_duration_ns <= 0)
@@ -389,8 +401,10 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
*/
i = genpd->state_idx;
do {
- if (idle_duration_ns >= (genpd->states[i].residency_ns +
- genpd->states[i].power_off_latency_ns)) {
+ if ((idle_duration_ns >= (genpd->states[i].residency_ns +
+ genpd->states[i].power_off_latency_ns)) &&
+ (global_constraint >= (genpd->states[i].power_on_latency_ns +
+ genpd->states[i].power_off_latency_ns))) {
genpd->state_idx = i;
genpd->gd->last_enter = now;
genpd->gd->reflect_residency = true;
---
base-commit: 26ffb3d6f02cd0935fb9fa3db897767beee1cb2a
change-id: 20250708-pmdomain_qos-29077f8b622e
Best regards,
--
Maulik Shah <maulik.shah@oss.qualcomm.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] pmdomain: governor: Consider CPU latency tolerance from pm_domain_cpu_gov
2025-07-09 8:30 [PATCH v2] pmdomain: governor: Consider CPU latency tolerance from pm_domain_cpu_gov Maulik Shah
@ 2025-07-09 11:30 ` Ulf Hansson
0 siblings, 0 replies; 2+ messages in thread
From: Ulf Hansson @ 2025-07-09 11:30 UTC (permalink / raw)
To: Maulik Shah; +Cc: linux-pm, linux-kernel, linux-arm-msm
On Wed, 9 Jul 2025 at 10:30, Maulik Shah <maulik.shah@oss.qualcomm.com> wrote:
>
> pm_domain_cpu_gov is selecting a cluster idle state but does not consider
> latency tolerance of child CPUs. This results in deeper cluster idle state
> whose latency does not meet latency tolerance requirement.
>
> Select deeper idle state only if global and device latency tolerance of all
> child CPUs meet.
>
> Test results on SM8750 with 300 usec PM-QoS on CPU0 which is less than
> domain idle state entry (2150) + exit (1983) usec latency mentioned in
> devicetree, demonstrate the issue.
>
> # echo 300 > /sys/devices/system/cpu/cpu0/power/pm_qos_resume_latency_us
>
> Before: (Usage is incrementing)
> ======
> # cat /sys/kernel/debug/pm_genpd/power-domain-cluster0/idle_states
> State Time Spent(ms) Usage Rejected Above Below
> S0 29817 537 8 270 0
>
> # cat /sys/kernel/debug/pm_genpd/power-domain-cluster0/idle_states
> State Time Spent(ms) Usage Rejected Above Below
> S0 30348 542 8 271 0
>
> After: (Usage is not incrementing due to latency tolerance)
> ======
> # cat /sys/kernel/debug/pm_genpd/power-domain-cluster0/idle_states
> State Time Spent(ms) Usage Rejected Above Below
> S0 39319 626 14 307 0
>
> # cat /sys/kernel/debug/pm_genpd/power-domain-cluster0/idle_states
> State Time Spent(ms) Usage Rejected Above Below
> S0 39319 626 14 307 0
>
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
Applied for fixes and by adding fixes/table-tags, thanks!
Kind regards
Uffe
> ---
> Changes in v2:
> - Rename device pointer to cpu_dev
> - Replace dev_pm_qos_read_value() with dev_pm_qos_raw_resume_latency()
> - Link to v1: https://lore.kernel.org/all/20250708-pmdomain_qos-v1-1-7c502f4c901a@oss.qualcomm.com
> ---
> drivers/pmdomain/governor.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pmdomain/governor.c b/drivers/pmdomain/governor.c
> index c1e148657c873a6b5b4d9c0f058d54cb020c56e2..39359811a93047b36443a1b9583962726f24b88b 100644
> --- a/drivers/pmdomain/governor.c
> +++ b/drivers/pmdomain/governor.c
> @@ -8,6 +8,7 @@
> #include <linux/pm_domain.h>
> #include <linux/pm_qos.h>
> #include <linux/hrtimer.h>
> +#include <linux/cpu.h>
> #include <linux/cpuidle.h>
> #include <linux/cpumask.h>
> #include <linux/ktime.h>
> @@ -349,6 +350,8 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
> struct cpuidle_device *dev;
> ktime_t domain_wakeup, next_hrtimer;
> ktime_t now = ktime_get();
> + struct device *cpu_dev;
> + s64 cpu_constraint, global_constraint;
> s64 idle_duration_ns;
> int cpu, i;
>
> @@ -359,6 +362,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
> if (!(genpd->flags & GENPD_FLAG_CPU_DOMAIN))
> return true;
>
> + global_constraint = cpu_latency_qos_limit();
> /*
> * Find the next wakeup for any of the online CPUs within the PM domain
> * and its subdomains. Note, we only need the genpd->cpus, as it already
> @@ -372,8 +376,16 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
> if (ktime_before(next_hrtimer, domain_wakeup))
> domain_wakeup = next_hrtimer;
> }
> +
> + cpu_dev = get_cpu_device(cpu);
> + if (cpu_dev) {
> + cpu_constraint = dev_pm_qos_raw_resume_latency(cpu_dev);
> + if (cpu_constraint < global_constraint)
> + global_constraint = cpu_constraint;
> + }
> }
>
> + global_constraint *= NSEC_PER_USEC;
> /* The minimum idle duration is from now - until the next wakeup. */
> idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
> if (idle_duration_ns <= 0)
> @@ -389,8 +401,10 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
> */
> i = genpd->state_idx;
> do {
> - if (idle_duration_ns >= (genpd->states[i].residency_ns +
> - genpd->states[i].power_off_latency_ns)) {
> + if ((idle_duration_ns >= (genpd->states[i].residency_ns +
> + genpd->states[i].power_off_latency_ns)) &&
> + (global_constraint >= (genpd->states[i].power_on_latency_ns +
> + genpd->states[i].power_off_latency_ns))) {
> genpd->state_idx = i;
> genpd->gd->last_enter = now;
> genpd->gd->reflect_residency = true;
>
> ---
> base-commit: 26ffb3d6f02cd0935fb9fa3db897767beee1cb2a
> change-id: 20250708-pmdomain_qos-29077f8b622e
>
> Best regards,
> --
> Maulik Shah <maulik.shah@oss.qualcomm.com>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-07-09 11:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-09 8:30 [PATCH v2] pmdomain: governor: Consider CPU latency tolerance from pm_domain_cpu_gov Maulik Shah
2025-07-09 11:30 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).