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
