On Thu, Dec 17, 2020 at 6:29 PM Rafael J. Wysocki <raf...@kernel.org> wrote:
>
> On Thu, Dec 17, 2020 at 6:09 PM Srinivas Pandruvada
> <srinivas.pandruv...@linux.intel.com> wrote:
> >
> > On Thu, 2020-12-17 at 16:24 +0100, Rafael J. Wysocki wrote:
> > > On Thu, Dec 17, 2020 at 4:21 PM Srinivas Pandruvada
> > > <srinivas.pandruv...@linux.intel.com> wrote:
[cut]
> > > > > Well, would something like the patch below work?
> > > > >
> > > > > ---
> > > > > drivers/cpufreq/intel_pstate.c | 16 +++++++++++++---
> > > > > 1 file changed, 13 insertions(+), 3 deletions(-)
> > > > >
> > > > > Index: linux-pm/drivers/cpufreq/intel_pstate.c
> > > > > =================================================================
> > > > > ==
> > > > > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> > > > > +++ linux-pm/drivers/cpufreq/intel_pstate.c
> > > > > @@ -2207,9 +2207,9 @@ static void intel_pstate_update_perf_lim
> > > > > unsigned int
> > > > > policy_min,
> > > > > unsigned int
> > > > > policy_max)
> > > > > {
> > > > > - int max_freq = intel_pstate_get_max_freq(cpu);
> > > > > int32_t max_policy_perf, min_policy_perf;
> > > > > int max_state, turbo_max;
> > > > > + int max_freq;
> > > > >
> > > > > /*
> > > > > * HWP needs some special consideration, because on BDX
> > > > > the
> > > > > @@ -2223,6 +2223,7 @@ static void intel_pstate_update_perf_lim
> > > > > cpu->pstate.max_pstate : cpu-
> > > > > > pstate.turbo_pstate;
> > > > > turbo_max = cpu->pstate.turbo_pstate;
> > > > > }
> > > > > + max_freq = max_state * cpu->pstate.scaling;
> > > > >
> > > > > max_policy_perf = max_state * policy_max / max_freq;
> > > > > if (policy_max == policy_min) {
> > > > > @@ -2325,9 +2326,18 @@ static void intel_pstate_adjust_policy_m
> > > > > static void intel_pstate_verify_cpu_policy(struct cpudata *cpu,
> > > > > struct
> > > > > cpufreq_policy_data
> > > > > *policy)
> > > > > {
> > > > > + int max_freq;
> > > > > +
> > > > > update_turbo_state();
> > > > > - cpufreq_verify_within_limits(policy, policy-
> > > > > > cpuinfo.min_freq,
> > > > > -
> > > > > intel_pstate_get_max_freq(cpu));
> > > > > + if (hwp_active) {
> > > > > + int max_state, turbo_max;
> > > > > +
> > > > > + intel_pstate_get_hwp_max(cpu->cpu, &turbo_max,
> > > > > &max_state);
> > > > > + max_freq = max_state * cpu->pstate.scaling;
> > > > > + } else {
> > > > > + max_freq = intel_pstate_get_max_freq(cpu);
> > > > > + }
> > > > > + cpufreq_verify_within_limits(policy, policy-
> > > > > > cpuinfo.min_freq, max_freq);
> > > > >
> > > > > intel_pstate_adjust_policy_max(cpu, policy);
> > > > > }
> > > > >
> > > > Should work.
> > > > I will test this patch and let you know once I get the system.
> > >
> > > Please do, thank you!
> >
> > This works. Please check if you can submit a change for this.
>
> I can do that, but I'm going to borrow some changelog pieces from the
> $subject patch.
>
> Will submit shortly.
Well, this only fixes the setting of the policy max limit AFAICS, but
pstate.max_pstate is used in computations in some places, so it looks
like it needs to be updated every time HWP_CAP is read, or do I
confuse things?
And if pstate.max_pstate needs to be updated then, shouldn't
pstate.turbo_pstate be updated then too (because it may change too as
a result of ISS updates)?
