Hello Raul Rangel, Paul Fagerburg,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/39791
to review the following change.
Change subject: arch/x86: Add Kconfig option for 2nd VGA BIOS image ......................................................................
arch/x86: Add Kconfig option for 2nd VGA BIOS image
Picasso and Dali need different video bioses even though they use the same code in most other places.
The Kconfig symbol names are changed from the downstream commit to make them more consistent with current coreboot code.
BUG=b:145817712 TEST=Build Dali vBIOS into the coreboot image
Change-Id: Ide0d061fda0abc78a74ddf97ba81fc3cf2b02e4f Signed-off-by: Martin Roth martinroth@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1956534 Reviewed-by: Paul Fagerburg pfagerburg@chromium.org Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/arch/x86/Makefile.inc M src/device/Kconfig 2 files changed, 34 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/39791/1
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 8731612..b28ef78 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -34,6 +34,11 @@ pci$(stripped_vgabios_id).rom-file := $(call strip_quotes,$(CONFIG_VGA_BIOS_FILE)) pci$(stripped_vgabios_id).rom-type := optionrom
+stripped_second_vbios_id = $(call strip_quotes,$(CONFIG_VGA_BIOS_SECOND_ID)) +cbfs-files-$(CONFIG_VGA_BIOS_SECOND) += pci$(stripped_second_vbios_id).rom +pci$(stripped_second_vbios_id).rom-file := $(call strip_quotes,$(CONFIG_VGA_BIOS_SECOND_FILE)) +pci$(stripped_second_vbios_id).rom-type := optionrom + stripped_vgabios_dgpu_id = $(call strip_quotes,$(CONFIG_VGA_BIOS_DGPU_ID)) cbfs-files-$(CONFIG_VGA_BIOS_DGPU) += pci$(stripped_vgabios_dgpu_id).rom pci$(stripped_vgabios_dgpu_id).rom-file := $(call strip_quotes,$(CONFIG_VGA_BIOS_DGPU_FILE)) diff --git a/src/device/Kconfig b/src/device/Kconfig index 25123ea..66130cc 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -680,6 +680,35 @@
Under GNU/Linux you can run `lspci -nn` to list the IDs of your PCI devices.
+config VGA_BIOS_SECOND + bool "Add a 2nd video BIOS image" + depends on ARCH_X86 && VGA_BIOS + help + Select this option if you have a 2nd video BIOS image that you would + like to add to your ROM. + +config VGA_BIOS_SECOND_FILE + string "2nd video BIOS path and filename" + depends on VGA_BIOS_SECOND + default "vbios2.bin" + help + The path and filename of the file to use as video BIOS. + +config VGA_BIOS_SECOND_ID + string "Graphics device PCI IDs" + depends on VGA_BIOS_SECOND + help + The comma-separated PCI vendor and device ID that would associate + your vBIOS to your video card. + + 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). + + Under GNU/Linux you can run `lspci -nn` to list the IDs of your PCI devices. + config VGA_BIOS_DGPU bool "Add a discrete VGA BIOS image" depends on VGA_BIOS
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39791 )
Change subject: arch/x86: Add Kconfig option for 2nd VGA BIOS image ......................................................................
Patch Set 1: Code-Review+1
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39791 )
Change subject: arch/x86: Add Kconfig option for 2nd VGA BIOS image ......................................................................
Patch Set 1: Code-Review+2
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39791 )
Change subject: arch/x86: Add Kconfig option for 2nd VGA BIOS image ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39791 )
Change subject: arch/x86: Add Kconfig option for 2nd VGA BIOS image ......................................................................
arch/x86: Add Kconfig option for 2nd VGA BIOS image
Picasso and Dali need different video bioses even though they use the same code in most other places.
The Kconfig symbol names are changed from the downstream commit to make them more consistent with current coreboot code.
BUG=b:145817712 TEST=Build Dali vBIOS into the coreboot image
Change-Id: Ide0d061fda0abc78a74ddf97ba81fc3cf2b02e4f Signed-off-by: Martin Roth martinroth@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1956534 Reviewed-by: Paul Fagerburg pfagerburg@chromium.org Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/39791 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Raul Rangel rrangel@chromium.org --- M src/arch/x86/Makefile.inc M src/device/Kconfig 2 files changed, 34 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve Paul Fagerburg: Looks good to me, approved
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 8731612..b28ef78 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -34,6 +34,11 @@ pci$(stripped_vgabios_id).rom-file := $(call strip_quotes,$(CONFIG_VGA_BIOS_FILE)) pci$(stripped_vgabios_id).rom-type := optionrom
+stripped_second_vbios_id = $(call strip_quotes,$(CONFIG_VGA_BIOS_SECOND_ID)) +cbfs-files-$(CONFIG_VGA_BIOS_SECOND) += pci$(stripped_second_vbios_id).rom +pci$(stripped_second_vbios_id).rom-file := $(call strip_quotes,$(CONFIG_VGA_BIOS_SECOND_FILE)) +pci$(stripped_second_vbios_id).rom-type := optionrom + stripped_vgabios_dgpu_id = $(call strip_quotes,$(CONFIG_VGA_BIOS_DGPU_ID)) cbfs-files-$(CONFIG_VGA_BIOS_DGPU) += pci$(stripped_vgabios_dgpu_id).rom pci$(stripped_vgabios_dgpu_id).rom-file := $(call strip_quotes,$(CONFIG_VGA_BIOS_DGPU_FILE)) diff --git a/src/device/Kconfig b/src/device/Kconfig index 25123ea..66130cc 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -680,6 +680,35 @@
Under GNU/Linux you can run `lspci -nn` to list the IDs of your PCI devices.
+config VGA_BIOS_SECOND + bool "Add a 2nd video BIOS image" + depends on ARCH_X86 && VGA_BIOS + help + Select this option if you have a 2nd video BIOS image that you would + like to add to your ROM. + +config VGA_BIOS_SECOND_FILE + string "2nd video BIOS path and filename" + depends on VGA_BIOS_SECOND + default "vbios2.bin" + help + The path and filename of the file to use as video BIOS. + +config VGA_BIOS_SECOND_ID + string "Graphics device PCI IDs" + depends on VGA_BIOS_SECOND + help + The comma-separated PCI vendor and device ID that would associate + your vBIOS to your video card. + + 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). + + Under GNU/Linux you can run `lspci -nn` to list the IDs of your PCI devices. + config VGA_BIOS_DGPU bool "Add a discrete VGA BIOS image" depends on VGA_BIOS
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39791 )
Change subject: arch/x86: Add Kconfig option for 2nd VGA BIOS image ......................................................................
Patch Set 2:
Automatic boot test returned (PASS/FAIL/TOTAL): 5/0/5 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1707 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1706 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1705 Non-emulation targets: HP_COMPAQ_8200_ELITE_SFF_PC using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1709 HP_COMPAQ_8200_ELITE_SFF_PC using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1708
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Felix Held (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Fagerburg (Code Review)
-
Raul Rangel (Code Review)