Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33738
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
device/Kconfig: Add HAVE_EXT_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. CONFIG_ENABLE_EXT_GFX to select all required kconfig to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/device/Kconfig 1 file changed, 16 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/1
diff --git a/src/device/Kconfig b/src/device/Kconfig index 0539062..0ee6158 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -682,4 +682,20 @@ I2C controller is not (yet) available. The platform code needs to provide bindings to manually toggle I2C lines.
+config HAVE_EXT_GFX + bool + +config ENABLE_EXT_GFX + bool "Enable Display over external PCI GFX card" + default n + depends on HAVE_EXT_GFX + select VGA_ROM_RUN + select ALWAYS_LOAD_OPROM + select ALWAYS_RUN_OPROM + select ON_DEVICE_ROM_LOAD + select FRAMEBUFFER_SET_VESA_MODE + help + Its possible to bring display through external graphics card in coreboot. + This option enables graphics initialization with external graphics card. + endmenu
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33738
to look at the new patch set (#2).
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
device/Kconfig: Add HAVE_EXT_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. CONFIG_ENABLE_EXT_GFX to select all required kconfig to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/device/Kconfig 1 file changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/2
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33738
to look at the new patch set (#3).
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
device/Kconfig: Add HAVE_EXT_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. CONFIG_ENABLE_EXT_GFX to select all required kconfig to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/device/Kconfig 1 file changed, 18 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 3:
(1 comment)
I do not see the usefulness of having that as a build time option. Also, why can’t the payload like SeaBIOS run the Option ROM?
https://review.coreboot.org/#/c/33738/3/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/#/c/33738/3/src/device/Kconfig@129 PS3, Line 129: select FRAMEBUFFER_SET_VESA_MODE Why?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 3: Code-Review-1
conflicts with Kconfig choice "Graphics initialization"
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 3:
Patch Set 3: Code-Review-1
conflicts with Kconfig choice "Graphics initialization"
'Graphics initialization" choice been designed with a viewpoint that it can only work with onboard GFX and doesn't consider any future option of only enable graphics over external card. I don't understand what you mean by conflicts with Graphics initialization? Using this Kconfig, i wish to select a bunch of required kconfig at once rather selecting from SOC/mainboard.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 3:
(1 comment)
Patch Set 3:
(1 comment)
I do not see the usefulness of having that as a build time option.
Its always been build time option through Kconfig either HAVE_FSP_GOP or VGA_RUN_ROM. do you have some example where both been selected and platform can dynamically pick GFX over off board card and bring display.
Also, why can’t the payload like SeaBIOS run the Option ROM?
I'm not sure if i can give answer. My effort here to make offboard card work with one intel RVP platform
https://review.coreboot.org/#/c/33738/3/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/#/c/33738/3/src/device/Kconfig@129 PS3, Line 129: select FRAMEBUFFER_SET_VESA_MODE
Why?
i don't understand what you mean by "Why" ? Coreboot policies are been designed to either enable GOP/VBT or option rom statically. I'm only looking at IA platform, where i could see FSP GOP is default enable through Kconfig from mainboard and FSP GOP is only responsible of initialize IGD and it doesn't bother to initialize and look for off board GFX card unless user unselect FSP GOP and selects VGA_ROM_RUN which is parallel to FSP GOP. Hence you could only support one way display over other
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 3: Code-Review-1
Patch Set 3:
Patch Set 3: Code-Review-1
conflicts with Kconfig choice "Graphics initialization"
'Graphics initialization" choice been designed with a viewpoint that it can only work with onboard GFX and doesn't consider any future option of only enable graphics over external card.
That's not true. It works with dGPUs already. Just select VGA_ROM_RUN.
I don't understand what you mean by conflicts with Graphics initialization? Using this Kconfig, i wish to select a bunch of required kconfig at once rather selecting from SOC/mainboard.
It conflicts with the existing choice as it selects specific Kconfigs for no reason.
Maybe you should give more details about the problem you encounter on your platform.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 3: -Code-Review
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 3:
Patch Set 3: Code-Review-1
Patch Set 3:
Patch Set 3: Code-Review-1
conflicts with Kconfig choice "Graphics initialization"
'Graphics initialization" choice been designed with a viewpoint that it can only work with onboard GFX and doesn't consider any future option of only enable graphics over external card.
That's not true. It works with dGPUs already. Just select VGA_ROM_RUN.
yes VGA_ROM_RUN should work for dGPU but today default VGA_ROM_RUN if VGA_BIOS where you like to include vga rom inside your cbfs with vid:did etc. I'm not telling it won't work dGPU over PCIe but its not well managed.
I don't understand what you mean by conflicts with Graphics initialization? Using this Kconfig, i wish to select a bunch of required kconfig at once rather selecting from SOC/mainboard.
It conflicts with the existing choice as it selects specific Kconfigs for no reason.
Eventually selecting VGA_ROM_RUN might selects few kconfig inherently which i have forced select using ENABLE_EXT_GFX so, it won't prompt. I understand your argument, but please check it will not duplicate the entire list of selection.
Maybe you should give more details about the problem you encounter on your platform.
How do my mainboard convey to my soc code that don't initialize onboard GFX and only lunch oprom in real mode with certain vesa mode set ? isn't it better we club everything under still Kconfig? i can achieve my work without this CL then i had to select multiple Kconfigs at make ?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 4:
Patch Set 3:
Patch Set 3: Code-Review-1
Patch Set 3:
Patch Set 3: Code-Review-1
conflicts with Kconfig choice "Graphics initialization"
'Graphics initialization" choice been designed with a viewpoint that it can only work with onboard GFX and doesn't consider any future option of only enable graphics over external card.
That's not true. It works with dGPUs already. Just select VGA_ROM_RUN.
yes VGA_ROM_RUN should work for dGPU but today default VGA_ROM_RUN if VGA_BIOS where you like to include vga rom inside your cbfs with vid:did etc. I'm not telling it won't work dGPU over PCIe but its not well managed.
Yes, that's true. External GPUs and multi GPU handling has never been a priority when adding new platforms. In case the mainboard doesn't have libgfxinit / native graphics init support, the default selection would already be VGA_ROM_RUN.
I don't understand what you mean by conflicts with Graphics initialization? Using this Kconfig, i wish to select a bunch of required kconfig at once rather selecting from SOC/mainboard.
It conflicts with the existing choice as it selects specific Kconfigs for no reason.
Eventually selecting VGA_ROM_RUN might selects few kconfig inherently which i have forced select using ENABLE_EXT_GFX so, it won't prompt. I understand your argument, but please check it will not duplicate the entire list of selection.
Looking at the next commit it only works for specific mainboard, which have the IGD disabled. If the mainboard doesn't have IGD, only PEG ports, why do you need a Kconfig for that? FSP should initialize the PEG port in all cases if it's marked as 'on'. The skylake code is able to handle external GPUs already.
Maybe you should give more details about the problem you encounter on your platform.
How do my mainboard convey to my soc code that don't initialize onboard GFX and only lunch oprom in real mode with certain vesa mode set ? isn't it better we club everything under still Kconfig? i can achieve my work without this CL then i had to select multiple Kconfigs at make ?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 3:
Patch Set 3: Code-Review-1
Patch Set 3:
Patch Set 3: Code-Review-1
conflicts with Kconfig choice "Graphics initialization"
'Graphics initialization" choice been designed with a viewpoint that it can only work with onboard GFX and doesn't consider any future option of only enable graphics over external card.
That's not true. It works with dGPUs already. Just select VGA_ROM_RUN.
yes VGA_ROM_RUN should work for dGPU but today default VGA_ROM_RUN if VGA_BIOS where you like to include vga rom inside your cbfs with vid:did etc. I'm not telling it won't work dGPU over PCIe but its not well managed.
Yes, that's true. External GPUs and multi GPU handling has never been a priority when adding new platforms. In case the mainboard doesn't have libgfxinit / native graphics init support, the default selection would already be VGA_ROM_RUN.
I don't understand what you mean by conflicts with Graphics initialization? Using this Kconfig, i wish to select a bunch of required kconfig at once rather selecting from SOC/mainboard.
It conflicts with the existing choice as it selects specific Kconfigs for no reason.
Eventually selecting VGA_ROM_RUN might selects few kconfig inherently which i have forced select using ENABLE_EXT_GFX so, it won't prompt. I understand your argument, but please check it will not duplicate the entire list of selection.
Looking at the next commit it only works for specific mainboard, which have the IGD disabled. If the mainboard doesn't have IGD, only PEG ports, why do you need a Kconfig for that? FSP should initialize the PEG port in all cases if it's marked as 'on'. The skylake code is able to handle external GPUs already.
[Subrata] I'm not trying to enable display over PEG ports, rather i'm planning to use PCIE over PCH to bring display. if you see there are 4 option 1. IGD 2. PEG 3. PCIE 4. Hybrid
I'm working to enable PCIE GFX cards to enable over PCH root bridge.
Maybe you should give more details about the problem you encounter on your platform.
How do my mainboard convey to my soc code that don't initialize onboard GFX and only lunch oprom in real mode with certain vesa mode set ? isn't it better we club everything under still Kconfig? i can achieve my work without this CL then i had to select multiple Kconfigs at make ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 5:
Patch Set 4:
[Subrata] I'm not trying to enable display over PEG ports, rather i'm planning to use PCIE over PCH to bring display. if you see there are 4 option
- IGD
- PEG
- PCIE
- Hybrid
I'm working to enable PCIE GFX cards to enable over PCH root bridge.
Aren't PEG and PCIE the same?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 4:
[Subrata] I'm not trying to enable display over PEG ports, rather i'm planning to use PCIE over PCH to bring display. if you see there are 4 option
- IGD
- PEG
- PCIE
- Hybrid
I'm working to enable PCIE GFX cards to enable over PCH root bridge.
Aren't PEG and PCIE the same?
PEG ports are over MCH and PCIE ports are PCH based
Hello Patrick Rudolph, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33738
to look at the new patch set (#6).
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
device/Kconfig: Add HAVE_EXT_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. CONFIG_ENABLE_EXT_GFX to select all required kconfig to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/device/Kconfig 1 file changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/6
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/#/c/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/#/c/33738/6/src/device/Kconfig@131 PS6, Line 131: Its It‘s
https://review.coreboot.org/#/c/33738/6/src/device/Kconfig@133 PS6, Line 133: to make IGD disable to disable IGD
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@131 PS6, Line 131: Its
It‘s
Done
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: to make IGD disable
to disable IGD
Done
Hello Patrick Rudolph, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33738
to look at the new patch set (#7).
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
device/Kconfig: Add HAVE_EXT_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. CONFIG_ENABLE_EXT_GFX to select all required kconfig to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/device/Kconfig 1 file changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/7
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 7: Code-Review-1
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable. This is not the case in the current patch or one of the following. Also I don't see any reason why you want to disable the onboard IGD.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
This is not the case in the current patch or one of the following.
I don't understand whats "not the case?"
Also I don't see any reason why you want to disable the onboard IGD.
In general BIOS policy, external display cards are having more privilege over on board graphics, hence in case external gfx been enable then we used to make on board gfx disable. You can verify on your desktop systems.
Hello Patrick Rudolph, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33738
to look at the new patch set (#8).
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
device/Kconfig: Add HAVE_EXT_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. CONFIG_ENABLE_EXT_GFX to select all required kconfig to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/device/Kconfig 1 file changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/8
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
(1 comment)
I guess the main issue is that you are mixing two unrelated graphic options here. 1. Preferred GPU in a (multi) GPU system 2. Preferred graphic initialisation method
To resolve that I propose a new Kconfig choice "Preferred GPU configuration" instead of a single Kconfig: * Disable all GPUs * Enable IGD only in multi GPU setup * Enable PEG only in multi GPU setup (default) * Enable all GPUs in multi GPU setup
It does not depend on graphic initialisation, but graphic initialisation would depend on it. For example you would never run GOP / libgfxinit in case of "Enable PEG only in multi GPU setup".
FSP would make use of the new choice to properly configure all graphic devices.
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
This is not the case in the current patch or one of the following. […]
Such a policy does not exist (yet). It's possible to use IGD and dGPU on some coreboot enabled platforms.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
Patch Set 8:
(1 comment)
I guess the main issue is that you are mixing two unrelated graphic options here.
- Preferred GPU in a (multi) GPU system
- Preferred graphic initialisation method
To resolve that I propose a new Kconfig choice "Preferred GPU configuration" instead of a single Kconfig:
- Disable all GPUs
- Enable IGD only in multi GPU setup
- Enable PEG only in multi GPU setup (default)
- Enable all GPUs in multi GPU setup
Yes, you can crate such config and align existing CB code.
It does not depend on graphic initialisation, but graphic initialisation would depend on it. For example you would never run GOP / libgfxinit in case of "Enable PEG only in multi GPU setup".
FSP would make use of the new choice to properly configure all graphic devices.
For external GFX card over PCIE initialization FSP doesn't come to picture, we just need some way to tell to not initialize on board GFX as priority is with GFX over PCIE
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
Such a policy does not exist (yet). […]
i don't see any platform so far where on board and off board GFX both working simultaneously. Just wondering how you tell your BIOS to through some display over which framebuffer, even today in coreboot there are single call to know framebuffer either from IGD BAR 0x18 or from external device BAR 0x10. Also how will you notify your OS to use simultaneous display ?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
i don't see any platform so far where on board and off board GFX both working simultaneously. […]
I'm referring at lb_framebuffer() function, if that would have true then coreboot should create an array of framebuffer[] to through display into all possible display output, like iGD, dGPU etc..
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
I'm referring at lb_framebuffer() function, if that would have true then coreboot should create an a […]
On Lenovo Thinkpads that support Nvidia Optimus we have two GPUs active. But only because two GPUs are active doesn't mean that you run graphics initialisation twice. Usually you run graphics init on the primary VGA device only.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
On Lenovo Thinkpads that support Nvidia Optimus we have two GPUs active. […]
I'm afraid that your assumption is not correct, with UEFI BIOS on Lenovo Thinkpad that I'm using is giving preference to external gfx card over IGD.
And yes that you are telling about running GFX init once is called policy that OEM decides as per their PRD.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8: Code-Review-1
(1 comment)
-1 as the introduced Kconfig options has no benefit and only confuses users. It increases fragmentation and depends on board specific code for no reason. It mixes preferred GPU selection on multi GPU setup platforms with legacy graphics initialisation features in coreboot.
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
I'm afraid that your assumption is not correct, with UEFI BIOS on Lenovo Thinkpad that I'm using is […]
That's how it works in coreboot.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
That's how it works in coreboot.
which mainboard is that?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
Patch Set 8: Code-Review-1
(1 comment)
-1 as the introduced Kconfig options has no benefit and only confuses users. It increases fragmentation and depends on board specific code for no reason.
external GFX card over PCIE is board specific right then what the problem if board select that option ?
It mixes preferred GPU selection on multi GPU setup platforms with legacy graphics initialisation features in coreboot.
sorry, i don't understand the issue here ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
which mainboard is that?
AFAIK, most Lenovo T4xx, T5xx and W5xx have such a setup. I also have other (yet to port) laptops with the same Nvidia Optimus setup.
Subrata Banik has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Abandoned
Subrata Banik has restored this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: device/Kconfig: Add HAVE_EXT_GFX kconfig ......................................................................
Restored
Hello Patrick Rudolph, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33738
to look at the new patch set (#9).
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. This kconfig to select required kconfig which are not default selected by VGA_ROM_RUN to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/icelake/Kconfig 1 file changed, 14 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/9
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX you still have FSP_GOP, thus HAVE_FSP_GOP is correct. but you likely don't want RUN_FSP_GOP.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
you still have FSP_GOP, thus HAVE_FSP_GOP is correct. […]
I think RUN_FSP_GOP is depends on HAVE_FSP_GOP and auto selected based on HAVE_FSP_GOP. thats the reason i have blocked HAVE_FSP_GOP selection here
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
I think RUN_FSP_GOP is depends on HAVE_FSP_GOP and auto selected based on HAVE_FSP_GOP. […]
Yes, I see the Kconfig logic behind. But, running graphics init on a different device than IGD doesn't make the GOP driver in FSP disappear. It's still there.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
But, running graphics init on a different device than IGD doesn't make the GOP driver in FSP disappear. It's still there.
yes, that's true but based on RUN_FSP_GOP select, we will fill vbt pointer
if (!CONFIG(RUN_FSP_GOP)) return NULL;
and inside FSP logic, we will again check if vbt point is NULL or not, if vbt pointer is not NULL, then only we execute GFX PEIM.efi module else not.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
But, running graphics init on a different device than IGD doesn't make the GOP driver in FSP disap […]
That's unrelated to HAVE_FSP_GOP. Running a driver and having a driver are different aspects.
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 211: select ALWAYS_LOAD_OPROM that conflicts with "Graphics initialization" menu, it only allows you to select VGA_ROM_RUN. If this is select by a mainboard, how can a user unselect it? For example if you have Linux as payload and you don't want graphics initialization in coreboot, what do you do?
Hello Patrick Rudolph, Aamir Bohra, Patrick Rudolph, Maulik V Vaghela, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33738
to look at the new patch set (#11).
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. This kconfig to select required kconfig which are not default selected by VGA_ROM_RUN to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/icelake/Kconfig 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/11
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
That's unrelated to HAVE_FSP_GOP. […]
fixed now
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 211: select ALWAYS_LOAD_OPROM
that conflicts with "Graphics initialization" menu, it only allows you to select VGA_ROM_RUN. […]
very good point, thanks i have missed that, i was always getting display earlier. now i have verified that with GBB flag 0x39, i'm seeing pre-os display and 0x0 won't bring display
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
fixed now
@Patrick R: can you please help to check once ?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
@Patrick R: can you please help to check once ?
Done
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 211: select ALWAYS_LOAD_OPROM
very good point, thanks i have missed that, i was always getting display earlier. […]
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11: Code-Review+1
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
Done
Done
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 24: select HAVE_FSP_GOP if !ENABLE_DISPLAY_OVER_EXT_PCIE_GFX
Done
Done
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/10/src/soc/intel/icelake/Kcon... PS10, Line 211: select ALWAYS_LOAD_OPROM
Done
Done
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/3/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/3/src/device/Kconfig@129 PS3, Line 129: select FRAMEBUFFER_SET_VESA_MODE
i don't understand what you mean by "Why" ? […]
Done
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/6/src/device/Kconfig@133 PS6, Line 133: graphics card and selecting this Kconfig will ask FSP to make IGD disable.
AFAIK, most Lenovo T4xx, T5xx and W5xx have such a setup. […]
Ack
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11: Code-Review-1
I still do not see an argument in the commit message, why this is intel/icelake specific.
If such a Kconfig option is introduced, it needs to be integrated into the existing options under `src/device`, where VGA_ROM_RUN and friends are.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
Patch Set 11: Code-Review-1
I still do not see an argument in the commit message, why this is intel/icelake specific.
If such a Kconfig option is introduced, it needs to be integrated into the existing options under `src/device`, where VGA_ROM_RUN and friends are.
HI Paul, if you look at gerrit discussion, Patrick had point that keeping such kconfig into common driver might increase overhead of maintenance hence he suggested to make it soc specific. So far we don't see any such soc/rvp going to use this feature. If we think going forward that we are keeping for all rvp/soc then we should move this code into common/driver. Do you agree ?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM Why is this "required" for external GFX?
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 211: select FRAMEBUFFER_SET_VESA_MODE if CHROMEOS Isn't this independent from the external GFX choice? Would internal graphics work in text mode with Chromeos?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
Why is this "required" for external GFX?
should_load_oprom()is required to execute pci based oprom
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 211: select FRAMEBUFFER_SET_VESA_MODE if CHROMEOS
Isn't this independent from the external GFX choice?
yes, its kind of, VGA_ROM_RUN default loads text mode which might not work with depthcharge
Would internal graphics work in text mode with Chromeos?
no, it doesn't work
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 11:
Subrata, please have a look at CB:34483.
Hello Patrick Rudolph, Aamir Bohra, Wonkyu Kim, Patrick Rudolph, Maulik V Vaghela, Paul Menzel, V Sowmya, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33738
to look at the new patch set (#12).
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. This kconfig to select required kconfig which are not default selected by VGA_ROM_RUN to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/icelake/Kconfig 1 file changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/33738/12
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(2 comments)
Patch Set 11:
Subrata, please have a look at CB:34483.
yes, i have +2'ed your CL and rebased mine on top of that.
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
should_load_oprom()is required to execute pci based oprom
Done
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 211: select FRAMEBUFFER_SET_VESA_MODE if CHROMEOS
Isn't this independent from the external GFX choice? […]
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
should_load_oprom()is required to execute pci based oprom
Yes, that's why it always returns 1 if the OpRom is about to be executed. Please see help text of ALWAYS_LOAD_OPROM. It's required for certain situations with OS drivers. I don't know any OS driver that wouldn't just ask the PCI device for the OpRom (which should work for every external GFX card).
Done
how???
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
should_load_oprom()is required to execute pci based oprom
Yes, that's why it always returns 1 if the OpRom is about to be executed.
As per ALWAYS_LOAD_OPROM help text, it need to be selected if we wish to load OpRom. Without selecting this Kconfig, Oprom won't execute during pci enumeration
Please see help text of ALWAYS_LOAD_OPROM. It's required for certain situations with OS drivers. I don't know any OS driver that wouldn't just ask the PCI device for the OpRom (which should work for every external GFX card)
i don't understand this part. .
Done
how???
I mean to say, we are already selecting ALWAYS_LOAD_OPROM from here. do you think its wrong ?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
should_load_oprom()is required to execute pci based oprom […]
i guess that help text trying to explain that chrome os very much dependent on vbt for gfx init rather oprom. as this is the case so far.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
i guess that help text trying to explain that chrome os very much dependent on vbt for gfx init rath […]
Just to check if this is a bug in Gerrit, I'm removing the "Resolved" flag.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
Just to check if this is a bug in Gerrit, I'm removing the "Resolved" flag.
Hmmm, this worked. Subrata, did you intentionally mark this as "Resolved" while we are discussing it?
ALWAYS_LOAD_OPROM is only needed if you wish to load the OpRom even if it *won't* be executed. The OS can do this by itself for cards that have an actual ROM. The VBT is an Intel concept. Intel's IGD does not have a physical ROM, so the VBT/VBIOS has to be loaded from CBFS (what the OS drivers usually can't do).
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
Just to check if this is a bug in Gerrit, I'm removing the "Resolved" flag.
Hmmm, this worked. Subrata, did you intentionally mark this as "Resolved" while we are discussing it?
I don't select that. not sure why everytime its been "Done".
ALWAYS_LOAD_OPROM is only needed if you wish to load the OpRom even if it *won't* be executed.
yes.
The OS can do this by itself for cards that have an actual
ROM. The VBT is an Intel concept. Intel's IGD does not have a physical ROM, so the VBT/VBIOS has to be loaded from CBFS (what the OS drivers usually can't do).
you are right. Always execute should be a problem but not load i believe
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
Just to check if this is a bug in Gerrit, I'm removing the "Resolved" flag. […]
@Nico, good to merge ?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
@Nico, good to merge ?
I have no idea what it is good for.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
I have no idea what it is good for.
i'm asking if you can submit your base cl so i can submit this cl too.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/11/src/soc/intel/icelake/Kcon... PS11, Line 210: select ALWAYS_LOAD_OPROM
i'm asking if you can submit your base cl so i can submit this cl too.
marking this resolved based on merit here
Hmmm, this worked. Subrata, did you intentionally mark this as "Resolved" while we are discussing it?
ALWAYS_LOAD_OPROM is only needed if you wish to load the OpRom even if it *won't* be executed. The OS can do this by itself for cards that have an actual ROM. The VBT is an Intel concept. Intel's IGD does not have a physical ROM, so the VBT/VBIOS has to be loaded from CBFS (what the OS drivers usually can't do).
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/12/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/12/src/soc/intel/icelake/Kcon... PS12, Line 210: select ALWAYS_LOAD_OPROM I am really sorry for getting back to this. But now only one Kconfig option is selected by this option. Can’t the ALWAYS_LOAD_OPROM description or name be improved, so that the new option is not needed anymore?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/12/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/12/src/soc/intel/icelake/Kcon... PS12, Line 210: select ALWAYS_LOAD_OPROM
I am really sorry for getting back to this. […]
i guess ALWAYS_LOAD_OPROM is very justified name to tell user that it will always load oprom without bothering if user would like to execute it or not.
and its choice of mainboard user to make a board design with oprom enable card/internal device to load that specifically. I don't think ALWAYS_LOAD_OPROM need a better name :)
only one thing might be in question, do we really need an another Kconfig ENABLE_DISPLAY_OVER_EXT_PCIE_GFX when its just matter of selecting ALWAYS_LOAD_OPROM, i believe answer would be, it might provide a better context for mainboard user to really know what we are doing or what we intended to do by selecting ENABLE_DISPLAY_OVER_EXT_PCIE_GFX config delicately
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33738/12/src/soc/intel/icelake/Kcon... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/33738/12/src/soc/intel/icelake/Kcon... PS12, Line 210: select ALWAYS_LOAD_OPROM
i guess ALWAYS_LOAD_OPROM is very justified name to tell user that it will always load oprom without […]
Ack
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33738 )
Change subject: soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig ......................................................................
soc/intel/icelake: Add ENABLE_DISPLAY_OVER_EXT_PCIE_GFX kconfig
This patch creates new kconfig option to bring display over external PCI based GFX card. This kconfig to select required kconfig which are not default selected by VGA_ROM_RUN to launch legacy oprom from pci based GFX card.
Change-Id: I8ebde69e38defbe3321eb5e5bbd632c209ae2cd8 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33738 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Wonkyu Kim wonkyu.kim@intel.com Reviewed-by: V Sowmya v.sowmya@intel.com --- M src/soc/intel/icelake/Kconfig 1 file changed, 11 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified V Sowmya: Looks good to me, approved Wonkyu Kim: Looks good to me, approved
diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 5dca44b..99000bb 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -205,4 +205,15 @@ depends on FSP_USE_REPO default "3rdparty/fsp/IceLakeFspBinPkg/Fsp.fd"
+config ENABLE_DISPLAY_OVER_EXT_PCIE_GFX + bool "Enable display over external PCIE GFX card" + select ALWAYS_LOAD_OPROM + help + It's possible to bring display through external graphics card over PCIE + in coreboot. This option enables graphics initialization with external + graphics card. + + Selected by mainboard that runs OpRom to perform display + initialization over attached PCIe GFX card. + endif
-
Angel Pons (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)
-
V Sowmya (Code Review)
-
Wonkyu Kim (Code Review)