2020-09-17 03:30:15

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH RFC 0/8] Introduce warming in thermal framework

Thermal framework today supports monitoring for rising temperatures and
subsequently initiating cooling action in case of a thermal trip point
being crossed. There are scenarios where a SoC need warming mitigating
action to be activated if the temperature falls below a cetain permissible
limit. Since warming action can be considered mirror opposite of cooling
action, most of the thermal framework can be re-used to achieve this. The
key assumption in this patch series is that a device can act either as a
warming device or a cooling device and not as both.

In order to support warming three extensions are needed in the thermal
framework.

1. Indication that a trip point is being monitored for falling temperature
and not rising temperature. We discussed two different ways to achieve this
during LPC. First option is to introduce a new trip type to indicate that a
trip is a cold trip(THERMAL_TRIP_COLD). The second option is to introduce a
new property for trip point that will indicate whether a trip point is
being monitored for rising temperature or falling temperature. The patch
series(patches 1-4) chooses the second approach since it allows trip points
of any type to be monitored for rising or falling temperature.Also this was
the preferred approach when discussed during LPC. The approach that
introduces a new cold trip type was posted on the list earlier as a RFC and
can be found at [1].

2. Extend the exisitng governors to handle monitoring of falling
temperature. The patch series(patches 5 & 6) extends the step wise governor
to monitor the falling temperature.Other governors return doing nothing if
the trip point they are being called for is being monitored for falling
temperature. The governors' mitigate function is called "throttle" in the
thermal framework and with this patch series it is a misnomer as the
function is called for both throttling and warming up. Ideally
"throttle" should be renamed to "mitigate" to improve readability of code.
The renaming is not part of this series.

3. Finally, the cooling device framework itself can be reused for a warming
device. As stated before a device can act either as a warming device or a
cooling device and not as both. With this the cooling state in the
framework can be considered as mitigating state with 0 as the state with no
thermal mitigation and higher the number higher the thermal mitigation.
Again what affects the code readability and comprehension is the term
"cooling" which is a misnomer here. Ideally the term "cooling" should be
renamed to "mitigating" and hence thermal_cooling_device will become
thermal_mitgating_device. The renaming is not part of the patch series as
even though the renaming is a simple search-replace, it will change a lot
of files. The patch series(patches 7 & 8) instead introduces a minimal set
of _warming_device_ apis to register and unregister warming devices which
internally is identical to the _cooling_device_ counterpart.

1. https://lkml.org/lkml/2020/7/10/639

Thara Gopinath (8):
dt-bindings: thermal: Introduce monitor-falling parameter to thermal
trip point binding
thermal: Introduce new property monitor_type for trip point.
thermal: thermal_of: Extend thermal dt driver to support
bi-directional monitoring of a thermal trip point.
thermal:core:Add genetlink notifications for monitoring falling
temperature
thermal: gov_step_wise: Extend thermal step-wise governor to monitor
falling temperature.
thermal: Modify thermal governors to do nothing for trip points being
monitored for falling temperature
thermal:core: Add is_warming_dev and supporting warming device api's
to the cooling dev framework.
soc:qcom:qcom_aoss: Change cooling_device_register to
warming_device_register

.../bindings/thermal/thermal-zones.yaml | 7 ++
drivers/soc/qcom/qcom_aoss.c | 6 +-
drivers/thermal/gov_bang_bang.c | 12 ++
drivers/thermal/gov_fair_share.c | 12 ++
drivers/thermal/gov_power_allocator.c | 12 ++
drivers/thermal/gov_step_wise.c | 62 +++++++---
drivers/thermal/thermal_core.c | 113 +++++++++++++++---
drivers/thermal/thermal_core.h | 2 +
drivers/thermal/thermal_of.c | 22 ++++
include/linux/thermal.h | 9 ++
include/uapi/linux/thermal.h | 5 +
11 files changed, 226 insertions(+), 36 deletions(-)

--
2.25.1


2020-09-17 03:30:34

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH RFC 7/8] thermal:core: Add is_warming_dev and supporting warming device api's to the cooling dev framework.

