[coreboot-gerrit] [S] Change in coreboot[master]: nb/intel/gm45/gma: Probe PCI resource once and first

[Felix Singer (Code Review)]

Felix Singer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75376 )

Change subject: nb/intel/gm45/gma: Probe PCI resource once and first
......................................................................

nb/intel/gm45/gma: Probe PCI resource once and first

The PCI resource should only be probed as part of the device
.init process. We can simply do that first and know that we
can use the global `gtt_res` from then on.

This simplifies the signature of gm45_get_lvds_edid_str(), and
makes changes to the API user (lenovo/x200) necessary.

Change-Id: I6c96f715abfa56dcb1cd89fde0fbaef3f1cb63ae
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/75376
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Singer 
---
M src/mainboard/lenovo/x200/blc.c
M src/northbridge/intel/gm45/gm45.h
M src/northbridge/intel/gm45/gma.c
3 files changed, 33 insertions(+), 11 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Felix Singer: Looks good to me, approved

diff --git a/src/mainboard/lenovo/x200/blc.c b/src/mainboard/lenovo/x200/blc.c
index 330f1bc..655be56 100644
--- a/src/mainboard/lenovo/x200/blc.c
+++ b/src/mainboard/lenovo/x200/blc.c
@@ -38,8 +38,7 @@

const char *mainboard_vbt_filename(void)
 {
-	struct device *gma = pcidev_path_on_root(PCI_DEVFN(0x2, 0));
-	u16 pwm_freq = gma ? get_blc_pwm_freq_value(gm45_get_lvds_edid_str(gma)) : 0;
+	u16 pwm_freq = get_blc_pwm_freq_value(gm45_get_lvds_edid_str());

if (pwm_freq == 0) {
    	printk(BIOS_DEBUG,
diff --git a/src/northbridge/intel/gm45/gm45.h b/src/northbridge/intel/gm45/gm45.h
index c85a16c..d30ebe3 100644
--- a/src/northbridge/intel/gm45/gm45.h
+++ b/src/northbridge/intel/gm45/gm45.h
@@ -449,7 +449,7 @@
 #include <device/device.h>
 #include <edid.h>

-const char *gm45_get_lvds_edid_str(struct device *dev);
+const char *gm45_get_lvds_edid_str(void);

struct acpi_rsdp;
 unsigned long northbridge_write_acpi_tables(const struct device *device, unsigned long start,
diff --git a/src/northbridge/intel/gm45/gma.c b/src/northbridge/intel/gm45/gma.c
index 6759e74..ef7b872 100644
--- a/src/northbridge/intel/gm45/gma.c
+++ b/src/northbridge/intel/gm45/gma.c
@@ -143,7 +143,7 @@
    							reg8));
 }

-const char *gm45_get_lvds_edid_str(struct device *dev)
+const char *gm45_get_lvds_edid_str(void)
 {
    u8 *mmio;
    u8 edid_data_lvds[128];
@@ -152,10 +152,10 @@

if (edid_str[0])
    	return edid_str;
-	if (!gtt_res)
-		gtt_res = probe_resource(dev, PCI_BASE_ADDRESS_0);
-	if (!gtt_res)
+	if (!gtt_res) {
+		printk(BIOS_ERR, "Never call %s() outside dev.init() context.\n", __func__);
    	return NULL;
+	}
    mmio = res2mmio(gtt_res, 0, 0);

printk(BIOS_DEBUG, "LVDS EDID\n");
@@ -176,15 +176,18 @@
    const struct northbridge_intel_gm45_config *const conf = dev->chip_info;
    const char *edid_str;

+	/* Probe MMIO resource first. It's needed even for
+	   intel_gma_init_igd_opregion() which may call back. */
+	gtt_res = probe_resource(dev, PCI_BASE_ADDRESS_0);
+	if (!gtt_res)
+		return;
+
    intel_gma_init_igd_opregion();

-	edid_str = gm45_get_lvds_edid_str(dev);
+	edid_str = gm45_get_lvds_edid_str();
    if (!edid_str)
    	printk(BIOS_ERR, "Failed to obtain LVDS EDID string!\n");

-	/* gtt_res should have been inited in gm45_get_lvds_edid_str() */
-	if (!gtt_res)
-		return;
    mmio = res2mmio(gtt_res, 0, 0);

/*