2024-04-10 17:47:01

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 01/16] thermal: core: Introduce .trip_crossed() callback for thermal governors

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>
---
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,





2024-04-19 08:47:58

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v1 01/16] thermal: core: Introduce .trip_crossed() callback for thermal governors



On 4/10/24 17:10, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>

2024-04-23 17:15:24

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v1 01/16] thermal: core: Introduce .trip_crossed() callback for thermal governors

On 10/04/2024 18:10, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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 <[email protected]>
> ---
> 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,
>
>
>

--
<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


2024-04-23 17:25:30

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v1 01/16] thermal: core: Introduce .trip_crossed() callback for thermal governors

On Tue, Apr 23, 2024 at 7:14 PM Daniel Lezcano
<[email protected]> wrote:
>
> On 10/04/2024 18:10, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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!

2024-04-23 18:22:44

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v1 01/16] thermal: core: Introduce .trip_crossed() callback for thermal governors

On 23/04/2024 19:25, Rafael J. Wysocki wrote:
> On Tue, Apr 23, 2024 at 7:14 PM Daniel Lezcano
> <[email protected]> wrote:
>>
>> On 10/04/2024 18:10, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---

[ ... ]

>>> + 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


--
<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