mb/prodrive/atlas: Configure PCIe CLKREQ

Intel Client PCIe* controller expects each device should drive the
SRCCLKREQ#. If the GPIO is set to native mode for a device, which does
not support SRCCLKREQ#, then during RTD3 exit link would not be
established. Because controller samples the SRCCLKREQ# before
detecting the device and break L1 as the system might enter L1SS as
controller detects SRCCLKREQ# as de-asserted.
As a workaround the Pins must not be configured in Native Mode (CLKREQ
native function). Therefore here they are not configured at all.
source: 689882 (intel document ID)
So apparently hardware doesn't sample SRCCLKREQ Pin if it's not
configured as such.

That workaround suggestion however also brought a patch to FSP, which
in turn causes the same bug (even if SRCLKREQ are not configured).
Usually in order to make use of root port power saving features (e.g.
clock gating), the Root port must either be disabled or a CLKREQ Pin
must be configured. The patch however removed that check before
enabling power management for the rootport.
Workaround (until FSP is fixed):
pretend to FSP that the rootports have a CLKREQ Pin attached, by
supplying them in the FSP UPDs. That will cause FSP to configure the
CLKREQ Pin and enable power management for said rootport, but it will
not crash on L1 entry/exit. That has been done on the Atlas board
(as workaround) for a short period of time (before applying FSP Fix)
like this:
      // RP 5 (the rootport you want to fix)
    - memupd->FspmConfig.PcieClkSrcUsage[2] = 4;
      // e.g. choose a clkreq pin that is not routed out
    - memupd->FspmConfig.PcieClkSrcClkReq[2] = 0;

Furthermore disable CpuPcieRpClockReqMsgEnable FSP-M options to prevent
the same issue, but for CPU root ports. If not done the following will
happen in coreboot:
[DEBUG]  PCI: 00:06.2 scanning...
[SPEW ]  do_pci_scan_bridge for PCI: 00:06.2
[DEBUG]  PCI: pci_scan_bus for bus 02
[DEBUG]  PCI: 02:00.0 [1344/5410] enabled
[INFO ]  PCIe: Common Clock Configuration already enabled
[INFO ]  PCIE CLK PM is not supported by endpoint
[INFO ]  ASPM: Enabled L1
[EMERG]  CPU Index 9 - APIC 32 Unexpected Exception:18 @ 10:76aeb93f - Halting
[EMERG]  Code: 0 eflags: 00000046 cr2: 00000000
[EMERG]  eax: 00000000 ebx: 00000009 ecx: 00000000 edx: 00000000
[EMERG]  edi: 00000009 esi: 76b218c4 ebp: 00000000 esp: 76b29100
[EMERG]  0x76aeb8f8:    c4 2c 5b 5e 5f 5d c3 56
[EMERG]  0x76aeb900:    53 83 ec 14 65 a1 00 00

Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>
Change-Id: If2acdc16f37cdae0292f55d210b058f82179bfb4
Reviewed-on: https://review.coreboot.org/c/coreboot/+/72392
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Lean Sheng Tan <sheng.tan@9elements.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>

5 files changed, 122 insertions, 50 deletions

diff --git a/src/mainboard/prodrive/atlas/Kconfig b/src/mainboard/prodrive/atlas/Kconfig
index e36f195b49..a779b3078e 100644
--- a/src/mainboard/prodrive/atlas/Kconfig
+++ b/src/mainboard/prodrive/atlas/Kconfig
@@ -50,6 +50,27 @@ config UART_FOR_CONSOLE
 config CBFS_SIZE
 	default 0x800000
 
+config PCIEXP_ASPM
+	bool
+	default n
+	help
+	  FSP is already taking care of ASPM, which is configured through the devicetree in coreboot
+	  on Alderlake Platforms. Disable it to save some boot time.
+
+config PCIEXP_L1_SUB_STATE
+	bool
+	default n
+	help
+	  Enabling PCIe L1 sub states is already done in FSP.
+	  Disable it to save some boot time.
+
+config PCIEXP_CLK_PM
+	bool
+	default n
+	help
+	  Enabling PCIe clock power management is already done in FSP.
+	  Disable it to save some boot time
+
 # This platform has limited means to display POST codes
 config NO_POST
 	default y
diff --git a/src/mainboard/prodrive/atlas/devicetree.cb b/src/mainboard/prodrive/atlas/devicetree.cb
index 86e86f5455..e9c818106f 100644
--- a/src/mainboard/prodrive/atlas/devicetree.cb
+++ b/src/mainboard/prodrive/atlas/devicetree.cb
@@ -71,62 +71,74 @@ chip soc/intel/alderlake
 		[PchSerialIoIndexUART2] = PchSerialIoDisabled,
 	}"
 
