Re: [PATCH v7 18/19] PM / devfreq: tegra30: Tune up MCCPU boost-down coefficient

[Dmitry Osipenko]

01.11.2019 10:45, Chanwoo Choi ÐÐÑÐÑ:
> On 19. 10. 30. ìì 7:00, Dmitry Osipenko wrote:
>> MCCPU boosts up very aggressively by 800% and boosts down very mildly by
>> 10%. This doesn't work well when system is idling because the very slow
>> de-boosting results in lots of consecutive-down interrupts, in result
>> memory stays clocked high and CPU doesn't enter deepest idling state
>> instead of keeping memory at lowest freq and having CPU cluster turned
>> off. A more faster de-boosting fixes the case of idling system and doesn't
>> affect the case of an active system.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>> ---
>>  drivers/devfreq/tegra30-devfreq.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>> index d0dd42856e5b..9a21a29198ee 100644
>> --- a/drivers/devfreq/tegra30-devfreq.c
>> +++ b/drivers/devfreq/tegra30-devfreq.c
>> @@ -123,7 +123,7 @@ static const struct tegra_devfreq_device_config actmon_device_configs[] = {
>>  		.offset = 0x200,
>>  		.irq_mask = 1 << 25,
>>  		.boost_up_coeff = 800,
>> -		.boost_down_coeff = 90,
>> +		.boost_down_coeff = 40,
>>  		.boost_up_threshold = 27,
>>  		.boost_down_threshold = 10,
>>  		.avg_dependency_threshold = 50000,
>>
> 
> Reviewed-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
> 
> IMO, I think that it is not good to change the threshold value
> on device driver directly when some requirement happen.
> Instead, better to get the threshold value from device-tree file.

I think in worst case these values could be tuned for a hardware
generation. The current hardcoded values should be good enough for everyone.

If we'll find that some differentiation is needed, then we could
consider making the pre-defined config per hardware generation by
specifying "data" fields of the of_device_id table.

It also should be possible add sysfs interface for userpace to change
the boosting values. Or make these values a driver's module parameters.

There are different variants of what could be done in order to
differentiate the configuration. But again, this is not needed for.