Julius Werner has posted comments on this change. ( https://review.coreboot.org/19764 )
Change subject: lib/edid: Split out fill_lb_framebuffer() ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/19764/5/src/lib/Kconfig File src/lib/Kconfig:
Line 4: Include fill_lb_framebuffer() implementation that caches an EDID. Does this really need to be split out and guarded by a separate Kconfig? I always like to avoid boilerplate Kconfigs that need to be selected by a large class of boards/chipsets as much as possible.
Looks like you're later removing the weak no-op fill_lb_framebuffer(). Can't you then just make the EDID one the new weak default? Looks like all chipsets that don't use the EDID code define this in their chipset-specific code, so it would make sense as a default.
https://review.coreboot.org/#/c/19764/5/src/mainboard/google/oak/Kconfig File src/mainboard/google/oak/Kconfig:
Line 38: select EDID_FB_HELPER Should be selected by the SoC. Even though the entry point for display functions is in mainboard code here, all the core display init code (that's based on EDID) is in the SoC directory.
(Also applies to daisy/exynos5250 and peach_pit/exynos5420, I think.)