Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34622 )
Change subject: vboot: Unify options to force display init ......................................................................
Patch Set 1:
I think this doesn't really do what we want and there's a bit of a misunderstanding what the options are supposed to mean, see my comment on CB:33844. VBOOT_MUST_REQUEST_DISPLAY means "on this platform, the display must be requested explicitly if you want it", or in other words "this platform is capable of skipping the display initialization if it is not needed".
I guess I have to work on the commit message. While the name might be odd (I got used to it already), the effect of VBOOT_MUST_REQUEST_DISPLAY is clear:
if (not "must request display") "report display enabled";
It does *not* mean that the display will always be on. (Maybe we can rename it to something that makes that more clear... we picked that name before Intel came up with VBOOT_MAY_SKIP_DISPLAY_INIT, with both of those side-by-side I agree that the distinction isn't very clear.)
The distinction is even less clear if we'd implement VBOOT_MAY_SKIP_ DISPLAY_INIT in vboot (and not around it), this way:
if (not "may skip display init") "force display enabled";
Because both translate to the same C code:
if (!CONFIG(...)) ctx.flags |= VB2_CONTEXT_DISPLAY_INIT;
And I think VBOOT_MAY_SKIP_DISPLAY_INIT should be implemented this way for roughly the same reasons that VBOOT_MUST_REQUEST_DISPLAY is implemented as it is: If vboot has the means to track the state of the display, we should keep that state tracking sync'ed with what core- boot does. And while we are at it, ALWAYS_RUN_OPROM should also be implemented this way, for the very same reason. While the three options don't do the same, yet, I think they all should simply control VB2_ CONTEXT_DISPLAY_INIT. That's what my change tries to do...
But I guess it goes to far at once. If that helps, I can break it down into smaller commits:
1. Replace VBOOT_MUST_REQUEST_DISPLAY with VBOOT_FORCE_DISPLAY_INIT (selected by the inverse set of gfx init drivers). 2. Give VBOOT_FORCE_DISPLAY_INIT a prompt (replaces VBOOT_MAY_SKIP_ DISPLAY_INIT). 3. Remove ALWAYS_RUN_OPROM.