[U-Boot] [PATCH v2 18/20] x86: Add a CPU driver for baytrail

Simon Glass sjg at chromium.org
Wed Apr 29 16:00:11 CEST 2015


Hi Bin,

On 29 April 2015 at 07:57, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Wed, Apr 29, 2015 at 10:25 AM, Simon Glass <sjg at chromium.org> wrote:
>> This driver supports multi-core init and sets up the CPU frequencies
>> correctly.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>> Changes in v2: None
>>
>>  arch/x86/cpu/baytrail/Makefile           |   1 +
>>  arch/x86/cpu/baytrail/cpu.c              | 206 +++++++++++++++++++++++++++++++
>>  arch/x86/include/asm/arch-baytrail/msr.h |  30 +++++
>>  3 files changed, 237 insertions(+)
>>  create mode 100644 arch/x86/cpu/baytrail/cpu.c
>>  create mode 100644 arch/x86/include/asm/arch-baytrail/msr.h
>>
>> diff --git a/arch/x86/cpu/baytrail/Makefile b/arch/x86/cpu/baytrail/Makefile
>> index 8914e8b..c78b644 100644
>> --- a/arch/x86/cpu/baytrail/Makefile
>> +++ b/arch/x86/cpu/baytrail/Makefile
>> @@ -4,6 +4,7 @@
>>  # SPDX-License-Identifier:     GPL-2.0+
>>  #
>>
>> +obj-y += cpu.o
>>  obj-y += early_uart.o
>>  obj-y += fsp_configs.o
>>  obj-y += pci.o
>> diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c
>> new file mode 100644
>> index 0000000..5a2a8ee
>> --- /dev/null
>> +++ b/arch/x86/cpu/baytrail/cpu.c
>> @@ -0,0 +1,206 @@
>> +/*
>> + * Copyright (C) 2015 Google, Inc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + *
>> + * Based on code from coreboot
>> + */
>> +
>> +#include <common.h>
>> +#include <cpu.h>
>> +#include <dm.h>
>> +#include <asm/cpu.h>
>> +#include <asm/lapic.h>
>> +#include <asm/mp.h>
>> +#include <asm/msr.h>
>> +#include <asm/turbo.h>
>> +#include <asm/arch/msr.h>
>> +
>> +#ifdef CONFIG_SMP
>> +static int enable_smis(struct udevice *cpu, void *unused)
>> +{
>> +       return 0;
>> +}
>
> What is this function for? Is this a must-have?

It's partly a placeholder, and also is intended to ensure that the APs
are all started before the main CPU continues execution.

>
>> +
>> +static struct mp_flight_record mp_steps[] = {
>> +       MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),
>> +       /* Wait for APs to finish initialization before proceeding. */
>> +       MP_FR_BLOCK_APS(NULL, NULL, enable_smis, NULL),
>> +};
>> +
>> +static int detect_num_cpus(void)
>> +{
>> +       int ecx = 0;
>> +
>> +       /*
>> +        * Use the algorithm described in Intel 64 and IA-32 Architectures
>> +        * Software Developer's Manual Volume 3 (3A, 3B & 3C): System
>> +        * Programming Guide, Jan-2015. Section 8.9.2: Hierarchical Mapping
>> +        * of CPUID Extended Topology Leaf.
>> +        */
>> +       while (1) {
>> +               struct cpuid_result leaf_b;
>> +
>> +               leaf_b = cpuid_ext(0xb, ecx);
>> +
>> +               /*
>> +                * Bay Trail doesn't have hyperthreading so just determine the
>> +                * number of cores by from level type (ecx[15:8] == * 2)
>> +                */
>> +               if ((leaf_b.ecx & 0xff00) == 0x0200)
>> +                       return leaf_b.ebx & 0xffff;
>> +               ecx++;
>> +       }
>> +}
>
> Since we already describe all cpus in the device tree, is this dynamic
> probe really needed?

