2024-04-10 17:46:40

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 09/16] thermal: gov_step_wise: Use trip thresholds instead of trip temperatures

From: Rafael J. Wysocki <[email protected]>

In principle, the Step-Wise governor should take trip hystereses into
account. After all, once a trip has been crossed on the way up,
mitigation is still needed until it is crossed on the way down.

For this reason, make it use trip thresholds that are computed by
the core when trips are crossed, so as to apply mitigations in the
hysteresis rages of trips that were crossed on the way up, but have
not been crossed on the way down yet.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/thermal/gov_step_wise.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/thermal/gov_step_wise.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_step_wise.c
+++ linux-pm/drivers/thermal/gov_step_wise.c
@@ -62,7 +62,8 @@ static unsigned long get_target_state(st
}

static void thermal_zone_trip_update(struct thermal_zone_device *tz,
- const struct thermal_trip *trip)
+ const struct thermal_trip *trip,
+ int trip_threshold)
{
int trip_id = thermal_zone_trip_id(tz, trip);
enum thermal_trend trend;
@@ -72,13 +73,13 @@ static void thermal_zone_trip_update(str

trend = get_tz_trend(tz, trip);

- if (tz->temperature >= trip->temperature) {
+ if (tz->temperature >= trip_threshold) {
throttle = true;
trace_thermal_zone_trip(tz, trip_id, trip->type);
}

dev_dbg(&tz->device, "Trip%d[type=%d,temp=%d]:trend=%d,throttle=%d\n",
- trip_id, trip->type, trip->temperature, trend, throttle);
+ trip_id, trip->type, trip_threshold, trend, throttle);

list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
if (instance->trip != trip)
@@ -131,7 +132,7 @@ static void step_wise_manage(struct ther
trip->type == THERMAL_TRIP_HOT)
continue;

- thermal_zone_trip_update(tz, trip);
+ thermal_zone_trip_update(tz, trip, td->threshold);
}

list_for_each_entry(instance, &tz->thermal_instances, tz_node)





2024-04-19 19:11:58

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v1 09/16] thermal: gov_step_wise: Use trip thresholds instead of trip temperatures



On 4/10/24 17:43, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> In principle, the Step-Wise governor should take trip hystereses into

s/hystereses/hysteresis/

> account. After all, once a trip has been crossed on the way up,
> mitigation is still needed until it is crossed on the way down.
>
> For this reason, make it use trip thresholds that are computed by
> the core when trips are crossed, so as to apply mitigations in the
> hysteresis rages of trips that were crossed on the way up, but have
> not been crossed on the way down yet.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/thermal/gov_step_wise.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/thermal/gov_step_wise.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_step_wise.c
> +++ linux-pm/drivers/thermal/gov_step_wise.c
> @@ -62,7 +62,8 @@ static unsigned long get_target_state(st
> }
>
> static void thermal_zone_trip_update(struct thermal_zone_device *tz,
> - const struct thermal_trip *trip)
> + const struct thermal_trip *trip,
> + int trip_threshold)
> {
> int trip_id = thermal_zone_trip_id(tz, trip);
> enum thermal_trend trend;
> @@ -72,13 +73,13 @@ static void thermal_zone_trip_update(str
>
> trend = get_tz_trend(tz, trip);
>
> - if (tz->temperature >= trip->temperature) {
> + if (tz->temperature >= trip_threshold) {

So this value in 'trip_threshold' might be lower than older
'trip->temperature', but not necessarily. Anyway, all good here.

> throttle = true;
> trace_thermal_zone_trip(tz, trip_id, trip->type);
> }
>
> dev_dbg(&tz->device, "Trip%d[type=%d,temp=%d]:trend=%d,throttle=%d\n",
> - trip_id, trip->type, trip->temperature, trend, throttle);
> + trip_id, trip->type, trip_threshold, trend, throttle);
>
> list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> if (instance->trip != trip)
> @@ -131,7 +132,7 @@ static void step_wise_manage(struct ther
> trip->type == THERMAL_TRIP_HOT)
> continue;
>
> - thermal_zone_trip_update(tz, trip);
> + thermal_zone_trip_update(tz, trip, td->threshold);
> }
>
> list_for_each_entry(instance, &tz->thermal_instances, tz_node)
>
>
>


LGTM w/ spelling fixed.

Reviewed-by: Lukasz Luba <[email protected]>

2024-04-24 07:04:10

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v1 09/16] thermal: gov_step_wise: Use trip thresholds instead of trip temperatures

On 10/04/2024 18:43, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> In principle, the Step-Wise governor should take trip hystereses into
> account. After all, once a trip has been crossed on the way up,
> mitigation is still needed until it is crossed on the way down.
>
> For this reason, make it use trip thresholds that are computed by
> the core when trips are crossed, so as to apply mitigations in the
> hysteresis rages of trips that were crossed on the way up, but have
> not been crossed on the way down yet.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---

Acked-by: Daniel Lezcano <[email protected]>

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog