[coreboot-gerrit] Change in ...coreboot[master]: src/device/pci: Add support for discrete VGA initialization and OpROM...

[mikeb mikeb (Code Review)]

Hello Mike Banon,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/31448

to review the following change.

Change subject: src/device/pci: Add support for discrete VGA initialization and OpROM loading
......................................................................

src/device/pci: Add support for discrete VGA initialization and OpROM loading

Required to make the discrete VGA working at some boards like AMD Lenovo G505S.
If CONFIG_MULTIPLE_VGA_ADAPTERS is enabled, coreboot will try to initialize the
discrete VGA and load OpROM on it. ACPI VFCT table will be created only for
discrete VGA instead of integrated, but everything seems to be working fine.
Maybe later we would find a way to make a common VFCT table for both adapters.

Based on the original patches by Hans Jürgen Kitter .

Signed-off-by: Mike Banon 
Signed-off-by: Hans Jürgen Kitter 
Change-Id: If3269a0911e86ea07014484365c7c830485e52ec
---
M src/device/pci_device.c
M src/device/pci_rom.c
2 files changed, 68 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/31448/1

diff --git a/src/device/pci_device.c b/src/device/pci_device.c
index 82033a6..96dec0b 100644
--- a/src/device/pci_device.c
+++ b/src/device/pci_device.c
@@ -764,7 +764,9 @@
     * ROMs when coming out of an S3 resume.
     */
    if (!IS_ENABLED(CONFIG_S3_VGA_ROM_RUN) && acpi_is_wakeup_s3() &&
-		((dev->class >> 8) == PCI_CLASS_DISPLAY_VGA))
+		(((dev->class >> 8) == PCI_CLASS_DISPLAY_VGA) ||
+			(IS_ENABLED(CONFIG_MULTIPLE_VGA_ADAPTERS) &&
+			((dev->class >> 8) == PCI_CLASS_DISPLAY_OTHER))))
    	return 0;
    if (IS_ENABLED(CONFIG_ALWAYS_LOAD_OPROM))
    	return 1;
@@ -778,25 +780,41 @@
 void pci_dev_init(struct device *dev)
 {
    struct rom_header *rom, *ram;
+	unsigned int pci_class = (dev->class >> 8);

if (!IS_ENABLED(CONFIG_VGA_ROM_RUN))
    	return;

-	/* Only execute VGA ROMs. */
-	if (((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA))
-		return;
+	/* Only load VGA ROMs. */
+	if (pci_class != PCI_CLASS_DISPLAY_VGA) {
+		if (!IS_ENABLED(CONFIG_MULTIPLE_VGA_ADAPTERS)) {
+			return;
+		} else if (pci_class != PCI_CLASS_DISPLAY_OTHER) {
+			return;
+		}
+	}

if (!should_load_oprom(dev))
    	return;
    timestamp_add_now(TS_OPROM_INITIALIZE);

rom = pci_rom_probe(dev);
-	if (rom == NULL)
+	if (rom == NULL) {
+		if (pci_class == PCI_CLASS_DISPLAY_VGA)
+			printk(BIOS_DEBUG, "Integrated VGA ROM probe failed\n");
+		else
+			printk(BIOS_DEBUG, "Discrete VGA ROM probe failed\n");
    	return;
+	}

ram = pci_rom_load(dev, rom);
-	if (ram == NULL)
+	if (ram == NULL) {
+		if (pci_class == PCI_CLASS_DISPLAY_VGA)
+			printk(BIOS_DEBUG, "Integrated VGA ROM load failed\n");
+		else
+			printk(BIOS_DEBUG, "Discrete VGA ROM load failed\n");
    	return;
+	}
    timestamp_add_now(TS_OPROM_COPY_END);

if (!should_run_oprom(dev))
@@ -804,7 +822,11 @@

run_bios(dev, (unsigned long)ram);
    gfx_set_init_done(1);
-	printk(BIOS_DEBUG, "VGA Option ROM was run\n");
+	if (pci_class == PCI_CLASS_DISPLAY_VGA)
+		printk(BIOS_DEBUG, "Integrated VGA Option ROM was run\n");
+	else
+		printk(BIOS_DEBUG, "Discrete VGA Option ROM was run\n");
+
    timestamp_add_now(TS_OPROM_END);
 }

diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c
index 82d9a30..60f9aed 100644
--- a/src/device/pci_rom.c
+++ b/src/device/pci_rom.c
@@ -179,13 +179,24 @@
 /* VBIOS may be modified after oprom init so use the copy if present. */
 static struct rom_header *check_initialized(struct device *dev)
 {
-	struct rom_header *run_rom;
+	struct rom_header *run_rom = NULL;
    struct pci_data *rom_data;

if (!IS_ENABLED(CONFIG_VGA_ROM_RUN))
    	return NULL;

-	run_rom = (struct rom_header *)(uintptr_t)PCI_VGA_RAM_IMAGE_START;
+	/* Set the start of shadow memory for this Integrated or Discrete VGA. */
+	if ((dev->class >> 8) == PCI_CLASS_DISPLAY_VGA) {
+		run_rom = (struct rom_header *)(uintptr_t)PCI_VGA_RAM_IMAGE_START;
+	} else if (IS_ENABLED(CONFIG_MULTIPLE_VGA_ADAPTERS)) {
+		if ((dev->class >> 8) == PCI_CLASS_DISPLAY_OTHER) {
+			run_rom = (struct rom_header *)(uintptr_t)PCI_RAM_IMAGE_START;
+		}
+	}
+
+	if (run_rom == NULL)
+		return NULL;
+
    if (read_le16(&run_rom->signature) != PCI_ROM_HDR)
    	return NULL;

@@ -217,11 +228,22 @@
    	return current;
    }

-	printk(BIOS_DEBUG, "           Copying %sVBIOS image from %p\n",
+	if (!IS_ENABLED(CONFIG_MULTIPLE_VGA_ADAPTERS)) {
+		/* Copy Integrated VGA VBIOS from CBFS to ACPI VFCT. */
+		printk(BIOS_DEBUG, "           Copying %sVBIOS image from %p\n",
    		rom == (struct rom_header *)
    				(uintptr_t)PCI_VGA_RAM_IMAGE_START ?
    		"initialized " : "",
    		rom);
+	} else {
+		/* Copy Discrete VGA VBIOS from CBFS to ACPI VFCT. */
+		printk(BIOS_DEBUG, "           Copying %sVBIOS image from %p\n",
+			rom == (struct rom_header *)
+					(uintptr_t)PCI_RAM_IMAGE_START ?
+			"initialized " : "",
+			rom);
+		/* TODO: do this also for Integrated VGA VBIOS. */
+	}

header->DeviceID = device->device;
    header->VendorID = device->vendor;
@@ -243,8 +265,20 @@
    struct rom_header *rom;

/* Only handle VGA devices */
-	if ((device->class >> 8) != PCI_CLASS_DISPLAY_VGA)
-		return current;
+
+	if (!IS_ENABLED(CONFIG_MULTIPLE_VGA_ADAPTERS)) {
+		if ((device->class >> 8) != PCI_CLASS_DISPLAY_VGA) {
+			return current;
+		}
+	} else {
+		/* Write ACPI VFCT only for Discrete VGA. */
+		if ((device->class >> 8) == PCI_CLASS_DISPLAY_OTHER) {
+			;
+		} else {
+			return current;
+		}
+		/* TODO: do this also for Integrated VGA. */
+	}

/* Only handle enabled devices */
    if (!device->enabled)