All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Javi Merino <javi.merino@arm.com>
To: linux-pm@vger.kernel.org
Cc: dmitry.torokhov@gmail.com, cywang@chromium.org,
	linux-kernel@vger.kernel.org, punit.agrawal@arm.com,
	djkurtz@chromium.org, edubezval@gmail.com,
	Javi Merino <javi.merino@arm.com>,
	Zhang Rui <rui.zhang@intel.com>
Subject: [PATCH v5 3/5] thermal: power_allocator: relax the requirement of two passive trip points
Date: Mon,  7 Sep 2015 14:19:27 +0100	[thread overview]
Message-ID: <1441631969-3272-4-git-send-email-javi.merino@arm.com> (raw)
In-Reply-To: <1441631969-3272-1-git-send-email-javi.merino@arm.com>

The power allocator governor currently requires that the thermal zone
has at least two passive trip points.  If there aren't, the governor
refuses to bind to the thermal zone.

This commit relaxes that requirement.  Now the governor will bind to all
thermal zones regardless of how many trip points they have.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 Documentation/thermal/power_allocator.txt |   2 +-
 drivers/thermal/power_allocator.c         | 101 +++++++++++++++++-------------
 2 files changed, 58 insertions(+), 45 deletions(-)

diff --git a/Documentation/thermal/power_allocator.txt b/Documentation/thermal/power_allocator.txt
index c3797b529991..a1ce2235f121 100644
--- a/Documentation/thermal/power_allocator.txt
+++ b/Documentation/thermal/power_allocator.txt
@@ -4,7 +4,7 @@ Power allocator governor tunables
 Trip points
 -----------
 
-The governor requires the following two passive trip points:
+The governor works optimally with the following two passive trip points:
 
 1.  "switch on" trip point: temperature above which the governor
     control loop starts operating.  This is the first passive trip
diff --git a/drivers/thermal/power_allocator.c b/drivers/thermal/power_allocator.c
index 7fa6685f9c5b..06e954cd81cc 100644
--- a/drivers/thermal/power_allocator.c
+++ b/drivers/thermal/power_allocator.c
@@ -24,6 +24,8 @@
 
 #include "thermal_core.h"
 
+#define INVALID_TRIP -1
+
 #define FRAC_BITS 10
 #define int_to_frac(x) ((x) << FRAC_BITS)
 #define frac_to_int(x) ((x) >> FRAC_BITS)
@@ -61,6 +63,8 @@ static inline s64 div_frac(s64 x, s64 y)
  *		Used to calculate the derivative term.
  * @trip_switch_on:	first passive trip point of the thermal zone.  The
  *			governor switches on when this trip point is crossed.
+ *			If the thermal zone only has one passive trip point,
+ *			@trip_switch_on should be INVALID_TRIP.
  * @trip_max_desired_temperature:	last passive trip point of the thermal
  *					zone.  The temperature we are
  *					controlling for.
@@ -432,43 +436,66 @@ unlock:
 	return ret;
 }
 
