On 19 July 2018 at 12:35, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > On Wednesday, June 20, 2018 7:22:08 PM CEST Ulf Hansson wrote: >> CPU devices and other regular devices may share the same PM domain and may >> also be hierarchically related via subdomains. In either case, all devices >> including CPUs, may be attached to a PM domain managed by genpd, that has >> an idle state with an enter/exit latency. >> >> Let's take these latencies into account in the state selection process by >> genpd's governor for CPUs. This means the governor, pm_domain_cpu_gov, >> becomes extended to satisfy both a state's residency and a potential dev PM >> QoS constraint. >> >> Cc: Lina Iyer <ilina@xxxxxxxxxxxxxx> >> Co-developed-by: Lina Iyer <lina.iyer@xxxxxxxxxx> >> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> >> --- >> drivers/base/power/domain_governor.c | 15 +++++++++++---- >> include/linux/pm_domain.h | 1 + >> 2 files changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c >> index 1aad55719537..03d4e9454ce9 100644 >> --- a/drivers/base/power/domain_governor.c >> +++ b/drivers/base/power/domain_governor.c >> @@ -214,8 +214,10 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) >> struct generic_pm_domain *genpd = pd_to_genpd(pd); >> struct gpd_link *link; >> >> - if (!genpd->max_off_time_changed) >> + if (!genpd->max_off_time_changed) { >> + genpd->state_idx = genpd->cached_power_down_state_idx; >> return genpd->cached_power_down_ok; >> + } >> >> /* >> * We have to invalidate the cached results for the masters, so >> @@ -240,6 +242,7 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) >> genpd->state_idx--; >> } >> >> + genpd->cached_power_down_state_idx = genpd->state_idx; >> return genpd->cached_power_down_ok; >> } >> >> @@ -255,6 +258,10 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd) >> s64 idle_duration_ns; >> int cpu, i; >> >> + /* Validate dev PM QoS constraints. */ >> + if (!default_power_down_ok(pd)) >> + return false; >> + >> if (!(genpd->flags & GENPD_FLAG_CPU_DOMAIN)) >> return true; >> >> @@ -276,9 +283,9 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd) >> /* >> * Find the deepest idle state that has its residency value satisfied >> * and by also taking into account the power off latency for the state. >> - * Start at the deepest supported state. >> + * Start at the state picked by the dev PM QoS constraint validation. >> */ >> - i = genpd->state_count - 1; >> + i = genpd->state_idx; >> do { >> if (!genpd->states[i].residency_ns) >> break; >> @@ -312,6 +319,6 @@ struct dev_power_governor pm_domain_always_on_gov = { >> }; >> >> struct dev_power_governor pm_domain_cpu_gov = { >> - .suspend_ok = NULL, >> + .suspend_ok = default_suspend_ok, >> .power_down_ok = cpu_power_down_ok, >> }; >> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h >> index 97901c833108..dbc69721cad8 100644 >> --- a/include/linux/pm_domain.h >> +++ b/include/linux/pm_domain.h >> @@ -81,6 +81,7 @@ struct generic_pm_domain { >> s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ >> bool max_off_time_changed; >> bool cached_power_down_ok; >> + bool cached_power_down_state_idx; >> int (*attach_dev)(struct generic_pm_domain *domain, >> struct device *dev); >> void (*detach_dev)(struct generic_pm_domain *domain, >> > > I don't see much value in splitting this patch off [07/26] and it actually > confused me, so it may as well confuse someone else. > The idea was to let people, explicitly, comment on the whether dev PM Qos constraints should be considered by the governor. However, I get your point, let's combine them! Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html