| Message ID | 2628456.Lt9SDvczpP@kreacher (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | thermal: core: Redesign the governor interface | expand |
On 4/10/24 17:13, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make the Step-Wise governor use the new .manage() callback instead of > .throttle(). > > Even though using .throttle() is not particularly problematic for the > Step-Wise governor, using .manage() instead still allows it to reduce > overhead by updating all of the colling devices once after setting s/colling/cooling/ > target values for all of the thermal instances. Make sense, good observation, it's pointless to call the thermal_cdev_update() in each trip point throttle() invocation. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/gov_step_wise.c | 39 +++++++++++++++++++++------------------ > 1 file changed, 21 insertions(+), 18 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 > @@ -109,34 +109,37 @@ static void thermal_zone_trip_update(str > } > } > > -/** > - * step_wise_throttle - throttles devices associated with the given zone > - * @tz: thermal_zone_device > - * @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 > - * devices associated with the zone and its particular trip point, by one > - * 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, > - const struct thermal_trip *trip) > +static void step_wise_manage(struct thermal_zone_device *tz) > { > + const struct thermal_trip_desc *td; > struct thermal_instance *instance; > > lockdep_assert_held(&tz->lock); > > - thermal_zone_trip_update(tz, trip); > + /* > + * Throttling Logic: Use the trend of the thermal zone to throttle. > + * If the thermal zone is 'heating up', throttle all of the cooling > + * devices associated with each trip point by one step. If the zone > + * is 'cooling down', it brings back the performance of the devices > + * by one step. > + */ > + for_each_trip_desc(tz, td) { > + const struct thermal_trip *trip = &td->trip; > + > + if (trip->temperature == THERMAL_TEMP_INVALID || > + trip->type == THERMAL_TRIP_CRITICAL || > + trip->type == THERMAL_TRIP_HOT) > + continue; > + > + thermal_zone_trip_update(tz, trip); > + } > > list_for_each_entry(instance, &tz->thermal_instances, tz_node) > thermal_cdev_update(instance->cdev); > - > - return 0; > } > > static struct thermal_governor thermal_gov_step_wise = { > - .name = "step_wise", > - .throttle = step_wise_throttle, > + .name = "step_wise", > + .manage = step_wise_manage, > }; > THERMAL_GOVERNOR_DECLARE(thermal_gov_step_wise); > > > LGTM w/ that 'cooling' spelling fixed. Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
On 10/04/2024 18:13, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make the Step-Wise governor use the new .manage() callback instead of > .throttle(). > > Even though using .throttle() is not particularly problematic for the > Step-Wise governor, using .manage() instead still allows it to reduce > overhead by updating all of the colling devices once after setting > target values for all of the thermal instances. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
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 @@ -109,34 +109,37 @@ static void thermal_zone_trip_update(str } } -/** - * step_wise_throttle - throttles devices associated with the given zone - * @tz: thermal_zone_device - * @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 - * devices associated with the zone and its particular trip point, by one - * 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, - const struct thermal_trip *trip) +static void step_wise_manage(struct thermal_zone_device *tz) { + const struct thermal_trip_desc *td; struct thermal_instance *instance; lockdep_assert_held(&tz->lock); - thermal_zone_trip_update(tz, trip); + /* + * Throttling Logic: Use the trend of the thermal zone to throttle. + * If the thermal zone is 'heating up', throttle all of the cooling + * devices associated with each trip point by one step. If the zone + * is 'cooling down', it brings back the performance of the devices + * by one step. + */ + for_each_trip_desc(tz, td) { + const struct thermal_trip *trip = &td->trip; + + if (trip->temperature == THERMAL_TEMP_INVALID || + trip->type == THERMAL_TRIP_CRITICAL || + trip->type == THERMAL_TRIP_HOT) + continue; + + thermal_zone_trip_update(tz, trip); + } list_for_each_entry(instance, &tz->thermal_instances, tz_node) thermal_cdev_update(instance->cdev); - - return 0; } static struct thermal_governor thermal_gov_step_wise = { - .name = "step_wise", - .throttle = step_wise_throttle, + .name = "step_wise", + .manage = step_wise_manage, }; THERMAL_GOVERNOR_DECLARE(thermal_gov_step_wise);