On Mon, Nov 15, 2021 at 2:40 PM Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # 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;
}
/*
