[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Bin Meng]

Christian, I got some time to look at this old patch, and I see my
last question remained unanswered.

+Andy,

Hi Andy,

On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Christian,
>
> On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> <christian.gmeiner at gmail.com> wrote:
> > Fixes performance related issue when running vxWorks 5/7 images.
>
> nits: vxWorks -> VxWorks
>
> > The overall memory performance (L1, L2 cache and ram) was measured
> > with Bandwidth [0].
> >
> > Without this patch we get following numbers:
> >  - sequential 128-bit reads:  ~5.2 GB/s
> >  - sequential 128-bit copy:   ~2.1 GB/s
> >  - random 32-bit writes:      ~1.2 GB/s
> >
> > With this patch patch we get the following numbers:
> >  - sequential 128-bit reads: ~18.0 GB/s
> >  - sequential 128-bit copy:   ~9.5 GB/s
> >  - random 32-bit writes:      ~5.0 GB/s
> >
> > [0] https://zsmith.co/bandwidth.html
> >
> > v1 -> v2:
> >  - incorporate feedback from Bin Meng
>
> This should not show in the commit message.
>
> >
> > Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
> > ---
> >  arch/x86/cpu/queensbay/Makefile |  2 +-
> >  arch/x86/cpu/queensbay/cpu.c    | 58 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 59 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/cpu/queensbay/cpu.c
> >
> > diff --git a/arch/x86/cpu/queensbay/Makefile b/arch/x86/cpu/queensbay/Makefile
> > index c0681995bd..3dd23465d4 100644
> > --- a/arch/x86/cpu/queensbay/Makefile
> > +++ b/arch/x86/cpu/queensbay/Makefile
> > @@ -5,4 +5,4 @@
> >  #
> >
> >  obj-y += fsp_configs.o irq.o
> > -obj-y += tnc.o
> > +obj-y += tnc.o cpu.o
> > diff --git a/arch/x86/cpu/queensbay/cpu.c b/arch/x86/cpu/queensbay/cpu.c
> > new file mode 100644
> > index 0000000000..805a94cc27
> > --- /dev/null
> > +++ b/arch/x86/cpu/queensbay/cpu.c
> > @@ -0,0 +1,58 @@
> > +/*
> > + * Copyright (C) 2018, Bachmann electronic GmbH
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <cpu.h>
> > +#include <dm.h>
> > +#include <asm/cpu.h>
> > +#include <asm/cpu_x86.h>
> > +#include <asm/msr.h>
> > +
> > +static void set_max_freq(void)
> > +{
> > +       msr_t msr;
> > +
> > +       /* Enable enhanced speed step */
> > +       msr = msr_read(MSR_IA32_MISC_ENABLES);
> > +       msr.lo |= (1 << 16);
> > +       msr_write(MSR_IA32_MISC_ENABLES, msr);
> > +
> > +       /* Set new performance state */
> > +       msr = msr_read(MSR_IA32_PERF_CTL);
> > +       msr.lo = 0x101f;
> > +       msr_write(MSR_IA32_PERF_CTL, msr);
> > +}
>
> I tried to find any documentation that describes the performance state
> values of the TunnelCreek processor, but in vain. However when I read
> the doc, I do have a question here:
>
> The enhanced speedstep technology is set to disabled by the processor
> after power-on, that means we don't need set the performance state
> (P-state) via the MSR_IA32_PERF_CTL and the processor itself should
> work under its maximum base frequency. So I believe this whole
> set_max_freq() is not needed. Can you clarify this?
>

I hope you can give some clarification about Enhanced Intel Speedstep
(EIST) technology. I read Intel 64 and IA-32 Architectures SDM volume
3: chapter 14 "POWER AND THERMAL MANAGEMENT" and it does not say much
about this EIST. Especially I want to know whether the process is
running at the highest frequency if EIST is disabled which is the case
after power-on (MSR_IA32_MISC_ENABLES bit[16] is 0).

A related discussion after Googling is:
https://forums.anandtech.com/threads/actual-difference-between-speedstep-and-turbo-boost.2226820/
It says: "The base frequency is the highest P-state called P0, and
every step below that using EIST goes P1, P2, etc. Turbo Mode only
activates when the CPU is at P0 state, then every speed above the base
clock is decided by the CPU."

So to me this seems to match my understanding about EIST. If this is
true, then I can't explain why Christian's patch is needed since the
EIST is disabled on TunnelCreek by default and the processor should
already run at the highest performance.

Regards,
Bin