With MinnowMax I'd like to support the single-core version of the
board also. It could have its own device tree, but I don't want to
break in this case. However, this case is not tested.

>
>> +
>> +static int baytrail_init_cpus(void)
>> +{
>> +       struct mp_params mp_params;
>> +
>> +       lapic_setup();
>> +
>> +       mp_params.num_cpus = detect_num_cpus();
>> +       mp_params.parallel_microcode_load = 0,
>> +       mp_params.flight_plan = &mp_steps[0];
>> +       mp_params.num_records = ARRAY_SIZE(mp_steps);
>> +       mp_params.microcode_pointer = 0;
>> +
>> +       if (mp_init(&mp_params)) {
>> +               printf("Warning: MP init failure\n");
>> +               return -EIO;
>> +       }
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>> +int x86_init_cpus(void)
>> +{
>> +#ifdef CONFIG_SMP
>> +       debug("Init additional CPUs\n");
>> +       baytrail_init_cpus();
>> +#endif
>> +
>> +       return 0;
>> +}
>> +
>> +void set_max_freq(void)
>
> Should this be static?

Yes

>
>> +{
>> +       msr_t perf_ctl;
>> +       msr_t msr;
>> +
>> +       /* Enable speed step */
>> +       msr = msr_read(MSR_IA32_MISC_ENABLES);
>> +       msr.lo |= (1 << 16);
>> +       msr_write(MSR_IA32_MISC_ENABLES, msr);
>> +
>> +       /*
>> +        * Set guaranteed ratio [21:16] from IACORE_RATIOS to bits [15:8] of
>> +        * the PERF_CTL
>> +        */
>> +       msr = msr_read(MSR_IACORE_RATIOS);
>> +       perf_ctl.lo = (msr.lo & 0x3f0000) >> 8;
>> +
>> +       /*
>> +        * Set guaranteed vid [21:16] from IACORE_VIDS to bits [7:0] of
>> +        * the PERF_CTL
>> +        */
>> +       msr = msr_read(MSR_IACORE_VIDS);
>> +       perf_ctl.lo |= (msr.lo & 0x7f0000) >> 16;
>> +       perf_ctl.hi = 0;
>> +
>> +       msr_write(MSR_IA32_PERF_CTL, perf_ctl);
>> +}
>> +
>> +static int cpu_x86_baytrail_probe(struct udevice *dev)
>> +{
>> +       debug("Init baytrail core\n");
>
> BayTrail?

OK

>
>> +
>> +       /*
>> +        * On bay trail the turbo disable bit is actually scoped at the
>
> BayTrail?
>
>> +        * building-block level, not package. For non-BSP cores that are
>> +        * within a building block, enable turbo. The cores within the BSP's
>> +        * building block will just see it already enabled and move on.
>> +        */
>> +       if (lapicid())
>> +               turbo_enable();
>> +
>> +       /* Dynamic L2 shrink enable and threshold */
>> +       msr_clrsetbits_64(MSR_PMG_CST_CONFIG_CONTROL, 0x3f000f, 0xe0008),
>> +
>> +       /* Disable C1E */
>> +       msr_clrsetbits_64(MSR_POWER_CTL, 2, 0);
>> +       msr_setbits_64(MSR_POWER_MISC, 0x44);
>> +
>> +       /* Set this core to max frequency ratio */
>> +       set_max_freq();
>> +
>> +       return 0;
>> +}
>> +
>> +static unsigned bus_freq(void)
>> +{
>> +       msr_t clk_info = msr_read(MSR_BSEL_CR_OVERCLOCK_CONTROL);
>> +       switch (clk_info.lo & 0x3) {
>> +       case 0:
>> +               return 83333333;
>> +       case 1:
>> +               return 100000000;
>> +       case 2:
>> +               return 133333333;
>> +       case 3:
>> +               return 116666666;
>> +       default:
>> +               return 0;
>> +       }
>> +}
>> +
>> +static unsigned long tsc_freq(void)
>> +{
>> +       msr_t platform_info;
>> +       ulong bclk = bus_freq();
>> +
>> +       if (!bclk)
>> +               return 0;
>> +
>> +       platform_info = msr_read(MSR_PLATFORM_INFO);
>> +
>> +       return bclk * ((platform_info.lo >> 8) & 0xff);
>> +}
>> +
>> +static int baytrail_get_info(struct udevice *dev, struct cpu_info *info)
>> +{
>> +       info->cpu_freq = tsc_freq();
>> +       info->features = 1 << CPU_FEAT_L1_CACHE | 1 << CPU_FEAT_MMU;
>> +
>> +       return 0;
>> +}
>> +
>> +static int cpu_x86_baytrail_bind(struct udevice *dev)
>> +{
>> +       struct cpu_platdata *plat = dev_get_parent_platdata(dev);
>> +
>> +       plat->cpu_id = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>> +                                     "intel,apic-id", -1);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct cpu_ops cpu_x86_baytrail_ops = {
>> +       .get_desc       = x86_cpu_get_desc,
>> +       .get_info       = baytrail_get_info,
>> +};
>> +
>> +static const struct udevice_id cpu_x86_baytrail_ids[] = {
>> +       { .compatible = "intel,baytrail-cpu" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(cpu_x86_baytrail_drv) = {
>> +       .name           = "cpu_x86_baytrail",
>> +       .id             = UCLASS_CPU,
>> +       .of_match       = cpu_x86_baytrail_ids,
>> +       .bind           = cpu_x86_baytrail_bind,
>> +       .probe          = cpu_x86_baytrail_probe,
>> +       .ops            = &cpu_x86_baytrail_ops,
>> +};
>> diff --git a/arch/x86/include/asm/arch-baytrail/msr.h b/arch/x86/include/asm/arch-baytrail/msr.h
>> new file mode 100644
>> index 0000000..1975aec
>> --- /dev/null
>> +++ b/arch/x86/include/asm/arch-baytrail/msr.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Copyright (C) 2015 Google, Inc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef __asm_arch_msr_h
>> +#define __asm_arch_msr_h
>
> Should be capital letters, or (see below)
>
>> +
>> +#define MSR_BSEL_CR_OVERCLOCK_CONTROL  0xcd
>> +#define MSR_PMG_CST_CONFIG_CONTROL     0xe2
>> +#define SINGLE_PCTL                    (1 << 11)
>> +#define MSR_POWER_MISC                 0x120
>> +#define ENABLE_ULFM_AUTOCM_MASK                (1 << 2)
>> +#define ENABLE_INDP_AUTOCM_MASK                (1 << 3)
>> +#define MSR_IA32_MISC_ENABLES          0x1a0
>> +#define MSR_POWER_CTL                  0x1fc
>> +#define MSR_PKG_POWER_SKU_UNIT         0x606
>> +#define MSR_IACORE_RATIOS              0x66a
>> +#define MSR_IACORE_TURBO_RATIOS                0x66c
>> +#define MSR_IACORE_VIDS                        0x66b
>> +#define MSR_IACORE_TURBO_VIDS          0x66d
>> +#define MSR_PKG_TURBO_CFG1             0x670
>> +#define MSR_CPU_TURBO_WKLD_CFG1                0x671
>> +#define MSR_CPU_TURBO_WKLD_CFG2                0x672
>> +#define MSR_CPU_THERM_CFG1             0x673
>> +#define MSR_CPU_THERM_CFG2             0x674
>> +#define MSR_CPU_THERM_SENS_CFG         0x675
>> +
>
> Should these be all put into arch/x86/include/asm/msr-index.h, a
> single place for all x86 processors' MSR?

I was worried that they might be specific to this CPU. But if they are
common then yes they should go in the common file.

Regards,
Simon


More information about the U-Boot mailing list