[PATCH v2 3/3] thermal: gov_step_wise: Allow cooling level to be reduced earlier

Posted by Rafael J. Wysocki 1 month, 1 week ago

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The current behavior of the Step-wise thermal governor is to increase
the cooling level one step at a time after trip point threshold passing
by thermal zone temperature until the temperature stops to rise and then
do nothing until it falls down below the (possibly new) trip point
threshold, at which point the cooling level is reduced straight to the
applicable minimum.

While this generally works, it is not in agreement with the throttling
logic description comment in step_wise_manage() any more after some
relatively recent changes, and in the case of passive cooling, it may
lead to undesirable performance oscillations between high and low
levels.

For this reason, modify the governor's cooling device state selection
function, get_target_state(), to reduce cooling by one level even if
the temperature is still above the thermal zone threshold, but the
temperature has started to fall down.  However, ensure that the cooling
level will remain above the applicable minimum in that case to pull
the zone temperature further down, possibly until it falls below the
trip threshold (which may now be equal to the low temperature of the
trip).

Doing so should help higher performance to be restored earlier in some
cases which is desirable especially for passive trip points with
relatively high hysteresis values.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/gov_step_wise.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

--- a/drivers/thermal/gov_step_wise.c
+++ b/drivers/thermal/gov_step_wise.c
@@ -20,7 +20,9 @@
  * If the temperature is higher than a trip point,
  *    a. if the trend is THERMAL_TREND_RAISING, use higher cooling
  *       state for this trip point
- *    b. if the trend is THERMAL_TREND_DROPPING, do nothing
+ *    b. if the trend is THERMAL_TREND_DROPPING, use a lower cooling state
+ *       for this trip point, but keep the cooling state above the applicable
+ *       minimum
  * If the temperature is lower than a trip point,
  *    a. if the trend is THERMAL_TREND_RAISING, do nothing
  *    b. if the trend is THERMAL_TREND_DROPPING, use the minimum applicable
