mbox series

[0/4] thermal: Introduce support for monitoring falling temperatures.

Message ID 1568859503-19725-1-git-send-email-thara.gopinath@linaro.org
Headers show
Series thermal: Introduce support for monitoring falling temperatures. | expand

Message

Thara Gopinath Sept. 19, 2019, 2:18 a.m. UTC
Thermal framework today supports monitoring for rising temperatures and
subsequently initiating cooling action in case of a trip point being 
crossed. There are scenarios where a SoC needs some warming action to be
activated if the temperature falls below a cetain allowable limit.
Since warming action can be considered mirror opposite of cooling action,
most of the thermal framework can be re-used to achieve this.

To enable thermal framework to monitor falling temperature, a new parameter
is added to the thermal trip point binding in the device tree to indicate
the direction(rising/falling) of temperature monitoring. Thermal DT
driver is extended to capture this information from the device tree 
entries and to reflect it in the thermal framework as a new enum
variable in the thermal trip point structure.
As an initial attempt, step-wise governor is extended to support
bi-directional monitoring of temprature if a trip point is hit, depending
on the newly introduced enum variable. Finally thermal sysfs entries are
extended to indicate the trip point monitor direction.

Patch series introducing various resources that are used as warming devices
on Qualcomm sdm845:
https://lkml.org/lkml/2019/7/29/749 (already merged)

https://lkml.org/lkml/2019/9/10/727 (under review)


Thara Gopinath (4):
  dt-bindings: thermal: Introduce monitor-falling binding to thermal
    trip point description
  thermal: Thermal core and sysfs changes needed to support
    bi-directional monitoring of trip points.
  thermal: of-thermal: Extend thermal dt driver to support
    bi-directional monitoring of a thermal trip point.
  thermal: step_wise: Extend thermal step-wise governor to monitor
    falling temperature.

 .../devicetree/bindings/thermal/thermal.txt        |  8 +++
 drivers/thermal/of-thermal.c                       | 22 ++++++++
 drivers/thermal/step_wise.c                        | 59 +++++++++++++++------
 drivers/thermal/thermal_sysfs.c                    | 60 ++++++++++++++++++++--
 include/linux/thermal.h                            | 10 ++++
 include/uapi/linux/thermal.h                       |  2 +-
 6 files changed, 141 insertions(+), 20 deletions(-)

-- 
2.1.4

Comments

Thara Gopinath Oct. 16, 2019, 9:24 p.m. UTC | #1
On 09/18/2019 10:18 PM, Thara Gopinath wrote:
> Thermal framework today supports monitoring for rising temperatures and

> subsequently initiating cooling action in case of a trip point being 

> crossed. There are scenarios where a SoC needs some warming action to be

> activated if the temperature falls below a cetain allowable limit.

> Since warming action can be considered mirror opposite of cooling action,

> most of the thermal framework can be re-used to achieve this.

> 

> To enable thermal framework to monitor falling temperature, a new parameter

> is added to the thermal trip point binding in the device tree to indicate

> the direction(rising/falling) of temperature monitoring. Thermal DT

> driver is extended to capture this information from the device tree 

> entries and to reflect it in the thermal framework as a new enum

> variable in the thermal trip point structure.

> As an initial attempt, step-wise governor is extended to support

> bi-directional monitoring of temprature if a trip point is hit, depending

> on the newly introduced enum variable. Finally thermal sysfs entries are

> extended to indicate the trip point monitor direction.

> 

> Patch series introducing various resources that are used as warming devices

> on Qualcomm sdm845:

> https://lkml.org/lkml/2019/7/29/749 (already merged)

> 

> https://lkml.org/lkml/2019/9/10/727 (under review)


Gentle reminder for reviews!
> 

> 

> Thara Gopinath (4):

>   dt-bindings: thermal: Introduce monitor-falling binding to thermal

>     trip point description

>   thermal: Thermal core and sysfs changes needed to support

>     bi-directional monitoring of trip points.

>   thermal: of-thermal: Extend thermal dt driver to support

>     bi-directional monitoring of a thermal trip point.

>   thermal: step_wise: Extend thermal step-wise governor to monitor

>     falling temperature.

> 

>  .../devicetree/bindings/thermal/thermal.txt        |  8 +++

>  drivers/thermal/of-thermal.c                       | 22 ++++++++

>  drivers/thermal/step_wise.c                        | 59 +++++++++++++++------

>  drivers/thermal/thermal_sysfs.c                    | 60 ++++++++++++++++++++--

>  include/linux/thermal.h                            | 10 ++++

>  include/uapi/linux/thermal.h                       |  2 +-

>  6 files changed, 141 insertions(+), 20 deletions(-)

> 



-- 
Warm Regards
Thara
Ram Chandrasekar Nov. 8, 2019, 7:54 p.m. UTC | #2
On 9/18/2019 8:18 PM, Thara Gopinath wrote:
>>From the step wise governor point of view, the policy decisions

> that has to taken on a thermal trip point that is defined to be monitored

