Re: [PATCH V4 4/4] thermal/drivers/cpu_cooling: Rename to cpufreq_cooling

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 09/12/2019 13:03, Daniel Lezcano wrote:
> On 09/12/2019 10:54, Martin Kepplinger wrote:
>>
>>
>> On 06.12.19 15:15, Daniel Lezcano wrote:
>>> On 06/12/2019 12:33, Martin Kepplinger wrote:
>>>> I tested this on the librem5-devkit and see the
>>>> cooling devices in sysfs. I configure ARM_PSCI_CPUIDLE, not ARM_CPUIDLE and
>>>> add the patch below in register the cooling device there. "psci_idle"
>>>> is listed as the cpuidle_driver.
>>>>
>>>> That's what I'm running, in case you want to see it all:
>>>> https://source.puri.sm/martin.kepplinger/linux-next/commits/next-20191205/librem5_cpuidle_mainline_atf
>>>>
>>>> so I add a trip temperature description like this:
>>>> https://source.puri.sm/martin.kepplinger/linux-next/commit/361f49f93ae2c477fd012790831cabd0ed976660
>>>>
>>>> When I let the SoC heat up, cpuidle cooling won't kick it. In sysfs:
>>>>
>>>> catting the relevant files in /sys/class/thermal after heating up,
>>>> if that makes sense:
>>>>
>>>> 87000
>>>> 85000
>>>> 85000
>>>> thermal-cpufreq-0
>>>> 1
>>>> thermal-idle-0
>>>> 0
>>>> thermal-idle-1                                                                  
>>>> 0                                                                               
>>>> thermal-idle-2
>>>> 0
>>>> thermal-idle-3
>>>> 0
>>>>
>>>> with ARM_CPUIDLE instead of ARM_PSCI_CPUIDLE (and registering the cooling dev
>>>> during cpuidle-arm.c init) I won't have a cpuidle driver and thus no cpu-sleep
>>>> state at all.
>>>>
>>>> Can you see where the problem here lies?
>>>
>>> Yes, I removed the registration via the DT.
>>>
>>> Can you try the following:
>>>
>>> diff --git a/drivers/cpuidle/dt_idle_states.c
>>> b/drivers/cpuidle/dt_idle_states.c
>>> index d06d21a9525d..01367ddec49a 100644
>>> --- a/drivers/cpuidle/dt_idle_states.c
>>> +++ b/drivers/cpuidle/dt_idle_states.c
>>> @@ -13,6 +13,7 @@
>>>  #include <linux/errno.h>
>>>  #include <linux/kernel.h>
>>>  #include <linux/module.h>
>>> +#include <linux/cpu_cooling.h>
>>>  #include <linux/of.h>
>>>  #include <linux/of_device.h>
>>>
>>> @@ -205,6 +206,9 @@ int dt_init_idle_driver(struct cpuidle_driver *drv,
>>>  			err = -EINVAL;
>>>  			break;
>>>  		}
>>> +
>>> +		cpuidle_of_cooling_register(state_node, drv);
>>> +
>>>  		of_node_put(state_node);
>>>  	}
>>>
>>> That's a hack for the moment.
>>>
>>
>> thanks. I could test that successfully. The only question would be: Is
>> is intentional how "non-aggressive" the cooling driver cools? I would
>> have expected it to basically inject more idle cycles earlier. I'd set
>> 75 degrees as trip point and at 85 degress is would only inject about 30
>> (of 100).

By the way, how many CPUs are injecting idle cycle when the mitigation
happens ?

>> You describe the "config values" in question in the documentation, but
>> I'm not sure what's the correct way to change them.
> 
> That is difficult to say without knowing the board behavior. Are you
> able to profile the temperature with the load? How fast the temperature
> increases? The aggressive behavior of the cooling device will depend on
> the governor which depends on the slope of the temperature increase and
> the sampling.
> 
> Can you give the pointer to the git tree with the DT definition of your
> board?
> 
> You can try by changing the idle duration to 10ms instead of the default
> 4ms.
> 
> You can also change the cooling states in the DT <&state 20 70>, so it
> will begin to mitigate at state 20. But I wouldn't recommend that.
> 
> Do you have the energy power model, so we can try with 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