From: Rafael J. Wysocki <[email protected]>
The Bang-Bang governor really is only concerned about trip point
crossing, so it can use the new .trip_crossed() callback instead of
throttle() that is not particularly suitable for it.
Modify it to do so which also takes trip hysteresis into account, so the
governor does not need to use it directly any more.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/thermal/gov_bang_bang.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
Index: linux-pm/drivers/thermal/gov_bang_bang.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_bang_bang.c
+++ linux-pm/drivers/thermal/gov_bang_bang.c
@@ -13,8 +13,9 @@
#include "thermal_core.h"
-static int thermal_zone_trip_update(struct thermal_zone_device *tz,
- const struct thermal_trip *trip)
+static void thermal_zone_trip_update(struct thermal_zone_device *tz,
+ const struct thermal_trip *trip,
+ bool crossed_up)
{
int trip_index = thermal_zone_trip_id(tz, trip);
struct thermal_instance *instance;
@@ -43,13 +44,12 @@ static int thermal_zone_trip_update(stru
}
/*
- * enable fan when temperature exceeds trip_temp and disable
- * the fan in case it falls below trip_temp minus hysteresis
+ * Enable the fan when the trip is crossed on the way up and
+ * disable it when the trip is crossed on the way down.
*/
- if (instance->target == 0 && tz->temperature >= trip->temperature)
+ if (instance->target == 0 && crossed_up)
instance->target = 1;
- else if (instance->target == 1 &&
- tz->temperature < trip->temperature - trip->hysteresis)
+ else if (instance->target == 1 && !crossed_up)
instance->target = 0;
dev_dbg(&instance->cdev->device, "target=%d\n",
@@ -59,14 +59,13 @@ static int thermal_zone_trip_update(stru
instance->cdev->updated = false; /* cdev needs update */
mutex_unlock(&instance->cdev->lock);
}
-
- return 0;
}
/**
* bang_bang_control - controls devices associated with the given zone
* @tz: thermal_zone_device
* @trip: the trip point
+ * @crossed_up: whether or not the trip has been crossed on the way up
*
* Regulation Logic: a two point regulation, deliver cooling state depending
* on the previous state shown in this diagram:
@@ -90,26 +89,22 @@ static int thermal_zone_trip_update(stru
* (trip_temp - hyst) so that the fan gets turned off again.
*
*/
-static int bang_bang_control(struct thermal_zone_device *tz,
- const struct thermal_trip *trip)
+static void bang_bang_control(struct thermal_zone_device *tz,
+ const struct thermal_trip *trip,
+ bool crossed_up)
{
struct thermal_instance *instance;
- int ret;
lockdep_assert_held(&tz->lock);
- ret = thermal_zone_trip_update(tz, trip);
- if (ret)
- return ret;
+ thermal_zone_trip_update(tz, trip, crossed_up);
list_for_each_entry(instance, &tz->thermal_instances, tz_node)
thermal_cdev_update(instance->cdev);
-
- return 0;
}
static struct thermal_governor thermal_gov_bang_bang = {
.name = "bang_bang",
- .throttle = bang_bang_control,
+ .trip_crossed = bang_bang_control,
};
THERMAL_GOVERNOR_DECLARE(thermal_gov_bang_bang);
On 4/10/24 17:04, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The Bang-Bang governor really is only concerned about trip point
> crossing, so it can use the new .trip_crossed() callback instead of
> .throttle() that is not particularly suitable for it.
>
> Modify it to do so which also takes trip hysteresis into account, so the
> governor does not need to use it directly any more.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/thermal/gov_bang_bang.c | 31 +++++++++++++------------------
> 1 file changed, 13 insertions(+), 18 deletions(-)
>
> Index: linux-pm/drivers/thermal/gov_bang_bang.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_bang_bang.c
> +++ linux-pm/drivers/thermal/gov_bang_bang.c
> @@ -13,8 +13,9 @@
>
> #include "thermal_core.h"
>
> -static int thermal_zone_trip_update(struct thermal_zone_device *tz,
> - const struct thermal_trip *trip)
> +static void thermal_zone_trip_update(struct thermal_zone_device *tz,
> + const struct thermal_trip *trip,
> + bool crossed_up)
> {
> int trip_index = thermal_zone_trip_id(tz, trip);
> struct thermal_instance *instance;
> @@ -43,13 +44,12 @@ static int thermal_zone_trip_update(stru
> }
>
> /*
> - * enable fan when temperature exceeds trip_temp and disable
> - * the fan in case it falls below trip_temp minus hysteresis
> + * Enable the fan when the trip is crossed on the way up and
> + * disable it when the trip is crossed on the way down.
> */
> - if (instance->target == 0 && tz->temperature >= trip->temperature)
> + if (instance->target == 0 && crossed_up)
> instance->target = 1;
> - else if (instance->target == 1 &&
> - tz->temperature < trip->temperature - trip->hysteresis)
> + else if (instance->target == 1 && !crossed_up)
> instance->target = 0;
>
> dev_dbg(&instance->cdev->device, "target=%d\n",
> @@ -59,14 +59,13 @@ static int thermal_zone_trip_update(stru
> instance->cdev->updated = false; /* cdev needs update */
> mutex_unlock(&instance->cdev->lock);
> }
> -
> - return 0;
> }
>
> /**
> * bang_bang_control - controls devices associated with the given zone
> * @tz: thermal_zone_device
> * @trip: the trip point
> + * @crossed_up: whether or not the trip has been crossed on the way up
> *
> * Regulation Logic: a two point regulation, deliver cooling state depending
> * on the previous state shown in this diagram:
> @@ -90,26 +89,22 @@ static int thermal_zone_trip_update(stru
> * (trip_temp - hyst) so that the fan gets turned off again.
> *
> */
> -static int bang_bang_control(struct thermal_zone_device *tz,
> - const struct thermal_trip *trip)
> +static void bang_bang_control(struct thermal_zone_device *tz,
> + const struct thermal_trip *trip,
> + bool crossed_up)
> {
> struct thermal_instance *instance;
> - int ret;
>
> lockdep_assert_held(&tz->lock);
>
> - ret = thermal_zone_trip_update(tz, trip);
> - if (ret)
> - return ret;
> + thermal_zone_trip_update(tz, trip, crossed_up);
>
> list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> thermal_cdev_update(instance->cdev);
> -
> - return 0;
> }
>
> static struct thermal_governor thermal_gov_bang_bang = {
> .name = "bang_bang",
> - .throttle = bang_bang_control,
> + .trip_crossed = bang_bang_control,
> };
> THERMAL_GOVERNOR_DECLARE(thermal_gov_bang_bang);
>
>
>
LGTM
Reviewed-by: Lukasz Luba <[email protected]>
On 10/04/2024 18:04, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The Bang-Bang governor really is only concerned about trip point
> crossing, so it can use the new .trip_crossed() callback instead of
> .throttle() that is not particularly suitable for it.
>
> Modify it to do so which also takes trip hysteresis into account, so the
> governor does not need to use it directly any more.
>
> 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