Get percpu max freq via HWP MSR register

PATCH Superseded Chen Yu 3 participants 7 messages Jan 11, 2021 – Jan 12, 2021

Subsystems (2)

CPU frequency scaling framework Intel pstate driver

lore

Share & tools

lore

Chen Yu

author

2021-01-11 07:40 UTC #0 lore raw

v2 cover Get percpu max freq via HWP MSR register

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(-)

Signature · 1 line

2.17.1

Chen Yu

author

2021-01-11 07:40 UTC #1 lore raw

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

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 <[email protected]>

---
 drivers/cpufreq/intel_pstate.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

drivers/cpufreq/intel_pstate.c +12 ~~-11~~

index eaf32ef7a030..bd3dd1be73ba 100644
@@ -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);

Signature · 1 line

2.17.1

Rafael J. Wysocki

maintainer

2021-01-11 12:45 UTC #2 lore raw

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

Quoted 3 lines · Chen Yu

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;

Chen Yu

author

2021-01-12 03:26 UTC #3 lore raw

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

Quoted 9 lines · Rafael J. Wysocki

Quoted 3 lines · Chen Yu
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.

Quoted 5 lines

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

Chen Yu

author

2021-01-11 07:41 UTC #4 lore raw

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

In reply to 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 <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
v2: per Srinivas' suggestion, avoid duplicated assignment of max_pstate. --

 drivers/cpufreq/intel_pstate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/cpufreq/intel_pstate.c +4 -2

index bd3dd1be73ba..f2d18991d969 100644
@@ -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)

Signature · 1 line

2.17.1

Srinivas Pandruvada

maintainer

2021-01-11 11:13 UTC #5 lore raw

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

Quoted 7 lines · 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

max non-turbo frequency (although code call max_freq).

Thanks,
Srinivas

Quoted 55 lines

, 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 <[email protected]>
Signed-off-by Chen Yu <[email protected]>
---
v2: per Srinivas' suggestion, avoid duplicated assignment of max_pstate.
--
drivers/cpufreq/intel_pstate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/cpufreq/intel_pstate.c +0 -1
index bd3dd1be73ba..f2d18991d969 100644
@@ -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-
Quoted 1 line
pstate.scaling;

        if (hwp_active && !hwp_mode_bdw) {
                unsigned int phy_max, current_max, guar_state;
drivers/cpufreq/intel_pstate.c +0 -0
@@ -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-
Quoted 1 line
pstate.scaling;
                cpu->pstate.turbo_pstate = phy_max; +               cpu->pstate.max_pstate = guar_state; +               cpu->pstate.max_freq = guar_state * cpu-
Quoted 1 line
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-
Quoted 1 line
pstate.scaling;
        }

        if (pstate_funcs.get_aperf_mperf_shift)

Chen Yu

author

2021-01-12 03:26 UTC #6 lore raw

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

Quoted 10 lines · Srinivas Pandruvada

Quoted 7 lines · 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
max non-turbo frequency (although code call max_freq).

Okay, will change.

Thanks,
Chenyu

Quoted 3 lines

Thanks,
Srinivas