| Message ID | 20211115134017.1257932-1-srinivas.pandruvada@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [UPDATE] cpufreq: intel_pstate: Fix EPP restore after offline/online | expand |
On Mon, Nov 15, 2021 at 2:40 PM Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > When using performance policy, EPP value is restored to non "performance" > mode EPP after offline and online. > > For example: > cat /sys/devices/system/cpu/cpu1/cpufreq/energy_performance_preference > performance > echo 0 > /sys/devices/system/cpu/cpu1/online > echo 1 > /sys/devices/system/cpu/cpu1/online > cat /sys/devices/system/cpu/cpu1/cpufreq/energy_performance_preference > balance_performance > > The commit 4adcf2e5829f ("cpufreq: intel_pstate: Add ->offline and ->online callbacks") > optimized save restore path of the HWP request MSR, when there is no > change in the policy. Also added special processing for performance mode > EPP. If EPP has been set to "performance" by the active mode "performance" > scaling algorithm, replace that value with the cached EPP. This ends up > replacing with cached EPP during offline, which is restored during online > again. > > So add a change which will set cpu_data->epp_policy to zero, when in > performance policy and has non zero epp. In this way EPP is set to zero > again. > > Fixes: 4adcf2e5829f ("cpufreq: intel_pstate: Add ->offline and ->online callbacks") > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > Cc: stable@vger.kernel.org # v5.9+ > --- > Update: Minor optimization to skip non performance policy code path > > drivers/cpufreq/intel_pstate.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > index 815df3daae9d..6d7d73a0c66b 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -936,11 +936,17 @@ static void intel_pstate_hwp_set(unsigned int cpu) > max = cpu_data->max_perf_ratio; > min = cpu_data->min_perf_ratio; > > - if (cpu_data->policy == CPUFREQ_POLICY_PERFORMANCE) > - min = max; > - > rdmsrl_on_cpu(cpu, MSR_HWP_REQUEST, &value); > > + if (cpu_data->policy == CPUFREQ_POLICY_PERFORMANCE) { > + min = max; > + epp = 0; > + if (boot_cpu_has(X86_FEATURE_HWP_EPP)) > + epp = (value >> 24) & 0xff; > + if (epp) > + cpu_data->epp_policy = 0; > + } I understand the bug, but it should not be necessary to check this every time intel_pstate_hwp_set() runs. > + > value &= ~HWP_MIN_PERF(~0L); > value |= HWP_MIN_PERF(min); > > -- Isn't the following sufficient (modulo the gmail-induced whitespace damage)? --- drivers/cpufreq/intel_pstate.c | 6 ++++++ 1 file changed, 6 insertions(+) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -1006,6 +1006,12 @@ static void intel_pstate_hwp_offline(str */ value &= ~GENMASK_ULL(31, 24); value |= HWP_ENERGY_PERF_PREFERENCE(cpu->epp_cached); + /* + * However, make sure that EPP will be set to "performance" when + * the CPU is brought back online again and the "performance" + * scaling algorithm is still in effect. + */ + cpu->epp_policy = CPUFREQ_POLICY_UNKNOWN; } /*
On Tue, 2021-11-16 at 18:47 +0100, Rafael J. Wysocki wrote: > On Mon, Nov 15, 2021 at 2:40 PM Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > When using performance policy, EPP value is restored to non > > "performance" > > mode EPP after offline and online. > > > > For example: > > cat > > /sys/devices/system/cpu/cpu1/cpufreq/energy_performance_preference > > performance > > echo 0 > /sys/devices/system/cpu/cpu1/online > > echo 1 > /sys/devices/system/cpu/cpu1/online > > cat > > /sys/devices/system/cpu/cpu1/cpufreq/energy_performance_preference > > balance_performance > > > > The commit 4adcf2e5829f ("cpufreq: intel_pstate: Add ->offline and > > ->online callbacks") > > optimized save restore path of the HWP request MSR, when there is > > no > > change in the policy. Also added special processing for performance > > mode > > EPP. If EPP has been set to "performance" by the active mode > > "performance" > > scaling algorithm, replace that value with the cached EPP. This > > ends up > > replacing with cached EPP during offline, which is restored during > > online > > again. > > > > So add a change which will set cpu_data->epp_policy to zero, when > > in > > performance policy and has non zero epp. In this way EPP is set to > > zero > > again. > > > > Fixes: 4adcf2e5829f ("cpufreq: intel_pstate: Add ->offline and - > > >online callbacks") > > Signed-off-by: Srinivas Pandruvada < > > srinivas.pandruvada@linux.intel.com> > > Cc: stable@vger.kernel.org # v5.9+ > > --- > > Update: Minor optimization to skip non performance policy code path > > > > drivers/cpufreq/intel_pstate.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/cpufreq/intel_pstate.c > > b/drivers/cpufreq/intel_pstate.c > > index 815df3daae9d..6d7d73a0c66b 100644 > > --- a/drivers/cpufreq/intel_pstate.c > > +++ b/drivers/cpufreq/intel_pstate.c > > @@ -936,11 +936,17 @@ static void intel_pstate_hwp_set(unsigned int > > cpu) > > max = cpu_data->max_perf_ratio; > > min = cpu_data->min_perf_ratio; > > > > - if (cpu_data->policy == CPUFREQ_POLICY_PERFORMANCE) > > - min = max; > > - > > rdmsrl_on_cpu(cpu, MSR_HWP_REQUEST, &value); > > > > + if (cpu_data->policy == CPUFREQ_POLICY_PERFORMANCE) { > > + min = max; > > + epp = 0; > > + if (boot_cpu_has(X86_FEATURE_HWP_EPP)) > > + epp = (value >> 24) & 0xff; > > + if (epp) > > + cpu_data->epp_policy = 0; > > + } > > I understand the bug, but it should not be necessary to check this > every time intel_pstate_hwp_set() runs. > > > + > > value &= ~HWP_MIN_PERF(~0L); > > value |= HWP_MIN_PERF(min); > > > > -- > > Isn't the following sufficient (modulo the gmail-induced whitespace > damage)? > > --- > drivers/cpufreq/intel_pstate.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > Index: linux-pm/drivers/cpufreq/intel_pstate.c > =================================================================== > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c > +++ linux-pm/drivers/cpufreq/intel_pstate.c > @@ -1006,6 +1006,12 @@ static void intel_pstate_hwp_offline(str > */ > value &= ~GENMASK_ULL(31, 24); > value |= HWP_ENERGY_PERF_PREFERENCE(cpu->epp_cached); > + /* > + * However, make sure that EPP will be set to "performance" > when > + * the CPU is brought back online again and the > "performance" > + * scaling algorithm is still in effect. > + */ > + cpu->epp_policy = CPUFREQ_POLICY_UNKNOWN; > } > > /* This works also. Thanks, Srinivas
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 815df3daae9d..6d7d73a0c66b 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -936,11 +936,17 @@ static void intel_pstate_hwp_set(unsigned int cpu) max = cpu_data->max_perf_ratio; min = cpu_data->min_perf_ratio; - if (cpu_data->policy == CPUFREQ_POLICY_PERFORMANCE) - min = max; - rdmsrl_on_cpu(cpu, MSR_HWP_REQUEST, &value); + if (cpu_data->policy == CPUFREQ_POLICY_PERFORMANCE) { + min = max; + epp = 0; + if (boot_cpu_has(X86_FEATURE_HWP_EPP)) + epp = (value >> 24) & 0xff; + if (epp) + cpu_data->epp_policy = 0; + } + value &= ~HWP_MIN_PERF(~0L); value |= HWP_MIN_PERF(min);
When using performance policy, EPP value is restored to non "performance" mode EPP after offline and online. For example: cat /sys/devices/system/cpu/cpu1/cpufreq/energy_performance_preference performance echo 0 > /sys/devices/system/cpu/cpu1/online echo 1 > /sys/devices/system/cpu/cpu1/online cat /sys/devices/system/cpu/cpu1/cpufreq/energy_performance_preference balance_performance The commit 4adcf2e5829f ("cpufreq: intel_pstate: Add ->offline and ->online callbacks") optimized save restore path of the HWP request MSR, when there is no change in the policy. Also added special processing for performance mode EPP. If EPP has been set to "performance" by the active mode "performance" scaling algorithm, replace that value with the cached EPP. This ends up replacing with cached EPP during offline, which is restored during online again. So add a change which will set cpu_data->epp_policy to zero, when in performance policy and has non zero epp. In this way EPP is set to zero again. Fixes: 4adcf2e5829f ("cpufreq: intel_pstate: Add ->offline and ->online callbacks") Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Cc: stable@vger.kernel.org # v5.9+ --- Update: Minor optimization to skip non performance policy code path drivers/cpufreq/intel_pstate.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)