[PATCH 0/2][v2] Get percpu max freq via HWP MSR register

[PATCH 0/2][v2] Get percpu max freq via HWP MSR register

[Chen Yu]

Asymmetric platforms might have different max cpu frequency between
small and big cores. Currently the intel_pstate driver uses package wide MSR
register that can not distinguish max cpu frequency between small and big cores
when turbo is disabled, which causes inconsistency compared to the scenario when
turbo mode is enabled. This patch changes the logic from package wide MSR register
to percpu HWP register so as to avoid this issue.

This path is based on Rafael's previous patchset to clean up the intel_pstate_get_hwp_max()
https://patchwork.kernel.org/project/linux-pm/patch/2241039.bdjsIDbar3@kreacher/


Chen Yu (2):
  cpufreq: intel_pstate: Add parameter to get guarantee
  cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if
    available

 drivers/cpufreq/intel_pstate.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

-- 
2.17.1

[PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency

[Chen Yu]

Add input parameter to receive guarantee pstate from intel_pstate_get_hwp_max()
for later use.

No functional change intended.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index eaf32ef7a030..bd3dd1be73ba 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -830,7 +830,7 @@ static struct freq_attr *hwp_cpufreq_attrs[] = {
 };
 
 static void intel_pstate_get_hwp_max(struct cpudata *cpu, int *phy_max,
-				     int *current_max)
+				     int *current_max, int *guar_max)
 {
 	u64 cap;
 
@@ -842,6 +842,7 @@ static void intel_pstate_get_hwp_max(struct cpudata *cpu, int *phy_max,
 		*current_max = HWP_HIGHEST_PERF(cap);
 
 	*phy_max = HWP_HIGHEST_PERF(cap);
+	*guar_max = HWP_GUARANTEED_PERF(cap);
 }
 
 static void intel_pstate_hwp_set(unsigned int cpu)
@@ -1205,7 +1206,7 @@ static ssize_t store_no_turbo(struct kobject *a, struct kobj_attribute *b,
 
 static void update_qos_request(enum freq_qos_req_type type)
 {
-	int max_state, turbo_max, freq, i, perf_pct;
+	int max_state, turbo_max, guar_state, freq, i, perf_pct;
 	struct freq_qos_request *req;
 	struct cpufreq_policy *policy;
 
@@ -1223,7 +1224,7 @@ static void update_qos_request(enum freq_qos_req_type type)
 			continue;
 
 		if (hwp_active)
-			intel_pstate_get_hwp_max(cpu, &turbo_max, &max_state);
+			intel_pstate_get_hwp_max(cpu, &turbo_max, &max_state, &guar_state);
 		else
 			turbo_max = cpu->pstate.turbo_pstate;
 
@@ -1731,9 +1732,9 @@ static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
 	cpu->pstate.max_freq = cpu->pstate.max_pstate * cpu->pstate.scaling;
 
 	if (hwp_active && !hwp_mode_bdw) {
-		unsigned int phy_max, current_max;
+		unsigned int phy_max, current_max, guar_state;
 
-		intel_pstate_get_hwp_max(cpu, &phy_max, &current_max);
+		intel_pstate_get_hwp_max(cpu, &phy_max, &current_max, &guar_state);
 		cpu->pstate.turbo_freq = phy_max * cpu->pstate.scaling;
 		cpu->pstate.turbo_pstate = phy_max;
 	} else {
@@ -2209,7 +2210,7 @@ static void intel_pstate_update_perf_limits(struct cpudata *cpu,
 					    unsigned int policy_max)
 {
 	int32_t max_policy_perf, min_policy_perf;
-	int max_state, turbo_max;
+	int max_state, turbo_max, guar_state;
 	int max_freq;
 
 	/*
@@ -2218,7 +2219,7 @@ static void intel_pstate_update_perf_limits(struct cpudata *cpu,
 	 * rather than pure ratios.
 	 */
 	if (hwp_active) {
-		intel_pstate_get_hwp_max(cpu, &turbo_max, &max_state);
+		intel_pstate_get_hwp_max(cpu, &turbo_max, &max_state, &guar_state);
 	} else {
 		max_state = global.no_turbo || global.turbo_disabled ?
 			cpu->pstate.max_pstate : cpu->pstate.turbo_pstate;
@@ -2331,9 +2332,9 @@ static void intel_pstate_verify_cpu_policy(struct cpudata *cpu,
 
 	update_turbo_state();
 	if (hwp_active) {
-		int max_state, turbo_max;
+		int max_state, turbo_max, guar_state;
 
-		intel_pstate_get_hwp_max(cpu, &turbo_max, &max_state);
+		intel_pstate_get_hwp_max(cpu, &turbo_max, &max_state, &guar_state);
 		max_freq = max_state * cpu->pstate.scaling;
 	} else {
 		max_freq = intel_pstate_get_max_freq(cpu);
@@ -2691,7 +2692,7 @@ static void intel_cpufreq_adjust_perf(unsigned int cpunum,
 
 static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
 {
-	int max_state, turbo_max, min_freq, max_freq, ret;
+	int max_state, turbo_max, guar_state, min_freq, max_freq, ret;
 	struct freq_qos_request *req;
 	struct cpudata *cpu;
 	struct device *dev;
@@ -2719,7 +2720,7 @@ static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	if (hwp_active) {
 		u64 value;
 
-		intel_pstate_get_hwp_max(cpu, &turbo_max, &max_state);
+		intel_pstate_get_hwp_max(cpu, &turbo_max, &max_state, &guar_state);
 		policy->transition_delay_us = INTEL_CPUFREQ_TRANSITION_DELAY_HWP;
 		rdmsrl_on_cpu(cpu->cpu, MSR_HWP_REQUEST, &value);
 		WRITE_ONCE(cpu->hwp_req_cached, value);
-- 
2.17.1

[PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available

[Chen Yu]

Currently when turbo is disabled(either by BIOS or by the user), the intel_pstate
driver reads the max frequency from the package-wide MSR_PLATFORM_INFO(0xce) register.
However on asymmetric platforms it is possible in theory that small and big core with
HWP enabled might have different max cpu frequency, because the MSR_HWP_CAPABILITIES
is percpu scope according to Intel Software Developer Manual.

The turbo max freq is already percpu basis in current code, thus make similar change
to the max non-turbo frequency as well.

Reported-by: Wendy Wang <wendy.wang@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v2: per Srinivas' suggestion, avoid duplicated assignment of max_pstate.
--
 drivers/cpufreq/intel_pstate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index bd3dd1be73ba..f2d18991d969 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1725,11 +1725,9 @@ static void intel_pstate_max_within_limits(struct cpudata *cpu)
 static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
 {
 	cpu->pstate.min_pstate = pstate_funcs.get_min();
-	cpu->pstate.max_pstate = pstate_funcs.get_max();
 	cpu->pstate.max_pstate_physical = pstate_funcs.get_max_physical();
 	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;
 
 	if (hwp_active && !hwp_mode_bdw) {
 		unsigned int phy_max, current_max, guar_state;
@@ -1737,8 +1735,12 @@ static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
 		intel_pstate_get_hwp_max(cpu, &phy_max, &current_max, &guar_state);
 		cpu->pstate.turbo_freq = phy_max * cpu->pstate.scaling;
 		cpu->pstate.turbo_pstate = phy_max;
+		cpu->pstate.max_pstate = guar_state;
+		cpu->pstate.max_freq = guar_state * cpu->pstate.scaling;
 	} else {
 		cpu->pstate.turbo_freq = cpu->pstate.turbo_pstate * cpu->pstate.scaling;
+		cpu->pstate.max_pstate = pstate_funcs.get_max();
+		cpu->pstate.max_freq = cpu->pstate.max_pstate * cpu->pstate.scaling;
 	}
 
 	if (pstate_funcs.get_aperf_mperf_shift)
-- 
2.17.1

Re: [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available

[Srinivas Pandruvada]

On Mon, 2021-01-11 at 15:43 +0800, Chen Yu wrote:
> Currently when turbo is disabled(either by BIOS or by the user), the
> intel_pstate
> driver reads the max frequency from the package-wide
> MSR_PLATFORM_INFO(0xce) register.
> However on asymmetric platforms it is possible in theory that small
> and big core with
> HWP enabled might have different max cpu frequency
max non-turbo frequency (although code call max_freq).

Thanks,
Srinivas

> , because the MSR_HWP_CAPABILITIES
> is percpu scope according to Intel Software Developer Manual.
> 
> The turbo max freq is already percpu basis in current code, thus make
> similar change
> to the max non-turbo frequency as well.
> 
> Reported-by: Wendy Wang <wendy.wang@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
> v2: per Srinivas' suggestion, avoid duplicated assignment of
> max_pstate.
> --
>  drivers/cpufreq/intel_pstate.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c
> b/drivers/cpufreq/intel_pstate.c
> index bd3dd1be73ba..f2d18991d969 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1725,11 +1725,9 @@ static void
> intel_pstate_max_within_limits(struct cpudata *cpu)
>  static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
>  {
>         cpu->pstate.min_pstate = pstate_funcs.get_min();
> -       cpu->pstate.max_pstate = pstate_funcs.get_max();
>         cpu->pstate.max_pstate_physical =
> pstate_funcs.get_max_physical();
>         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;
>  
>         if (hwp_active && !hwp_mode_bdw) {
>                 unsigned int phy_max, current_max, guar_state;
> @@ -1737,8 +1735,12 @@ static void
> intel_pstate_get_cpu_pstates(struct cpudata *cpu)
>                 intel_pstate_get_hwp_max(cpu, &phy_max, &current_max,
> &guar_state);
>                 cpu->pstate.turbo_freq = phy_max * cpu-
> >pstate.scaling;
>                 cpu->pstate.turbo_pstate = phy_max;
> +               cpu->pstate.max_pstate = guar_state;
> +               cpu->pstate.max_freq = guar_state * cpu-
> >pstate.scaling;
>         } else {
>                 cpu->pstate.turbo_freq = cpu->pstate.turbo_pstate *
> cpu->pstate.scaling;
> +               cpu->pstate.max_pstate = pstate_funcs.get_max();
> +               cpu->pstate.max_freq = cpu->pstate.max_pstate * cpu-
> >pstate.scaling;
>         }
>  
>         if (pstate_funcs.get_aperf_mperf_shift)

Re: [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency

[Rafael J. Wysocki]

On Mon, Jan 11, 2021 at 8:40 AM Chen Yu <yu.c.chen@intel.com> wrote:
>
> Add input parameter to receive guarantee pstate from intel_pstate_get_hwp_max()
> for later use.

I'm not a fan of this change, because the new argument will only be
needed in one place where intel_pstate_get_hwp_max() and it requires
adding redundant local vars and pointless updates elsewhere.

Besides, in intel_pstate_get_cpu_pstates() you can do something like
this after calling intel_pstate_get_hwp_max():

cpu->pstate.max_pstate = HWP_GUARANTEED_PERF(READ_ONCE(cpu->hwp_cap_cached));
cpu->pstate.max_freq = cpu->pstate.max_pstate * cpu->pstate.scaling;

Re: [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available

[Chen Yu]

On Mon, Jan 11, 2021 at 03:11:02AM -0800, Srinivas Pandruvada wrote:
> On Mon, 2021-01-11 at 15:43 +0800, Chen Yu wrote:
> > Currently when turbo is disabled(either by BIOS or by the user), the
> > intel_pstate
> > driver reads the max frequency from the package-wide
> > MSR_PLATFORM_INFO(0xce) register.
> > However on asymmetric platforms it is possible in theory that small
> > and big core with
> > HWP enabled might have different max cpu frequency
> max non-turbo frequency (although code call max_freq).
>
Okay, will change.

Thanks,
Chenyu
> Thanks,
> Srinivas
> 

Re: [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency

[Chen Yu]

On Mon, Jan 11, 2021 at 01:44:09PM +0100, Rafael J. Wysocki wrote:
> On Mon, Jan 11, 2021 at 8:40 AM Chen Yu <yu.c.chen@intel.com> wrote:
> >
> > Add input parameter to receive guarantee pstate from intel_pstate_get_hwp_max()
> > for later use.
> 
> I'm not a fan of this change, because the new argument will only be
> needed in one place where intel_pstate_get_hwp_max() and it requires
> adding redundant local vars and pointless updates elsewhere.
> 
> Besides, in intel_pstate_get_cpu_pstates() you can do something like
> this after calling intel_pstate_get_hwp_max():
> 
> cpu->pstate.max_pstate = HWP_GUARANTEED_PERF(READ_ONCE(cpu->hwp_cap_cached));
> cpu->pstate.max_freq = cpu->pstate.max_pstate * cpu->pstate.scaling;
Okay, will do in next version.

Thanks,
Chenyu