Hi All,
This is a v2 of
https://lore.kernel.org/linux-pm/13365827.uLZWGnKmhe@kreacher/
which only slightly updates 2 patches and adds some tags, so the
cover letter below is still applicable:
While working on https://lore.kernel.org/linux-pm/4846448.GXAFRqVoOG@kreacher/
I started to change thermal governors so as to reduce the usage of trip
indices in them. At that time, I was concerned that they could not be
replaced with trip pointers overall, because they were needed in certain
situations (tracing, debug messages, userspace governor) and computing them
whenever they were needed would be extra overhead with relatively weak
justification. In the meantime, however, I realized that for a given trip
pointer, it is actually trivial to compute the corresponding index: it is
sufficient to subtract the start of the trips[] table in the thermal zone
containing the trip from that trip pointer for this purpose. Patch [1/6]
modifies thermal_zone_trip_id() in accordance with this observation.
Now that the cost of computing a trip index for a given trip pointer and
thermal zone is not a concern any more, the governors can be generally
switched over to using trip pointers for representing trips. One of the
things they need to do sometimes, though, is to iterate over trips in a
given thermal zone (the core needs to do that too, of course) and
for_each_thermal_trip() is somewhat inconvenient for this purpose, because
it requires callback functions to be defined and in some cases new data
types need to be introduced just in order to use it. For this reason,
patch [2/6] adds a macro for iterating over trip points in a given thermal
zone with the help of a trip pointer and changes for_each_thermal_trip() to
use that macro internally.
Patches [3-5/6] change individual governors to prepare them for using trip
pointers everywhere for representing trips, except for the trip argument of
the .throttle() callback and patch [6/6] finally changes the .throttle()
callback definition so that it takes a trip pointer as the argument
representing the trip.
Please refer to the individual patch changelogs for details.
Thanks!
From: Rafael J. Wysocki <[email protected]>
A trip index can be computed right away as a difference between the
value of a trip pointer pointing to the given trip object and the
start of the trips[] table in the given thermal zone, so change
thermal_zone_trip_id() accordingly.
No intentional functional impact (except for some speedup).
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v1 -> v2: Rebase on top of current linux-next
---
drivers/thermal/thermal_trip.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/drivers/thermal/thermal_trip.c
@@ -173,12 +173,9 @@ int thermal_zone_set_trip(struct thermal
int thermal_zone_trip_id(struct thermal_zone_device *tz,
const struct thermal_trip *trip)
{
- int i;
-
- for (i = 0; i < tz->num_trips; i++) {
- if (&tz->trips[i] == trip)
- return i;
- }
-
- return -ENODATA;
+ /*
+ * Assume the trip to be located within the bounds of the thermal
+ * zone's trips[] table.
+ */
+ return trip - tz->trips;
}
From: Rafael J. Wysocki <[email protected]>
Make get_trip_level() use for_each_trip() to iterate over trip points
and make it call thermal_zone_trip_id() to obtain the integer ID of a
given trip point so as to avoid relying on the knowledge of struct
thermal_zone_device internals.
The general functionality is not expected to be changed.
This change causes the governor to use trip pointers instead of trip
indices everywhere except for the fair_share_throttle() second argument
that will be modified subsequently along with the definition of the
governor .throttle() callback.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v1 -> v2: Eliminate redundant NULL check from the loop in
get_trip_level() (Daniel).
---
drivers/thermal/gov_fair_share.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 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
@@ -15,29 +15,27 @@
#include "thermal_core.h"
-/**
- * get_trip_level: - obtains the current trip level for a zone
- * @tz: thermal zone device
- */
static int get_trip_level(struct thermal_zone_device *tz)
{
- struct thermal_trip trip;
- int count;
+ const struct thermal_trip *trip, *level_trip = NULL;
+ int trip_level;
- for (count = 0; count < tz->num_trips; count++) {
- __thermal_zone_get_trip(tz, count, &trip);
- if (tz->temperature < trip.temperature)
+ for_each_trip(tz, trip) {
+ if (trip->temperature >= tz->temperature)
break;
+
+ level_trip = trip;
}
- /*
- * count > 0 only if temperature is greater than first trip
- * point, in which case, trip_point = count - 1
- */
- if (count > 0)
- trace_thermal_zone_trip(tz, count - 1, trip.type);
+ /* Bail out if the temperature is not greater than any trips. */
+ if (!level_trip)
+ return 0;
+
+ trip_level = thermal_zone_trip_id(tz, level_trip);
+
+ trace_thermal_zone_trip(tz, trip_level, level_trip->type);
- return count;
+ return trip_level;
}
static long get_target_state(struct thermal_zone_device *tz,
On 12/10/2023 20:25, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> A trip index can be computed right away as a difference between the
> value of a trip pointer pointing to the given trip object and the
> start of the trips[] table in the given thermal zone, so change
> thermal_zone_trip_id() accordingly.
>
> No intentional functional impact (except for some speedup).
>
> 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
On 12/10/2023 20:29, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Make get_trip_level() use for_each_trip() to iterate over trip points
> and make it call thermal_zone_trip_id() to obtain the integer ID of a
> given trip point so as to avoid relying on the knowledge of struct
> thermal_zone_device internals.
>
> The general functionality is not expected to be changed.
>
> This change causes the governor to use trip pointers instead of trip
> indices everywhere except for the fair_share_throttle() second argument
> that will be modified subsequently along with the definition of the
> governor .throttle() callback.
>
> 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