Linux Power Management development
 help / color / mirror / Atom feed
* [RFC PATCH 4/12] thermal: introduce .get_trend() callback
From: Zhang Rui @ 2012-06-11  3:20 UTC (permalink / raw)
  To: linux-pm, linux-acpi@vger.kernel.org


tc1 and tc2 are ACPI platform specific concepts, introduce
.get_trend() as a more general solution.


Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/thermal.c        |   32 ++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_sys.c |   19 +++++++++++++++++--
 include/linux/thermal.h       |    9 +++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)

Index: rtd3/drivers/acpi/thermal.c
===================================================================
--- rtd3.orig/drivers/acpi/thermal.c
+++ rtd3/drivers/acpi/thermal.c
@@ -706,6 +706,37 @@ static int thermal_get_crit_temp(struct
 		return -EINVAL;
 }
 
+static int thermal_get_trend(struct thermal_zone_device *thermal,
+				int trip, enum thermal_trend *trend)
+{
+	struct acpi_thermal *tz = thermal->devdata;
+	enum thermal_trip_type type;
+	unsigned long trip_temp;
+	int i;
+
+	if (thermal_get_trip_type(thermal, trip, &type))
+		return -EINVAL;
+
+	/* Only PASSIVE trip points need TREND */
+	if (type != THERMAL_TRIP_PASSIVE)
+		return -EINVAL;
+
+	if (thermal_get_trip_temp(thermal, trip, &trip_temp))
+		return -EINVAL;
+
+	/*
+	 * tz->temperature has already been updated by generic thermal layer,
+	 * before this callback being invoked
+	 */
+	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
+		+ (tz->trips.passive.tc2 * (tz->temperature - trip_temp));
+
+	*trend = i > 0 ? THERMAL_TREND_RAISING :
+		(i < 0 ? THERMAL_TREND_DROPPING : THERMAL_TREND_NONE);
+	return 0;
+}
+
+
 static int thermal_notify(struct thermal_zone_device *thermal, int trip,
 			   enum thermal_trip_type trip_type)
 {
@@ -835,6 +866,7 @@ static const struct thermal_zone_device_
 	.get_trip_type = thermal_get_trip_type,
 	.get_trip_temp = thermal_get_trip_temp,
 	.get_crit_temp = thermal_get_crit_temp,
+	.get_trend = thermal_get_trend,
 	.notify = thermal_notify,
 };
 
Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -657,6 +657,18 @@ thermal_remove_hwmon_sysfs(struct therma
 }
 #endif
 
