Messages in this thread | | | Date | Tue, 27 Jun 2017 09:50:41 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH] cpufreq: dt: Set default policy->transition_delay_ns |
| |
On 27-06-17, 02:15, Rafael J. Wysocki wrote: > On Monday, May 22, 2017 04:57:27 PM Viresh Kumar wrote: > > On 22-05-17, 19:17, Leo Yan wrote: > > > This afternoon Amit pointed me for this patch, should fix as below? > > > Otherwise it seems directly assign the same value from unit 'ns' to > > > 'us' but without any value conversion. > > > > > > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c > > > index 76877a6..dcc90fc 100644 > > > --- a/kernel/sched/cpufreq_schedutil.c > > > +++ b/kernel/sched/cpufreq_schedutil.c > > > @@ -538,7 +538,7 @@ static int sugov_init(struct cpufreq_policy *policy) > > > unsigned int lat; > > > > > > tunables->rate_limit_us = LATENCY_MULTIPLIER; > > > - lat = policy->cpuinfo.transition_latency / NSEC_PER_USEC;
I think the above line is just fine and the below one is incorrect, as we wanted to convert transition latency to usec here (i.e. in the units of rate_limit_us).
> > > + lat = policy->cpuinfo.transition_latency / NSEC_PER_MSEC; > > > if (lat) > > > tunables->rate_limit_us *= lat; > > > } > > > > I will let Rafael comment in as well. NSEC_PER_USEC is used in the > > earlier governors as well (ondemand/conservative) in exactly the same > > way as schedutil is using. > > The reason why it is used by schedutil is because the other governors used it > that way. IOW, doesn't matter. :-)
But I feel the value of LATENCY_MULTIPLIER (1000) is way too high. It currently says that if freq-switching takes time X, then we should wait for 999X time before we change the freq again.
Perhaps LATENCY_MULTIPLIER should be just 10 or 20 here. For a platform with transition_latency 500 us, rate_limit_us comes to 500 ms. Which is absurd. We ideally want it to be around 10-20 ms here. And compared to other ARM platforms, 500 us transition_latency is very low. It normally is around 1-3 ms for ARM32 platforms.
@Rafael: Will it be fine to lower down the value of LATENCY_MULTIPLIER?
-- viresh
| |