Re: [linux-pm] [RFC PATCH 1/2] thermal: Add a new trip type to usecooling device instance number

From: Amit Kachhap
Date: Wed Dec 21 2011 - 00:11:35 EST

Next message: Minchan Kim: "[PATCH] vmalloc: remove #ifdef in function body"
Previous message: Dave Chinner: "Re: [PATCH] radix_tree: take radix_tree_path off stack"
In reply to: Vincent Guittot: "Re: [linux-pm] [RFC PATCH 1/2] thermal: Add a new trip type to usecooling device instance number"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Vincent,

Thanks for the review.
Well actually your are correct that current temperature and last
temperature can be used to increase or decrease the cpu frequency. But
this has to be done again in cooling devices so to make the cooling
devices generic and to avoid the temperature comparison again this new
trip type passes the cooling device instance id.
Also about your queries that this may add dependency between trip
index and cooling state. This is actually needed and this dependency
is created when the cooling device is binded with trip points(For
cpufreq type cooling device just the instance of cooling device is
associated with trip points). More over the existing PASSIVE cooling
trip type does the same thing and iterates across all the cooling
state.

Thanks,
Amit Daniel
>
> On 20 December 2011 18:07, Vincent Guittot <vincent.guittot@xxxxxxxxxx> wrote:
>>
>> Hi Amit,
>>
>> I'm not sure that using the trip index for setting the state of a
>> cooling device is a generic solution because you are adding a
>> dependency between the trip index and the cooling device state that
>> might be difficult to handle. This dependency implies that a cooling
>> device like cpufreq_cooling_device must be registered in the 1st trips
>> of a thermal_zone which is not possible when we want to register 2
>> cpufreq_cooling_devices in the same thermal_zone.
>> You should only rely on the current and last temperatures to detect if
>> a trip_temp has been crossed and you should increase or decrease the
>> current state of the cooling device accordingly.
>>
>> something like below should work with cpufreq_cooling_device and will
>> not add any constraint on the trip index. The state of a cooling
>> device is increased/decreased once for each trip
>>
>> + case THERMAL_TRIP_STATE_ACTIVE:
>> + list_for_each_entry(instance, &tz->cooling_devices,
>> + node) {
>> + if (instance->trip != count)
>> + continue;
>> +
>> + cdev = instance->cdev;
>> +
>> + if ((temp >= trip_temp)
>> + && (trip_temp > tz->last_temperature)) {
>> + cdev->ops->get_max_state(cdev,
>> + &max_state);
>> + cdev->ops->get_cur_state(cdev,
>> + &current_state);
>> + if (++current_state <= max_state)
>> + cdev->ops->set_cur_state(cdev,
>> + current_state);
>> + }
>> + else if ((temp < trip_temp)
>> + && (trip_temp <= tz->last_temperature)) {
>> + cdev->ops->get_cur_state(cdev,
>> + &current_state);
>> + if (current_state > 0)
>> + cdev->ops->set_cur_state(cdev,
>> + --current_state);
>> + }
>> + break;
>>
>> Regards,
>> Vincent
>>
>> On 13 December 2011 16:13, Amit Daniel Kachhap <amit.kachhap@xxxxxxxxxx> wrote:
>> > This patch adds a new trip type THERMAL_TRIP_STATE_ACTIVE. This
>> > trip behaves same as THERMAL_TRIP_ACTIVE but also passes the cooling
>> > device instance number. This helps the cooling device registered as
>> > different instances to perform appropriate cooling action decision in
>> > the set_cur_state call back function.
>> >
>> > Also since the trip temperature's are in ascending order so some logic
>> > is put in place to skip the un-necessary checks.
>> >
>> > Signed-off-by: Amit Daniel Kachhap <amit.kachhap@xxxxxxxxxx>
>> > ---
>> > Documentation/thermal/sysfs-api.txt | 4 ++--
>> > drivers/thermal/thermal_sys.c | 27 ++++++++++++++++++++++++++-
>> > include/linux/thermal.h | 1 +
>> > 3 files changed, 29 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>> > index b61e46f..5c1d44e 100644
>> > --- a/Documentation/thermal/sysfs-api.txt
>> > +++ b/Documentation/thermal/sysfs-api.txt
>> > @@ -184,8 +184,8 @@ trip_point_[0-*]_temp
>> >
>> > trip_point_[0-*]_type
>> > Strings which indicate the type of the trip point.
>> > - E.g. it can be one of critical, hot, passive, active[0-*] for ACPI
>> > - thermal zone.
>> > + E.g. it can be one of critical, hot, passive, active[0-1],
>> > + state-active[0-*] for ACPI thermal zone.
>> > RO, Optional
>> >
>> > cdev[0-*]
>> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> > index dd9a574..72b1ab3 100644
>> > --- a/drivers/thermal/thermal_sys.c
>> > +++ b/drivers/thermal/thermal_sys.c
>> > @@ -192,6 +192,8 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr,
>> > return sprintf(buf, "passive\n");
>> > case THERMAL_TRIP_ACTIVE:
>> > return sprintf(buf, "active\n");
>> > + case THERMAL_TRIP_STATE_ACTIVE:
>> > + return sprintf(buf, "state-active\n");
>> > default:
>> > return sprintf(buf, "unknown\n");
>> > }
>> > @@ -1035,7 +1037,7 @@ EXPORT_SYMBOL(thermal_cooling_device_unregister);
>> > void thermal_zone_device_update(struct thermal_zone_device *tz)
>> > {
>> > int count, ret = 0;
>> > - long temp, trip_temp;
>> > + long temp, trip_temp, max_state, last_trip_change = 0;
>> > enum thermal_trip_type trip_type;
>> > struct thermal_cooling_device_instance *instance;
>> > struct thermal_cooling_device *cdev;
>> > @@ -1086,6 +1088,29 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>> > cdev->ops->set_cur_state(cdev, 0);
>> > }
>> > break;
>> > + case THERMAL_TRIP_STATE_ACTIVE:
>> > + list_for_each_entry(instance, &tz->cooling_devices,
>> > + node) {
>> > + if (instance->trip != count)
>> > + continue;
>> > +
>> > + if (temp <= last_trip_change)
>> > + continue;
>> > +
>> > + cdev = instance->cdev;
>> > + cdev->ops->get_max_state(cdev, &max_state);
>> > +
>> > + if ((temp >= trip_temp) &&
>> > + ((count + 1) <= max_state))
>> > + cdev->ops->set_cur_state(cdev,
>> > + count + 1);
>> > + else if ((temp < trip_temp) &&
>> > + (count <= max_state))
>> > + cdev->ops->set_cur_state(cdev, count);
>> > +
>> > + last_trip_change = trip_temp;
>> > + }
>> > + break;
>> > case THERMAL_TRIP_PASSIVE:
>> > if (temp >= trip_temp || tz->passive)
>> > thermal_zone_device_passive(tz, temp,
>> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> > index 47b4a27..d7d0a27 100644
>> > --- a/include/linux/thermal.h
>> > +++ b/include/linux/thermal.h
>> > @@ -42,6 +42,7 @@ enum thermal_trip_type {
>> > THERMAL_TRIP_PASSIVE,
>> > THERMAL_TRIP_HOT,
>> > THERMAL_TRIP_CRITICAL,
>> > + THERMAL_TRIP_STATE_ACTIVE,
>> > };
>> >
>> > struct thermal_zone_device_ops {
>> > --
>> > 1.7.1
>> >
>> > _______________________________________________
>> > linux-pm mailing list
>> > linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
>> > https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Minchan Kim: "[PATCH] vmalloc: remove #ifdef in function body"
Previous message: Dave Chinner: "Re: [PATCH] radix_tree: take radix_tree_path off stack"
In reply to: Vincent Guittot: "Re: [linux-pm] [RFC PATCH 1/2] thermal: Add a new trip type to usecooling device instance number"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]