Re: [PATCH] drivers: thermal: step_wise: add support for hysteresis

From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Ram Chandrasekar <rkumbako@codeaurora.org>,
	Amit Kucheria <amit.kucheria@linaro.org>,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	bjorn.andersson@linaro.org, swboyd@chromium.org,
	j-keerthy@ti.com, thara.gopinath@linaro.org,
	Zhang Rui <rui.zhang@intel.com>,
	Eduardo Valentin <edubezval@gmail.com>,
	Amit Kucheria <amit.kucheria@verdurent.com>
Cc: Lina Iyer <ilina@codeaurora.org>, linux-pm@vger.kernel.org
Subject: Re: [PATCH] drivers: thermal: step_wise: add support for hysteresis
Date: Thu, 9 Jan 2020 23:46:13 +0100	[thread overview]
Message-ID: <211a066d-f801-fb21-a14d-2b00765d2547@linaro.org> (raw)
In-Reply-To: <4f75a862-4525-fe11-ed03-f53a13926c15@codeaurora.org>

On 08/01/2020 01:31, Ram Chandrasekar wrote:
> 
> 
> On 12/11/2019 6:35 AM, Daniel Lezcano wrote:
>> On 21/11/2019 06:50, Amit Kucheria wrote:
>>> From: Ram Chandrasekar <rkumbako@codeaurora.org>
>>>
>>> Currently, step wise governor increases the mitigation when the
>>> temperature goes above a threshold and decreases the mitigation when the
>>> temperature goes below the threshold.
>>>
>>> If there is a case where the
>>> temperature is wavering around the threshold, the mitigation will be
>>> applied and removed every iteration, which is not very efficient.
>>>
>>> The use of hysteresis temperature could avoid this ping-pong of
>>> mitigation by relaxing the mitigation to happen only when the
>>> temperature goes below this lower hysteresis value.
>>
>> What I'm worried about is how the hysteresis is used in the current
>> code, where the destination of this data is to set the value in the
>> sensor hardware if it is supported.
>>
>> Using the hysteresis in the governor seems like abusing the initial
>> purpose of this information.
>>
>> Moreover, the hysteresis creates a gray area where the above algorithm
>> (DROPPING && !throttle) => state-- or (RAISING && throttle) => state++
>> may drop the performances because we will continue mitigating even below
>> the threshold.
>>
>> As the governor is an open-loop controller, I'm not sure if we can do
>> something except adding some kind of low pass filter to prevent
>> mitigation bounces.
>>
> 
> We have two different use cases for the step wise algorithm, and the
> hysteresis makes sense only in one.
> 
> For example, say we are controlling CPU junction temperature at 95C.
> When using step wise, mitigation is applied iteratively and there is a
> possibility that temperature can shoot up before the algorithm can reach
> an optimal mitigation level to keep the temperature near threshold.
> 
> In order to help this state machine, we use a second back stop rule in
> the same thermal zone at a higher temperature (say 105C) with a
> hysteresis of 10C to mitigate CPU to a fixed value, by specifying
> upper/lower limit to be the same. The idea is that the second rule will
> place a hard hammer to bring the temperature down close to 95C and then
> it will remove the mitigation. Once mitigation is removed, the junction
> temperature rule state machine will re-adjust from that point to an
> optimal mitigation level. The junction temperature rule doesn’t use
> hysteresis.
> 
> Another example is skin temperature mitigation for mobile devices, where
> the step wise algorithm with hysteresis just reduces the operating max
> frequency to a fixed value, when the threshold is reached. And the
> junction temperature rule starts mitigating from this operating max.
> 
> That is the reason we have not generalized or mandated the hysteresis
> usage in this patch. The idea is to use it selectively based on use case.

Did you ever try the IPA governor?

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