Re: [PATCH 1/2] x86: intel_pstate: Fix wrong definition of Disable Energy Efficiency Optimization bit

[Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>]

On Mon, 2019-04-15 at 11:32 +0300, Liran Alon wrote:
> > On 15 Apr 2019, at 5:00, Srinivas Pandruvada <
> > srinivas.pandruvada@linux.intel.com> wrote:
> > 
> > On Sun, 2019-04-14 at 23:48 +0300, Liran Alon wrote:
> > > Bit definition can be found in Intel SDM Section 2.16 MSRS IN THE
> > > 6TH
> > > GENERATION, 7TH GENERATION AND 8TH GENERATION
> > > INTEL® CORE™ PROCESSORS, INTEL® XEON® PROCESSOR SCALABLE
> > > FAMILY, AND FUTURE INTEL® CORE™ PROCESSORS.
> > > 
> > > Definition of all Skylake MSR_POWER_CTL bits can also be found at
> > > EDK2
> > > source at UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h union
> > > MSR_SKYLAKE_POWER_CTL_REGISTER.
> > > 
> > > Fixes: 6e978b22efa1 ("cpufreq: intel_pstate: Disable energy
> > > efficiency optimization")
> > 
> > What are you trying to address? This bit 19 has a special meaning
> > when
> > system is in HWP mode. So this is correct.
> > 
> > Bit 20 has a different meaning depending on legacy or in HWP mode.
> > 
> > Thanks,
> > Srinivas
> > 
> 
> Maybe I’m misinterpreting Intel SDM, but it seems to me that bit 19
> in MSR_POWER_CTL is always "Disable Race to Halt Optimization (R/W)”
> while bit 20 is the "Disable Energy Efficiency Optimization (R/W)”.
> 
> I didn’t find a place in Intel SDM where it is discussed that bit 19
> have a special meaning when system is in HWP mode.
> Can you point me to relevant place in Intel SDM?
> 

SDM doesn't describe the algorithms. This is a feature of Intel Speed
Shift Technology aka HWP. Both bits target disabling some energy
efficiency features of the processor. I wish there are some better
names of these bits. Ideas is to pick the best for a platform based on
the performance needs. Here based on the experiments, setting bit 19
gave the required performance on Kaby Lake desktops.

So unless you found some performance/power issue with setting of bit 19
vs bit 20, on Kaby Lake based platforms, we shouldn't change (may be
rename as per SDM definition).

Thanks,
Srinivas

> Thanks,
> -Liran
>