2024-04-10 17:48:05

by Rafael J. Wysocki

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

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

In principle, the Fair Share 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 if the
zone temperature is in a hysteresis rage of one or more 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_fair_share.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

Index: linux-pm/drivers/thermal/gov_fair_share.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_fair_share.c
+++ linux-pm/drivers/thermal/gov_fair_share.c
@@ -17,28 +17,26 @@

static int get_trip_level(struct thermal_zone_device *tz)
{
- const struct thermal_trip *level_trip = NULL;
+ const struct thermal_trip_desc *level_td = NULL;
const struct thermal_trip_desc *td;
int trip_level = -1;

for_each_trip_desc(tz, td) {
- const struct thermal_trip *trip = &td->trip;
-
- if (trip->temperature >= tz->temperature)
+ if (td->threshold > tz->temperature)
continue;

trip_level++;

- if (!level_trip || trip->temperature > level_trip->temperature)
- level_trip = trip;
+ if (!level_td || td->threshold > level_td->threshold)
+ level_td = td;
}

/* Bail out if the temperature is not greater than any trips. */
if (trip_level < 0)
return 0;

- trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, level_trip),
- level_trip->type);
+ trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, &level_td->trip),
+ level_td->trip.type);

return trip_level;
}





2024-04-19 19:18:14

by Lukasz Luba

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



On 4/10/24 17:58, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> In principle, the Fair Share governor should take trip hystereses

s/hystereses/hysteresis/

> 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 if the
> zone temperature is in a hysteresis rage of one or more 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_fair_share.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/thermal/gov_fair_share.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_fair_share.c
> +++ linux-pm/drivers/thermal/gov_fair_share.c
> @@ -17,28 +17,26 @@
>
> static int get_trip_level(struct thermal_zone_device *tz)
> {
> - const struct thermal_trip *level_trip = NULL;
> + const struct thermal_trip_desc *level_td = NULL;
> const struct thermal_trip_desc *td;
> int trip_level = -1;
>
> for_each_trip_desc(tz, td) {
> - const struct thermal_trip *trip = &td->trip;
> -
> - if (trip->temperature >= tz->temperature)
> + if (td->threshold > tz->temperature)
> continue;
>
> trip_level++;
>
> - if (!level_trip || trip->temperature > level_trip->temperature)
> - level_trip = trip;
> + if (!level_td || td->threshold > level_td->threshold)
> + level_td = td;
> }
>
> /* Bail out if the temperature is not greater than any trips. */
> if (trip_level < 0)
> return 0;
>
> - trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, level_trip),
> - level_trip->type);
> + trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, &level_td->trip),
> + level_td->trip.type);
>
> return trip_level;
> }
>
>
>


LGTM w/ spelling fixed.

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