From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933018AbdHVNlR (ORCPT ); Tue, 22 Aug 2017 09:41:17 -0400 Received: from cloudserver094114.home.net.pl ([79.96.170.134]:62404 "EHLO cloudserver094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932528AbdHVNjr (ORCPT ); Tue, 22 Aug 2017 09:39:47 -0400 From: "Rafael J. Wysocki" To: Viresh Kumar Cc: linux-pm@vger.kernel.org, Vincent Guittot , leonard.crestez@nxp.com, kernel@pengutronix.de, shawnguo@kernel.org, fabio.estevam@nxp.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH V3 resend] cpufreq: Cap the default transition delay value to 10 ms Date: Tue, 22 Aug 2017 15:31:14 +0200 Message-ID: <2949076.nmXvTJI3EO@aspire.rjw.lan> In-Reply-To: <00a3cebb2febc73dd275cbff2a24c73fb11ff255.1502941122.git.viresh.kumar@linaro.org> References: <00a3cebb2febc73dd275cbff2a24c73fb11ff255.1502941122.git.viresh.kumar@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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!