[U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq

[Bin Meng]

Hi Christian,

On Tue, Apr 10, 2018 at 4:01 PM, Christian Gmeiner
<christian.gmeiner at gmail.com> wrote:
> Fixes performance issue when running vx5/vx7 images.

Could you elaborate more on what performance issue you are seeing?

nits: better to spell out "VxWorks"

>
> Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
> ---
>  arch/x86/cpu/queensbay/Makefile |  2 +-
>  arch/x86/cpu/queensbay/cpu.c    | 61 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 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..e98e4ac8ef
> --- /dev/null
> +++ b/arch/x86/cpu/queensbay/cpu.c
> @@ -0,0 +1,61 @@
> +/*
> + * Copyright (C) 2016, Bachmann electronic GmbH

nits: 2018?

> + *
> + * 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;
> +
> +       msr = msr_read(MSR_IA32_MISC_ENABLES);
> +       msr.lo |= (1 << 16);

Please use macro instead of magic numbers

> +       msr_write(MSR_IA32_MISC_ENABLES, msr);
> +
> +       msr = msr_read(MSR_IA32_PERF_CTL);
> +       msr.lo = 0x101f;

ditto

> +       msr_write(MSR_IA32_PERF_CTL, msr);
> +}
> +
> +static int cpu_x86_queensbay_probe(struct udevice *dev)

Queensbay is the platform codename, not the core codename. Queensbay =
TunnelCreek (the processor) + Topcliff (the bridge chipset).

> +{
> +       if (!ll_boot_init())
> +               return 0;
> +       debug("Init Queensbay core\n");

Should be "TunnelCreek"

> +
> +       /* Set core to max frequency ratio */
> +       set_max_freq();
> +
> +       return 0;
> +}
> +
> +static int queensbay_get_count(struct udevice *dev)
> +{
> +       return 2;
> +}

Please use cpu_x86_get_count() instead of hard code.

> +
> +static const struct cpu_ops cpu_x86_queensbay_ops = {
> +       .get_desc       = cpu_x86_get_desc,
> +       .get_count      = queensbay_get_count,
> +};
> +
> +static const struct udevice_id cpu_x86_queensbay_ids[] = {
> +       { .compatible = "intel,queensbay-cpu" },

This should be "intel,tunnelcreek-cpu"

> +       { }
> +};
> +
> +U_BOOT_DRIVER(cpu_x86_queensbay_drv) = {
> +       .name           = "cpu_x86_queensbay",
> +       .id             = UCLASS_CPU,
> +       .of_match       = cpu_x86_queensbay_ids,
> +       .bind           = cpu_x86_bind,
> +       .probe          = cpu_x86_queensbay_probe,
> +       .ops            = &cpu_x86_queensbay_ops,
> +};
> --

Regards,
Bin