[M] Change in coreboot[main]: [UNTESTED, WIP] device: drop unnecessary CHECK_REV_IN_OPROM_NAME option
Attention is currently required from: Fred Reitberger, Jason Glenesk, Matt DeVillier.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82598?usp=email )
Change subject: [UNTESTED, WIP] device: drop unnecessary CHECK_REV_IN_OPROM_NAME option ......................................................................
[UNTESTED, WIP] device: drop unnecessary CHECK_REV_IN_OPROM_NAME option
The CHECK_REV_IN_OPROM_NAME Kconfig option was once introduced to solve the problem of the PCI VID/DID combination of the iGPU not being sufficient information to know which VGA BIOS file to run, so a new function that also check the PCI revision of that device was introduces. Later it turned out that there might be a case where even that isn't sufficient, so the soc_is_raven2() function was used in the remap function to always use the correct VBIOS file.
Now that we use the VBIOS images with only the PCI VID and DID in the CBFS file name, SaeaBIOS will find the VBIOS with the same ID as the iGPU in CBFS and we don't need the workaround to add a third VBIOS image via VGA_BIOS_DGPU_* that has the name that SeaBIOS expects. This will result in SeaBIOS now running the VBIOS that has the same PCI VID/DID as the hardware which will be the wrong one in the RV2 silicon showing the PCO silicon PCI VID/DID, but that was also the case with the VGA_BIOS_DGPU_* workaround where the board's Kconfig just selected one of the two possible images during build time and hoped that it was the correct one for that actual hardware.
Signed-off-by: Felix Held felix-coreboot@felixheld.de Change-Id: Ia6de533c536044698d85404427719b8f534870fa --- M src/device/Kconfig M src/device/pci_rom.c M src/mainboard/amd/bilby/Kconfig M src/mainboard/amd/mandolin/Kconfig M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/graphics.c 6 files changed, 11 insertions(+), 70 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/82598/1
diff --git a/src/device/Kconfig b/src/device/Kconfig index 404c73d..f78f3fd 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -890,12 +890,6 @@
Under GNU/Linux you can run `lspci -nn` to list the IDs of your PCI devices.
-config CHECK_REV_IN_OPROM_NAME - def_bool n - help - Select this in the platform BIOS or chipset if the option rom has a revision - that needs to be checked when searching CBFS. - config VGA_BIOS_DGPU bool "Add a discrete VGA BIOS image" depends on VGA_BIOS diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index aca55d6..b9210b0 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -12,7 +12,6 @@ #include <acpi/acpigen.h>
/* Rmodules don't like weak symbols. */ -void __weak map_oprom_vendev_rev(u32 *vendev, u8 *rev) { return; } u32 __weak map_oprom_vendev(u32 vendev) { return vendev; }
void vga_oprom_preload(void) @@ -39,35 +38,16 @@ return cbfs_map(name, NULL); }
-static void *cbfs_boot_map_optionrom_revision(uint16_t vendor, uint16_t device, uint8_t rev) -{ - char name[20] = "pciXXXX,XXXX,XX.rom"; - - snprintf(name, sizeof(name), "pci%04hx,%04hx,%02hhx.rom", vendor, device, rev); - - return cbfs_map(name, NULL); -} - struct rom_header *pci_rom_probe(const struct device *dev) { struct rom_header *rom_header = NULL; struct pci_data *rom_data; - u8 rev = pci_read_config8(dev, PCI_REVISION_ID); - u8 mapped_rev = rev; u32 vendev = (dev->vendor << 16) | dev->device; u32 mapped_vendev = vendev;
/* If the ROM is in flash, then don't check the PCI device for it. */ - if (CONFIG(CHECK_REV_IN_OPROM_NAME)) { - map_oprom_vendev_rev(&mapped_vendev, &mapped_rev); - rom_header = cbfs_boot_map_optionrom_revision(mapped_vendev >> 16, - mapped_vendev & 0xffff, - mapped_rev); - } else { - mapped_vendev = map_oprom_vendev(vendev); - rom_header = cbfs_boot_map_optionrom(mapped_vendev >> 16, - mapped_vendev & 0xffff); - } + mapped_vendev = map_oprom_vendev(vendev); + rom_header = cbfs_boot_map_optionrom(mapped_vendev >> 16, mapped_vendev & 0xffff);
if (rom_header) { printk(BIOS_DEBUG, "In CBFS, ROM address for %s = %p\n", diff --git a/src/mainboard/amd/bilby/Kconfig b/src/mainboard/amd/bilby/Kconfig index 19fcbe5..9573204 100644 --- a/src/mainboard/amd/bilby/Kconfig +++ b/src/mainboard/amd/bilby/Kconfig @@ -80,18 +80,6 @@ Picasso's LPC bus signals are MUXed with some of the EMMC signals. Select this option if LPC signals are required.
-#TODO: remove this hack to not break graphics in combination with SeaBIOS -config VGA_BIOS_DGPU_ID - string - default "1002,15d8" - help - The default VGA BIOS PCI vendor/device ID should be set to the - result of the map_oprom_vendev() function in northbridge.c. - -config VGA_BIOS_DGPU_FILE - string - default "3rdparty/amd_blobs/picasso/PicassoGenericVbios.bin" - if !EM100 # EM100 defaults in soc/amd/common/blocks/spi/Kconfig config EFS_SPI_READ_MODE default 3 # Quad IO (1-1-4) diff --git a/src/mainboard/amd/mandolin/Kconfig b/src/mainboard/amd/mandolin/Kconfig index 252404b..ce3af3b 100644 --- a/src/mainboard/amd/mandolin/Kconfig +++ b/src/mainboard/amd/mandolin/Kconfig @@ -106,19 +106,6 @@ Picasso's LPC bus signals are MUXed with some of the EMMC signals. Select this option if LPC signals are required.
-#TODO: remove this hack to not break graphics in combination with SeaBIOS -config VGA_BIOS_DGPU_ID - string - default "1002,15d8" - help - The default VGA BIOS PCI vendor/device ID should be set to the - result of the map_oprom_vendev() function in northbridge.c. - -config VGA_BIOS_DGPU_FILE - string - default "3rdparty/amd_blobs/picasso/PicassoGenericVbios.bin" if BOARD_AMD_MANDOLIN - default "3rdparty/amd_blobs/picasso/Raven2GenericVbios.bin" if BOARD_AMD_CEREME - if !EM100 # EM100 defaults in soc/amd/common/blocks/spi/Kconfig config EFS_SPI_READ_MODE default 3 # Quad IO (1-1-4) diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index d6c3fbd..9e70953 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -230,7 +230,7 @@
config VGA_BIOS_ID string - default "1002,15d8,c1" + default "1002,15d8" help The default VGA BIOS PCI vendor/device ID should be set to the result of the map_oprom_vendev_rev() function in graphics.c. @@ -244,25 +244,18 @@
config VGA_BIOS_SECOND_ID string - default "1002,15dd,c4" + default "1002,15dd" help Some Dali and all Pollock APUs need a different VBIOS than some other Dali and all Picasso APUs, but don't always have a different PCI vendor/device IDs, so we need an alternate method to determine the - correct video BIOS. In map_oprom_vendev_rev(), we look at the return + correct video BIOS. In map_oprom_vendev(), we look at the return value of soc_is_raven2() and decide which rom to load.
config VGA_BIOS_SECOND_FILE string default "3rdparty/amd_blobs/picasso/Raven2GenericVbios.bin"
-config CHECK_REV_IN_OPROM_NAME - bool - default y - help - Select this in the platform BIOS or chipset if the option rom has a - revision that needs to be checked when searching CBFS. - config S3_VGA_ROM_RUN bool default n diff --git a/src/soc/amd/picasso/graphics.c b/src/soc/amd/picasso/graphics.c index 3c2e0f8..bd7ec77 100644 --- a/src/soc/amd/picasso/graphics.c +++ b/src/soc/amd/picasso/graphics.c @@ -6,20 +6,19 @@ #include <soc/soc_util.h> #include <stdint.h>
-void map_oprom_vendev_rev(u32 *vendev, u8 *rev) +u32 map_oprom_vendev(u32 vendev) { - if (*vendev == PICASSO_VBIOS_VID_DID) { + if (vendev == PICASSO_VBIOS_VID_DID) { /* Check if the RV2 video bios needs to be used instead of the RV1/PCO one */ if (soc_is_raven2()) { printk(BIOS_NOTICE, "Using RV2 VBIOS.\n"); - *vendev = RAVEN2_VBIOS_VID_DID; - *rev = RAVEN2_VBIOS_REV; + return RAVEN2_VBIOS_VID_DID; } else { printk(BIOS_NOTICE, "Using RV1/PCO VBIOS.\n"); - *rev = PICASSO_VBIOS_REV; } - } else if (*vendev == RAVEN2_VBIOS_VID_DID) { + } else if (vendev == RAVEN2_VBIOS_VID_DID) { printk(BIOS_NOTICE, "Using RV2 VBIOS.\n"); - *rev = RAVEN2_VBIOS_REV; } + + return vendev; }
-
Felix Held (Code Review)