Re: [RFC][PATCH 7/7] cpufreq: intel_pstate: Change P-state selection algorithm for Core

[Rafael J. Wysocki]

On Friday, August 19, 2016 04:47:29 PM Peter Zijlstra wrote:
> On Sat, Aug 13, 2016 at 08:59:01AM -0700, Doug Smythies wrote:
> > My previous replies (and see below) have suggested that some filtering
> > is needed on the target pstate, otherwise, and dependant on the type of
> > workload, it tends to oscillate.
> > 
> > I added the IIR (Infinite Impulse Response) filter that I have suggested in the past:
> 
> One question though; why is this filter intel_pstate specific? Should we
> not do this in generic code?

The intel_pstate algorithm is based on the feedback registers and I'm not sure
if the same effect appears if utilization is computed in a different way.

> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index c43ef55..262ec5f 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -1313,7 +1318,74 @@ static inline int32_t get_target_pstate_default(struct cpudata *cpu)
> >         cpu->iowait_boost >>= 1;
> > 
> >         pstate = cpu->pstate.turbo_pstate;
> 
> > +       unfiltered_target = (pstate + (pstate >> 2)) * busy_frac;
> 
> > +       duration_ns = cpu->sample.time - cpu->last_sample_time;
> > +
> > +       scaled_gain = div_u64(int_tofp(duration_ns) *
> > +               int_tofp(pid_params.p_gain_pct), int_tofp(pid_params.sample_rate_ns));
> 
> Drop int_to_fp() on one of the dividend terms and in the divisor. Same
> end result since they divide away against one another but reduces the
> risk of overflow.
> 
> Also, sample_rate_ns, really!? A rate is in [1/s], should that thing be
> called period_ns ?

That's an old name that hasn't been changed for quite a while.  That said
"period" or "interval" would be better.

Thanks,
Rafael