Messages in this thread | | | From | Chanwoo Choi <> | Date | Wed, 11 Oct 2017 22:33:16 +0900 | Subject | Re: [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
| |