@@ -51,6 +53,17 @@
 	if (throttle) {
 		if (trend == THERMAL_TREND_RAISING)
 			return clamp(cur_state + 1, instance->lower, instance->upper);
+
+		/*
+		 * If the zone temperature is falling, the cooling level can
+		 * be reduced, but it should still be above the lower state of
+		 * the given thermal instance to pull the temperature further
+		 * down.
+		 */
+		if (trend == THERMAL_TREND_DROPPING)
+			return clamp(cur_state - 1,
+				     min(instance->lower + 1, instance->upper),
+				     instance->upper);
 	} else if (trend == THERMAL_TREND_DROPPING) {
 		if (cur_state <= instance->lower)
 			return THERMAL_NO_TARGET;

Re: [PATCH v2 3/3] thermal: gov_step_wise: Allow cooling level to be reduced earlier

Posted by Lukasz Luba 4 weeks, 1 day ago


On 8/25/25 14:31, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The current behavior of the Step-wise thermal governor is to increase
> the cooling level one step at a time after trip point threshold passing
> by thermal zone temperature until the temperature stops to rise and then
> do nothing until it falls down below the (possibly new) trip point
> threshold, at which point the cooling level is reduced straight to the
> applicable minimum.

Quite long single sentence to describe these stuff...

> 
> While this generally works, it is not in agreement with the throttling
> logic description comment in step_wise_manage() any more after some
> relatively recent changes, and in the case of passive cooling, it may
> lead to undesirable performance oscillations between high and low
> levels.
> 
> For this reason, modify the governor's cooling device state selection
> function, get_target_state(), to reduce cooling by one level even if
> the temperature is still above the thermal zone threshold, but the
> temperature has started to fall down.  However, ensure that the cooling
> level will remain above the applicable minimum in that case to pull
> the zone temperature further down, possibly until it falls below the
> trip threshold (which may now be equal to the low temperature of the
> trip).
> 
> Doing so should help higher performance to be restored earlier in some
> cases which is desirable especially for passive trip points with
> relatively high hysteresis values.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/thermal/gov_step_wise.c |   15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> --- a/drivers/thermal/gov_step_wise.c
> +++ b/drivers/thermal/gov_step_wise.c
> @@ -20,7 +20,9 @@
>    * If the temperature is higher than a trip point,
>    *    a. if the trend is THERMAL_TREND_RAISING, use higher cooling
>    *       state for this trip point
> - *    b. if the trend is THERMAL_TREND_DROPPING, do nothing
> + *    b. if the trend is THERMAL_TREND_DROPPING, use a lower cooling state
> + *       for this trip point, but keep the cooling state above the applicable
> + *       minimum
>    * If the temperature is lower than a trip point,
>    *    a. if the trend is THERMAL_TREND_RAISING, do nothing
>    *    b. if the trend is THERMAL_TREND_DROPPING, use the minimum applicable
> @@ -51,6 +53,17 @@
>   	if (throttle) {
>   		if (trend == THERMAL_TREND_RAISING)
>   			return clamp(cur_state + 1, instance->lower, instance->upper);
> +
> +		/*
> +		 * If the zone temperature is falling, the cooling level can
> +		 * be reduced, but it should still be above the lower state of
> +		 * the given thermal instance to pull the temperature further
> +		 * down.
> +		 */
> +		if (trend == THERMAL_TREND_DROPPING)
> +			return clamp(cur_state - 1,
> +				     min(instance->lower + 1, instance->upper),
> +				     instance->upper);
>   	} else if (trend == THERMAL_TREND_DROPPING) {
>   		if (cur_state <= instance->lower)
>   			return THERMAL_NO_TARGET;
> 
> 
> 

That make sense

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH v2 3/3] thermal: gov_step_wise: Allow cooling level to be reduced earlier

Posted by Rafael J. Wysocki 4 weeks, 1 day ago

On Thu, Sep 4, 2025 at 9:32 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
>
> On 8/25/25 14:31, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > The current behavior of the Step-wise thermal governor is to increase
> > the cooling level one step at a time after trip point threshold passing
> > by thermal zone temperature until the temperature stops to rise and then
> > do nothing until it falls down below the (possibly new) trip point
> > threshold, at which point the cooling level is reduced straight to the
> > applicable minimum.
>
> Quite long single sentence to describe these stuff...

Yes, it is long.  I'll try to rearrange it when applying.

> >
> > While this generally works, it is not in agreement with the throttling
> > logic description comment in step_wise_manage() any more after some
> > relatively recent changes, and in the case of passive cooling, it may
> > lead to undesirable performance oscillations between high and low
> > levels.
> >
> > For this reason, modify the governor's cooling device state selection
> > function, get_target_state(), to reduce cooling by one level even if
> > the temperature is still above the thermal zone threshold, but the
> > temperature has started to fall down.  However, ensure that the cooling
> > level will remain above the applicable minimum in that case to pull
> > the zone temperature further down, possibly until it falls below the
> > trip threshold (which may now be equal to the low temperature of the
> > trip).
> >
> > Doing so should help higher performance to be restored earlier in some
> > cases which is desirable especially for passive trip points with
> > relatively high hysteresis values.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >   drivers/thermal/gov_step_wise.c |   15 ++++++++++++++-
> >   1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > --- a/drivers/thermal/gov_step_wise.c
> > +++ b/drivers/thermal/gov_step_wise.c
> > @@ -20,7 +20,9 @@
> >    * If the temperature is higher than a trip point,
> >    *    a. if the trend is THERMAL_TREND_RAISING, use higher cooling
> >    *       state for this trip point
> > - *    b. if the trend is THERMAL_TREND_DROPPING, do nothing
> > + *    b. if the trend is THERMAL_TREND_DROPPING, use a lower cooling state
> > + *       for this trip point, but keep the cooling state above the applicable
> > + *       minimum
> >    * If the temperature is lower than a trip point,
> >    *    a. if the trend is THERMAL_TREND_RAISING, do nothing
> >    *    b. if the trend is THERMAL_TREND_DROPPING, use the minimum applicable
> > @@ -51,6 +53,17 @@
> >       if (throttle) {
> >               if (trend == THERMAL_TREND_RAISING)
> >                       return clamp(cur_state + 1, instance->lower, instance->upper);
> > +
> > +             /*
> > +              * If the zone temperature is falling, the cooling level can
> > +              * be reduced, but it should still be above the lower state of
> > +              * the given thermal instance to pull the temperature further
> > +              * down.
> > +              */
> > +             if (trend == THERMAL_TREND_DROPPING)
> > +                     return clamp(cur_state - 1,
> > +                                  min(instance->lower + 1, instance->upper),
> > +                                  instance->upper);
> >       } else if (trend == THERMAL_TREND_DROPPING) {
> >               if (cur_state <= instance->lower)
> >                       return THERMAL_NO_TARGET;
> >
> >
> >
>
> That make sense
>
> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>