Hi

> We propose to take advantage of a proprietary driver Intel already supports, validates and includes in FSP silicon: the Intel Graphics PEIM (Pre-EFI Initialization Module) driver also known as the GOP (Graphical Output Protocol) driver.

Usually the reasoning for using a binary is because the hardware cannot be publicly documented (e.g. DRAM controller) or because they are cryptographically signed.
That is however not the case for Intel display controllers as typically both code (Intel i915 driver) and documentation exist.
Maybe it's just marketing, but I was under the impression that Intel is actively promoting open source on the graphics side with initiatives as oneAPI.

I think allowing binary only PEI modules just because they exist and are supported by the vendor is a very slippery slope.
The same argument could be applied to pretty much everything (just include your code doing X as a PEI module), which goes against the project goals of coreboot being an open source project.
This is of course not part of your proposal but I'm cautious of it.

To nuance this, other vendors only provide a proprietary VBIOS as an option for graphic init so it's not particularly worse.

> 2.1. PEI services

> µGOP depends on a limited of PEI services:

  1. InstallPpi() to install the PEIM Graphics PPI
  2. LocatePpi() to access PEIM-to-PEIM Interface (PPI) Dependencies
  3. AllocatePool() to dynamically allocate memory to handle internal data structure such as display information …
  4. GetHobList() and CreateHob() to access Hand Off Blocks (HOB) holding runtime data
  5. ReportStatusCode() to report debug information which coreboot prints using printk.

> Those services implemented in coreboot are pretty straightforward and fit in less than 300 lines of code.

That looks like some form of linking, which might result in legal troubles as the GPL does not allow it.
We already have code similar to ReportStatusCode and ramstage PPI so maybe it's not a problem.

On a technical note: only coreboot's ramstage has a heap. Romstage has less resources available so we avoided using a heap thus far. AllocatePool would break that tradition. How does one know how much heap is needed? It's best to avoid memory allocations at runtime.


> We also noticed that µGOP is faster to bring-up graphics than libgfxinit. Indeed, according to previously captured numbers on Raptor Lake compared to some number of µGOP on Meteor Lake, µGOP is three times faster to bring up graphics than libgfxinit on an eDP panel (119 ms vs 373 ms).

Is this relevant? I was under the impression that the display is only used to notify the user of a very long DRAM init, where regular boots use cached results. If the dram init is orders of magnitude longer than the display init this performance difference is meaningless.

> it is compatible with our software convergence goals

Can you elaborate on this?


Thanks for taking the time to clearly present your arguments!

Arthur


On Tue, Aug 22, 2023 at 3:35 PM Nico Huber <nico.h@gmx.de> wrote:
Hi Jeremy,

On 14.08.23 22:52, Compostella, Jeremy wrote:
> We propose to take advantage of a proprietary driver Intel already supports, validates and includes in FSP silicon: the Intel Graphics PEIM (Pre-EFI Initialization Module) driver also known as the GOP (Graphical Output Protocol) driver.

just to make sure nobody makes wrong assumptions: Will the uGOP be
open-source or proprietary as well? I first thought the latter. But
your proposed code-flow looks like some sort of dynamic linking with
coreboot.

> We intend to keep providing such a binary base solution on the long run as it addresses our software convergence goals and is compatible with early platform development stage constraints. [libgfxinit] supports can always be added later by the open-source community once the Graphics Programmer Reference Manuals are published.

Sad to hear about this decision. It seems Intel is forgetting about
non-consumer products (e.g. embedded market) where the code isn't
needed years ahead of a platform launch.

> We also noticed that microGOP is faster to bring-up graphics than libgfxinit. Indeed, according to previously captured numbers on Raptor Lake compared to some number of microGOP on Meteor Lake, microGOP is three times faster to bring up graphics than libgfxinit on an eDP panel (119 ms vs 373 ms).

Configuring the hardware and bringing up the eDP link should take
about 20~30ms mostly depending on how long it takes to read the
EDID. The longer delays are likely about panel power sequencing.
IIRC, libgfxinit falls back to hardcoded default values if the
sequencer is unconfigured, while the GOP just leaves it like that.
Chromebooks often skip the configuration[1] in firmware and leave
it to the OS driver. Using wrong delays probably doesn't hurt on
a rare interactive boot. However, I guess doing this on regular
boots might not be the best idea.

Nico

[1] https://doc.coreboot.org/gfx/display-panel.html

_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-leave@coreboot.org