* [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq
@ 2018-04-10 8:01 Christian Gmeiner
2018-04-10 8:17 ` Bin Meng
0 siblings, 1 reply; 8+ messages in thread
From: Christian Gmeiner @ 2018-04-10 8:01 UTC (permalink / raw)
To: u-boot
Fixes performance issue when running vx5/vx7 images.
Signed-off-by: Christian Gmeiner <christian.gmeiner@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
+ *
+ * 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);
+ msr_write(MSR_IA32_MISC_ENABLES, msr);
+
+ msr = msr_read(MSR_IA32_PERF_CTL);
+ msr.lo = 0x101f;
+ msr_write(MSR_IA32_PERF_CTL, msr);
+}
+
+static int cpu_x86_queensbay_probe(struct udevice *dev)
+{
+ if (!ll_boot_init())
+ return 0;
+ debug("Init Queensbay core\n");
+
+ /* Set core to max frequency ratio */
+ set_max_freq();
+
+ return 0;
+}
+
+static int queensbay_get_count(struct udevice *dev)
+{
+ return 2;
+}
+
+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" },
+ { }
+};
+
+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,
+};
--
2.14.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq
2018-04-10 8:01 [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq Christian Gmeiner
@ 2018-04-10 8:17 ` Bin Meng
2018-04-10 8:47 ` Christian Gmeiner
0 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2018-04-10 8:17 UTC (permalink / raw)
To: u-boot
Hi Christian,
On Tue, Apr 10, 2018 at 4:01 PM, Christian Gmeiner
<christian.gmeiner@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@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq
2018-04-10 8:17 ` Bin Meng
@ 2018-04-10 8:47 ` Christian Gmeiner
2018-04-10 9:07 ` Bin Meng
0 siblings, 1 reply; 8+ messages in thread
From: Christian Gmeiner @ 2018-04-10 8:47 UTC (permalink / raw)
To: u-boot
Hi Bin
Thanks for you quick review!
>
> On Tue, Apr 10, 2018 at 4:01 PM, Christian Gmeiner
> <christian.gmeiner@gmail.com> wrote:
>> Fixes performance issue when running vx5/vx7 images.
>
> Could you elaborate more on what performance issue you are seeing?
>
The cache performance was bad without this change. We found this problem
when switching from bldk to u-boot.
old: https://imagebin.ca/v/3xssw7stbY6A
new: https://imagebin.ca/v/3xstHMCC9fmJ
> nits: better to spell out "VxWorks"
ok
>
>>
>> Signed-off-by: Christian Gmeiner <christian.gmeiner@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?
>
This change was made some months ago :) But yeah.. can change it.
>> + *
>> + * 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
ok
>
>> + msr_write(MSR_IA32_MISC_ENABLES, msr);
>> +
>> + msr = msr_read(MSR_IA32_PERF_CTL);
>> + msr.lo = 0x101f;
>
> ditto
ok
>
>> + 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"
>
ok
>> +
>> + /* 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.
>
That one uses the dts as basis for this information and I need to
add two cpu nodes. I also thought about using cpuid for this. Not
sure what is better.
>> +
>> +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"
ok
>
>> + { }
>> +};
>> +
>> +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,
>> +};
>> --
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq
2018-04-10 8:47 ` Christian Gmeiner
@ 2018-04-10 9:07 ` Bin Meng
2018-04-10 9:09 ` Christian Gmeiner
0 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2018-04-10 9:07 UTC (permalink / raw)
To: u-boot
Hi Christian,
On Tue, Apr 10, 2018 at 4:47 PM, Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
> Hi Bin
>
> Thanks for you quick review!
>
>>
>> On Tue, Apr 10, 2018 at 4:01 PM, Christian Gmeiner
>> <christian.gmeiner@gmail.com> wrote:
>>> Fixes performance issue when running vx5/vx7 images.
>>
>> Could you elaborate more on what performance issue you are seeing?
>>
>
> The cache performance was bad without this change. We found this problem
> when switching from bldk to u-boot.
>
> old: https://imagebin.ca/v/3xssw7stbY6A
> new: https://imagebin.ca/v/3xstHMCC9fmJ
Thank you for the benchmark data. Then please consider describing your
benchmark test suite and on what configuration the data is worse
without this patch (eg: sequential 128-bit read) in the commit
message. This will help other people understand the patch and the
commit in the future.
>
>> nits: better to spell out "VxWorks"
>
> ok
>
>>
>>>
>>> Signed-off-by: Christian Gmeiner <christian.gmeiner@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?
>>
>
> This change was made some months ago :) But yeah.. can change it.
>
>>> + *
>>> + * 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
>
> ok
>
>>
>>> + msr_write(MSR_IA32_MISC_ENABLES, msr);
>>> +
>>> + msr = msr_read(MSR_IA32_PERF_CTL);
>>> + msr.lo = 0x101f;
>>
>> ditto
>
> ok
>
>>
>>> + 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"
>>
>
> ok
>
>>> +
>>> + /* 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.
>>
>
> That one uses the dts as basis for this information and I need to
> add two cpu nodes. I also thought about using cpuid for this. Not
> sure what is better.
>
I don't understand. Isn't your board U-Boot ported from Intel Crown
Bay? There is arch/x86/dts/crownbay.dts already. You don't need do any
mods in the dts.
>>> +
>>> +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"
>
> ok
>
>>
>>> + { }
>>> +};
>>> +
>>> +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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq
2018-04-10 9:07 ` Bin Meng
@ 2018-04-10 9:09 ` Christian Gmeiner
2018-04-10 9:11 ` Bin Meng
0 siblings, 1 reply; 8+ messages in thread
From: Christian Gmeiner @ 2018-04-10 9:09 UTC (permalink / raw)
To: u-boot
Hi Bin,
2018-04-10 11:07 GMT+02:00 Bin Meng <bmeng.cn@gmail.com>:
> Hi Christian,
>
> On Tue, Apr 10, 2018 at 4:47 PM, Christian Gmeiner
> <christian.gmeiner@gmail.com> wrote:
>> Hi Bin
>>
>> Thanks for you quick review!
>>
>>>
>>> On Tue, Apr 10, 2018 at 4:01 PM, Christian Gmeiner
>>> <christian.gmeiner@gmail.com> wrote:
>>>> Fixes performance issue when running vx5/vx7 images.
>>>
>>> Could you elaborate more on what performance issue you are seeing?
>>>
>>
>> The cache performance was bad without this change. We found this problem
>> when switching from bldk to u-boot.
>>
>> old: https://imagebin.ca/v/3xssw7stbY6A
>> new: https://imagebin.ca/v/3xstHMCC9fmJ
>
> Thank you for the benchmark data. Then please consider describing your
> benchmark test suite and on what configuration the data is worse
> without this patch (eg: sequential 128-bit read) in the commit
> message. This will help other people understand the patch and the
> commit in the future.
>
Will do.
>>
>>> nits: better to spell out "VxWorks"
>>
>> ok
>>
>>>
>>>>
>>>> Signed-off-by: Christian Gmeiner <christian.gmeiner@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?
>>>
>>
>> This change was made some months ago :) But yeah.. can change it.
>>
>>>> + *
>>>> + * 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
>>
>> ok
>>
>>>
>>>> + msr_write(MSR_IA32_MISC_ENABLES, msr);
>>>> +
>>>> + msr = msr_read(MSR_IA32_PERF_CTL);
>>>> + msr.lo = 0x101f;
>>>
>>> ditto
>>
>> ok
>>
>>>
>>>> + 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"
>>>
>>
>> ok
>>
>>>> +
>>>> + /* 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.
>>>
>>
>> That one uses the dts as basis for this information and I need to
>> add two cpu nodes. I also thought about using cpuid for this. Not
>> sure what is better.
>>
>
> I don't understand. Isn't your board U-Boot ported from Intel Crown
> Bay? There is arch/x86/dts/crownbay.dts already. You don't need do any
> mods in the dts.
>
The dts is not in its ideal state I would say but that is fixable. I
hope to send
out patches for this board soon.
>>>> +
>>>> +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"
>>
>> ok
>>
>>>
>>>> + { }
>>>> +};
>>>> +
>>>> +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,
>>>> +};
>>>> --
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq
2018-04-10 9:09 ` Christian Gmeiner
@ 2018-04-10 9:11 ` Bin Meng
2018-04-10 9:48 ` Christian Gmeiner
0 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2018-04-10 9:11 UTC (permalink / raw)
To: u-boot
Hi Christian,
On Tue, Apr 10, 2018 at 5:09 PM, Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
> Hi Bin,
>
> 2018-04-10 11:07 GMT+02:00 Bin Meng <bmeng.cn@gmail.com>:
>> Hi Christian,
>>
>> On Tue, Apr 10, 2018 at 4:47 PM, Christian Gmeiner
>> <christian.gmeiner@gmail.com> wrote:
>>> Hi Bin
>>>
>>> Thanks for you quick review!
>>>
>>>>
>>>> On Tue, Apr 10, 2018 at 4:01 PM, Christian Gmeiner
>>>> <christian.gmeiner@gmail.com> wrote:
>>>>> Fixes performance issue when running vx5/vx7 images.
>>>>
>>>> Could you elaborate more on what performance issue you are seeing?
>>>>
>>>
>>> The cache performance was bad without this change. We found this problem
>>> when switching from bldk to u-boot.
>>>
>>> old: https://imagebin.ca/v/3xssw7stbY6A
>>> new: https://imagebin.ca/v/3xstHMCC9fmJ
>>
>> Thank you for the benchmark data. Then please consider describing your
>> benchmark test suite and on what configuration the data is worse
>> without this patch (eg: sequential 128-bit read) in the commit
>> message. This will help other people understand the patch and the
>> commit in the future.
>>
>
> Will do.
>
>>>
>>>> nits: better to spell out "VxWorks"
>>>
>>> ok
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Christian Gmeiner <christian.gmeiner@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?
>>>>
>>>
>>> This change was made some months ago :) But yeah.. can change it.
>>>
>>>>> + *
>>>>> + * 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
>>>
>>> ok
>>>
>>>>
>>>>> + msr_write(MSR_IA32_MISC_ENABLES, msr);
>>>>> +
>>>>> + msr = msr_read(MSR_IA32_PERF_CTL);
>>>>> + msr.lo = 0x101f;
>>>>
>>>> ditto
>>>
>>> ok
>>>
>>>>
>>>>> + 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"
>>>>
>>>
>>> ok
>>>
>>>>> +
>>>>> + /* 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.
>>>>
>>>
>>> That one uses the dts as basis for this information and I need to
>>> add two cpu nodes. I also thought about using cpuid for this. Not
>>> sure what is better.
>>>
>>
>> I don't understand. Isn't your board U-Boot ported from Intel Crown
>> Bay? There is arch/x86/dts/crownbay.dts already. You don't need do any
>> mods in the dts.
>>
>
> The dts is not in its ideal state I would say but that is fixable. I
> hope to send
> out patches for this board soon.
>
Great. Looking forward to your board support patch.
Regards,
Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq
2018-04-10 9:11 ` Bin Meng
@ 2018-04-10 9:48 ` Christian Gmeiner
2018-04-10 10:00 ` Bin Meng
0 siblings, 1 reply; 8+ messages in thread
From: Christian Gmeiner @ 2018-04-10 9:48 UTC (permalink / raw)
To: u-boot
2018-04-10 11:11 GMT+02:00 Bin Meng <bmeng.cn@gmail.com>:
> Hi Christian,
>
> On Tue, Apr 10, 2018 at 5:09 PM, Christian Gmeiner
> <christian.gmeiner@gmail.com> wrote:
>> Hi Bin,
>>
>> 2018-04-10 11:07 GMT+02:00 Bin Meng <bmeng.cn@gmail.com>:
>>> Hi Christian,
>>>
>>> On Tue, Apr 10, 2018 at 4:47 PM, Christian Gmeiner
>>> <christian.gmeiner@gmail.com> wrote:
>>>> Hi Bin
>>>>
>>>> Thanks for you quick review!
>>>>
>>>>>
>>>>> On Tue, Apr 10, 2018 at 4:01 PM, Christian Gmeiner
>>>>> <christian.gmeiner@gmail.com> wrote:
>>>>>> Fixes performance issue when running vx5/vx7 images.
>>>>>
>>>>> Could you elaborate more on what performance issue you are seeing?
>>>>>
>>>>
>>>> The cache performance was bad without this change. We found this problem
>>>> when switching from bldk to u-boot.
>>>>
>>>> old: https://imagebin.ca/v/3xssw7stbY6A
>>>> new: https://imagebin.ca/v/3xstHMCC9fmJ
>>>
>>> Thank you for the benchmark data. Then please consider describing your
>>> benchmark test suite and on what configuration the data is worse
>>> without this patch (eg: sequential 128-bit read) in the commit
>>> message. This will help other people understand the patch and the
>>> commit in the future.
>>>
>>
>> Will do.
>>
>>>>
>>>>> nits: better to spell out "VxWorks"
>>>>
>>>> ok
>>>>
>>>>>
>>>>>>
>>>>>> Signed-off-by: Christian Gmeiner <christian.gmeiner@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?
>>>>>
>>>>
>>>> This change was made some months ago :) But yeah.. can change it.
>>>>
>>>>>> + *
>>>>>> + * 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
>>>>
>>>> ok
>>>>
>>>>>
>>>>>> + msr_write(MSR_IA32_MISC_ENABLES, msr);
>>>>>> +
>>>>>> + msr = msr_read(MSR_IA32_PERF_CTL);
>>>>>> + msr.lo = 0x101f;
>>>>>
>>>>> ditto
>>>>
>>>> ok
I *think* I have read this msr running under a 'good' bios so I am not
really sure
what the meaning of the bits are. I tried a quick google and did not found any
E6xx datasheet containing the description. Is it okay to just use the
magic value here?
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq
2018-04-10 9:48 ` Christian Gmeiner
@ 2018-04-10 10:00 ` Bin Meng
0 siblings, 0 replies; 8+ messages in thread
From: Bin Meng @ 2018-04-10 10:00 UTC (permalink / raw)
To: u-boot
Hi Christian,
On Tue, Apr 10, 2018 at 5:48 PM, Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
> 2018-04-10 11:11 GMT+02:00 Bin Meng <bmeng.cn@gmail.com>:
>> Hi Christian,
>>
>> On Tue, Apr 10, 2018 at 5:09 PM, Christian Gmeiner
>> <christian.gmeiner@gmail.com> wrote:
>>> Hi Bin,
>>>
>>> 2018-04-10 11:07 GMT+02:00 Bin Meng <bmeng.cn@gmail.com>:
>>>> Hi Christian,
>>>>
>>>> On Tue, Apr 10, 2018 at 4:47 PM, Christian Gmeiner
>>>> <christian.gmeiner@gmail.com> wrote:
>>>>> Hi Bin
>>>>>
>>>>> Thanks for you quick review!
>>>>>
>>>>>>
>>>>>> On Tue, Apr 10, 2018 at 4:01 PM, Christian Gmeiner
>>>>>> <christian.gmeiner@gmail.com> wrote:
>>>>>>> Fixes performance issue when running vx5/vx7 images.
>>>>>>
>>>>>> Could you elaborate more on what performance issue you are seeing?
>>>>>>
>>>>>
>>>>> The cache performance was bad without this change. We found this problem
>>>>> when switching from bldk to u-boot.
>>>>>
>>>>> old: https://imagebin.ca/v/3xssw7stbY6A
>>>>> new: https://imagebin.ca/v/3xstHMCC9fmJ
>>>>
>>>> Thank you for the benchmark data. Then please consider describing your
>>>> benchmark test suite and on what configuration the data is worse
>>>> without this patch (eg: sequential 128-bit read) in the commit
>>>> message. This will help other people understand the patch and the
>>>> commit in the future.
>>>>
>>>
>>> Will do.
>>>
>>>>>
>>>>>> nits: better to spell out "VxWorks"
>>>>>
>>>>> ok
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Christian Gmeiner <christian.gmeiner@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?
>>>>>>
>>>>>
>>>>> This change was made some months ago :) But yeah.. can change it.
>>>>>
>>>>>>> + *
>>>>>>> + * 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
>>>>>
>>>>> ok
>>>>>
>>>>>>
>>>>>>> + msr_write(MSR_IA32_MISC_ENABLES, msr);
>>>>>>> +
>>>>>>> + msr = msr_read(MSR_IA32_PERF_CTL);
>>>>>>> + msr.lo = 0x101f;
>>>>>>
>>>>>> ditto
>>>>>
>>>>> ok
>
> I *think* I have read this msr running under a 'good' bios so I am not
> really sure
> what the meaning of the bits are. I tried a quick google and did not found any
> E6xx datasheet containing the description. Is it okay to just use the
> magic value here?
>
I will see if I can find anything.
Regards,
Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-10 10:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-10 8:01 [U-Boot] [PATCH] x86: queensbay: switche P state to the highest freq Christian Gmeiner
2018-04-10 8:17 ` Bin Meng
2018-04-10 8:47 ` Christian Gmeiner
2018-04-10 9:07 ` Bin Meng
2018-04-10 9:09 ` Christian Gmeiner
2018-04-10 9:11 ` Bin Meng
2018-04-10 9:48 ` Christian Gmeiner
2018-04-10 10:00 ` Bin Meng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.