[coreboot-gerrit] Change in coreboot[master]: lib/edid: Split out fill_lb_framebuffer()

[Julius Werner (Code Review)]

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.)


-- 
To view, visit https://review.coreboot.org/19764
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I90634b835bd8e2d150b1c714328a5b2774d891bd
Gerrit-PatchSet: 5
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes