Hello Martin Roth, Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34421
to review the following change.
Change subject: soc/amd/picasso: Update CPU support ......................................................................
soc/amd/picasso: Update CPU support
Change the Stoney Ridge ID to Picasso. Rename family 15h. Get the number of cores/threads from CPUID as all D18 registers are new.
Change-Id: I44c45db637897f6caf320032c9f79a3a1ab4d6c9 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/cpu.c M src/soc/amd/picasso/include/soc/cpu.h M src/soc/amd/picasso/include/soc/northbridge.h 4 files changed, 10 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/34421/1
diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index daeb7b6..1b49c24 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -32,6 +32,7 @@ #include <amdblocks/acpi.h> #include <soc/acpi.h> #include <soc/pci_devs.h> +#include <soc/cpu.h> #include <soc/southbridge.h> #include <soc/northbridge.h> #include <soc/nvs.h> @@ -241,10 +242,7 @@ { int cores, cpu;
- /* Picasso is single node, just report # of cores */ - cores = pci_read_config32(SOC_NB_DEV, NB_CAPABILITIES2) & CMP_CAP_MASK; - cores++; /* number of cores is CmpCap+1 */ - + cores = get_cpu_count(); printk(BIOS_DEBUG, "ACPI \_PR report %d core(s)\n", cores);
/* Generate BSP _PR.P000 */ diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c index bee2b4b..537a13e 100644 --- a/src/soc/amd/picasso/cpu.c +++ b/src/soc/amd/picasso/cpu.c @@ -2,7 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2015-2016 Intel Corp. - * Copyright (C) 2017 Advanced Micro Devices, Inc. + * Copyright (C) 2017-2019 Advanced Micro Devices, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -53,10 +53,9 @@ x86_mtrr_check(); }
-static int get_cpu_count(void) +int get_cpu_count(void) { - return (pci_read_config16(SOC_HT_DEV, D18F0_CPU_CNT) & CPU_CNT_MASK) - + 1; + return 1 + (cpuid_ecx(0x80000008) & 0xff); }
static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, @@ -117,22 +116,22 @@ set_warm_reset_flag(); }
-static void model_15_init(struct device *dev) +static void model_17_init(struct device *dev) { check_mca(); setup_lapic(); }
static struct device_operations cpu_dev_ops = { - .init = model_15_init, + .init = model_17_init, };
static struct cpu_device_id cpu_table[] = { - { X86_VENDOR_AMD, 0x670f00 }, + { X86_VENDOR_AMD, 0x810f81 }, { 0, 0 }, };
-static const struct cpu_driver model_15 __cpu_driver = { +static const struct cpu_driver model_17 __cpu_driver = { .ops = &cpu_dev_ops, .id_table = cpu_table, }; diff --git a/src/soc/amd/picasso/include/soc/cpu.h b/src/soc/amd/picasso/include/soc/cpu.h index 95bef37..c138131 100644 --- a/src/soc/amd/picasso/include/soc/cpu.h +++ b/src/soc/amd/picasso/include/soc/cpu.h @@ -24,6 +24,7 @@ #define CSTATE_BASE_REG 0xc0010073
void picasso_init_cpus(struct device *dev); +int get_cpu_count(void); void check_mca(void);
#endif /* __PICASSO_CPU_H__ */ diff --git a/src/soc/amd/picasso/include/soc/northbridge.h b/src/soc/amd/picasso/include/soc/northbridge.h index 65705b9..286b0b0 100644 --- a/src/soc/amd/picasso/include/soc/northbridge.h +++ b/src/soc/amd/picasso/include/soc/northbridge.h @@ -95,10 +95,6 @@ #define D18F1_VGAEN 0xf4 # define VGA_ADDR_ENABLE (1 << 0)
-/* D18F5 */ -#define NB_CAPABILITIES2 0x84 -#define CMP_CAP_MASK 0xff - enum { /* SMM handler area. */ SMM_SUBREGION_HANDLER,
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34421 )
Change subject: soc/amd/picasso: Update CPU support ......................................................................
Patch Set 1: Code-Review+2
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34421
to look at the new patch set (#5).
Change subject: soc/amd/picasso: Update CPU support ......................................................................
soc/amd/picasso: Update CPU support
Change the Stoney Ridge ID to Picasso. Rename family 15h. Get the number of cores/threads from CPUID as all D18 registers are new.
Change-Id: I44c45db637897f6caf320032c9f79a3a1ab4d6c9 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/cpu.c M src/soc/amd/picasso/include/soc/cpu.h M src/soc/amd/picasso/include/soc/northbridge.h 5 files changed, 11 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/34421/5
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34421
to look at the new patch set (#6).
Change subject: soc/amd/picasso: Update CPU support ......................................................................
soc/amd/picasso: Update CPU support
Change the Stoney Ridge ID to Picasso. Rename family 15h. Get the number of cores/threads from CPUID as all D18 registers are new.
Change-Id: I44c45db637897f6caf320032c9f79a3a1ab4d6c9 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/cpu.c M src/soc/amd/picasso/include/soc/cpu.h M src/soc/amd/picasso/include/soc/northbridge.h 5 files changed, 11 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/34421/6
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34421 )
Change subject: soc/amd/picasso: Update CPU support ......................................................................
Patch Set 6: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34421 )
Change subject: soc/amd/picasso: Update CPU support ......................................................................
soc/amd/picasso: Update CPU support
Change the Stoney Ridge ID to Picasso. Rename family 15h. Get the number of cores/threads from CPUID as all D18 registers are new.
Change-Id: I44c45db637897f6caf320032c9f79a3a1ab4d6c9 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34421 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Martin Roth martinroth@google.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/cpu.c M src/soc/amd/picasso/include/soc/cpu.h M src/soc/amd/picasso/include/soc/northbridge.h 5 files changed, 11 insertions(+), 16 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 1c2ec84..5c5179d 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -27,6 +27,7 @@ select ARCH_ROMSTAGE_X86_32 select ARCH_RAMSTAGE_X86_32 select X86_AMD_FIXED_MTRRS + select X86_AMD_INIT_SIPI select ACPI_AMD_HARDWARE_SLEEP_VALUES select COLLECT_TIMESTAMPS_NO_TSC select DRIVERS_I2C_DESIGNWARE diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 5f897fe..cc06496 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -32,6 +32,7 @@ #include <amdblocks/acpi.h> #include <soc/acpi.h> #include <soc/pci_devs.h> +#include <soc/cpu.h> #include <soc/southbridge.h> #include <soc/northbridge.h> #include <soc/nvs.h> @@ -238,10 +239,7 @@ { int cores, cpu;
- /* Picasso is single node, just report # of cores */ - cores = pci_read_config32(SOC_NB_DEV, NB_CAPABILITIES2) & CMP_CAP_MASK; - cores++; /* number of cores is CmpCap+1 */ - + cores = get_cpu_count(); printk(BIOS_DEBUG, "ACPI \_PR report %d core(s)\n", cores);
/* Generate BSP _PR.P000 */ diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c index 5c2ca43..21818e4 100644 --- a/src/soc/amd/picasso/cpu.c +++ b/src/soc/amd/picasso/cpu.c @@ -2,7 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2015-2016 Intel Corp. - * Copyright (C) 2017 Advanced Micro Devices, Inc. + * Copyright (C) 2017-2019 Advanced Micro Devices, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -54,10 +54,9 @@ x86_mtrr_check(); }
-static int get_cpu_count(void) +int get_cpu_count(void) { - return (pci_read_config16(SOC_HT_DEV, D18F0_CPU_CNT) & CPU_CNT_MASK) - + 1; + return 1 + (cpuid_ecx(0x80000008) & 0xff); }
static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, @@ -118,22 +117,22 @@ set_warm_reset_flag(); }
-static void model_15_init(struct device *dev) +static void model_17_init(struct device *dev) { check_mca(); setup_lapic(); }
static struct device_operations cpu_dev_ops = { - .init = model_15_init, + .init = model_17_init, };
static struct cpu_device_id cpu_table[] = { - { X86_VENDOR_AMD, 0x670f00 }, + { X86_VENDOR_AMD, 0x810f81 }, { 0, 0 }, };
-static const struct cpu_driver model_15 __cpu_driver = { +static const struct cpu_driver model_17 __cpu_driver = { .ops = &cpu_dev_ops, .id_table = cpu_table, }; diff --git a/src/soc/amd/picasso/include/soc/cpu.h b/src/soc/amd/picasso/include/soc/cpu.h index 7bc1810..e57d2d3 100644 --- a/src/soc/amd/picasso/include/soc/cpu.h +++ b/src/soc/amd/picasso/include/soc/cpu.h @@ -22,6 +22,7 @@ #define SOC_EARLY_VMTRR_TEMPRAM 2
void picasso_init_cpus(struct device *dev); +int get_cpu_count(void); void check_mca(void);
#endif /* __PICASSO_CPU_H__ */ diff --git a/src/soc/amd/picasso/include/soc/northbridge.h b/src/soc/amd/picasso/include/soc/northbridge.h index 667a49e..e423ab1 100644 --- a/src/soc/amd/picasso/include/soc/northbridge.h +++ b/src/soc/amd/picasso/include/soc/northbridge.h @@ -73,10 +73,6 @@ #define D18F1_VGAEN 0xf4 # define VGA_ADDR_ENABLE (1 << 0)
-/* D18F5 */ -#define NB_CAPABILITIES2 0x84 -#define CMP_CAP_MASK 0xff - void amd_initcpuio(void);
void domain_enable_resources(struct device *dev);
-
Marshall Dawson (Code Review) -
Martin Roth (Code Review)