-	# Enable PCH PCIE RP 5, 6, 7, 8, 9 using free running CLK (0x80)
-	# Clock source is shared hence marked as free running.
-	register "pch_pcie_rp[PCH_RP(5)]" = "{
-		.flags = PCIE_RP_CLK_SRC_UNUSED,
-	}"
-	register "pch_pcie_rp[PCH_RP(6)]" = "{
-		.flags = PCIE_RP_CLK_SRC_UNUSED,
-	}"
-	register "pch_pcie_rp[PCH_RP(7)]" = "{
-		.flags = PCIE_RP_CLK_SRC_UNUSED,
-	}"
-	register "pch_pcie_rp[PCH_RP(8)]" = "{
-		.flags = PCIE_RP_CLK_SRC_UNUSED,
-	}"
-	register "pch_pcie_rp[PCH_RP(9)]" = "{
-		.flags = PCIE_RP_CLK_SRC_UNUSED,
-	}"
+	# Clock source 0 is shared between PCH RP 5, 6, 7, 8, 9 and CPU RP 1, 2, 3
+	# Clock source 0 is therefore marked as FREE_RUNNING
+	# Set PCIE_RP_CLK_SRC_UNUSED on the root ports using clock source 0 so that
+	# we don't get a warning at boot about a missing clock definition.
+	register "pcie_clk_config_flag[0]" = "PCIE_CLK_FREE_RUNNING"
+
+	register "pch_pcie_rp[PCH_RP(5)].flags" = "PCIE_RP_CLK_SRC_UNUSED | PCIE_RP_CLK_REQ_UNUSED | PCIE_RP_LTR | PCIE_RP_AER"
+	register "pch_pcie_rp[PCH_RP(6)].flags" = "PCIE_RP_CLK_SRC_UNUSED | PCIE_RP_CLK_REQ_UNUSED | PCIE_RP_LTR | PCIE_RP_AER"
+	register "pch_pcie_rp[PCH_RP(7)].flags" = "PCIE_RP_CLK_SRC_UNUSED | PCIE_RP_CLK_REQ_UNUSED | PCIE_RP_LTR | PCIE_RP_AER"
+	register "pch_pcie_rp[PCH_RP(8)].flags" = "PCIE_RP_CLK_SRC_UNUSED | PCIE_RP_CLK_REQ_UNUSED | PCIE_RP_LTR | PCIE_RP_AER"
+	register "pch_pcie_rp[PCH_RP(9)].flags" = "PCIE_RP_CLK_SRC_UNUSED | PCIE_RP_CLK_REQ_UNUSED | PCIE_RP_LTR | PCIE_RP_AER" # UFS or general purpose RP
+
+	register "pch_pcie_rp[PCH_RP(5)].pcie_rp_aspm" = "ASPM_DISABLE"
+	register "pch_pcie_rp[PCH_RP(6)].pcie_rp_aspm" = "ASPM_DISABLE"
+	register "pch_pcie_rp[PCH_RP(7)].pcie_rp_aspm" = "ASPM_DISABLE"
+	register "pch_pcie_rp[PCH_RP(8)].pcie_rp_aspm" = "ASPM_DISABLE"
+	register "pch_pcie_rp[PCH_RP(9)].pcie_rp_aspm" = "ASPM_DISABLE"
+
+	register "pch_pcie_rp[PCH_RP(5)].PcieRpL1Substates" = "L1_SS_DISABLED"
+	register "pch_pcie_rp[PCH_RP(6)].PcieRpL1Substates" = "L1_SS_DISABLED"
+	register "pch_pcie_rp[PCH_RP(7)].PcieRpL1Substates" = "L1_SS_DISABLED"
+	register "pch_pcie_rp[PCH_RP(8)].PcieRpL1Substates" = "L1_SS_DISABLED"
+	register "pch_pcie_rp[PCH_RP(9)].PcieRpL1Substates" = "L1_SS_DISABLED"
+
 	# Enable PCIe-to-i225 bridge using clk 1
+	#TODO set clk_req, once it's connected on atlas. clk_req now defaults to 0,
+	#     because using 0xFF (unused) would trigger a bug.
 	register "pch_pcie_rp[PCH_RP(10)]" = "{
 		.clk_src = 1,
 		.flags = PCIE_RP_LTR | PCIE_RP_AER | PCIE_RP_BUILT_IN,
