soc/amd/cezanne: Turn off gpp clock request for disabled devices

The current behavior does not actually check if a device is present
before enabling the corresponding gpp_clkx_clock_request_mapping bits
which may cause issues with L1SS. This change sets the corresponding
gpp_clkx_clock_request_mapping to off if the corresponding device is
disabled.

BUG=b:202252869
TEST=Checked that value of GPP_CLK_CNTRL matched the expected value
when devices are enabled/disabled, checked that physically removing a
device that is marked as enabled also disables the corresponding clk req
BRANCH=guybrush

Signed-off-by: Robert Zieba <robertzieba@google.com>
Change-Id: I77389372c60bdec572622a3b49484d4789fd4e4c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61259
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

4 files changed, 106 insertions, 8 deletions

diff --git a/src/mainboard/amd/majolica/Makefile.inc b/src/mainboard/amd/majolica/Makefile.inc
index 23e9dd1b5e..59bc3cf0dd 100644
--- a/src/mainboard/amd/majolica/Makefile.inc
+++ b/src/mainboard/amd/majolica/Makefile.inc
@@ -6,6 +6,7 @@ bootblock-y += early_gpio.c
 romstage-y += port_descriptors.c
 
 ramstage-y += chromeos.c
+ramstage-y += port_descriptors.c
 
 APCB_SOURCES = $(MAINBOARD_BLOBS_DIR)/APCB_CZN_D4.bin
 APCB_SOURCES_RECOVERY = $(MAINBOARD_BLOBS_DIR)/APCB_CZN_D4_DefaultRecovery.bin
diff --git a/src/mainboard/google/guybrush/Makefile.inc b/src/mainboard/google/guybrush/Makefile.inc
index cdfb53bd63..1158dcfaca 100644
--- a/src/mainboard/google/guybrush/Makefile.inc
+++ b/src/mainboard/google/guybrush/Makefile.inc
@@ -8,6 +8,7 @@ romstage-y += romstage.c
 
 ramstage-y += mainboard.c
 ramstage-y += ec.c
+ramstage-y += port_descriptors.c
 ramstage-$(CONFIG_CHROMEOS) += chromeos.c
 
 all-y += spi_speeds.c
diff --git a/src/soc/amd/cezanne/chip.h b/src/soc/amd/cezanne/chip.h
index 1e59153ef5..b01c6c7fa8 100644
--- a/src/soc/amd/cezanne/chip.h
+++ b/src/soc/amd/cezanne/chip.h
@@ -12,6 +12,12 @@
 #include <types.h>
 #include <vendorcode/amd/fsp/cezanne/FspUsb.h>
 