> for falling temprature is the mirror opposite of the decisions it has

> to take on a trip point that is monitored for rising temperature.

> 

> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

> ---

>   drivers/thermal/step_wise.c | 59 +++++++++++++++++++++++++++++++++------------

>   1 file changed, 44 insertions(+), 15 deletions(-)

> 

> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c

> index 6e051cb..aa8e0a0 100644

> --- a/drivers/thermal/step_wise.c

> +++ b/drivers/thermal/step_wise.c

> @@ -35,7 +35,8 @@

>    *       deactivate the thermal instance

>    */

>   static unsigned long get_target_state(struct thermal_instance *instance,

> -				enum thermal_trend trend, bool throttle)

> +				enum thermal_trend trend, bool throttle,

> +				enum thermal_trip_monitor_type type)

>   {

>   	struct thermal_cooling_device *cdev = instance->cdev;

>   	unsigned long cur_state;

> @@ -65,11 +66,21 @@ static unsigned long get_target_state(struct thermal_instance *instance,

>   

>   	switch (trend) {

>   	case THERMAL_TREND_RAISING:

> -		if (throttle) {

> -			next_target = cur_state < instance->upper ?

> -				    (cur_state + 1) : instance->upper;

> -			if (next_target < instance->lower)

> -				next_target = instance->lower;

> +		if (type == THERMAL_TRIP_MONITOR_FALLING) {

> +			if (cur_state <= instance->lower) {

> +				if (!throttle)

> +					next_target = THERMAL_NO_TARGET;

> +			} else {

> +				if (!throttle)

> +					next_target = cur_state - 1;

> +			}

> +		} else {

> +			if (throttle) {

> +				next_target = cur_state < instance->upper ?

> +					    (cur_state + 1) : instance->upper;

> +				if (next_target < instance->lower)

> +					next_target = instance->lower;

> +			}

>   		}

>   		break;

>   	case THERMAL_TREND_RAISE_FULL:

> @@ -77,14 +88,23 @@ static unsigned long get_target_state(struct thermal_instance *instance,

>   			next_target = instance->upper;

>   		break;

>   	case THERMAL_TREND_DROPPING:

> -		if (cur_state <= instance->lower) {

> -			if (!throttle)

> -				next_target = THERMAL_NO_TARGET;

> +		if (type == THERMAL_TRIP_MONITOR_FALLING) {

> +			if (throttle) {

> +				next_target = cur_state < instance->upper ?

> +					(cur_state + 1) : instance->upper;

> +				if (next_target < instance->lower)

> +					next_target = instance->lower;

> +			}

>   		} else {

> -			if (!throttle) {

> -				next_target = cur_state - 1;

> -				if (next_target > instance->upper)

> -					next_target = instance->upper;

> +			if (cur_state <= instance->lower) {

> +				if (!throttle)

> +					next_target = THERMAL_NO_TARGET;

> +			} else {

> +				if (!throttle) {

> +					next_target = cur_state - 1;

> +					if (next_target > instance->upper)

> +						next_target = instance->upper;

> +				}

>   			}

>   		}

>   		break;

> @@ -117,6 +137,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)

>   {

>   	int trip_temp;

>   	enum thermal_trip_type trip_type;

> +	enum thermal_trip_monitor_type monitor_type =

> +					THERMAL_TRIP_MONITOR_RISING;

>   	enum thermal_trend trend;

>   	struct thermal_instance *instance;

>   	bool throttle = false;

> @@ -130,9 +152,15 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)

>   		tz->ops->get_trip_type(tz, trip, &trip_type);

>   	}

>   

> +	if (tz->ops->get_trip_monitor_type)

> +		tz->ops->get_trip_monitor_type(tz, trip, &monitor_type);

> +

>   	trend = get_tz_trend(tz, trip);

>   

> -	if (tz->temperature >= trip_temp) {

> +	if (((monitor_type == THERMAL_TRIP_MONITOR_RISING) &&

> +	      (tz->temperature >= trip_temp)) ||

> +	      ((monitor_type == THERMAL_TRIP_MONITOR_FALLING) &&

> +	      (tz->temperature <= trip_temp))) {

Governors monitoring warming devices need to have support for 
hysteresis. Assume a case where the device is in idle when the 
temperature goes below threshold and we trigger a mitigation. Even a 
minimal workload or even the processing of the threshold by the governor 
could warm the device and put the temperature above the threshold and we 
will have to remove any mitigation. To avoid this ping-pong, its best to 
add a hysteresis support.
>   		throttle = true;

>   		trace_thermal_zone_trip(tz, trip, trip_type);

>   	}

> @@ -147,7 +175,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)

>   			continue;

>   

>   		old_target = instance->target;

> -		instance->target = get_target_state(instance, trend, throttle);

> +		instance->target = get_target_state(instance, trend, throttle,

> +						    monitor_type);

>   		dev_dbg(&instance->cdev->device, "old_target=%d, target=%d\n",

>   					old_target, (int)instance->target);

>   

>