| Message ID | 6103874.lOV4Wx5bFT@rjwysocki.net |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] thermal: gov_bang_bang: Adjust states of all uninitialized instances | expand |
On 8/26/24 17:23, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > If a cooling device is registered after a thermal zone it should be > bound to and the trip point it should be bound to has already been > crossed by the zone temperature on the way up, the cooling device's > state may need to be adjusted, but the Bang-bang governor will not > do that because its .manage() callback only looks at thermal instances > for trip points whose thresholds are below or at the zone temperature. > > Address this by updating bang_bang_manage() to look at all of the > uninitialized thermal instances and setting their target states in > accordance with the position of the zone temperature with respect to > the threshold of the given trip point. > > Fixes: 5f64b4a1ab1b ("thermal: gov_bang_bang: Add .manage() callback") > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > > Found by code inspection while considering a related change. > > I don't thik it is super-urgent, but it qualifies as 6.12 material IMV. > > --- > drivers/thermal/gov_bang_bang.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > Index: linux-pm/drivers/thermal/gov_bang_bang.c > =================================================================== > --- linux-pm.orig/drivers/thermal/gov_bang_bang.c > +++ linux-pm/drivers/thermal/gov_bang_bang.c > @@ -92,23 +92,21 @@ static void bang_bang_manage(struct ther > > for_each_trip_desc(tz, td) { > const struct thermal_trip *trip = &td->trip; > + bool turn_on; > > - if (tz->temperature >= td->threshold || > - trip->temperature == THERMAL_TEMP_INVALID || > + if (trip->temperature == THERMAL_TEMP_INVALID || > trip->type == THERMAL_TRIP_CRITICAL || > trip->type == THERMAL_TRIP_HOT) > continue; > > /* > - * If the initial cooling device state is "on", but the zone > - * temperature is not above the trip point, the core will not > - * call bang_bang_control() until the zone temperature reaches > - * the trip point temperature which may be never. In those > - * cases, set the initial state of the cooling device to 0. > + * Adjust the target states for uninitialized thermal instances > + * to the thermal zone temperature and the trip point threshold. > */ > + turn_on = tz->temperature >= td->threshold; > list_for_each_entry(instance, &tz->thermal_instances, tz_node) { > if (!instance->initialized && instance->trip == trip) > - bang_bang_set_instance_target(instance, 0); > + bang_bang_set_instance_target(instance, turn_on); > } > } > > > > That makes sense. Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
On 26.08.24 18:23, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > If a cooling device is registered after a thermal zone it should be > bound to and the trip point it should be bound to has already been > crossed by the zone temperature on the way up, the cooling device's > state may need to be adjusted, but the Bang-bang governor will not > do that because its .manage() callback only looks at thermal instances > for trip points whose thresholds are below or at the zone temperature. > > Address this by updating bang_bang_manage() to look at all of the > uninitialized thermal instances and setting their target states in > accordance with the position of the zone temperature with respect to > the threshold of the given trip point. > > Fixes: 5f64b4a1ab1b ("thermal: gov_bang_bang: Add .manage() callback") > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Acked-by: Peter Kaestle <peter@piie.net> > --- > > Found by code inspection while considering a related change. > > I don't thik it is super-urgent, but it qualifies as 6.12 material IMV. > > --- > drivers/thermal/gov_bang_bang.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > Index: linux-pm/drivers/thermal/gov_bang_bang.c > =================================================================== > --- linux-pm.orig/drivers/thermal/gov_bang_bang.c > +++ linux-pm/drivers/thermal/gov_bang_bang.c > @@ -92,23 +92,21 @@ static void bang_bang_manage(struct ther > > for_each_trip_desc(tz, td) { > const struct thermal_trip *trip = &td->trip; > + bool turn_on; > > - if (tz->temperature >= td->threshold || > - trip->temperature == THERMAL_TEMP_INVALID || > + if (trip->temperature == THERMAL_TEMP_INVALID || > trip->type == THERMAL_TRIP_CRITICAL || > trip->type == THERMAL_TRIP_HOT) > continue; > > /* > - * If the initial cooling device state is "on", but the zone > - * temperature is not above the trip point, the core will not > - * call bang_bang_control() until the zone temperature reaches > - * the trip point temperature which may be never. In those > - * cases, set the initial state of the cooling device to 0. > + * Adjust the target states for uninitialized thermal instances > + * to the thermal zone temperature and the trip point threshold. > */ > + turn_on = tz->temperature >= td->threshold; > list_for_each_entry(instance, &tz->thermal_instances, tz_node) { > if (!instance->initialized && instance->trip == trip) > - bang_bang_set_instance_target(instance, 0); > + bang_bang_set_instance_target(instance, turn_on); > } > } > > >
Index: linux-pm/drivers/thermal/gov_bang_bang.c =================================================================== --- linux-pm.orig/drivers/thermal/gov_bang_bang.c +++ linux-pm/drivers/thermal/gov_bang_bang.c @@ -92,23 +92,21 @@ static void bang_bang_manage(struct ther for_each_trip_desc(tz, td) { const struct thermal_trip *trip = &td->trip; + bool turn_on; - if (tz->temperature >= td->threshold || - trip->temperature == THERMAL_TEMP_INVALID || + if (trip->temperature == THERMAL_TEMP_INVALID || trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT) continue; /* - * If the initial cooling device state is "on", but the zone - * temperature is not above the trip point, the core will not - * call bang_bang_control() until the zone temperature reaches - * the trip point temperature which may be never. In those - * cases, set the initial state of the cooling device to 0. + * Adjust the target states for uninitialized thermal instances + * to the thermal zone temperature and the trip point threshold. */ + turn_on = tz->temperature >= td->threshold; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { if (!instance->initialized && instance->trip == trip) - bang_bang_set_instance_target(instance, 0); + bang_bang_set_instance_target(instance, turn_on); } }