Patch Set 3:

Patch Set 3:

Patch Set 3:

Is this for mainboards that have both integrated and discrete
graphics onboard? or only discrete graphics?

Very nice question Nico, i was about to come to this disucssion.
We are looking for two mainboard design

1. only discrete GFX over PCIE (no IGD)=> This i'm could able to achieve easily using MAINBOARD_HAS_DGPU_VGA_INIT and MAINBOARD_NO_FSP_GOP Kconfig select from mainboard.

I guess MAINBOARD_HAS_DGPU_VGA_INIT is not even needed in this
case because VGA_ROM_RUN would be the only option.

I have laptops which only have a dGPU, so the only option is to run the VBIOS (or not, e.g. when using SeaBIOS). The iGPU PCI device is disabled in the devicetree.

Problem with our VGA_BIOS Kconfig is that, it expects to have VBIOS as part of coreboot rather probing PCIE card and load/execute VBIOS from there. Hence thought is adding a new Kconfig which will make things clear that mb has DGPU and VBIOS is inside that. coreboot don't need to pack VBIOS.

And for disabling IGD, we have FSP UPD to make IGD disable prior to PCI enumeration hence we can ignore devicetree PCI device if needed.


>
> Yes, if VGA_ROM_RUN is outside choice then we could select for case #1 here directly from mb kconfig and we don't need any new kconfig option like this.
>
> >
> > > 2. Both onboard (IGD) and discrete GFX (DGPU) and based on some switch change the display dynamically => This is problem at present as RUN_FSP_GOP and VGA_ROM_RUN both are part of choice option hence couldn't able to select both from mainboard. Ideally for this type of mainboard design, we need to have both RUN_FSP_GOP and VGA_ROM_RUN enable in Kconfig and coreboot decide based on switch position if like to call FSP for IGD display or load/execute VGA OpROM for DGPU init. Right now, i'm commenting choice option in my local repo and selecting both Kconfig to make this dynamic display possible.
> >
> > The choice is a bit obsolete now. Originally, we could not
> > select multiple options because of linking errors. Now, since
> > Patrick's topic:framebuffer_info, we can actually build
> > multiple drivers for grapchics initialization at once.
> >
> > Maybe it's time to get rid of the choice?
>
> yes, i think that is the need for now, because board design need more option.

One example of this case is a desktop mainboard. To choose a primary video adapter, the usual behavior is to use a dGPU if present, otherwise fall back to the iGPU. The `if present` part could easily be determined after coreboot does PCI enumeration, but FSP-S gets executed before that point... I know some blobs (Haswell MRC.bin) do some sort of PCI enumeration in romstage to detect whether a dGPU is present, but IMHO it's a rather crude approach. Still, it's better than nothing. Any other ideas are welcome.

Another thing to consider: we need to keep the functionality of the ONBOARD_VGA_IS_PRIMARY Kconfig setting. This is useful when a dGPU is present but coreboot should still use the iGPU as primary video adapter. On laptops with Nvidia Optimus or the AMD counterpart (AMD Enduro, IIRC), this option needs to be enabled, and should be selected from the mainboard's Kconfig. This case is easy to handle: ensure the iGPU is to be enabled, and always use it as primary video adapter. It shouldn't be necessary to run the VBIOS of the dGPU, but I have no idea.

If multiple drivers
are run, we'd end up with multiple framebuffers. But we could
add some more Kconfigs, e.g. "Only initialize primary graphics
adapter". If, in your case, the switch disables either GPU,
the other would automatically be primary I guess.

yes, default primary and HW switch makes IGD disable and DGPU enable via OpRom
>
> (There should still be a choice if we have multiple drivers
> for a single GPU, though, e.g. libgfxinit vs. the unnecessary
> FSP/GOP blob).

I agree. I feel the easiest approach is to add a new option for dGPU lightup, as the only choice is whether to have coreboot run the VBIOS or not. However, I fear we may need to touch Kconfig on all mainboards to only show the valid options for a given board. It's work, but it shouldn't be too hard. For example, `MAINBOARD_HAS_LIBGFXINIT` indicates the iGPU is usable.

View Change

To view, visit change 49016. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iecb2fcdb105af449bc20ad727759cdef17d5e376
Gerrit-Change-Number: 49016
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie@chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Stefan Reinauer <reinauer@chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Nico Huber <nico.h@gmx.de>
Gerrit-Comment-Date: Fri, 01 Jan 2021 04:47:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment