* [PATCH 0/2][v2] Get percpu max freq via HWP MSR register
@ 2021-01-11 7:43 Chen Yu
2021-01-11 7:43 ` [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency Chen Yu
2021-01-11 7:43 ` [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available Chen Yu
0 siblings, 2 replies; 7+ messages in thread
From: Chen Yu @ 2021-01-11 7:43 UTC (permalink / raw)
To: Srinivas Pandruvada, Len Brown, Rafael J. Wysocki, Viresh Kumar
Cc: linux-pm, linux-kernel, Zhang Rui, Wendy Wang, 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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency 2021-01-11 7:43 [PATCH 0/2][v2] Get percpu max freq via HWP MSR register Chen Yu @ 2021-01-11 7:43 ` Chen Yu 2021-01-11 12:44 ` Rafael J. Wysocki 2021-01-11 7:43 ` [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available Chen Yu 1 sibling, 1 reply; 7+ messages in thread From: Chen Yu @ 2021-01-11 7:43 UTC (permalink / raw) To: Srinivas Pandruvada, Len Brown, Rafael J. Wysocki, Viresh Kumar Cc: linux-pm, linux-kernel, Zhang Rui, Wendy Wang, 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, ¤t_max); + intel_pstate_get_hwp_max(cpu, &phy_max, ¤t_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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency 2021-01-11 7:43 ` [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency Chen Yu @ 2021-01-11 12:44 ` Rafael J. Wysocki 2021-01-12 3:29 ` Chen Yu 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2021-01-11 12:44 UTC (permalink / raw) To: Chen Yu Cc: Srinivas Pandruvada, Len Brown, Rafael J. Wysocki, Viresh Kumar, Linux PM, Linux Kernel Mailing List, Zhang Rui, Wendy Wang 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; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency 2021-01-11 12:44 ` Rafael J. Wysocki @ 2021-01-12 3:29 ` Chen Yu 0 siblings, 0 replies; 7+ messages in thread From: Chen Yu @ 2021-01-12 3:29 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Srinivas Pandruvada, Len Brown, Rafael J. Wysocki, Viresh Kumar, Linux PM, Linux Kernel Mailing List, Zhang Rui, Wendy Wang 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available 2021-01-11 7:43 [PATCH 0/2][v2] Get percpu max freq via HWP MSR register Chen Yu 2021-01-11 7:43 ` [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency Chen Yu @ 2021-01-11 7:43 ` Chen Yu 2021-01-11 11:11 ` Srinivas Pandruvada 1 sibling, 1 reply; 7+ messages in thread From: Chen Yu @ 2021-01-11 7:43 UTC (permalink / raw) To: Srinivas Pandruvada, Len Brown, Rafael J. Wysocki, Viresh Kumar Cc: linux-pm, linux-kernel, Zhang Rui, Wendy Wang, 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, ¤t_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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available 2021-01-11 7:43 ` [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available Chen Yu @ 2021-01-11 11:11 ` Srinivas Pandruvada 2021-01-12 3:28 ` Chen Yu 0 siblings, 1 reply; 7+ messages in thread From: Srinivas Pandruvada @ 2021-01-11 11:11 UTC (permalink / raw) To: Chen Yu, Len Brown, Rafael J. Wysocki, Viresh Kumar Cc: linux-pm, linux-kernel, Zhang Rui, Wendy Wang 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, ¤t_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) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available 2021-01-11 11:11 ` Srinivas Pandruvada @ 2021-01-12 3:28 ` Chen Yu 0 siblings, 0 replies; 7+ messages in thread From: Chen Yu @ 2021-01-12 3:28 UTC (permalink / raw) To: Srinivas Pandruvada Cc: Len Brown, Rafael J. Wysocki, Viresh Kumar, linux-pm, linux-kernel, Zhang Rui, Wendy Wang 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-12 3:26 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-11 7:43 [PATCH 0/2][v2] Get percpu max freq via HWP MSR register Chen Yu 2021-01-11 7:43 ` [PATCH 1/2][v2] cpufreq: intel_pstate: Add parameter to get guarantee frequency Chen Yu 2021-01-11 12:44 ` Rafael J. Wysocki 2021-01-12 3:29 ` Chen Yu 2021-01-11 7:43 ` [PATCH 2/2][v2] cpufreq: intel_pstate: Get percpu max freq via HWP MSR register if available Chen Yu 2021-01-11 11:11 ` Srinivas Pandruvada 2021-01-12 3:28 ` Chen Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).