Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3c501536-2620-0022-df93-415d490fc1a5@linaro.org>
Date:   Mon, 12 Jun 2023 11:07:07 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH] thermal: Allow selecting the bang-bang governor as
 default
Content-Language: en-US
To:     Thierry Reding <thierry.reding@gmail.com>,
        "Rafael J . Wysocki" <rafael@kernel.org>
Cc:     Amit Kucheria <amitk@kernel.org>, Zhang Rui <rui.zhang@intel.com>,
        linux-pm@vger.kernel.org, linux-tegra@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230609124408.3788680-1-thierry.reding@gmail.com>
From:   Daniel Lezcano <daniel.lezcano@linaro.org>
In-Reply-To: <20230609124408.3788680-1-thierry.reding@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


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

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