diff mbox

PM / Domains: Fix validation of latency constraints in genpd governor

Message ID 1444745428-4396-1-git-send-email-ulf.hansson@linaro.org
State Accepted
Commit a98f1b78ecf325bf29c9d3d1eb38cbc9340000af
Headers show

Commit Message

Ulf Hansson Oct. 13, 2015, 2:10 p.m. UTC
Commit ba2bbfbf6307 ("PM / Domains: Remove intermediate states from the
power off sequence") changed the power off sequence in genpd. That also
required some updates regarding the validation of latency constraints in
the genpd governor. Unfortunate that wasn't covered, so let's fix this.

From a runtime PM and latency point of view, we need to consider the worst
case scenario while validating latency constraints. That's typically when
a call to pm_runtime_get_sync() needs to wait for a ongoing runtime
suspend operation to be carried out, as it then also needs to wait for the
device to be runtime resumed again.

The above mentioned commit made the genpd governor's ->stop_ok() callback
responsible of validating genpd's device's runtime suspend/resume latency.
In other words, the constraint needs to be validated towards the relevant
latencies present in genpd's ->runtime_suspend|resume() callbacks.

Earlier, that included latencies from the ->stop|start() callbacks, but as
->save|restore_state() are now also being invoked from genpd's
->runtime_suspend|resume() and to comply with the worst case scenario,
let's take also those latencies into account.

Fixes: ba2bbfbf6307 ("PM / Domains: Remove intermediate states from the
power off sequence")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain_governor.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

Comments

Rafael J. Wysocki Oct. 14, 2015, 9:57 p.m. UTC | #1
On Tuesday, October 13, 2015 04:10:28 PM Ulf Hansson wrote:
> Commit ba2bbfbf6307 ("PM / Domains: Remove intermediate states from the
> power off sequence") changed the power off sequence in genpd. That also
> required some updates regarding the validation of latency constraints in
> the genpd governor. Unfortunate that wasn't covered, so let's fix this.
> 
> From a runtime PM and latency point of view, we need to consider the worst
> case scenario while validating latency constraints. That's typically when
> a call to pm_runtime_get_sync() needs to wait for a ongoing runtime
> suspend operation to be carried out, as it then also needs to wait for the
> device to be runtime resumed again.
> 
> The above mentioned commit made the genpd governor's ->stop_ok() callback
> responsible of validating genpd's device's runtime suspend/resume latency.
> In other words, the constraint needs to be validated towards the relevant
> latencies present in genpd's ->runtime_suspend|resume() callbacks.
> 
> Earlier, that included latencies from the ->stop|start() callbacks, but as
> ->save|restore_state() are now also being invoked from genpd's
> ->runtime_suspend|resume() and to comply with the worst case scenario,
> let's take also those latencies into account.
> 
> Fixes: ba2bbfbf6307 ("PM / Domains: Remove intermediate states from the
> power off sequence")
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Applied, thanks!

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index 2a4154a..85e17ba 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -77,13 +77,16 @@  static bool default_stop_ok(struct device *dev)
 				      dev_update_qos_constraint);
 
 	if (constraint_ns > 0) {
-		constraint_ns -= td->start_latency_ns;
+		constraint_ns -= td->save_state_latency_ns +
+				td->stop_latency_ns +
+				td->start_latency_ns +
+				td->restore_state_latency_ns;
 		if (constraint_ns == 0)
 			return false;
 	}
 	td->effective_constraint_ns = constraint_ns;
-	td->cached_stop_ok = constraint_ns > td->stop_latency_ns ||
-				constraint_ns == 0;
+	td->cached_stop_ok = constraint_ns >= 0;
+
 	/*
 	 * The children have been suspended already, so we don't need to take
 	 * their stop latencies into account here.
@@ -126,18 +129,6 @@  static bool default_power_down_ok(struct dev_pm_domain *pd)
 
 	off_on_time_ns = genpd->power_off_latency_ns +
 				genpd->power_on_latency_ns;
-	/*
-	 * It doesn't make sense to remove power from the domain if saving
-	 * the state of all devices in it and the power off/power on operations
-	 * take too much time.
-	 *
-	 * All devices in this domain have been stopped already at this point.
-	 */
-	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
-		if (pdd->dev->driver)
-			off_on_time_ns +=
-				to_gpd_data(pdd)->td.save_state_latency_ns;
-	}
 
 	min_off_time_ns = -1;
 	/*
@@ -193,7 +184,6 @@  static bool default_power_down_ok(struct dev_pm_domain *pd)
 		 * constraint_ns cannot be negative here, because the device has
 		 * been suspended.
 		 */
-		constraint_ns -= td->restore_state_latency_ns;
 		if (constraint_ns <= off_on_time_ns)
 			return false;