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.

View Change

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I52288b0d5f33cd11d84e609e039dc4ea16ff7bdf
Gerrit-Change-Number: 34622
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra@intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar@intel.com>
Gerrit-Reviewer: Boon Tiong Teo <boon.tiong.teo@intel.com>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela@intel.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar@intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya@intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Joel Kitching <kitching@google.com>
Gerrit-CC: Lean Sheng Tan <lean.sheng.tan@intel.com>
Gerrit-Comment-Date: Tue, 30 Jul 2019 07:54:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment