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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
