[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJZ5v0gUyr1B2YUrPDf62rXjHxWR3Ss1noLbm6wDW5QJdv=jKA@mail.gmail.com>
Date: Mon, 18 Jun 2018 12:53:59 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc: Len Brown <lenb@...nel.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Viresh Kumar <viresh.kumar@...aro.org>,
Linux PM <linux-pm@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cpufreq: intel_pstate: Fix scaling max/min limits with
Turbo 3.0
On Fri, Jun 15, 2018 at 11:12 PM, Srinivas Pandruvada
<srinivas.pandruvada@...ux.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.pandruvada@...ux.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
>
Powered by blists - more mailing lists