lkml.org 
[lkml]   [2017]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V2] thermal/drivers/step_wise: Fix temperature regulation misbehavior
From
Date


On Friday 08 September 2017 05:17 PM, Daniel Lezcano wrote:
> On 08/09/2017 11:49, Keerthy wrote:
>>
>>
>> On Friday 08 September 2017 02:35 PM, Daniel Lezcano wrote:
>>> There is a particular situation when the cooling device is cpufreq and the heat
>>> dissipation is not efficient enough where the temperature increases little by
>>> little until reaching the critical threshold and leading to a SoC reset.
>>>
>>> The behavior is reproducible on a hikey6220 with bad heat dissipation (eg.
>>> stacked with other boards).
>>>
>>> Running a simple C program doing while(1); for each CPU of the SoC makes the
>>> temperature to reach the passive regulation trip point and ends up to the
>>> maximum allowed temperature followed by a reset.
>>>
>>> This issue has been also reported by running the libhugetlbfs test suite.
>>>
>>> What is observed is a ping pong between two cpu frequencies, 1.2GHz and 900MHz
>>> while the temperature continues to grow.
>>>
>>> It appears the step wise governor calls get_target_state() the first time with
>>> the throttle set to true and the trend to 'raising'. The code selects logically
>>> the next state, so the cpu frequency decreases from 1.2GHz to 900MHz, so far so
>>> good. The temperature decreases immediately but still stays greater than the
>>> trip point, then get_target_state() is called again, this time with the
>>> throttle set to true *and* the trend to 'dropping'. From there the algorithm
>>> assumes we have to step down the state and the cpu frequency jumps back to
>>> 1.2GHz. But the temperature is still higher than the trip point, so
>>> get_target_state() is called with throttle=1 and trend='raising' again, we jump
>>> to 900MHz, then get_target_state() is called with throttle=1 and
>>> trend='dropping', we jump to 1.2GHz, etc ... but the temperature does not
>>> stabilizes and continues to increase.
>>>
>>> [ 237.922654] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=1,throttle=1
>>> [ 237.922678] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=1,throttle=1
>>> [ 237.922690] thermal cooling_device0: cur_state=0
>>> [ 237.922701] thermal cooling_device0: old_target=0, target=1
>>> [ 238.026656] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=2,throttle=1
>>> [ 238.026680] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=2,throttle=1
>>> [ 238.026694] thermal cooling_device0: cur_state=1
>>> [ 238.026707] thermal cooling_device0: old_target=1, target=0
>>> [ 238.134647] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=1,throttle=1
>>> [ 238.134667] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=1,throttle=1
>>> [ 238.134679] thermal cooling_device0: cur_state=0
>>> [ 238.134690] thermal cooling_device0: old_target=0, target=1
>>>
>>> In this situation the temperature continues to increase while the trend is
>>> oscillating between 'dropping' and 'raising'. We need to keep the current state
>>> untouched if the throttle is set, so the temperature can decrease or a higher
>>> state could be selected, thus prevening this oscillation.
>>>
>>> Keeping the next_target untouched when 'throttle' is true at 'dropping' time
>>> fixes the issue.
>>>
>>> The following traces show the governor does not change the next state if
>>> trend==2 (dropping) and throttle==1.
>>>
>>> [ 2306.127987] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=1,throttle=1
>>> [ 2306.128009] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=1,throttle=1
>>> [ 2306.128021] thermal cooling_device0: cur_state=0
>>> [ 2306.128031] thermal cooling_device0: old_target=0, target=1
>>> [ 2306.231991] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=2,throttle=1
>>> [ 2306.232016] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=2,throttle=1
>>> [ 2306.232030] thermal cooling_device0: cur_state=1
>>> [ 2306.232042] thermal cooling_device0: old_target=1, target=1
>>> [ 2306.335982] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=0,throttle=1
>>> [ 2306.336006] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=0,throttle=1
>>> [ 2306.336021] thermal cooling_device0: cur_state=1
>>> [ 2306.336034] thermal cooling_device0: old_target=1, target=1
>>> [ 2306.439984] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=2,throttle=1
>>> [ 2306.440008] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=2,throttle=0
>>> [ 2306.440022] thermal cooling_device0: cur_state=1
>>> [ 2306.440034] thermal cooling_device0: old_target=1, target=0
>>>
>>> [ ... ]
>>>
>>> After a while, if the temperature continues to increase, the next state becomes
>>> 2 which is 720MHz on the hikey. That results in the temperature stabilizing
>>> around the trip point.
>>>
>>> [ 2455.831982] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=1,throttle=1
>>> [ 2455.832006] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=1,throttle=0
>>> [ 2455.832019] thermal cooling_device0: cur_state=1
>>> [ 2455.832032] thermal cooling_device0: old_target=1, target=1
>>> [ 2455.935985] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=0,throttle=1
>>> [ 2455.936013] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=0,throttle=0
>>> [ 2455.936027] thermal cooling_device0: cur_state=1
>>> [ 2455.936040] thermal cooling_device0: old_target=1, target=1
>>> [ 2456.043984] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=0,throttle=1
>>> [ 2456.044009] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=0,throttle=0
>>> [ 2456.044023] thermal cooling_device0: cur_state=1
>>> [ 2456.044036] thermal cooling_device0: old_target=1, target=1
>>> [ 2456.148001] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=1,throttle=1
>>> [ 2456.148028] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=1,throttle=1
>>> [ 2456.148042] thermal cooling_device0: cur_state=1
>>> [ 2456.148055] thermal cooling_device0: old_target=1, target=2
>>> [ 2456.252009] thermal thermal_zone0: Trip0[type=1,temp=65000]:trend=2,throttle=1
>>> [ 2456.252041] thermal thermal_zone0: Trip1[type=1,temp=75000]:trend=2,throttle=0
>>> [ 2456.252058] thermal cooling_device0: cur_state=2
>>> [ 2456.252075] thermal cooling_device0: old_target=2, target=1
>>>
>>> IOW, this change is needed to keep the state for a cooling device if the
>>> temperature trend is oscillating while the temperature increases slightly.
>>>
>>> Without this change, the situation above leads to a catastrophic crash by a
>>> hardware reset on hikey.
>>
>> Daniel,
>>
>> By design this governor is throttling and un-throttling based on the
>> computed trend.
>
> Hi Keerthy,
>
> that was true until commit 3dbfff3d (Nov 2012) but now the function has:

