Thanks for the explanation, I get what you're trying to do now. I think it should work except for two issues:

1. It removes the safety-check for "did someone forget to set the right option". The problem is that when someone creates a new mainboard, they may not know that they were supposed to select FORCE_DISPLAY_INIT because they don't use display_init_required() anyway. That may lead to unnecessary extra resets in some edge cases that are hard to notice. (Then again, I don't think we're going to make a lot more Chromebooks that init their displays unconditionally in the future, so if you think this makes things cleaner I'm willing to let go of that extra assertion.)
2. It doesn't do the right thing for devices that don't have a display at all. In order to keep the vboot API small, we have the slightly weird requirement that a device which doesn't have a display must pass DISPLAY_INIT in the context of vb2api_fw_phase1() to avoid those extra resets (the current code just does that because they would never select MUST_REQUEST_DISPLAY). Maybe there's some other Kconfig we could check in addition to determine whether there's a display at all? (Maybe HAVE_LINEAR_FRAMEBUFFER? But that option doesn't seem to be super well-maintained because it doesn't really do much... e.g. MT8183/GOOGLE_KUKUI doesn't seem to set it. If we wanted to rely on that it would be good to have an assertion that reminds people to select it from their SoCs... maybe in set_vbe_mode_info_valid()?)

View Change

3 comments:

• File src/mainboard/google/oak/Kconfig:

  • Patch Set #1, Line 49: select VBOOT_FORCE_DISPLAY_INIT

    This seems wrong? Oak can skip display init (used to be selected from mt8173/Kconfig). (I think some of the other cases may also be wrong, didn't check them all. In general, unless there's explicit display code at the mainboard level, this option should probably be set at the SoC level.)

• File src/security/vboot/Kconfig:

  • Patch Set #1, Line 165: initialization code is not run.

    We should document somewhere that boards/SoCs that do not call display_init_required() need to select this option.

• File src/security/vboot/vboot_logic.c:

  • Patch Set #1, Line 361: /* Mainboard/SoC always initializes display. */

    nit: comment could use updating (may be user-decided now)

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