A couple of fixes I applied to get Cello in a slightly better shape (although the observed SATA timeout issue requires a change in the core code)
Ard Biesheuvel (6): Platforms/AMD/Styx: remove incorrect timer frequency Platforms/AMD/Styx: set SATA port mode to Gen3 on all ports Platforms/AMD/StyxDtbLoaderLib: disable SMMUs for absent hardware Platforms/AMD/Cello: add device tree support Platforms/AMD/Cello: reduce core count to 4 Platforms/AMD/Cello: set firmware vendor field to 'LeMaker Cello'
Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 29 ++++++++++++++------ Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf | 9 ++++++ Platforms/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c | 9 +++++- Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc | 7 ++--- Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 6 +--- 5 files changed, 40 insertions(+), 20 deletions(-)
The ARM generic timer needs to be programmed with the actual frequency of the input clock, but this can only be done from the most privileged execution mode implemented by the hardware. UEFI on AArch64 usually executes in EL2, which is not the most privileged execution mode in most cases, and so the timer driver is set up to deal with this: no attempt is made to program the PCD value PcdArmArchTimerFreqInHz into the frequency register. However, a non-zero PCD value is still treated as an override for the register value, in case the programmed value is known to be incorrect.
However, on the various Styx based platforms, the PCD value is set to an incorrect non-zero value, and so the routines that convert time delays into cycle counts are off by 33% (187.5 MHz vs 250 MHz). This may affect timeouts related to SATA link training, and other low level routines that rely on accurate timekeeping.
So remove the explicit PCD settings, so they default to 0, letting the driver use the programmed value instead.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 5 ----- Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc | 5 ----- Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 5 ----- 3 files changed, 15 deletions(-)
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index f068713bf0b8..ddb944d0beb4 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -400,11 +400,6 @@ DEFINE DO_KCS = 0 gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xE112F000
# - # ARM Architectual Timer Frequency - # - gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|187500000 - - # # Cello has 2 SATA ports on the first controller. # gAmdStyxTokenSpaceGuid.PcdSata0PortCount|2 diff --git a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc index b1a7cfd4c4a8..f6d2d37014dd 100644 --- a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc +++ b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc @@ -402,11 +402,6 @@ DEFINE DO_KCS = 1 gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xE112F000
# - # ARM Architectual Timer Frequency - # - gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|187500000 - - # # 2 ports active on Overdrive 1000 # gAmdStyxTokenSpaceGuid.PcdSata0PortCount|2 diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc index 98f5c9452dcd..7ac3ce3760fa 100644 --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc @@ -409,11 +409,6 @@ DEFINE DO_KCS = 1 gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xE112F000
# - # ARM Architectual Timer Frequency - # - gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|187500000 - - # # Overdrive B1 has 14 SATA ports across 2 controllers. # gAmdStyxTokenSpaceGuid.PcdSata0PortCount|8
The SATA related PCDs consumed by AmdSataInitLib contain a PcdSataPortMode PCD that sets the port mode to all ports. This PCD defaults to zero, while the code in question has no default, i.e., the value 0 is not treated as either 1, 2 or 3, and so the init sequence is not carried out correctly.
While observed SATA link detection issues on CelloBoard appear to be unrelated to this (i.e., this change did not improve the situation), let's set the correct values nonetheless.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 1 + Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc | 2 ++ Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 1 + 3 files changed, 4 insertions(+)
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index ddb944d0beb4..d10c0901c811 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -404,6 +404,7 @@ DEFINE DO_KCS = 0 # gAmdStyxTokenSpaceGuid.PcdSata0PortCount|2 gAmdStyxTokenSpaceGuid.PcdSata1PortCount|0 + gAmdStyxTokenSpaceGuid.PcdSataPortMode|0xf
# PCIe Support gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xF0000000 diff --git a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc index f6d2d37014dd..298cf3eb1c28 100644 --- a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc +++ b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc @@ -406,6 +406,8 @@ DEFINE DO_KCS = 1 # gAmdStyxTokenSpaceGuid.PcdSata0PortCount|2 gAmdStyxTokenSpaceGuid.PcdSata1PortCount|0 + gAmdStyxTokenSpaceGuid.PcdSataPortMode|0xf +
# PCIe Support gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xF0000000 diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc index 7ac3ce3760fa..d0b541f9bfa1 100644 --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc @@ -413,6 +413,7 @@ DEFINE DO_KCS = 1 # gAmdStyxTokenSpaceGuid.PcdSata0PortCount|8 gAmdStyxTokenSpaceGuid.PcdSata1PortCount|6 + gAmdStyxTokenSpaceGuid.PcdSataPortMode|0xfffffff
# PCIe Support gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xF0000000
The logic regarding how the various SMMUs are exposed in the device tree is inverted, in the sense that they are present in the static DTB image, and are removed if no SMMU support is requested.
However, the logic is flawed in the sense that it did not remove SMMUs for hardware that is not there to begin with, i.e., the XGBE network ports on Cello/Softiron 1000 or the second SATA controller on B1 silicon.
So fix that.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Platforms/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c b/Platforms/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c index 093db6517c1a..0da00655396e 100644 --- a/Platforms/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c +++ b/Platforms/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c @@ -261,11 +261,18 @@ SetSocIdStatus ( if (!PcdGetBool (PcdEnableSmmus)) { DisableSmmu (Fdt, "iommu-map", "/smb/smmu@e0a00000", "/smb/pcie@f0000000"); DisableSmmu (Fdt, "iommus", "/smb/smmu@e0200000", "/smb/sata@e0300000"); + } + + if (!PcdGetBool (PcdEnableSmmus) || !IsRevB1 || FixedPcdGet8 (PcdSata1PortCount) == 0) { DisableSmmu (Fdt, "iommus", "/smb/smmu@e0c00000", "/smb/sata@e0d00000"); + } + #if DO_XGBE + if (!PcdGetBool (PcdEnableSmmus)) +#endif + { DisableSmmu (Fdt, "iommus", "/smb/smmu@e0600000", "/smb/xgmac@e0700000"); DisableSmmu (Fdt, "iommus", "/smb/smmu@e0800000", "/smb/xgmac@e0900000"); -#endif } }
Now that we have the ability to switch between ACPI and DT support in the UEFI setup menu, let's add DT support to Cello as well. The change is trivial, and it improves the utility of this board for development.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 19 +++++++++++++++++-- Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf | 9 +++++++++ 2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index d10c0901c811..2b041192c887 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -470,9 +470,11 @@ DEFINE DO_KCS = 0 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x0 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0x0
-[PcdsDynamicExHii.common.DEFAULT] +[PcdsDynamicHii] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
+ gAmdStyxTokenSpaceGuid.PcdEnableSmmus|L"StyxEnableSmmus"|gAmdStyxVariableGuid|0x0|FALSE + ################################################################################ # # Components Section - list of all EDK II Modules needed by this Platform @@ -556,6 +558,15 @@ DEFINE DO_KCS = 0 AmdModulePkg/Iscp/IscpDxe.inf
# + # FDT support + # + EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf { + <LibraryClasses> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf + DtPlatformDtbLoaderLib|OpenPlatformPkg/Platforms/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.inf + } + + # # PCI support # AmdModulePkg/Gionb/Gionb.inf @@ -608,7 +619,11 @@ DEFINE DO_KCS = 0 # # ACPI Support # - MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf + MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf { + <LibraryClasses> + NULL|EmbeddedPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf + } + OpenPlatformPkg/Platforms/AMD/Styx/AcpiTables/AcpiAml.inf OpenPlatformPkg/Platforms/AMD/Styx/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf index 29103531a224..6f7428f0c4ca 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf @@ -139,6 +139,15 @@ READ_LOCK_STATUS = TRUE INF AmdModulePkg/Iscp/IscpDxe.inf
# + # FDT support + # + INF EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf + + FILE FREEFORM = 25462CDA-221F-47DF-AC1D-259CFAA4E326 { + SECTION RAW = OpenPlatformPkg/Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dtb + } + + # # PCI support # INF AmdModulePkg/Gionb/Gionb.inf
Update the NUM_CORES define to accurately reflect the core count of the SoC part used on the CelloBoard.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index 2b041192c887..f7d29a49a2dc 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -17,7 +17,7 @@ ################################################################################ [Defines]
-DEFINE NUM_CORES = 8 +DEFINE NUM_CORES = 4 DEFINE DO_KCS = 0
PLATFORM_NAME = Cello
Set the user visible 'firmware vendor' string to 'LeMaker Cello', which is more accurate, and could be helpful when dealing with bug reports, given that it is reported in the Linux kernel log as well.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index f7d29a49a2dc..091914c047a3 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -357,7 +357,7 @@ DEFINE DO_KCS = 0 gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
- gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"AMD Seattle" + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"LeMaker Cello"
# Number of configured cores gArmPlatformTokenSpaceGuid.PcdCoreCount|$(NUM_CORES)
-
Ard Biesheuvel