Daniel,

I understand that commit. It still is stepwise every time you see the
trend toggling you end up changing the cooling level. That is what i meant.

>
> [ ... ]
>
> 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;
> }
> break;
>
> If "the trend is raising and we have to throttle" then state++
>
> The change I'm proposing is an action which is the mirror of the one above.
>
> If "the trend is dropping and we don't have to throttle" then state--

I understand this completely.

>
>
>> Why not add an intermediate trip point with the highest cooling enabled.
>> Say High alert trip point that allows only the lowest OPP for cpufreq to
>> operate.
>>
>> For example: alert trip is at 100C (where cpufreq cooling kicks in)
>> critical trip is at 125C(shutdown temperature).
>>
>> We have Something like below @110C which allows only the lowest
>> frequency or lowest 2 frequencies based on experimentation:
>>
>> +&cpu_trips {
>> + cpu_high_alert: cpu_high_alert {
>> + temperature = <110000>; /* millicelsius */
>> + hysteresis = <2000>; /* millicelsius */
>> + type = "passive";
>> + };
>> +};
>> +
>> +&cpu_cooling_maps {
>> + map1: map1 {
>> + trip = <&cpu_high_alert>;
>> + cooling-device =
>> + <&cpu0 2 THERMAL_NO_LIMIT>;
>> + };
>> +};
>> +
>>
>> I have seen this problem myself on dra7 platforms. The above will cool
>> the device post 110C by keeping the cpu frequency at the lowest or lower
>> values. That way when you are in between 100 - 110C you can still get
>> highest performance depending on the trend computed and post 110C.
>
> Yeah, so we can drop the graveyard legendary card at clash royal without
> any lag and boil our hands ;)
>
> Seriously, from my POV, that is a hack to workaround a governor which is
> unable to stabilize the temperature in a specific situation.
>
> This patch fixes this.


This definitely is an issue i have seen on dra7 and if step_wise by
design can accommodate this change, It will fix the issue!

Then you should be updating the documentation as well.

/*
* If the temperature is higher than a trip point,
* a. if the trend is THERMAL_TREND_RAISING, use higher cooling
* state for this trip point
* b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
* state for this trip point

Now with your patch applied the point b. will change to:

b. if the trend is THERMAL_TREND_DROPPING, Maintain the same cooling
level till the temperature drops below trip point.

Regards,
Keerthy

>
> -- Daniel
>

\
 
 \ /
  Last update: 2017-09-08 14:26    [W:0.085 / U:1.696 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site