Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48858 )
Change subject: soc/intel/apollolake: Fix FSP/GOP display init ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patch Set 1:
(1 comment)
Patch Set 1: Code-Review+2
Maybe bailing out is the best idea after all. If somebody cares about correct configuration when FSP/GOP is used, they can have a deeper look into it (we just don't know yet which registers to configure before/after the GOP runs).
Maybe leave a comment about that? e.g.
/* In case of RUN_FSP_GOP, the graphics hardware might be enabled already. Proper configuration around FSP/GOP needs further investigation. */ if (CONFIG(RUN_FSP_GOP)) return;
As discussed on IRC:
We do not know enough about GOP (missing source) and programming settings after the panel was enabled is not the right way. Thus, ack for this patch. We should bail out on all platforms.
https://review.coreboot.org/c/coreboot/+/48858/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48858/1//COMMIT_MSG@8 PS1, Line 8: : Configuring the panel power and backlight for an internal display : when using FSP/GOP display init causes FSP to turn the backlight off
this should be fixed by correcting register programming (it was overridden and panel set to off. […]
As discussed on IRC:
We do not know enough about GOP (missing source) and programming settings after the panel was enabled is not the right way. Thus, ack for this patch. We should bail out on all platforms.
CB:48881 will be abandoned for the same reason