lkml.org 
[lkml]   [2017]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3 5/8] PM / devfreq: Get the available next frequency on update_devfreq()
On Wed, Oct 11, 2017 at 8:30 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>> The update_devfreq() considers only user frequency (min_freq/max_freq)
>> and the next target_freq provided by the governor. But, the commit
>> a76caf55e5b35 ("thermal: Add devfreq cooling") is able to disable
>> OPP as a cooling device. In result, the update_devfreq() have to
>> consider the 'opp->available' status in order to decicde the next freq
>> by the devfreq_recommended_opp().
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>> drivers/devfreq/devfreq.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 1c4b377cacfb..3b9662ffe603 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -255,6 +255,7 @@ static int devfreq_notify_transition(struct devfreq *devfreq,
>> int update_devfreq(struct devfreq *devfreq)
>> {
>> struct devfreq_freqs freqs;
>> + struct dev_pm_opp *opp;
>> unsigned long freq, cur_freq;
>> int err = 0;
>> u32 flags = 0;
>> @@ -273,7 +274,7 @@ int update_devfreq(struct devfreq *devfreq)
>> return err;
>>
>> /*
>> - * Adjust the frequency with user freq and QoS.
>> + * Adjust the frequency with user freq, QoS and available freq.
>> *
>> * List from the highest priority
>> * max_freq
>> @@ -289,6 +290,12 @@ int update_devfreq(struct devfreq *devfreq)
>> flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */
>> }
>>
>> + opp = devfreq_recommended_opp(devfreq->dev.parent, &freq, flags);
>> + if (IS_ERR(opp))
>> + return PTR_ERR(opp);
>> + freq = dev_pm_opp_get_freq(opp);
>> + dev_pm_opp_put(opp);
>> +
>
> Is this really necessary?

The requirement is due to devfreq_cooling device using
dev_pm_opp_disable/enable().

I added the detailed explanation on cover letter as following:
If this code is not included, the notifiee using TRANSITION_NOTIFIER
receives the wrong next target_freq. On the cpufreq, cpufreq doesn't
use the 'dev_pm_opp_disable/enable()' function and then there is no
the same issue on cpufreq.

[Cover letter's description about this patch]
For example,
- devfreq's min_freq is 100Mhz and max_freq is 700Mhz.
- OPP disabled 500/600/700Mhz due to devfreq-cooling.c.
- simple_ondemand govenor decided the next target_freq (600Mhz)
|----------|-------------------------------------------------------------|
|Freq(MHz) |100 |200 |300 |400 |500 |600 |70 0 |
|Devfreq |min_freq| | | | | |max_freq|
|OPP avail |enabled |enabled|enabled|enabled |Disabled| Disabled|Disabled|
|Ondmenad | | | | | |next_freq| |
|------------------------------------------------------------------------|

In result,
- Before this patch, target_freq is 600Mhz
and TRANSITION_NOTIFIER sends the next_freq is 600Mhz to the notifiee.
- After this patch, target_freq is 400Mhz because 500/600 were disabled by OPP.
and TRANSITION_NOTIFIER sends the next_freq is 400Mhz to the notifiee.
--------------

>
> devfreq_recommended_opp is going to be called by the device driver
> invoked by devfreq->profile->target() function anyway.
>
> We are now going to call devfreq_recommended_opp twice in this context.
>
>> if (devfreq->profile->get_cur_freq)
>> devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq);
>> else
>> --

Right. The devfreq_recommended_opp() is called twice.
I wish there was a better way.

--
Best Regards,
Chanwoo Choi
Samsung Electronics

\
 
 \ /
  Last update: 2017-10-11 15:34    [W:0.321 / U:0.484 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site