[U-Boot] [PATCH 066/126] x86: spl: Support init of a PUNIT

[Simon Glass]

Hi Bin,

On Wed, 9 Oct 2019 at 08:02, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > The x86 power unit handles power management. Support initing this device
> > which is modelled as a new type of system controller since there are no
> > operations needed.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >  arch/x86/include/asm/cpu.h |  1 +
> >  arch/x86/lib/spl.c         | 40 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 41 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> > index feee0f915f6..21a05dab7de 100644
> > --- a/arch/x86/include/asm/cpu.h
> > +++ b/arch/x86/include/asm/cpu.h
> > @@ -55,6 +55,7 @@ enum {
> >         X86_SYSCON_PINCONF,     /* Intel x86 pin configuration */
> >         X86_SYSCON_PMU,         /* Power Management Unit */
> >         X86_SYSCON_SCU,         /* System Controller Unit */
> > +       X86_SYSCON_PUNIT,       /* Power unit */
> >  };
> >
> >  struct cpuid_result {
> > diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
> > index 2baac913837..db1ce67a590 100644
> > --- a/arch/x86/lib/spl.c
> > +++ b/arch/x86/lib/spl.c
> > @@ -5,11 +5,15 @@
> >
> >  #include <common.h>
> >  #include <debug_uart.h>
> > +#include <dm.h>
> >  #include <malloc.h>
> >  #include <spl.h>
> > +#include <syscon.h>
> >  #include <asm/cpu.h>
> > +#include <asm/cpu_common.h>
> >  #include <asm/mrccache.h>
> >  #include <asm/mtrr.h>
> > +#include <asm/pci.h>
> >  #include <asm/processor.h>
> >  #include <asm/spl.h>
> >  #include <asm-generic/sections.h>
> > @@ -21,6 +25,32 @@ __weak int arch_cpu_init_dm(void)
> >         return 0;
> >  }
> >
> > +#ifdef CONFIG_TPL
> > +
> > +static int set_max_freq(void)
> > +{
> > +       if (cpu_get_burst_mode_state() == BURST_MODE_UNAVAILABLE) {
> > +               /*
> > +                * Burst Mode has been factory-configured as disabled and is not
> > +                * available in this physical processor package
> > +                */
> > +               debug("Burst Mode is factory-disabled\n");
> > +               return -ENOENT;
> > +       }
> > +
> > +       /* Enable burst mode */
> > +       cpu_set_burst_mode(true);
> > +
> > +       /* Enable speed step */
> > +       cpu_set_eist(true);
> > +
> > +       /* Set P-State ratio */
> > +       cpu_set_p_state_to_turbo_ratio();
> > +
> > +       return 0;
> > +}
> > +#endif
> > +
> >  static int x86_spl_init(void)
> >  {
> >  #ifndef CONFIG_TPL
> > @@ -31,6 +61,8 @@ static int x86_spl_init(void)
> >          * place it immediately below CONFIG_SYS_TEXT_BASE.
> >          */
> >         char *ptr = (char *)0x110000;
> > +#else
> > +       struct udevice *punit;
> >  #endif
> >         int ret;
> >
> > @@ -101,6 +133,14 @@ static int x86_spl_init(void)
> >                 return ret;
> >         }
> >         mtrr_commit(true);
> > +#else
> > +       ret = syscon_get_by_driver_data(X86_SYSCON_PUNIT, &punit);
> > +       if (ret)
> > +               debug("Could not find PUNIT (err-%d)\n", ret);
>
> err=%d
>
> But I wonder what's the purpose of init punit at this point?

See punit_init() - among other things it tells the systemagent that
U-Boot is running and sets the thermal target for the device.

>
> > +
> > +       ret = set_max_freq();
>
> Can we delay this to U-Boot proper? I don't see a need to bring the
> core up to speed at this early stage.

Yes, but it does add time. SPL is 8ms slower without this, and getting
to board_init_r() in U-Boot is 50ms slower.


>
> > +       if (ret)
> > +               debug("Failed to set CPU frequency (err=%d)\n", ret);
> >  #endif
> >
> >         return 0;
> > --
>
> Regards,
> Bin