+enum gpp_clk_req {
+	GPP_CLK_ON,  /* GPP clock always on; default */
+	GPP_CLK_REQ, /* GPP clock controlled by corresponding #CLK_REQx pin */
+	GPP_CLK_OFF, /* GPP clk off */
+};
+
 struct soc_amd_cezanne_config {
 	struct soc_amd_common_config common_config;
 	u8 i2c_scl_reset;
@@ -91,11 +97,7 @@ struct soc_amd_cezanne_config {
 
 	/* The array index is the general purpose PCIe clock output number. Values in here
 	   aren't the values written to the register to have the default to be always on. */
-	enum {
-		GPP_CLK_ON,	/* GPP clock always on; default */
-		GPP_CLK_REQ,	/* GPP clock controlled by corresponding #CLK_REQx pin */
-		GPP_CLK_OFF,	/* GPP clk off */
-	} gpp_clk_config[GPP_CLK_OUTPUT_COUNT];
+	enum gpp_clk_req gpp_clk_config[GPP_CLK_OUTPUT_COUNT];
 
 	/* performance policy for the PCIe links: power consumption vs. link speed */
 	enum {
diff --git a/src/soc/amd/cezanne/fch.c b/src/soc/amd/cezanne/fch.c
index 2c57f08d96..b791037e0d 100644
--- a/src/soc/amd/cezanne/fch.c
+++ b/src/soc/amd/cezanne/fch.c
@@ -4,14 +4,19 @@
 #include <amdblocks/acpimmio.h>
 #include <amdblocks/amd_pci_util.h>
 #include <amdblocks/gpio.h>
+#include <amdblocks/i2c.h>
 #include <amdblocks/smi.h>
 #include <assert.h>
 #include <bootstate.h>
 #include <cpu/x86/smm.h>
-#include <amdblocks/i2c.h>
+#include <device/device.h>
+#include <device/pci.h>
+#include <device/pci_ids.h>
+#include <fw_config.h>
 #include <soc/amd_pci_int_defs.h>
 #include <soc/iomap.h>
 #include <soc/i2c.h>
+#include <soc/platform_descriptors.h>
 #include <soc/smi.h>
 #include <soc/southbridge.h>
 #include "chip.h"
@@ -129,10 +134,97 @@ static void fch_init_resets(void)
 	pm_write16(PWR_RESET_CFG, pm_read16(PWR_RESET_CFG) | TOGGLE_ALL_PWR_GOOD);
 }
 
-/* configure the general purpose PCIe clock outputs according to the devicetree settings */
+/* Update gpp glk req config based on DXIO descriptors and enabled devices. */
+static void gpp_dxio_update_clk_req_config(enum gpp_clk_req *gpp_clk_config,
+					   size_t gpp_clk_config_num)
+{
+	const fsp_dxio_descriptor *dxio_descs = NULL;
+	const fsp_ddi_descriptor *ddi_descs = NULL;
+	size_t dxio_num = 0;
+	size_t ddi_num = 0;
+
+	mainboard_get_dxio_ddi_descriptors(&dxio_descs, &dxio_num, &ddi_descs, &ddi_num);
+	if (dxio_descs == NULL) {
+		printk(BIOS_WARNING,
+			"No DXIO descriptors found, GPP clk req may not reflect enabled devices\n");
+		return;
+	}
+
+	for (int i = 0; i < dxio_num; i++) {
+		const fsp_dxio_descriptor *dxio_desc = &dxio_descs[i];
+
+		/*  Only consider PCIe and unused engine types. */
+		if (dxio_desc->engine_type != PCIE_ENGINE
+			&& dxio_desc->engine_type != UNUSED_ENGINE)
+			continue;
+		enum cpm_clk_req dxio_clk_req = dxio_desc->clk_req;
+
+		/* CLK_DISABLE means there's no corresponding clk req line in use */
+		if (dxio_clk_req == CLK_DISABLE)
+			continue;
+
+		/*
+		 * dxio_clk_req is only 4 bits so having CLK_ENABLE as a value for
+		 * a descriptor should cause a compiler error. 0xF isn't a
+		 * valid clk_req value according to AMD's internal code either.
+		 * This is here to draw attention in case this code is ever used
+		 * in a situation where this has changed.
+		 */
+		if (dxio_clk_req == (CLK_ENABLE & 0xF)) {
+			printk(BIOS_WARNING,
+				"CLK_ENABLE is an invalid clk_req value for PCIe device %d.%d, DXIO descriptor %d\n",
+				dxio_desc->device_number, dxio_desc->function_number, i);
+			continue;
+		}
+
+		/* cpm_clk_req 0 is CLK_DISABLE */
+		int gpp_req_index = dxio_clk_req - CLK_REQ0;
+		/* Ensure that our index is valid */
+		if (gpp_req_index < 0 || gpp_req_index >= gpp_clk_config_num) {
+			printk(BIOS_ERR, "Failed to convert DXIO clk req value %d to GPP clk req index for PCIe device %d.%d, DXIO descriptor %d, clk req settings may be incorrect\n",
+				dxio_clk_req, dxio_desc->device_number,
+				dxio_desc->function_number, i);
+			continue;
+		}
+
+		const struct device *pci_device = pcidev_path_on_root(
+			PCI_DEVFN(dxio_desc->device_number, dxio_desc->function_number));
+		if (pci_device == NULL) {
+			gpp_clk_config[gpp_req_index] = GPP_CLK_OFF;
+			printk(BIOS_WARNING,
+				"Cannot find PCIe device %d.%d, disabling GPP clk req %d, DXIO descriptor %d\n",
+				dxio_desc->device_number, dxio_desc->function_number, i,
+				gpp_req_index);
+			continue;
+		}
+
+		/* PCIe devices haven't been fully set up yet, so directly read the vendor id
+		 * and device id to determine if a device is physically present. If a device
+		 * is not present then the id should be 0xffffffff. 0x00000000, 0xffff0000,
+		 * and 0x0000ffff are there to account for any odd failure cases. */
+		u32 id = pci_read_config32(pci_device, PCI_VENDOR_ID);
+		bool enabled = pci_device->enabled && (id != 0xffffffff) &&
+			(id != 0x00000000) && (id != 0x0000ffff) && (id != 0xffff0000);
+
+		/* Inform of possible mismatches between devices and SoC gpp_clk_config. */
+		if (!enabled && gpp_clk_config[gpp_req_index] != GPP_CLK_OFF) {
+			gpp_clk_config[gpp_req_index] = GPP_CLK_OFF;
+			printk(BIOS_INFO,
+				"PCIe device %d.%d disabled, disabling GPP clk req %d, DXIO descriptor %d\n",
+				dxio_desc->device_number, dxio_desc->function_number,
+				gpp_req_index, i);
+		} else if (enabled && gpp_clk_config[gpp_req_index] == GPP_CLK_OFF) {
+			printk(BIOS_INFO,
+				"PCIe device %d.%d enabled, GPP clk req is off, DXIO descriptor %d\n",
+				dxio_desc->device_number, dxio_desc->function_number, i);
+		}
+	}
+}
+
+/* Configure the general purpose PCIe clock outputs according to the devicetree settings */
 static void gpp_clk_setup(void)
 {
-	const struct soc_amd_cezanne_config *cfg = config_of_soc();
+	struct soc_amd_cezanne_config *cfg = config_of_soc();
 
 	/* look-up table to be able to iterate over the PCIe clock output settings */
 	const uint8_t gpp_clk_shift_lut[GPP_CLK_OUTPUT_COUNT] = {
@@ -147,8 +239,10 @@ static void gpp_clk_setup(void)
 
 	uint32_t gpp_clk_ctl = misc_read32(GPP_CLK_CNTRL);
 
+	gpp_dxio_update_clk_req_config(&cfg->gpp_clk_config[0], GPP_CLK_OUTPUT_COUNT);
 	for (int i = 0; i < GPP_CLK_OUTPUT_COUNT; i++) {
 		gpp_clk_ctl &= ~GPP_CLK_REQ_MASK(gpp_clk_shift_lut[i]);
+
 		/*
 		 * The remapping of values is done so that the default of the enum used for the
 		 * devicetree settings is the clock being enabled, so that a missing devicetree