Attention is currently required from: Paul Menzel, Tim Wawrzynczak, Subrata Banik, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49016 )
Change subject: device: Select VGA_ROM_RUN for non-FSP solution with PCIE DGPU ......................................................................
Patch Set 5:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/49016/comment/025ad6ba_c0aca6d6 PS5, Line 7: non-FSP solution It doesn't really matter if FSP is used or not (coreboot is not 100% FSP). "Select VGA_ROM_RUN for PCIe DGPU" would say more (AFAIK, we don't have any drivers for DGPUs in coreboot, hence that we don't have a driver would be implicit).
https://review.coreboot.org/c/coreboot/+/49016/comment/18282bb4_601bf907 PS5, Line 12: 1. For onboard graphics initialization, GFX PEIM module inside : FSP binary is taking care of graphics initialization based on : HAVE_FSP_GOP Kconfig option and input VBT file. Hence no need : to load/execute legacy VGA OpROM in order to initialize onboard GFX. : : 2. In case of non-FSP solution, platform needs to select VGA_ROM_RUN : Kconfig to perform graphics initialization for PCI-E based discrete : card through VGA OpRom (SoC or Mainboard user can't select VGA_ROM_RUN : directly because it's part of choice option). This is only background information and mostly wrong (wrt. coreboot as a whole). It ignores all open-source means of graphics initialization.
In case you want to keep it here, you could make 1. about "coreboot graphics drivers" in general. No need to highlight the niche FSP spare wheel here.
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/49016/comment/d87e175d_90ccc766 PS4, Line 53: discrete graphics card
I'll have to take a closer look. Let me try something.
It doesn't seem like we need the IGPU/DGPU distinction right now. Loading OpRoms from cards is the default, so we treat them the same currently.
Also, if we'd add individual options for IGPU/DGPU later, I'd still prefer to have a single one that just says `VGA_ROM_RUN` is the default (see also my comment on PS5, line 65).
Subrata, I guess it would be enough for now to update the help text, i.e. remove 2x "discrete".
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/49016/comment/43bd8609_993872ae PS5, Line 64: default NO_GFX_INIT if (VGA_BIOS || VGA_ROM_RUN_DEFAULT) && PAYLOAD_SEABIOS : default VGA_ROM_RUN if VGA_BIOS || VGA_ROM_RUN_DEFAULT Looking at this again, I realized that we should only have `VGA_ROM_RUN_DEFAULT` here and let `VGA_BIOS` select it. I can easily do that in a follow-up if you don't want to delay this change.