On Fri, Jun 15, 2018 at 11:12 PM, Srinivas Pandruvada <srinivas.pandruv...@linux.intel.com> wrote: > When scaling max/min settings are changed, internally they are converted > to a ratio using the max turbo 1 core turbo frequency. This works fine > when 1 core max is same irrespective of the core. But under Turbo 3.0, > this will not be the case. For example: > Core 0: max turbo pstate: 43 (4.3GHz) > Core 1: max turbo pstate: 45 (4.5GHz) > In this case 1 core turbo ratio will be maximum of all, so it will be > 45 (4.5GHz). Suppose scaling max is set to 4GHz (ratio 40) for all cores > ,then on core one it will be > = max_state * policy->max / max_freq; > = 43 * (4000000/4500000) = 38 (3.8GHz) > = 38 > which is 200MHz less than the desired. > On core2, it will be correctly set to ratio 40 (4GHz). Same holds true > for scaling min frequency limit. So this requires usage of correct turbo > max frequency for core one, which in this case is 4.3GHz. So we need to > adjust per CPU cpu->pstate.turbo_freq using the maximum HWP ratio of that > core. > > This change uses the HWP capability of a core to adjust max turbo > frequency. But since Broadwell HWP doesn't use ratios in the HWP > capabilities, we have use legacy max 1 core turbo ratio. This is not a > problem as the HWP capabilities don't differ among cores in Broadwell. > So we need to check for non Broadwell CPU model for applying this change. > > While doing this change also optimize usage of x86_match_cpu(). Currently > using, different x86_cpu_id structure values for each feature. We can > consolidate, so that we don't have to introduce one more here. Added HWP > feature flag to each CPU model, instead of a new definition.
I'd prefer a fix that can be propagated to -stable and then the optimization as a separate patch. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruv...@linux.intel.com> > --- > drivers/cpufreq/intel_pstate.c | 43 > ++++++++++++++++++++++++++++++------------ > 1 file changed, 31 insertions(+), 12 deletions(-) > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > index 1de5ec8..26d1fc7 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -308,6 +308,12 @@ static struct global_params global; > static DEFINE_MUTEX(intel_pstate_driver_lock); > static DEFINE_MUTEX(intel_pstate_limits_lock); > > +#define INTEL_PSTATE_HWP_FEATURE_DISABLE_EE 0x01 > +#define INTEL_PSTATE_HWP_PERF_BOOST 0x02 > +#define INTEL_PSTATE_HWP_BROADWELL 0x04 > + > +static u32 intel_pstate_hwp_features; Why isn't it a member of strict cpudata? > + > #ifdef CONFIG_ACPI > > static bool intel_pstate_get_ppc_enable_status(void) > @@ -1413,7 +1419,16 @@ static void intel_pstate_get_cpu_pstates(struct > cpudata *cpu) > cpu->pstate.turbo_pstate = pstate_funcs.get_turbo(); > cpu->pstate.scaling = pstate_funcs.get_scaling(); > cpu->pstate.max_freq = cpu->pstate.max_pstate * cpu->pstate.scaling; > - cpu->pstate.turbo_freq = cpu->pstate.turbo_pstate * > cpu->pstate.scaling; > + > + if (hwp_active && > + !(intel_pstate_hwp_features & INTEL_PSTATE_HWP_BROADWELL)) { > + unsigned int phy_max, current_max; > + > + intel_pstate_get_hwp_max(cpu->cpu, &phy_max, ¤t_max); > + cpu->pstate.turbo_freq = phy_max * cpu->pstate.scaling; > + } else { > + cpu->pstate.turbo_freq = cpu->pstate.turbo_pstate * > cpu->pstate.scaling; > + } > > if (pstate_funcs.get_aperf_mperf_shift) > cpu->aperf_mperf_shift = pstate_funcs.get_aperf_mperf_shift(); > @@ -1789,14 +1804,16 @@ static const struct x86_cpu_id > intel_pstate_cpu_oob_ids[] __initconst = { > {} > }; > > -static const struct x86_cpu_id intel_pstate_cpu_ee_disable_ids[] = { > - ICPU(INTEL_FAM6_KABYLAKE_DESKTOP, core_funcs), > - {} > -}; > - > -static const struct x86_cpu_id intel_pstate_hwp_boost_ids[] = { > - ICPU(INTEL_FAM6_SKYLAKE_X, core_funcs), > - ICPU(INTEL_FAM6_SKYLAKE_DESKTOP, core_funcs), > +#define ICPU1(model, feature) \ > + { X86_VENDOR_INTEL, 6, model, X86_FEATURE_APERFMPERF,\ > + feature} > + > +static const struct x86_cpu_id intel_pstate_hwp_feature_ids[] = { > + ICPU1(INTEL_FAM6_BROADWELL_X, INTEL_PSTATE_HWP_BROADWELL), > + ICPU1(INTEL_FAM6_BROADWELL_XEON_D, INTEL_PSTATE_HWP_BROADWELL), > + ICPU1(INTEL_FAM6_KABYLAKE_DESKTOP, > INTEL_PSTATE_HWP_FEATURE_DISABLE_EE), > + ICPU1(INTEL_FAM6_SKYLAKE_DESKTOP, INTEL_PSTATE_HWP_PERF_BOOST), > + ICPU1(INTEL_FAM6_SKYLAKE_X, INTEL_PSTATE_HWP_PERF_BOOST), > {} > }; > > @@ -1825,14 +1842,16 @@ static int intel_pstate_init_cpu(unsigned int cpunum) > if (hwp_active) { > const struct x86_cpu_id *id; > > - id = x86_match_cpu(intel_pstate_cpu_ee_disable_ids); > + id = x86_match_cpu(intel_pstate_hwp_feature_ids); > if (id) > + intel_pstate_hwp_features = id->driver_data; > + > + if (intel_pstate_hwp_features & > INTEL_PSTATE_HWP_FEATURE_DISABLE_EE) > intel_pstate_disable_ee(cpunum); > > intel_pstate_hwp_enable(cpu); > > - id = x86_match_cpu(intel_pstate_hwp_boost_ids); > - if (id) > + if (intel_pstate_hwp_features & INTEL_PSTATE_HWP_PERF_BOOST) > hwp_boost = true; > } > > -- > 2.7.5 >