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:
(3 comments)
- It removes the safety-check ...
I have a more thorough API in mind to check if some gfx init should be run or not. I'll start a discussion on the ML when I have the time. I'd hope that display_init_required() would be harder to miss when it's part of a bigger story (being able to handle multiple active gfx drivers and decide which one should be active etc.).
- It doesn't do the right thing for devices that don't
have a display at all...
Thanks for elaborating. Hidden feature that is easy to miss, indeed. CONFIG_LINEAR_FRAMEBUFFER should be the right thing. Just one question, is it `VB2_CONTEXT_DISPLAY_INIT` that is passed on or `VBOOT_WD_FLAG_DISPLAY_INIT`?
https://review.coreboot.org/c/coreboot/+/34622/1/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/34622/1/src/device/Kconfig@24 PS1, Line 24: config HAVE_VBE_LINEAR_FRAMEBUFFER
A bit off-topic, but since I just looked through this again... […]
Difference is in Kconfig texts presented to the user in the "Framebuffer mode" choice below. In common code, only CONFIG_LINEAR_FRAMEBUFFER should matter.
https://review.coreboot.org/c/coreboot/+/34622/1/src/drivers/aspeed/ast2050/... File src/drivers/aspeed/ast2050/Kconfig:
https://review.coreboot.org/c/coreboot/+/34622/1/src/drivers/aspeed/ast2050/... PS1, Line 10: select VBOOT_FORCE_DISPLAY_INIT if MAINBOARD_DO_NATIVE_VGA_INIT
Since we want to set the option when display init is just never enabled at all too, I think you don' […]
Welcome to the messy PC world :) That a board has this chip, doesn't mean it's the only one that might have a display attached. Other code that is effective at runtime might still query display_init_ required().
https://review.coreboot.org/c/coreboot/+/34622/1/src/mainboard/google/oak/Kc... File src/mainboard/google/oak/Kconfig:
https://review.coreboot.org/c/coreboot/+/34622/1/src/mainboard/google/oak/Kc... PS1, Line 49: select VBOOT_FORCE_DISPLAY_INIT
edit: Okay, I remember now that Oak's display init starts from the mainboard. […]
No, you are right. Oak calls display_init_required(), just that the mainboard code does that and the soc Kconfig selects VBOOT_MUST_REQUEST_DISPLAY is confusing.