[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <201204242355.48498.rjw@sisk.pl>
Date: Tue, 24 Apr 2012 23:55:48 +0200
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Linux PM list <linux-pm@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Kevin Hilman <khilman@...com>,
Magnus Damm <magnus.damm@...il.com>, linux-sh@...r.kernel.org,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
markgross@...gnar.org, Jean Pihet <j-pihet@...com>
Subject: Re: [PATCH 2/3] PM / Domains: Rework default device stop governor function
The subject is wrong, it should be:
[PATCH 2/3] PM / Domains: Rework default domain power off governor function
On Tuesday, April 24, 2012, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@...k.pl>
>
> The existing default domain power down governor function for PM
> domains, default_power_down_ok(), is supposed to check whether or not
> the PM QoS latency constraints of the devices in the domain will be
> violated if the domain is turned off by pm_genpd_poweroff().
> However, the computations carried out by it don't reflect the
> definition of the PM QoS latency constrait in
> Documentation/ABI/testing/sysfs-devices-power.
>
> Make default_power_down_ok() follow the definition of the PM QoS
> latency constrait. In particular, make it only take latencies into
> account, because it doesn't matter how much time has elapsed since
> the domain's devices were suspended for the computation.
>
> Remove the break_even_ns and power_off_time fields from
> struct generic_pm_domain, because they are not necessary any more.
>
> Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
> ---
> drivers/base/power/domain.c | 1
> drivers/base/power/domain_governor.c | 62 ++++++++++++++---------------------
> include/linux/pm_domain.h | 2 -
> 3 files changed, 25 insertions(+), 40 deletions(-)
>
> Index: linux/drivers/base/power/domain_governor.c
> ===================================================================
> --- linux.orig/drivers/base/power/domain_governor.c
> +++ linux/drivers/base/power/domain_governor.c
> @@ -80,8 +80,8 @@ static bool default_power_down_ok(struct
> struct pm_domain_data *pdd;
> s64 min_dev_off_time_ns;
> s64 off_on_time_ns;
> - ktime_t time_now = ktime_get();
>
> + genpd->max_off_time_ns = -1;
> off_on_time_ns = genpd->power_off_latency_ns +
> genpd->power_on_latency_ns;
> /*
> @@ -109,8 +109,6 @@ static bool default_power_down_ok(struct
> if (sd_max_off_ns < 0)
> continue;
>
> - sd_max_off_ns -= ktime_to_ns(ktime_sub(time_now,
> - sd->power_off_time));
> /*
> * Check if the subdomain is allowed to be off long enough for
> * the current domain to turn off and on (that's how much time
> @@ -125,53 +123,43 @@ static bool default_power_down_ok(struct
> */
> min_dev_off_time_ns = -1;
> list_for_each_entry(pdd, &genpd->dev_list, list_node) {
> - struct gpd_timing_data *td;
> - struct device *dev = pdd->dev;
> - s64 dev_off_time_ns;
> + s64 constraint_ns;
>
> - if (!dev->driver || dev->power.max_time_suspended_ns < 0)
> + if (!pdd->dev->driver)
> continue;
>
> - td = &to_gpd_data(pdd)->td;
> - dev_off_time_ns = dev->power.max_time_suspended_ns -
> - (td->start_latency_ns + td->restore_state_latency_ns +
> - ktime_to_ns(ktime_sub(time_now,
> - dev->power.suspend_time)));
> - if (dev_off_time_ns <= off_on_time_ns)
> + /*
> + * Check if the device is allowed to be off long enough for the
> + * domain to turn off and on (that's how much time it will
> + * have to wait worst case).
> + */
> + constraint_ns = to_gpd_data(pdd)->td.effective_constraint_ns;
> + if (constraint_ns == 0)
> + continue;
> +
> + if (constraint_ns <= off_on_time_ns)
> return false;
>
> - if (min_dev_off_time_ns > dev_off_time_ns
> + if (min_dev_off_time_ns > constraint_ns
> || min_dev_off_time_ns < 0)
> - min_dev_off_time_ns = dev_off_time_ns;
> - }
> -
> - if (min_dev_off_time_ns < 0) {
> - /*
> - * There are no latency constraints, so the domain can spend
> - * arbitrary time in the "off" state.
> - */
> - genpd->max_off_time_ns = -1;
> - return true;
> + min_dev_off_time_ns = constraint_ns;
> }
>
> /*
> - * The difference between the computed minimum delta and the time needed
> - * to turn the domain on is the maximum theoretical time this domain can
> - * spend in the "off" state.
> + * If the computed minimum device off time is negative, there are no
> + * latency constraints, so the domain can spend arbitrary time in the
> + * "off" state.
> */
> - min_dev_off_time_ns -= genpd->power_on_latency_ns;
> + if (min_dev_off_time_ns < 0)
> + return true;
>
> /*
> - * If the difference between the computed minimum delta and the time
> - * needed to turn the domain off and back on on is smaller than the
> - * domain's power break even time, removing power from the domain is not
> - * worth it.
> + * The difference between the computed minimum device off time and the
> + * time needed to turn the domain on is the maximum theoretical time
> + * this domain can spend in the "off" state.
> */
> - if (genpd->break_even_ns >
> - min_dev_off_time_ns - genpd->power_off_latency_ns)
> - return false;
> -
> - genpd->max_off_time_ns = min_dev_off_time_ns;
> + genpd->max_off_time_ns = min_dev_off_time_ns -
> + genpd->power_on_latency_ns;
> return true;
> }
>
> Index: linux/include/linux/pm_domain.h
> ===================================================================
> --- linux.orig/include/linux/pm_domain.h
> +++ linux/include/linux/pm_domain.h
> @@ -70,9 +70,7 @@ struct generic_pm_domain {
> int (*power_on)(struct generic_pm_domain *domain);
> s64 power_on_latency_ns;
> struct gpd_dev_ops dev_ops;
> - s64 break_even_ns; /* Power break even for the entire domain. */
> s64 max_off_time_ns; /* Maximum allowed "suspended" time. */
> - ktime_t power_off_time;
> struct device_node *of_node; /* Node in device tree */
> };
>
> Index: linux/drivers/base/power/domain.c
> ===================================================================
> --- linux.orig/drivers/base/power/domain.c
> +++ linux/drivers/base/power/domain.c
> @@ -443,7 +443,6 @@ static int pm_genpd_poweroff(struct gene
> }
>
> genpd->status = GPD_STATE_POWER_OFF;
> - genpd->power_off_time = ktime_get();
>
> /* Update PM QoS information for devices in the domain. */
> list_for_each_entry_reverse(pdd, &genpd->dev_list, list_node) {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists