diff options
author | Felix Held <felix-coreboot@felixheld.de> | 2024-05-22 00:42:13 +0200 |
---|---|---|
committer | Felix Held <felix-coreboot@felixheld.de> | 2024-05-23 21:54:26 +0000 |
commit | bfc92cb94414b3a51490fefa203180914da06fbb (patch) | |
tree | e8c7a905a093a05014344b72e8552f7399ddc3f1 /src/device | |
parent | 47eed41dcb2170b84c96c68c28208b5a97124503 (diff) |
device: drop unnecessary CHECK_REV_IN_OPROM_NAME option
The CHECK_REV_IN_OPROM_NAME Kconfig option was introduced to solve the
problem of the PCI VID/DID combination of the Picasso iGPU not being
sufficient information to know which VGA BIOS file to run, so a new
function that additionally checks the PCI revision of that device was
introduced. Later it turned out that there might be a case where even
that isn't sufficient, so the soc_is_raven2() function is used in the
remap function to always use the correct VBIOS file.
Picasso is the only SoC that selected the CHECK_REV_IN_OPROM_NAME
Kconfig option, so all other SoCs are unaffected by this change.
Now that we use the VBIOS images with only the PCI VID and DID in the
CBFS file name for Picasso, SeaBIOS 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. The only board where this patch
might cause a regression compared to the old behavior is the AMD Cereme
reference board with Pollock APU, but I'm not even sure if any coreboot
developer still has one of those boards, so I'm willing to accept that.
To properly solve the problem with SeaBIOS using the correct VBIOS file
in all cases, we'd need to generate that info during coreboot runtime
and somehow pass it to SeaBIOS, but that's out of scope for this patch.
TEST=On Mandolin with PCO silicon, the display output in both SeaBIOS
and Ubuntu still works. Booting Windows 10 via the pre-built EDK2
payload that I'm using also resulted in the display output working.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Ia6de533c536044698d85404427719b8f534870fa
Reviewed-on: https://review.coreboot.org/c/coreboot/+/82598
Reviewed-by: Matt DeVillier <matt.devillier@amd.corp-partner.google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Diffstat (limited to 'src/device')
-rw-r--r-- | src/device/Kconfig | 24 | ||||
-rw-r--r-- | src/device/pci_rom.c | 24 |
2 files changed, 11 insertions, 37 deletions
diff --git a/src/device/Kconfig b/src/device/Kconfig index 404c73d250..0fe0a0923f 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -846,18 +846,18 @@ config VGA_BIOS_ID depends on VGA_BIOS default "1106,3230" help - The comma-separated PCI vendor and device ID with optional revision if that - feature is enabled that would associate your vBIOS to your video card. + The comma-separated PCI vendor and device ID that would associate + your vBIOS to your video card. - Example: 1106,3230 or 1106,3230,a3 + Example: 1106,3230 In the above example 1106 is the PCI vendor ID (in hex, but without the "0x" prefix) and 3230 specifies the PCI device ID of the - video card (also in hex, without "0x" prefix). a3 specifies the revision. + video card (also in hex, without "0x" prefix). This ID needs to match the PCI VID and DID in the VGA BIOS file's header and also needs to match the value returned by map_oprom_vendev - or map_oprom_vendev_rev if the remapping feature is used. + if the remapping feature is used. Under GNU/Linux you can run `lspci -nn` to list the IDs of your PCI devices. @@ -879,23 +879,17 @@ config VGA_BIOS_SECOND_ID string "Graphics device PCI IDs" depends on VGA_BIOS_SECOND help - The comma-separated PCI vendor and device ID with optional revision if that - feature is enabled that would associate your vBIOS to your video card. + The comma-separated PCI vendor and device ID that would associate + your vBIOS to your video card. - Example: 1106,3230 or 1106,3230,a3 + Example: 1106,3230 In the above example 1106 is the PCI vendor ID (in hex, but without the "0x" prefix) and 3230 specifies the PCI device ID of the - video card (also in hex, without "0x" prefix). a3 specifies the revision. + video card (also in hex, without "0x" prefix). 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 aca55d6774..b9210b0d65 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 @@ static void *cbfs_boot_map_optionrom(uint16_t vendor, uint16_t device) 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", |