sravan/system76-coreboot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

device: drop unnecessary CHECK_REV_IN_OPROM_NAME option

Browse Source 
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>
This commit is contained in:
Felix Held
2024-05-22 00:42:13 +02:00
parent 47eed41dcb
commit bfc92cb944
7 changed files with 21 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
src/device/Kconfig
View File

@@ -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

24
src/device/pci_rom.c
View File

@@ -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",

1
src/include/device/pci_rom.h
View File

@@ -55,7 +55,6 @@ pci_rom_write_acpi_tables(const struct device *device,
void pci_rom_ssdt(const struct device *device);
void map_oprom_vendev_rev(u32 *vendev, u8 *rev);
u32 map_oprom_vendev(u32 vendev);
int verified_boot_should_run_oprom(struct rom_header *rom_header);

12
src/mainboard/amd/bilby/Kconfig
View File

@@ -80,18 +80,6 @@ config BILBY_LPC
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)

13
src/mainboard/amd/mandolin/Kconfig
View File

@@ -106,19 +106,6 @@ config MANDOLIN_LPC
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)

15
src/soc/amd/picasso/Kconfig
View File

@@ -230,10 +230,10 @@ config MAX_CPUS
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.
result of the map_oprom_vendev() function in graphics.c.
config VGA_BIOS_FILE
string
@@ -244,25 +244,18 @@ config VGA_BIOS_SECOND
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

13
src/soc/amd/picasso/graphics.c
View File

@@ -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;
}