Thread (3 messages) 3 messages, 2 authors, 2021-11-17

Re: [UPDATE][PATCH] cpufreq: intel_pstate: Fix EPP restore after offline/online

From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Date: 2021-11-17 02:09:32
Also in: lkml, stable

On Tue, 2021-11-16 at 18:47 +0100, Rafael J. Wysocki wrote:
quoted hunk ↗ jump to hunk
On Mon, Nov 15, 2021 at 2:40 PM Srinivas Pandruvada
[off-list ref] wrote:
quoted
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 -
quoted
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.
quoted
+
        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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help