-		.pcie_rp_aspm = ASPM_DISABLE,
-	}"
-	register "pcie_clk_config_flag[0]" = "PCIE_CLK_FREE_RUNNING"
-
-	# Enable CPU PCIE RP 1, 2, 3 using free running CLK (0x80)
-	# Clock source is shared hence marked as free running.
-	register "cpu_pcie_rp[CPU_RP(1)]" = "{
-		.flags = PCIE_RP_LTR | PCIE_RP_AER | PCIE_RP_CLK_SRC_UNUSED,
-	}"
-	register "cpu_pcie_rp[CPU_RP(2)]" = "{
-		.flags = PCIE_RP_LTR | PCIE_RP_AER | PCIE_RP_CLK_SRC_UNUSED,
-	}"
-	register "cpu_pcie_rp[CPU_RP(3)]" = "{
-		.flags = PCIE_RP_LTR | PCIE_RP_AER | PCIE_RP_CLK_SRC_UNUSED,
+		.pcie_rp_aspm = ASPM_AUTO,
 	}"
 
 	device domain 0 on
-		device ref pcie5_0 on end
+		device ref pcie5_0 on
+			register "cpu_pcie_rp[CPU_RP(2)].flags" = "PCIE_RP_LTR | PCIE_RP_AER | PCIE_RP_CLK_SRC_UNUSED"
+			register "cpu_pcie_rp[CPU_RP(2)].pcie_rp_aspm" = "ASPM_AUTO"
+			register "cpu_pcie_rp[CPU_RP(2)].PcieRpL1Substates" = "L1_SS_DISABLED"
+		end
 		device ref igpu on end
+		# without DDT enabled, edk2 doesn't even finish (TODO)
 		device ref dtt on end
-		device ref pcie4_0 on end
-		device ref pcie4_1 on end
-		device ref crashlog off end
+		device ref pcie4_0 on
+			register "cpu_pcie_rp[CPU_RP(1)].flags" = "PCIE_RP_LTR | PCIE_RP_AER | PCIE_RP_CLK_SRC_UNUSED"
+			register "cpu_pcie_rp[CPU_RP(1)].pcie_rp_aspm" = "ASPM_DISABLE"
+			register "cpu_pcie_rp[CPU_RP(1)].PcieRpL1Substates" = "L1_SS_DISABLED"
+		end
+		device ref pcie4_1 on
+			register "cpu_pcie_rp[CPU_RP(3)].flags" = "PCIE_RP_LTR | PCIE_RP_AER | PCIE_RP_CLK_SRC_UNUSED"
+			register "cpu_pcie_rp[CPU_RP(3)].pcie_rp_aspm" = "ASPM_DISABLE"
+			register "cpu_pcie_rp[CPU_RP(3)].PcieRpL1Substates" = "L1_SS_DISABLED"
+		end
+		# TODO try enabling crashlog
+		device ref crashlog on end
 		device ref ish on end
-		device ref ufs on end
+		device ref ufs off end
 		device ref tcss_xhci on end
 		device ref xhci on end
 		device ref heci1 on end
 		device ref sata on end
+		# pcie_rp[1-4] is used for USB
 		device ref pcie_rp5 on end
 		device ref pcie_rp6 on end
 		device ref pcie_rp7 on end
 		device ref pcie_rp8 on end
 		device ref pcie_rp9 on end
 		device ref pcie_rp10 on end
+		# pcie_rp[11-12] is used for SATA
 		device ref uart0 on end
 		device ref uart1 on end
 		device ref pch_espi on
diff --git a/src/mainboard/prodrive/atlas/early_gpio.c b/src/mainboard/prodrive/atlas/early_gpio.c
index 40457cfbc3..70f28961aa 100644
--- a/src/mainboard/prodrive/atlas/early_gpio.c
+++ b/src/mainboard/prodrive/atlas/early_gpio.c
@@ -28,6 +28,8 @@ static const struct pad_config early_gpio_table[] = {
 	PAD_CFG_NF(GPP_D17, NONE, DEEP, NF1),
 	/* UART1 TX */
 	PAD_CFG_NF(GPP_D18, NONE, DEEP, NF1),
+
+	PAD_CFG_GPO(GPP_E8, 1, PLTRST),     /* PERST_CB_RESET_N */
 };
 
 void configure_early_gpio_pads(void)
diff --git a/src/mainboard/prodrive/atlas/gpio.c b/src/mainboard/prodrive/atlas/gpio.c
index fd75900f73..bccb85a739 100644
--- a/src/mainboard/prodrive/atlas/gpio.c
+++ b/src/mainboard/prodrive/atlas/gpio.c
@@ -4,7 +4,35 @@
 
 #include "gpio.h"
 
