When thermal zone device is updated, it doesn't need to check
every trip points and its handling mathod even current temperature
doesn't exceed the trip's temperature. To modify those dissipatve
mechanism, this patch introduces the way to get current thermal
trip point to call only correspond trip point handling.
Signed-off-by: Jonghwa Lee <[email protected]>
Signed-off-by: MyungJoo Ham <[email protected]>
---
drivers/thermal/thermal_core.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index ce4384a..1cc4825 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -333,14 +333,6 @@ static void handle_non_critical_trips(struct thermal_zone_device *tz,
static void handle_critical_trips(struct thermal_zone_device *tz,
int trip, enum thermal_trip_type trip_type)
{
- long trip_temp;
-
- tz->ops->get_trip_temp(tz, trip, &trip_temp);
-
- /* If we have not crossed the trip_temp, we do not care. */
- if (tz->temperature < trip_temp)
- return;
-
if (tz->ops->notify)
tz->ops->notify(tz, trip, trip_type);
@@ -437,14 +429,28 @@ static void update_temperature(struct thermal_zone_device *tz)
mutex_unlock(&tz->lock);
}
+static int thermal_zone_get_current_trip(struct thermal_zone_device *tz)
+{
+ int trip;
+ long trip_temp;
+
+ for (trip = tz->trips - 1; trip > 0; trip--) {
+ tz->ops->get_trip_temp(tz, trip, &trip_temp);
+ if (tz->temperature > trip_temp)
+ continue;
+ }
+ return trip;
+}
+
void thermal_zone_device_update(struct thermal_zone_device *tz)
{
- int count;
+ int trip;
update_temperature(tz);
- for (count = 0; count < tz->trips; count++)
- handle_thermal_trip(tz, count);
+ trip = thermal_zone_get_current_trip(tz);
+
+ handle_thermal_trip(tz, trip);
}
EXPORT_SYMBOL_GPL(thermal_zone_device_update);
--
1.7.9.5
> -----Original Message-----
> From: Jonghwa Lee [mailto:[email protected]]
> Sent: Saturday, May 18, 2013 5:51 PM
> To: [email protected]
> Cc: [email protected]; Zhang, Rui; Eduardo Valentin; Amit
> Dinel Kachhap; Jonghwa Lee; MyungJoo Ham
> Subject: [PATCH 3/3] Thermal:core: Handle trips focused on current trip
> point only.
> Importance: High
>
> When thermal zone device is updated, it doesn't need to check every
> trip points and its handling mathod even current temperature doesn't
> exceed the trip's temperature. To modify those dissipatve mechanism,
> this patch introduces the way to get current thermal trip point to call
> only correspond trip point handling.
>
> Signed-off-by: Jonghwa Lee <[email protected]>
> Signed-off-by: MyungJoo Ham <[email protected]>
NAK.
> ---
> drivers/thermal/thermal_core.c | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c index ce4384a..1cc4825 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -333,14 +333,6 @@ static void handle_non_critical_trips(struct
> thermal_zone_device *tz, static void handle_critical_trips(struct
> thermal_zone_device *tz,
> int trip, enum thermal_trip_type trip_type) {
> - long trip_temp;
> -
> - tz->ops->get_trip_temp(tz, trip, &trip_temp);
> -
> - /* If we have not crossed the trip_temp, we do not care. */
> - if (tz->temperature < trip_temp)
> - return;
> -
> if (tz->ops->notify)
> tz->ops->notify(tz, trip, trip_type);
>
> @@ -437,14 +429,28 @@ static void update_temperature(struct
> thermal_zone_device *tz)
> mutex_unlock(&tz->lock);
> }
>
> +static int thermal_zone_get_current_trip(struct thermal_zone_device
> +*tz) {
> + int trip;
> + long trip_temp;
> +
> + for (trip = tz->trips - 1; trip > 0; trip--) {
> + tz->ops->get_trip_temp(tz, trip, &trip_temp);
> + if (tz->temperature > trip_temp)
> + continue;
> + }
> + return trip;
> +}
> +
> void thermal_zone_device_update(struct thermal_zone_device *tz) {
> - int count;
> + int trip;
>
> update_temperature(tz);
>
> - for (count = 0; count < tz->trips; count++)
> - handle_thermal_trip(tz, count);
> + trip = thermal_zone_get_current_trip(tz);
> +
> + handle_thermal_trip(tz, trip);
Say, trip point 1 for thermal zone 0 is 60C,
The system is running above 60C for somethime,
thus the thermal_instance for this trip point is running at upper_limit.
When the temperature suddenly drops below 60C,
we still need to handle trip point 1 to deactivate it.
Thanks,
rui
> }
> EXPORT_SYMBOL_GPL(thermal_zone_device_update);
>
> --
> 1.7.9.5
On 2013년 05월 21일 01:00, Zhang, Rui wrote:
>
>
>> -----Original Message-----
>> From: Jonghwa Lee [mailto:[email protected]]
>> Sent: Saturday, May 18, 2013 5:51 PM
>> To: [email protected]
>> Cc: [email protected]; Zhang, Rui; Eduardo Valentin; Amit
>> Dinel Kachhap; Jonghwa Lee; MyungJoo Ham
>> Subject: [PATCH 3/3] Thermal:core: Handle trips focused on current trip
>> point only.
>> Importance: High
>>
>> When thermal zone device is updated, it doesn't need to check every
>> trip points and its handling mathod even current temperature doesn't
>> exceed the trip's temperature. To modify those dissipatve mechanism,
>> this patch introduces the way to get current thermal trip point to call
>> only correspond trip point handling.
>>
>> Signed-off-by: Jonghwa Lee <[email protected]>
>> Signed-off-by: MyungJoo Ham <[email protected]>
>
> NAK.
>
>> ---
>> drivers/thermal/thermal_core.c | 28 +++++++++++++++++-----------
>> 1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/thermal/thermal_core.c
>> b/drivers/thermal/thermal_core.c index ce4384a..1cc4825 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -333,14 +333,6 @@ static void handle_non_critical_trips(struct
>> thermal_zone_device *tz, static void handle_critical_trips(struct
>> thermal_zone_device *tz,
>> int trip, enum thermal_trip_type trip_type) {
>> - long trip_temp;
>> -
>> - tz->ops->get_trip_temp(tz, trip, &trip_temp);
>> -
>> - /* If we have not crossed the trip_temp, we do not care. */
>> - if (tz->temperature < trip_temp)
>> - return;
>> -
>> if (tz->ops->notify)
>> tz->ops->notify(tz, trip, trip_type);
>>
>> @@ -437,14 +429,28 @@ static void update_temperature(struct
>> thermal_zone_device *tz)
>> mutex_unlock(&tz->lock);
>> }
>>
>> +static int thermal_zone_get_current_trip(struct thermal_zone_device
>> +*tz) {
>> + int trip;
>> + long trip_temp;
>> +
>> + for (trip = tz->trips - 1; trip > 0; trip--) {
>> + tz->ops->get_trip_temp(tz, trip, &trip_temp);
>> + if (tz->temperature > trip_temp)
>> + continue;
>> + }
>> + return trip;
>> +}
>> +
>> void thermal_zone_device_update(struct thermal_zone_device *tz) {
>> - int count;
>> + int trip;
>>
>> update_temperature(tz);
>>
>> - for (count = 0; count < tz->trips; count++)
>> - handle_thermal_trip(tz, count);
>> + trip = thermal_zone_get_current_trip(tz);
>> +
>> + handle_thermal_trip(tz, trip);
>
> Say, trip point 1 for thermal zone 0 is 60C,
> The system is running above 60C for somethime,
> thus the thermal_instance for this trip point is running at upper_limit.
> When the temperature suddenly drops below 60C,
> we still need to handle trip point 1 to deactivate it.
>
Okay, I understood. I missed the point that governor will handle a cooling
device within certain trip point described in thermal instance.
But still I don't think this is the best behaviour. Let say we were in trip
level 2nd and moving to trip level 1st then we should call governor twice for
applying trip 1 level. Why don't we just call once? And whenever we call
handle_thermal_trip() with all trips, monitor_thermal_work() will also be called
at the same time. I think we can make this work more clearly and intuitively.
let me think of it more,,,
Thanks,
Jonghwa.
> Thanks,
> rui
>> }
>> EXPORT_SYMBOL_GPL(thermal_zone_device_update);
>>
>> --
>> 1.7.9.5
>
>
On Tue, 2013-05-21 at 12:40 +0900, [email protected] wrote:
> On 2013년 05월 21일 01:00, Zhang, Rui wrote:
>
> >
> >
> >> -----Original Message-----
> >> From: Jonghwa Lee [mailto:[email protected]]
> >> Sent: Saturday, May 18, 2013 5:51 PM
> >> To: [email protected]
> >> Cc: [email protected]; Zhang, Rui; Eduardo Valentin; Amit
> >> Dinel Kachhap; Jonghwa Lee; MyungJoo Ham
> >> Subject: [PATCH 3/3] Thermal:core: Handle trips focused on current trip
> >> point only.
> >> Importance: High
> >>
> >> When thermal zone device is updated, it doesn't need to check every
> >> trip points and its handling mathod even current temperature doesn't
> >> exceed the trip's temperature. To modify those dissipatve mechanism,
> >> this patch introduces the way to get current thermal trip point to call
> >> only correspond trip point handling.
> >>
> >> Signed-off-by: Jonghwa Lee <[email protected]>
> >> Signed-off-by: MyungJoo Ham <[email protected]>
> >
> > NAK.
> >
> >> ---
> >> drivers/thermal/thermal_core.c | 28 +++++++++++++++++-----------
> >> 1 file changed, 17 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/thermal/thermal_core.c
> >> b/drivers/thermal/thermal_core.c index ce4384a..1cc4825 100644
> >> --- a/drivers/thermal/thermal_core.c
> >> +++ b/drivers/thermal/thermal_core.c
> >> @@ -333,14 +333,6 @@ static void handle_non_critical_trips(struct
> >> thermal_zone_device *tz, static void handle_critical_trips(struct
> >> thermal_zone_device *tz,
> >> int trip, enum thermal_trip_type trip_type) {
> >> - long trip_temp;
> >> -
> >> - tz->ops->get_trip_temp(tz, trip, &trip_temp);
> >> -
> >> - /* If we have not crossed the trip_temp, we do not care. */
> >> - if (tz->temperature < trip_temp)
> >> - return;
> >> -
> >> if (tz->ops->notify)
> >> tz->ops->notify(tz, trip, trip_type);
> >>
> >> @@ -437,14 +429,28 @@ static void update_temperature(struct
> >> thermal_zone_device *tz)
> >> mutex_unlock(&tz->lock);
> >> }
> >>
> >> +static int thermal_zone_get_current_trip(struct thermal_zone_device
> >> +*tz) {
> >> + int trip;
> >> + long trip_temp;
> >> +
> >> + for (trip = tz->trips - 1; trip > 0; trip--) {
> >> + tz->ops->get_trip_temp(tz, trip, &trip_temp);
> >> + if (tz->temperature > trip_temp)
> >> + continue;
> >> + }
> >> + return trip;
> >> +}
> >> +
> >> void thermal_zone_device_update(struct thermal_zone_device *tz) {
> >> - int count;
> >> + int trip;
> >>
> >> update_temperature(tz);
> >>
> >> - for (count = 0; count < tz->trips; count++)
> >> - handle_thermal_trip(tz, count);
> >> + trip = thermal_zone_get_current_trip(tz);
> >> +
> >> + handle_thermal_trip(tz, trip);
> >
> > Say, trip point 1 for thermal zone 0 is 60C,
> > The system is running above 60C for somethime,
> > thus the thermal_instance for this trip point is running at upper_limit.
> > When the temperature suddenly drops below 60C,
> > we still need to handle trip point 1 to deactivate it.
> >
>
>
> Okay, I understood. I missed the point that governor will handle a cooling
> device within certain trip point described in thermal instance.
> But still I don't think this is the best behaviour. Let say we were in trip
> level 2nd and moving to trip level 1st then we should call governor twice for
> applying trip 1 level.
Right.
IMO, the governor is used to get the next proper cooling state for each
referred thermal instance.
So I think it is okay to call it twice.
> Why don't we just call once? And whenever we call
> handle_thermal_trip() with all trips, monitor_thermal_work() will also be called
> at the same time.
hmm, what is your question about this?
> I think we can make this work more clearly and intuitively.
> let me think of it more,,,
>
sure.
thanks,
rui
> Thanks,
> Jonghwa.
>
> > Thanks,
> > rui
> >> }
> >> EXPORT_SYMBOL_GPL(thermal_zone_device_update);
> >>
> >> --
> >> 1.7.9.5
> >
> >
>
>