Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33844 )
Change subject: src/security/vboot: Add option to skip display init with vboot 2.0 ......................................................................
Patch Set 7:
Hi folks,
First of all, sorry for the breakage. I didn't know that our Kconfig actually has an error-warning for unmet dependencies, I thought it just ignores them. If so many boards are using vboot in different ways now, it would be great to make Jenkins aware of that so we can find these things before patches go in. (I think you just need to submit a config into $(top)/configs?)
The purpose of this change was to keep supporting the different modes that Chrome OS needs as well as the new stuff Intel wants to do with vboot. You're right that the commit message is just wrong, should've read that more closely. Essentially, we're trying to support all the following things:
1. Chrome OS boards want to skip display init when they're in normal (non-dev, non-recovery) mode, for boot speed. 2. Only some Chrome OS boards actually *can* skip display init (those that call the display_init_required() function from SoC/mainboard code and act differently based on the result). For those that cannot, it's beneficial that vboot knows the display is on anyway (that avoids an extra reboot to "turn the display on" in some cases). 3. Intel wants to use vboot but wants the display to be always on, no matter what. 4. Chrome OS and Intel use cases may share the same SoC code (i.e. the part that wants to call display_init_required() and act differently based on the result) and having a CONFIG_CHROMEOS check in there would be ugly.
So we came up with two Kconfigs: 1. VBOOT_MUST_REQUEST_DISPLAY has been there for a long time (used to be called VBOOT_OPROM_MATTERS, which was a somewhat outdated name for the same thing) and essentially means VBOOT_CAN_SKIP_DISPLAY_INIT (I argued with Joel a lot about which name would be clearer for that one, maybe we picked wrong). This means that the mainboard/SoC is technically capable of skipping display init (meaning that it calls display_init_required() somewhere). This one just depends on what the mainboard/SoC code does and is therefore not user-selectable. 2. VBOOT_MAY_SKIP_DISPLAY_INIT is the new one for Intel's use case. That one is supposed to decide whether we actually *want* to skip display init in any case, or just always init unconditionally. I thought that Chrome OS case is sort of a special case (that we know that our payload never needs the display in normal mode), and initializing the display has always been the normal default for coreboot, so I thought that defaulting this to false makes more sense. It's a thing the user needs to decide based on what their payload wants to do, that's why it is user selectable.
We use CONFIG_CHROMEOS here (and in plenty of other cases) to change behavior as a convenience. If we know that we want a Kconfig to be a certain way for all our devices, doing it this way is much easier than having to track this in our custom defconfig for every single board (and having it there doesn't really hurt anyone else, so I hope you guys are okay with that practice). This wasn't intentionally trying to break things for every non-Chrome OS board or anything, of course... just a patch that changes default behavior (for what I believed to be good reasons that should make sense for most), that patch being unfortunately broken in a way none of us noticed, and independent from that Chrome OS preferring the other (now non-standard) behavior. (Note that I did CC Philipp to try to make sure every vboot-using group I'm aware of had a chance to see the change.)
So I think the easiest fix would be to reupload the patch without the 'depends on' line and that should do the right thing everywhere. The alternative would be to switch all mainboard/SoC Kconfigs to control this via default rather than select, or to qualify the select with an 'if VBOOT_MAY_SKIP_DISPLAY_INIT'. But that just adds extra boilerplate to all those files so I think the former solution is probably better.
I'll also take a look at Nico's suggestion.