+static void thermal_get_trend (struct thermal_zone_device *tz,
+		int trip, enum thermal_trend *trend)
+{
+	if (tz->ops->get_trend)
+		if (!tz->ops->get_trend(tz, trip, trend))
+			return;
+
+	*trend = tz->temperature >= tz->last_temperature ?
+		 THERMAL_TREND_RAISING : THERMAL_TREND_DROPPING;
+	return;
+}
+
 static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
 					    int delay)
 {
@@ -691,6 +703,8 @@ static void thermal_zone_device_passive(
 	if (temp >= trip_temp) {
 		tz->passive = true;
 
+		thermal_get_trend(tz, trip, (enum thermal_trend *)&trend);
+
 		trend = (tz->tc1 * (temp - tz->last_temperature)) +
 			(tz->tc2 * (temp - trip_temp));
 
@@ -1049,6 +1063,9 @@ void thermal_zone_device_update(struct t
 		goto leave;
 	}
 
+	tz->last_temperature = tz->temperature;
+	tz->temperature = temp;
+
 	for (count = 0; count < tz->trips; count++) {
 		tz->ops->get_trip_type(tz, count, &trip_type);
 		tz->ops->get_trip_temp(tz, count, &trip_temp);
@@ -1108,8 +1125,6 @@ void thermal_zone_device_update(struct t
 		thermal_zone_device_passive(tz, temp, tz->forced_passive,
 					    THERMAL_TRIPS_NONE);
 
-	tz->last_temperature = temp;
-
 leave:
 	if (tz->passive)
 		thermal_zone_device_set_polling(tz, tz->passive_delay);
Index: rtd3/include/linux/thermal.h
===================================================================
--- rtd3.orig/include/linux/thermal.h
+++ rtd3/include/linux/thermal.h
@@ -44,6 +44,12 @@ enum thermal_trip_type {
 	THERMAL_TRIP_CRITICAL,
 };
 
+enum thermal_trend {
+	THERMAL_TREND_NONE,
+	THERMAL_TREND_RAISING,
+	THERMAL_TREND_DROPPING,
+};
+
 struct thermal_zone_device_ops {
 	int (*bind) (struct thermal_zone_device *,
 		     struct thermal_cooling_device *);
@@ -59,6 +65,8 @@ struct thermal_zone_device_ops {
 	int (*get_trip_temp) (struct thermal_zone_device *, int,
 			      unsigned long *);
 	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
+	int (*get_trend) (struct thermal_zone_device *, int,
+			  enum thermal_trend *);
 	int (*notify) (struct thermal_zone_device *, int,
 		       enum thermal_trip_type);
 };
@@ -95,6 +103,7 @@ struct thermal_zone_device {
 	int tc2;
 	int passive_delay;
 	int polling_delay;
+	int temperature;
 	int last_temperature;
 	bool passive;
 	unsigned int forced_passive;

^ permalink raw reply

* [RFC PATCH 5/12] thermal: remove tc1/tc2 in generic thermal layer
From: Zhang Rui @ 2012-06-11  3:20 UTC (permalink / raw)
  To: linux-pm, linux-acpi@vger.kernel.org
  Cc: Amit Kachhap, eduardo, R, Durgadoss, Len, Brown,
	Rafael J. Wysocki, Matthew Garrett


Remove tc1/tc2 in generic thermal layer.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/thermal.c                   |    5 +----
 drivers/platform/x86/acerhdf.c           |    2 +-
 drivers/platform/x86/intel_mid_thermal.c |    2 +-
 drivers/thermal/spear_thermal.c          |    2 +-
 drivers/thermal/thermal_sys.c            |   15 ++-------------
 include/linux/thermal.h                  |    4 +---
 6 files changed, 7 insertions(+), 23 deletions(-)

Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -266,9 +266,6 @@ passive_store(struct device *dev, struct
 		tz->passive_delay = 0;
 	}
 
-	tz->tc1 = 1;
-	tz->tc2 = 1;
-
 	tz->forced_passive = state;
 
 	thermal_zone_device_update(tz);
@@ -705,9 +702,6 @@ static void thermal_zone_device_passive(
 
 		thermal_get_trend(tz, trip, (enum thermal_trend *)&trend);
 
-		trend = (tz->tc1 * (temp - tz->last_temperature)) +
-			(tz->tc2 * (temp - trip_temp));
-
 		/* Heating up? */
 		if (trend > 0) {
 			list_for_each_entry(instance, &tz->cooling_devices,
@@ -1143,8 +1137,6 @@ EXPORT_SYMBOL(thermal_zone_device_update
  * @trips:	the number of trip points the thermal zone support
  * @devdata:	private device data
  * @ops:	standard thermal zone device callbacks
- * @tc1:	thermal coefficient 1 for passive calculations
- * @tc2:	thermal coefficient 2 for passive calculations
  * @passive_delay: number of milliseconds to wait between polls when
  *		   performing passive cooling
  * @polling_delay: number of milliseconds to wait between polls when checking
@@ -1152,13 +1144,12 @@ EXPORT_SYMBOL(thermal_zone_device_update
  *		   driven systems)
  *
  * thermal_zone_device_unregister() must be called when the device is no
- * longer needed. The passive cooling formula uses tc1 and tc2 as described in
- * section 11.1.5.1 of the ACPI specification 3.0.
+ * longer needed. The passive cooling depends on the .get_trend() return value.
  */
 struct thermal_zone_device *thermal_zone_device_register(char *type,
 	int trips, void *devdata,
 	const struct thermal_zone_device_ops *ops,
-	int tc1, int tc2, int passive_delay, int polling_delay)
+	int passive_delay, int polling_delay)
 {
 	struct thermal_zone_device *tz;
 	struct thermal_cooling_device *pos;
@@ -1194,8 +1185,6 @@ struct thermal_zone_device *thermal_zone
 	tz->device.class = &thermal_class;
 	tz->devdata = devdata;
 	tz->trips = trips;
-	tz->tc1 = tc1;
-	tz->tc2 = tc2;
 	tz->passive_delay = passive_delay;
 	tz->polling_delay = polling_delay;
 
Index: rtd3/include/linux/thermal.h
===================================================================
--- rtd3.orig/include/linux/thermal.h
+++ rtd3/include/linux/thermal.h
@@ -99,8 +99,6 @@ struct thermal_zone_device {
 	struct device device;
 	void *devdata;
 	int trips;
-	int tc1;
-	int tc2;
 	int passive_delay;
 	int polling_delay;
 	int temperature;
@@ -147,7 +145,7 @@ enum {
 #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
 
 struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,
-		const struct thermal_zone_device_ops *, int tc1, int tc2,
+		const struct thermal_zone_device_ops *,
 		int passive_freq, int polling_freq);
 void thermal_zone_device_unregister(struct thermal_zone_device *);
 
Index: rtd3/drivers/acpi/thermal.c
===================================================================
--- rtd3.orig/drivers/acpi/thermal.c
+++ rtd3/drivers/acpi/thermal.c
@@ -893,16 +893,13 @@ static int acpi_thermal_register_thermal
 		tz->thermal_zone =
 			thermal_zone_device_register("acpitz", trips, tz,
 						     &acpi_thermal_zone_ops,
-						     tz->trips.passive.tc1,
-						     tz->trips.passive.tc2,
 						     tz->trips.passive.tsp*100,
 						     tz->polling_frequency*100);
 	else
 		tz->thermal_zone =
 			thermal_zone_device_register("acpitz", trips, tz,
 						     &acpi_thermal_zone_ops,
-						     0, 0, 0,
-						     tz->polling_frequency*100);
+						     0, tz->polling_frequency*100);
 	if (IS_ERR(tz->thermal_zone))
 		return -ENODEV;
 
Index: rtd3/drivers/platform/x86/acerhdf.c
===================================================================
--- rtd3.orig/drivers/platform/x86/acerhdf.c
+++ rtd3/drivers/platform/x86/acerhdf.c
@@ -661,7 +661,7 @@ static int acerhdf_register_thermal(void
 		return -EINVAL;
 
 	thz_dev = thermal_zone_device_register("acerhdf", 1, NULL,
-					      &acerhdf_dev_ops, 0, 0, 0,
+					      &acerhdf_dev_ops, 0,
 					      (kernelmode) ? interval*1000 : 0);
 	if (IS_ERR(thz_dev))
 		return -EINVAL;
Index: rtd3/drivers/platform/x86/intel_mid_thermal.c
===================================================================
--- rtd3.orig/drivers/platform/x86/intel_mid_thermal.c
+++ rtd3/drivers/platform/x86/intel_mid_thermal.c
@@ -499,7 +499,7 @@ static int mid_thermal_probe(struct plat
 			goto err;
 		}
 		pinfo->tzd[i] = thermal_zone_device_register(name[i],
-				0, td_info, &tzd_ops, 0, 0, 0, 0);
+				0, td_info, &tzd_ops, 0, 0);
 		if (IS_ERR(pinfo->tzd[i])) {
 			kfree(td_info);
 			ret = PTR_ERR(pinfo->tzd[i]);
Index: rtd3/drivers/thermal/spear_thermal.c
===================================================================
--- rtd3.orig/drivers/thermal/spear_thermal.c
+++ rtd3/drivers/thermal/spear_thermal.c
@@ -148,7 +148,7 @@ static int spear_thermal_probe(struct pl
 	writel_relaxed(stdev->flags, stdev->thermal_base);
 
 	spear_thermal = thermal_zone_device_register("spear_thermal", 0,
-				stdev, &ops, 0, 0, 0, 0);
+				stdev, &ops, 0, 0);
 	if (IS_ERR(spear_thermal)) {
 		dev_err(&pdev->dev, "thermal zone device is NULL\n");
 		ret = PTR_ERR(spear_thermal);



^ permalink raw reply

* [RFC PATCH 6/12] thermal: introduce active cooling algorithm
From: Zhang Rui @ 2012-06-11  3:20 UTC (permalink / raw)
  To: linux-pm, linux-acpi@vger.kernel.org


Introduce thermal_zone_trip_update() to update the cooling state
of all the cooling devices that are binded to an active trip point.

This will be used for passive cooling as well, in the future patches.
as both active and passive cooling can share the same algorithm,
which is

1. if the temperature is higher than a trip point,
   a. if the trend is THERMAL_TREND_RAISING, use higher cooling
      state for this trip point
   b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
      state for this trip point

2. if the temperature is lower than a trip point, use lower
   cooling state for this trip point.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/thermal.c        |    7 ++-
 drivers/thermal/thermal_sys.c |   91 ++++++++++++++++++++++++++++++------------
 2 files changed, 71 insertions(+), 27 deletions(-)

Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -1035,6 +1035,70 @@ void thermal_cooling_device_unregister(s
 }
 EXPORT_SYMBOL(thermal_cooling_device_unregister);
 
+/*
+ * Cooling algorithm for active trip points
+ *
+ * 1. if the temperature is higher than a trip point,
+ *    a. if the trend is THERMAL_TREND_RAISING, use higher cooling
+ *       state for this trip point
+ *    b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
+ *       state for this trip point
+ *
+ * 2. if the temperature is lower than a trip point, use lower
+ *    cooling state for this trip point
+ *
+ * Note that this behaves the same as the previous passive cooling
+ * algorithm.
+ */
+
+static void thermal_zone_trip_update(struct thermal_zone_device *tz,
+				     int trip, long temp)
+{
+	struct thermal_cooling_device_instance *instance;
+	struct thermal_cooling_device *cdev = NULL;
+	unsigned long cur_state, max_state;
+	long trip_temp;
+	enum thermal_trend trend;
+
+	tz->ops->get_trip_temp(tz, trip, &trip_temp);
+
+	if (temp >= trip_temp) {
+		thermal_get_trend(tz, trip, &trend);
+
+		list_for_each_entry(instance, &tz->cooling_devices, node) {
+			if (instance->trip != trip)
+				continue;
+
+			cdev = instance->cdev;
+
+			cdev->ops->get_cur_state(cdev, &cur_state);
+			cdev->ops->get_max_state(cdev, &max_state);
+
+			if (trend == THERMAL_TREND_RAISING) {
+				cur_state = cur_state < instance->upper ?
+					    (cur_state + 1) : instance->upper;
+			} else if (trend == THERMAL_TREND_DROPPING) {
+				cur_state = cur_state > instance->lower ?
+				    (cur_state - 1) : instance->lower;
+			}
+			cdev->ops->set_cur_state(cdev, cur_state);
+		}
+	} else {	/* below trip */
+		list_for_each_entry(instance, &tz->cooling_devices, node) {
+			if (instance->trip != trip)
+				continue;
+
+			cdev = instance->cdev;
+			cdev->ops->get_cur_state(cdev, &cur_state);
+
+			cur_state = cur_state > instance->lower ?
+				    (cur_state - 1) : instance->lower;
+			cdev->ops->set_cur_state(cdev, cur_state);
+		}
+	}
+
+	return;
+}
 /**
  * thermal_zone_device_update - force an update of a thermal zone's state
  * @ttz:	the thermal zone to update
@@ -1045,9 +1109,6 @@ void thermal_zone_device_update(struct t
 	int count, ret = 0;
 	long temp, trip_temp;
 	enum thermal_trip_type trip_type;
-	struct thermal_cooling_device_instance *instance;
-	struct thermal_cooling_device *cdev;
-	unsigned long cur_state, max_state;
 
 	mutex_lock(&tz->lock);
 
@@ -1083,29 +1144,7 @@ void thermal_zone_device_update(struct t
 					tz->ops->notify(tz, count, trip_type);
 			break;
 		case THERMAL_TRIP_ACTIVE:
-			list_for_each_entry(instance, &tz->cooling_devices,
-					    node) {
-				if (instance->trip != count)
-					continue;
-
-				cdev = instance->cdev;
-
-				cdev->ops->get_cur_state(cdev, &cur_state);
-				cdev->ops->get_max_state(cdev, &max_state);
-
-				if (temp >= trip_temp)
-					cur_state =
-						cur_state < instance->upper ?
-						(cur_state + 1) :
-						instance->upper;
-				else
-					cur_state =
-						cur_state > instance->lower ?
-						(cur_state - 1) :
-						instance->lower;
-
-				cdev->ops->set_cur_state(cdev, cur_state);
-			}
+			thermal_zone_trip_update(tz, count, temp);
 			break;
 		case THERMAL_TRIP_PASSIVE:
 			if (temp >= trip_temp || tz->passive)
Index: rtd3/drivers/acpi/thermal.c
===================================================================
--- rtd3.orig/drivers/acpi/thermal.c
+++ rtd3/drivers/acpi/thermal.c
@@ -717,7 +717,12 @@ static int thermal_get_trend(struct ther
 	if (thermal_get_trip_type(thermal, trip, &type))
 		return -EINVAL;
 
-	/* Only PASSIVE trip points need TREND */
+	if (type == THERMAL_TRIP_ACTIVE) {
+		/* aggressive active cooling */
+		*trend = THERMAL_TREND_RAISING;
+		return 0;
+	}
+
 	if (type != THERMAL_TRIP_PASSIVE)
 		return -EINVAL;
 

^ permalink raw reply

* [RFC PATCH 8/12] thermal: rename thermal_zone_device.cooling_devices to thermal_zone_device.instances
From: Zhang Rui @ 2012-06-11  3:20 UTC (permalink / raw)
  To: linux-pm, linux-acpi@vger.kernel.org
  Cc: Amit Kachhap, eduardo, R, Durgadoss, Len, Brown,
	Rafael J. Wysocki, Matthew Garrett


Rename
thermal_zone_device.cooling_devices to thermal_zone_device.instances.

thermal_zone_device.cooling_devices is not accurate
as this is a list for thermal instances, rather than cooling devices.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal_sys.c |   18 +++++++++---------
 include/linux/thermal.h       |    4 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -704,7 +704,7 @@ static void thermal_zone_device_passive(
 
 		/* Heating up? */
 		if (trend > 0) {
-			list_for_each_entry(instance, &tz->cooling_devices,
+			list_for_each_entry(instance, &tz->instances,
 					    node) {
 				if (instance->trip != trip)
 					continue;
@@ -715,7 +715,7 @@ static void thermal_zone_device_passive(
 					cdev->ops->set_cur_state(cdev, state);
 			}
 		} else if (trend < 0) { /* Cooling off? */
-			list_for_each_entry(instance, &tz->cooling_devices,
+			list_for_each_entry(instance, &tz->instances,
 					    node) {
 				if (instance->trip != trip)
 					continue;
@@ -736,7 +736,7 @@ static void thermal_zone_device_passive(
 	 * and avoid thrashing around the passive trip point.  Note that we
 	 * assume symmetry.
 	 */
-	list_for_each_entry(instance, &tz->cooling_devices, node) {
+	list_for_each_entry(instance, &tz->instances, node) {
 		if (instance->trip != trip)
 			continue;
 		cdev = instance->cdev;
@@ -833,13 +833,13 @@ int thermal_zone_bind_cooling_device(str
 		goto remove_symbol_link;
 
 	mutex_lock(&tz->lock);
-	list_for_each_entry(pos, &tz->cooling_devices, node)
+	list_for_each_entry(pos, &tz->instances, node)
 	    if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
 		result = -EEXIST;
 		break;
 	}
 	if (!result)
-		list_add_tail(&dev->node, &tz->cooling_devices);
+		list_add_tail(&dev->node, &tz->instances);
 	mutex_unlock(&tz->lock);
 
 	if (!result)
@@ -872,7 +872,7 @@ int thermal_zone_unbind_cooling_device(s
 	struct thermal_instance *pos, *next;
 
 	mutex_lock(&tz->lock);
-	list_for_each_entry_safe(pos, next, &tz->cooling_devices, node) {
+	list_for_each_entry_safe(pos, next, &tz->instances, node) {
 		if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
 			list_del(&pos->node);
 			mutex_unlock(&tz->lock);
@@ -1066,7 +1066,7 @@ static void thermal_zone_trip_update(str
 	if (temp >= trip_temp) {
 		thermal_get_trend(tz, trip, &trend);
 
-		list_for_each_entry(instance, &tz->cooling_devices, node) {
+		list_for_each_entry(instance, &tz->instances, node) {
 			if (instance->trip != trip)
 				continue;
 
@@ -1085,7 +1085,7 @@ static void thermal_zone_trip_update(str
 			cdev->ops->set_cur_state(cdev, cur_state);
 		}
 	} else {	/* below trip */
-		list_for_each_entry(instance, &tz->cooling_devices, node) {
+		list_for_each_entry(instance, &tz->instances, node) {
 			if (instance->trip != trip)
 				continue;
 
@@ -1210,7 +1210,7 @@ struct thermal_zone_device *thermal_zone
 	if (!tz)
 		return ERR_PTR(-ENOMEM);
 
-	INIT_LIST_HEAD(&tz->cooling_devices);
+	INIT_LIST_HEAD(&tz->instances);
 	idr_init(&tz->idr);
 	mutex_init(&tz->lock);
 	result = get_idr(&thermal_tz_idr, &thermal_idr_lock, &tz->id);
Index: rtd3/include/linux/thermal.h
===================================================================
--- rtd3.orig/include/linux/thermal.h
+++ rtd3/include/linux/thermal.h
@@ -106,9 +106,9 @@ struct thermal_zone_device {
 	bool passive;
 	unsigned int forced_passive;
 	const struct thermal_zone_device_ops *ops;
-	struct list_head cooling_devices;
+	struct list_head instances;
 	struct idr idr;
-	struct mutex lock;	/* protect cooling devices list */
+	struct mutex lock;	/* protect instances list */
 	struct list_head node;
 	struct delayed_work poll_queue;
 };



^ permalink raw reply

* [RFC PATCH 9/12] thermal: rename thermal_instance.node to thermal_instance.tz_node
From: Zhang Rui @ 2012-06-11  3:20 UTC (permalink / raw)
  To: linux-pm, linux-acpi@vger.kernel.org
  Cc: Amit Kachhap, R, Durgadoss, eduardo, Rafael J. Wysocki,
	Matthew Garrett, Len, Brown


Rename thermal_instance.node to thermal_instance.tz_node.

thermal_instance should be referenced by both thermal zone devices
and thermal cooling devices.

Rename thermal_instance.node to thermal_instance.tz_node in this patch
and thermal_instanace.cdev_node will be introduced in next patch.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal_sys.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -56,7 +56,7 @@ struct thermal_instance {
 	unsigned long lower;	/* Lowest cooling state for this trip point */
 	char attr_name[THERMAL_NAME_LENGTH];
 	struct device_attribute attr;
-	struct list_head node;
+	struct list_head tz_node; /* node in tz->instances */
 };
 
 static DEFINE_IDR(thermal_tz_idr);
@@ -705,7 +705,7 @@ static void thermal_zone_device_passive(
 		/* Heating up? */
 		if (trend > 0) {
 			list_for_each_entry(instance, &tz->instances,
-					    node) {
+					    tz_node) {
 				if (instance->trip != trip)
 					continue;
 				cdev = instance->cdev;
@@ -716,7 +716,7 @@ static void thermal_zone_device_passive(
 			}
 		} else if (trend < 0) { /* Cooling off? */
 			list_for_each_entry(instance, &tz->instances,
-					    node) {
+					    tz_node) {
 				if (instance->trip != trip)
 					continue;
 				cdev = instance->cdev;
@@ -736,7 +736,7 @@ static void thermal_zone_device_passive(
 	 * and avoid thrashing around the passive trip point.  Note that we
 	 * assume symmetry.
 	 */
-	list_for_each_entry(instance, &tz->instances, node) {
+	list_for_each_entry(instance, &tz->instances, tz_node) {
 		if (instance->trip != trip)
 			continue;
 		cdev = instance->cdev;
@@ -832,13 +832,13 @@ int thermal_zone_bind_cooling_device(str
 		goto remove_symbol_link;
 
 	mutex_lock(&tz->lock);
-	list_for_each_entry(pos, &tz->instances, node)
+	list_for_each_entry(pos, &tz->instances, tz_node)
 	    if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
 		result = -EEXIST;
 		break;
 	}
 	if (!result)
-		list_add_tail(&dev->node, &tz->instances);
+		list_add_tail(&dev->tz_node, &tz->instances);
 	mutex_unlock(&tz->lock);
 
 	if (!result)
@@ -871,9 +871,9 @@ int thermal_zone_unbind_cooling_device(s
 	struct thermal_instance *pos, *next;
 
 	mutex_lock(&tz->lock);
-	list_for_each_entry_safe(pos, next, &tz->instances, node) {
+	list_for_each_entry_safe(pos, next, &tz->instances, tz_node) {
 		if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
-			list_del(&pos->node);
+			list_del(&pos->tz_node);
 			mutex_unlock(&tz->lock);
 			goto unbind;
 		}
@@ -1065,7 +1065,7 @@ static void thermal_zone_trip_update(str
 	if (temp >= trip_temp) {
 		thermal_get_trend(tz, trip, &trend);
 
-		list_for_each_entry(instance, &tz->instances, node) {
+		list_for_each_entry(instance, &tz->instances, tz_node) {
 			if (instance->trip != trip)
 				continue;
 
@@ -1084,7 +1084,7 @@ static void thermal_zone_trip_update(str
 			cdev->ops->set_cur_state(cdev, cur_state);
 		}
 	} else {	/* below trip */
-		list_for_each_entry(instance, &tz->instances, node) {
+		list_for_each_entry(instance, &tz->instances, tz_node) {
 			if (instance->trip != trip)
 				continue;
 



^ permalink raw reply

* [RFC PATCH 10/12] thermal: reference thermal instances in thermal_cooling_device
From: Zhang Rui @ 2012-06-11  3:20 UTC (permalink / raw)
  To: linux-pm, linux-acpi@vger.kernel.org
  Cc: Amit Kachhap, eduardo, R, Durgadoss, Rafael J. Wysocki,
	Len, Brown, Matthew Garrett


reference thermal instances in thermal_cooling_device.

so that cooling device can know the cooling state requirement
of all the thermal instances.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal_sys.c |    7 ++++++-
 include/linux/thermal.h       |    1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

Index: rtd3/include/linux/thermal.h
===================================================================
--- rtd3.orig/include/linux/thermal.h
+++ rtd3/include/linux/thermal.h
@@ -86,6 +86,7 @@ struct thermal_cooling_device {
 	struct device device;
 	void *devdata;
 	const struct thermal_cooling_device_ops *ops;
+	struct list_head instances;
 	struct list_head node;
 };
 
Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -57,6 +57,7 @@ struct thermal_instance {
 	char attr_name[THERMAL_NAME_LENGTH];
 	struct device_attribute attr;
 	struct list_head tz_node; /* node in tz->instances */
+	struct list_head cdev_node; /* node in cdev->instances */
 };
 
 static DEFINE_IDR(thermal_tz_idr);
@@ -837,8 +838,10 @@ int thermal_zone_bind_cooling_device(str
 		result = -EEXIST;
 		break;
 	}
-	if (!result)
+	if (!result) {
 		list_add_tail(&dev->tz_node, &tz->instances);
+		list_add_tail(&dev->cdev_node, &cdev->instances);
+	}
 	mutex_unlock(&tz->lock);
 
 	if (!result)
@@ -874,6 +877,7 @@ int thermal_zone_unbind_cooling_device(s
 	list_for_each_entry_safe(pos, next, &tz->instances, tz_node) {
 		if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
 			list_del(&pos->tz_node);
+			list_del(&pos->cdev_node);
 			mutex_unlock(&tz->lock);
 			goto unbind;
 		}
@@ -943,6 +947,7 @@ thermal_cooling_device_register(char *ty
 	}
 
 	strcpy(cdev->type, type);
+	INIT_LIST_HEAD(&cdev->instances);
 	cdev->ops = ops;
 	cdev->device.class = &thermal_class;
 	cdev->devdata = devdata;



^ permalink raw reply

* [RFC PATCH 11/12]  thermal: introduce cooling state arbitrator
From: Zhang Rui @ 2012-06-11  3:20 UTC (permalink / raw)
  To: linux-pm, linux-acpi@vger.kernel.org
  Cc: Amit Kachhap, eduardo, Rafael J. Wysocki, Len, Brown,
	R, Durgadoss, Matthew Garrett


Introduce simple arbitrator for setting device cooling state,
to fix the problem that a cooling device may be referenced by
by multiple trip points in multiple thermal zones.


With this patch, we have two stages for updating a thermal zone,
1. check if a thermal_instance needs to be updated or not
2. update the cooling device, based on the target cooling state
   of all its instances.

Note that, currently, the cooling device is set to the deepest
cooling state required.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal_sys.c |   41 ++++++++++++++++++++++++++++++++++++++---
 include/linux/thermal.h       |    1 +
 2 files changed, 39 insertions(+), 3 deletions(-)

Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -54,6 +54,7 @@ struct thermal_instance {
 	int trip;
 	unsigned long upper;	/* Highest cooling state for this trip point */
 	unsigned long lower;	/* Lowest cooling state for this trip point */
+	unsigned long target;	/* expected cooling state */
 	char attr_name[THERMAL_NAME_LENGTH];
 	struct device_attribute attr;
 	struct list_head tz_node; /* node in tz->instances */
@@ -812,6 +813,7 @@ int thermal_zone_bind_cooling_device(str
 	dev->trip = trip;
 	dev->upper = upper;
 	dev->lower = lower;
+	dev->target = -1;
 
 	result = get_idr(&tz->idr, &tz->lock, &dev->id);
 	if (result)
@@ -949,6 +951,7 @@ thermal_cooling_device_register(char *ty
 	strcpy(cdev->type, type);
 	INIT_LIST_HEAD(&cdev->instances);
 	cdev->ops = ops;
+	cdev->updated = 1;
 	cdev->device.class = &thermal_class;
 	cdev->devdata = devdata;
 	dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
@@ -1040,6 +1043,32 @@ void thermal_cooling_device_unregister(s
 }
 EXPORT_SYMBOL(thermal_cooling_device_unregister);
 
+static void thermal_zone_do_update(struct thermal_zone_device *tz)
+{
+	struct thermal_instance *instance1, *instance2;
+	struct thermal_cooling_device *cdev;
+	int target;
+
+	list_for_each_entry(instance1, &tz->instances, tz_node) {
+		cdev = instance1->cdev;
+
+		/* cooling device has already been updated*/
+		if (cdev->updated)
+			continue;
+
+		target = 0;
+		/* Make sure cdev enters the deepest cooling state */
+		list_for_each_entry(instance2, &cdev->instances, cdev_node) {
+			if (instance2->target == -1)
+				continue;
+			if (instance2->target > target)
+				target = instance2->target;
+		}
+		cdev->ops->set_cur_state(cdev, target);
+		cdev->updated = 1;
+	}
+}
+
 /*
  * Cooling algorithm for active trip points
  *
@@ -1086,19 +1115,24 @@ static void thermal_zone_trip_update(str
 				cur_state = cur_state > instance->lower ?
 				    (cur_state - 1) : instance->lower;
 			}
-			cdev->ops->set_cur_state(cdev, cur_state);
+			instance->target = cur_state;
+			cdev->updated = 0; /* cooling device needs update */
 		}
 	} else {	/* below trip */
 		list_for_each_entry(instance, &tz->instances, tz_node) {
 			if (instance->trip != trip)
 				continue;
 
+			/* Do not use the deacitve thermal instance */
+			if (instance->target == -1)
+				continue;
 			cdev = instance->cdev;
 			cdev->ops->get_cur_state(cdev, &cur_state);
 
 			cur_state = cur_state > instance->lower ?
-				    (cur_state - 1) : instance->lower;
-			cdev->ops->set_cur_state(cdev, cur_state);
+				    (cur_state - 1) : -1;
+			instance->target = cur_state;
+			cdev->updated = 0; /* cooling device needs update */
 		}
 	}
 
@@ -1159,6 +1193,7 @@ void thermal_zone_device_update(struct t
 		}
 	}
 
+	thermal_zone_do_update(tz);
 	if (tz->forced_passive)
 		thermal_zone_device_passive(tz, temp, tz->forced_passive,
 					    THERMAL_TRIPS_NONE);
Index: rtd3/include/linux/thermal.h
===================================================================
--- rtd3.orig/include/linux/thermal.h
+++ rtd3/include/linux/thermal.h
@@ -86,6 +86,7 @@ struct thermal_cooling_device {
 	struct device device;
 	void *devdata;
 	const struct thermal_cooling_device_ops *ops;
+	int updated; /* 1 if the cooling device does not need update */
 	struct list_head instances;
 	struct list_head node;
 };



^ permalink raw reply

* [RFC PATCH 12/12] thermal: Unify the code for both active and passive cooling
From: Zhang Rui @ 2012-06-11  3:20 UTC (permalink / raw)
  To: linux-pm, linux-acpi@vger.kernel.org
  Cc: Amit Kachhap, eduardo, Matthew Garrett, Len, Brown,
	Rafael J. Wysocki, R, Durgadoss


Unify the code for both active and passive cooling.

Remove thermal_zone_device_passive(). And use
thermal_zone_trip_update() and thermal_zone_do_update()
for both active and passive cooling.

Note that with this patch applied, we are able to support
multiple passive trip points for one thermal zone.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal_sys.c |   97 +++++++++---------------------------------
 1 file changed, 23 insertions(+), 74 deletions(-)

Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -684,73 +684,6 @@ static void thermal_zone_device_set_poll
 				      msecs_to_jiffies(delay));
 }
 
-static void thermal_zone_device_passive(struct thermal_zone_device *tz,
-					int temp, int trip_temp, int trip)
-{
-	int trend = 0;
-	struct thermal_instance *instance;
-	struct thermal_cooling_device *cdev;
-	long state, max_state;
-
-	/*
-	 * Above Trip?
-	 * -----------
-	 * Calculate the thermal trend (using the passive cooling equation)
-	 * and modify the performance limit for all passive cooling devices
-	 * accordingly.  Note that we assume symmetry.
-	 */
-	if (temp >= trip_temp) {
-		tz->passive = true;
-
-		thermal_get_trend(tz, trip, (enum thermal_trend *)&trend);
-
-		/* Heating up? */
-		if (trend > 0) {
-			list_for_each_entry(instance, &tz->instances,
-					    tz_node) {
-				if (instance->trip != trip)
-					continue;
-				cdev = instance->cdev;
-				cdev->ops->get_cur_state(cdev, &state);
-				cdev->ops->get_max_state(cdev, &max_state);
-				if (state++ < max_state)
-					cdev->ops->set_cur_state(cdev, state);
-			}
-		} else if (trend < 0) { /* Cooling off? */
-			list_for_each_entry(instance, &tz->instances,
-					    tz_node) {
-				if (instance->trip != trip)
-					continue;
-				cdev = instance->cdev;
-				cdev->ops->get_cur_state(cdev, &state);
-				cdev->ops->get_max_state(cdev, &max_state);
-				if (state > 0)
-					cdev->ops->set_cur_state(cdev, --state);
-			}
-		}
-		return;
-	}
-
-	/*
-	 * Below Trip?
-	 * -----------
-	 * Implement passive cooling hysteresis to slowly increase performance
-	 * and avoid thrashing around the passive trip point.  Note that we
-	 * assume symmetry.
-	 */
-	list_for_each_entry(instance, &tz->instances, tz_node) {
-		if (instance->trip != trip)
-			continue;
-		cdev = instance->cdev;
-		cdev->ops->get_cur_state(cdev, &state);
-		cdev->ops->get_max_state(cdev, &max_state);
-		if (state > 0)
-			cdev->ops->set_cur_state(cdev, --state);
-		if (state == 0)
-			tz->passive = false;
-	}
-}
-
 static void thermal_zone_device_check(struct work_struct *work)
 {
 	struct thermal_zone_device *tz = container_of(work, struct
@@ -1070,7 +1003,7 @@ static void thermal_zone_do_update(struc
 }
 
 /*
- * Cooling algorithm for active trip points
+ * Cooling algorithm for both active and passive cooling
  *
  * 1. if the temperature is higher than a trip point,
  *    a. if the trend is THERMAL_TREND_RAISING, use higher cooling
@@ -1092,9 +1025,16 @@ static void thermal_zone_trip_update(str
 	struct thermal_cooling_device *cdev = NULL;
 	unsigned long cur_state, max_state;
 	long trip_temp;
+	enum thermal_trip_type trip_type;
 	enum thermal_trend trend;
 
-	tz->ops->get_trip_temp(tz, trip, &trip_temp);
+	if (trip == THERMAL_TRIPS_NONE) {
+		trip_temp = tz->forced_passive;
+		trip_type = THERMAL_TRIPS_NONE;
+	} else {
+		tz->ops->get_trip_temp(tz, trip, &trip_temp);
+		tz->ops->get_trip_type(tz, trip, &trip_type);
+	}
 
 	if (temp >= trip_temp) {
 		thermal_get_trend(tz, trip, &trend);
@@ -1115,6 +1055,12 @@ static void thermal_zone_trip_update(str
 				cur_state = cur_state > instance->lower ?
 				    (cur_state - 1) : instance->lower;
 			}
+
+			/* activate a passive thermal instance */
+			if (trip_type == THERMAL_TRIP_PASSIVE &&
+			    instance->target == -1)
+				tz->passive++;
+
 			instance->target = cur_state;
 			cdev->updated = 0; /* cooling device needs update */
 		}
@@ -1131,6 +1077,11 @@ static void thermal_zone_trip_update(str
 
 			cur_state = cur_state > instance->lower ?
 				    (cur_state - 1) : -1;
+
+			/* deactivate a passive thermal instance */
+			if (trip_type == THERMAL_TRIP_PASSIVE &&
+			    cur_state == -1)
+				tz->passive--;
 			instance->target = cur_state;
 			cdev->updated = 0; /* cooling device needs update */
 		}
@@ -1187,16 +1138,14 @@ void thermal_zone_device_update(struct t
 			break;
 		case THERMAL_TRIP_PASSIVE:
 			if (temp >= trip_temp || tz->passive)
-				thermal_zone_device_passive(tz, temp,
-							    trip_temp, count);
+				thermal_zone_trip_update(tz, count, temp);
 			break;
 		}
 	}
 
-	thermal_zone_do_update(tz);
 	if (tz->forced_passive)
-		thermal_zone_device_passive(tz, temp, tz->forced_passive,
-					    THERMAL_TRIPS_NONE);
+		thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE, temp);
+	thermal_zone_do_update(tz);
 
 leave:
 	if (tz->passive)



^ permalink raw reply

* Re: [RFC PATCH 3/12] thermal: set upper and lower limits when binding
From: Amit Kachhap @ 2012-06-11  4:21 UTC (permalink / raw)
  To: Zhang Rui
  Cc: linux-pm, linux-acpi@vger.kernel.org, R, Durgadoss, eduardo,
	Len, Brown, Rafael J. Wysocki, Matthew Garrett
In-Reply-To: <1339384801.1492.156.camel@rui.sh.intel.com>

Hi Rui,

I like this approach of multiple cooling states for ACTIVE type cooling devices.
Some comments added.

On 11 June 2012 08:50, Zhang Rui <rui.zhang@intel.com> wrote:
>
> Set upper and lower limits when binding
> a thermal cooling device to a thermal zone device.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/thermal.c         |   34 ++++++++++++++++++++++++----------
>  drivers/platform/x86/acerhdf.c |    2 +-
>  drivers/thermal/thermal_sys.c  |   20 ++++++++++++++------
>  include/linux/thermal.h        |    3 ++-
>  4 files changed, 41 insertions(+), 18 deletions(-)
>
> Index: rtd3/drivers/thermal/thermal_sys.c
> ===================================================================
> --- rtd3.orig/drivers/thermal/thermal_sys.c
> +++ rtd3/drivers/thermal/thermal_sys.c
> @@ -248,7 +248,7 @@ passive_store(struct device *dev, struct
>                                     sizeof("Processor")))
>                                thermal_zone_bind_cooling_device(tz,
>                                                                 THERMAL_TRIPS_NONE,
> -                                                                cdev);
> +                                                                cdev, -1, -1);
>                }
>                mutex_unlock(&thermal_list_lock);
>                if (!tz->passive_delay)
> @@ -760,7 +760,8 @@ static void thermal_zone_device_check(st
>  */
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>                                     int trip,
> -                                    struct thermal_cooling_device *cdev)
> +                                    struct thermal_cooling_device *cdev,
> +                                    long upper, long lower)
>  {
>        struct thermal_cooling_device_instance *dev;
>        struct thermal_cooling_device_instance *pos;
> @@ -784,6 +785,15 @@ int thermal_zone_bind_cooling_device(str
>        if (tz != pos1 || cdev != pos2)
>                return -EINVAL;
>
> +       cdev->ops->get_max_state(cdev, &max_state);
> +
> +       /* lower default 0, upper default max_state */
> +       lower = lower < 0 ? 0 : lower;
> +       upper = upper < 0 ? max_state : upper;
Also some check can be added here like this,
           cdev->ops->get_max_state(cdev, &cur_state);


> +
> +       if (lower > upper || upper > max_state)
> +               return -EINVAL;
> +
>        dev =
>            kzalloc(sizeof(struct thermal_cooling_device_instance), GFP_KERNEL);
>        if (!dev)
> @@ -791,10 +801,8 @@ int thermal_zone_bind_cooling_device(str
>        dev->tz = tz;
>        dev->cdev = cdev;
>        dev->trip = trip;
> -
> -       cdev->ops->get_max_state(dev, &max_state);
> -       dev->upper = max_state;
> -       dev->lower = 0;
> +       dev->upper = upper;
> +       dev->lower = lower;
>
>        result = get_idr(&tz->idr, &tz->lock, &dev->id);
>        if (result)
> Index: rtd3/include/linux/thermal.h
> ===================================================================
> --- rtd3.orig/include/linux/thermal.h
> +++ rtd3/include/linux/thermal.h
> @@ -143,7 +143,8 @@ struct thermal_zone_device *thermal_zone
>  void thermal_zone_device_unregister(struct thermal_zone_device *);
>
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
> -                                    struct thermal_cooling_device *);
> +                                    struct thermal_cooling_device *,
> +                                    long, long);
>  int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int,
>                                       struct thermal_cooling_device *);
>  void thermal_zone_device_update(struct thermal_zone_device *);
> Index: rtd3/drivers/acpi/thermal.c
> ===================================================================
> --- rtd3.orig/drivers/acpi/thermal.c
> +++ rtd3/drivers/acpi/thermal.c
> @@ -729,11 +729,9 @@ static int thermal_notify(struct thermal
>        return 0;
>  }
>
> -typedef int (*cb)(struct thermal_zone_device *, int,
> -                 struct thermal_cooling_device *);
>  static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
>                                        struct thermal_cooling_device *cdev,
> -                                       cb action)
> +                                       int bind)
>  {
>        struct acpi_device *device = cdev->devdata;
>        struct acpi_thermal *tz = thermal->devdata;
> @@ -758,7 +756,14 @@ static int acpi_thermal_cooling_device_c
>                        handle = tz->trips.passive.devices.handles[i];
>                        status = acpi_bus_get_device(handle, &dev);
>                        if (ACPI_SUCCESS(status) && (dev == device)) {
> -                               result = action(thermal, trip, cdev);
> +                               if (bind)
> +                                       result =
> +                                               thermal_zone_bind_cooling_device(
> +                                                       thermal, trip, cdev, -1, -1);
> +                               else
> +                                       result =
> +                                               thermal_zone_unbind_cooling_device(
> +                                                       thermal, trip, cdev);
>                                if (result)
>                                        goto failed;
>                        }
> @@ -775,7 +780,13 @@ static int acpi_thermal_cooling_device_c
>                        handle = tz->trips.active[i].devices.handles[j];
>                        status = acpi_bus_get_device(handle, &dev);
>                        if (ACPI_SUCCESS(status) && (dev == device)) {
> -                               result = action(thermal, trip, cdev);
> +                               if (bind)
> +                                       result =
> +                                               thermal_zone_bind_cooling_device(
> +                                                       thermal, trip, cdev, -1, -1);
> +                               else
> +                                       result = thermal_zone_unbind_cooling_device(
> +                                                       thermal, trip, cdev);
>                                if (result)
>                                        goto failed;
>                        }
> @@ -786,7 +797,12 @@ static int acpi_thermal_cooling_device_c
>                handle = tz->devices.handles[i];
>                status = acpi_bus_get_device(handle, &dev);
>                if (ACPI_SUCCESS(status) && (dev == device)) {
> -                       result = action(thermal, -1, cdev);
> +                       if (bind)
> +                               result = thermal_zone_bind_cooling_device(thermal,
> +                                                               -1, cdev, -1, -1);
> +                       else
> +                               result = thermal_zone_unbind_cooling_device(thermal,
> +                                                               -1, cdev);
>                        if (result)
>                                goto failed;
>                }
> @@ -800,16 +816,14 @@ static int
>  acpi_thermal_bind_cooling_device(struct thermal_zone_device *thermal,
>                                        struct thermal_cooling_device *cdev)
>  {
> -       return acpi_thermal_cooling_device_cb(thermal, cdev,
> -                               thermal_zone_bind_cooling_device);
> +       return acpi_thermal_cooling_device_cb(thermal, cdev, 1);
>  }
>
>  static int
>  acpi_thermal_unbind_cooling_device(struct thermal_zone_device *thermal,
>                                        struct thermal_cooling_device *cdev)
>  {
> -       return acpi_thermal_cooling_device_cb(thermal, cdev,
> -                               thermal_zone_unbind_cooling_device);
> +       return acpi_thermal_cooling_device_cb(thermal, cdev, 0);
>  }
>
>  static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> Index: rtd3/drivers/platform/x86/acerhdf.c
> ===================================================================
> --- rtd3.orig/drivers/platform/x86/acerhdf.c
> +++ rtd3/drivers/platform/x86/acerhdf.c
> @@ -329,7 +329,7 @@ static int acerhdf_bind(struct thermal_z
>        if (cdev != cl_dev)
>                return 0;
>
> -       if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
> +       if (thermal_zone_bind_cooling_device(thermal, 0, cdev, -1, -1)) {
>                pr_err("error binding cooling dev\n");
>                return -EINVAL;
>        }
>
>

^ permalink raw reply

* Re: [RFC PATCH 3/12] thermal: set upper and lower limits when binding
From: Amit Kachhap @ 2012-06-11  4:27 UTC (permalink / raw)
  To: Zhang Rui
  Cc: linux-pm, linux-acpi@vger.kernel.org, R, Durgadoss, eduardo,
	Len, Brown, Rafael J. Wysocki, Matthew Garrett
In-Reply-To: <1339384801.1492.156.camel@rui.sh.intel.com>

Hi Rui,

Sorry for sending one incomplete mail before.
I like this approach of multiple cooling states for ACTIVE type cooling devices.
Some comments added.

On 11 June 2012 08:50, Zhang Rui <rui.zhang@intel.com> wrote:
>
> Set upper and lower limits when binding
> a thermal cooling device to a thermal zone device.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/thermal.c         |   34 ++++++++++++++++++++++++----------
>  drivers/platform/x86/acerhdf.c |    2 +-
>  drivers/thermal/thermal_sys.c  |   20 ++++++++++++++------
>  include/linux/thermal.h        |    3 ++-
>  4 files changed, 41 insertions(+), 18 deletions(-)
>
> Index: rtd3/drivers/thermal/thermal_sys.c
> ===================================================================
> --- rtd3.orig/drivers/thermal/thermal_sys.c
> +++ rtd3/drivers/thermal/thermal_sys.c
> @@ -248,7 +248,7 @@ passive_store(struct device *dev, struct
>                                     sizeof("Processor")))
>                                thermal_zone_bind_cooling_device(tz,
>                                                                 THERMAL_TRIPS_NONE,
> -                                                                cdev);
> +                                                                cdev, -1, -1);
>                }
>                mutex_unlock(&thermal_list_lock);
>                if (!tz->passive_delay)
> @@ -760,7 +760,8 @@ static void thermal_zone_device_check(st
>  */
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>                                     int trip,
> -                                    struct thermal_cooling_device *cdev)
> +                                    struct thermal_cooling_device *cdev,
> +                                    long upper, long lower)
>  {
>        struct thermal_cooling_device_instance *dev;
>        struct thermal_cooling_device_instance *pos;
> @@ -784,6 +785,15 @@ int thermal_zone_bind_cooling_device(str
>        if (tz != pos1 || cdev != pos2)
>                return -EINVAL;
>
> +       cdev->ops->get_max_state(cdev, &max_state);
> +
> +       /* lower default 0, upper default max_state */
> +       lower = lower < 0 ? 0 : lower;
> +       upper = upper < 0 ? max_state : upper;
> +
> +       if (lower > upper || upper > max_state)
> +               return -EINVAL;
> +
>        dev =
>            kzalloc(sizeof(struct thermal_cooling_device_instance), GFP_KERNEL);
>        if (!dev)
> @@ -791,10 +801,8 @@ int thermal_zone_bind_cooling_device(str
>        dev->tz = tz;
>        dev->cdev = cdev;
>        dev->trip = trip;
> -
> -       cdev->ops->get_max_state(dev, &max_state);
> -       dev->upper = max_state;
> -       dev->lower = 0;
> +       dev->upper = upper;
> +       dev->lower = lower;
Also some check can be added here like this,
          cdev->ops->get_max_state(cdev, &cur_state);
          if (cur_state !=  lower) {
              print_err("Set the initial state of the cooling device
to lower\n");
              return -EINVAL;
           }
           This is needed because in your thermal_zone_update
implementation the state change is from lower - upper.
>
>        result = get_idr(&tz->idr, &tz->lock, &dev->id);
>        if (result)
> Index: rtd3/include/linux/thermal.h
> ===================================================================
> --- rtd3.orig/include/linux/thermal.h
> +++ rtd3/include/linux/thermal.h
> @@ -143,7 +143,8 @@ struct thermal_zone_device *thermal_zone
>  void thermal_zone_device_unregister(struct thermal_zone_device *);
>
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
> -                                    struct thermal_cooling_device *);
> +                                    struct thermal_cooling_device *,
> +                                    long, long);
>  int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int,
>                                       struct thermal_cooling_device *);
>  void thermal_zone_device_update(struct thermal_zone_device *);
> Index: rtd3/drivers/acpi/thermal.c
> ===================================================================
> --- rtd3.orig/drivers/acpi/thermal.c
> +++ rtd3/drivers/acpi/thermal.c
> @@ -729,11 +729,9 @@ static int thermal_notify(struct thermal
>        return 0;
>  }
>
> -typedef int (*cb)(struct thermal_zone_device *, int,
> -                 struct thermal_cooling_device *);
>  static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
>                                        struct thermal_cooling_device *cdev,
> -                                       cb action)
> +                                       int bind)
>  {
>        struct acpi_device *device = cdev->devdata;
>        struct acpi_thermal *tz = thermal->devdata;
> @@ -758,7 +756,14 @@ static int acpi_thermal_cooling_device_c
>                        handle = tz->trips.passive.devices.handles[i];
>                        status = acpi_bus_get_device(handle, &dev);
>                        if (ACPI_SUCCESS(status) && (dev == device)) {
> -                               result = action(thermal, trip, cdev);
> +                               if (bind)
> +                                       result =
> +                                               thermal_zone_bind_cooling_device(
> +                                                       thermal, trip, cdev, -1, -1);
> +                               else
> +                                       result =
> +                                               thermal_zone_unbind_cooling_device(
> +                                                       thermal, trip, cdev);
>                                if (result)
>                                        goto failed;
>                        }
> @@ -775,7 +780,13 @@ static int acpi_thermal_cooling_device_c
>                        handle = tz->trips.active[i].devices.handles[j];
>                        status = acpi_bus_get_device(handle, &dev);
>                        if (ACPI_SUCCESS(status) && (dev == device)) {
> -                               result = action(thermal, trip, cdev);
> +                               if (bind)
> +                                       result =
> +                                               thermal_zone_bind_cooling_device(
> +                                                       thermal, trip, cdev, -1, -1);
> +                               else
> +                                       result = thermal_zone_unbind_cooling_device(
> +                                                       thermal, trip, cdev);
>                                if (result)
>                                        goto failed;
>                        }
> @@ -786,7 +797,12 @@ static int acpi_thermal_cooling_device_c
>                handle = tz->devices.handles[i];
>                status = acpi_bus_get_device(handle, &dev);
>                if (ACPI_SUCCESS(status) && (dev == device)) {
> -                       result = action(thermal, -1, cdev);
> +                       if (bind)
> +                               result = thermal_zone_bind_cooling_device(thermal,
> +                                                               -1, cdev, -1, -1);
> +                       else
> +                               result = thermal_zone_unbind_cooling_device(thermal,
> +                                                               -1, cdev);
>                        if (result)
>                                goto failed;
>                }
> @@ -800,16 +816,14 @@ static int
>  acpi_thermal_bind_cooling_device(struct thermal_zone_device *thermal,
>                                        struct thermal_cooling_device *cdev)
>  {
> -       return acpi_thermal_cooling_device_cb(thermal, cdev,
> -                               thermal_zone_bind_cooling_device);
> +       return acpi_thermal_cooling_device_cb(thermal, cdev, 1);
>  }
>
>  static int
>  acpi_thermal_unbind_cooling_device(struct thermal_zone_device *thermal,
>                                        struct thermal_cooling_device *cdev)
>  {
> -       return acpi_thermal_cooling_device_cb(thermal, cdev,
> -                               thermal_zone_unbind_cooling_device);
> +       return acpi_thermal_cooling_device_cb(thermal, cdev, 0);
>  }
>
>  static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> Index: rtd3/drivers/platform/x86/acerhdf.c
> ===================================================================
> --- rtd3.orig/drivers/platform/x86/acerhdf.c
> +++ rtd3/drivers/platform/x86/acerhdf.c
> @@ -329,7 +329,7 @@ static int acerhdf_bind(struct thermal_z
>        if (cdev != cl_dev)
>                return 0;
>
> -       if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
> +       if (thermal_zone_bind_cooling_device(thermal, 0, cdev, -1, -1)) {
>                pr_err("error binding cooling dev\n");
>                return -EINVAL;
>        }
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH 6/12] thermal: introduce active cooling algorithm
From: Amit Kachhap @ 2012-06-11  4:51 UTC (permalink / raw)
  To: Zhang Rui; +Cc: linux-acpi@vger.kernel.org, linux-pm
In-Reply-To: <1339384817.1492.160.camel@rui.sh.intel.com>

On 11 June 2012 08:50, Zhang Rui <rui.zhang@intel.com> wrote:
>
> Introduce thermal_zone_trip_update() to update the cooling state
> of all the cooling devices that are binded to an active trip point.
>
> This will be used for passive cooling as well, in the future patches.
> as both active and passive cooling can share the same algorithm,
> which is
>
> 1. if the temperature is higher than a trip point,
>   a. if the trend is THERMAL_TREND_RAISING, use higher cooling
>      state for this trip point
>   b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
>      state for this trip point
>
> 2. if the temperature is lower than a trip point, use lower
>   cooling state for this trip point.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/thermal.c        |    7 ++-
>  drivers/thermal/thermal_sys.c |   91 ++++++++++++++++++++++++++++++------------
>  2 files changed, 71 insertions(+), 27 deletions(-)
>
> Index: rtd3/drivers/thermal/thermal_sys.c
> ===================================================================
> --- rtd3.orig/drivers/thermal/thermal_sys.c
> +++ rtd3/drivers/thermal/thermal_sys.c
> @@ -1035,6 +1035,70 @@ void thermal_cooling_device_unregister(s
>  }
>  EXPORT_SYMBOL(thermal_cooling_device_unregister);
>
> +/*
> + * Cooling algorithm for active trip points
> + *
> + * 1. if the temperature is higher than a trip point,
> + *    a. if the trend is THERMAL_TREND_RAISING, use higher cooling
> + *       state for this trip point
> + *    b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
> + *       state for this trip point
> + *
> + * 2. if the temperature is lower than a trip point, use lower
> + *    cooling state for this trip point
> + *
> + * Note that this behaves the same as the previous passive cooling
> + * algorithm.
> + */
> +
> +static void thermal_zone_trip_update(struct thermal_zone_device *tz,
> +                                    int trip, long temp)
> +{
> +       struct thermal_cooling_device_instance *instance;
> +       struct thermal_cooling_device *cdev = NULL;
> +       unsigned long cur_state, max_state;
> +       long trip_temp;
> +       enum thermal_trend trend;
> +
> +       tz->ops->get_trip_temp(tz, trip, &trip_temp);
> +
> +       if (temp >= trip_temp) {
> +               thermal_get_trend(tz, trip, &trend);
> +
> +               list_for_each_entry(instance, &tz->cooling_devices, node) {
> +                       if (instance->trip != trip)
> +                               continue;
> +
> +                       cdev = instance->cdev;
> +
> +                       cdev->ops->get_cur_state(cdev, &cur_state);
> +                       cdev->ops->get_max_state(cdev, &max_state);
> +
> +                       if (trend == THERMAL_TREND_RAISING) {
> +                               cur_state = cur_state < instance->upper ?
> +                                           (cur_state + 1) : instance->upper;
> +                       } else if (trend == THERMAL_TREND_DROPPING) {
> +                               cur_state = cur_state > instance->lower ?
> +                                   (cur_state - 1) : instance->lower;
> +                       }
> +                       cdev->ops->set_cur_state(cdev, cur_state);
> +               }
> +       } else {        /* below trip */
> +               list_for_each_entry(instance, &tz->cooling_devices, node) {
> +                       if (instance->trip != trip)
> +                               continue;
Can we use the trend callback here even if the current temperature is
less than the trip temperature. Basically according to trend it should
deactivate the cooling devices as done for activating cooling devices.
> +
> +                       cdev = instance->cdev;
> +                       cdev->ops->get_cur_state(cdev, &cur_state);
> +
> +                       cur_state = cur_state > instance->lower ?
> +                                   (cur_state - 1) : instance->lower;
> +                       cdev->ops->set_cur_state(cdev, cur_state);
> +               }
> +       }
> +
> +       return;
> +}
>  /**
>  * thermal_zone_device_update - force an update of a thermal zone's state
>  * @ttz:       the thermal zone to update
> @@ -1045,9 +1109,6 @@ void thermal_zone_device_update(struct t
>        int count, ret = 0;
>        long temp, trip_temp;
>        enum thermal_trip_type trip_type;
> -       struct thermal_cooling_device_instance *instance;
> -       struct thermal_cooling_device *cdev;
> -       unsigned long cur_state, max_state;
>
>        mutex_lock(&tz->lock);
>
> @@ -1083,29 +1144,7 @@ void thermal_zone_device_update(struct t
>                                        tz->ops->notify(tz, count, trip_type);
>                        break;
>                case THERMAL_TRIP_ACTIVE:
> -                       list_for_each_entry(instance, &tz->cooling_devices,
> -                                           node) {
> -                               if (instance->trip != count)
> -                                       continue;
> -
> -                               cdev = instance->cdev;
> -
> -                               cdev->ops->get_cur_state(cdev, &cur_state);
> -                               cdev->ops->get_max_state(cdev, &max_state);
> -
> -                               if (temp >= trip_temp)
> -                                       cur_state =
> -                                               cur_state < instance->upper ?
> -                                               (cur_state + 1) :
> -                                               instance->upper;
> -                               else
> -                                       cur_state =
> -                                               cur_state > instance->lower ?
> -                                               (cur_state - 1) :
> -                                               instance->lower;
> -
> -                               cdev->ops->set_cur_state(cdev, cur_state);
> -                       }
> +                       thermal_zone_trip_update(tz, count, temp);
>                        break;
>                case THERMAL_TRIP_PASSIVE:
>                        if (temp >= trip_temp || tz->passive)
> Index: rtd3/drivers/acpi/thermal.c
> ===================================================================
> --- rtd3.orig/drivers/acpi/thermal.c
> +++ rtd3/drivers/acpi/thermal.c
> @@ -717,7 +717,12 @@ static int thermal_get_trend(struct ther
>        if (thermal_get_trip_type(thermal, trip, &type))
>                return -EINVAL;
>
> -       /* Only PASSIVE trip points need TREND */
> +       if (type == THERMAL_TRIP_ACTIVE) {
> +               /* aggressive active cooling */
> +               *trend = THERMAL_TREND_RAISING;
> +               return 0;
> +       }
> +
>        if (type != THERMAL_TRIP_PASSIVE)
>                return -EINVAL;
>
>
>

^ permalink raw reply

* Re: [RFC PATCH 7/12] thermal: rename structure thermal_cooling_device_instance to thermal_instance
From: Zhang Rui @ 2012-06-11  6:21 UTC (permalink / raw)
  To: linux-pm; +Cc: linux-acpi@vger.kernel.org
In-Reply-To: <1339384821.1492.161.camel@rui.sh.intel.com>

seems this patch is missed. Resend it.

On 一, 2012-06-11 at 11:20 +0800, Zhang Rui wrote:
> Rename structure thermal_cooling_device_instance to thermal_instance.
> 
> This struct is used to describe the behavior for a thermal
> cooling device on a certain trip point for a certain thremal zone.
> 
> thermal_cooling_device_instance is not accurate, as a cooling device
> can be used for more than one trip point in one thermal zone device.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/thermal/thermal_sys.c |   18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> Index: rtd3/drivers/thermal/thermal_sys.c
> ===================================================================
> --- rtd3.orig/drivers/thermal/thermal_sys.c
> +++ rtd3/drivers/thermal/thermal_sys.c
> @@ -46,7 +46,7 @@ MODULE_LICENSE("GPL");
>   * a certain cooling device on a certain trip point
>   * in a certain thermal zone
>   */
> -struct thermal_cooling_device_instance {
> +struct thermal_instance {
>  	int id;
>  	char name[THERMAL_NAME_LENGTH];
>  	struct thermal_zone_device *tz;
> @@ -388,10 +388,10 @@ static ssize_t
>  thermal_cooling_device_trip_point_show(struct device *dev,
>  				       struct device_attribute *attr, char *buf)
>  {
> -	struct thermal_cooling_device_instance *instance;
> +	struct thermal_instance *instance;
>  
>  	instance =
> -	    container_of(attr, struct thermal_cooling_device_instance, attr);
> +	    container_of(attr, struct thermal_instance, attr);
>  
>  	if (instance->trip == THERMAL_TRIPS_NONE)
>  		return sprintf(buf, "-1\n");
> @@ -686,7 +686,7 @@ static void thermal_zone_device_passive(
>  					int temp, int trip_temp, int trip)
>  {
>  	int trend = 0;
> -	struct thermal_cooling_device_instance *instance;
> +	struct thermal_instance *instance;
>  	struct thermal_cooling_device *cdev;
>  	long state, max_state;
>  
> @@ -771,8 +771,8 @@ int thermal_zone_bind_cooling_device(str
>  				     struct thermal_cooling_device *cdev,
>  				     long upper, long lower)
>  {
> -	struct thermal_cooling_device_instance *dev;
> -	struct thermal_cooling_device_instance *pos;
> +	struct thermal_instance *dev;
> +	struct thermal_instance *pos;
>  	struct thermal_zone_device *pos1;
>  	struct thermal_cooling_device *pos2;
>  	unsigned long max_state;
> @@ -803,7 +803,7 @@ int thermal_zone_bind_cooling_device(str
>  		return -EINVAL;
>  
>  	dev =
> -	    kzalloc(sizeof(struct thermal_cooling_device_instance), GFP_KERNEL);
> +	    kzalloc(sizeof(struct thermal_instance), GFP_KERNEL);
>  	if (!dev)
>  		return -ENOMEM;
>  	dev->tz = tz;
> @@ -868,7 +868,7 @@ int thermal_zone_unbind_cooling_device(s
>  				       int trip,
>  				       struct thermal_cooling_device *cdev)
>  {
> -	struct thermal_cooling_device_instance *pos, *next;
> +	struct thermal_instance *pos, *next;
>  
>  	mutex_lock(&tz->lock);
>  	list_for_each_entry_safe(pos, next, &tz->cooling_devices, node) {
> @@ -1054,7 +1054,7 @@ EXPORT_SYMBOL(thermal_cooling_device_unr
>  static void thermal_zone_trip_update(struct thermal_zone_device *tz,
>  				     int trip, long temp)
>  {
> -	struct thermal_cooling_device_instance *instance;
> +	struct thermal_instance *instance;
>  	struct thermal_cooling_device *cdev = NULL;
>  	unsigned long cur_state, max_state;
>  	long trip_temp;
> 


_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

^ permalink raw reply

* Re: [RFC PATCH 3/12] thermal: set upper and lower limits when binding
From: Zhang Rui @ 2012-06-11  6:30 UTC (permalink / raw)
  To: Amit Kachhap; +Cc: linux-acpi@vger.kernel.org, linux-pm
In-Reply-To: <CAK44p21ZtBR_ubDr4y9i4c81cBVRyBkr=T=CyWrwS=csQjtnfw@mail.gmail.com>

On 一, 2012-06-11 at 09:57 +0530, Amit Kachhap wrote:
> Hi Rui,
> 
> Sorry for sending one incomplete mail before.
> I like this approach of multiple cooling states for ACTIVE type cooling devices.

Great, thanks.

> Some comments added.
> On 11 June 2012 08:50, Zhang Rui <rui.zhang@intel.com> wrote:
> >
> > Set upper and lower limits when binding
> > a thermal cooling device to a thermal zone device.
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/thermal.c         |   34 ++++++++++++++++++++++++----------
> >  drivers/platform/x86/acerhdf.c |    2 +-
> >  drivers/thermal/thermal_sys.c  |   20 ++++++++++++++------
> >  include/linux/thermal.h        |    3 ++-
> >  4 files changed, 41 insertions(+), 18 deletions(-)
> >
> > Index: rtd3/drivers/thermal/thermal_sys.c
> > ===================================================================
> > --- rtd3.orig/drivers/thermal/thermal_sys.c
> > +++ rtd3/drivers/thermal/thermal_sys.c
> > @@ -248,7 +248,7 @@ passive_store(struct device *dev, struct
> >                                     sizeof("Processor")))
> >                                thermal_zone_bind_cooling_device(tz,
> >                                                                 THERMAL_TRIPS_NONE,
> > -                                                                cdev);
> > +                                                                cdev, -1, -1);
> >                }
> >                mutex_unlock(&thermal_list_lock);
> >                if (!tz->passive_delay)
> > @@ -760,7 +760,8 @@ static void thermal_zone_device_check(st
> >  */
> >  int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
> >                                     int trip,
> > -                                    struct thermal_cooling_device *cdev)
> > +                                    struct thermal_cooling_device *cdev,
> > +                                    long upper, long lower)
> >  {
> >        struct thermal_cooling_device_instance *dev;
> >        struct thermal_cooling_device_instance *pos;
> > @@ -784,6 +785,15 @@ int thermal_zone_bind_cooling_device(str
> >        if (tz != pos1 || cdev != pos2)
> >                return -EINVAL;
> >
> > +       cdev->ops->get_max_state(cdev, &max_state);
> > +
> > +       /* lower default 0, upper default max_state */
> > +       lower = lower < 0 ? 0 : lower;
> > +       upper = upper < 0 ? max_state : upper;
> > +
> > +       if (lower > upper || upper > max_state)
> > +               return -EINVAL;
> > +
> >        dev =
> >            kzalloc(sizeof(struct thermal_cooling_device_instance), GFP_KERNEL);
> >        if (!dev)
> > @@ -791,10 +801,8 @@ int thermal_zone_bind_cooling_device(str
> >        dev->tz = tz;
> >        dev->cdev = cdev;
> >        dev->trip = trip;
> > -
> > -       cdev->ops->get_max_state(dev, &max_state);
> > -       dev->upper = max_state;
> > -       dev->lower = 0;
> > +       dev->upper = upper;
> > +       dev->lower = lower;
> Also some check can be added here like this,
>           cdev->ops->get_max_state(cdev, &cur_state);

you mean set_cur_state?
>           if (cur_state !=  lower) {
>               print_err("Set the initial state of the cooling device
> to lower\n");
>               return -EINVAL;
>            }
>            This is needed because in your thermal_zone_update
> implementation the state change is from lower - upper.

No. we should not set it.
Say, you are binding a fan to a active trip point 50, and the current
temperature is 30.

And further more, if you read all the patch set, you can see that every
binding is actually invalid from the beginning.
So the upper and lower limits are just indicators here, it makes real
sense only if this binding is activated.

thanks,
rui

> >
> >        result = get_idr(&tz->idr, &tz->lock, &dev->id);
> >        if (result)
> > Index: rtd3/include/linux/thermal.h
> > ===================================================================
> > --- rtd3.orig/include/linux/thermal.h
> > +++ rtd3/include/linux/thermal.h
> > @@ -143,7 +143,8 @@ struct thermal_zone_device *thermal_zone
> >  void thermal_zone_device_unregister(struct thermal_zone_device *);
> >
> >  int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
> > -                                    struct thermal_cooling_device *);
> > +                                    struct thermal_cooling_device *,
> > +                                    long, long);
> >  int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int,
> >                                       struct thermal_cooling_device *);
> >  void thermal_zone_device_update(struct thermal_zone_device *);
> > Index: rtd3/drivers/acpi/thermal.c
> > ===================================================================
> > --- rtd3.orig/drivers/acpi/thermal.c
> > +++ rtd3/drivers/acpi/thermal.c
> > @@ -729,11 +729,9 @@ static int thermal_notify(struct thermal
> >        return 0;
> >  }
> >
> > -typedef int (*cb)(struct thermal_zone_device *, int,
> > -                 struct thermal_cooling_device *);
> >  static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
> >                                        struct thermal_cooling_device *cdev,
> > -                                       cb action)
> > +                                       int bind)
> >  {
> >        struct acpi_device *device = cdev->devdata;
> >        struct acpi_thermal *tz = thermal->devdata;
> > @@ -758,7 +756,14 @@ static int acpi_thermal_cooling_device_c
> >                        handle = tz->trips.passive.devices.handles[i];
> >                        status = acpi_bus_get_device(handle, &dev);
> >                        if (ACPI_SUCCESS(status) && (dev == device)) {
> > -                               result = action(thermal, trip, cdev);
> > +                               if (bind)
> > +                                       result =
> > +                                               thermal_zone_bind_cooling_device(
> > +                                                       thermal, trip, cdev, -1, -1);
> > +                               else
> > +                                       result =
> > +                                               thermal_zone_unbind_cooling_device(
> > +                                                       thermal, trip, cdev);
> >                                if (result)
> >                                        goto failed;
> >                        }
> > @@ -775,7 +780,13 @@ static int acpi_thermal_cooling_device_c
> >                        handle = tz->trips.active[i].devices.handles[j];
> >                        status = acpi_bus_get_device(handle, &dev);
> >                        if (ACPI_SUCCESS(status) && (dev == device)) {
> > -                               result = action(thermal, trip, cdev);
> > +                               if (bind)
> > +                                       result =
> > +                                               thermal_zone_bind_cooling_device(
> > +                                                       thermal, trip, cdev, -1, -1);
> > +                               else
> > +                                       result = thermal_zone_unbind_cooling_device(
> > +                                                       thermal, trip, cdev);
> >                                if (result)
> >                                        goto failed;
> >                        }
> > @@ -786,7 +797,12 @@ static int acpi_thermal_cooling_device_c
> >                handle = tz->devices.handles[i];
> >                status = acpi_bus_get_device(handle, &dev);
> >                if (ACPI_SUCCESS(status) && (dev == device)) {
> > -                       result = action(thermal, -1, cdev);
> > +                       if (bind)
> > +                               result = thermal_zone_bind_cooling_device(thermal,
> > +                                                               -1, cdev, -1, -1);
> > +                       else
> > +                               result = thermal_zone_unbind_cooling_device(thermal,
> > +                                                               -1, cdev);
> >                        if (result)
> >                                goto failed;
> >                }
> > @@ -800,16 +816,14 @@ static int
> >  acpi_thermal_bind_cooling_device(struct thermal_zone_device *thermal,
> >                                        struct thermal_cooling_device *cdev)
> >  {
> > -       return acpi_thermal_cooling_device_cb(thermal, cdev,
> > -                               thermal_zone_bind_cooling_device);
> > +       return acpi_thermal_cooling_device_cb(thermal, cdev, 1);
> >  }
> >
> >  static int
> >  acpi_thermal_unbind_cooling_device(struct thermal_zone_device *thermal,
> >                                        struct thermal_cooling_device *cdev)
> >  {
> > -       return acpi_thermal_cooling_device_cb(thermal, cdev,
> > -                               thermal_zone_unbind_cooling_device);
> > +       return acpi_thermal_cooling_device_cb(thermal, cdev, 0);
> >  }
> >
> >  static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> > Index: rtd3/drivers/platform/x86/acerhdf.c
> > ===================================================================
> > --- rtd3.orig/drivers/platform/x86/acerhdf.c
> > +++ rtd3/drivers/platform/x86/acerhdf.c
> > @@ -329,7 +329,7 @@ static int acerhdf_bind(struct thermal_z
> >        if (cdev != cl_dev)
> >                return 0;
> >
> > -       if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
> > +       if (thermal_zone_bind_cooling_device(thermal, 0, cdev, -1, -1)) {
> >                pr_err("error binding cooling dev\n");
> >                return -EINVAL;
> >        }
> >
> >


_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

^ permalink raw reply

* Re: [RFC PATCH 6/12] thermal: introduce active cooling algorithm
From: Zhang Rui @ 2012-06-11  6:38 UTC (permalink / raw)
  To: Amit Kachhap; +Cc: linux-acpi@vger.kernel.org, linux-pm
In-Reply-To: <CAK44p227RorAEGcikmXSYHJizxVvwbQkZY9fSS0MvpsA4XyATQ@mail.gmail.com>

On 一, 2012-06-11 at 10:21 +0530, Amit Kachhap wrote:
> On 11 June 2012 08:50, Zhang Rui <rui.zhang@intel.com> wrote:
> >
> > Introduce thermal_zone_trip_update() to update the cooling state
> > of all the cooling devices that are binded to an active trip point.
> >
> > This will be used for passive cooling as well, in the future patches.
> > as both active and passive cooling can share the same algorithm,
> > which is
> >
> > 1. if the temperature is higher than a trip point,
> >   a. if the trend is THERMAL_TREND_RAISING, use higher cooling
> >      state for this trip point
> >   b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
> >      state for this trip point
> >
> > 2. if the temperature is lower than a trip point, use lower
> >   cooling state for this trip point.
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/thermal.c        |    7 ++-
> >  drivers/thermal/thermal_sys.c |   91 ++++++++++++++++++++++++++++++------------
> >  2 files changed, 71 insertions(+), 27 deletions(-)
> >
> > Index: rtd3/drivers/thermal/thermal_sys.c
> > ===================================================================
> > --- rtd3.orig/drivers/thermal/thermal_sys.c
> > +++ rtd3/drivers/thermal/thermal_sys.c
> > @@ -1035,6 +1035,70 @@ void thermal_cooling_device_unregister(s
> >  }
> >  EXPORT_SYMBOL(thermal_cooling_device_unregister);
> >
> > +/*
> > + * Cooling algorithm for active trip points
> > + *
> > + * 1. if the temperature is higher than a trip point,
> > + *    a. if the trend is THERMAL_TREND_RAISING, use higher cooling
> > + *       state for this trip point
> > + *    b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
> > + *       state for this trip point
> > + *
> > + * 2. if the temperature is lower than a trip point, use lower
> > + *    cooling state for this trip point
> > + *
> > + * Note that this behaves the same as the previous passive cooling
> > + * algorithm.
> > + */
> > +
> > +static void thermal_zone_trip_update(struct thermal_zone_device *tz,
> > +                                    int trip, long temp)
> > +{
> > +       struct thermal_cooling_device_instance *instance;
> > +       struct thermal_cooling_device *cdev = NULL;
> > +       unsigned long cur_state, max_state;
> > +       long trip_temp;
> > +       enum thermal_trend trend;
> > +
> > +       tz->ops->get_trip_temp(tz, trip, &trip_temp);
> > +
> > +       if (temp >= trip_temp) {
> > +               thermal_get_trend(tz, trip, &trend);
> > +
> > +               list_for_each_entry(instance, &tz->cooling_devices, node) {
> > +                       if (instance->trip != trip)
> > +                               continue;
> > +
> > +                       cdev = instance->cdev;
> > +
> > +                       cdev->ops->get_cur_state(cdev, &cur_state);
> > +                       cdev->ops->get_max_state(cdev, &max_state);
> > +
> > +                       if (trend == THERMAL_TREND_RAISING) {
> > +                               cur_state = cur_state < instance->upper ?
> > +                                           (cur_state + 1) : instance->upper;
> > +                       } else if (trend == THERMAL_TREND_DROPPING) {
> > +                               cur_state = cur_state > instance->lower ?
> > +                                   (cur_state - 1) : instance->lower;
> > +                       }
> > +                       cdev->ops->set_cur_state(cdev, cur_state);
> > +               }
> > +       } else {        /* below trip */
> > +               list_for_each_entry(instance, &tz->cooling_devices, node) {
> > +                       if (instance->trip != trip)
> > +                               continue;
> Can we use the trend callback here even if the current temperature is
> less than the trip temperature. Basically according to trend it should
> deactivate the cooling devices as done for activating cooling devices.

yes, this depends on the cooling algorithm we want to use.
When the temperature is lower than the trip point, if we want to stop
the fan immediately instead of spinning down step by step, we can do
this.

Actually, what we are discussing right now should be some cooling
governor stuff. which I'm not sure if we should introduce to the generic
thermal manager or not, right now. :)

thanks,
rui

> > +
> > +                       cdev = instance->cdev;
> > +                       cdev->ops->get_cur_state(cdev, &cur_state);
> > +
> > +                       cur_state = cur_state > instance->lower ?
> > +                                   (cur_state - 1) : instance->lower;
> > +                       cdev->ops->set_cur_state(cdev, cur_state);
> > +               }
> > +       }
> > +
> > +       return;
> > +}
> >  /**
> >  * thermal_zone_device_update - force an update of a thermal zone's state
> >  * @ttz:       the thermal zone to update
> > @@ -1045,9 +1109,6 @@ void thermal_zone_device_update(struct t
> >        int count, ret = 0;
> >        long temp, trip_temp;
> >        enum thermal_trip_type trip_type;
> > -       struct thermal_cooling_device_instance *instance;
> > -       struct thermal_cooling_device *cdev;
> > -       unsigned long cur_state, max_state;
> >
> >        mutex_lock(&tz->lock);
> >
> > @@ -1083,29 +1144,7 @@ void thermal_zone_device_update(struct t
> >                                        tz->ops->notify(tz, count, trip_type);
> >                        break;
> >                case THERMAL_TRIP_ACTIVE:
> > -                       list_for_each_entry(instance, &tz->cooling_devices,
> > -                                           node) {
> > -                               if (instance->trip != count)
> > -                                       continue;
> > -
> > -                               cdev = instance->cdev;
> > -
> > -                               cdev->ops->get_cur_state(cdev, &cur_state);
> > -                               cdev->ops->get_max_state(cdev, &max_state);
> > -
> > -                               if (temp >= trip_temp)
> > -                                       cur_state =
> > -                                               cur_state < instance->upper ?
> > -                                               (cur_state + 1) :
> > -                                               instance->upper;
> > -                               else
> > -                                       cur_state =
> > -                                               cur_state > instance->lower ?
> > -                                               (cur_state - 1) :
> > -                                               instance->lower;
> > -
> > -                               cdev->ops->set_cur_state(cdev, cur_state);
> > -                       }
> > +                       thermal_zone_trip_update(tz, count, temp);
> >                        break;
> >                case THERMAL_TRIP_PASSIVE:
> >                        if (temp >= trip_temp || tz->passive)
> > Index: rtd3/drivers/acpi/thermal.c
> > ===================================================================
> > --- rtd3.orig/drivers/acpi/thermal.c
> > +++ rtd3/drivers/acpi/thermal.c
> > @@ -717,7 +717,12 @@ static int thermal_get_trend(struct ther
> >        if (thermal_get_trip_type(thermal, trip, &type))
> >                return -EINVAL;
> >
> > -       /* Only PASSIVE trip points need TREND */
> > +       if (type == THERMAL_TRIP_ACTIVE) {
> > +               /* aggressive active cooling */
> > +               *trend = THERMAL_TREND_RAISING;
> > +               return 0;
> > +       }
> > +
> >        if (type != THERMAL_TRIP_PASSIVE)
> >                return -EINVAL;
> >
> >
> >


_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

^ permalink raw reply

* Re: [RFC PATCH 0/12] generic thermal layer enhancement
From: Eduardo Valentin @ 2012-06-11  9:49 UTC (permalink / raw)
  To: Zhang Rui
  Cc: linux-pm, linux-acpi@vger.kernel.org, Amit Kachhap, R, Durgadoss,
	Rafael J. Wysocki, eduardo, Len, Brown, Matthew Garrett
In-Reply-To: <1339384751.1492.153.camel@rui.sh.intel.com>

Hello Rui,

On Mon, Jun 11, 2012 at 11:19:11AM +0800, Zhang Rui wrote:
> Hi, all,
> 
> this patch set is made to enhance the current generic thermal layer.
> It fixes several gaps discussed in
> http://marc.info/?l=linux-acpi&m=133836783425764&w=2
> and introduces a simple arbitrator to fix the "one cooling devices
> referenced by multiple trip points in multiple thermal zones" problem.

Thanks for taking this further. With code it is much better to progress.

BTW, are you keeping this series somewhere in a branch?

>  
> The whole idea is
> 1) make sure we have multiple cooling states for both active cooling and
>    passive cooling devices. (patch 1,2,3)

Nice!

> 2) remove passive specific requirement, aka, tc1/tc2, and
>    introduce .get_trend() instead, for both active and passive cooling
>    algorithm. (patch 4,5)

Cool. The .get_trend() might be also helpful in case there are sensors
that provide trending computation, or at least, some history buffer.

So, I definitely agree with this approach.

> 3) introduce new function thermal_zone_trip_update(), this contains the
>    code for the general cooling algorithm. (patch 6)

Ok.

> 4) rename some thermal structures. Use thermal_instance instead of
>    thermal_cooling_device_instance, as this structure is used to
>    describe the behavior of a certain cooling device for a certain trip
>    point in a certain thermal zone.
>    thermal_zone_device has a list of all the thermal instances in this
>    thermal zone so that it can update them when the temperature changes.
>    thermal_cooling_device has a list of all the thermal instances for
>    this cooling device so that it can make decision which cooling state
>    should be in when the temperature changes. (patch 7,8,9,10)

Ok.

> 5) introduce a simple arbitrator. (patch 11)
>    When temperature changes, we update a thermal zone in two stages,
>    a) thermal_zone_trip_point() is the general cooling algorithm to
>       update the thermal instances in the thermal zone device
>    b) thermal_zone_do_update() is the arbitrator for the cooling device
>       to choose the deepest cooling state required to enter.

The arbitrator is good starting point. I will have a deeper look on it.

But we may be careful to solve the constraint issue only at thermal code
level. There might be conflicting constraints coming from PM QoS layer,
for instance.

> 6) unify the code for both passive and active cooling.

This is good.

> 
> TODO, add locking mechanism. I know this is important but as this patch
> set changes the thermal layer a(lot, I just want to make sure I'm in the
> right direction before going on.

Right. I second you on the incremental approach.

> 
> BTW, in theory, they should make no change to the behavior of the
> current generic thermal layer users. But I have just done the build
> test.

OK. I will fetch them and give them a trial on my side, then send better review.

> 
> Any comments are welcome.
> 
> thanks,
> rui
> 
> --------------------------------------------------------------------
>  drivers/acpi/thermal.c                   |   76 +++++++--
>  drivers/platform/x86/acerhdf.c           |    4 +-
>  drivers/platform/x86/intel_mid_thermal.c |    2 +-
>  driverk/thermal/spear_thermal.c          |    2 +-
>  drivers/thermal/thermal_sys.c            |  293
> ++++++++++++++++++------------
>  include/linux/thermal.h                  |   22 ++-
>  7 files changed, 271 insertions(+), 148 deletions(-)
> 
> 

--
Eduardo Valentin

^ permalink raw reply

* Re: [RFC PATCH 4/12] thermal:  introduce .get_trend() callback
From: Eduardo Valentin @ 2012-06-11  9:54 UTC (permalink / raw)
  To: Zhang Rui
  Cc: linux-pm, linux-acpi@vger.kernel.org, Amit Kachhap, R, Durgadoss,
	eduardo, Len, Brown, Rafael J. Wysocki, Matthew Garrett
In-Reply-To: <1339384806.1492.157.camel@rui.sh.intel.com>

Hello Rui,

On Mon, Jun 11, 2012 at 11:20:06AM +0800, Zhang Rui wrote:
> 
> tc1 and tc2 are ACPI platform specific concepts, introduce
> .get_trend() as a more general solution.
> 
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/thermal.c        |   32 ++++++++++++++++++++++++++++++++
>  drivers/thermal/thermal_sys.c |   19 +++++++++++++++++--
>  include/linux/thermal.h       |    9 +++++++++
>  3 files changed, 58 insertions(+), 2 deletions(-)
> 
> Index: rtd3/drivers/acpi/thermal.c
> ===================================================================
> --- rtd3.orig/drivers/acpi/thermal.c
> +++ rtd3/drivers/acpi/thermal.c
> @@ -706,6 +706,37 @@ static int thermal_get_crit_temp(struct
>  		return -EINVAL;
>  }
>  
> +static int thermal_get_trend(struct thermal_zone_device *thermal,
> +				int trip, enum thermal_trend *trend)
> +{
> +	struct acpi_thermal *tz = thermal->devdata;
> +	enum thermal_trip_type type;
> +	unsigned long trip_temp;
> +	int i;
> +
> +	if (thermal_get_trip_type(thermal, trip, &type))
> +		return -EINVAL;
> +
> +	/* Only PASSIVE trip points need TREND */
> +	if (type != THERMAL_TRIP_PASSIVE)
> +		return -EINVAL;
> +
> +	if (thermal_get_trip_temp(thermal, trip, &trip_temp))
> +		return -EINVAL;
> +
> +	/*
> +	 * tz->temperature has already been updated by generic thermal layer,
> +	 * before this callback being invoked
> +	 */
> +	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
> +		+ (tz->trips.passive.tc2 * (tz->temperature - trip_temp));
> +
> +	*trend = i > 0 ? THERMAL_TREND_RAISING :
> +		(i < 0 ? THERMAL_TREND_DROPPING : THERMAL_TREND_NONE);
> +	return 0;
> +}
> +
> +
>  static int thermal_notify(struct thermal_zone_device *thermal, int trip,
>  			   enum thermal_trip_type trip_type)
>  {
> @@ -835,6 +866,7 @@ static const struct thermal_zone_device_
>  	.get_trip_type = thermal_get_trip_type,
>  	.get_trip_temp = thermal_get_trip_temp,
>  	.get_crit_temp = thermal_get_crit_temp,
> +	.get_trend = thermal_get_trend,
>  	.notify = thermal_notify,
>  };
>  
> Index: rtd3/drivers/thermal/thermal_sys.c
> ===================================================================
> --- rtd3.orig/drivers/thermal/thermal_sys.c
> +++ rtd3/drivers/thermal/thermal_sys.c
> @@ -657,6 +657,18 @@ thermal_remove_hwmon_sysfs(struct therma
>  }
>  #endif
>  
> +static void thermal_get_trend (struct thermal_zone_device *tz,
> +		int trip, enum thermal_trend *trend)
> +{
> +	if (tz->ops->get_trend)
> +		if (!tz->ops->get_trend(tz, trip, trend))
> +			return;
> +
> +	*trend = tz->temperature >= tz->last_temperature ?
> +		 THERMAL_TREND_RAISING : THERMAL_TREND_DROPPING;
> +	return;
> +}
> +
>  static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
>  					    int delay)
>  {
> @@ -691,6 +703,8 @@ static void thermal_zone_device_passive(
>  	if (temp >= trip_temp) {
>  		tz->passive = true;
>  
> +		thermal_get_trend(tz, trip, (enum thermal_trend *)&trend);
> +
>  		trend = (tz->tc1 * (temp - tz->last_temperature)) +
>  			(tz->tc2 * (temp - trip_temp));
>  
> @@ -1049,6 +1063,9 @@ void thermal_zone_device_update(struct t
>  		goto leave;
>  	}
>  
> +	tz->last_temperature = tz->temperature;
> +	tz->temperature = temp;
> +
>  	for (count = 0; count < tz->trips; count++) {
>  		tz->ops->get_trip_type(tz, count, &trip_type);
>  		tz->ops->get_trip_temp(tz, count, &trip_temp);
> @@ -1108,8 +1125,6 @@ void thermal_zone_device_update(struct t
>  		thermal_zone_device_passive(tz, temp, tz->forced_passive,
>  					    THERMAL_TRIPS_NONE);
>  
> -	tz->last_temperature = temp;
> -
>  leave:
>  	if (tz->passive)
>  		thermal_zone_device_set_polling(tz, tz->passive_delay);
> Index: rtd3/include/linux/thermal.h
> ===================================================================
> --- rtd3.orig/include/linux/thermal.h
> +++ rtd3/include/linux/thermal.h
> @@ -44,6 +44,12 @@ enum thermal_trip_type {
>  	THERMAL_TRIP_CRITICAL,
>  };
>  
> +enum thermal_trend {
> +	THERMAL_TREND_NONE,
> +	THERMAL_TREND_RAISING,
> +	THERMAL_TREND_DROPPING,
> +};
> +

Nip: Can you please add some doc for the expected definition for all enum entries above?
Is TREND_NONE supposed to mean "Stable", for instance?

>  struct thermal_zone_device_ops {
>  	int (*bind) (struct thermal_zone_device *,
>  		     struct thermal_cooling_device *);
> @@ -59,6 +65,8 @@ struct thermal_zone_device_ops {
>  	int (*get_trip_temp) (struct thermal_zone_device *, int,
>  			      unsigned long *);
>  	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
> +	int (*get_trend) (struct thermal_zone_device *, int,
> +			  enum thermal_trend *);
>  	int (*notify) (struct thermal_zone_device *, int,
>  		       enum thermal_trip_type);
>  };
> @@ -95,6 +103,7 @@ struct thermal_zone_device {
>  	int tc2;
>  	int passive_delay;
>  	int polling_delay;
> +	int temperature;
>  	int last_temperature;
>  	bool passive;
>  	unsigned int forced_passive;
> 
> 

^ permalink raw reply

* Re: [RFC PATCH 6/12] thermal: introduce active cooling algorithm
From: Eduardo Valentin @ 2012-06-11 11:44 UTC (permalink / raw)
  To: Zhang Rui
  Cc: Amit Kachhap, linux-pm, linux-acpi@vger.kernel.org, eduardo,
	R, Durgadoss, Len, Brown, Rafael J. Wysocki, Matthew Garrett
In-Reply-To: <1339396704.1492.193.camel@rui.sh.intel.com>


Hello,

On Mon, Jun 11, 2012 at 02:38:24PM +0800, Zhang Rui wrote:
> On 一, 2012-06-11 at 10:21 +0530, Amit Kachhap wrote:
> > On 11 June 2012 08:50, Zhang Rui <rui.zhang@intel.com> wrote:
> > >
> > > Introduce thermal_zone_trip_update() to update the cooling state
> > > of all the cooling devices that are binded to an active trip point.
> > >
> > > This will be used for passive cooling as well, in the future patches.
> > > as both active and passive cooling can share the same algorithm,
> > > which is
> > >
> > > 1. if the temperature is higher than a trip point,
> > >   a. if the trend is THERMAL_TREND_RAISING, use higher cooling
> > >      state for this trip point
> > >   b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
> > >      state for this trip point
> > >
> > > 2. if the temperature is lower than a trip point, use lower
> > >   cooling state for this trip point.
> > >
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > ---
> > >  drivers/acpi/thermal.c        |    7 ++-
> > >  drivers/thermal/thermal_sys.c |   91 ++++++++++++++++++++++++++++++------------
> > >  2 files changed, 71 insertions(+), 27 deletions(-)
> > >
> > > Index: rtd3/drivers/thermal/thermal_sys.c
> > > ===================================================================
> > > --- rtd3.orig/drivers/thermal/thermal_sys.c
> > > +++ rtd3/drivers/thermal/thermal_sys.c
> > > @@ -1035,6 +1035,70 @@ void thermal_cooling_device_unregister(s
> > >  }
> > >  EXPORT_SYMBOL(thermal_cooling_device_unregister);
> > >
> > > +/*
> > > + * Cooling algorithm for active trip points
> > > + *
> > > + * 1. if the temperature is higher than a trip point,
> > > + *    a. if the trend is THERMAL_TREND_RAISING, use higher cooling
> > > + *       state for this trip point
> > > + *    b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
> > > + *       state for this trip point
> > > + *
> > > + * 2. if the temperature is lower than a trip point, use lower
> > > + *    cooling state for this trip point
> > > + *
> > > + * Note that this behaves the same as the previous passive cooling
> > > + * algorithm.
> > > + */
> > > +
> > > +static void thermal_zone_trip_update(struct thermal_zone_device *tz,
> > > +                                    int trip, long temp)
> > > +{
> > > +       struct thermal_cooling_device_instance *instance;
> > > +       struct thermal_cooling_device *cdev = NULL;
> > > +       unsigned long cur_state, max_state;
> > > +       long trip_temp;
> > > +       enum thermal_trend trend;
> > > +
> > > +       tz->ops->get_trip_temp(tz, trip, &trip_temp);
> > > +
> > > +       if (temp >= trip_temp) {
> > > +               thermal_get_trend(tz, trip, &trend);
> > > +
> > > +               list_for_each_entry(instance, &tz->cooling_devices, node) {
> > > +                       if (instance->trip != trip)
> > > +                               continue;
> > > +
> > > +                       cdev = instance->cdev;
> > > +
> > > +                       cdev->ops->get_cur_state(cdev, &cur_state);
> > > +                       cdev->ops->get_max_state(cdev, &max_state);
> > > +
> > > +                       if (trend == THERMAL_TREND_RAISING) {
> > > +                               cur_state = cur_state < instance->upper ?
> > > +                                           (cur_state + 1) : instance->upper;
> > > +                       } else if (trend == THERMAL_TREND_DROPPING) {
> > > +                               cur_state = cur_state > instance->lower ?
> > > +                                   (cur_state - 1) : instance->lower;
> > > +                       }
> > > +                       cdev->ops->set_cur_state(cdev, cur_state);
> > > +               }
> > > +       } else {        /* below trip */
> > > +               list_for_each_entry(instance, &tz->cooling_devices, node) {
> > > +                       if (instance->trip != trip)
> > > +                               continue;
> > Can we use the trend callback here even if the current temperature is
> > less than the trip temperature. Basically according to trend it should
> > deactivate the cooling devices as done for activating cooling devices.
> 
> yes, this depends on the cooling algorithm we want to use.
> When the temperature is lower than the trip point, if we want to stop
> the fan immediately instead of spinning down step by step, we can do
> this.
> 
> Actually, what we are discussing right now should be some cooling
> governor stuff. which I'm not sure if we should introduce to the generic
> thermal manager or not, right now. :)

Rui, this is an interesting point. Do you have plans to have governor like approach
in the generic thermal fw? Or are you sticking to the zone/cooling binding?

> 
> thanks,
> rui
> 
> > > +
> > > +                       cdev = instance->cdev;
> > > +                       cdev->ops->get_cur_state(cdev, &cur_state);
> > > +
> > > +                       cur_state = cur_state > instance->lower ?
> > > +                                   (cur_state - 1) : instance->lower;
> > > +                       cdev->ops->set_cur_state(cdev, cur_state);
> > > +               }
> > > +       }
> > > +
> > > +       return;
> > > +}
> > >  /**
> > >  * thermal_zone_device_update - force an update of a thermal zone's state
> > >  * @ttz:       the thermal zone to update
> > > @@ -1045,9 +1109,6 @@ void thermal_zone_device_update(struct t
> > >        int count, ret = 0;
> > >        long temp, trip_temp;
> > >        enum thermal_trip_type trip_type;
> > > -       struct thermal_cooling_device_instance *instance;
> > > -       struct thermal_cooling_device *cdev;
> > > -       unsigned long cur_state, max_state;
> > >
> > >        mutex_lock(&tz->lock);
> > >
> > > @@ -1083,29 +1144,7 @@ void thermal_zone_device_update(struct t
> > >                                        tz->ops->notify(tz, count, trip_type);
> > >                        break;
> > >                case THERMAL_TRIP_ACTIVE:
> > > -                       list_for_each_entry(instance, &tz->cooling_devices,
> > > -                                           node) {
> > > -                               if (instance->trip != count)
> > > -                                       continue;
> > > -
> > > -                               cdev = instance->cdev;
> > > -
> > > -                               cdev->ops->get_cur_state(cdev, &cur_state);
> > > -                               cdev->ops->get_max_state(cdev, &max_state);
> > > -
> > > -                               if (temp >= trip_temp)
> > > -                                       cur_state =
> > > -                                               cur_state < instance->upper ?
> > > -                                               (cur_state + 1) :
> > > -                                               instance->upper;
> > > -                               else
> > > -                                       cur_state =
> > > -                                               cur_state > instance->lower ?
> > > -                                               (cur_state - 1) :
> > > -                                               instance->lower;
> > > -
> > > -                               cdev->ops->set_cur_state(cdev, cur_state);
> > > -                       }
> > > +                       thermal_zone_trip_update(tz, count, temp);
> > >                        break;
> > >                case THERMAL_TRIP_PASSIVE:
> > >                        if (temp >= trip_temp || tz->passive)
> > > Index: rtd3/drivers/acpi/thermal.c
> > > ===================================================================
> > > --- rtd3.orig/drivers/acpi/thermal.c
> > > +++ rtd3/drivers/acpi/thermal.c
> > > @@ -717,7 +717,12 @@ static int thermal_get_trend(struct ther
> > >        if (thermal_get_trip_type(thermal, trip, &type))
> > >                return -EINVAL;
> > >
> > > -       /* Only PASSIVE trip points need TREND */
> > > +       if (type == THERMAL_TRIP_ACTIVE) {
> > > +               /* aggressive active cooling */
> > > +               *trend = THERMAL_TREND_RAISING;
> > > +               return 0;
> > > +       }
> > > +
> > >        if (type != THERMAL_TRIP_PASSIVE)
> > >                return -EINVAL;
> > >
> > >
> > >
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH 6/12] thermal: introduce active cooling algorithm
From: R, Durgadoss @ 2012-06-11 11:48 UTC (permalink / raw)
  To: eduardo.valentin@ti.com, Zhang, Rui; +Cc: linux-acpi@vger.kernel.org, linux-pm
In-Reply-To: <20120611114450.GC3649@besouro>

Hi Eduardo,

> > > Can we use the trend callback here even if the current temperature is
> > > less than the trip temperature. Basically according to trend it should
> > > deactivate the cooling devices as done for activating cooling devices.
> >
> > yes, this depends on the cooling algorithm we want to use.
> > When the temperature is lower than the trip point, if we want to stop
> > the fan immediately instead of spinning down step by step, we can do
> > this.
> >
> > Actually, what we are discussing right now should be some cooling
> > governor stuff. which I'm not sure if we should introduce to the generic
> > thermal manager or not, right now. :)
> 
> Rui, this is an interesting point. Do you have plans to have governor like approach
> in the generic thermal fw? Or are you sticking to the zone/cooling binding?

Yes We have. I am almost done.
Will submit patches by tomorrow.

Unfortunately, I cannot re-base my code now on top of Rui's patches.
So, I will submit an RFC version and we can work to resolve the conflicts.

Thanks,
Durga

^ permalink raw reply

* Re: Suspend/resume regressions on Lenovo S10-3
From: Srivatsa S. Bhat @ 2012-06-11 13:31 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Kevin Hilman, Arnd Bergmann, Linux PM mailing list, Tomas M.,
	LKML, Len Brown, preeti, linux-pm, Ferenc Wagner,
	Arjan van de Ven, Jean Pihet
In-Reply-To: <4FCF5249.2070600@linux.vnet.ibm.com>

Hi Dave,

On 06/06/2012 06:21 PM, Deepthi Dharwar wrote:

> On 05/28/2012 07:23 AM, Dave Hansen wrote:
> 
>> I have a Lenovo S10-3 Atom netbook.  It's always had some amount of
>> trouble working with the intel_idle driver, so I usually compile that
>> out an use the acpi one.


What problem did you face with intel_idle driver? Is it suspend/resume
related?

I see a comment in drivers/idle/intel_idle.c such as:

/*
 * Known limitations
 * [...]
 *
 * ACPI has a .suspend hack to turn off deep c-statees during suspend
 * to avoid complications with the lapic timer workaround.
 * Have not seen issues with suspend, but may need same workaround here.
 *
 */

So, if you are facing suspend issues with the intel_idle driver, we probably
need to add that same workaround here as well, to make it work.

Please let us know what problem you are facing.

Regards,
Srivatsa S. Bhat

>>  However, just after 3.1, suspend/resume broke.
>>  'echo mem > /sys/power/state' would hang before suspending.  I bisected
>> it down to the commits around:
>>
>> 	e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
>>
>> by Deepthi.  But, current mainline (v3.4-07644-g07acfc2) hangs with a
>> different symptom: it suspends, but hangs on resume from suspend.  I
>> think _that_ delta in the behavior was caused by:
>>
>> 	3439a8da16bcad6b0982ece938c9f8299bb53584
>>
>>         ACPI / cpuidle: Remove acpi_idle_suspend (to fix suspend
>> 		regression)
>>
>> It's a bit of a pain to bisect these two different things in parallel.
>> I was trying to tell git bisect 'good' on working suspend/resume, 'bad'
>> on the hang during resume, and 'skip' on the hangs _during_ suspend.  83
>> kernels in, I'm not sure that's working very well. :)
>>
>> Deepthi, do you have any idea why your patches broke me in the first
>> place?  Perhaps we should fix that regression first before we go on and
>> try to figure out what changed to let it suspend again, but break later.
> 
> 
> Hi Dave,
> 
> Sorry about my patches breaking your suspend-resume.
> 
> I, basically tried out building and booting 3.1 kernel with 
> my patch set to reproduce the failure. I could clearly
> see suspend not happening. It turns out to be 
> a bug with my first patch in global registration
> series submitted earlier.
> 
> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
> 
> The following patch, fixes the suspend issues
> seen on my laptop due to earlier cpuidle cleanup 
> (Lenevo T420 booting with acpi_idle enabled). 
> Can you please give this a try
> on top of my patch set (without Rafael's fix) 
> and see if it fixes the problem for you. 
> I am not reverting acpi_idle_suspend flag and 
> hopefully it should resume fine too. 
> 
> ---
> 
> This patch fixes suspend-resume issue seen in the kernel 3.1
> series using acpi_idle_driver because of cpuidle global 
> registration cleanup.
> Here, when acpi_idle_suspend flag was set ( during suspend)
> the interrupts were not getting enabled in acpi_idle_enter_bm()
> routine which was causing the system to hang.
> 
> 
> Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
> 
> ---
>  drivers/acpi/processor_idle.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 24fe3af..6e35293 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -895,8 +895,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>  	if (unlikely(!pr))
>  		return -EINVAL;
> 
> -
>  	if (acpi_idle_suspend) {
> +		local_irq_disable();
> +		local_irq_enable();
>  		cpu_relax();
>  		return -EINVAL;
>  	}
> 
> Cheers,
> Deepthi
> 



-- 
Regards,
Srivatsa S. Bhat
IBM Linux Technology Center

^ permalink raw reply

* Re: Suspend/resume regressions on Lenovo S10-3
From: Dave Hansen @ 2012-06-11 14:57 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: Kevin Hilman, Arnd Bergmann, Linux PM mailing list, Tomas M.,
	LKML, Len Brown, preeti, linux-pm, Ferenc Wagner,
	Arjan van de Ven, Jean Pihet
In-Reply-To: <4FD5F329.2090302@linux.vnet.ibm.com>

On 06/11/2012 06:31 AM, Srivatsa S. Bhat wrote:
> Hi Dave,
> 
> On 06/06/2012 06:21 PM, Deepthi Dharwar wrote:
> 
>> On 05/28/2012 07:23 AM, Dave Hansen wrote:
>>
>>> I have a Lenovo S10-3 Atom netbook.  It's always had some amount of
>>> trouble working with the intel_idle driver, so I usually compile that
>>> out an use the acpi one.
> 
> 
> What problem did you face with intel_idle driver? Is it suspend/resume
> related?

With intel_idle, sometimes it won't boot, others it won't suspend/resume:

  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/674075

I'm actually not sure how it behaves on current kernels.

^ permalink raw reply

* Re: [RFC PATCH 0/12] generic thermal layer enhancement
From: Zhang Rui @ 2012-06-12  3:01 UTC (permalink / raw)
  To: eduardo.valentin
  Cc: linux-pm, linux-acpi@vger.kernel.org, Amit Kachhap, R, Durgadoss,
	Rafael J. Wysocki, Len, Brown, Matthew Garrett
In-Reply-To: <20120611094940.GA3649@besouro>

On 一, 2012-06-11 at 12:49 +0300, Eduardo Valentin wrote:
> Hello Rui,
> 
> On Mon, Jun 11, 2012 at 11:19:11AM +0800, Zhang Rui wrote:
> > Hi, all,
> > 
> > this patch set is made to enhance the current generic thermal layer.
> > It fixes several gaps discussed in
> > http://marc.info/?l=linux-acpi&m=133836783425764&w=2
> > and introduces a simple arbitrator to fix the "one cooling devices
> > referenced by multiple trip points in multiple thermal zones" problem.
> 
> Thanks for taking this further. With code it is much better to progress.
> 
> BTW, are you keeping this series somewhere in a branch?
> 
Not yet.
But I'm thinking of create one. :)

> >  
> > The whole idea is
> > 1) make sure we have multiple cooling states for both active cooling and
> >    passive cooling devices. (patch 1,2,3)
> 
> Nice!
> 
> > 2) remove passive specific requirement, aka, tc1/tc2, and
> >    introduce .get_trend() instead, for both active and passive cooling
> >    algorithm. (patch 4,5)
> 
> Cool. The .get_trend() might be also helpful in case there are sensors
> that provide trending computation, or at least, some history buffer.
> 
> So, I definitely agree with this approach.
> 
> > 3) introduce new function thermal_zone_trip_update(), this contains the
> >    code for the general cooling algorithm. (patch 6)
> 
> Ok.
> 
> > 4) rename some thermal structures. Use thermal_instance instead of
> >    thermal_cooling_device_instance, as this structure is used to
> >    describe the behavior of a certain cooling device for a certain trip
> >    point in a certain thermal zone.
> >    thermal_zone_device has a list of all the thermal instances in this
> >    thermal zone so that it can update them when the temperature changes.
> >    thermal_cooling_device has a list of all the thermal instances for
> >    this cooling device so that it can make decision which cooling state
> >    should be in when the temperature changes. (patch 7,8,9,10)
> 
> Ok.
> 
> > 5) introduce a simple arbitrator. (patch 11)
> >    When temperature changes, we update a thermal zone in two stages,
> >    a) thermal_zone_trip_point() is the general cooling algorithm to
> >       update the thermal instances in the thermal zone device
> >    b) thermal_zone_do_update() is the arbitrator for the cooling device
> >       to choose the deepest cooling state required to enter.
> 
> The arbitrator is good starting point. I will have a deeper look on it.
> 
> But we may be careful to solve the constraint issue only at thermal code
> level. There might be conflicting constraints coming from PM QoS layer,
> for instance.
> 
hmmm, do you have a link for the discussions/patches mentioned?

I'll take a look at this. :)

> > 6) unify the code for both passive and active cooling.
> 
> This is good.
> 
> > 
> > TODO, add locking mechanism. I know this is important but as this patch
> > set changes the thermal layer a(lot, I just want to make sure I'm in the
> > right direction before going on.
> 
> Right. I second you on the incremental approach.
> 
> > 
> > BTW, in theory, they should make no change to the behavior of the
> > current generic thermal layer users. But I have just done the build
> > test.
> 
> OK. I will fetch them and give them a trial on my side, then send better review.
> 
Great. That would be really helpful. Thanks for your interest on this,
Eduardo! :)

-rui

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

^ permalink raw reply

* Re: [RFC PATCH 4/12] thermal: introduce .get_trend() callback
From: Zhang Rui @ 2012-06-12  4:59 UTC (permalink / raw)
  To: eduardo.valentin; +Cc: linux-acpi@vger.kernel.org, linux-pm
In-Reply-To: <20120611095443.GB3649@besouro>

On 一, 2012-06-11 at 12:54 +0300, Eduardo Valentin wrote:
> Hello Rui,
> 
> On Mon, Jun 11, 2012 at 11:20:06AM +0800, Zhang Rui wrote:
> > 
> > tc1 and tc2 are ACPI platform specific concepts, introduce
> > .get_trend() as a more general solution.
> > 
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/thermal.c        |   32 ++++++++++++++++++++++++++++++++
> >  drivers/thermal/thermal_sys.c |   19 +++++++++++++++++--
> >  include/linux/thermal.h       |    9 +++++++++
> >  3 files changed, 58 insertions(+), 2 deletions(-)
> > 
> > Index: rtd3/drivers/acpi/thermal.c
> > ===================================================================
> > --- rtd3.orig/drivers/acpi/thermal.c
> > +++ rtd3/drivers/acpi/thermal.c
> > @@ -706,6 +706,37 @@ static int thermal_get_crit_temp(struct
> >  		return -EINVAL;
> >  }
> >  
> > +static int thermal_get_trend(struct thermal_zone_device *thermal,
> > +				int trip, enum thermal_trend *trend)
> > +{
> > +	struct acpi_thermal *tz = thermal->devdata;
> > +	enum thermal_trip_type type;
> > +	unsigned long trip_temp;
> > +	int i;
> > +
> > +	if (thermal_get_trip_type(thermal, trip, &type))
> > +		return -EINVAL;
> > +
> > +	/* Only PASSIVE trip points need TREND */
> > +	if (type != THERMAL_TRIP_PASSIVE)
> > +		return -EINVAL;
> > +
> > +	if (thermal_get_trip_temp(thermal, trip, &trip_temp))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * tz->temperature has already been updated by generic thermal layer,
> > +	 * before this callback being invoked
> > +	 */
> > +	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
> > +		+ (tz->trips.passive.tc2 * (tz->temperature - trip_temp));
> > +
> > +	*trend = i > 0 ? THERMAL_TREND_RAISING :
> > +		(i < 0 ? THERMAL_TREND_DROPPING : THERMAL_TREND_NONE);
> > +	return 0;
> > +}
> > +
> > +
> >  static int thermal_notify(struct thermal_zone_device *thermal, int trip,
> >  			   enum thermal_trip_type trip_type)
> >  {
> > @@ -835,6 +866,7 @@ static const struct thermal_zone_device_
> >  	.get_trip_type = thermal_get_trip_type,
> >  	.get_trip_temp = thermal_get_trip_temp,
> >  	.get_crit_temp = thermal_get_crit_temp,
> > +	.get_trend = thermal_get_trend,
> >  	.notify = thermal_notify,
> >  };
> >  
> > Index: rtd3/drivers/thermal/thermal_sys.c
> > ===================================================================
> > --- rtd3.orig/drivers/thermal/thermal_sys.c
> > +++ rtd3/drivers/thermal/thermal_sys.c
> > @@ -657,6 +657,18 @@ thermal_remove_hwmon_sysfs(struct therma
> >  }
> >  #endif
> >  
> > +static void thermal_get_trend (struct thermal_zone_device *tz,
> > +		int trip, enum thermal_trend *trend)
> > +{
> > +	if (tz->ops->get_trend)
> > +		if (!tz->ops->get_trend(tz, trip, trend))
> > +			return;
> > +
> > +	*trend = tz->temperature >= tz->last_temperature ?
> > +		 THERMAL_TREND_RAISING : THERMAL_TREND_DROPPING;
> > +	return;
> > +}
> > +
> >  static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
> >  					    int delay)
> >  {
> > @@ -691,6 +703,8 @@ static void thermal_zone_device_passive(
> >  	if (temp >= trip_temp) {
> >  		tz->passive = true;
> >  
> > +		thermal_get_trend(tz, trip, (enum thermal_trend *)&trend);
> > +
> >  		trend = (tz->tc1 * (temp - tz->last_temperature)) +
> >  			(tz->tc2 * (temp - trip_temp));
> >  
> > @@ -1049,6 +1063,9 @@ void thermal_zone_device_update(struct t
> >  		goto leave;
> >  	}
> >  
> > +	tz->last_temperature = tz->temperature;
> > +	tz->temperature = temp;
> > +
> >  	for (count = 0; count < tz->trips; count++) {
> >  		tz->ops->get_trip_type(tz, count, &trip_type);
> >  		tz->ops->get_trip_temp(tz, count, &trip_temp);
> > @@ -1108,8 +1125,6 @@ void thermal_zone_device_update(struct t
> >  		thermal_zone_device_passive(tz, temp, tz->forced_passive,
> >  					    THERMAL_TRIPS_NONE);
> >  
> > -	tz->last_temperature = temp;
> > -
> >  leave:
> >  	if (tz->passive)
> >  		thermal_zone_device_set_polling(tz, tz->passive_delay);
> > Index: rtd3/include/linux/thermal.h
> > ===================================================================
> > --- rtd3.orig/include/linux/thermal.h
> > +++ rtd3/include/linux/thermal.h
> > @@ -44,6 +44,12 @@ enum thermal_trip_type {
> >  	THERMAL_TRIP_CRITICAL,
> >  };
> >  
> > +enum thermal_trend {
> > +	THERMAL_TREND_NONE,
> > +	THERMAL_TREND_RAISING,
> > +	THERMAL_TREND_DROPPING,
> > +};
> > +
> 
> Nip: Can you please add some doc for the expected definition for all enum entries above?
> Is TREND_NONE supposed to mean "Stable", for instance?
> 
yep. TREND_STABLE sounds better. I'm not good at naming.

thanks,
rui
> >  struct thermal_zone_device_ops {
> >  	int (*bind) (struct thermal_zone_device *,
> >  		     struct thermal_cooling_device *);
> > @@ -59,6 +65,8 @@ struct thermal_zone_device_ops {
> >  	int (*get_trip_temp) (struct thermal_zone_device *, int,
> >  			      unsigned long *);
> >  	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
> > +	int (*get_trend) (struct thermal_zone_device *, int,
> > +			  enum thermal_trend *);
> >  	int (*notify) (struct thermal_zone_device *, int,
> >  		       enum thermal_trip_type);
> >  };
> > @@ -95,6 +103,7 @@ struct thermal_zone_device {
> >  	int tc2;
> >  	int passive_delay;
> >  	int polling_delay;
> > +	int temperature;
> >  	int last_temperature;
> >  	bool passive;
> >  	unsigned int forced_passive;
> > 
> > 


_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

^ permalink raw reply

* Re: [RFC PATCH 3/12] thermal: set upper and lower limits when binding
From: Eduardo Valentin @ 2012-06-12  9:52 UTC (permalink / raw)
  To: Zhang Rui; +Cc: linux-acpi@vger.kernel.org, linux-pm
In-Reply-To: <1339384801.1492.156.camel@rui.sh.intel.com>

Hello Rui,

On Mon, Jun 11, 2012 at 11:20:01AM +0800, Zhang Rui wrote:
> 
> Set upper and lower limits when binding
> a thermal cooling device to a thermal zone device.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/thermal.c         |   34 ++++++++++++++++++++++++----------
>  drivers/platform/x86/acerhdf.c |    2 +-
>  drivers/thermal/thermal_sys.c  |   20 ++++++++++++++------
>  include/linux/thermal.h        |    3 ++-
>  4 files changed, 41 insertions(+), 18 deletions(-)
> 
> Index: rtd3/drivers/thermal/thermal_sys.c
> ===================================================================
> --- rtd3.orig/drivers/thermal/thermal_sys.c
> +++ rtd3/drivers/thermal/thermal_sys.c
> @@ -248,7 +248,7 @@ passive_store(struct device *dev, struct
>  				     sizeof("Processor")))
>  				thermal_zone_bind_cooling_device(tz,
>  								 THERMAL_TRIPS_NONE,
> -								 cdev);
> +								 cdev, -1, -1);
>  		}
>  		mutex_unlock(&thermal_list_lock);
>  		if (!tz->passive_delay)
> @@ -760,7 +760,8 @@ static void thermal_zone_device_check(st
>   */
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  				     int trip,
> -				     struct thermal_cooling_device *cdev)
> +				     struct thermal_cooling_device *cdev,
> +				     long upper, long lower)
>  {
>  	struct thermal_cooling_device_instance *dev;
>  	struct thermal_cooling_device_instance *pos;
> @@ -784,6 +785,15 @@ int thermal_zone_bind_cooling_device(str
>  	if (tz != pos1 || cdev != pos2)
>  		return -EINVAL;
>  
> +	cdev->ops->get_max_state(cdev, &max_state);
> +
> +	/* lower default 0, upper default max_state */
> +	lower = lower < 0 ? 0 : lower;
> +	upper = upper < 0 ? max_state : upper;
> +
> +	if (lower > upper || upper > max_state)
> +		return -EINVAL;
> +

>From this version of the code, I assume we want to:
a. Allow cooling binding instances to have ranges overlapping.
b. Allow a binding with only one state (lower == upper).

Is this the expected behavior?

>  	dev =
>  	    kzalloc(sizeof(struct thermal_cooling_device_instance), GFP_KERNEL);
>  	if (!dev)
> @@ -791,10 +801,8 @@ int thermal_zone_bind_cooling_device(str
>  	dev->tz = tz;
>  	dev->cdev = cdev;
>  	dev->trip = trip;
> -
> -	cdev->ops->get_max_state(dev, &max_state);
> -	dev->upper = max_state;
> -	dev->lower = 0;
> +	dev->upper = upper;
> +	dev->lower = lower;
>  
>  	result = get_idr(&tz->idr, &tz->lock, &dev->id);
>  	if (result)
> Index: rtd3/include/linux/thermal.h
> ===================================================================
> --- rtd3.orig/include/linux/thermal.h
> +++ rtd3/include/linux/thermal.h
> @@ -143,7 +143,8 @@ struct thermal_zone_device *thermal_zone
>  void thermal_zone_device_unregister(struct thermal_zone_device *);
>  
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
> -				     struct thermal_cooling_device *);
> +				     struct thermal_cooling_device *,
> +				     long, long);
>  int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int,
>  				       struct thermal_cooling_device *);
>  void thermal_zone_device_update(struct thermal_zone_device *);
> Index: rtd3/drivers/acpi/thermal.c
> ===================================================================
> --- rtd3.orig/drivers/acpi/thermal.c
> +++ rtd3/drivers/acpi/thermal.c
> @@ -729,11 +729,9 @@ static int thermal_notify(struct thermal
>  	return 0;
>  }
>  
> -typedef int (*cb)(struct thermal_zone_device *, int,
> -		  struct thermal_cooling_device *);
>  static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
>  					struct thermal_cooling_device *cdev,
> -					cb action)
> +					int bind)
>  {
>  	struct acpi_device *device = cdev->devdata;
>  	struct acpi_thermal *tz = thermal->devdata;
> @@ -758,7 +756,14 @@ static int acpi_thermal_cooling_device_c
>  			handle = tz->trips.passive.devices.handles[i];
>  			status = acpi_bus_get_device(handle, &dev);
>  			if (ACPI_SUCCESS(status) && (dev == device)) {
> -				result = action(thermal, trip, cdev);
> +				if (bind)
> +					result =
> +						thermal_zone_bind_cooling_device(
> +							thermal, trip, cdev, -1, -1);
> +				else
> +					result =
> +						thermal_zone_unbind_cooling_device(
> +							thermal, trip, cdev);
>  				if (result)
>  					goto failed;
>  			}
> @@ -775,7 +780,13 @@ static int acpi_thermal_cooling_device_c
>  			handle = tz->trips.active[i].devices.handles[j];
>  			status = acpi_bus_get_device(handle, &dev);
>  			if (ACPI_SUCCESS(status) && (dev == device)) {
> -				result = action(thermal, trip, cdev);
> +				if (bind)
> +					result =
> +						thermal_zone_bind_cooling_device(
> +							thermal, trip, cdev, -1, -1);
> +				else
> +					result = thermal_zone_unbind_cooling_device(
> +							thermal, trip, cdev);
>  				if (result)
>  					goto failed;
>  			}
> @@ -786,7 +797,12 @@ static int acpi_thermal_cooling_device_c
>  		handle = tz->devices.handles[i];
>  		status = acpi_bus_get_device(handle, &dev);
>  		if (ACPI_SUCCESS(status) && (dev == device)) {
> -			result = action(thermal, -1, cdev);
> +			if (bind)
> +				result = thermal_zone_bind_cooling_device(thermal,
> +								-1, cdev, -1, -1);
> +			else
> +				result = thermal_zone_unbind_cooling_device(thermal,
> +								-1, cdev);
>  			if (result)
>  				goto failed;
>  		}
> @@ -800,16 +816,14 @@ static int
>  acpi_thermal_bind_cooling_device(struct thermal_zone_device *thermal,
>  					struct thermal_cooling_device *cdev)
>  {
> -	return acpi_thermal_cooling_device_cb(thermal, cdev,
> -				thermal_zone_bind_cooling_device);
> +	return acpi_thermal_cooling_device_cb(thermal, cdev, 1);
>  }
>  
>  static int
>  acpi_thermal_unbind_cooling_device(struct thermal_zone_device *thermal,
>  					struct thermal_cooling_device *cdev)
>  {
> -	return acpi_thermal_cooling_device_cb(thermal, cdev,
> -				thermal_zone_unbind_cooling_device);
> +	return acpi_thermal_cooling_device_cb(thermal, cdev, 0);
>  }
>  
>  static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> Index: rtd3/drivers/platform/x86/acerhdf.c
> ===================================================================
> --- rtd3.orig/drivers/platform/x86/acerhdf.c
> +++ rtd3/drivers/platform/x86/acerhdf.c
> @@ -329,7 +329,7 @@ static int acerhdf_bind(struct thermal_z
>  	if (cdev != cl_dev)
>  		return 0;
>  
> -	if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
> +	if (thermal_zone_bind_cooling_device(thermal, 0, cdev, -1, -1)) {
>  		pr_err("error binding cooling dev\n");
>  		return -EINVAL;
>  	}
> 
> 

^ permalink raw reply

* [PATCH 0/4] thermal sys: couple of fixes and cleanups
From: Eduardo Valentin @ 2012-06-12 13:25 UTC (permalink / raw)
  To: rui.zhang, durgadoss.r; +Cc: linux-pm

Hello Rui and thermal folks,

Here is a couple of fixes and cleanups on the thermal_sys.c.

One important change is the netlink channel, which I am proposing
to have the zone id in each message. Rui, do you know if this could
cause a breakage on userspace applications?

All best,

Eduardo Valentin (4):
  thermal: Use thermal zone device id in netlink messages
  thermal: remove unnecessary include
  thermal: cleanup: use dev_* helper functions
  thermal sys: check for invalid trip setup when registering thermal
    device

 Documentation/thermal/sysfs-api.txt |    5 +++--
 drivers/thermal/thermal_sys.c       |   23 ++++++++++++++++-------
 include/linux/thermal.h             |    6 ++++--
 3 files changed, 23 insertions(+), 11 deletions(-)

-- 
1.7.7.1.488.ge8e1c

^ permalink raw reply

* [RFC] [PATCH 1/4] thermal: Use thermal zone device id in netlink messages
From: Eduardo Valentin @ 2012-06-12 13:25 UTC (permalink / raw)
  To: rui.zhang, durgadoss.r; +Cc: linux-pm
In-Reply-To: <1339507519-14434-1-git-send-email-eduardo.valentin@ti.com>

This patch changes the function thermal_generate_netlink_event
to receive a thermal zone device instead of a originator id.

This way, the messages will always be bound to a thermal zone.

Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
 Documentation/thermal/sysfs-api.txt |    5 +++--
 drivers/thermal/thermal_sys.c       |    8 ++++++--
 include/linux/thermal.h             |    6 ++++--
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 1733ab9..670644a 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -285,8 +285,9 @@ The framework includes a simple notification mechanism, in the form of a
 netlink event. Netlink socket initialization is done during the _init_
 of the framework. Drivers which intend to use the notification mechanism
 just need to call thermal_generate_netlink_event() with two arguments viz
-(originator, event). Typically the originator will be an integer assigned
-to a thermal_zone_device when it registers itself with the framework. The
+(originator, event). The originator is a pointer to struct thermal_zone_device
+from where the event has been originated. An integer which represents the
+thermal zone device will be used in the message to identify the zone. The
 event will be one of:{THERMAL_AUX0, THERMAL_AUX1, THERMAL_CRITICAL,
 THERMAL_DEV_FAULT}. Notification can be sent when the current temperature
 crosses any of the configured thresholds.
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 022bacb..650cefb 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1287,7 +1287,8 @@ static struct genl_multicast_group thermal_event_mcgrp = {
 	.name = THERMAL_GENL_MCAST_GROUP_NAME,
 };
 
-int thermal_generate_netlink_event(u32 orig, enum events event)
+int thermal_generate_netlink_event(struct thermal_zone_device *tz,
+					enum events event)
 {
 	struct sk_buff *skb;
 	struct nlattr *attr;
@@ -1297,6 +1298,9 @@ int thermal_generate_netlink_event(u32 orig, enum events event)
 	int result;
 	static unsigned int thermal_event_seqnum;
 
+	if (!tz)
+		return -EINVAL;
+
 	/* allocate memory */
 	size = nla_total_size(sizeof(struct thermal_genl_event)) +
 	       nla_total_size(0);
@@ -1331,7 +1335,7 @@ int thermal_generate_netlink_event(u32 orig, enum events event)
 
 	memset(thermal_event, 0, sizeof(struct thermal_genl_event));
 
-	thermal_event->orig = orig;
+	thermal_event->orig = tz->id;
 	thermal_event->event = event;
 
 	/* send multicast genetlink message */
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 796f1ff..b754203 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -152,9 +152,11 @@ struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
 
 #ifdef CONFIG_NET
-extern int thermal_generate_netlink_event(u32 orig, enum events event);
+extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
+						enum events event);
 #else
-static inline int thermal_generate_netlink_event(u32 orig, enum events event)
+static int thermal_generate_netlink_event(struct thermal_zone_device *tz,
+						enum events event)
 {
 	return 0;
 }
-- 
1.7.7.1.488.ge8e1c

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox