From 78869767f2ad3a98d2c830b718a503d8b0a94b26 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 12 Oct 2023 20:25:06 +0200 Subject: thermal: trip: Simplify computing trip indices 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 Acked-by: Daniel Lezcano Reviewed-by: Lukasz Luba Tested-by: Lukasz Luba --- drivers/thermal/thermal_trip.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'drivers/thermal') diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 80f3cdb2f30e..6dff0dae6ef6 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -173,12 +173,9 @@ int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id, 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; } -- cgit v1.2.3 From 234ed6f5fbedf7bc84f9adc08c3e11ca8588ffd8 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 12 Oct 2023 20:26:46 +0200 Subject: thermal: trip: Define for_each_trip() macro Define a new macro for_each_trip() to be used by the thermal core code and thermal governors for walking trips in a given thermal zone. Modify for_each_thermal_trip() to use this macro instead of an open- coded loop over trips. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano Reviewed-by: Lukasz Luba --- drivers/thermal/thermal_core.h | 3 +++ drivers/thermal/thermal_trip.c | 7 ++++--- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'drivers/thermal') diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 024e82ebf592..4702d3a152f9 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -116,6 +116,9 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event); /* Helpers */ +#define for_each_trip(__tz, __trip) \ + for (__trip = __tz->trips; __trip - __tz->trips < __tz->num_trips; __trip++) + void __thermal_zone_set_trips(struct thermal_zone_device *tz); int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, struct thermal_trip *trip); diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 6dff0dae6ef6..9e07535d99bb 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -13,10 +13,11 @@ int for_each_thermal_trip(struct thermal_zone_device *tz, int (*cb)(struct thermal_trip *, void *), void *data) { - int i, ret; + struct thermal_trip *trip; + int ret; - for (i = 0; i < tz->num_trips; i++) { - ret = cb(&tz->trips[i], data); + for_each_trip(tz, trip) { + ret = cb(trip, data); if (ret) return ret; } -- cgit v1.2.3 From 276f1ede95164b54f55c82d40e9df218109704d6 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 12 Oct 2023 20:29:43 +0200 Subject: thermal: gov_fair_share: Rearrange get_trip_level() 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 Acked-by: Daniel Lezcano --- drivers/thermal/gov_fair_share.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'drivers/thermal') diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c index 2abeb8979f50..41effa87d3cd 100644 --- a/drivers/thermal/gov_fair_share.c +++ b/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, -- cgit v1.2.3 From 94be1d27aa8d6f0a9e09517445abf468de24fab3 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 12 Oct 2023 20:31:38 +0200 Subject: thermal: gov_power_allocator: Use trip pointers instead of trip indices Modify the power allocator thermal governor to use trip pointers instead of trip indices everywhere except for the power_allocator_throttle() second argument that will be changed subsequently along with the definition of the .throttle() governor callback. The general functionality is not expected to be changed. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano Reviewed-by: Lukasz Luba Tested-by: Lukasz Luba --- drivers/thermal/gov_power_allocator.c | 127 +++++++++++++--------------------- 1 file changed, 49 insertions(+), 78 deletions(-) (limited to 'drivers/thermal') diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 1faf55446ba2..c0d29d286374 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -16,8 +16,6 @@ #include "thermal_core.h" -#define INVALID_TRIP -1 - #define FRAC_BITS 10 #define int_to_frac(x) ((x) << FRAC_BITS) #define frac_to_int(x) ((x) >> FRAC_BITS) @@ -55,23 +53,23 @@ static inline s64 div_frac(s64 x, s64 y) * @err_integral: accumulated error in the PID controller. * @prev_err: error in the previous iteration of the PID controller. * Used to calculate the derivative term. + * @sustainable_power: Sustainable power (heat) that this thermal zone can + * dissipate * @trip_switch_on: first passive trip point of the thermal zone. The * governor switches on when this trip point is crossed. * If the thermal zone only has one passive trip point, - * @trip_switch_on should be INVALID_TRIP. + * @trip_switch_on should be NULL. * @trip_max_desired_temperature: last passive trip point of the thermal * zone. The temperature we are * controlling for. - * @sustainable_power: Sustainable power (heat) that this thermal zone can - * dissipate */ struct power_allocator_params { bool allocated_tzp; s64 err_integral; s32 prev_err; - int trip_switch_on; - int trip_max_desired_temperature; u32 sustainable_power; + const struct thermal_trip *trip_switch_on; + const struct thermal_trip *trip_max_desired_temperature; }; /** @@ -90,14 +88,12 @@ static u32 estimate_sustainable_power(struct thermal_zone_device *tz) u32 sustainable_power = 0; struct thermal_instance *instance; struct power_allocator_params *params = tz->governor_data; - const struct thermal_trip *trip_max_desired_temperature = - &tz->trips[params->trip_max_desired_temperature]; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { struct thermal_cooling_device *cdev = instance->cdev; u32 min_power; - if (instance->trip != trip_max_desired_temperature) + if (instance->trip != params->trip_max_desired_temperature) continue; if (!cdev_is_power_actor(cdev)) @@ -116,24 +112,22 @@ static u32 estimate_sustainable_power(struct thermal_zone_device *tz) * estimate_pid_constants() - Estimate the constants for the PID controller * @tz: thermal zone for which to estimate the constants * @sustainable_power: sustainable power for the thermal zone - * @trip_switch_on: trip point number for the switch on temperature + * @trip_switch_on: trip point for the switch on temperature * @control_temp: target temperature for the power allocator governor * * This function is used to update the estimation of the PID * controller constants in struct thermal_zone_parameters. */ static void estimate_pid_constants(struct thermal_zone_device *tz, - u32 sustainable_power, int trip_switch_on, + u32 sustainable_power, + const struct thermal_trip *trip_switch_on, int control_temp) { - struct thermal_trip trip; u32 temperature_threshold = control_temp; - int ret; s32 k_i; - ret = __thermal_zone_get_trip(tz, trip_switch_on, &trip); - if (!ret) - temperature_threshold -= trip.temperature; + if (trip_switch_on) + temperature_threshold -= trip_switch_on->temperature; /* * estimate_pid_constants() tries to find appropriate default @@ -386,7 +380,7 @@ static int allocate_power(struct thermal_zone_device *tz, struct thermal_instance *instance; struct power_allocator_params *params = tz->governor_data; const struct thermal_trip *trip_max_desired_temperature = - &tz->trips[params->trip_max_desired_temperature]; + params->trip_max_desired_temperature; u32 *req_power, *max_power, *granted_power, *extra_actor_power; u32 *weighted_req_power; u32 total_req_power, max_allocatable_power, total_weighted_req_power; @@ -496,7 +490,7 @@ static int allocate_power(struct thermal_zone_device *tz, } /** - * get_governor_trips() - get the number of the two trip points that are key for this governor + * get_governor_trips() - get the two trip points that are key for this governor * @tz: thermal zone to operate on * @params: pointer to private data for this governor * @@ -513,46 +507,36 @@ static int allocate_power(struct thermal_zone_device *tz, static void get_governor_trips(struct thermal_zone_device *tz, struct power_allocator_params *params) { - int i, last_active, last_passive; - bool found_first_passive; - - found_first_passive = false; - last_active = INVALID_TRIP; - last_passive = INVALID_TRIP; - - for (i = 0; i < tz->num_trips; i++) { - struct thermal_trip trip; - int ret; - - ret = __thermal_zone_get_trip(tz, i, &trip); - if (ret) { - dev_warn(&tz->device, - "Failed to get trip point %d type: %d\n", i, - ret); - continue; - } - - if (trip.type == THERMAL_TRIP_PASSIVE) { - if (!found_first_passive) { - params->trip_switch_on = i; - found_first_passive = true; - } else { - last_passive = i; + const struct thermal_trip *first_passive = NULL; + const struct thermal_trip *last_passive = NULL; + const struct thermal_trip *last_active = NULL; + const struct thermal_trip *trip; + + for_each_trip(tz, trip) { + switch (trip->type) { + case THERMAL_TRIP_PASSIVE: + if (!first_passive) { + first_passive = trip; + break; } - } else if (trip.type == THERMAL_TRIP_ACTIVE) { - last_active = i; - } else { + last_passive = trip; + break; + case THERMAL_TRIP_ACTIVE: + last_active = trip; + break; + default: break; } } - if (last_passive != INVALID_TRIP) { + if (last_passive) { + params->trip_switch_on = first_passive; params->trip_max_desired_temperature = last_passive; - } else if (found_first_passive) { - params->trip_max_desired_temperature = params->trip_switch_on; - params->trip_switch_on = INVALID_TRIP; + } else if (first_passive) { + params->trip_switch_on = NULL; + params->trip_max_desired_temperature = first_passive; } else { - params->trip_switch_on = INVALID_TRIP; + params->trip_switch_on = NULL; params->trip_max_desired_temperature = last_active; } } @@ -567,14 +551,12 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update) { struct thermal_instance *instance; struct power_allocator_params *params = tz->governor_data; - const struct thermal_trip *trip_max_desired_temperature = - &tz->trips[params->trip_max_desired_temperature]; u32 req_power; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { struct thermal_cooling_device *cdev = instance->cdev; - if ((instance->trip != trip_max_desired_temperature) || + if (instance->trip != params->trip_max_desired_temperature || (!cdev_is_power_actor(instance->cdev))) continue; @@ -636,7 +618,6 @@ static int power_allocator_bind(struct thermal_zone_device *tz) { int ret; struct power_allocator_params *params; - struct thermal_trip trip; ret = check_power_actors(tz); if (ret) @@ -661,13 +642,11 @@ static int power_allocator_bind(struct thermal_zone_device *tz) get_governor_trips(tz, params); - if (tz->num_trips > 0) { - ret = __thermal_zone_get_trip(tz, params->trip_max_desired_temperature, - &trip); - if (!ret) - estimate_pid_constants(tz, tz->tzp->sustainable_power, - params->trip_switch_on, - trip.temperature); + if (params->trip_max_desired_temperature) { + int temp = params->trip_max_desired_temperature->temperature; + + estimate_pid_constants(tz, tz->tzp->sustainable_power, + params->trip_switch_on, temp); } reset_pid_controller(params); @@ -697,11 +676,10 @@ static void power_allocator_unbind(struct thermal_zone_device *tz) tz->governor_data = NULL; } -static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_id) +static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_index) { struct power_allocator_params *params = tz->governor_data; - struct thermal_trip trip; - int ret; + const struct thermal_trip *trip = &tz->trips[trip_index]; bool update; lockdep_assert_held(&tz->lock); @@ -710,12 +688,12 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_id) * We get called for every trip point but we only need to do * our calculations once */ - if (trip_id != params->trip_max_desired_temperature) + if (trip != params->trip_max_desired_temperature) return 0; - ret = __thermal_zone_get_trip(tz, params->trip_switch_on, &trip); - if (!ret && (tz->temperature < trip.temperature)) { - update = (tz->last_temperature >= trip.temperature); + trip = params->trip_switch_on; + if (trip && tz->temperature < trip->temperature) { + update = tz->last_temperature >= trip->temperature; tz->passive = 0; reset_pid_controller(params); allow_maximum_power(tz, update); @@ -724,14 +702,7 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_id) tz->passive = 1; - ret = __thermal_zone_get_trip(tz, params->trip_max_desired_temperature, &trip); - if (ret) { - dev_warn(&tz->device, "Failed to get the maximum desired temperature: %d\n", - ret); - return ret; - } - - return allocate_power(tz, trip.temperature); + return allocate_power(tz, params->trip_max_desired_temperature->temperature); } static struct thermal_governor thermal_gov_power_allocator = { -- cgit v1.2.3 From fdcf70ed4e1606cd5a5a48f666583053ae4c3978 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 12 Oct 2023 20:33:28 +0200 Subject: thermal: gov_step_wise: Fold update_passive_instance() into its caller Fold update_passive_instance() into thermal_zone_trip_update() that is its only caller so as to make the code in question easier to follow. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano Reviewed-by: Lukasz Luba --- drivers/thermal/gov_step_wise.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) (limited to 'drivers/thermal') diff --git a/drivers/thermal/gov_step_wise.c b/drivers/thermal/gov_step_wise.c index 849dc1ec8d27..8bc63a1d361e 100644 --- a/drivers/thermal/gov_step_wise.c +++ b/drivers/thermal/gov_step_wise.c @@ -68,17 +68,6 @@ static unsigned long get_target_state(struct thermal_instance *instance, return next_target; } -static void update_passive_instance(struct thermal_zone_device *tz, - enum thermal_trip_type type, int value) -{ - /* - * If value is +1, activate a passive instance. - * If value is -1, deactivate a passive instance. - */ - if (type == THERMAL_TRIP_PASSIVE) - tz->passive += value; -} - static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip_id) { const struct thermal_trip *trip = &tz->trips[trip_id]; @@ -109,14 +98,17 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip_id if (instance->initialized && old_target == instance->target) continue; - /* Activate a passive thermal instance */ if (old_target == THERMAL_NO_TARGET && - instance->target != THERMAL_NO_TARGET) - update_passive_instance(tz, trip->type, 1); - /* Deactivate a passive thermal instance */ - else if (old_target != THERMAL_NO_TARGET && - instance->target == THERMAL_NO_TARGET) - update_passive_instance(tz, trip->type, -1); + instance->target != THERMAL_NO_TARGET) { + /* Activate a passive thermal instance */ + if (trip->type == THERMAL_TRIP_PASSIVE) + tz->passive++; + } else if (old_target != THERMAL_NO_TARGET && + instance->target == THERMAL_NO_TARGET) { + /* Deactivate a passive thermal instance */ + if (trip->type == THERMAL_TRIP_PASSIVE) + tz->passive--; + } instance->initialized = true; mutex_lock(&instance->cdev->lock); -- cgit v1.2.3 From 8c35b1f472533b0df1b8f1f1afcaf4395cdb2256 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 12 Oct 2023 20:34:50 +0200 Subject: thermal: core: Pass trip pointer to governor throttle callback Modify the governor .throttle() callback definition so that it takes a trip pointer instead of a trip index as its second argument, adjust the governors accordingly and update the core code invoking .throttle(). This causes the governors to become independent of the representation of the list of trips in the thermal zone structure. This change is not expected to alter the general functionality. Signed-off-by: Rafael J. Wysocki Reviewed-by: Daniel Lezcano --- drivers/thermal/gov_bang_bang.c | 8 +++--- drivers/thermal/gov_fair_share.c | 6 ++--- drivers/thermal/gov_power_allocator.c | 4 +-- drivers/thermal/gov_step_wise.c | 12 +++++---- drivers/thermal/gov_user_space.c | 8 +++--- drivers/thermal/thermal_core.c | 50 +++++++++++++++++------------------ drivers/thermal/thermal_core.h | 2 +- drivers/thermal/thermal_helpers.c | 3 +-- include/linux/thermal.h | 3 ++- 9 files changed, 51 insertions(+), 45 deletions(-) (limited to 'drivers/thermal') diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c index 49cdfaa3a927..6ddf0accdc98 100644 --- a/drivers/thermal/gov_bang_bang.c +++ b/drivers/thermal/gov_bang_bang.c @@ -13,9 +13,10 @@ #include "thermal_core.h" -static int thermal_zone_trip_update(struct thermal_zone_device *tz, int trip_index) +static int thermal_zone_trip_update(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { - const struct thermal_trip *trip = &tz->trips[trip_index]; + int trip_index = thermal_zone_trip_id(tz, trip); struct thermal_instance *instance; if (!trip->hysteresis) @@ -89,7 +90,8 @@ static int thermal_zone_trip_update(struct thermal_zone_device *tz, int trip_ind * (trip_temp - hyst) so that the fan gets turned off again. * */ -static int bang_bang_control(struct thermal_zone_device *tz, int trip) +static int bang_bang_control(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { struct thermal_instance *instance; int ret; diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c index 41effa87d3cd..538abb7de4e2 100644 --- a/drivers/thermal/gov_fair_share.c +++ b/drivers/thermal/gov_fair_share.c @@ -47,7 +47,7 @@ static long get_target_state(struct thermal_zone_device *tz, /** * fair_share_throttle - throttles devices associated with the given zone * @tz: thermal_zone_device - * @trip_index: trip point index + * @trip: trip point * * Throttling Logic: This uses three parameters to calculate the new * throttle state of the cooling devices associated with the given zone. @@ -63,9 +63,9 @@ static long get_target_state(struct thermal_zone_device *tz, * (Heavily assumes the trip points are in ascending order) * new_state of cooling device = P3 * P2 * P1 */ -static int fair_share_throttle(struct thermal_zone_device *tz, int trip_index) +static int fair_share_throttle(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { - const struct thermal_trip *trip = &tz->trips[trip_index]; struct thermal_instance *instance; int total_weight = 0; int total_instance = 0; diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index c0d29d286374..83d4f451b1a9 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -676,10 +676,10 @@ static void power_allocator_unbind(struct thermal_zone_device *tz) tz->governor_data = NULL; } -static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_index) +static int power_allocator_throttle(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { struct power_allocator_params *params = tz->governor_data; - const struct thermal_trip *trip = &tz->trips[trip_index]; bool update; lockdep_assert_held(&tz->lock); diff --git a/drivers/thermal/gov_step_wise.c b/drivers/thermal/gov_step_wise.c index 8bc63a1d361e..5436aa58d41e 100644 --- a/drivers/thermal/gov_step_wise.c +++ b/drivers/thermal/gov_step_wise.c @@ -68,15 +68,16 @@ static unsigned long get_target_state(struct thermal_instance *instance, return next_target; } -static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip_id) +static void thermal_zone_trip_update(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { - const struct thermal_trip *trip = &tz->trips[trip_id]; + int trip_id = thermal_zone_trip_id(tz, trip); enum thermal_trend trend; struct thermal_instance *instance; bool throttle = false; int old_target; - trend = get_tz_trend(tz, trip_id); + trend = get_tz_trend(tz, trip); if (tz->temperature >= trip->temperature) { throttle = true; @@ -120,7 +121,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip_id /** * step_wise_throttle - throttles devices associated with the given zone * @tz: thermal_zone_device - * @trip: trip point index + * @trip: trip point * * Throttling Logic: This uses the trend of the thermal zone to throttle. * If the thermal zone is 'heating up' this throttles all the cooling @@ -128,7 +129,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip_id * step. If the zone is 'cooling down' it brings back the performance of * the devices by one step. */ -static int step_wise_throttle(struct thermal_zone_device *tz, int trip) +static int step_wise_throttle(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { struct thermal_instance *instance; diff --git a/drivers/thermal/gov_user_space.c b/drivers/thermal/gov_user_space.c index 8bc1c22aaf03..7a1790b7e8f5 100644 --- a/drivers/thermal/gov_user_space.c +++ b/drivers/thermal/gov_user_space.c @@ -25,11 +25,12 @@ static int user_space_bind(struct thermal_zone_device *tz) /** * notify_user_space - Notifies user space about thermal events * @tz: thermal_zone_device - * @trip: trip point index + * @trip: trip point * * This function notifies the user space through UEvents. */ -static int notify_user_space(struct thermal_zone_device *tz, int trip) +static int notify_user_space(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { char *thermal_prop[5]; int i; @@ -38,7 +39,8 @@ static int notify_user_space(struct thermal_zone_device *tz, int trip) thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type); thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature); - thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", trip); + thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", + thermal_zone_trip_id(tz, trip)); thermal_prop[3] = kasprintf(GFP_KERNEL, "EVENT=%d", tz->notify_event); thermal_prop[4] = NULL; kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 2b345f25dfa4..9c17d35ccbbd 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -307,7 +307,8 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz) thermal_zone_device_set_polling(tz, tz->polling_delay_jiffies); } -static void handle_non_critical_trips(struct thermal_zone_device *tz, int trip) +static void handle_non_critical_trips(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { tz->governor ? tz->governor->throttle(tz, trip) : def_governor->throttle(tz, trip); @@ -329,44 +330,43 @@ void thermal_zone_device_critical(struct thermal_zone_device *tz) EXPORT_SYMBOL(thermal_zone_device_critical); static void handle_critical_trips(struct thermal_zone_device *tz, - int trip, int trip_temp, enum thermal_trip_type trip_type) + const struct thermal_trip *trip) { /* If we have not crossed the trip_temp, we do not care. */ - if (trip_temp <= 0 || tz->temperature < trip_temp) + if (trip->temperature <= 0 || tz->temperature < trip->temperature) return; - trace_thermal_zone_trip(tz, trip, trip_type); + trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type); - if (trip_type == THERMAL_TRIP_HOT && tz->ops->hot) - tz->ops->hot(tz); - else if (trip_type == THERMAL_TRIP_CRITICAL) + if (trip->type == THERMAL_TRIP_CRITICAL) tz->ops->critical(tz); + else if (tz->ops->hot) + tz->ops->hot(tz); } -static void handle_thermal_trip(struct thermal_zone_device *tz, int trip_id) +static void handle_thermal_trip(struct thermal_zone_device *tz, + const struct thermal_trip *trip) { - struct thermal_trip trip; - - __thermal_zone_get_trip(tz, trip_id, &trip); - - if (trip.temperature == THERMAL_TEMP_INVALID) + if (trip->temperature == THERMAL_TEMP_INVALID) return; if (tz->last_temperature != THERMAL_TEMP_INVALID) { - if (tz->last_temperature < trip.temperature && - tz->temperature >= trip.temperature) - thermal_notify_tz_trip_up(tz->id, trip_id, + if (tz->last_temperature < trip->temperature && + tz->temperature >= trip->temperature) + thermal_notify_tz_trip_up(tz->id, + thermal_zone_trip_id(tz, trip), tz->temperature); - if (tz->last_temperature >= trip.temperature && - tz->temperature < (trip.temperature - trip.hysteresis)) - thermal_notify_tz_trip_down(tz->id, trip_id, + if (tz->last_temperature >= trip->temperature && + tz->temperature < trip->temperature - trip->hysteresis) + thermal_notify_tz_trip_down(tz->id, + thermal_zone_trip_id(tz, trip), tz->temperature); } - if (trip.type == THERMAL_TRIP_CRITICAL || trip.type == THERMAL_TRIP_HOT) - handle_critical_trips(tz, trip_id, trip.temperature, trip.type); + if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT) + handle_critical_trips(tz, trip); else - handle_non_critical_trips(tz, trip_id); + handle_non_critical_trips(tz, trip); } static void update_temperature(struct thermal_zone_device *tz) @@ -403,7 +403,7 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) void __thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event) { - int count; + const struct thermal_trip *trip; if (atomic_read(&in_suspend)) return; @@ -422,8 +422,8 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, tz->notify_event = event; - for (count = 0; count < tz->num_trips; count++) - handle_thermal_trip(tz, count); + for_each_trip(tz, trip) + handle_thermal_trip(tz, trip); monitor_thermal_zone(tz); } diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 4702d3a152f9..0a3b3ec5120b 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -70,7 +70,7 @@ static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev) void thermal_cdev_update(struct thermal_cooling_device *); void __thermal_cdev_update(struct thermal_cooling_device *cdev); -int get_tz_trend(struct thermal_zone_device *tz, int trip_index); +int get_tz_trend(struct thermal_zone_device *tz, const struct thermal_trip *trip); struct thermal_instance * get_thermal_instance(struct thermal_zone_device *tz, diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c index c1d0af73c85d..69e8ea4aa908 100644 --- a/drivers/thermal/thermal_helpers.c +++ b/drivers/thermal/thermal_helpers.c @@ -22,9 +22,8 @@ #include "thermal_core.h" #include "thermal_trace.h" -int get_tz_trend(struct thermal_zone_device *tz, int trip_index) +int get_tz_trend(struct thermal_zone_device *tz, const struct thermal_trip *trip) { - struct thermal_trip *trip = tz->trips ? &tz->trips[trip_index] : NULL; enum thermal_trend trend; if (tz->emul_temperature || !tz->ops->get_trend || diff --git a/include/linux/thermal.h b/include/linux/thermal.h index c8600e313909..cee814d5d1ac 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -199,7 +199,8 @@ struct thermal_governor { char name[THERMAL_NAME_LENGTH]; int (*bind_to_tz)(struct thermal_zone_device *tz); void (*unbind_from_tz)(struct thermal_zone_device *tz); - int (*throttle)(struct thermal_zone_device *tz, int trip); + int (*throttle)(struct thermal_zone_device *tz, + const struct thermal_trip *trip); struct list_head governor_list; }; -- cgit v1.2.3 From cf3986f8c01d355490d0ac6024391b989a9d1e9d Mon Sep 17 00:00:00 2001 From: "Nícolas F. R. A. Prado" Date: Fri, 22 Sep 2023 14:44:03 -0400 Subject: thermal: core: Don't update trip points inside the hysteresis range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When searching for the trip points that need to be set, the nearest higher trip point's temperature is used for the high trip, while the nearest lower trip point's temperature minus the hysteresis is used for the low trip. The issue with this logic is that when the current temperature is inside a trip point's hysteresis range, both high and low trips will come from the same trip point. As a consequence instability can still occur like this: * the temperature rises slightly and enters the hysteresis range of a trip point * polling happens and updates the trip points to the hysteresis range * the temperature falls slightly, exiting the hysteresis range, crossing the trip point and triggering an IRQ, the trip points are updated * repeat So even though the current hysteresis implementation prevents instability from happening due to IRQs triggering on the same temperature value, both ways, it doesn't prevent it from happening due to an IRQ on one way and polling on the other. To properly implement a hysteresis behavior, when inside the hysteresis range, don't update the trip points. This way, the previously set trip points will stay in effect, which will in a way remember the previous state (if the temperature signal came from above or below the range) and therefore have the right trip point already set. The exception is if there was no previous trip point set, in which case a previous state doesn't exist, and so it's sensible to allow the hysteresis range as trip points. The following logs show the current behavior when running on a real machine: [ 202.524658] thermal thermal_zone0: new temperature boundaries: -2147483647 < x < 40000 203.562817: thermal_temperature: thermal_zone=vpu0-thermal id=0 temp_prev=36986 temp=37979 [ 203.562845] thermal thermal_zone0: new temperature boundaries: 37000 < x < 40000 204.176059: thermal_temperature: thermal_zone=vpu0-thermal id=0 temp_prev=37979 temp=40028 [ 204.176089] thermal thermal_zone0: new temperature boundaries: 37000 < x < 100000 205.226813: thermal_temperature: thermal_zone=vpu0-thermal id=0 temp_prev=40028 temp=38652 [ 205.226842] thermal thermal_zone0: new temperature boundaries: 37000 < x < 40000 And with this patch applied: [ 184.933415] thermal thermal_zone0: new temperature boundaries: -2147483647 < x < 40000 185.981182: thermal_temperature: thermal_zone=vpu0-thermal id=0 temp_prev=36986 temp=37872 186.744685: thermal_temperature: thermal_zone=vpu0-thermal id=0 temp_prev=37872 temp=40058 [ 186.744716] thermal thermal_zone0: new temperature boundaries: 37000 < x < 100000 187.773284: thermal_temperature: thermal_zone=vpu0-thermal id=0 temp_prev=40058 temp=38698 Fixes: 060c034a9741 ("thermal: Add support for hardware-tracked trip points") Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: AngeloGioacchino Del Regno Co-developed-by: Rafael J. Wysocki Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_trip.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'drivers/thermal') diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 9e07535d99bb..e42456442c68 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -65,6 +65,7 @@ void __thermal_zone_set_trips(struct thermal_zone_device *tz) { struct thermal_trip trip; int low = -INT_MAX, high = INT_MAX; + bool same_trip = false; int i, ret; lockdep_assert_held(&tz->lock); @@ -73,6 +74,7 @@ void __thermal_zone_set_trips(struct thermal_zone_device *tz) return; for (i = 0; i < tz->num_trips; i++) { + bool low_set = false; int trip_low; ret = __thermal_zone_get_trip(tz, i , &trip); @@ -81,18 +83,31 @@ void __thermal_zone_set_trips(struct thermal_zone_device *tz) trip_low = trip.temperature - trip.hysteresis; - if (trip_low < tz->temperature && trip_low > low) + if (trip_low < tz->temperature && trip_low > low) { low = trip_low; + low_set = true; + same_trip = false; + } if (trip.temperature > tz->temperature && - trip.temperature < high) + trip.temperature < high) { high = trip.temperature; + same_trip = low_set; + } } /* No need to change trip points */ if (tz->prev_low_trip == low && tz->prev_high_trip == high) return; + /* + * If "high" and "low" are the same, skip the change unless this is the + * first time. + */ + if (same_trip && (tz->prev_low_trip != -INT_MAX || + tz->prev_high_trip != INT_MAX)) + return; + tz->prev_low_trip = low; tz->prev_high_trip = high; -- cgit v1.2.3 From c27d08f786aca71a94b06437d983a5518ec90ee8 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 13 Oct 2023 17:14:05 +0200 Subject: thermal: ACPI: Include the right header file It is not necessary to include thermal_core.h into thermal_acpi.c, because none of the code in there depends on anything in the former, except for the linux/thermal.h, but it is better to include that one directly instead of including the entire thermal_core.h, so make that change. No functional impact. Fixes: 7a0e39748861 ("thermal: ACPI: Add ACPI trip point routines") Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_acpi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/thermal') diff --git a/drivers/thermal/thermal_acpi.c b/drivers/thermal/thermal_acpi.c index 0e5698818f69..43eaf0f2ff49 100644 --- a/drivers/thermal/thermal_acpi.c +++ b/drivers/thermal/thermal_acpi.c @@ -8,8 +8,7 @@ */ #include #include - -#include "thermal_core.h" +#include /* * Minimum temperature for full military grade is 218°K (-55°C) and -- cgit v1.2.3