-/* Pad configuration in ramstage */
+/*
+ * Pad configuration in ramstage
+ * Intel Client PCIe* controller expects each device should drive the SRCCLKREQ#.
+ * If the GPIO is set to native mode for a device, which do not support SRCCLKREQ#,
+ * then during RTD3 exit link would not be established. Because controller samples
+ * the SRCCLKREQ# before detecting the device and break L1 as the system might enter L1SS
+ * as controller detects SRCCLKREQ# as de-asserted.
+ * As a workaround the Pins must not be configured in Native Mode (CLKREQ mode).
+ * Therefore here they are not configured at all.
+ * source: 689882 (intel document ID)
+ *
+ * So apparently hardware doesn't sample SRCCLKREQ Pin if it's not configured as such.
+ * That workaround suggestion however also brought a patch to FSP, which in
+ * turn causes the same Bug (even if SRCLKREQ are not configured). Usually
+ * in order to make use of root port power saving features (e.g.
+ * clock gating), the Root port must either be disabled or a CLKREQ Pin
+ * must be configured. The patch however removed that check before
+ * enabling power management for the rootport.
+ * Workaround (until FSP is fixed):
+ *
+ * Workaround:
+ * pretend to FSP that the rootports have a CLKREQ Pin attached, by supplying them in the
+ * FSP UPDs. That will cause FSP to configure the Pin or CLKREQ and enable power
+ * management for said rootport, but it will not crash on L1 entry/exit. That has been done
+ * on the Atlas board (as workaround) for a short period of time (before applying FSP Fix)
+ * like this:
+ *      - memupd->FspmConfig.PcieClkSrcUsage[2] = 4; // RP 5 (the rootport you want to fix)
+ *      - memupd->FspmConfig.PcieClkSrcClkReq[2] = 0; // e.g. choose a clkreq pin that is not routed out
+ */
 static const struct pad_config gpio_table[] = {
 	/*
 	 * Do not program virtual wire CPU CLKREQ pins, since CLKREQ of cpu rootports are not
@@ -13,7 +41,8 @@ static const struct pad_config gpio_table[] = {
 
 	/* ------- GPIO Group GPP_A ------- */
 	PAD_NC(GPP_A7, NONE),
-	PAD_NC(GPP_A12, NONE),
+	PAD_CFG_GPI(GPP_A8, NONE, DEEP), // HSID_0 / CLKREQ 7
+	//PAD_NC(GPP_A12, NONE), CLKREQ 9B ???
 	PAD_CFG_NF(GPP_A14, NONE, DEEP, NF1), // USB_2_3_OC_N
 	PAD_CFG_NF(GPP_A15, NONE, DEEP, NF1), // USB_4_5_OC_N
 	PAD_CFG_NF(GPP_A16, NONE, DEEP, NF1), // USB_6_7_OC_N
@@ -46,15 +75,10 @@ static const struct pad_config gpio_table[] = {
 	PAD_NC(GPP_D1, NONE),
 	PAD_NC(GPP_D2, NONE),
 	PAD_NC(GPP_D3, NONE),
-	PAD_NC(GPP_D5, NONE),
-	/*
-	 * If this pin is configured by coreboot the pci device (i225) connected to this pin
-	 * will not show up in lspci tool in anymore. Configuring CLKREQ seems to be an issue
-	 * with current FSP-S. see also comment at top of file.
-	 */
-	//PAD_CFG_NF(GPP_D6, NONE, DEEP, NF1), // PHY0_CLKREQ
-	PAD_NC(GPP_D7, NONE),
-	PAD_NC(GPP_D8, NONE),
+	//PAD_NC(GPP_D5, NONE), CLKREQ 0
+	//PAD_CFG_NF(GPP_D6, NONE, DEEP, NF1), // PHY0_CLKREQ / CLKREQ 1
+	//PAD_NC(GPP_D7, NONE), CLKREQ 2
+	//PAD_NC(GPP_D8, NONE), CLKREQ 3
 	PAD_NC(GPP_D9, NONE),
 	PAD_NC(GPP_D10, NONE),
 	PAD_NC(GPP_D11, NONE),
@@ -66,7 +90,7 @@ static const struct pad_config gpio_table[] = {
 	PAD_NC(GPP_D19, NONE),
 
 	/* ------- GPIO Group GPP_E ------- */
-	PAD_NC(GPP_E0, NONE),
+	//PAD_NC(GPP_E0, NONE), CLKREQ 9
 	PAD_NC(GPP_E1, NONE),
 	PAD_NC(GPP_E2, NONE),
 	PAD_CFG_GPI(GPP_E3, NONE, DEEP),			/* TPM INT (todo: check) */
@@ -74,7 +98,7 @@ static const struct pad_config gpio_table[] = {
 	PAD_NC(GPP_E5, NONE),
 	PAD_CFG_GPI(GPP_E6, NONE, DEEP),
 	PAD_CFG_GPI_SMI_LOW(GPP_E7, NONE, DEEP, EDGE_SINGLE),	/* EC SMI# */
-	PAD_CFG_GPO(GPP_E8, 1, DEEP),				/* PERST_CB_RESET_N */
+	PAD_CFG_GPO(GPP_E8, 1, PLTRST),				/* PERST_CB_RESET_N */
 	PAD_CFG_NF(GPP_E9, NONE, DEEP, NF1),			/* USB_0_1_OC_N */
 	PAD_NC(GPP_E10, NONE),
 	PAD_NC(GPP_E11, NONE),
@@ -82,7 +106,7 @@ static const struct pad_config gpio_table[] = {
 	PAD_NC(GPP_E13, NONE),
 	PAD_CFG_NF(GPP_E14, NONE, DEEP, NF1),			/* DP0_HPD (VGA_RED) */
 	PAD_NC(GPP_E15, NONE),
-	PAD_NC(GPP_E16, NONE),
+	//PAD_NC(GPP_E16, NONE), CLKREQ 8
 	PAD_NC(GPP_E17, NONE),
 	PAD_CFG_NF(GPP_E18, NONE, DEEP, NF1),			/* DP3_DDC_CTRLCLK */
 	PAD_CFG_NF(GPP_E19, NONE, DEEP, NF1),			/* DP3_DDC_CTRLDATA */
@@ -103,6 +127,7 @@ static const struct pad_config gpio_table[] = {
 	PAD_CFG_GPI(GPP_F13, NONE, DEEP),
 	PAD_NC(GPP_F17, NONE),
 	PAD_NC(GPP_F18, NONE),
+	PAD_CFG_GPI(GPP_F19, NONE, DEEP), // HSID_1 / CLKREQ 6
 	PAD_NC(GPP_F20, NONE),
 	PAD_NC(GPP_F21, NONE),
 	PAD_CFG_GPO(GPP_F22, 1, DEEP),				/* PERST_PHY0_N */
@@ -116,6 +141,8 @@ static const struct pad_config gpio_table[] = {
 	PAD_NC(GPP_H13, NONE),
 	PAD_CFG_NF(GPP_H15, NONE, DEEP, NF1),			/* DDPB_CTRLCLK */
 	PAD_CFG_NF(GPP_H17, NONE, DEEP, NF1),			/* DDPB_CTRLDATA */
+	PAD_CFG_GPI(GPP_H19, NONE, DEEP), // HSID_3 / CLKREQ 4
+	PAD_CFG_GPI(GPP_H23, NONE, DEEP), // HSID_2 / CLKREQ 5
 
 	/* ------- GPIO Group GPP_R ------- */
 	PAD_CFG_NF(GPP_R0, NONE, DEEP, NF1), // HDA_BCLK
diff --git a/src/mainboard/prodrive/atlas/romstage_fsp_params.c b/src/mainboard/prodrive/atlas/romstage_fsp_params.c
index dc5403f868..5c523c5cdc 100644
--- a/src/mainboard/prodrive/atlas/romstage_fsp_params.c
+++ b/src/mainboard/prodrive/atlas/romstage_fsp_params.c
@@ -50,4 +50,14 @@ void mainboard_memory_init_params(FSPM_UPD *memupd)
 	memupd->FspmConfig.PchHdaAudioLinkHdaEnable = 1;
 	memupd->FspmConfig.PchHdaSdiEnable[0] = 1;
 	memupd->FspmConfig.PchHdaSdiEnable[1] = 1;
+
+	// CPU rootports do not have a ClockReq connected on Atlas. If this is not done,
+	// the following will happens:
+	//    - FSP will enable power management for cpu rootport.
+	//    - coreboot enables ASPM on CPU root port on pci enemuration
+	//    - machine exception is thrown, when trying to access pci configuration space after
+	//      enabling ASPM src/device/pciexp_device.c:pciexp_tune_dev().
+	memupd->FspmConfig.CpuPcieRpClockReqMsgEnable[0] = 0;
+	memupd->FspmConfig.CpuPcieRpClockReqMsgEnable[1] = 0;
+	memupd->FspmConfig.CpuPcieRpClockReqMsgEnable[2] = 0;
 }