-static int get_governor_trips(struct thermal_zone_device *tz,
-			      struct power_allocator_params *params)
+/**
+ * get_governor_trips() - get the number of the two trip points that are key for this governor
+ * @tz:	thermal zone to operate on
+ * @params:	pointer to private data for this governor
+ *
+ * The power allocator governor works optimally with two trips points:
+ * a "switch on" trip point and a "maximum desired temperature".  These
+ * are defined as the first and last passive trip points.
+ *
+ * If there is only one trip point, then that's considered to be the
+ * "maximum desired temperature" trip point and the governor is always
+ * on.  If there are no passive or active trip points, then the
+ * governor won't do anything.  In fact, its throttle function
+ * won't be called at all.
+ */
+static void get_governor_trips(struct thermal_zone_device *tz,
+			       struct power_allocator_params *params)
 {
-	int i, ret, last_passive;
+	int i, last_active, last_passive;
 	bool found_first_passive;
 
 	found_first_passive = false;
-	last_passive = -1;
-	ret = -EINVAL;
+	last_active = INVALID_TRIP;
+	last_passive = INVALID_TRIP;
 
 	for (i = 0; i < tz->trips; i++) {
 		enum thermal_trip_type type;
+		int ret;
 
 		ret = tz->ops->get_trip_type(tz, i, &type);
-		if (ret)
-			return ret;
+		if (ret) {
+			dev_warn(&tz->device,
+				 "Failed to get trip point %d type: %d\n", i,
+				 ret);
+			continue;
+		}
 
-		if (!found_first_passive) {
-			if (type == THERMAL_TRIP_PASSIVE) {
+		if (type == THERMAL_TRIP_PASSIVE) {
+			if (!found_first_passive) {
 				params->trip_switch_on = i;
 				found_first_passive = true;
+			} else  {
+				last_passive = i;
 			}
-		} else if (type == THERMAL_TRIP_PASSIVE) {
-			last_passive = i;
+		} else if (type == THERMAL_TRIP_ACTIVE) {
+			last_active = i;
 		} else {
 			break;
 		}
 	}
 
-	if (last_passive != -1) {
+	if (last_passive != INVALID_TRIP) {
 		params->trip_max_desired_temperature = last_passive;
-		ret = 0;
+	} else if (found_first_passive) {
+		params->trip_max_desired_temperature = params->trip_switch_on;
+		params->trip_switch_on = INVALID_TRIP;
 	} else {
-		ret = -EINVAL;
+		params->trip_switch_on = INVALID_TRIP;
+		params->trip_max_desired_temperature = last_active;
 	}
-
-	return ret;
 }
 
 static void reset_pid_controller(struct power_allocator_params *params)
@@ -497,11 +524,10 @@ static void allow_maximum_power(struct thermal_zone_device *tz)
  * power_allocator_bind() - bind the power_allocator governor to a thermal zone
  * @tz:	thermal zone to bind it to
  *
- * Check that the thermal zone is valid for this governor, that is, it
- * has two thermal trips.  If so, initialize the PID controller
- * parameters and bind it to the thermal zone.
+ * Initialize the PID controller parameters and bind it to the thermal
+ * zone.
  *
- * Return: 0 on success, -EINVAL if the trips were invalid or -ENOMEM
+ * Return: 0 on success, -EINVAL if the thermal zone doesn't have tzp or -ENOMEM
  * if we ran out of memory.
  */
 static int power_allocator_bind(struct thermal_zone_device *tz)
@@ -520,30 +546,23 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
 	if (!tz->tzp->sustainable_power)
 		dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
 
-	ret = get_governor_trips(tz, params);
-	if (ret) {
-		dev_err(&tz->device,
-			"thermal zone %s has wrong trip setup for power allocator\n",
-			tz->type);
-		goto free;
-	}
+	get_governor_trips(tz, params);
 
-	ret = tz->ops->get_trip_temp(tz, params->trip_max_desired_temperature,
-				     &control_temp);
-	if (ret)
-		goto free;
+	if (tz->trips > 0) {
+		ret = tz->ops->get_trip_temp(tz,
+					params->trip_max_desired_temperature,
+					&control_temp);
+		if (!ret)
+			estimate_pid_constants(tz, tz->tzp->sustainable_power,
+					       params->trip_switch_on,
+					       control_temp, false);
+	}
 
-	estimate_pid_constants(tz, tz->tzp->sustainable_power,
-			       params->trip_switch_on, control_temp, false);
 	reset_pid_controller(params);
 
 	tz->governor_data = params;
 
 	return 0;
-
-free:
-	kfree(params);
-	return ret;
 }
 
 static void power_allocator_unbind(struct thermal_zone_device *tz)
@@ -574,13 +593,7 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip)
 
 	ret = tz->ops->get_trip_temp(tz, params->trip_switch_on,
 				     &switch_on_temp);
-	if (ret) {
-		dev_warn(&tz->device,
-			 "Failed to get switch on temperature: %d\n", ret);
-		return ret;
-	}
-
-	if (current_temp < switch_on_temp) {
+	if (!ret && (current_temp < switch_on_temp)) {
 		tz->passive = 0;
 		reset_pid_controller(params);
 		allow_maximum_power(tz);
-- 
1.9.1


  parent reply	other threads:[~2015-09-07 13:20 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-04 16:39 [PATCH] thermal: remove power allocator from list of default governors Dmitry Torokhov
2015-08-05  8:37 ` Javi Merino
2015-08-05 16:18   ` Srinivas Pandruvada
2015-08-05 16:35   ` Dmitry Torokhov
2015-08-05 18:49     ` Eduardo Valentin
2015-08-06  8:30       ` Javi Merino
2015-08-10 16:04         ` [PATCH 0/3] Let the power allocator thermal governor run on any thermal zone Javi Merino
2015-08-10 16:04           ` [PATCH 1/3] thermal: power_allocator: relax the requirement of a sustainable_power in tzp Javi Merino
2015-08-10 16:04           ` [PATCH 2/3] thermal: power_allocator: relax the requirement of two passive trip points Javi Merino
2015-08-10 16:04           ` [PATCH 3/3] thermal: power_allocator: exit early if there are no cooling devices Javi Merino
2015-08-11 10:21           ` [PATCH v2 0/4] Let the power allocator thermal governor run on any thermal zone Javi Merino
2015-08-11 10:21             ` [PATCH v2 1/4] thermal: power_allocator: relax the requirement of a sustainable_power in tzp Javi Merino
2015-08-11 13:42               ` Punit Agrawal
2015-08-11 17:37                 ` Javi Merino
2015-08-12 11:05               ` Daniel Kurtz
2015-08-11 10:21             ` [PATCH v2 2/4] thermal: power_allocator: relax the requirement of two passive trip points Javi Merino
2015-08-12 11:13               ` Daniel Kurtz
2015-08-12 16:46                 ` Javi Merino
2015-08-11 10:21             ` [PATCH v2 3/4] thermal: power_allocator: don't require tzp to be present for the thermal zone Javi Merino
2015-08-11 10:21             ` [PATCH v2 4/4] thermal: power_allocator: exit early if there are no cooling devices Javi Merino
2015-08-17 17:36             ` [PATCH v3 0/4] Let the power allocator thermal governor run on any thermal zone Javi Merino
2015-08-17 17:36               ` [PATCH v3 1/4] thermal: power_allocator: relax the requirement of a sustainable_power in tzp Javi Merino
2015-08-20 22:16                 ` Eduardo Valentin
2015-08-24 18:37                   ` Javi Merino
2015-08-17 17:36               ` [PATCH v3 2/4] thermal: power_allocator: relax the requirement of two passive trip points Javi Merino
2015-08-17 17:36               ` [PATCH v3 3/4] thermal: power_allocator: don't require tzp to be present for the thermal zone Javi Merino
2015-08-17 17:36               ` [PATCH v3 4/4] thermal: power_allocator: exit early if there are no cooling devices Javi Merino
2015-08-26 13:26               ` [PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone Javi Merino
2015-08-26 13:26                 ` [PATCH v4 1/5] thermal: Add a function to get the minimum power Javi Merino
2015-08-26 13:26                 ` [PATCH v4 2/5] thermal: power_allocator: relax the requirement of a sustainable_power in tzp Javi Merino
2015-08-26 13:26                 ` [PATCH v4 3/5] thermal: power_allocator: relax the requirement of two passive trip points Javi Merino
2015-08-26 13:26                 ` [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone Javi Merino
2015-08-28  2:18                   ` Daniel Kurtz
2015-08-28 16:28                     ` Javi Merino
2015-08-31 13:14                       ` Daniel Kurtz
2015-08-26 13:26                 ` [PATCH v4 5/5] thermal: power_allocator: exit early if there are no cooling devices Javi Merino
2015-09-02 15:11                 ` [PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone Javi Merino
2015-09-07 13:19                 ` [PATCH v5 " Javi Merino
2015-09-07 13:19                   ` [PATCH v5 1/5] thermal: Add a function to get the minimum power Javi Merino
2015-09-07 13:19                   ` [PATCH v5 2/5] thermal: power_allocator: relax the requirement of a sustainable_power in tzp Javi Merino
2015-09-07 13:19                   ` Javi Merino [this message]
2015-09-07 13:19                   ` [PATCH v5 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone Javi Merino
2015-09-07 13:19                   ` [PATCH v5 5/5] thermal: power_allocator: exit early if there are no cooling devices Javi Merino
2015-09-08  1:46                   ` [PATCH v5 0/5] Let the power allocator thermal governor run on any thermal zone Daniel Kurtz
2015-09-14 13:23                   ` [PATCH v6 " Javi Merino
2015-09-14 13:23                     ` [PATCH v6 1/5] thermal: Add a function to get the minimum power Javi Merino
2015-09-14 13:23                     ` [PATCH v6 2/5] thermal: power_allocator: relax the requirement of a sustainable_power in tzp Javi Merino
2015-09-14 13:23                     ` [PATCH v6 3/5] thermal: power_allocator: relax the requirement of two passive trip points Javi Merino
2015-09-14 13:23                     ` [PATCH v6 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone Javi Merino
2015-09-14 13:23                     ` [PATCH v6 5/5] thermal: power_allocator: exit early if there are no cooling devices Javi Merino
2015-09-14  3:04                 ` [PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone Eduardo Valentin
2015-09-14 13:32                   ` Javi Merino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1441631969-3272-4-git-send-email-javi.merino@arm.com \
    --to=javi.merino@arm.com \
    --cc=cywang@chromium.org \
    --cc=djkurtz@chromium.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=edubezval@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=punit.agrawal@arm.com \
    --cc=rui.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.