[3/3] Thermal:core: Handle trips focused on current trip point only.
diff mbox

Message ID 1368870663-1225-1-git-send-email-jonghwa3.lee@samsung.com
State Rejected
Delegated to: Zhang Rui
Headers show

Commit Message

Jonghwa Lee May 18, 2013, 9:51 a.m. UTC
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 <jonghwa3.lee@samsung.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 drivers/thermal/thermal_core.c |   28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

Comments

Zhang Rui May 20, 2013, 4 p.m. UTC | #1
> -----Original Message-----
> From: Jonghwa Lee [mailto:jonghwa3.lee@samsung.com]
> Sent: Saturday, May 18, 2013 5:51 PM
> To: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; 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 <jonghwa3.lee@samsung.com>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jonghwa Lee May 21, 2013, 3:40 a.m. UTC | #2
On 2013? 05? 21? 01:00, Zhang, Rui wrote:

> 
> 
>> -----Original Message-----
>> From: Jonghwa Lee [mailto:jonghwa3.lee@samsung.com]
>> Sent: Saturday, May 18, 2013 5:51 PM
>> To: linux-pm@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org; 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 <jonghwa3.lee@samsung.com>
>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> 
> 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
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang Rui May 23, 2013, 2:10 a.m. UTC | #3
On Tue, 2013-05-21 at 12:40 +0900, jonghwa3.lee@samsung.com wrote:
> On 2013? 05? 21? 01:00, Zhang, Rui wrote:
> 
> > 
> > 
> >> -----Original Message-----
> >> From: Jonghwa Lee [mailto:jonghwa3.lee@samsung.com]
> >> Sent: Saturday, May 18, 2013 5:51 PM
> >> To: linux-pm@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org; 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 <jonghwa3.lee@samsung.com>
> >> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> > 
> > 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
> > 
> > 
> 
> 


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

Patch
diff mbox

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);