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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
     }

     /*



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux