Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH V3 resend] cpufreq: Cap the default transition delay value to 10 ms | Date | Tue, 22 Aug 2017 15:31:14 +0200 |
| |
On Thursday, August 17, 2017 5:42:27 AM CEST Viresh Kumar wrote: > If transition_delay_us isn't defined by the cpufreq driver, the default > value of transition delay (time after which the cpufreq governor will > try updating the frequency again) is currently calculated by multiplying > transition_latency (nsec) with LATENCY_MULTIPLIER (1000) and then > converting this time to usec. That gives the exact same value as > transition_latency, just that the time unit is usec instead of nsec. > > With acpi-cpufreq for example, transition_latency is set to around 10 > usec and we get transition delay as 10 ms. Which seems to be a > reasonable amount of time to reevaluate the frequency again. > > But for platforms where frequency switching isn't that fast (like ARM), > the transition_latency varies from 500 usec to 3 ms, and the transition > delay becomes 500 ms to 3 seconds. Of course, that is a pretty bad > default value to start with. > > We can try to come across a better formula (instead of multiplying with > LATENCY_MULTIPLIER) to solve this problem, but will that be worth it ? > > This patch tries a simple approach and caps the maximum value of default > transition delay to 10 ms. Of course, userspace can still come in and > change this value anytime or individual drivers can rather provide > transition_delay_us instead. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > One of the IMX shows odd behavior after this patch is applied and stays > at max freq for little longer. More detailed debugging is required to be > done by some IMX guys (who haven't replied for the last 3 weeks on that > email thread, apart from Leonard). > > As per Leonard as well, we shouldn't stop this patch from getting merged > and so sending it again. > > drivers/cpufreq/cpufreq.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index b2cc98551fc3..c7ae67d6886d 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -532,8 +532,19 @@ unsigned int cpufreq_policy_transition_delay_us(struct cpufreq_policy *policy) > return policy->transition_delay_us; > > latency = policy->cpuinfo.transition_latency / NSEC_PER_USEC; > - if (latency) > - return latency * LATENCY_MULTIPLIER; > + if (latency) { > + /* > + * For platforms that can change the frequency very fast (< 10 > + * us), the above formula gives a decent transition delay. But > + * for platforms where transition_latency is in milliseconds, it > + * ends up giving unrealistic values. > + * > + * Cap the default transition delay to 10 ms, which seems to be > + * a reasonable amount of time after which we should reevaluate > + * the frequency. > + */ > + return min(latency * LATENCY_MULTIPLIER, (unsigned int)10000); > + } > > return LATENCY_MULTIPLIER; > } >
Applied, thanks!
| |