Re: [RFC] the generic thermal layer enhancement

[Zhang Rui <rui.zhang@intel.com>]

On 三, 2012-05-30 at 05:05 -0600, R, Durgadoss wrote:
> Hi Eduardo,
> 
> > 
> > For G1+G2, I agree with your proposal. I had some discussion with Amit
> > regarding this. In his series of patches we increase / decrease the cooling
> > device state linearly and steadily.
> > 
> > But if we would have what you are saying, we could bind cooling device
> > set of states with trip points.
> 
> True, We want to bind the levels of cooling with the trips points a thermal zone has.
> But we might not get a 1-1 mapping always.
> 
> > 
> > I fully support this option and could cook up something on this.
> > The TC1 and TC2 should go inside the .get_trend() callbacks for ACPI.
> > Should probably go away from the registration function that we have
> > currently.
> 
> I realize I just said the same thing :-)
> 
> > 
> > We could have generic trending computation though. Based on timestamping
> > and temperature reads, and make it available for zones that want to used it.
> 
> Agree, but I would like this go into the platform thermal drivers.

But at least we need a dummy function in thermal_sys.c, as a backup.

>  And then when
> those drivers notify the framework they can specify the trend also. This sort of
> notification is not there, but that is what I am implementing these days..
> Hope to submit this patch in a week's time..
> 
You are also working on this?
here is the prototype patch I made for this.
Surely we can change the logic in thermal_get_trend() in thermal_sys.c
It would be great if you can attach your patch as well.

---
 drivers/acpi/thermal.c        |   36 ++++++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_sys.c |   24 ++++++++++++++++++------
 include/linux/thermal.h       |    9 +++++++++
 3 files changed, 63 insertions(+), 6 deletions(-)

Index: rtd3/drivers/acpi/thermal.c
===================================================================
--- rtd3.orig/drivers/acpi/thermal.c
+++ rtd3/drivers/acpi/thermal.c
@@ -706,6 +706,41 @@ 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 ACTIVE and PASSIVE trip points need TREND */
+	if (type != THERMAL_TRIP_PASSIVE && type != THERMAL_TRIP_ACTIVE)
+		return -EINVAL;
+	/*  */
+	if (type == THERMAL_TRIP_ACTIVE) {
+		*trend = THERMAL_TREND_RAISING;
+		return 0;
+	}
+
+	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)
 {
@@ -821,6 +856,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
@@ -650,6 +650,19 @@ thermal_remove_hwmon_sysfs(struct therma
 }
 #endif
 
+static void thermal_get_trend(struct thermal_zone_device *tz,
+		enum thermal_trip_type type, enum thermal_trend *trend)
+{
+	if (!tz->ops->get_trend) {
+		trend = THERMAL_TREND_DEFAULT;
+		return;
+	}
+
+	if (tz->ops->get_trend(tz, type, trend))
+		trend = THERMAL_TREND_DEFAULT;
+	return;
+}
+
 static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
 					    int delay)
 {
@@ -669,10 +682,11 @@ static void thermal_zone_device_set_poll
 static void thermal_zone_device_passive(struct thermal_zone_device *tz,
 					int temp, int trip_temp, int trip)
 {
-	int trend = 0;
+	enum thermal_trend trend = THERMAL_TREND_DEFAULT;
 	struct thermal_cooling_device_instance *instance;
 	struct thermal_cooling_device *cdev;
 	long state, max_state;
+	int ret =
 
 	/*
 	 * Above Trip?
@@ -684,11 +698,9 @@ static void thermal_zone_device_passive(
 	if (temp >= trip_temp) {
 		tz->passive = true;
 
-		trend = (tz->tc1 * (temp - tz->last_temperature)) +
-			(tz->tc2 * (temp - trip_temp));
+		thermal_get_trend(tz, trip, &trend);
 
-		/* Heating up? */
-		if (trend > 0) {
+		if (trend == THERMAL_TREND_RAISING) {
 			list_for_each_entry(instance, &tz->cooling_devices,
 					    node) {
 				if (instance->trip != trip)
@@ -699,7 +711,7 @@ static void thermal_zone_device_passive(
 				if (state++ < max_state)
 					cdev->ops->set_cur_state(cdev, state);
 			}
-		} else if (trend < 0) { /* Cooling off? */
+		} else if (trend == THERMAL_TREND_DROPPING) {
 			list_for_each_entry(instance, &tz->cooling_devices,
 					    node) {
 				if (instance->trip != trip)
Index: rtd3/include/linux/thermal.h
===================================================================
--- rtd3.orig/include/linux/thermal.h
+++ rtd3/include/linux/thermal.h
@@ -44,6 +44,13 @@ enum thermal_trip_type {
 	THERMAL_TRIP_CRITICAL,
 };
 
+enum thermal_trend {
+	THERMAL_TREND_NONE,
+	THERMAL_TREND_RAISING,
+	THERMAL_TREND_DROPPING,
+	THERMAL_TREND_DEFAULT = THERMAL_TREND_RAISING, /* aggressive cooling */
+};
+
 struct thermal_zone_device_ops {
 	int (*bind) (struct thermal_zone_device *,
 		     struct thermal_cooling_device *);
@@ -59,6 +66,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);
 };


> > > >     case THERMAL_TRIP_ACTIVE:
> > > >     case THERMAL_TRIP_PASSIVE:
> > > >          ...
> > > >          tz->ops->get_trend();
> > 
> > Would the get_trend take into account if we are cooling with active or passive
> > cooling device?
> 
> To me, it does not matter. It is up to the framework to decide and throttle,
> the respective cooling devices according to the trend.
> 
> > 
> > > >          if (trend == HEATING)
> > > >                  cdev->ops->set_cur_state(cdev, cur_state++);
> > > >          else if (trend == COOLING)
> > > >                  cdev->ops->set_cur_state(cdev, cur_state--);
> > > >          break;
> > 
> > I believe we should have something for temperature stabilization there as well.
> > 
> > Besides, if we go with this generic policy, then the zone update would be much
> > simpler no?
> 
> Yes, and that’s what we want too  :-)
> 
> > Here are some other thoughts:
> > G6. Another point is, would it make sense to allow for policy extension? Meaning,
> > the zone update would call a callback to request for update from the zone
> > device driver?
> > 
> > G7. How do we solve cooling devices being shared between different thermal
> > zones?
> > Should we have a better cooling device constraint management?
> 
> This is another thing that was haunting me for quite some time.
> And What I have in mind is a mapping kind of thing in the platform layer,
> that will provide details about which cooling device is shared with whom.
> The framework can then use this and figure out the association among various devices.
> I am testing it out, and will submit once it comes to a good shape.
> 
I think this is your answer for G11 rather than G7, no?

thanks,
rui


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