RE: cpufreq: intel_pstate: EPB with performance governor

["Doug Smythies" <dsmythies@telus.net>]

Hi Rafael,

Thank you for your reply.

On 2020.07.15 09:47 Rafael J. Wysocki wrote:
> On Fri, Jul 10, 2020 at 3:34 PM Doug Smythies <dsmythies@telus.net> wrote:
> >
> > Hi Srinivas and/or Rafael,
> >
> > Can you please confirm or deny that an older
> > commit:
> >
> > commit 8442885fca09b2d26375b9fe507759879a6f661e
> > cpufreq: intel_pstate: Set EPP/EPB to 0 in performance mode
> >
> > has been superseded by:
> >
> > arch/x86/kernel/cpu/intel_epb.c
> 
> No, it hasn't.
> 
> However, intel_pstate only touches the EPB if EPP is not supported,
> which should become a non-issue after this patch posted by me
> yesterday: https://patchwork.kernel.org/patch/11663315/
> 
> If EPP is supported, intel_pstate will use it and it will never look
> at the EPB even.
> 
> > and that now there is no way to have some default EPB (say 6) for
> > governors other than performance, while still getting an EPB of 0
> > for the performance governor.
> 
> If EPP is supported, what happens to the EPB is completely orthogonal
> to cpufreq etc.

Yes, I know.
I am talking about when HWP is disabled.

And I do not understand your reference to that patch from yesterday,
as it seems unrelated to me.

> 
> So it is possible to have the EPB different from 0, but it should be
> the same for all governors unless changed via energy_perf_bias.

And I am saying that contradicts the earlier referenced patch.
We want EPB set to 0 for performance mode, otherwise I challenge the
name "performance" governor.

Yes, EPB can be whatever for the other governors.

> 
> If EPP is not supported, though, then without the patch mentioned
> above, intel_pstate may fiddle with the EPB.
> 
> > Additional notes:
> > Both my test computers have EPB as 0 upon startup,
> 
> That is before intel_epb_init() runs, because it will change the EPB
> to "normal" (6).

Yes, I know.

> 
> > But I also tried this:
> >
> > diff --git a/arch/x86/kernel/cpu/intel_epb.c b/arch/x86/kernel/cpu/intel_epb.c
> > index f4dd73396f28..b536e381cd56 100644
> > --- a/arch/x86/kernel/cpu/intel_epb.c
> > +++ b/arch/x86/kernel/cpu/intel_epb.c
> > @@ -74,7 +74,8 @@ static int intel_epb_save(void)
> >
> >  static void intel_epb_restore(void)
> >  {
> > -       u64 val = this_cpu_read(saved_epb);
> > +//     u64 val = this_cpu_read(saved_epb);
> > +       u64 val = 6;
> >         u64 epb;
> >
> >         rdmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
> >
> > which did get rid of this message:
> > kernel: [    0.102158] ENERGY_PERF_BIAS: Set to 'normal', was 'performance'
> 
> Which is exactly what happens when this message is printed.
> 
> Instead of commenting out the line of code above, which is not a
> correct thing to do in general,

Yes, of course. That was just a test, and the only way I could think of
to prove that the system started with it as 0.

> you can simply set the EPB to 0 via
> energy_perf_bias for all CPUs and it should stick.

And I am saying I should not have to do that, or even know about it,
when I want to use the performance governor.
But yes, I expect the driver to remember the default, or otherwise set,
value of EPB for all the other governors.

... Doug