From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751769AbdG1EsU (ORCPT ); Fri, 28 Jul 2017 00:48:20 -0400 Received: from mail-pg0-f48.google.com ([74.125.83.48]:32898 "EHLO mail-pg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751614AbdG1EsT (ORCPT ); Fri, 28 Jul 2017 00:48:19 -0400 Date: Fri, 28 Jul 2017 10:18:15 +0530 From: Viresh Kumar To: Peter Zijlstra Cc: Rafael Wysocki , Ingo Molnar , linux-pm@vger.kernel.org, Vincent Guittot , linux@dominikbrodowski.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH V3 2/9] cpufreq: Use transition_delay_us for legacy governors as well Message-ID: <20170728044815.GS352@vireshk-i7> References: <46734e7c365a024632e8b1d52990d4400899727b.1500373914.git.viresh.kumar@linaro.org> <20170724161719.g7d5puvyk2lpinyw@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170724161719.g7d5puvyk2lpinyw@hirez.programming.kicks-ass.net> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 24-07-17, 18:17, Peter Zijlstra wrote: > On Wed, Jul 19, 2017 at 03:42:42PM +0530, Viresh Kumar wrote: > > The policy->transition_delay_us field is used only by the schedutil > > governor currently, and this field describes how fast the driver wants > > the cpufreq governor to change CPUs frequency. It should rather be a > > common thing across all governors, as it doesn't have any schedutil > > dependency here. > > > > Create a new helper cpufreq_policy_transition_delay_us() to get the > > transition delay across all governors. > > > > Signed-off-by: Viresh Kumar > > --- > > drivers/cpufreq/cpufreq.c | 15 +++++++++++++++ > > drivers/cpufreq/cpufreq_governor.c | 9 +-------- > > include/linux/cpufreq.h | 1 + > > kernel/sched/cpufreq_schedutil.c | 11 +---------- > > 4 files changed, 18 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > > index 9bf97a366029..c426d21822f7 100644 > > --- a/drivers/cpufreq/cpufreq.c > > +++ b/drivers/cpufreq/cpufreq.c > > @@ -524,6 +524,21 @@ unsigned int cpufreq_driver_resolve_freq(struct cpufreq_policy *policy, > > } > > EXPORT_SYMBOL_GPL(cpufreq_driver_resolve_freq); > > > > +unsigned int cpufreq_policy_transition_delay_us(struct cpufreq_policy *policy) > > +{ > > + unsigned int latency; > > + > > + if (policy->transition_delay_us) > > + return policy->transition_delay_us; > > + > > + latency = policy->cpuinfo.transition_latency / NSEC_PER_USEC; > > + if (latency) > > + return latency * LATENCY_MULTIPLIER; > > + > > + return LATENCY_MULTIPLIER; > > +} > > +EXPORT_SYMBOL_GPL(cpufreq_policy_transition_delay_us); > > I realize you're just moving code about, but _why_ are we doing that > division? We are doing division by NSEC_PER_USEC because the values of sampling_rate and rate_limit_us are in us, while transition_latency is in ns. We shouldn't be breaking the userspace ABI and so if we want, we can actually make transition_latency be stored in us instead. Though I am not sure why are we multiplying with LATENCY_MULTIPLIER (1000) as that seems to be a huge value. -- viresh