Hi Everyone,
This is an update of
https://lore.kernel.org/linux-pm/4558384.LvFx2qVVIh@kreacher/
and
https://lore.kernel.org/linux-pm/2331888.ElGaqSPkdT@kreacher/
which rebases the first patch on top of 6.9-rc2, adds 3 patches and adjusts
the third patch from v2.
The original description of the first two patches still applies:
> Patch [1/2] is based on the observation that the threshold field in struct
> thermal_trip really should be core-internal and to make that happen it
> introduces a wrapper structure around struct thermal_trip for internal
> use in the core.
>
> Patch [2/2] moves the definition of the new structure and the struct
> thermal_zone_device one to a local header file in the core to enforce
> more separation between the core and drivers.
>
> The patches are not expected to introduce any observable differences in
> behavior, so please let me know if you see any of that.
Note that these patches were first sent before the merge window and have not
really changed since then (except for a minor rebase of the first patch in
this series). Moreover, no comments regarding the merit of these patches
have been made shared, so if this continues, I will be considering them as
good to go by the end of this week.
Patch [3/6] is a rewrite of comments regarding trip crossing and threshold
computations.
Patch [4/6] updates the trip crossing detection code to consolidate the
threshold initialization with trip crossing on the way up.
Patch [5/6] ([3/3] in v2) adds a mechanism to sort notifications and debug
calls taking place during one invocation of __thermal_zone_device_update() so
they always go in temperature order.
Patch [6/6] relocates the critical and trip point handling to avoid a
redundant temperature check.
The series applies on top of 6.9-rc2 and I'm planning to create a test
branch containing it.
Thanks!
From: Rafael J. Wysocki <[email protected]>
If multiple trip points are crossed in one go and the trips table in
the thermal zone device object is not sorted, the corresponding trip
point crossing notifications sent to user space will not be ordered
either.
Moreover, if the trips table is sorted by trip temperature in ascending
order, the trip crossing notifications on the way up will be sent in that
order too, but the trip crossing notifications on the way down will be
sent in the reverse order.
This is generally confusing and it is better to make the kernel send the
notifications in the order of growing (on the way up) or falling (on the
way down) trip temperature.
To achieve that, instead of sending a trip crossing notification and
recording a trip crossing event in the statistics right away from
handle_thermal_trip(), put the trip in question on a list that will be
sorted by __thermal_zone_device_update() after processing all of the trips
and before sending the notifications and recording trip crossing events.
Link: https://lore.kernel.org/linux-pm/[email protected]/
Reported-by: Daniel Lezcano <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v2 -> v3: Rebase, update changelog, add notify_temp to struct thermal_trip_desc
v1 -> v2: New patch
---
drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++------
drivers/thermal/thermal_core.h | 2 ++
2 files changed, 37 insertions(+), 6 deletions(-)
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -15,6 +15,7 @@
#include <linux/slab.h>
#include <linux/kdev_t.h>
#include <linux/idr.h>
+#include <linux/list_sort.h>
#include <linux/thermal.h>
#include <linux/reboot.h>
#include <linux/string.h>
@@ -361,7 +362,9 @@ static void handle_critical_trips(struct
}
static void handle_thermal_trip(struct thermal_zone_device *tz,
- struct thermal_trip_desc *td)
+ struct thermal_trip_desc *td,
+ struct list_head *way_up_list,
+ struct list_head *way_down_list)
{
const struct thermal_trip *trip = &td->trip;
int old_threshold;
@@ -387,8 +390,8 @@ static void handle_thermal_trip(struct t
* In that case, the trip temperature becomes the new threshold.
*/
if (tz->temperature < trip->temperature - trip->hysteresis) {
- thermal_notify_tz_trip_down(tz, trip);
- thermal_debug_tz_trip_down(tz, trip);
+ list_add(&td->notify_list_node, way_down_list);
+ td->notify_temp = trip->temperature - trip->hysteresis;
} else {
td->threshold -= trip->hysteresis;
}
@@ -398,8 +401,8 @@ static void handle_thermal_trip(struct t
* if the zone temperature exceeds the trip one. The new
* threshold is then set to the low temperature of the trip.
*/
- thermal_notify_tz_trip_up(tz, trip);
- thermal_debug_tz_trip_up(tz, trip);
+ list_add_tail(&td->notify_list_node, way_up_list);
+ td->notify_temp = trip->temperature;
td->threshold -= trip->hysteresis;
}
@@ -452,10 +455,24 @@ static void thermal_zone_device_init(str
pos->initialized = false;
}
+static int thermal_trip_notify_cmp(void *ascending, const struct list_head *a,
+ const struct list_head *b)
+{
+ struct thermal_trip_desc *tda = container_of(a, struct thermal_trip_desc,
+ notify_list_node);
+ struct thermal_trip_desc *tdb = container_of(b, struct thermal_trip_desc,
+ notify_list_node);
+ int ret = tdb->notify_temp - tda->notify_temp;
+
+ return ascending ? ret : -ret;
+}
+
void __thermal_zone_device_update(struct thermal_zone_device *tz,
enum thermal_notify_event event)
{
struct thermal_trip_desc *td;
+ LIST_HEAD(way_down_list);
+ LIST_HEAD(way_up_list);
if (tz->suspended)
return;
@@ -470,7 +487,19 @@ void __thermal_zone_device_update(struct
tz->notify_event = event;
for_each_trip_desc(tz, td)
- handle_thermal_trip(tz, td);
+ handle_thermal_trip(tz, td, &way_up_list, &way_down_list);
+
+ list_sort(&way_up_list, &way_up_list, thermal_trip_notify_cmp);
+ 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);
+ }
+
+ 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);
+ }
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
@@ -17,6 +17,8 @@
struct thermal_trip_desc {
struct thermal_trip trip;
+ struct list_head notify_list_node;
+ int notify_temp;
int threshold;
};
From: Rafael J. Wysocki <[email protected]>
The threshold field in struct thermal_trip is only used internally by
the thermal core and it is better to prevent drivers from misusing it.
It also takes some space unnecessarily in the trip tables passed by
drivers to the core during thermal zone registration.
For this reason, introduce struct thermal_trip_desc as a wrapper around
struct thermal_trip, move the threshold field directly into it and make
the thermal core store struct thermal_trip_desc objects in the internal
thermal zone trip tables. Adjust all of the code using trip tables in
the thermal core accordingly.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v2 -> v3: Rebase on top of 6.9-rc2, minor changelog update.
v1 -> v2: No changes.
---
drivers/thermal/gov_fair_share.c | 7 +++--
drivers/thermal/gov_power_allocator.c | 6 ++--
drivers/thermal/thermal_core.c | 46 +++++++++++++++++++---------------
drivers/thermal/thermal_core.h | 7 +++--
drivers/thermal/thermal_debugfs.c | 6 ++--
drivers/thermal/thermal_helpers.c | 8 +++--
drivers/thermal/thermal_netlink.c | 6 ++--
drivers/thermal/thermal_sysfs.c | 20 +++++++-------
drivers/thermal/thermal_trip.c | 15 +++++------
include/linux/thermal.h | 9 ++++--
10 files changed, 78 insertions(+), 52 deletions(-)
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -61,7 +61,6 @@ enum thermal_notify_event {
* struct thermal_trip - representation of a point in temperature domain
* @temperature: temperature value in miliCelsius
* @hysteresis: relative hysteresis in miliCelsius
- * @threshold: trip crossing notification threshold miliCelsius
* @type: trip point type
* @priv: pointer to driver data associated with this trip
* @flags: flags representing binary properties of the trip
@@ -69,12 +68,16 @@ enum thermal_notify_event {
struct thermal_trip {
int temperature;
int hysteresis;
- int threshold;
enum thermal_trip_type type;
u8 flags;
void *priv;
};
+struct thermal_trip_desc {
+ struct thermal_trip trip;
+ int threshold;
+};
+
#define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
#define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
@@ -203,7 +206,7 @@ struct thermal_zone_device {
#ifdef CONFIG_THERMAL_DEBUGFS
struct thermal_debugfs *debugfs;
#endif
- struct thermal_trip trips[] __counted_by(num_trips);
+ struct thermal_trip_desc trips[] __counted_by(num_trips);
};
/**
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -361,17 +361,19 @@ static void handle_critical_trips(struct
}
static void handle_thermal_trip(struct thermal_zone_device *tz,
- struct thermal_trip *trip)
+ struct thermal_trip_desc *td)
{
+ const struct thermal_trip *trip = &td->trip;
+
if (trip->temperature == THERMAL_TEMP_INVALID)
return;
if (tz->last_temperature == THERMAL_TEMP_INVALID) {
/* Initialization. */
- trip->threshold = trip->temperature;
- if (tz->temperature >= trip->threshold)
- trip->threshold -= trip->hysteresis;
- } else if (tz->last_temperature < trip->threshold) {
+ td->threshold = trip->temperature;
+ if (tz->temperature >= td->threshold)
+ td->threshold -= trip->hysteresis;
+ } else if (tz->last_temperature < td->threshold) {
/*
* The trip threshold is equal to the trip temperature, unless
* the latter has changed in the meantime. In either case,
@@ -382,9 +384,9 @@ static void handle_thermal_trip(struct t
if (tz->temperature >= trip->temperature) {
thermal_notify_tz_trip_up(tz, trip);
thermal_debug_tz_trip_up(tz, trip);
- trip->threshold = trip->temperature - trip->hysteresis;
+ td->threshold = trip->temperature - trip->hysteresis;
} else {
- trip->threshold = trip->temperature;
+ td->threshold = trip->temperature;
}
} else {
/*
@@ -400,9 +402,9 @@ static void handle_thermal_trip(struct t
if (tz->temperature < trip->temperature - trip->hysteresis) {
thermal_notify_tz_trip_down(tz, trip);
thermal_debug_tz_trip_down(tz, trip);
- trip->threshold = trip->temperature;
+ td->threshold = trip->temperature;
} else {
- trip->threshold = trip->temperature - trip->hysteresis;
+ td->threshold = trip->temperature - trip->hysteresis;
}
}
@@ -458,7 +460,7 @@ static void thermal_zone_device_init(str
void __thermal_zone_device_update(struct thermal_zone_device *tz,
enum thermal_notify_event event)
{
- struct thermal_trip *trip;
+ struct thermal_trip_desc *td;
if (tz->suspended)
return;
@@ -472,8 +474,8 @@ void __thermal_zone_device_update(struct
tz->notify_event = event;
- for_each_trip(tz, trip)
- handle_thermal_trip(tz, trip);
+ for_each_trip_desc(tz, td)
+ handle_thermal_trip(tz, td);
monitor_thermal_zone(tz);
}
@@ -766,7 +768,7 @@ int thermal_zone_bind_cooling_device(str
if (trip_index < 0 || trip_index >= tz->num_trips)
return -EINVAL;
- return thermal_bind_cdev_to_trip(tz, &tz->trips[trip_index], cdev,
+ return thermal_bind_cdev_to_trip(tz, &tz->trips[trip_index].trip, cdev,
upper, lower, weight);
}
EXPORT_SYMBOL_GPL(thermal_zone_bind_cooling_device);
@@ -825,7 +827,7 @@ int thermal_zone_unbind_cooling_device(s
if (trip_index < 0 || trip_index >= tz->num_trips)
return -EINVAL;
- return thermal_unbind_cdev_from_trip(tz, &tz->trips[trip_index], cdev);
+ return thermal_unbind_cdev_from_trip(tz, &tz->trips[trip_index].trip, cdev);
}
EXPORT_SYMBOL_GPL(thermal_zone_unbind_cooling_device);
@@ -1221,16 +1223,19 @@ static void thermal_set_delay_jiffies(un
int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp)
{
- int i, ret = -EINVAL;
+ const struct thermal_trip_desc *td;
+ int ret = -EINVAL;
if (tz->ops.get_crit_temp)
return tz->ops.get_crit_temp(tz, temp);
mutex_lock(&tz->lock);
- for (i = 0; i < tz->num_trips; i++) {
- if (tz->trips[i].type == THERMAL_TRIP_CRITICAL) {
- *temp = tz->trips[i].temperature;
+ for_each_trip_desc(tz, td) {
+ const struct thermal_trip *trip = &td->trip;
+
+ if (trip->type == THERMAL_TRIP_CRITICAL) {
+ *temp = trip->temperature;
ret = 0;
break;
}
@@ -1274,7 +1279,9 @@ thermal_zone_device_register_with_trips(
const struct thermal_zone_params *tzp,
int passive_delay, int polling_delay)
{
+ const struct thermal_trip *trip = trips;
struct thermal_zone_device *tz;
+ struct thermal_trip_desc *td;
int id;
int result;
struct thermal_governor *governor;
@@ -1339,7 +1346,8 @@ thermal_zone_device_register_with_trips(
tz->device.class = thermal_class;
tz->devdata = devdata;
tz->num_trips = num_trips;
- memcpy(tz->trips, trips, num_trips * sizeof(*trips));
+ for_each_trip_desc(tz, td)
+ td->trip = *trip++;
thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -120,8 +120,11 @@ void thermal_governor_update_tz(struct t
enum thermal_notify_event reason);
/* Helpers */
-#define for_each_trip(__tz, __trip) \
- for (__trip = __tz->trips; __trip - __tz->trips < __tz->num_trips; __trip++)
+#define for_each_trip_desc(__tz, __td) \
+ for (__td = __tz->trips; __td - __tz->trips < __tz->num_trips; __td++)
+
+#define trip_to_trip_desc(__trip) \
+ container_of(__trip, struct thermal_trip_desc, trip)
void __thermal_zone_set_trips(struct thermal_zone_device *tz);
int thermal_zone_trip_id(const struct thermal_zone_device *tz,
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -88,7 +88,7 @@ trip_point_type_show(struct device *dev,
if (sscanf(attr->attr.name, "trip_point_%d_type", &trip_id) != 1)
return -EINVAL;
- switch (tz->trips[trip_id].type) {
+ switch (tz->trips[trip_id].trip.type) {
case THERMAL_TRIP_CRITICAL:
return sprintf(buf, "critical\n");
case THERMAL_TRIP_HOT:
@@ -120,7 +120,7 @@ trip_point_temp_store(struct device *dev
mutex_lock(&tz->lock);
- trip = &tz->trips[trip_id];
+ trip = &tz->trips[trip_id].trip;
if (temp != trip->temperature) {
if (tz->ops.set_trip_temp) {
@@ -150,7 +150,7 @@ trip_point_temp_show(struct device *dev,
if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1)
return -EINVAL;
- return sprintf(buf, "%d\n", tz->trips[trip_id].temperature);
+ return sprintf(buf, "%d\n", tz->trips[trip_id].trip.temperature);
}
static ssize_t
@@ -171,7 +171,7 @@ trip_point_hyst_store(struct device *dev
mutex_lock(&tz->lock);
- trip = &tz->trips[trip_id];
+ trip = &tz->trips[trip_id].trip;
if (hyst != trip->hysteresis) {
trip->hysteresis = hyst;
@@ -194,7 +194,7 @@ trip_point_hyst_show(struct device *dev,
if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1)
return -EINVAL;
- return sprintf(buf, "%d\n", tz->trips[trip_id].hysteresis);
+ return sprintf(buf, "%d\n", tz->trips[trip_id].trip.hysteresis);
}
static ssize_t
@@ -393,7 +393,7 @@ static const struct attribute_group *the
*/
static int create_trip_attrs(struct thermal_zone_device *tz)
{
- const struct thermal_trip *trip;
+ const struct thermal_trip_desc *td;
struct attribute **attrs;
/* This function works only for zones with at least one trip */
@@ -429,8 +429,8 @@ static int create_trip_attrs(struct ther
return -ENOMEM;
}
- for_each_trip(tz, trip) {
- int indx = thermal_zone_trip_id(tz, trip);
+ for_each_trip_desc(tz, td) {
+ int indx = thermal_zone_trip_id(tz, &td->trip);
/* create trip type attribute */
snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
@@ -452,7 +452,7 @@ static int create_trip_attrs(struct ther
tz->trip_temp_attrs[indx].name;
tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
- if (trip->flags & THERMAL_TRIP_FLAG_RW_TEMP) {
+ if (td->trip.flags & THERMAL_TRIP_FLAG_RW_TEMP) {
tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
tz->trip_temp_attrs[indx].attr.store =
trip_point_temp_store;
@@ -467,7 +467,7 @@ static int create_trip_attrs(struct ther
tz->trip_hyst_attrs[indx].name;
tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
- if (trip->flags & THERMAL_TRIP_FLAG_RW_HYST) {
+ if (td->trip.flags & THERMAL_TRIP_FLAG_RW_HYST) {
tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
tz->trip_hyst_attrs[indx].attr.store =
trip_point_hyst_store;
Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -744,7 +744,7 @@ static void tze_seq_stop(struct seq_file
static int tze_seq_show(struct seq_file *s, void *v)
{
struct thermal_zone_device *tz = s->private;
- struct thermal_trip *trip;
+ struct thermal_trip_desc *td;
struct tz_episode *tze;
const char *type;
int trip_id;
@@ -757,7 +757,9 @@ static int tze_seq_show(struct seq_file
seq_printf(s, "| trip | type | temp(°mC) | hyst(°mC) | duration | avg(°mC) | min(°mC) | max(°mC) |\n");
- for_each_trip(tz, trip) {
+ for_each_trip_desc(tz, td) {
+ const struct thermal_trip *trip = &td->trip;
+
/*
* There is no possible mitigation happening at the
* critical trip point, so the stats will be always
Index: linux-pm/drivers/thermal/thermal_netlink.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_netlink.c
+++ linux-pm/drivers/thermal/thermal_netlink.c
@@ -442,7 +442,7 @@ out_cancel_nest:
static int thermal_genl_cmd_tz_get_trip(struct param *p)
{
struct sk_buff *msg = p->msg;
- const struct thermal_trip *trip;
+ const struct thermal_trip_desc *td;
struct thermal_zone_device *tz;
struct nlattr *start_trip;
int id;
@@ -462,7 +462,9 @@ static int thermal_genl_cmd_tz_get_trip(
mutex_lock(&tz->lock);
- for_each_trip(tz, trip) {
+ for_each_trip_desc(tz, td) {
+ const struct thermal_trip *trip = &td->trip;
+
if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_ID,
thermal_zone_trip_id(tz, trip)) ||
nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, trip->type) ||
Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/drivers/thermal/thermal_trip.c
@@ -13,11 +13,11 @@ int for_each_thermal_trip(struct thermal
int (*cb)(struct thermal_trip *, void *),
void *data)
{
- struct thermal_trip *trip;
+ struct thermal_trip_desc *td;
int ret;
- for_each_trip(tz, trip) {
- ret = cb(trip, data);
+ for_each_trip_desc(tz, td) {
+ ret = cb(&td->trip, data);
if (ret)
return ret;
}
@@ -63,7 +63,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_num_t
*/
void __thermal_zone_set_trips(struct thermal_zone_device *tz)
{
- const struct thermal_trip *trip;
+ const struct thermal_trip_desc *td;
int low = -INT_MAX, high = INT_MAX;
int ret;
@@ -72,7 +72,8 @@ void __thermal_zone_set_trips(struct the
if (!tz->ops.set_trips)
return;
- for_each_trip(tz, trip) {
+ for_each_trip_desc(tz, td) {
+ const struct thermal_trip *trip = &td->trip;
int trip_low;
trip_low = trip->temperature - trip->hysteresis;
@@ -110,7 +111,7 @@ int __thermal_zone_get_trip(struct therm
if (!tz || trip_id < 0 || trip_id >= tz->num_trips || !trip)
return -EINVAL;
- *trip = tz->trips[trip_id];
+ *trip = tz->trips[trip_id].trip;
return 0;
}
EXPORT_SYMBOL_GPL(__thermal_zone_get_trip);
@@ -135,7 +136,7 @@ int thermal_zone_trip_id(const struct th
* Assume the trip to be located within the bounds of the thermal
* zone's trips[] table.
*/
- return trip - tz->trips;
+ return trip_to_trip_desc(trip) - tz->trips;
}
void thermal_zone_trip_updated(struct thermal_zone_device *tz,
const struct thermal_trip *trip)
Index: linux-pm/drivers/thermal/gov_fair_share.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_fair_share.c
+++ linux-pm/drivers/thermal/gov_fair_share.c
@@ -17,10 +17,13 @@
static int get_trip_level(struct thermal_zone_device *tz)
{
- const struct thermal_trip *trip, *level_trip = NULL;
+ const struct thermal_trip *level_trip = NULL;
+ const struct thermal_trip_desc *td;
int trip_level = -1;
- for_each_trip(tz, trip) {
+ for_each_trip_desc(tz, td) {
+ const struct thermal_trip *trip = &td->trip;
+
if (trip->temperature >= tz->temperature)
continue;
Index: linux-pm/drivers/thermal/gov_power_allocator.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_power_allocator.c
+++ linux-pm/drivers/thermal/gov_power_allocator.c
@@ -496,9 +496,11 @@ static void get_governor_trips(struct th
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;
+ const struct thermal_trip_desc *td;
+
+ for_each_trip_desc(tz, td) {
+ const struct thermal_trip *trip = &td->trip;
- for_each_trip(tz, trip) {
switch (trip->type) {
case THERMAL_TRIP_PASSIVE:
if (!first_passive) {
Index: linux-pm/drivers/thermal/thermal_helpers.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_helpers.c
+++ linux-pm/drivers/thermal/thermal_helpers.c
@@ -50,7 +50,7 @@ get_thermal_instance(struct thermal_zone
mutex_lock(&tz->lock);
mutex_lock(&cdev->lock);
- trip = &tz->trips[trip_index];
+ trip = &tz->trips[trip_index].trip;
list_for_each_entry(pos, &tz->thermal_instances, tz_node) {
if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
@@ -82,7 +82,7 @@ EXPORT_SYMBOL(get_thermal_instance);
*/
int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
{
- const struct thermal_trip *trip;
+ const struct thermal_trip_desc *td;
int crit_temp = INT_MAX;
int ret = -EINVAL;
@@ -91,7 +91,9 @@ int __thermal_zone_get_temp(struct therm
ret = tz->ops.get_temp(tz, temp);
if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
- for_each_trip(tz, trip) {
+ for_each_trip_desc(tz, td) {
+ const struct thermal_trip *trip = &td->trip;
+
if (trip->type == THERMAL_TRIP_CRITICAL) {
crit_temp = trip->temperature;
break;
From: Rafael J. Wysocki <[email protected]>
If a trip point is already exceeded by the zone temperature at the
initialization time, no trip crossing notification is send regarding
this even though mitigation should be started then.
Address this by rearranging the code in handle_thermal_trip() to
send a trip crossing notification for trip points already exceeded
by the zone temperature initially which also allows to reduce its
size by using the observation that the initialization and regular
trip crossing on the way up become the same case then.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v2 -> v3: New patch
---
drivers/thermal/thermal_core.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -364,6 +364,7 @@ static void handle_thermal_trip(struct t
struct thermal_trip_desc *td)
{
const struct thermal_trip *trip = &td->trip;
+ int old_threshold;
if (trip->temperature == THERMAL_TEMP_INVALID)
return;
@@ -375,25 +376,11 @@ static void handle_thermal_trip(struct t
* is what needs to be compared with the previous zone temperature
* to decide which action to take.
*/
- if (tz->last_temperature == THERMAL_TEMP_INVALID) {
- /* Initialization. */
- td->threshold = trip->temperature;
- if (tz->temperature >= td->threshold)
- td->threshold -= trip->hysteresis;
- } else if (tz->last_temperature < td->threshold) {
- /*
- * There is no mitigation under way, so it needs to be started
- * if the zone temperature exceeds the trip one. The new
- * threshold is then set to the low temperature of the trip.
- */
- if (tz->temperature >= trip->temperature) {
- thermal_notify_tz_trip_up(tz, trip);
- thermal_debug_tz_trip_up(tz, trip);
- td->threshold = trip->temperature - trip->hysteresis;
- } else {
- td->threshold = trip->temperature;
- }
- } else {
+ old_threshold = td->threshold;
+ td->threshold = trip->temperature;
+
+ if (tz->last_temperature >= old_threshold &&
+ tz->last_temperature != THERMAL_TEMP_INVALID) {
/*
* Mitigation is under way, so it needs to stop if the zone
* temperature falls below the low temperature of the trip.
@@ -402,10 +389,18 @@ static void handle_thermal_trip(struct t
if (tz->temperature < trip->temperature - trip->hysteresis) {
thermal_notify_tz_trip_down(tz, trip);
thermal_debug_tz_trip_down(tz, trip);
- td->threshold = trip->temperature;
} else {
- td->threshold = trip->temperature - trip->hysteresis;
+ td->threshold -= trip->hysteresis;
}
+ } else if (tz->temperature >= trip->temperature) {
+ /*
+ * There is no mitigation under way, so it needs to be started
+ * if the zone temperature exceeds the trip one. The new
+ * threshold is then set to the low temperature of the trip.
+ */
+ thermal_notify_tz_trip_up(tz, trip);
+ thermal_debug_tz_trip_up(tz, trip);
+ td->threshold -= trip->hysteresis;
}
if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT)
From: Rafael J. Wysocki <[email protected]>
Move the definitions of struct thermal_trip_desc and struct
thermal_zone_device to an internal header file in the thermal core,
as they don't need to be accessible to any code other than the thermal
core and so they don't need to be present in a global header.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v2 -> v3: Minor changelog update
v1 -> v2: No changes
---
drivers/thermal/thermal_core.h | 85 +++++++++++++++++++++++++++++++++++++++
drivers/thermal/thermal_trace.h | 2
include/linux/thermal.h | 87 ----------------------------------------
3 files changed, 89 insertions(+), 85 deletions(-)
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -15,6 +15,91 @@
#include "thermal_netlink.h"
#include "thermal_debugfs.h"
+struct thermal_trip_desc {
+ struct thermal_trip trip;
+ int threshold;
+};
+
+/**
+ * struct thermal_zone_device - structure for a thermal zone
+ * @id: unique id number for each thermal zone
+ * @type: the thermal zone device type
+ * @device: &struct device for this thermal zone
+ * @removal: removal completion
+ * @trip_temp_attrs: attributes for trip points for sysfs: trip temperature
+ * @trip_type_attrs: attributes for trip points for sysfs: trip type
+ * @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis
+ * @mode: current mode of this thermal zone
+ * @devdata: private pointer for device private data
+ * @num_trips: number of trip points the thermal zone supports
+ * @passive_delay_jiffies: number of jiffies to wait between polls when
+ * performing passive cooling.
+ * @polling_delay_jiffies: number of jiffies to wait between polls when
+ * checking whether trip points have been crossed (0 for
+ * interrupt driven systems)
+ * @temperature: current temperature. This is only for core code,
+ * drivers should use thermal_zone_get_temp() to get the
+ * current temperature
+ * @last_temperature: previous temperature read
+ * @emul_temperature: emulated temperature when using CONFIG_THERMAL_EMULATION
+ * @passive: 1 if you've crossed a passive trip point, 0 otherwise.
+ * @prev_low_trip: the low current temperature if you've crossed a passive
+ trip point.
+ * @prev_high_trip: the above current temperature if you've crossed a
+ passive trip point.
+ * @need_update: if equals 1, thermal_zone_device_update needs to be invoked.
+ * @ops: operations this &thermal_zone_device supports
+ * @tzp: thermal zone parameters
+ * @governor: pointer to the governor for this thermal zone
+ * @governor_data: private pointer for governor data
+ * @thermal_instances: list of &struct thermal_instance of this thermal zone
+ * @ida: &struct ida to generate unique id for this zone's cooling
+ * devices
+ * @lock: lock to protect thermal_instances list
+ * @node: node in thermal_tz_list (in thermal_core.c)
+ * @poll_queue: delayed work for polling
+ * @notify_event: Last notification event
+ * @suspended: thermal zone suspend indicator
+ * @trips: array of struct thermal_trip objects
+ */
+struct thermal_zone_device {
+ int id;
+ char type[THERMAL_NAME_LENGTH];
+ struct device device;
+ struct completion removal;
+ struct attribute_group trips_attribute_group;
+ struct thermal_attr *trip_temp_attrs;
+ struct thermal_attr *trip_type_attrs;
+ struct thermal_attr *trip_hyst_attrs;
+ enum thermal_device_mode mode;
+ void *devdata;
+ int num_trips;
+ unsigned long passive_delay_jiffies;
+ unsigned long polling_delay_jiffies;
+ int temperature;
+ int last_temperature;
+ int emul_temperature;
+ int passive;
+ int prev_low_trip;
+ int prev_high_trip;
+ atomic_t need_update;
+ struct thermal_zone_device_ops ops;
+ struct thermal_zone_params *tzp;
+ struct thermal_governor *governor;
+ void *governor_data;
+ struct list_head thermal_instances;
+ struct ida ida;
+ struct mutex lock;
+ struct list_head node;
+ struct delayed_work poll_queue;
+ enum thermal_notify_event notify_event;
+ bool suspended;
+#ifdef CONFIG_THERMAL_DEBUGFS
+ struct thermal_debugfs *debugfs;
+#endif
+ struct thermal_trip_desc trips[] __counted_by(num_trips);
+};
+
/* Default Thermal Governor */
#if defined(CONFIG_THERMAL_DEFAULT_GOV_STEP_WISE)
#define DEFAULT_THERMAL_GOVERNOR "step_wise"
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -73,17 +73,14 @@ struct thermal_trip {
void *priv;
};
-struct thermal_trip_desc {
- struct thermal_trip trip;
- int threshold;
-};
-
#define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
#define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
#define THERMAL_TRIP_FLAG_RW (THERMAL_TRIP_FLAG_RW_TEMP | \
THERMAL_TRIP_FLAG_RW_HYST)
+struct thermal_zone_device;
+
struct thermal_zone_device_ops {
int (*bind) (struct thermal_zone_device *,
struct thermal_cooling_device *);
@@ -130,86 +127,6 @@ struct thermal_cooling_device {
};
/**
- * struct thermal_zone_device - structure for a thermal zone
- * @id: unique id number for each thermal zone
- * @type: the thermal zone device type
- * @device: &struct device for this thermal zone
- * @removal: removal completion
- * @trip_temp_attrs: attributes for trip points for sysfs: trip temperature
- * @trip_type_attrs: attributes for trip points for sysfs: trip type
- * @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis
- * @mode: current mode of this thermal zone
- * @devdata: private pointer for device private data
- * @num_trips: number of trip points the thermal zone supports
- * @passive_delay_jiffies: number of jiffies to wait between polls when
- * performing passive cooling.
- * @polling_delay_jiffies: number of jiffies to wait between polls when
- * checking whether trip points have been crossed (0 for
- * interrupt driven systems)
- * @temperature: current temperature. This is only for core code,
- * drivers should use thermal_zone_get_temp() to get the
- * current temperature
- * @last_temperature: previous temperature read
- * @emul_temperature: emulated temperature when using CONFIG_THERMAL_EMULATION
- * @passive: 1 if you've crossed a passive trip point, 0 otherwise.
- * @prev_low_trip: the low current temperature if you've crossed a passive
- trip point.
- * @prev_high_trip: the above current temperature if you've crossed a
- passive trip point.
- * @need_update: if equals 1, thermal_zone_device_update needs to be invoked.
- * @ops: operations this &thermal_zone_device supports
- * @tzp: thermal zone parameters
- * @governor: pointer to the governor for this thermal zone
- * @governor_data: private pointer for governor data
- * @thermal_instances: list of &struct thermal_instance of this thermal zone
- * @ida: &struct ida to generate unique id for this zone's cooling
- * devices
- * @lock: lock to protect thermal_instances list
- * @node: node in thermal_tz_list (in thermal_core.c)
- * @poll_queue: delayed work for polling
- * @notify_event: Last notification event
- * @suspended: thermal zone suspend indicator
- * @trips: array of struct thermal_trip objects
- */
-struct thermal_zone_device {
- int id;
- char type[THERMAL_NAME_LENGTH];
- struct device device;
- struct completion removal;
- struct attribute_group trips_attribute_group;
- struct thermal_attr *trip_temp_attrs;
- struct thermal_attr *trip_type_attrs;
- struct thermal_attr *trip_hyst_attrs;
- enum thermal_device_mode mode;
- void *devdata;
- int num_trips;
- unsigned long passive_delay_jiffies;
- unsigned long polling_delay_jiffies;
- int temperature;
- int last_temperature;
- int emul_temperature;
- int passive;
- int prev_low_trip;
- int prev_high_trip;
- atomic_t need_update;
- struct thermal_zone_device_ops ops;
- struct thermal_zone_params *tzp;
- struct thermal_governor *governor;
- void *governor_data;
- struct list_head thermal_instances;
- struct ida ida;
- struct mutex lock;
- struct list_head node;
- struct delayed_work poll_queue;
- enum thermal_notify_event notify_event;
- bool suspended;
-#ifdef CONFIG_THERMAL_DEBUGFS
- struct thermal_debugfs *debugfs;
-#endif
- struct thermal_trip_desc trips[] __counted_by(num_trips);
-};
-
-/**
* struct thermal_governor - structure that holds thermal governor information
* @name: name of the governor
* @bind_to_tz: callback called when binding to a thermal zone. If it
Index: linux-pm/drivers/thermal/thermal_trace.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trace.h
+++ linux-pm/drivers/thermal/thermal_trace.h
@@ -9,6 +9,8 @@
#include <linux/thermal.h>
#include <linux/tracepoint.h>
+#include "thermal_core.h"
+
TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL);
TRACE_DEFINE_ENUM(THERMAL_TRIP_HOT);
TRACE_DEFINE_ENUM(THERMAL_TRIP_PASSIVE);
From: Rafael J. Wysocki <[email protected]>
Make the comments regarding trip crossing and threshold updates in
handle_thermal_trip() slightly more clear.
No functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v2 -> v3: New patch
---
drivers/thermal/thermal_core.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -368,6 +368,13 @@ static void handle_thermal_trip(struct t
if (trip->temperature == THERMAL_TEMP_INVALID)
return;
+ /*
+ * If the trip temperature or hysteresis has been updated recently,
+ * the threshold needs to be computed again using the new values.
+ * However, its initial value still reflects the old ones and that
+ * is what needs to be compared with the previous zone temperature
+ * to decide which action to take.
+ */
if (tz->last_temperature == THERMAL_TEMP_INVALID) {
/* Initialization. */
td->threshold = trip->temperature;
@@ -375,11 +382,9 @@ static void handle_thermal_trip(struct t
td->threshold -= trip->hysteresis;
} else if (tz->last_temperature < td->threshold) {
/*
- * The trip threshold is equal to the trip temperature, unless
- * the latter has changed in the meantime. In either case,
- * the trip is crossed if the current zone temperature is at
- * least equal to its temperature, but otherwise ensure that
- * the threshold and the trip temperature will be equal.
+ * There is no mitigation under way, so it needs to be started
+ * if the zone temperature exceeds the trip one. The new
+ * threshold is then set to the low temperature of the trip.
*/
if (tz->temperature >= trip->temperature) {
thermal_notify_tz_trip_up(tz, trip);
@@ -390,14 +395,9 @@ static void handle_thermal_trip(struct t
}
} else {
/*
- * The previous zone temperature was above or equal to the trip
- * threshold, which would be equal to the "low temperature" of
- * the trip (its temperature minus its hysteresis), unless the
- * trip temperature or hysteresis had changed. In either case,
- * the trip is crossed if the current zone temperature is below
- * the low temperature of the trip, but otherwise ensure that
- * the trip threshold will be equal to the low temperature of
- * the trip.
+ * Mitigation is under way, so it needs to stop if the zone
+ * temperature falls below the low temperature of the trip.
+ * In that case, the trip temperature becomes the new threshold.
*/
if (tz->temperature < trip->temperature - trip->hysteresis) {
thermal_notify_tz_trip_down(tz, trip);
On Tue, Apr 2, 2024 at 9:04 PM Rafael J. Wysocki <[email protected]> wrote:
>
> Hi Everyone,
>
> This is an update of
>
> https://lore.kernel.org/linux-pm/4558384.LvFx2qVVIh@kreacher/
>
> and
>
> https://lore.kernel.org/linux-pm/2331888.ElGaqSPkdT@kreacher/
>
> which rebases the first patch on top of 6.9-rc2, adds 3 patches and adjusts
> the third patch from v2.
>
> The original description of the first two patches still applies:
>
> > Patch [1/2] is based on the observation that the threshold field in struct
> > thermal_trip really should be core-internal and to make that happen it
> > introduces a wrapper structure around struct thermal_trip for internal
> > use in the core.
> >
> > Patch [2/2] moves the definition of the new structure and the struct
> > thermal_zone_device one to a local header file in the core to enforce
> > more separation between the core and drivers.
> >
> > The patches are not expected to introduce any observable differences in
> > behavior, so please let me know if you see any of that.
>
> Note that these patches were first sent before the merge window and have not
> really changed since then (except for a minor rebase of the first patch in
> this series). Moreover, no comments regarding the merit of these patches
> have been made shared, so if this continues, I will be considering them as
> good to go by the end of this week.
>
> Patch [3/6] is a rewrite of comments regarding trip crossing and threshold
> computations.
>
> Patch [4/6] updates the trip crossing detection code to consolidate the
> threshold initialization with trip crossing on the way up.
>
> Patch [5/6] ([3/3] in v2) adds a mechanism to sort notifications and debug
> calls taking place during one invocation of __thermal_zone_device_update() so
> they always go in temperature order.
>
> Patch [6/6] relocates the critical and trip point handling to avoid a
> redundant temperature check.
>
> The series applies on top of 6.9-rc2 and I'm planning to create a test
> branch containing it.
As promised:
https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=thermal-core-testing
On 4/2/24 20:42, Rafael J. Wysocki wrote:
> On Tue, Apr 2, 2024 at 9:04 PM Rafael J. Wysocki <[email protected]> wrote:
>>
>> Hi Everyone,
>>
>> This is an update of
>>
>> https://lore.kernel.org/linux-pm/4558384.LvFx2qVVIh@kreacher/
>>
>> and
>>
>> https://lore.kernel.org/linux-pm/2331888.ElGaqSPkdT@kreacher/
>>
>> which rebases the first patch on top of 6.9-rc2, adds 3 patches and adjusts
>> the third patch from v2.
>>
>> The original description of the first two patches still applies:
>>
>>> Patch [1/2] is based on the observation that the threshold field in struct
>>> thermal_trip really should be core-internal and to make that happen it
>>> introduces a wrapper structure around struct thermal_trip for internal
>>> use in the core.
>>>
>>> Patch [2/2] moves the definition of the new structure and the struct
>>> thermal_zone_device one to a local header file in the core to enforce
>>> more separation between the core and drivers.
>>>
>>> The patches are not expected to introduce any observable differences in
>>> behavior, so please let me know if you see any of that.
>>
>> Note that these patches were first sent before the merge window and have not
>> really changed since then (except for a minor rebase of the first patch in
>> this series). Moreover, no comments regarding the merit of these patches
>> have been made shared, so if this continues, I will be considering them as
>> good to go by the end of this week.
>>
>> Patch [3/6] is a rewrite of comments regarding trip crossing and threshold
>> computations.
>>
>> Patch [4/6] updates the trip crossing detection code to consolidate the
>> threshold initialization with trip crossing on the way up.
>>
>> Patch [5/6] ([3/3] in v2) adds a mechanism to sort notifications and debug
>> calls taking place during one invocation of __thermal_zone_device_update() so
>> they always go in temperature order.
>>
>> Patch [6/6] relocates the critical and trip point handling to avoid a
>> redundant temperature check.
>>
>> The series applies on top of 6.9-rc2 and I'm planning to create a test
>> branch containing it.
>
> As promised:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=thermal-core-testing
Thanks Rafael for the handy branch. I'll play with it today.
Regards,
Lukasz
On 4/2/24 19:56, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The threshold field in struct thermal_trip is only used internally by
> the thermal core and it is better to prevent drivers from misusing it.
> It also takes some space unnecessarily in the trip tables passed by
> drivers to the core during thermal zone registration.
>
> For this reason, introduce struct thermal_trip_desc as a wrapper around
> struct thermal_trip, move the threshold field directly into it and make
> the thermal core store struct thermal_trip_desc objects in the internal
> thermal zone trip tables. Adjust all of the code using trip tables in
> the thermal core accordingly.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> v2 -> v3: Rebase on top of 6.9-rc2, minor changelog update.
>
> v1 -> v2: No changes.
>
> ---
> drivers/thermal/gov_fair_share.c | 7 +++--
> drivers/thermal/gov_power_allocator.c | 6 ++--
> drivers/thermal/thermal_core.c | 46 +++++++++++++++++++---------------
> drivers/thermal/thermal_core.h | 7 +++--
> drivers/thermal/thermal_debugfs.c | 6 ++--
> drivers/thermal/thermal_helpers.c | 8 +++--
> drivers/thermal/thermal_netlink.c | 6 ++--
> drivers/thermal/thermal_sysfs.c | 20 +++++++-------
> drivers/thermal/thermal_trip.c | 15 +++++------
> include/linux/thermal.h | 9 ++++--
> 10 files changed, 78 insertions(+), 52 deletions(-)
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -61,7 +61,6 @@ enum thermal_notify_event {
> * struct thermal_trip - representation of a point in temperature domain
> * @temperature: temperature value in miliCelsius
> * @hysteresis: relative hysteresis in miliCelsius
> - * @threshold: trip crossing notification threshold miliCelsius
> * @type: trip point type
> * @priv: pointer to driver data associated with this trip
> * @flags: flags representing binary properties of the trip
> @@ -69,12 +68,16 @@ enum thermal_notify_event {
> struct thermal_trip {
> int temperature;
> int hysteresis;
> - int threshold;
> enum thermal_trip_type type;
> u8 flags;
> void *priv;
> };
>
> +struct thermal_trip_desc {
> + struct thermal_trip trip;
> + int threshold;
> +};
> +
> #define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
> #define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
>
> @@ -203,7 +206,7 @@ struct thermal_zone_device {
> #ifdef CONFIG_THERMAL_DEBUGFS
> struct thermal_debugfs *debugfs;
> #endif
> - struct thermal_trip trips[] __counted_by(num_trips);
> + struct thermal_trip_desc trips[] __counted_by(num_trips);
> };
>
> /**
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -361,17 +361,19 @@ static void handle_critical_trips(struct
> }
>
> static void handle_thermal_trip(struct thermal_zone_device *tz,
> - struct thermal_trip *trip)
> + struct thermal_trip_desc *td)
> {
> + const struct thermal_trip *trip = &td->trip;
> +
> if (trip->temperature == THERMAL_TEMP_INVALID)
> return;
>
> if (tz->last_temperature == THERMAL_TEMP_INVALID) {
> /* Initialization. */
> - trip->threshold = trip->temperature;
> - if (tz->temperature >= trip->threshold)
> - trip->threshold -= trip->hysteresis;
> - } else if (tz->last_temperature < trip->threshold) {
> + td->threshold = trip->temperature;
> + if (tz->temperature >= td->threshold)
> + td->threshold -= trip->hysteresis;
> + } else if (tz->last_temperature < td->threshold) {
> /*
> * The trip threshold is equal to the trip temperature, unless
> * the latter has changed in the meantime. In either case,
> @@ -382,9 +384,9 @@ static void handle_thermal_trip(struct t
> if (tz->temperature >= trip->temperature) {
> thermal_notify_tz_trip_up(tz, trip);
> thermal_debug_tz_trip_up(tz, trip);
> - trip->threshold = trip->temperature - trip->hysteresis;
> + td->threshold = trip->temperature - trip->hysteresis;
> } else {
> - trip->threshold = trip->temperature;
> + td->threshold = trip->temperature;
> }
> } else {
> /*
> @@ -400,9 +402,9 @@ static void handle_thermal_trip(struct t
> if (tz->temperature < trip->temperature - trip->hysteresis) {
> thermal_notify_tz_trip_down(tz, trip);
> thermal_debug_tz_trip_down(tz, trip);
> - trip->threshold = trip->temperature;
> + td->threshold = trip->temperature;
> } else {
> - trip->threshold = trip->temperature - trip->hysteresis;
> + td->threshold = trip->temperature - trip->hysteresis;
> }
> }
>
> @@ -458,7 +460,7 @@ static void thermal_zone_device_init(str
> void __thermal_zone_device_update(struct thermal_zone_device *tz,
> enum thermal_notify_event event)
> {
> - struct thermal_trip *trip;
> + struct thermal_trip_desc *td;
>
> if (tz->suspended)
> return;
> @@ -472,8 +474,8 @@ void __thermal_zone_device_update(struct
>
> tz->notify_event = event;
>
> - for_each_trip(tz, trip)
> - handle_thermal_trip(tz, trip);
> + for_each_trip_desc(tz, td)
> + handle_thermal_trip(tz, td);
>
> monitor_thermal_zone(tz);
> }
> @@ -766,7 +768,7 @@ int thermal_zone_bind_cooling_device(str
> if (trip_index < 0 || trip_index >= tz->num_trips)
> return -EINVAL;
>
> - return thermal_bind_cdev_to_trip(tz, &tz->trips[trip_index], cdev,
> + return thermal_bind_cdev_to_trip(tz, &tz->trips[trip_index].trip, cdev,
> upper, lower, weight);
> }
> EXPORT_SYMBOL_GPL(thermal_zone_bind_cooling_device);
> @@ -825,7 +827,7 @@ int thermal_zone_unbind_cooling_device(s
> if (trip_index < 0 || trip_index >= tz->num_trips)
> return -EINVAL;
>
> - return thermal_unbind_cdev_from_trip(tz, &tz->trips[trip_index], cdev);
> + return thermal_unbind_cdev_from_trip(tz, &tz->trips[trip_index].trip, cdev);
> }
> EXPORT_SYMBOL_GPL(thermal_zone_unbind_cooling_device);
>
> @@ -1221,16 +1223,19 @@ static void thermal_set_delay_jiffies(un
>
> int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp)
> {
> - int i, ret = -EINVAL;
> + const struct thermal_trip_desc *td;
> + int ret = -EINVAL;
>
> if (tz->ops.get_crit_temp)
> return tz->ops.get_crit_temp(tz, temp);
>
> mutex_lock(&tz->lock);
>
> - for (i = 0; i < tz->num_trips; i++) {
> - if (tz->trips[i].type == THERMAL_TRIP_CRITICAL) {
> - *temp = tz->trips[i].temperature;
> + for_each_trip_desc(tz, td) {
> + const struct thermal_trip *trip = &td->trip;
> +
> + if (trip->type == THERMAL_TRIP_CRITICAL) {
> + *temp = trip->temperature;
> ret = 0;
> break;
> }
> @@ -1274,7 +1279,9 @@ thermal_zone_device_register_with_trips(
> const struct thermal_zone_params *tzp,
> int passive_delay, int polling_delay)
> {
> + const struct thermal_trip *trip = trips;
> struct thermal_zone_device *tz;
> + struct thermal_trip_desc *td;
> int id;
> int result;
> struct thermal_governor *governor;
> @@ -1339,7 +1346,8 @@ thermal_zone_device_register_with_trips(
> tz->device.class = thermal_class;
> tz->devdata = devdata;
> tz->num_trips = num_trips;
> - memcpy(tz->trips, trips, num_trips * sizeof(*trips));
> + for_each_trip_desc(tz, td)
> + td->trip = *trip++;
>
> thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
> thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -120,8 +120,11 @@ void thermal_governor_update_tz(struct t
> enum thermal_notify_event reason);
>
> /* Helpers */
> -#define for_each_trip(__tz, __trip) \
> - for (__trip = __tz->trips; __trip - __tz->trips < __tz->num_trips; __trip++)
> +#define for_each_trip_desc(__tz, __td) \
> + for (__td = __tz->trips; __td - __tz->trips < __tz->num_trips; __td++)
> +
> +#define trip_to_trip_desc(__trip) \
> + container_of(__trip, struct thermal_trip_desc, trip)
>
> void __thermal_zone_set_trips(struct thermal_zone_device *tz);
> int thermal_zone_trip_id(const struct thermal_zone_device *tz,
> Index: linux-pm/drivers/thermal/thermal_sysfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> +++ linux-pm/drivers/thermal/thermal_sysfs.c
> @@ -88,7 +88,7 @@ trip_point_type_show(struct device *dev,
> if (sscanf(attr->attr.name, "trip_point_%d_type", &trip_id) != 1)
> return -EINVAL;
>
> - switch (tz->trips[trip_id].type) {
> + switch (tz->trips[trip_id].trip.type) {
This could be a bit shorter, with some helper variable, but that's
minor cosmetic.
> case THERMAL_TRIP_CRITICAL:
> return sprintf(buf, "critical\n");
> case THERMAL_TRIP_HOT:
> @@ -120,7 +120,7 @@ trip_point_temp_store(struct device *dev
>
> mutex_lock(&tz->lock);
>
> - trip = &tz->trips[trip_id];
> + trip = &tz->trips[trip_id].trip;
>
> if (temp != trip->temperature) {
> if (tz->ops.set_trip_temp) {
> @@ -150,7 +150,7 @@ trip_point_temp_show(struct device *dev,
> if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1)
> return -EINVAL;
>
> - return sprintf(buf, "%d\n", tz->trips[trip_id].temperature);
> + return sprintf(buf, "%d\n", tz->trips[trip_id].trip.temperature);
> }
>
> static ssize_t
> @@ -171,7 +171,7 @@ trip_point_hyst_store(struct device *dev
>
> mutex_lock(&tz->lock);
>
> - trip = &tz->trips[trip_id];
> + trip = &tz->trips[trip_id].trip;
>
> if (hyst != trip->hysteresis) {
> trip->hysteresis = hyst;
> @@ -194,7 +194,7 @@ trip_point_hyst_show(struct device *dev,
> if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1)
> return -EINVAL;
>
> - return sprintf(buf, "%d\n", tz->trips[trip_id].hysteresis);
> + return sprintf(buf, "%d\n", tz->trips[trip_id].trip.hysteresis);
> }
>
> static ssize_t
> @@ -393,7 +393,7 @@ static const struct attribute_group *the
> */
> static int create_trip_attrs(struct thermal_zone_device *tz)
> {
> - const struct thermal_trip *trip;
> + const struct thermal_trip_desc *td;
> struct attribute **attrs;
>
> /* This function works only for zones with at least one trip */
> @@ -429,8 +429,8 @@ static int create_trip_attrs(struct ther
> return -ENOMEM;
> }
>
> - for_each_trip(tz, trip) {
> - int indx = thermal_zone_trip_id(tz, trip);
> + for_each_trip_desc(tz, td) {
> + int indx = thermal_zone_trip_id(tz, &td->trip);
>
> /* create trip type attribute */
> snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
> @@ -452,7 +452,7 @@ static int create_trip_attrs(struct ther
> tz->trip_temp_attrs[indx].name;
> tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
> tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
> - if (trip->flags & THERMAL_TRIP_FLAG_RW_TEMP) {
> + if (td->trip.flags & THERMAL_TRIP_FLAG_RW_TEMP) {
> tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
> tz->trip_temp_attrs[indx].attr.store =
> trip_point_temp_store;
> @@ -467,7 +467,7 @@ static int create_trip_attrs(struct ther
> tz->trip_hyst_attrs[indx].name;
> tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
> tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
> - if (trip->flags & THERMAL_TRIP_FLAG_RW_HYST) {
> + if (td->trip.flags & THERMAL_TRIP_FLAG_RW_HYST) {
> tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
> tz->trip_hyst_attrs[indx].attr.store =
> trip_point_hyst_store;
> Index: linux-pm/drivers/thermal/thermal_debugfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_debugfs.c
> +++ linux-pm/drivers/thermal/thermal_debugfs.c
> @@ -744,7 +744,7 @@ static void tze_seq_stop(struct seq_file
> static int tze_seq_show(struct seq_file *s, void *v)
> {
> struct thermal_zone_device *tz = s->private;
> - struct thermal_trip *trip;
> + struct thermal_trip_desc *td;
> struct tz_episode *tze;
> const char *type;
> int trip_id;
> @@ -757,7 +757,9 @@ static int tze_seq_show(struct seq_file
>
> seq_printf(s, "| trip | type | temp(°mC) | hyst(°mC) | duration | avg(°mC) | min(°mC) | max(°mC) |\n");
>
> - for_each_trip(tz, trip) {
> + for_each_trip_desc(tz, td) {
> + const struct thermal_trip *trip = &td->trip;
> +
> /*
> * There is no possible mitigation happening at the
> * critical trip point, so the stats will be always
> Index: linux-pm/drivers/thermal/thermal_netlink.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_netlink.c
> +++ linux-pm/drivers/thermal/thermal_netlink.c
> @@ -442,7 +442,7 @@ out_cancel_nest:
> static int thermal_genl_cmd_tz_get_trip(struct param *p)
> {
> struct sk_buff *msg = p->msg;
> - const struct thermal_trip *trip;
> + const struct thermal_trip_desc *td;
> struct thermal_zone_device *tz;
> struct nlattr *start_trip;
> int id;
> @@ -462,7 +462,9 @@ static int thermal_genl_cmd_tz_get_trip(
>
> mutex_lock(&tz->lock);
>
> - for_each_trip(tz, trip) {
> + for_each_trip_desc(tz, td) {
> + const struct thermal_trip *trip = &td->trip;
> +
> if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_ID,
> thermal_zone_trip_id(tz, trip)) ||
> nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, trip->type) ||
> Index: linux-pm/drivers/thermal/thermal_trip.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_trip.c
> +++ linux-pm/drivers/thermal/thermal_trip.c
> @@ -13,11 +13,11 @@ int for_each_thermal_trip(struct thermal
> int (*cb)(struct thermal_trip *, void *),
> void *data)
> {
> - struct thermal_trip *trip;
> + struct thermal_trip_desc *td;
> int ret;
>
> - for_each_trip(tz, trip) {
> - ret = cb(trip, data);
> + for_each_trip_desc(tz, td) {
> + ret = cb(&td->trip, data);
> if (ret)
> return ret;
> }
> @@ -63,7 +63,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_num_t
> */
> void __thermal_zone_set_trips(struct thermal_zone_device *tz)
> {
> - const struct thermal_trip *trip;
> + const struct thermal_trip_desc *td;
> int low = -INT_MAX, high = INT_MAX;
> int ret;
>
> @@ -72,7 +72,8 @@ void __thermal_zone_set_trips(struct the
> if (!tz->ops.set_trips)
> return;
>
> - for_each_trip(tz, trip) {
> + for_each_trip_desc(tz, td) {
> + const struct thermal_trip *trip = &td->trip;
> int trip_low;
>
> trip_low = trip->temperature - trip->hysteresis;
> @@ -110,7 +111,7 @@ int __thermal_zone_get_trip(struct therm
> if (!tz || trip_id < 0 || trip_id >= tz->num_trips || !trip)
> return -EINVAL;
>
> - *trip = tz->trips[trip_id];
> + *trip = tz->trips[trip_id].trip;
> return 0;
> }
> EXPORT_SYMBOL_GPL(__thermal_zone_get_trip);
> @@ -135,7 +136,7 @@ int thermal_zone_trip_id(const struct th
> * Assume the trip to be located within the bounds of the thermal
> * zone's trips[] table.
> */
> - return trip - tz->trips;
> + return trip_to_trip_desc(trip) - tz->trips;
> }
> void thermal_zone_trip_updated(struct thermal_zone_device *tz,
> const struct thermal_trip *trip)
> Index: linux-pm/drivers/thermal/gov_fair_share.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_fair_share.c
> +++ linux-pm/drivers/thermal/gov_fair_share.c
> @@ -17,10 +17,13 @@
>
> static int get_trip_level(struct thermal_zone_device *tz)
> {
> - const struct thermal_trip *trip, *level_trip = NULL;
> + const struct thermal_trip *level_trip = NULL;
> + const struct thermal_trip_desc *td;
> int trip_level = -1;
>
> - for_each_trip(tz, trip) {
> + for_each_trip_desc(tz, td) {
> + const struct thermal_trip *trip = &td->trip;
> +
> if (trip->temperature >= tz->temperature)
> continue;
>
> Index: linux-pm/drivers/thermal/gov_power_allocator.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_power_allocator.c
> +++ linux-pm/drivers/thermal/gov_power_allocator.c
> @@ -496,9 +496,11 @@ static void get_governor_trips(struct th
> 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;
> + const struct thermal_trip_desc *td;
> +
> + for_each_trip_desc(tz, td) {
> + const struct thermal_trip *trip = &td->trip;
>
> - for_each_trip(tz, trip) {
> switch (trip->type) {
> case THERMAL_TRIP_PASSIVE:
> if (!first_passive) {
> Index: linux-pm/drivers/thermal/thermal_helpers.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_helpers.c
> +++ linux-pm/drivers/thermal/thermal_helpers.c
> @@ -50,7 +50,7 @@ get_thermal_instance(struct thermal_zone
> mutex_lock(&tz->lock);
> mutex_lock(&cdev->lock);
>
> - trip = &tz->trips[trip_index];
> + trip = &tz->trips[trip_index].trip;
>
> list_for_each_entry(pos, &tz->thermal_instances, tz_node) {
> if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
> @@ -82,7 +82,7 @@ EXPORT_SYMBOL(get_thermal_instance);
> */
> int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
> {
> - const struct thermal_trip *trip;
> + const struct thermal_trip_desc *td;
> int crit_temp = INT_MAX;
> int ret = -EINVAL;
>
> @@ -91,7 +91,9 @@ int __thermal_zone_get_temp(struct therm
> ret = tz->ops.get_temp(tz, temp);
>
> if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
> - for_each_trip(tz, trip) {
> + for_each_trip_desc(tz, td) {
> + const struct thermal_trip *trip = &td->trip;
> +
> if (trip->type == THERMAL_TRIP_CRITICAL) {
> crit_temp = trip->temperature;
> break;
>
>
>
LGTM,
Reviewed-by: Lukasz Luba <[email protected]>
On 4/2/24 19:57, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Move the definitions of struct thermal_trip_desc and struct
> thermal_zone_device to an internal header file in the thermal core,
> as they don't need to be accessible to any code other than the thermal
> core and so they don't need to be present in a global header.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> v2 -> v3: Minor changelog update
>
> v1 -> v2: No changes
>
> ---
> drivers/thermal/thermal_core.h | 85 +++++++++++++++++++++++++++++++++++++++
> drivers/thermal/thermal_trace.h | 2
> include/linux/thermal.h | 87 ----------------------------------------
> 3 files changed, 89 insertions(+), 85 deletions(-)
>
Reviewed-by: Lukasz Luba <[email protected]>
On 4/2/24 19:59, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Make the comments regarding trip crossing and threshold updates in
> handle_thermal_trip() slightly more clear.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> v2 -> v3: New patch
>
> ---
> drivers/thermal/thermal_core.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -368,6 +368,13 @@ static void handle_thermal_trip(struct t
> if (trip->temperature == THERMAL_TEMP_INVALID)
> return;
>
> + /*
> + * If the trip temperature or hysteresis has been updated recently,
> + * the threshold needs to be computed again using the new values.
> + * However, its initial value still reflects the old ones and that
> + * is what needs to be compared with the previous zone temperature
> + * to decide which action to take.
> + */
> if (tz->last_temperature == THERMAL_TEMP_INVALID) {
> /* Initialization. */
> td->threshold = trip->temperature;
> @@ -375,11 +382,9 @@ static void handle_thermal_trip(struct t
> td->threshold -= trip->hysteresis;
> } else if (tz->last_temperature < td->threshold) {
> /*
> - * The trip threshold is equal to the trip temperature, unless
> - * the latter has changed in the meantime. In either case,
> - * the trip is crossed if the current zone temperature is at
> - * least equal to its temperature, but otherwise ensure that
> - * the threshold and the trip temperature will be equal.
> + * There is no mitigation under way, so it needs to be started
> + * if the zone temperature exceeds the trip one. The new
> + * threshold is then set to the low temperature of the trip.
> */
> if (tz->temperature >= trip->temperature) {
> thermal_notify_tz_trip_up(tz, trip);
> @@ -390,14 +395,9 @@ static void handle_thermal_trip(struct t
> }
> } else {
> /*
> - * The previous zone temperature was above or equal to the trip
> - * threshold, which would be equal to the "low temperature" of
> - * the trip (its temperature minus its hysteresis), unless the
> - * trip temperature or hysteresis had changed. In either case,
> - * the trip is crossed if the current zone temperature is below
> - * the low temperature of the trip, but otherwise ensure that
> - * the trip threshold will be equal to the low temperature of
> - * the trip.
> + * Mitigation is under way, so it needs to stop if the zone
> + * temperature falls below the low temperature of the trip.
> + * In that case, the trip temperature becomes the new threshold.
> */
> if (tz->temperature < trip->temperature - trip->hysteresis) {
> thermal_notify_tz_trip_down(tz, trip);
>
>
>
Reviewed-by: Lukasz Luba <[email protected]>
On 4/2/24 20:03, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> If multiple trip points are crossed in one go and the trips table in
> the thermal zone device object is not sorted, the corresponding trip
> point crossing notifications sent to user space will not be ordered
> either.
>
> Moreover, if the trips table is sorted by trip temperature in ascending
> order, the trip crossing notifications on the way up will be sent in that
> order too, but the trip crossing notifications on the way down will be
> sent in the reverse order.
>
> This is generally confusing and it is better to make the kernel send the
> notifications in the order of growing (on the way up) or falling (on the
> way down) trip temperature.
>
> To achieve that, instead of sending a trip crossing notification and
> recording a trip crossing event in the statistics right away from
> handle_thermal_trip(), put the trip in question on a list that will be
> sorted by __thermal_zone_device_update() after processing all of the trips
> and before sending the notifications and recording trip crossing events.
>
> Link: https://lore.kernel.org/linux-pm/[email protected]/
> Reported-by: Daniel Lezcano <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> v2 -> v3: Rebase, update changelog, add notify_temp to struct thermal_trip_desc
>
> v1 -> v2: New patch
>
> ---
> drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++------
> drivers/thermal/thermal_core.h | 2 ++
> 2 files changed, 37 insertions(+), 6 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -15,6 +15,7 @@
> #include <linux/slab.h>
> #include <linux/kdev_t.h>
> #include <linux/idr.h>
> +#include <linux/list_sort.h>
> #include <linux/thermal.h>
> #include <linux/reboot.h>
> #include <linux/string.h>
> @@ -361,7 +362,9 @@ static void handle_critical_trips(struct
> }
>
> static void handle_thermal_trip(struct thermal_zone_device *tz,
> - struct thermal_trip_desc *td)
> + struct thermal_trip_desc *td,
> + struct list_head *way_up_list,
> + struct list_head *way_down_list)
> {
> const struct thermal_trip *trip = &td->trip;
> int old_threshold;
> @@ -387,8 +390,8 @@ static void handle_thermal_trip(struct t
> * In that case, the trip temperature becomes the new threshold.
> */
> if (tz->temperature < trip->temperature - trip->hysteresis) {
> - thermal_notify_tz_trip_down(tz, trip);
> - thermal_debug_tz_trip_down(tz, trip);
> + list_add(&td->notify_list_node, way_down_list);
> + td->notify_temp = trip->temperature - trip->hysteresis;
> } else {
> td->threshold -= trip->hysteresis;
> }
> @@ -398,8 +401,8 @@ static void handle_thermal_trip(struct t
> * if the zone temperature exceeds the trip one. The new
> * threshold is then set to the low temperature of the trip.
> */
> - thermal_notify_tz_trip_up(tz, trip);
> - thermal_debug_tz_trip_up(tz, trip);
> + list_add_tail(&td->notify_list_node, way_up_list);
> + td->notify_temp = trip->temperature;
> td->threshold -= trip->hysteresis;
> }
>
> @@ -452,10 +455,24 @@ static void thermal_zone_device_init(str
> pos->initialized = false;
> }
>
> +static int thermal_trip_notify_cmp(void *ascending, const struct list_head *a,
> + const struct list_head *b)
> +{
> + struct thermal_trip_desc *tda = container_of(a, struct thermal_trip_desc,
> + notify_list_node);
> + struct thermal_trip_desc *tdb = container_of(b, struct thermal_trip_desc,
> + notify_list_node);
> + int ret = tdb->notify_temp - tda->notify_temp;
> +
> + return ascending ? ret : -ret;
> +}
> +
> void __thermal_zone_device_update(struct thermal_zone_device *tz,
> enum thermal_notify_event event)
> {
> struct thermal_trip_desc *td;
> + LIST_HEAD(way_down_list);
> + LIST_HEAD(way_up_list);
>
> if (tz->suspended)
> return;
> @@ -470,7 +487,19 @@ void __thermal_zone_device_update(struct
> tz->notify_event = event;
>
> for_each_trip_desc(tz, td)
> - handle_thermal_trip(tz, td);
> + handle_thermal_trip(tz, td, &way_up_list, &way_down_list);
> +
> + list_sort(&way_up_list, &way_up_list, thermal_trip_notify_cmp);
> + 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);
> + }
> +
> + 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);
> + }
>
> 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
> @@ -17,6 +17,8 @@
>
> struct thermal_trip_desc {
> struct thermal_trip trip;
> + struct list_head notify_list_node;
> + int notify_temp;
> int threshold;
> };
>
>
>
>
Reviewed-by: Lukasz Luba <[email protected]>
On 4/2/24 20:02, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> If a trip point is already exceeded by the zone temperature at the
> initialization time, no trip crossing notification is send regarding
> this even though mitigation should be started then.
>
> Address this by rearranging the code in handle_thermal_trip() to
> send a trip crossing notification for trip points already exceeded
> by the zone temperature initially which also allows to reduce its
> size by using the observation that the initialization and regular
> trip crossing on the way up become the same case then.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> v2 -> v3: New patch
>
> ---
> drivers/thermal/thermal_core.c | 37 ++++++++++++++++---------------------
> 1 file changed, 16 insertions(+), 21 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -364,6 +364,7 @@ static void handle_thermal_trip(struct t
> struct thermal_trip_desc *td)
> {
> const struct thermal_trip *trip = &td->trip;
> + int old_threshold;
>
> if (trip->temperature == THERMAL_TEMP_INVALID)
> return;
> @@ -375,25 +376,11 @@ static void handle_thermal_trip(struct t
> * is what needs to be compared with the previous zone temperature
> * to decide which action to take.
> */
> - if (tz->last_temperature == THERMAL_TEMP_INVALID) {
> - /* Initialization. */
> - td->threshold = trip->temperature;
> - if (tz->temperature >= td->threshold)
> - td->threshold -= trip->hysteresis;
> - } else if (tz->last_temperature < td->threshold) {
> - /*
> - * There is no mitigation under way, so it needs to be started
> - * if the zone temperature exceeds the trip one. The new
> - * threshold is then set to the low temperature of the trip.
> - */
> - if (tz->temperature >= trip->temperature) {
> - thermal_notify_tz_trip_up(tz, trip);
> - thermal_debug_tz_trip_up(tz, trip);
> - td->threshold = trip->temperature - trip->hysteresis;
> - } else {
> - td->threshold = trip->temperature;
> - }
> - } else {
> + old_threshold = td->threshold;
> + td->threshold = trip->temperature;
> +
> + if (tz->last_temperature >= old_threshold &&
> + tz->last_temperature != THERMAL_TEMP_INVALID) {
> /*
> * Mitigation is under way, so it needs to stop if the zone
> * temperature falls below the low temperature of the trip.
> @@ -402,10 +389,18 @@ static void handle_thermal_trip(struct t
> if (tz->temperature < trip->temperature - trip->hysteresis) {
> thermal_notify_tz_trip_down(tz, trip);
> thermal_debug_tz_trip_down(tz, trip);
> - td->threshold = trip->temperature;
> } else {
> - td->threshold = trip->temperature - trip->hysteresis;
> + td->threshold -= trip->hysteresis;
> }
> + } else if (tz->temperature >= trip->temperature) {
> + /*
> + * There is no mitigation under way, so it needs to be started
> + * if the zone temperature exceeds the trip one. The new
> + * threshold is then set to the low temperature of the trip.
> + */
> + thermal_notify_tz_trip_up(tz, trip);
> + thermal_debug_tz_trip_up(tz, trip);
> + td->threshold -= trip->hysteresis;
> }
>
> if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT)
>
>
>
Reviewed-by: Lukasz Luba <[email protected]>
On 02/04/2024 20:57, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Move the definitions of struct thermal_trip_desc and struct
> thermal_zone_device to an internal header file in the thermal core,
> as they don't need to be accessible to any code other than the thermal
> core and so they don't need to be present in a global header.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
--
<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
On 02/04/2024 20:59, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Make the comments regarding trip crossing and threshold updates in
> handle_thermal_trip() slightly more clear.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
--
<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