Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
mb/pcengines/apu2: add reset logic for PCIe slots
PC Engines apu2 had many problems with PCIe cards detection. The cards were inconsistently detected when booted from G3, S5 or after a reboot. AGESA can reset PCIe slots using GPIO via callback. Use it to reset the slots that support using GPIO as reset signal.
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I8ff7db6ff85cce45b84729be905e6c895a24f6f2 --- M src/mainboard/pcengines/apu2/BiosCallOuts.c M src/mainboard/pcengines/apu2/OemCustomize.c M src/mainboard/pcengines/apu2/romstage.c 3 files changed, 94 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/39703/1
diff --git a/src/mainboard/pcengines/apu2/BiosCallOuts.c b/src/mainboard/pcengines/apu2/BiosCallOuts.c index d6f8d84..9763c4d 100644 --- a/src/mainboard/pcengines/apu2/BiosCallOuts.c +++ b/src/mainboard/pcengines/apu2/BiosCallOuts.c @@ -13,6 +13,7 @@ */
#include <AGESA.h> +#include <amdblocks/acpimmio.h> #include <console/console.h> #include <spd_bin.h> #include <northbridge/amd/agesa/BiosCallOuts.h> @@ -24,6 +25,7 @@ #include "hudson.h"
static AGESA_STATUS board_ReadSpd_from_cbfs(UINT32 Func, UINTN Data, VOID *ConfigPtr); +static AGESA_STATUS board_GnbPcieSlotReset (UINT32 Func, UINTN Data, VOID *ConfigPtr);
const BIOS_CALLOUT_STRUCT BiosCallouts[] = { @@ -32,6 +34,7 @@ {AGESA_READ_SPD_RECOVERY, agesa_NoopUnsupported }, {AGESA_RUNFUNC_ONAP, agesa_RunFuncOnAp }, {AGESA_GET_IDS_INIT_DATA, agesa_EmptyIdsInitData }, + {AGESA_GNB_PCIE_SLOT_RESET, board_GnbPcieSlotReset }, {AGESA_HOOKBEFORE_DQS_TRAINING, agesa_NoopSuccess }, {AGESA_HOOKBEFORE_EXIT_SELF_REF, agesa_NoopSuccess } }; @@ -139,3 +142,71 @@
return AGESA_SUCCESS; } + +/* PCIE slot reset control */ +static AGESA_STATUS board_GnbPcieSlotReset (UINT32 Func, UINTN Data, VOID *ConfigPtr) +{ + AGESA_STATUS Status; + PCIe_SLOT_RESET_INFO *ResetInfo; + uint32_t GpioData; + uint8_t GpioValue; + + ResetInfo = ConfigPtr; + Status = AGESA_UNSUPPORTED; + + switch (ResetInfo->ResetId) { + /* + * ResetID 1 = PCIE_RST# affects all PCIe slots on all boards except + * apu2. ResetID 1 does not need any GPIO. + */ + case 1: + Status = AGESA_SUCCESS; + break; + case 51: /* GPIO51 resets mPCIe1 slot on apu2 */ + switch (ResetInfo->ResetControl) { + case AssertSlotReset: + GpioData = gpio1_read32(0x8); + printk(BIOS_DEBUG, "%s: ResetID %u assert %08x\n", + __func__, ResetInfo->ResetId, GpioData); + GpioValue = gpio1_read8(0xa); + GpioValue &= ~BIT6; + gpio1_write8(0xa, GpioValue); + Status = AGESA_SUCCESS; + break; + case DeassertSlotReset: + GpioData = gpio1_read32(0x8); + printk(BIOS_DEBUG, "%s: ResetID %u deassert %08x\n", + __func__, ResetInfo->ResetId, GpioData); + GpioValue = gpio1_read8(0xa); + GpioValue |= BIT6; + gpio1_write8(0xa, GpioValue); + Status = AGESA_SUCCESS; + break; + } + break; + case 55: /* GPIO51 resets mPCIe2 slot on apu2 */ + switch (ResetInfo->ResetControl) { + case AssertSlotReset: + GpioData = gpio1_read32(0xc); + printk(BIOS_DEBUG, "%s: ResetID %u assert %08x\n", + __func__, ResetInfo->ResetId, GpioData); + GpioValue = gpio1_read8(0xe); + GpioValue &= ~BIT6; + gpio1_write8(0xa, GpioValue); + Status = AGESA_SUCCESS; + break; + case DeassertSlotReset: + GpioData = gpio1_read32(0xc); + printk(BIOS_DEBUG, "%s: ResetID %u deassert %08x\n", + __func__, ResetInfo->ResetId, GpioData); + GpioValue = gpio1_read8(0xe); + GpioValue |= BIT6; + gpio1_write8(0xa, GpioValue); + Status = AGESA_SUCCESS; + break; + } + break; + } + + return Status; +} diff --git a/src/mainboard/pcengines/apu2/OemCustomize.c b/src/mainboard/pcengines/apu2/OemCustomize.c index 99b9d51..6339e0f 100644 --- a/src/mainboard/pcengines/apu2/OemCustomize.c +++ b/src/mainboard/pcengines/apu2/OemCustomize.c @@ -15,6 +15,15 @@ #include <AGESA.h> #include <northbridge/amd/agesa/state_machine.h>
+#define PCIE_NIC_RESET_ID 1 + +#if CONFIG(BOARD_PCENGINES_APU2) +#define PCIE_GFX_RESET_ID 55 +#define PCIE_PORT3_RESET_ID 51 +#else +#define PCIE_GFX_RESET_ID PCIE_NIC_RESET_ID +#define PCIE_PORT3_RESET_ID PCIE_NIC_RESET_ID +#endif
static const PCIe_PORT_DESCRIPTOR PortList[] = { { @@ -24,7 +33,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x01, 0) + AspmDisabled, PCIE_PORT3_RESET_ID, 0) }, /* Initialize Port descriptor (PCIe port, Lanes 1, PCI Device Number 2, ...) */ { @@ -34,7 +43,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x02, 0) + AspmDisabled, PCIE_NIC_RESET_ID, 0) }, /* Initialize Port descriptor (PCIe port, Lanes 2, PCI Device Number 2, ...) */ { @@ -44,7 +53,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x03, 0) + AspmDisabled, PCIE_NIC_RESET_ID, 0) }, /* Initialize Port descriptor (PCIe port, Lanes 3, PCI Device Number 2, ...) */ { @@ -54,7 +63,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x04, 0) + AspmDisabled, PCIE_NIC_RESET_ID, 0) }, /* Initialize Port descriptor (PCIe port, Lanes 4-7, PCI Device Number 4, ...) */ { @@ -64,7 +73,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x05, 0) + AspmDisabled, PCIE_GFX_RESET_ID, 0) } };
diff --git a/src/mainboard/pcengines/apu2/romstage.c b/src/mainboard/pcengines/apu2/romstage.c index 7565bfc..34c42b4 100644 --- a/src/mainboard/pcengines/apu2/romstage.c +++ b/src/mainboard/pcengines/apu2/romstage.c @@ -38,6 +38,15 @@
/* Release GPIO32/33 for other uses. */ pm_write8(0xea, 1); + + /* + * Assert resets on the PCIe slots, since AGESA calls deassert callout + * only. Only apu2 uses GPIOs to reset PCIe slots. + */ + if (CONFIG(BOARD_PCENGINES_APU2)) { + gpio1_write8(0xa, gpio1_read8(0xa) & ~(1 << 6)); + gpio1_write8(0xe, gpio1_read8(0xe) & ~(1 << 6)); + } }
static void early_lpc_init(void)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39703/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/BiosCallOuts.c:
https://review.coreboot.org/c/coreboot/+/39703/1/src/mainboard/pcengines/apu... PS1, Line 28: static AGESA_STATUS board_GnbPcieSlotReset (UINT32 Func, UINTN Data, VOID *ConfigPtr); space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39703/1/src/mainboard/pcengines/apu... PS1, Line 147: static AGESA_STATUS board_GnbPcieSlotReset (UINT32 Func, UINTN Data, VOID *ConfigPtr) space prohibited between function name and open parenthesis '('
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39703/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/BiosCallOuts.c:
https://review.coreboot.org/c/coreboot/+/39703/1/src/mainboard/pcengines/apu... PS1, Line 28: static AGESA_STATUS board_GnbPcieSlotReset (UINT32 Func, UINTN Data, VOID *ConfigPtr);
space prohibited between function name and open parenthesis '('
then... maybe use PciEx? This would keep the alignment for the parameters
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
Patch Set 1: Code-Review+1
Hello build bot (Jenkins), Paul Menzel, Angel Pons, Piotr Król,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39703
to look at the new patch set (#2).
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
mb/pcengines/apu2: add reset logic for PCIe slots
PC Engines apu2 had many problems with PCIe cards detection. The cards were inconsistently detected when booted from G3, S5 or after a reboot. AGESA can reset PCIe slots using GPIO via callback. Use it to reset the slots that support using GPIO as reset signal.
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I8ff7db6ff85cce45b84729be905e6c895a24f6f2 --- M src/mainboard/pcengines/apu2/BiosCallOuts.c M src/mainboard/pcengines/apu2/OemCustomize.c M src/mainboard/pcengines/apu2/romstage.c 3 files changed, 94 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/39703/2
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39703/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/BiosCallOuts.c:
https://review.coreboot.org/c/coreboot/+/39703/1/src/mainboard/pcengines/apu... PS1, Line 28: static AGESA_STATUS board_GnbPcieSlotReset (UINT32 Func, UINTN Data, VOID *ConfigPtr);
then... […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
Patch Set 2: Code-Review+2
Michał Żygowski has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
mb/pcengines/apu2: add reset logic for PCIe slots
PC Engines apu2 had many problems with PCIe cards detection. The cards were inconsistently detected when booted from G3, S5 or after a reboot. AGESA can reset PCIe slots using GPIO via callback. Use it to reset the slots that support using GPIO as reset signal.
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I8ff7db6ff85cce45b84729be905e6c895a24f6f2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/39703 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/pcengines/apu2/BiosCallOuts.c M src/mainboard/pcengines/apu2/OemCustomize.c M src/mainboard/pcengines/apu2/romstage.c 3 files changed, 94 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/mainboard/pcengines/apu2/BiosCallOuts.c b/src/mainboard/pcengines/apu2/BiosCallOuts.c index d6f8d84..69af3f9 100644 --- a/src/mainboard/pcengines/apu2/BiosCallOuts.c +++ b/src/mainboard/pcengines/apu2/BiosCallOuts.c @@ -13,6 +13,7 @@ */
#include <AGESA.h> +#include <amdblocks/acpimmio.h> #include <console/console.h> #include <spd_bin.h> #include <northbridge/amd/agesa/BiosCallOuts.h> @@ -24,6 +25,7 @@ #include "hudson.h"
static AGESA_STATUS board_ReadSpd_from_cbfs(UINT32 Func, UINTN Data, VOID *ConfigPtr); +static AGESA_STATUS board_GnbPciExSlotReset(UINT32 Func, UINTN Data, VOID *ConfigPtr);
const BIOS_CALLOUT_STRUCT BiosCallouts[] = { @@ -32,6 +34,7 @@ {AGESA_READ_SPD_RECOVERY, agesa_NoopUnsupported }, {AGESA_RUNFUNC_ONAP, agesa_RunFuncOnAp }, {AGESA_GET_IDS_INIT_DATA, agesa_EmptyIdsInitData }, + {AGESA_GNB_PCIE_SLOT_RESET, board_GnbPciExSlotReset }, {AGESA_HOOKBEFORE_DQS_TRAINING, agesa_NoopSuccess }, {AGESA_HOOKBEFORE_EXIT_SELF_REF, agesa_NoopSuccess } }; @@ -139,3 +142,71 @@
return AGESA_SUCCESS; } + +/* PCIE slot reset control */ +static AGESA_STATUS board_GnbPciExSlotReset(UINT32 Func, UINTN Data, VOID *ConfigPtr) +{ + AGESA_STATUS Status; + PCIe_SLOT_RESET_INFO *ResetInfo; + uint32_t GpioData; + uint8_t GpioValue; + + ResetInfo = ConfigPtr; + Status = AGESA_UNSUPPORTED; + + switch (ResetInfo->ResetId) { + /* + * ResetID 1 = PCIE_RST# affects all PCIe slots on all boards except + * apu2. ResetID 1 does not need any GPIO. + */ + case 1: + Status = AGESA_SUCCESS; + break; + case 51: /* GPIO51 resets mPCIe1 slot on apu2 */ + switch (ResetInfo->ResetControl) { + case AssertSlotReset: + GpioData = gpio1_read32(0x8); + printk(BIOS_DEBUG, "%s: ResetID %u assert %08x\n", + __func__, ResetInfo->ResetId, GpioData); + GpioValue = gpio1_read8(0xa); + GpioValue &= ~BIT6; + gpio1_write8(0xa, GpioValue); + Status = AGESA_SUCCESS; + break; + case DeassertSlotReset: + GpioData = gpio1_read32(0x8); + printk(BIOS_DEBUG, "%s: ResetID %u deassert %08x\n", + __func__, ResetInfo->ResetId, GpioData); + GpioValue = gpio1_read8(0xa); + GpioValue |= BIT6; + gpio1_write8(0xa, GpioValue); + Status = AGESA_SUCCESS; + break; + } + break; + case 55: /* GPIO51 resets mPCIe2 slot on apu2 */ + switch (ResetInfo->ResetControl) { + case AssertSlotReset: + GpioData = gpio1_read32(0xc); + printk(BIOS_DEBUG, "%s: ResetID %u assert %08x\n", + __func__, ResetInfo->ResetId, GpioData); + GpioValue = gpio1_read8(0xe); + GpioValue &= ~BIT6; + gpio1_write8(0xa, GpioValue); + Status = AGESA_SUCCESS; + break; + case DeassertSlotReset: + GpioData = gpio1_read32(0xc); + printk(BIOS_DEBUG, "%s: ResetID %u deassert %08x\n", + __func__, ResetInfo->ResetId, GpioData); + GpioValue = gpio1_read8(0xe); + GpioValue |= BIT6; + gpio1_write8(0xa, GpioValue); + Status = AGESA_SUCCESS; + break; + } + break; + } + + return Status; +} diff --git a/src/mainboard/pcengines/apu2/OemCustomize.c b/src/mainboard/pcengines/apu2/OemCustomize.c index 99b9d51..6339e0f 100644 --- a/src/mainboard/pcengines/apu2/OemCustomize.c +++ b/src/mainboard/pcengines/apu2/OemCustomize.c @@ -15,6 +15,15 @@ #include <AGESA.h> #include <northbridge/amd/agesa/state_machine.h>
+#define PCIE_NIC_RESET_ID 1 + +#if CONFIG(BOARD_PCENGINES_APU2) +#define PCIE_GFX_RESET_ID 55 +#define PCIE_PORT3_RESET_ID 51 +#else +#define PCIE_GFX_RESET_ID PCIE_NIC_RESET_ID +#define PCIE_PORT3_RESET_ID PCIE_NIC_RESET_ID +#endif
static const PCIe_PORT_DESCRIPTOR PortList[] = { { @@ -24,7 +33,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x01, 0) + AspmDisabled, PCIE_PORT3_RESET_ID, 0) }, /* Initialize Port descriptor (PCIe port, Lanes 1, PCI Device Number 2, ...) */ { @@ -34,7 +43,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x02, 0) + AspmDisabled, PCIE_NIC_RESET_ID, 0) }, /* Initialize Port descriptor (PCIe port, Lanes 2, PCI Device Number 2, ...) */ { @@ -44,7 +53,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x03, 0) + AspmDisabled, PCIE_NIC_RESET_ID, 0) }, /* Initialize Port descriptor (PCIe port, Lanes 3, PCI Device Number 2, ...) */ { @@ -54,7 +63,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x04, 0) + AspmDisabled, PCIE_NIC_RESET_ID, 0) }, /* Initialize Port descriptor (PCIe port, Lanes 4-7, PCI Device Number 4, ...) */ { @@ -64,7 +73,7 @@ HotplugDisabled, PcieGenMaxSupported, PcieGenMaxSupported, - AspmDisabled, 0x05, 0) + AspmDisabled, PCIE_GFX_RESET_ID, 0) } };
diff --git a/src/mainboard/pcengines/apu2/romstage.c b/src/mainboard/pcengines/apu2/romstage.c index 7565bfc..34c42b4 100644 --- a/src/mainboard/pcengines/apu2/romstage.c +++ b/src/mainboard/pcengines/apu2/romstage.c @@ -38,6 +38,15 @@
/* Release GPIO32/33 for other uses. */ pm_write8(0xea, 1); + + /* + * Assert resets on the PCIe slots, since AGESA calls deassert callout + * only. Only apu2 uses GPIOs to reset PCIe slots. + */ + if (CONFIG(BOARD_PCENGINES_APU2)) { + gpio1_write8(0xa, gpio1_read8(0xa) & ~(1 << 6)); + gpio1_write8(0xe, gpio1_read8(0xe) & ~(1 << 6)); + } }
static void early_lpc_init(void)
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 5/0/5 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1676 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1675 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1674 Non-emulation targets: HP_COMPAQ_8200_ELITE_SFF_PC using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1678 HP_COMPAQ_8200_ELITE_SFF_PC using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1677
Please note: This test is under development and might not be accurate at all!
Michał Żygowski has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/39703 )
Change subject: mb/pcengines/apu2: add reset logic for PCIe slots ......................................................................
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
build bot (Jenkins) (Code Review)
-
Michał Żygowski (Code Review)
-
Paul Menzel (Code Review)