[coreboot-gerrit] Change in ...coreboot[master]: device/pci_rom: Fix redundant pci_rom_probe() calls

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33909 )

Change subject: device/pci_rom: Fix redundant pci_rom_probe() calls
......................................................................

device/pci_rom: Fix redundant pci_rom_probe() calls

For the PCI_VENDOR_ID_ATI case, we can rely on
pci_rom_acpi_fill_vfct() to make the call if necessary.

For hardware other than ATI, pci_rom_probe() was already
called from pci_rom_ssdt() and pci_dev_init(), so
PCI_ROM_ADDRESS BAR is already enabled, if requested so.

Change-Id: I0ea893a9ac7ba480840ebf5570d8fe0d9e20938f
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33909
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Rudolph 
Reviewed-by: Mike Banon 
---
M src/arch/x86/acpi.c
M src/device/pci_rom.c
2 files changed, 13 insertions(+), 14 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved
  Mike Banon: Looks good to me, approved

diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c
index 80923e3..8ab993e 100644
--- a/src/arch/x86/acpi.c
+++ b/src/arch/x86/acpi.c
@@ -763,11 +763,14 @@
    memcpy(header->asl_compiler_id, ASLC, 4);

header->asl_compiler_revision = asl_revision;
-	header->length = sizeof(struct acpi_vfct);
    header->revision = get_acpi_table_revision(VFCT);

current = acpi_fill_vfct(device, vfct, current);

+	/* If no BIOS image, return with header->length == 0. */
+	if (!vfct->VBIOSImageOffset)
+		return;
+
    /* (Re)calculate length and checksum. */
    header->length = current - (unsigned long)vfct;
    header->checksum = acpi_checksum((void *)vfct, header->length);
diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c
index 7322e57..31fe115 100644
--- a/src/device/pci_rom.c
+++ b/src/device/pci_rom.c
@@ -209,8 +209,6 @@
    struct acpi_vfct_image_hdr *header = &vfct_struct->image_hdr;
    struct rom_header *rom;

-	vfct_struct->VBIOSImageOffset = (size_t)header - (size_t)vfct_struct;
-
    rom = check_initialized(device);
    if (!rom)
    	rom = pci_rom_probe(device);
@@ -233,6 +231,8 @@
    header->ImageLength = rom->size * 512;
    memcpy((void *)&header->VbiosContent, rom, header->ImageLength);

+	vfct_struct->VBIOSImageOffset = (size_t)header - (size_t)vfct_struct;
+
    current += header->ImageLength;
    return current;
 }
@@ -241,9 +241,6 @@
 pci_rom_write_acpi_tables(struct device *device, unsigned long current,
    		  struct acpi_rsdp *rsdp)
 {
-	struct acpi_vfct *vfct;
-	struct rom_header *rom;
-
    /* Only handle VGA devices */
    if ((device->class >> 8) != PCI_CLASS_DISPLAY_VGA)
    	return current;
@@ -252,19 +249,18 @@
    if (!device->enabled)
    	return current;

-	/* Probe for option rom */
-	rom = pci_rom_probe(device);
-	if (!rom)
-		return current;
-
    /* AMD/ATI uses VFCT */
    if (device->vendor == PCI_VENDOR_ID_ATI) {
+		struct acpi_vfct *vfct;
+
    	current = ALIGN_UP(current, 8);
-		printk(BIOS_DEBUG, "ACPI:    * VFCT at %lx\n", current);
    	vfct = (struct acpi_vfct *)current;
    	acpi_create_vfct(device, vfct, pci_rom_acpi_fill_vfct);
-		current += vfct->header.length;
-		acpi_add_table(rsdp, vfct);
+		if (vfct->header.length) {
+			printk(BIOS_DEBUG, "ACPI:    * VFCT at %lx\n", current);
+			current += vfct->header.length;
+			acpi_add_table(rsdp, vfct);
+		}
    }

return current;