Change in coreboot[master]: [WIP]device/Kconfig: Introduce separate graphics menu for Intel
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... [WIP]device/Kconfig: Introduce separate graphics menu for Intel Allows to select an Intel graphics driver and a "generic" graphics driver. TODO: Split it into even more chunks. Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> --- M payloads/external/SeaBIOS/Kconfig M src/device/Kconfig M src/device/pci_device.c M src/device/pci_rom.c M src/drivers/intel/gma/Makefile.inc M src/drivers/intel/gma/int15.h M src/mainboard/bostentech/gbyt4/mainboard.c M src/mainboard/google/link/mainboard.c M src/mainboard/google/rambi/mainboard.c M src/mainboard/kontron/ktqm77/mainboard.c M src/northbridge/intel/i945/Kconfig M src/northbridge/intel/i945/gma.c M src/northbridge/intel/pineview/Kconfig M src/northbridge/intel/pineview/gma.c M src/soc/intel/common/block/timer/Kconfig 15 files changed, 83 insertions(+), 42 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/49176/1 diff --git a/payloads/external/SeaBIOS/Kconfig b/payloads/external/SeaBIOS/Kconfig index 9aea2d0..6399116 100644 --- a/payloads/external/SeaBIOS/Kconfig +++ b/payloads/external/SeaBIOS/Kconfig @@ -64,7 +64,7 @@ config SEABIOS_VGA_COREBOOT prompt "Include generated option rom that implements legacy VGA BIOS compatibility" default y if !VENDOR_EMULATION - depends on !VGA_ROM_RUN && (VGA_TEXT_FRAMEBUFFER || LINEAR_FRAMEBUFFER) + depends on !(VGA_ROM_RUN || VGA_ROM_RUN_INTEL) && (VGA_TEXT_FRAMEBUFFER || LINEAR_FRAMEBUFFER) bool help coreboot can initialize the GPU of some mainboards. diff --git a/src/device/Kconfig b/src/device/Kconfig index baa7110..6df9c17 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -71,8 +71,6 @@ default NO_GFX_INIT if VGA_BIOS && PAYLOAD_SEABIOS default VGA_ROM_RUN if VGA_BIOS default MAINBOARD_DO_NATIVE_VGA_INIT - default MAINBOARD_USE_LIBGFXINIT - default RUN_FSP_GOP if INTEL_GMA_HAVE_VBT config MAINBOARD_DO_NATIVE_VGA_INIT bool "Use native graphics init" @@ -82,16 +80,6 @@ display without the need of a binary only VGA OPROM. Enabling this option may be faster, but also lacks flexibility in setting modes. -config MAINBOARD_USE_LIBGFXINIT - bool "Use libgfxinit" - depends on MAINBOARD_HAS_LIBGFXINIT - select HAVE_VGA_TEXT_FRAMEBUFFER - select HAVE_LINEAR_FRAMEBUFFER - select VGA if VGA_TEXT_FRAMEBUFFER - help - Use the SPARK library `libgfxinit` for the native graphics - initialization. This requires an Ada toolchain. - # TODO: Explain differences (if any) for onboard cards. config VGA_ROM_RUN bool "Run VGA Option ROMs" @@ -106,6 +94,56 @@ more complete BIOS interrupt services available than coreboot, which some option ROMs require in order to function correctly. +config NO_GFX_INIT + bool "None" + depends on !MAINBOARD_FORCE_NATIVE_VGA_INIT + help + Select this to not perform any graphics initialization in + coreboot. This is useful if the payload (e.g. SeaBIOS) can + initialize graphics or if pre-boot graphics are not required. + +endchoice + +choice + prompt "Intel Integrated Graphics initialization" + default NO_INTEL_GFX_INIT if VGA_BIOS && PAYLOAD_SEABIOS + default VGA_ROM_RUN_INTEL if VGA_BIOS + default MAINBOARD_DO_NATIVE_INTEL_VGA_INIT + default MAINBOARD_USE_LIBGFXINIT + default RUN_FSP_GOP if INTEL_GMA_HAVE_VBT + +config MAINBOARD_DO_NATIVE_INTEL_VGA_INIT + bool "Use native graphics init" + depends on MAINBOARD_HAS_NATIVE_INTEL_VGA_INIT + help + Some mainboards, such as the Asus P5gc, allow initializing the + display without the need of a binary only VGA OPROM. Enabling this + option may be faster, but also lacks flexibility in setting modes + in the payload or legacy OS. + +config MAINBOARD_USE_LIBGFXINIT + bool "Use libgfxinit" + depends on MAINBOARD_HAS_LIBGFXINIT + select HAVE_VGA_TEXT_FRAMEBUFFER + select HAVE_LINEAR_FRAMEBUFFER + select VGA if VGA_TEXT_FRAMEBUFFER + help + Use the SPARK library `libgfxinit` for the native graphics + initialization. This requires an Ada toolchain. + +config VGA_ROM_RUN_INTEL + bool "Run Legacy Intel VGA Option ROMs" + depends on PCI && ARCH_X86 && !MAINBOARD_FORCE_NATIVE_VGA_INIT + select HAVE_VGA_TEXT_FRAMEBUFFER + help + Execute Intel VGA Option ROMs in coreboot if found. This can be + used to enable onboard Intel video cards when not using a SeaBIOS + payload. + + When using a SeaBIOS payload it runs all option ROMs with much + more complete BIOS interrupt services available than coreboot, + which some option ROMs require in order to function correctly. + config RUN_FSP_GOP bool "Run a GOP driver" depends on HAVE_FSP_GOP && !MAINBOARD_NO_FSP_GOP @@ -115,13 +153,12 @@ to run a GOP blob. This option enables graphics initialization with such a blob. -config NO_GFX_INIT +config NO_INTEL_GFX_INIT bool "None" - depends on !MAINBOARD_FORCE_NATIVE_VGA_INIT help Select this to not perform any graphics initialization in - coreboot. This is useful if the payload (e.g. SeaBIOS) can - initialize graphics or if pre-boot graphics are not required. + coreboot on the IGD. This is useful if the payload (e.g. SeaBIOS) + can initialize graphics or if pre-boot graphics are not required. endchoice @@ -138,7 +175,7 @@ config S3_VGA_ROM_RUN bool "Re-run VGA Option ROMs on S3 resume" default y - depends on VGA_ROM_RUN && HAVE_ACPI_RESUME + depends on (VGA_ROM_RUN || VGA_ROM_RUN_INTEL) && HAVE_ACPI_RESUME help Execute VGA Option ROMs in coreboot when resuming from S3 suspend. @@ -150,7 +187,7 @@ config ALWAYS_LOAD_OPROM def_bool n - depends on VGA_ROM_RUN + depends on VGA_ROM_RUN || VGA_ROM_RUN_INTEL help Always load option ROMs if any are found. The decision to run the ROM is still determined at runtime, but the distinction @@ -171,7 +208,7 @@ bool "Load Option ROMs on PCI devices" default n if PAYLOAD_SEABIOS default y if !PAYLOAD_SEABIOS - depends on VGA_ROM_RUN + depends on VGA_ROM_RUN || VGA_ROM_RUN_INTEL help Load Option ROMs stored on PCI/PCIe/AGP VGA devices in coreboot. @@ -190,7 +227,7 @@ prompt "Option ROM execution type" default PCI_OPTION_ROM_RUN_YABEL if !ARCH_X86 default PCI_OPTION_ROM_RUN_REALMODE if ARCH_X86 - depends on VGA_ROM_RUN + depends on VGA_ROM_RUN || VGA_ROM_RUN_INTEL config PCI_OPTION_ROM_RUN_REALMODE prompt "Native mode" diff --git a/src/device/pci_device.c b/src/device/pci_device.c index a7eb91d..bfdeeaf 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -718,6 +718,10 @@ static int should_load_oprom(struct device *dev) { + if (!CONFIG(VGA_ROM_RUN) && CONFIG(VGA_ROM_RUN_INTEL) && + dev->vendor != PCI_VENDOR_ID_INTEL) + return 0; + /* If S3_VGA_ROM_RUN is disabled, skip running VGA option * ROMs when coming out of an S3 resume. */ @@ -737,7 +741,7 @@ { struct rom_header *rom, *ram; - if (!CONFIG(VGA_ROM_RUN)) + if (!CONFIG(VGA_ROM_RUN) && !CONFIG(VGA_ROM_RUN_INTEL)) return; /* Only execute VGA ROMs. */ diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index 4224c65..13ee765 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -178,7 +178,7 @@ struct rom_header *run_rom; struct pci_data *rom_data; - if (!CONFIG(VGA_ROM_RUN)) + if (!CONFIG(VGA_ROM_RUN) && !CONFIG(VGA_ROM_RUN_INTEL)) return NULL; run_rom = (struct rom_header *)(uintptr_t)PCI_VGA_RAM_IMAGE_START; diff --git a/src/drivers/intel/gma/Makefile.inc b/src/drivers/intel/gma/Makefile.inc index 964d13e..d73f800 100644 --- a/src/drivers/intel/gma/Makefile.inc +++ b/src/drivers/intel/gma/Makefile.inc @@ -2,7 +2,7 @@ ramstage-$(CONFIG_INTEL_DDI) += intel_ddi.c ramstage-$(CONFIG_INTEL_EDID) += edid.c vbt.c -ifeq ($(CONFIG_VGA_ROM_RUN),y) +ifeq ($(CONFIG_VGA_ROM_RUN_INTEL),y) ramstage-$(CONFIG_INTEL_INT15) += int15.c endif ramstage-$(CONFIG_INTEL_GMA_ACPI) += acpi.c diff --git a/src/drivers/intel/gma/int15.h b/src/drivers/intel/gma/int15.h index cddd4ac..461d144 100644 --- a/src/drivers/intel/gma/int15.h +++ b/src/drivers/intel/gma/int15.h @@ -27,7 +27,7 @@ GMA_INT15_ACTIVE_LFP_EDP = 0x03, }; -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) /* Install custom int15 handler for VGA OPROM */ void install_intel_vga_int15_handler(int active_lfp, int pfit, int display, int panel_type); #else diff --git a/src/mainboard/bostentech/gbyt4/mainboard.c b/src/mainboard/bostentech/gbyt4/mainboard.c index d0131bc..857a0c2 100644 --- a/src/mainboard/bostentech/gbyt4/mainboard.c +++ b/src/mainboard/bostentech/gbyt4/mainboard.c @@ -2,12 +2,12 @@ #include <device/device.h> #include <console/console.h> -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) #include <x86emu/x86emu.h> #endif #include <arch/interrupt.h> -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) static int int15_handler(void) { int res = 1; @@ -98,7 +98,7 @@ static void mainboard_enable(struct device *dev) { -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) /* Install custom int15 handler for VGA OPROM */ mainboard_interrupt_handlers(0x15, &int15_handler); #endif diff --git a/src/mainboard/google/link/mainboard.c b/src/mainboard/google/link/mainboard.c index c4b621a..fd04031 100644 --- a/src/mainboard/google/link/mainboard.c +++ b/src/mainboard/google/link/mainboard.c @@ -4,7 +4,7 @@ #include <device/device.h> #include <device/pci_ops.h> #include <console/console.h> -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) #include <x86emu/x86emu.h> #endif #include <acpi/acpi.h> @@ -18,7 +18,7 @@ #include <ec/google/chromeec/ec.h> #include <vendorcode/google/chromeos/chromeos.h> -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) static int int15_handler(void) { int res = 0; @@ -171,7 +171,7 @@ dev->ops->init = mainboard_init; dev->ops->get_smbios_data = link_onboard_smbios_data; dev->ops->acpi_inject_dsdt = chromeos_dsdt_generator; -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) /* Install custom int15 handler for VGA OPROM */ mainboard_interrupt_handlers(0x15, &int15_handler); #endif diff --git a/src/mainboard/google/rambi/mainboard.c b/src/mainboard/google/rambi/mainboard.c index 41d9795..894cc74 100644 --- a/src/mainboard/google/rambi/mainboard.c +++ b/src/mainboard/google/rambi/mainboard.c @@ -2,7 +2,7 @@ #include <device/device.h> #include <console/console.h> -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) #include <x86emu/x86emu.h> #endif #include <arch/interrupt.h> @@ -13,7 +13,7 @@ #include <bootstate.h> #include <vendorcode/google/chromeos/chromeos.h> -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) static int int15_handler(void) { int res = 1; @@ -144,7 +144,7 @@ dev->ops->init = mainboard_init; dev->ops->get_smbios_data = mainboard_smbios_data; dev->ops->acpi_inject_dsdt = chromeos_dsdt_generator; -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) /* Install custom int15 handler for VGA OPROM */ mainboard_interrupt_handlers(0x15, &int15_handler); #endif diff --git a/src/mainboard/kontron/ktqm77/mainboard.c b/src/mainboard/kontron/ktqm77/mainboard.c index 95e639a..c571c7f 100644 --- a/src/mainboard/kontron/ktqm77/mainboard.c +++ b/src/mainboard/kontron/ktqm77/mainboard.c @@ -4,14 +4,14 @@ #include <device/device.h> #include <device/pci_def.h> #include <console/console.h> -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) #include <x86emu/x86emu.h> #endif #include <option.h> #include <arch/interrupt.h> #include <southbridge/intel/bd82x6x/pch.h> -#if CONFIG(VGA_ROM_RUN) +#if CONFIG(VGA_ROM_RUN_INTEL) static int int15_handler(void) { int res = 0; diff --git a/src/northbridge/intel/i945/Kconfig b/src/northbridge/intel/i945/Kconfig index 091d2f1..919331b 100644 --- a/src/northbridge/intel/i945/Kconfig +++ b/src/northbridge/intel/i945/Kconfig @@ -12,7 +12,7 @@ select INTEL_GMA_ACPI select INTEL_GMA_SSC_ALTERNATE_REF select INTEL_EDID - select HAVE_VGA_TEXT_FRAMEBUFFER if MAINBOARD_DO_NATIVE_VGA_INIT + select HAVE_VGA_TEXT_FRAMEBUFFER if MAINBOARD_DO_NATIVE_INTEL_VGA_INIT select PARALLEL_MP config VBOOT diff --git a/src/northbridge/intel/i945/gma.c b/src/northbridge/intel/i945/gma.c index b396eff..7447ab2 100644 --- a/src/northbridge/intel/i945/gma.c +++ b/src/northbridge/intel/i945/gma.c @@ -672,7 +672,7 @@ if (!CONFIG(NO_GFX_INIT)) pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER); - if (CONFIG(MAINBOARD_DO_NATIVE_VGA_INIT)) { + if (CONFIG(MAINBOARD_DO_NATIVE_INTEL_VGA_INIT)) { int vga_disable = (pci_read_config16(dev, GGC) & 2) >> 1; if (acpi_is_wakeup_s3()) { printk(BIOS_INFO, diff --git a/src/northbridge/intel/pineview/Kconfig b/src/northbridge/intel/pineview/Kconfig index 962825a..fe4fd6b 100644 --- a/src/northbridge/intel/pineview/Kconfig +++ b/src/northbridge/intel/pineview/Kconfig @@ -10,8 +10,8 @@ select HAVE_DEBUG_RAM_SETUP select VGA select MAINBOARD_HAS_NATIVE_INTEL_VGA_INIT - select HAVE_VGA_TEXT_FRAMEBUFFER if MAINBOARD_DO_NATIVE_VGA_INIT - select INTEL_EDID if MAINBOARD_DO_NATIVE_VGA_INIT + select HAVE_VGA_TEXT_FRAMEBUFFER if MAINBOARD_DO_NATIVE_INTEL_VGA_INIT + select INTEL_EDID if MAINBOARD_DO_NATIVE_INTEL_VGA_INIT select INTEL_GMA_ACPI select PARALLEL_MP select NO_CBFS_MCACHE diff --git a/src/northbridge/intel/pineview/gma.c b/src/northbridge/intel/pineview/gma.c index e0ed0f0..fcd05f8 100644 --- a/src/northbridge/intel/pineview/gma.c +++ b/src/northbridge/intel/pineview/gma.c @@ -224,7 +224,7 @@ if (!CONFIG(NO_GFX_INIT)) pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER); - if (!CONFIG(MAINBOARD_DO_NATIVE_VGA_INIT)) { + if (!CONFIG(MAINBOARD_DO_NATIVE_INTEL_VGA_INIT)) { /* PCI init, will run VBIOS */ pci_dev_init(dev); } else { diff --git a/src/soc/intel/common/block/timer/Kconfig b/src/soc/intel/common/block/timer/Kconfig index 42613a8..f7d705c 100644 --- a/src/soc/intel/common/block/timer/Kconfig +++ b/src/soc/intel/common/block/timer/Kconfig @@ -5,7 +5,7 @@ config USE_LEGACY_8254_TIMER bool "Use Legacy 8254 Timer" - default y if PAYLOAD_SEABIOS || VGA_ROM_RUN + default y if PAYLOAD_SEABIOS || VGA_ROM_RUN || VGA_ROM_RUN_INTEL default n help Setting this makes the Legacy 8254 Timer available by disabling -- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-MessageType: newchange
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Wed, 06 Jan 2021 16:46:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/49176/1/src/device/Kconfig File src/device/Kconfig: https://review.coreboot.org/c/coreboot/+/49176/1/src/device/Kconfig@134 PS1, Line 134: VGA_ROM_RUN_INTEL problem for this Kconfig is still depends on VGA_BIOS and VGA_BIOS expect to provide oprom as part of coreboot CBFS, few more Kconfig gets added in due course without any real purpose. -- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 07 Jan 2021 02:39:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/49176/1/src/device/Kconfig File src/device/Kconfig: https://review.coreboot.org/c/coreboot/+/49176/1/src/device/Kconfig@134 PS1, Line 134: VGA_ROM_RUN_INTEL
problem for this Kconfig is still depends on VGA_BIOS and VGA_BIOS expect to provide oprom as part o […] Any ideas how to improve the situation?
-- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 07 Jan 2021 07:29:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik@intel.com> Gerrit-MessageType: comment
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/49176/1/src/device/Kconfig File src/device/Kconfig: https://review.coreboot.org/c/coreboot/+/49176/1/src/device/Kconfig@134 PS1, Line 134: VGA_ROM_RUN_INTEL
Any ideas how to improve the situation? unfortunately we need some Kconfig to tell if vga oprom inside cbfs that might help to resolve this problem ? then irrespective of dgpu or igd we can use that config
-- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 07 Jan 2021 08:48:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph@9elements.com> Comment-In-Reply-To: Subrata Banik <subrata.banik@intel.com> Gerrit-MessageType: comment
Hello build bot (Jenkins), Nico Huber, Damien Zammit, Martin Roth, Patrick Georgi, Stefan Reinauer, Subrata Banik, Angel Pons, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/49176 to look at the new patch set (#2). Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... [WIP]device/Kconfig: Introduce separate graphics menu for Intel Allow a more fine grained selection with graphics driver to run. On multi GPU systems this allows to run multiple grahics drivers at the same time. Allows to select an Intel graphics driver and a "generic" graphics driver. TODO: Split it into even more chunks. Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> --- M payloads/external/SeaBIOS/Kconfig M src/device/Kconfig M src/device/graphics/Kconfig A src/device/graphics/intel/Kconfig M src/device/pci_device.c M src/device/pci_rom.c M src/drivers/intel/gma/Makefile.inc M src/drivers/intel/gma/int15.h M src/mainboard/bostentech/gbyt4/mainboard.c M src/mainboard/google/link/mainboard.c M src/mainboard/google/rambi/mainboard.c M src/mainboard/kontron/ktqm77/mainboard.c M src/northbridge/intel/i945/Kconfig M src/northbridge/intel/i945/gma.c M src/northbridge/intel/pineview/Kconfig M src/northbridge/intel/pineview/gma.c M src/soc/intel/common/block/timer/Kconfig 17 files changed, 122 insertions(+), 67 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/49176/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Attention is currently required from: Subrata Banik. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Patch Set 2: (1 comment) File src/device/Kconfig: https://review.coreboot.org/c/coreboot/+/49176/comment/a6cd0b66_dc8e54d9 PS1, Line 134: VGA_ROM_RUN_INTEL
unfortunately we need some Kconfig to tell if vga oprom inside cbfs that might help to resolve this […] CB:31449 added an option to add a 2nd VBIOS. This was necessary for AMD laptops with switchable graphics, because both the iGPU and dGPU need a VBIOS.
-- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Subrata Banik <subrata.banik@intel.com> Gerrit-Comment-Date: Thu, 07 Jan 2021 16:01:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph@9elements.com> Comment-In-Reply-To: Subrata Banik <subrata.banik@intel.com> Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Angel Pons. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Patch Set 2: (1 comment) File src/device/Kconfig: https://review.coreboot.org/c/coreboot/+/49176/comment/fbc33db2_cea8cbd9 PS1, Line 134: VGA_ROM_RUN_INTEL
CB:31449 added an option to add a 2nd VBIOS. […] CB:31449 CL is good as it creates dedicated Kconfig for DGPU with VBIOS as part of CBFS but how do we handle DGPU where VBIOS is embedded into and not required to pack anything into CBFS?
-- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Comment-Date: Thu, 07 Jan 2021 17:48:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph@9elements.com> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: Subrata Banik <subrata.banik@intel.com> Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Patch Set 2: (3 comments) File src/device/graphics/intel/Kconfig: https://review.coreboot.org/c/coreboot/+/49176/comment/25c08e8c_14012c94 PS2, Line 12: Selected by mainboards that do not have any graphics ports connected to the SoC. I would repurpose this option to allow mainboards to hide Intel graphics init options from Kconfig, e.g. supermicro/x11_lga1151_series. I know of several other mainboards where the iGPU is unusable. https://review.coreboot.org/c/coreboot/+/49176/comment/ce42a2a1_9c5ff1fb PS2, Line 21: config MAINBOARD_FORCE_NATIVE_INTEL_VGA_INIT Nothing needs to select this option, so I would not even add it in the first place. https://review.coreboot.org/c/coreboot/+/49176/comment/03445e3b_2b4fba1f PS2, Line 72: a nit: *FSP* GOP driver -- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Patrick Rudolph <siro@das-labor.org> Gerrit-Comment-Date: Thu, 07 Jan 2021 18:19:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Subrata Banik, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49176 ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Patch Set 2: (3 comments) File src/device/Kconfig: https://review.coreboot.org/c/coreboot/+/49176/comment/e24724be_5e913487 PS1, Line 79: such as the Google Link This is crying for an update ;) https://review.coreboot.org/c/coreboot/+/49176/comment/4eac727a_bdeeeac3 PS1, Line 98: bool "None" This should eventually be renamed, e.g. "Device specific drivers only". Because the selection in the device-specific choices would overrule this one. https://review.coreboot.org/c/coreboot/+/49176/comment/41b63721_7bbece69 PS1, Line 134: VGA_ROM_RUN_INTEL
CB:31449 CL is good as it creates dedicated Kconfig for DGPU with VBIOS as part of CBFS but how do w […] Well, both this and the VGA_ROM_RUN option don't depend on files in CBFS. It's only the default that is hard to achieve. But I don't see a problem: Subrata's CB:49016 allows a mainboard to let VGA_ROM_RUN default to `y` for DGPUs. This choice is for IGPUs and independent.
E.g. one can select MAINBOARD_USE_LIBGFXINIT in this choice for an IGPU and still have VGA_ROM_RUN for the DGPU. -- To view, visit https://review.coreboot.org/c/coreboot/+/49176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Subrata Banik <subrata.banik@intel.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Patrick Rudolph <siro@das-labor.org> Gerrit-Comment-Date: Fri, 08 Jan 2021 16:56:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph@9elements.com> Comment-In-Reply-To: Subrata Banik <subrata.banik@intel.com> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/49176?usp=email ) Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Intel ...................................................................... Abandoned This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author. -- To view, visit https://review.coreboot.org/c/coreboot/+/49176?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I410fecfdb8a99ca520b2c05edd9ed2558d7e0a92 Gerrit-Change-Number: 49176 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: abandon
participants (5)
-
Angel Pons (Code Review) -
Martin L Roth (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Subrata Banik (Code Review)