Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31372
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
soc/amd: Add support to soc merlinfalcon
Merlinfalcon is essentially stoneyridge (00670F00), but using a carizo CPU. Therefore add CPU_CARIZO config parameter and minor changes when this config is selected.
BUG=b:none. TEST=Tested later with padmelon board.
Change-Id: I67cf11432901857354a8f7192b330dacad383a9e Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/acpi/cpu.asl M src/soc/amd/stoneyridge/chip.h M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/pci_devs.h M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/southbridge.c 8 files changed, 66 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31372/1
diff --git a/src/soc/amd/stoneyridge/Kconfig b/src/soc/amd/stoneyridge/Kconfig index 80798d9..2d1ac1e 100644 --- a/src/soc/amd/stoneyridge/Kconfig +++ b/src/soc/amd/stoneyridge/Kconfig @@ -23,6 +23,15 @@ help AMD Stoney Ridge FT4 support
+config STONEYRIDGE_CPU_CARIZO + bool + default n + help + For SOC MerlinFalcon, select SOC_AMD_STONEYRIDGE_FP4 and + STONEYRIDGE_CPU_CARIZO as the only difference between StoneyRidge + and MerlinFalcon is the internal CPU, and therefore the video + (3rdparty/blobs/soc/amd/merlinfalcon). + if SOC_AMD_STONEYRIDGE_FP4 || SOC_AMD_STONEYRIDGE_FT4
config CPU_SPECIFIC_OPTIONS @@ -128,6 +137,7 @@
config VGA_BIOS_ID string + default "1002,9874" if STONEYRIDGE_CPU_CARIZO default "1002,98e4" help The default VGA BIOS PCI vendor/device ID should be set to the diff --git a/src/soc/amd/stoneyridge/acpi/cpu.asl b/src/soc/amd/stoneyridge/acpi/cpu.asl index 414326e..94638b0 100644 --- a/src/soc/amd/stoneyridge/acpi/cpu.asl +++ b/src/soc/amd/stoneyridge/acpi/cpu.asl @@ -34,7 +34,15 @@ /* Return a package containing enabled processor entries */ Method (PPKG) { - If (LGreaterEqual (\PCNT, 2)) { + If (LGreaterEqual (\PCNT, 4)) { + Return (Package () + { + _PR.P000, + _PR.P001, + _PR.P002, + _PR.P003 + }) + } ElseIf (LGreaterEqual (\PCNT, 2)) { Return (Package () { _PR.P000, diff --git a/src/soc/amd/stoneyridge/chip.h b/src/soc/amd/stoneyridge/chip.h index 92223d1..432ed04 100644 --- a/src/soc/amd/stoneyridge/chip.h +++ b/src/soc/amd/stoneyridge/chip.h @@ -24,8 +24,13 @@ #include <arch/acpi_device.h>
#define MAX_NODES 1 +#if IS_ENABLED(CONFIG_STONEYRIDGE_CPU_CARIZO) +#define MAX_DRAM_CH 2 +#define MAX_DIMMS_PER_CH 4 +#else #define MAX_DRAM_CH 1 #define MAX_DIMMS_PER_CH 2 +#endif
#define STONEY_I2C_DEV_MAX 4
diff --git a/src/soc/amd/stoneyridge/cpu.c b/src/soc/amd/stoneyridge/cpu.c index 1d9804d..3c47ec8 100644 --- a/src/soc/amd/stoneyridge/cpu.c +++ b/src/soc/amd/stoneyridge/cpu.c @@ -145,7 +145,11 @@ };
static struct cpu_device_id cpu_table[] = { +#if IS_ENABLED(CONFIG_STONEYRIDGE_CPU_CARIZO) + { X86_VENDOR_AMD, 0x660f01 }, +#else { X86_VENDOR_AMD, 0x670f00 }, +#endif { 0, 0 }, };
diff --git a/src/soc/amd/stoneyridge/include/soc/pci_devs.h b/src/soc/amd/stoneyridge/include/soc/pci_devs.h index 20ab99b..60125b1 100644 --- a/src/soc/amd/stoneyridge/include/soc/pci_devs.h +++ b/src/soc/amd/stoneyridge/include/soc/pci_devs.h @@ -43,14 +43,16 @@ /* Internal Graphics */ #define GFX_DEV 0x1 #define GFX_FUNC 0 -#define GFX_DEVID 0x98e4 /* subject to SKU/OPN variation */ +#define GFX_DEVID_CZ 0x9874 /* subject to SKU/OPN variation */ +#define GFX_DEVID_ST 0x98e4 /* subject to SKU/OPN variation */ #define GFX_DEVFN PCI_DEVFN(GFX_DEV, GFX_FUNC) #define SOC_GFX_DEV _SOC_DEV(GFX_DEV, GFX_FUNC)
/* HD Audio 0 */ #define HDA0_DEV 0x1 #define HDA0_FUNC 1 -#define HDA0_DEVID 0x15b3 +#define HDA0_DEVID_CZ 0x9840 +#define HDA0_DEVID_ST 0x15b3 #define HDA0_DEVFN PCI_DEVFN(HDA0_DEV, HDA0_FUNC) #define SOC_HDA0_DEV _SOC_DEV(HDA0_DEV, HDA0_FUNC)
@@ -113,42 +115,48 @@ /* HT Configuration */ #define HT_DEV 0x18 #define HT_FUNC 0 -#define HT_DEVID 0x15b0 +#define HT_DEVID_CZ 0x1570 +#define HT_DEVID_ST 0x15b0 #define HT_DEVFN PCI_DEVFN(HT_DEV, HT_FUNC) #define SOC_HT_DEV _SOC_DEV(HT_DEV, HT_FUNC)
/* Address Maps */ #define ADDR_DEV 0x18 #define ADDR_FUNC 1 -#define ADDR_DEVID 0x15b1 +#define ADDR_DEVID_CZ 0x1571 +#define ADDR_DEVID_ST 0x15b1 #define ADDR_DEVFN PCI_DEVFN(ADDR_DEV, ADDR_FUNC) #define SOC_ADDR_DEV _SOC_DEV(ADDR_DEV, ADDR_FUNC)
/* DRAM Configuration */ #define DCT_DEV 0x18 #define DCT_FUNC 2 -#define DCT_DEVID 0x15b2 +#define DCT_DEVID_CZ 0x1572 +#define DCT_DEVID_ST 0x15b2 #define DCT_DEVFN PCI_DEVFN(DCT_DEV, DCT_FUNC) #define SOC_DCT_DEV _SOC_DEV(DCT_DEV, DCT_FUNC)
/* Misc. Configuration */ #define MISC_DEV 0x18 #define MISC_FUNC 3 -#define MISC_DEVID 0x15b3 +#define MISC_DEVID_CZ 0x1573 +#define MISC_DEVID_ST 0x15b3 #define MISC_DEVFN PCI_DEVFN(MISC_DEV, MISC_FUNC) #define SOC_MISC_DEV _SOC_DEV(MISC_DEV, MISC_FUNC)
/* PM Configuration */ #define PM_DEV 0x18 #define PM_FUNC 4 -#define PM_DEVID 0x15b4 +#define PM_DEVID_CZ 0x1574 +#define PM_DEVID_ST 0x15b4 #define PM_DEVFN PCI_DEVFN(PM_DEV, PM_FUNC) #define SOC_PM_DEV _SOC_DEV(PM_DEV, PM_FUNC)
/* Northbridge Configuration */ #define NB_DEV 0x18 #define NB_FUNC 5 -#define NB_DEVID 0x15b5 +#define NB_DEVID_CZ 0x1575 +#define NB_DEVID_ST 0x15b5 #define NB_DEVFN PCI_DEVFN(NB_DEV, NB_FUNC) #define SOC_NB_DEV _SOC_DEV(NB_DEV, NB_FUNC)
diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h index 3ae6b4a..705fe7a 100644 --- a/src/soc/amd/stoneyridge/include/soc/southbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h @@ -186,6 +186,7 @@ #define MISC_CLK_CNTL1 0x40 #define CG1PLL_FBDIV_TEST BIT(26) #define OSCOUT1_CLK_OUTPUT_ENB BIT(2) /* 0 = Enabled, 1 = Disabled */ +#define OSCOUT2_CLK_OUTPUT_ENB BIT(7) /* 0 = Enabled, 1 = Disabled */
/* XHCI_PM Registers: 0xfed81c00 */ #define XHCI_PM_INDIRECT_INDEX 0x48 @@ -477,7 +478,7 @@ void enable_aoac_devices(void); void sb_enable_rom(void); void configure_stoneyridge_i2c(void); -void sb_clk_output_48Mhz(void); +void sb_clk_output_48Mhz(u32 osc); void sb_disable_4dw_burst(void); void sb_enable(struct device *dev); void southbridge_final(void *chip_info); diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index 4a856a9..0e7b4b8 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -338,7 +338,11 @@ static const struct pci_driver family15_northbridge __pci_driver = { .ops = &northbridge_operations, .vendor = PCI_VENDOR_ID_AMD, +#if IS_ENABLED(CONFIG_STONEYRIDGE_CPU_CARIZO) + .device = PCI_DEVICE_ID_AMD_15H_MODEL_606F_NB_HT, +#else .device = PCI_DEVICE_ID_AMD_15H_MODEL_707F_NB_HT, +#endif };
/* @@ -453,8 +457,13 @@ { u32 new_vendev; new_vendev = +#if IS_ENABLED(CONFIG_STONEYRIDGE_CPU_CARIZO) + ((vendev >= 0x10029870) && (vendev <= 0x1002987f)) ? + 0x10029870 : vendev; +#else ((vendev >= 0x100298e0) && (vendev <= 0x100298ef)) ? 0x100298e0 : vendev; +#endif
if (vendev != new_vendev) printk(BIOS_NOTICE, "Mapping PCI device %8x to %8x\n", diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index c8d66ac..798bcb4 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -389,7 +389,7 @@ pm_write32(PM_DECODE_EN, reg | LEGACY_IO_EN); }
-void sb_clk_output_48Mhz(void) +void sb_clk_output_48Mhz(u32 osc) { u32 ctrl; u32 *misc_clk_cntl_1_ptr = (u32 *)(uintptr_t)(MISC_MMIO_BASE @@ -401,8 +401,16 @@ */ ctrl = read32(misc_clk_cntl_1_ptr);
- /* clear the OSCOUT1_ClkOutputEnb to enable the 48 Mhz clock */ - ctrl &= ~OSCOUT1_CLK_OUTPUT_ENB; + switch (osc) { + case 1: + ctrl &= ~OSCOUT1_CLK_OUTPUT_ENB; + break; + case 2: + ctrl &= ~OSCOUT2_CLK_OUTPUT_ENB; + break; + default: + return; /* do nothing if invalid */ + } write32(misc_clk_cntl_1_ptr, ctrl); }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31372 )
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/cpu.c File src/soc/amd/stoneyridge/cpu.c:
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/cpu.c@150 PS1, Line 150: #else braces {} are not necessary for single statement blocks
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31372 )
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
Patch Set 1:
(1 comment)
This change is ready for review.
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/northbridge.... File src/soc/amd/stoneyridge/northbridge.c:
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/northbridge.... PS1, Line 342: device You'll want to change this from .device to .devices and point to a devices list.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31372 )
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
Patch Set 1:
(5 comments)
This change is ready for review.
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/include/soc/... File src/soc/amd/stoneyridge/include/soc/southbridge.h:
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/include/soc/... PS1, Line 189: #define OSCOUT2_CLK_OUTPUT_ENB BIT(7) /* 0 = Enabled, 1 = Disabled */
This should be split out into a separate patch and applied to ST first.
ST only has 1 oscillator, it makes no sense to apply it to ST alone. This is for SOC that have more then 1 core. Stoney has 1 core with hyperthread, Merlin has 2 cores with hyperthread. It's one oscillator per core.
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/northbridge.... File src/soc/amd/stoneyridge/northbridge.c:
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/northbridge.... PS1, Line 341: #if IS_ENABLED(CONFIG_STONEYRIDGE_CPU_CARIZO)
No need for #if #else because this driver is to be compatible with both.
Will see how to do it, maybe following Martin's suggestion.
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/northbridge.... PS1, Line 342: device
You'll want to change this from .device to .devices and point to a devices list.
Thanks for the tip.
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/northbridge.... PS1, Line 460: #if IS_ENABLED(CONFIG_STONEYRIDGE_CPU_CARIZO)
Remove the #if #else like above. Just do the logic in code for both.
Again, will see how to do it.
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/southbridge.... File src/soc/amd/stoneyridge/southbridge.c:
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/southbridge.... PS1, Line 392: void sb_clk_output_48Mhz(u32 osc)
Split the change to this function to a separate patch (as mentioned in the .h file).
Even though it makes no sense for stoney alone?
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31372 )
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
Patch Set 1:
(2 comments)
Patch Set 1:
(1 comment)
This change is ready for review.
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/include/soc/... File src/soc/amd/stoneyridge/include/soc/pci_devs.h:
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/include/soc/... PS1, Line 118: #define HT_DEVID_CZ 0x1570 : #define HT_DEVID_ST 0x15b0
Skip the graphics. Yes, HDA should be added. […]
This is what I'm planning to do... do you have a different idea? /* HD Audio 0 */ #define HDA0_DEV 0x1 #define HDA0_FUNC 1 #if IS_ENABLED(CONFIG_STONEYRIDGE_CPU_MF) #define HDA0_DEVID PCI_DEVICE_ID_AMD_15H_MODEL_606F_HDA #else #define HDA0_DEVID PCI_DEVICE_ID_AMD_15H_MODEL_707F_HDA #endif #define HDA0_DEVFN PCI_DEVFN(HDA0_DEV, HDA0_FUNC) #define SOC_HDA0_DEV _SOC_DEV(HDA0_DEV, HDA0_FUNC)
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/include/soc/... File src/soc/amd/stoneyridge/include/soc/southbridge.h:
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/include/soc/... PS1, Line 189: #define OSCOUT2_CLK_OUTPUT_ENB BIT(7) /* 0 = Enabled, 1 = Disabled */
This is not about the core(s). They're auxiliary clocks from the FCH to external devices. […]
Ok, I was not aware of this difference between Intel and AMD hyperthread. I have worked mostly with Intel chips throughout my career. I only ever did 2 AMD CPU, both a long time ago(12y and 17y).
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31372 )
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
Patch Set 1:
(1 comment)
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/include/soc/... File src/soc/amd/stoneyridge/include/soc/pci_devs.h:
https://review.coreboot.org/#/c/31372/1/src/soc/amd/stoneyridge/include/soc/... PS1, Line 118: #define HT_DEVID_CZ 0x1570 : #define HT_DEVID_ST 0x15b0
I don't see that we're using the xxxx_DEVID defines at all. […]
And remove them from here? As far as I could search, we are not using them, and I don't know why you placed them here, except that it makes easier to whoever wants to use them to know the appropriate value to use.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31372 )
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
Patch Set 2:
(2 comments)
This change is ready for review.
https://review.coreboot.org/#/c/31372/2/src/soc/amd/stoneyridge/Kconfig File src/soc/amd/stoneyridge/Kconfig:
https://review.coreboot.org/#/c/31372/2/src/soc/amd/stoneyridge/Kconfig@26 PS2, Line 26: STONEYRIDGE_CPU_MF
This name is bugging me because they're separate products. […]
Ok, will do. I was kind of avoiding it...
https://review.coreboot.org/#/c/31372/2/src/soc/amd/stoneyridge/Kconfig@30 PS2, Line 30: For SOC MerlinFalcon, select SOC_AMD_STONEYRIDGE_FP4 and : STONEYRIDGE_CPU_MF as the only difference between StoneyRidge : and MerlinFalcon is the internal CPU, and therefore the video : (3rdparty/blobs/soc/amd/merlinfalcon) and AGESA.
It took me a while to see what you were doing here. […]
Not really. If I use SOC_AMD_MERLINFALCON_FP4, some of the code will change and no longer need selecting stoneyridge fp4. Originally I was trying to avoid lines with more than 80 characters in some places.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31372 )
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
Patch Set 2:
(1 comment)
This change is ready for review.
https://review.coreboot.org/#/c/31372/2/src/soc/amd/stoneyridge/Kconfig File src/soc/amd/stoneyridge/Kconfig:
https://review.coreboot.org/#/c/31372/2/src/soc/amd/stoneyridge/Kconfig@30 PS2, Line 30: For SOC MerlinFalcon, select SOC_AMD_STONEYRIDGE_FP4 and : STONEYRIDGE_CPU_MF as the only difference between StoneyRidge : and MerlinFalcon is the internal CPU, and therefore the video : (3rdparty/blobs/soc/amd/merlinfalcon) and AGESA.
I think you missed what I was suggesting. Change this whole thing to be: […]
I understood perfectly. I just said it's no longer needed when I define SOC_AMD_MERLINFALCON_FP4 due to the changes in Kconfig that this definition entitles.
Richard Spiegel has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31372 )
Change subject: soc/amd: Add support to soc merlinfalcon ......................................................................
Abandoned
Code was developed with wrong padmelon, not using merlinfalcon. New one coming in.
-
build bot (Jenkins) (Code Review) -
Martin Roth (Code Review)
-
Richard Spiegel (Code Review)