Introduce a variable is_warming_dev to indicate if a "cooling" device is
actually a warming device or not. Also introduce api's to register and
unregister warming device.

This is a temporary patch. If we agree to replace the term "cooling" with
"mitigating" (or any other appropriate term) in the thermal framework, this
patch can be dropped. Also I have not added warming_device_register api for
all the versions of cooling_device_register because
devm_thermal_of_warming_device_register is the only api needed in the
kernel today as is evident by the next patch in this series.

Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/thermal/thermal_core.c | 88 +++++++++++++++++++++++++++++-----
include/linux/thermal.h | 7 +++
2 files changed, 83 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index bfd436379408..4aae48a80e00 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1101,7 +1101,8 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
*/
static struct thermal_cooling_device *
__thermal_cooling_device_register(struct device_node *np,
- const char *type, void *devdata,
+ const char *type, bool is_warming_dev,
+ void *devdata,
const struct thermal_cooling_device_ops *ops)
{
struct thermal_cooling_device *cdev;
@@ -1134,6 +1135,7 @@ __thermal_cooling_device_register(struct device_node *np,
cdev->updated = false;
cdev->device.class = &thermal_class;
cdev->devdata = devdata;
+ cdev->is_warming_dev = is_warming_dev;
thermal_cooling_device_setup_sysfs(cdev);
dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
result = device_register(&cdev->device);
@@ -1178,7 +1180,7 @@ struct thermal_cooling_device *
thermal_cooling_device_register(const char *type, void *devdata,
const struct thermal_cooling_device_ops *ops)
{
- return __thermal_cooling_device_register(NULL, type, devdata, ops);
+ return __thermal_cooling_device_register(NULL, type, false, devdata, ops);
}
EXPORT_SYMBOL_GPL(thermal_cooling_device_register);

@@ -1202,7 +1204,7 @@ thermal_of_cooling_device_register(struct device_node *np,
const char *type, void *devdata,
const struct thermal_cooling_device_ops *ops)
{
- return __thermal_cooling_device_register(np, type, devdata, ops);
+ return __thermal_cooling_device_register(np, type, false, devdata, ops);
}
EXPORT_SYMBOL_GPL(thermal_of_cooling_device_register);

@@ -1242,7 +1244,7 @@ devm_thermal_of_cooling_device_register(struct device *dev,
if (!ptr)
return ERR_PTR(-ENOMEM);

- tcd = __thermal_cooling_device_register(np, type, devdata, ops);
+ tcd = __thermal_cooling_device_register(np, type, false, devdata, ops);
if (IS_ERR(tcd)) {
devres_free(ptr);
return tcd;
@@ -1255,6 +1257,49 @@ devm_thermal_of_cooling_device_register(struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_thermal_of_cooling_device_register);

+/**
+ * devm_thermal_of_warming_device_register() - register an OF thermal warming
+ * device
+ * @dev: a valid struct device pointer of a sensor device.
+ * @np: a pointer to a device tree node.
+ * @type: the thermal cooling device type.
+ * @devdata: device private data.
+ * @ops: standard thermal cooling devices callbacks.
+ *
+ * This function will register a warming device with device tree node reference.
+ * This interface function adds a new thermal warming device (fan/processor/...)
+ * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
+ * to all the thermal zone devices registered at the same time.
+ *
+ * Return: a pointer to the created struct thermal_cooling_device or an
+ * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
+ */
+struct thermal_cooling_device *
+devm_thermal_of_warming_device_register(struct device *dev,
+ struct device_node *np,
+ char *type, void *devdata,
+ const struct thermal_cooling_device_ops *ops)
+{
+ struct thermal_cooling_device **ptr, *tcd;
+
+ ptr = devres_alloc(thermal_cooling_device_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ tcd = __thermal_cooling_device_register(np, type, true, devdata, ops);
+ if (IS_ERR(tcd)) {
+ devres_free(ptr);
+ return tcd;
+ }
+
+ *ptr = tcd;
+ devres_add(dev, ptr);
+
+ return tcd;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_of_warming_device_register);
+
static void __unbind(struct thermal_zone_device *tz, int mask,
struct thermal_cooling_device *cdev)
{
@@ -1265,14 +1310,8 @@ static void __unbind(struct thermal_zone_device *tz, int mask,
thermal_zone_unbind_cooling_device(tz, i, cdev);
}

-/**
- * thermal_cooling_device_unregister - removes a thermal cooling device
- * @cdev: the thermal cooling device to remove.
- *
- * thermal_cooling_device_unregister() must be called when a registered
- * thermal cooling device is no longer needed.
- */
-void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
+static void
+__thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
{
int i;
const struct thermal_zone_params *tzp;
@@ -1319,8 +1358,33 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
thermal_cooling_device_destroy_sysfs(cdev);
put_device(&cdev->device);
}
+
+/**
+ * thermal_cooling_device_unregister - removes a thermal cooling device
+ * @cdev: the thermal cooling device to remove.
+ *
+ * thermal_cooling_device_unregister() must be called when a registered
+ * thermal cooling device is no longer needed.
+ */
+void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
+{
+ __thermal_cooling_device_unregister(cdev);
+}
EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister);

+/**
+ * thermal_warming_device_unregister - removes a thermal warming device
+ * @cdev: the thermal warming device to remove.
+ *
+ * thermal_warming_device_unregister() must be called when a registered
+ * thermal warming device is no longer needed.
+ */
+void thermal_warming_device_unregister(struct thermal_cooling_device *cdev)
+{
+ __thermal_cooling_device_unregister(cdev);
+}
+EXPORT_SYMBOL_GPL(thermal_warming_device_unregister);
+
static void bind_tz(struct thermal_zone_device *tz)
{
int i, ret;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a50ed958d0bd..455120f485dd 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -96,6 +96,7 @@ struct thermal_cooling_device_ops {

struct thermal_cooling_device {
int id;
+ bool is_warming_dev;
char type[THERMAL_NAME_LENGTH];
struct device device;
struct device_node *np;
@@ -393,7 +394,13 @@ devm_thermal_of_cooling_device_register(struct device *dev,
struct device_node *np,
char *type, void *devdata,
const struct thermal_cooling_device_ops *ops);
+struct thermal_cooling_device *
+devm_thermal_of_warming_device_register(struct device *dev,
+ struct device_node *np,
+ char *type, void *devdata,
+ const struct thermal_cooling_device_ops *ops);
void thermal_cooling_device_unregister(struct thermal_cooling_device *);
+void thermal_warming_device_unregister(struct thermal_cooling_device *);
struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
int thermal_zone_get_slope(struct thermal_zone_device *tz);
--
2.25.1

2020-09-17 03:30:31

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH RFC 4/8] thermal:core:Add genetlink notifications for monitoring falling temperature

Add notification calls for trip points that are being monitored for
falling temperatures.

Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/thermal/thermal_core.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 72bf159bcecc..bfd436379408 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -417,6 +417,7 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
{
enum thermal_trip_type type;
+ enum thermal_trip_monitor_type mon_type;
int trip_temp, hyst = 0;

/* Ignore disabled trip points */
@@ -428,13 +429,25 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
if (tz->ops->get_trip_hyst)
tz->ops->get_trip_hyst(tz, trip, &hyst);

+ if (tz->ops->get_trip_mon_type)
+ tz->ops->get_trip_mon_type(tz, trip, &mon_type);
+
if (tz->last_temperature != THERMAL_TEMP_INVALID) {
- if (tz->last_temperature < trip_temp &&
- tz->temperature >= trip_temp)
- thermal_notify_tz_trip_up(tz->id, trip);
- if (tz->last_temperature >= trip_temp &&
- tz->temperature < (trip_temp - hyst))
- thermal_notify_tz_trip_down(tz->id, trip);
+ if (mon_type == THERMAL_TRIP_MONITOR_FALLING) {
+ if (tz->last_temperature > trip_temp &&
+ tz->temperature <= trip_temp)
+ thermal_notify_tz_trip_down(tz->id, trip);
+ if (tz->last_temperature <= trip_temp &&
+ tz->temperature > (trip_temp + hyst))
+ thermal_notify_tz_trip_up(tz->id, trip);
+ } else {
+ if (tz->last_temperature < trip_temp &&
+ tz->temperature >= trip_temp)
+ thermal_notify_tz_trip_up(tz->id, trip);
+ if (tz->last_temperature >= trip_temp &&
+ tz->temperature < (trip_temp - hyst))
+ thermal_notify_tz_trip_down(tz->id, trip);
+ }
}

if (type == THERMAL_TRIP_CRITICAL || type == THERMAL_TRIP_HOT)
--
2.25.1

2020-09-17 03:31:04

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH RFC 8/8] soc:qcom:qcom_aoss: Change cooling_device_register to warming_device_register

Always on subsystem host resources cx and ebi that are used as warming
devices. Use the newly introduce _warming_device_register to register
these devices with the thermal framework.

Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/soc/qcom/qcom_aoss.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
index ed2c687c16b3..4f65c03a5def 100644
--- a/drivers/soc/qcom/qcom_aoss.c
+++ b/drivers/soc/qcom/qcom_aoss.c
@@ -461,7 +461,7 @@ static int qmp_cooling_device_add(struct qmp *qmp,
qmp_cdev->qmp = qmp;
qmp_cdev->state = !qmp_cdev_max_state;
qmp_cdev->name = cdev_name;
- qmp_cdev->cdev = devm_thermal_of_cooling_device_register
+ qmp_cdev->cdev = devm_thermal_of_warming_device_register
(qmp->dev, node,
cdev_name,
qmp_cdev, &qmp_cooling_device_ops);
@@ -501,7 +501,7 @@ static int qmp_cooling_devices_register(struct qmp *qmp)

unroll:
while (--count >= 0)
- thermal_cooling_device_unregister
+ thermal_warming_device_unregister
(qmp->cooling_devs[count].cdev);

return ret;
@@ -512,7 +512,7 @@ static void qmp_cooling_devices_remove(struct qmp *qmp)
int i;

for (i = 0; i < QMP_NUM_COOLING_RESOURCES; i++)
- thermal_cooling_device_unregister(qmp->cooling_devs[i].cdev);
+ thermal_warming_device_unregister(qmp->cooling_devs[i].cdev);
}

static int qmp_probe(struct platform_device *pdev)
--
2.25.1

2020-09-17 03:32:44

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH RFC 5/8] thermal: gov_step_wise: Extend thermal step-wise governor to monitor falling temperature.

From the step wise governor point of view, the policy decisions
that has to taken on a thermal trip point that is defined to be monitored
for falling temperature is the mirror opposite of the decisions it has
to take on a trip point that is monitored for rising temperature.

Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/thermal/gov_step_wise.c | 62 +++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/gov_step_wise.c b/drivers/thermal/gov_step_wise.c
index 2ae7198d3067..c036ff7b4fb2 100644
--- a/drivers/thermal/gov_step_wise.c
+++ b/drivers/thermal/gov_step_wise.c
@@ -35,7 +35,8 @@
* deactivate the thermal instance
*/
static unsigned long get_target_state(struct thermal_instance *instance,
- enum thermal_trend trend, bool throttle)
+ enum thermal_trend trend, bool throttle,
+ enum thermal_trip_monitor_type type)
{
struct thermal_cooling_device *cdev = instance->cdev;
unsigned long cur_state;
@@ -65,11 +66,24 @@ static unsigned long get_target_state(struct thermal_instance *instance,

switch (trend) {
case THERMAL_TREND_RAISING:
- if (throttle) {
- next_target = cur_state < instance->upper ?
- (cur_state + 1) : instance->upper;
- if (next_target < instance->lower)
- next_target = instance->lower;
+ if (type == THERMAL_TRIP_MONITOR_FALLING) {
+ if (cur_state <= instance->lower) {
+ if (!throttle)
+ next_target = THERMAL_NO_TARGET;
+ } else {
+ if (!throttle) {
+ next_target = cur_state - 1;
+ if (next_target > instance->upper)
+ next_target = instance->upper;
+ }
+ }
+ } else {
+ if (throttle) {
+ next_target = cur_state < instance->upper ?
+ (cur_state + 1) : instance->upper;
+ if (next_target < instance->lower)
+ next_target = instance->lower;
+ }
}
break;
case THERMAL_TREND_RAISE_FULL:
@@ -77,14 +91,23 @@ static unsigned long get_target_state(struct thermal_instance *instance,
next_target = instance->upper;
break;
case THERMAL_TREND_DROPPING:
- if (cur_state <= instance->lower) {
- if (!throttle)
- next_target = THERMAL_NO_TARGET;
+ if (type == THERMAL_TRIP_MONITOR_FALLING) {
+ if (throttle) {
+ next_target = cur_state < instance->upper ?
+ (cur_state + 1) : instance->upper;
+ if (next_target < instance->lower)
+ next_target = instance->lower;
+ }
} else {
- if (!throttle) {
- next_target = cur_state - 1;
- if (next_target > instance->upper)
- next_target = instance->upper;
+ if (cur_state <= instance->lower) {
+ if (!throttle)
+ next_target = THERMAL_NO_TARGET;
+ } else {
+ if (!throttle) {
+ next_target = cur_state - 1;
+ if (next_target > instance->upper)
+ next_target = instance->upper;
+ }
}
}
break;
@@ -117,6 +140,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
{
int trip_temp;
enum thermal_trip_type trip_type;
+ enum thermal_trip_monitor_type monitor_type =
+ THERMAL_TRIP_MONITOR_RISING;
enum thermal_trend trend;
struct thermal_instance *instance;
bool throttle = false;
@@ -130,9 +155,15 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
tz->ops->get_trip_type(tz, trip, &trip_type);
}

+ if (tz->ops->get_trip_mon_type)
+ tz->ops->get_trip_mon_type(tz, trip, &monitor_type);
+
trend = get_tz_trend(tz, trip);

- if (tz->temperature >= trip_temp) {
+ if (((monitor_type == THERMAL_TRIP_MONITOR_RISING) &&
+ (tz->temperature >= trip_temp)) ||
+ ((monitor_type == THERMAL_TRIP_MONITOR_FALLING) &&
+ (tz->temperature <= trip_temp))) {
throttle = true;
trace_thermal_zone_trip(tz, trip, trip_type);
}
@@ -147,7 +178,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
continue;

old_target = instance->target;
- instance->target = get_target_state(instance, trend, throttle);
+ instance->target = get_target_state(instance, trend,
+ throttle, monitor_type);
dev_dbg(&instance->cdev->device, "old_target=%d, target=%d\n",
old_target, (int)instance->target);

--
2.25.1

2020-09-17 03:33:06

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH RFC 2/8] thermal: Introduce new property monitor_type for trip point.

Thermal trip points can be defined to indicate whether a
temperature rise or a temperature fall is to be monitored. This
property can now be defined in the DT bindings for a trip point.
To support this following three changes are introduced to thermal
core and sysfs code.
1. Define a new variable in thermal_trip to capture the monitor
rising/falling information from trip point DT bindings.
2. Define a new ops in thermal_zone_device_ops that can be populated
to indicate whether a trip is being monitored for rising or falling
temperature. If the ops is not populated or if the binding is missing
in the DT, it is assumed that the trip is being monitored for rising
temperature. (default behavior today)

Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/thermal/thermal_core.h | 2 ++
include/linux/thermal.h | 2 ++
include/uapi/linux/thermal.h | 5 +++++
3 files changed, 9 insertions(+)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index e00fc5585ea8..c56addfe2284 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -77,12 +77,14 @@ int power_actor_set_power(struct thermal_cooling_device *cdev,
* @temperature: temperature value in miliCelsius
* @hysteresis: relative hysteresis in miliCelsius
* @type: trip point type
+ * @monitor_type: trip point monitor type
*/
struct thermal_trip {
struct device_node *np;
int temperature;
int hysteresis;
enum thermal_trip_type type;
+ enum thermal_trip_monitor_type monitor_type;
};

int get_tz_trend(struct thermal_zone_device *tz, int trip);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 42ef807e5d84..a50ed958d0bd 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -72,6 +72,8 @@ struct thermal_zone_device_ops {
int (*set_trip_temp) (struct thermal_zone_device *, int, int);
int (*get_trip_hyst) (struct thermal_zone_device *, int, int *);
int (*set_trip_hyst) (struct thermal_zone_device *, int, int);
+ int (*get_trip_mon_type)(struct thermal_zone_device *, int,
+ enum thermal_trip_monitor_type *);
int (*get_crit_temp) (struct thermal_zone_device *, int *);
int (*set_emul_temp) (struct thermal_zone_device *, int);
int (*get_trend) (struct thermal_zone_device *, int,
diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
index c105054cbb57..d3bb4e4fad69 100644
--- a/include/uapi/linux/thermal.h
+++ b/include/uapi/linux/thermal.h
@@ -16,6 +16,11 @@ enum thermal_trip_type {
THERMAL_TRIP_CRITICAL,
};

+enum thermal_trip_monitor_type {
+ THERMAL_TRIP_MONITOR_RISING = 0,
+ THERMAL_TRIP_MONITOR_FALLING
+};
+
/* Adding event notification support elements */
#define THERMAL_GENL_FAMILY_NAME "thermal"
#define THERMAL_GENL_VERSION 0x01
--
2.25.1

2020-09-17 03:33:24

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH RFC 6/8] thermal: Modify thermal governors to do nothing for trip points being monitored for falling temperature

For now, thermal governors other than step wise governorr do not support
monitoring of falling temperature. Hence, in case of calls to the governor
for trip points marked as THERMAL_TRIP_MONITOR_FALLING, return doing
nothing.

Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/thermal/gov_bang_bang.c | 12 ++++++++++++
drivers/thermal/gov_fair_share.c | 12 ++++++++++++
drivers/thermal/gov_power_allocator.c | 12 ++++++++++++
3 files changed, 36 insertions(+)

diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c
index 991a1c54296d..a662047e5961 100644
--- a/drivers/thermal/gov_bang_bang.c
+++ b/drivers/thermal/gov_bang_bang.c
@@ -99,6 +99,18 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
static int bang_bang_control(struct thermal_zone_device *tz, int trip)
{
struct thermal_instance *instance;
+ enum thermal_trip_monitor_type monitor_type =
+ THERMAL_TRIP_MONITOR_RISING;
+
+ /*
+ * Return doing nothing if the trip point is monitored for
+ * falling temperature
+ */
+ if (tz->ops->get_trip_mon_type) {
+ tz->ops->get_trip_mon_type(tz, trip, &monitor_type);
+ if (monitor_type == THERMAL_TRIP_MONITOR_FALLING)
+ return 0;
+ }

thermal_zone_trip_update(tz, trip);

diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c
index aaa07180ab48..064ad6ed67ad 100644
--- a/drivers/thermal/gov_fair_share.c
+++ b/drivers/thermal/gov_fair_share.c
@@ -81,6 +81,18 @@ static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
int total_weight = 0;
int total_instance = 0;
int cur_trip_level = get_trip_level(tz);
+ enum thermal_trip_monitor_type monitor_type =
+ THERMAL_TRIP_MONITOR_RISING;
+
+ /*
+ * Return doing nothing if the trip point is monitored for
+ * falling temperature
+ */
+ if (tz->ops->get_trip_mon_type) {
+ tz->ops->get_trip_mon_type(tz, trip, &monitor_type);
+ if (monitor_type == THERMAL_TRIP_MONITOR_FALLING)
+ return 0;
+ }

list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
if (instance->trip != trip)
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 5cb518d8f156..0f674cd1b9b8 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -606,6 +606,8 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip)
{
int ret;
int switch_on_temp, control_temp;
+ enum thermal_trip_monitor_type monitor_type =
+ THERMAL_TRIP_MONITOR_RISING;
struct power_allocator_params *params = tz->governor_data;

/*
@@ -615,6 +617,16 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip)
if (trip != params->trip_max_desired_temperature)
return 0;

+ /*
+ * Return doing nothing if the trip point is monitored for
+ * falling temperature
+ */
+ if (tz->ops->get_trip_mon_type) {
+ tz->ops->get_trip_mon_type(tz, trip, &monitor_type);
+ if (monitor_type == THERMAL_TRIP_MONITOR_FALLING)
+ return 0;
+ }
+
ret = tz->ops->get_trip_temp(tz, params->trip_switch_on,
&switch_on_temp);
if (!ret && (tz->temperature < switch_on_temp)) {
--
2.25.1

2020-10-19 18:46:23

by Thara Gopinath

[permalink] [raw]
Subject: Re: [PATCH RFC 0/8] Introduce warming in thermal framework

On Wed, 16 Sep 2020 at 23:22, Thara Gopinath <[email protected]> wrote:
>
> Thermal framework today supports monitoring for rising temperatures and
> subsequently initiating cooling action in case of a thermal trip point
> being crossed. There are scenarios where a SoC need warming mitigating
> action to be activated if the temperature falls below a cetain permissible
> limit. Since warming action can be considered mirror opposite of cooling
> action, most of the thermal framework can be re-used to achieve this. The
> key assumption in this patch series is that a device can act either as a
> warming device or a cooling device and not as both.
>
> In order to support warming three extensions are needed in the thermal
> framework.
>
> 1. Indication that a trip point is being monitored for falling temperature
> and not rising temperature. We discussed two different ways to achieve this
> during LPC. First option is to introduce a new trip type to indicate that a
> trip is a cold trip(THERMAL_TRIP_COLD). The second option is to introduce a
> new property for trip point that will indicate whether a trip point is
> being monitored for rising temperature or falling temperature. The patch
> series(patches 1-4) chooses the second approach since it allows trip points
> of any type to be monitored for rising or falling temperature.Also this was
> the preferred approach when discussed during LPC. The approach that
> introduces a new cold trip type was posted on the list earlier as a RFC and
> can be found at [1].
>
> 2. Extend the exisitng governors to handle monitoring of falling
> temperature. The patch series(patches 5 & 6) extends the step wise governor
> to monitor the falling temperature.Other governors return doing nothing if
> the trip point they are being called for is being monitored for falling
> temperature. The governors' mitigate function is called "throttle" in the
> thermal framework and with this patch series it is a misnomer as the
> function is called for both throttling and warming up. Ideally
> "throttle" should be renamed to "mitigate" to improve readability of code.
> The renaming is not part of this series.
>
> 3. Finally, the cooling device framework itself can be reused for a warming
> device. As stated before a device can act either as a warming device or a
> cooling device and not as both. With this the cooling state in the
> framework can be considered as mitigating state with 0 as the state with no
> thermal mitigation and higher the number higher the thermal mitigation.
> Again what affects the code readability and comprehension is the term
> "cooling" which is a misnomer here. Ideally the term "cooling" should be
> renamed to "mitigating" and hence thermal_cooling_device will become
> thermal_mitgating_device. The renaming is not part of the patch series as
> even though the renaming is a simple search-replace, it will change a lot
> of files. The patch series(patches 7 & 8) instead introduces a minimal set
> of _warming_device_ apis to register and unregister warming devices which
> internally is identical to the _cooling_device_ counterpart.

Gentle ping for review..

>
> 1. https://lkml.org/lkml/2020/7/10/639
>
> Thara Gopinath (8):
> dt-bindings: thermal: Introduce monitor-falling parameter to thermal
> trip point binding
> thermal: Introduce new property monitor_type for trip point.
> thermal: thermal_of: Extend thermal dt driver to support
> bi-directional monitoring of a thermal trip point.
> thermal:core:Add genetlink notifications for monitoring falling
> temperature
> thermal: gov_step_wise: Extend thermal step-wise governor to monitor
> falling temperature.
> thermal: Modify thermal governors to do nothing for trip points being
> monitored for falling temperature
> thermal:core: Add is_warming_dev and supporting warming device api's
> to the cooling dev framework.
> soc:qcom:qcom_aoss: Change cooling_device_register to
> warming_device_register
>
> .../bindings/thermal/thermal-zones.yaml | 7 ++
> drivers/soc/qcom/qcom_aoss.c | 6 +-
> drivers/thermal/gov_bang_bang.c | 12 ++
> drivers/thermal/gov_fair_share.c | 12 ++
> drivers/thermal/gov_power_allocator.c | 12 ++
> drivers/thermal/gov_step_wise.c | 62 +++++++---
> drivers/thermal/thermal_core.c | 113 +++++++++++++++---
> drivers/thermal/thermal_core.h | 2 +
> drivers/thermal/thermal_of.c | 22 ++++
> include/linux/thermal.h | 9 ++
> include/uapi/linux/thermal.h | 5 +
> 11 files changed, 226 insertions(+), 36 deletions(-)
>
> --
> 2.25.1
>


--
Warm Regards
Thara

2020-10-20 14:42:42

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH RFC 0/8] Introduce warming in thermal framework

On 19/10/2020 20:42, Thara Gopinath wrote:
> On Wed, 16 Sep 2020 at 23:22, Thara Gopinath <[email protected]> wrote:
>>
>> Thermal framework today supports monitoring for rising temperatures and
>> subsequently initiating cooling action in case of a thermal trip point
>> being crossed. There are scenarios where a SoC need warming mitigating
>> action to be activated if the temperature falls below a cetain permissible
>> limit. Since warming action can be considered mirror opposite of cooling
>> action, most of the thermal framework can be re-used to achieve this. The
>> key assumption in this patch series is that a device can act either as a
>> warming device or a cooling device and not as both.
>>
>> In order to support warming three extensions are needed in the thermal
>> framework.
>>
>> 1. Indication that a trip point is being monitored for falling temperature
>> and not rising temperature. We discussed two different ways to achieve this
>> during LPC. First option is to introduce a new trip type to indicate that a
>> trip is a cold trip(THERMAL_TRIP_COLD). The second option is to introduce a
>> new property for trip point that will indicate whether a trip point is
>> being monitored for rising temperature or falling temperature. The patch
>> series(patches 1-4) chooses the second approach since it allows trip points
>> of any type to be monitored for rising or falling temperature.Also this was
>> the preferred approach when discussed during LPC. The approach that
>> introduces a new cold trip type was posted on the list earlier as a RFC and
>> can be found at [1].
>>
>> 2. Extend the exisitng governors to handle monitoring of falling
>> temperature. The patch series(patches 5 & 6) extends the step wise governor
>> to monitor the falling temperature.Other governors return doing nothing if
>> the trip point they are being called for is being monitored for falling
>> temperature. The governors' mitigate function is called "throttle" in the
>> thermal framework and with this patch series it is a misnomer as the
>> function is called for both throttling and warming up. Ideally
>> "throttle" should be renamed to "mitigate" to improve readability of code.
>> The renaming is not part of this series.
>>
>> 3. Finally, the cooling device framework itself can be reused for a warming
>> device. As stated before a device can act either as a warming device or a
>> cooling device and not as both. With this the cooling state in the
>> framework can be considered as mitigating state with 0 as the state with no
>> thermal mitigation and higher the number higher the thermal mitigation.
>> Again what affects the code readability and comprehension is the term
>> "cooling" which is a misnomer here. Ideally the term "cooling" should be
>> renamed to "mitigating" and hence thermal_cooling_device will become
>> thermal_mitgating_device. The renaming is not part of the patch series as
>> even though the renaming is a simple search-replace, it will change a lot
>> of files. The patch series(patches 7 & 8) instead introduces a minimal set
>> of _warming_device_ apis to register and unregister warming devices which
>> internally is identical to the _cooling_device_ counterpart.
>
> Gentle ping for review..

Pong, review before the end of this week.



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