| Message ID | 20190925145750.200592-67-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: Add initial support for apollolake | expand |
Hi Simon, On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <sjg@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@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? > + > + 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. > + if (ret) > + debug("Failed to set CPU frequency (err=%d)\n", ret); > #endif > > return 0; > -- Regards, Bin
Hi Bin, On Wed, 9 Oct 2019 at 08:02, Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Simon, > > On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <sjg@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@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
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); + + ret = set_max_freq(); + if (ret) + debug("Failed to set CPU frequency (err=%d)\n", ret); #endif return 0;
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@chromium.org> --- arch/x86/include/asm/cpu.h | 1 + arch/x86/lib/spl.c | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)