| Message ID | 2009494.usQuhbGJ8B@kreacher |
|---|---|
| State | New |
| Headers | show |
| Series | thermal: core: Redesign the governor interface | expand |
On 4/10/24 17:10, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Introduce a new thermal governor callback called .trip_crossed() > that will be invoked whenever a trip point is crossed by the zone > temperature, either on the way up or on the way down. > > The trip crossing direction information will be passed to it and if > multiple trips are crossed in the same direction during one thermal zone > update, the new callback will be invoked for them in temperature order, > either ascending or descending, depending on the trip crossing > direction. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/thermal_core.c | 19 +++++++++++++++++-- > drivers/thermal/thermal_core.h | 4 ++++ > 2 files changed, 21 insertions(+), 2 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_core.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.c > +++ linux-pm/drivers/thermal/thermal_core.c > @@ -302,11 +302,21 @@ static void monitor_thermal_zone(struct > thermal_zone_device_set_polling(tz, tz->polling_delay_jiffies); > } > > +static struct thermal_governor *thermal_get_tz_governor(struct thermal_zone_device *tz) > +{ > + if (tz->governor) > + return tz->governor; > + > + return def_governor; > +} > + > 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); > + struct thermal_governor *governor = thermal_get_tz_governor(tz); > + > + if (governor->throttle) > + governor->throttle(tz, trip); > } > > void thermal_governor_update_tz(struct thermal_zone_device *tz, > @@ -470,6 +480,7 @@ static int thermal_trip_notify_cmp(void > void __thermal_zone_device_update(struct thermal_zone_device *tz, > enum thermal_notify_event event) > { > + struct thermal_governor *governor = thermal_get_tz_governor(tz); > struct thermal_trip_desc *td; > LIST_HEAD(way_down_list); > LIST_HEAD(way_up_list); > @@ -493,12 +504,16 @@ void __thermal_zone_device_update(struct > list_for_each_entry(td, &way_up_list, notify_list_node) { > thermal_notify_tz_trip_up(tz, &td->trip); > thermal_debug_tz_trip_up(tz, &td->trip); > + if (governor->trip_crossed) > + governor->trip_crossed(tz, &td->trip, true); > } > > list_sort(NULL, &way_down_list, thermal_trip_notify_cmp); > list_for_each_entry(td, &way_down_list, notify_list_node) { > thermal_notify_tz_trip_down(tz, &td->trip); > thermal_debug_tz_trip_down(tz, &td->trip); > + if (governor->trip_crossed) > + governor->trip_crossed(tz, &td->trip, false); > } > > monitor_thermal_zone(tz); > Index: linux-pm/drivers/thermal/thermal_core.h > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.h > +++ linux-pm/drivers/thermal/thermal_core.h > @@ -30,6 +30,7 @@ struct thermal_trip_desc { > * otherwise it fails. > * @unbind_from_tz: callback called when a governor is unbound from a > * thermal zone. > + * @trip_crossed: called for trip points that have just been crossed > * @throttle: callback called for every trip point even if temperature is > * below the trip point temperature > * @update_tz: callback called when thermal zone internals have changed, e.g. > @@ -40,6 +41,9 @@ struct thermal_governor { > const char *name; > int (*bind_to_tz)(struct thermal_zone_device *tz); > void (*unbind_from_tz)(struct thermal_zone_device *tz); > + void (*trip_crossed)(struct thermal_zone_device *tz, > + const struct thermal_trip *trip, > + bool crossed_up); > int (*throttle)(struct thermal_zone_device *tz, > const struct thermal_trip *trip); > void (*update_tz)(struct thermal_zone_device *tz, > > > LGTM Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
On 10/04/2024 18:10, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Introduce a new thermal governor callback called .trip_crossed() > that will be invoked whenever a trip point is crossed by the zone > temperature, either on the way up or on the way down. > > The trip crossing direction information will be passed to it and if > multiple trips are crossed in the same direction during one thermal zone > update, the new callback will be invoked for them in temperature order, > either ascending or descending, depending on the trip crossing > direction. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/thermal_core.c | 19 +++++++++++++++++-- > drivers/thermal/thermal_core.h | 4 ++++ > 2 files changed, 21 insertions(+), 2 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_core.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.c > +++ linux-pm/drivers/thermal/thermal_core.c > @@ -302,11 +302,21 @@ static void monitor_thermal_zone(struct > thermal_zone_device_set_polling(tz, tz->polling_delay_jiffies); > } > > +static struct thermal_governor *thermal_get_tz_governor(struct thermal_zone_device *tz) > +{ > + if (tz->governor) > + return tz->governor; > + > + return def_governor; > +} > + > 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); > + struct thermal_governor *governor = thermal_get_tz_governor(tz); > + > + if (governor->throttle) > + governor->throttle(tz, trip); > } > > void thermal_governor_update_tz(struct thermal_zone_device *tz, > @@ -470,6 +480,7 @@ static int thermal_trip_notify_cmp(void > void __thermal_zone_device_update(struct thermal_zone_device *tz, > enum thermal_notify_event event) > { > + struct thermal_governor *governor = thermal_get_tz_governor(tz); > struct thermal_trip_desc *td; > LIST_HEAD(way_down_list); > LIST_HEAD(way_up_list); > @@ -493,12 +504,16 @@ void __thermal_zone_device_update(struct > list_for_each_entry(td, &way_up_list, notify_list_node) { > thermal_notify_tz_trip_up(tz, &td->trip); > thermal_debug_tz_trip_up(tz, &td->trip); > + if (governor->trip_crossed) > + governor->trip_crossed(tz, &td->trip, true); Is it possible to wrap this into a function ? So we keep the calls at the same level in this block > } > > list_sort(NULL, &way_down_list, thermal_trip_notify_cmp); > list_for_each_entry(td, &way_down_list, notify_list_node) { > thermal_notify_tz_trip_down(tz, &td->trip); > thermal_debug_tz_trip_down(tz, &td->trip); > + if (governor->trip_crossed) > + governor->trip_crossed(tz, &td->trip, false); idem > } > > monitor_thermal_zone(tz); > Index: linux-pm/drivers/thermal/thermal_core.h > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.h > +++ linux-pm/drivers/thermal/thermal_core.h > @@ -30,6 +30,7 @@ struct thermal_trip_desc { > * otherwise it fails. > * @unbind_from_tz: callback called when a governor is unbound from a > * thermal zone. > + * @trip_crossed: called for trip points that have just been crossed > * @throttle: callback called for every trip point even if temperature is > * below the trip point temperature > * @update_tz: callback called when thermal zone internals have changed, e.g. > @@ -40,6 +41,9 @@ struct thermal_governor { > const char *name; > int (*bind_to_tz)(struct thermal_zone_device *tz); > void (*unbind_from_tz)(struct thermal_zone_device *tz); > + void (*trip_crossed)(struct thermal_zone_device *tz, > + const struct thermal_trip *trip, > + bool crossed_up); > int (*throttle)(struct thermal_zone_device *tz, > const struct thermal_trip *trip); > void (*update_tz)(struct thermal_zone_device *tz, > > >
On Tue, Apr 23, 2024 at 7:14 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 10/04/2024 18:10, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Introduce a new thermal governor callback called .trip_crossed() > > that will be invoked whenever a trip point is crossed by the zone > > temperature, either on the way up or on the way down. > > > > The trip crossing direction information will be passed to it and if > > multiple trips are crossed in the same direction during one thermal zone > > update, the new callback will be invoked for them in temperature order, > > either ascending or descending, depending on the trip crossing > > direction. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > drivers/thermal/thermal_core.c | 19 +++++++++++++++++-- > > drivers/thermal/thermal_core.h | 4 ++++ > > 2 files changed, 21 insertions(+), 2 deletions(-) > > > > Index: linux-pm/drivers/thermal/thermal_core.c > > =================================================================== > > --- linux-pm.orig/drivers/thermal/thermal_core.c > > +++ linux-pm/drivers/thermal/thermal_core.c > > @@ -302,11 +302,21 @@ static void monitor_thermal_zone(struct > > thermal_zone_device_set_polling(tz, tz->polling_delay_jiffies); > > } > > > > +static struct thermal_governor *thermal_get_tz_governor(struct thermal_zone_device *tz) > > +{ > > + if (tz->governor) > > + return tz->governor; > > + > > + return def_governor; > > +} > > + > > 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); > > + struct thermal_governor *governor = thermal_get_tz_governor(tz); > > + > > + if (governor->throttle) > > + governor->throttle(tz, trip); > > } > > > > void thermal_governor_update_tz(struct thermal_zone_device *tz, > > @@ -470,6 +480,7 @@ static int thermal_trip_notify_cmp(void > > void __thermal_zone_device_update(struct thermal_zone_device *tz, > > enum thermal_notify_event event) > > { > > + struct thermal_governor *governor = thermal_get_tz_governor(tz); > > struct thermal_trip_desc *td; > > LIST_HEAD(way_down_list); > > LIST_HEAD(way_up_list); > > @@ -493,12 +504,16 @@ void __thermal_zone_device_update(struct > > list_for_each_entry(td, &way_up_list, notify_list_node) { > > thermal_notify_tz_trip_up(tz, &td->trip); > > thermal_debug_tz_trip_up(tz, &td->trip); > > + if (governor->trip_crossed) > > + governor->trip_crossed(tz, &td->trip, true); > > Is it possible to wrap this into a function ? So we keep the calls at > the same level in this block I can send a separate patch for this if you want me to. Thanks!
On 23/04/2024 19:25, Rafael J. Wysocki wrote: > On Tue, Apr 23, 2024 at 7:14 PM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: >> >> On 10/04/2024 18:10, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> Introduce a new thermal governor callback called .trip_crossed() >>> that will be invoked whenever a trip point is crossed by the zone >>> temperature, either on the way up or on the way down. >>> >>> The trip crossing direction information will be passed to it and if >>> multiple trips are crossed in the same direction during one thermal zone >>> update, the new callback will be invoked for them in temperature order, >>> either ascending or descending, depending on the trip crossing >>> direction. >>> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> --- [ ... ] >>> + if (governor->trip_crossed) >>> + governor->trip_crossed(tz, &td->trip, true); >> >> Is it possible to wrap this into a function ? So we keep the calls at >> the same level in this block > > I can send a separate patch for this if you want me to. Yes, sure
Index: linux-pm/drivers/thermal/thermal_core.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.c +++ linux-pm/drivers/thermal/thermal_core.c @@ -302,11 +302,21 @@ static void monitor_thermal_zone(struct thermal_zone_device_set_polling(tz, tz->polling_delay_jiffies); } +static struct thermal_governor *thermal_get_tz_governor(struct thermal_zone_device *tz) +{ + if (tz->governor) + return tz->governor; + + return def_governor; +} + 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); + struct thermal_governor *governor = thermal_get_tz_governor(tz); + + if (governor->throttle) + governor->throttle(tz, trip); } void thermal_governor_update_tz(struct thermal_zone_device *tz, @@ -470,6 +480,7 @@ static int thermal_trip_notify_cmp(void void __thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event) { + struct thermal_governor *governor = thermal_get_tz_governor(tz); struct thermal_trip_desc *td; LIST_HEAD(way_down_list); LIST_HEAD(way_up_list); @@ -493,12 +504,16 @@ void __thermal_zone_device_update(struct list_for_each_entry(td, &way_up_list, notify_list_node) { thermal_notify_tz_trip_up(tz, &td->trip); thermal_debug_tz_trip_up(tz, &td->trip); + if (governor->trip_crossed) + governor->trip_crossed(tz, &td->trip, true); } list_sort(NULL, &way_down_list, thermal_trip_notify_cmp); list_for_each_entry(td, &way_down_list, notify_list_node) { thermal_notify_tz_trip_down(tz, &td->trip); thermal_debug_tz_trip_down(tz, &td->trip); + if (governor->trip_crossed) + governor->trip_crossed(tz, &td->trip, false); } monitor_thermal_zone(tz); Index: linux-pm/drivers/thermal/thermal_core.h =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.h +++ linux-pm/drivers/thermal/thermal_core.h @@ -30,6 +30,7 @@ struct thermal_trip_desc { * otherwise it fails. * @unbind_from_tz: callback called when a governor is unbound from a * thermal zone. + * @trip_crossed: called for trip points that have just been crossed * @throttle: callback called for every trip point even if temperature is * below the trip point temperature * @update_tz: callback called when thermal zone internals have changed, e.g. @@ -40,6 +41,9 @@ struct thermal_governor { const char *name; int (*bind_to_tz)(struct thermal_zone_device *tz); void (*unbind_from_tz)(struct thermal_zone_device *tz); + void (*trip_crossed)(struct thermal_zone_device *tz, + const struct thermal_trip *trip, + bool crossed_up); int (*throttle)(struct thermal_zone_device *tz, const struct thermal_trip *trip); void (*update_tz)(struct thermal_zone_device *tz,