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@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;
> }
>
> /*
This works also.
Thanks,
Srinivas
