[coreboot-gerrit] Change in coreboot[master]: soc/intel/tigerlake: Configure L1Substates for PCH Root ports

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39412 )

Change subject: soc/intel/tigerlake: Configure L1Substates for PCH Root ports
......................................................................

soc/intel/tigerlake: Configure L1Substates for PCH Root ports

Set value for PcieRpL1Substates according to devicetree.

Chip config parameter PcieRpL1Substates uses (UPD value + 1)
because UPD value of 0 for PcieRpL1Substates means disabled for FSP.
In order to ensure that mainboard setting does not disable L1 substates
incorrectly, chip config parameter values are offset by 1 with 0 meaning
use FSP UPD default.

get_l1_substate_control() ensures that the right UPD value is set in
fsp_params.

Chip config parameter values
0: Use FSP UPD default
1: Disable L1 substates
2: Use L1.1
3: Use L1.2 (FSP UPD default)

BUG=none
BRANCH=none
TEST=Boot up and check FSP log for PCIe config for this values

Signed-off-by: Wonkyu Kim 
Change-Id: I66743a29ad182bd49b501ae73b79270a9eb88450
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39412
Reviewed-by: Caveh Jalali 
Reviewed-by: Nick Vaccaro 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params_tgl.c
2 files changed, 32 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nick Vaccaro: Looks good to me, approved
  Caveh Jalali: Looks good to me, but someone else must approve
  Wonkyu Kim: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h
index d23148a..9fc70f8 100644
--- a/src/soc/intel/tigerlake/chip.h
+++ b/src/soc/intel/tigerlake/chip.h
@@ -122,6 +122,14 @@
     * clksrc. */
    uint8_t PcieClkSrcClkReq[CONFIG_MAX_PCIE_CLOCKS];

+	/* PCIe RP L1 substate */
+	enum L1_substates_control {
+		L1_SS_FSP_DEFAULT,
+		L1_SS_DISABLED,
+		L1_SS_L1_1,
+		L1_SS_L1_2,
+	} PcieRpL1Substates[CONFIG_MAX_ROOT_PORTS];
+
    /* SMBus */
    uint8_t SmbusEnable;

diff --git a/src/soc/intel/tigerlake/fsp_params_tgl.c b/src/soc/intel/tigerlake/fsp_params_tgl.c
index 14997c5..7230a4c 100644
--- a/src/soc/intel/tigerlake/fsp_params_tgl.c
+++ b/src/soc/intel/tigerlake/fsp_params_tgl.c
@@ -28,6 +28,25 @@
 #include <soc/soc_chip.h>
 #include <string.h>

+/*
+ * Chip config parameter PcieRpL1Substates uses (UPD value + 1)
+ * because UPD value of 0 for PcieRpL1Substates means disabled for FSP.
+ * In order to ensure that mainboard setting does not disable L1 substates
+ * incorrectly, chip config parameter values are offset by 1 with 0 meaning
+ * use FSP UPD default. get_l1_substate_control() ensures that the right UPD
+ * value is set in fsp_params.
+ * 0: Use FSP UPD default
+ * 1: Disable L1 substates
+ * 2: Use L1.1
+ * 3: Use L1.2 (FSP UPD default)
+ */
+static int get_l1_substate_control(enum L1_substates_control ctl)
+{
+	if ((ctl > L1_SS_L1_2) || (ctl == L1_SS_FSP_DEFAULT))
+		ctl = L1_SS_L1_2;
+	return ctl - 1;
+}
+
 static void parse_devicetree(FSP_S_CONFIG *params)
 {
    const struct soc_intel_tigerlake_config *config;
@@ -113,6 +132,11 @@
    	}
    }

+	/* RP Configs */
+	for (i = 0; i < CONFIG_MAX_ROOT_PORTS; i++)
+		params->PcieRpL1Substates[i] =
+			get_l1_substate_control(config->PcieRpL1Substates[i]);
+
    /* Enable xDCI controller if enabled in devicetree and allowed */
    dev = pcidev_on_root(PCH_DEV_SLOT_XHCI, 1);
    if (dev) {