diff mbox series

thermal: Allow selecting the bang-bang governor as default

Message ID 20230609124408.3788680-1-thierry.reding@gmail.com
State Accepted
Headers show
Series thermal: Allow selecting the bang-bang governor as default | expand

Commit Message

Thierry Reding June 9, 2023, 12:44 p.m. UTC
From: Thierry Reding <treding@nvidia.com>

For many setups the bang-bang governor is exactly what we want. Many
ARM SoC-based devices use fans to cool down the entire SoC and that
works well only with the bang-bang governor because it uses the
hysteresis in order to let the fan run for a while to cool the SoC
down below the trip point before switching it off again.

The step-wise governor will behave strangely in these situations. It
doesn't use the hysteresis, so it can lead to situations where the fan
is turned on for only a very brief period and then is switched back off,
only to get switched back on again very quickly because the SoC hasn't
cooled down very much.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/thermal/Kconfig        | 8 ++++++++
 drivers/thermal/thermal_core.h | 2 ++
 2 files changed, 10 insertions(+)

Comments

Daniel Lezcano June 12, 2023, 9:07 a.m. UTC | #1
Hi Thierry,

On 09/06/2023 14:44, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> For many setups the bang-bang governor is exactly what we want. Many
> ARM SoC-based devices use fans to cool down the entire SoC and that
> works well only with the bang-bang governor because it uses the
> hysteresis in order to let the fan run for a while to cool the SoC
> down below the trip point before switching it off again.

Yeah, that trip point detection is screwed up at the moment, but we are 
on the way to solve that. From there, we should be able to have the 
step_wise governor working as the bang-bang governor and remove this one.

Meanwhile, the change sounds ok for me.

Rafael, may I pick this change?



> The step-wise governor will behave strangely in these situations. It
> doesn't use the hysteresis, so it can lead to situations where the fan
> is turned on for only a very brief period and then is switched back off,
> only to get switched back on again very quickly because the SoC hasn't
> cooled down very much.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>   drivers/thermal/Kconfig        | 8 ++++++++
>   drivers/thermal/thermal_core.h | 2 ++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 4cd7ab707315..19a4b33cb564 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -130,6 +130,14 @@ config THERMAL_DEFAULT_GOV_POWER_ALLOCATOR
>   	  system and device power allocation. This governor can only
>   	  operate on cooling devices that implement the power API.
>   
> +config THERMAL_DEFAULT_GOV_BANG_BANG
> +	bool "bang_bang"
> +	depends on THERMAL_GOV_BANG_BANG
> +	help
> +	  Use the bang_bang governor as default. This throttles the
> +	  devices one step at the time, taking into account the trip
> +	  point hysteresis.
> +
>   endchoice
>   
>   config THERMAL_GOV_FAIR_SHARE
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 3d4a787c6b28..17c1bbed734d 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -23,6 +23,8 @@
>   #define DEFAULT_THERMAL_GOVERNOR       "user_space"
>   #elif defined(CONFIG_THERMAL_DEFAULT_GOV_POWER_ALLOCATOR)
>   #define DEFAULT_THERMAL_GOVERNOR       "power_allocator"
> +#elif defined(CONFIG_THERMAL_DEFAULT_GOV_BANG_BANG)
> +#define DEFAULT_THERMAL_GOVERNOR       "bang_bang"
>   #endif
>   
>   /* Initial state of a cooling device during binding */
Rafael J. Wysocki June 12, 2023, 5:43 p.m. UTC | #2
On Mon, Jun 12, 2023 at 11:07 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
>
> Hi Thierry,
>
> On 09/06/2023 14:44, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > For many setups the bang-bang governor is exactly what we want. Many
> > ARM SoC-based devices use fans to cool down the entire SoC and that
> > works well only with the bang-bang governor because it uses the
> > hysteresis in order to let the fan run for a while to cool the SoC
> > down below the trip point before switching it off again.
>
> Yeah, that trip point detection is screwed up at the moment, but we are
> on the way to solve that. From there, we should be able to have the
> step_wise governor working as the bang-bang governor and remove this one.
>
> Meanwhile, the change sounds ok for me.
>
> Rafael, may I pick this change?

Please do, thank you!
Daniel Lezcano June 13, 2023, 7 a.m. UTC | #3
On 09/06/2023 14:44, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> For many setups the bang-bang governor is exactly what we want. Many
> ARM SoC-based devices use fans to cool down the entire SoC and that
> works well only with the bang-bang governor because it uses the
> hysteresis in order to let the fan run for a while to cool the SoC
> down below the trip point before switching it off again.
> 
> The step-wise governor will behave strangely in these situations. It
> doesn't use the hysteresis, so it can lead to situations where the fan
> is turned on for only a very brief period and then is switched back off,
> only to get switched back on again very quickly because the SoC hasn't
> cooled down very much.

Applied, thanks
diff mbox series

Patch

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 4cd7ab707315..19a4b33cb564 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -130,6 +130,14 @@  config THERMAL_DEFAULT_GOV_POWER_ALLOCATOR
 	  system and device power allocation. This governor can only
 	  operate on cooling devices that implement the power API.
 
+config THERMAL_DEFAULT_GOV_BANG_BANG
+	bool "bang_bang"
+	depends on THERMAL_GOV_BANG_BANG
+	help
+	  Use the bang_bang governor as default. This throttles the
+	  devices one step at the time, taking into account the trip
+	  point hysteresis.
+
 endchoice
 
 config THERMAL_GOV_FAIR_SHARE
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 3d4a787c6b28..17c1bbed734d 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -23,6 +23,8 @@ 
 #define DEFAULT_THERMAL_GOVERNOR       "user_space"
 #elif defined(CONFIG_THERMAL_DEFAULT_GOV_POWER_ALLOCATOR)
 #define DEFAULT_THERMAL_GOVERNOR       "power_allocator"
+#elif defined(CONFIG_THERMAL_DEFAULT_GOV_BANG_BANG)
+#define DEFAULT_THERMAL_GOVERNOR       "bang_bang"
 #endif
 
 /* Initial state of a cooling device during binding */