[coreboot-gerrit] Change in coreboot[master]: lib/edid_fill_fb: Support multiple framebuffers

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39002 )

Change subject: lib/edid_fill_fb: Support multiple framebuffers
......................................................................

Patch Set 6:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39002/6/src/include/edid.h 
File src/include/edid.h:

https://review.coreboot.org/c/coreboot/+/39002/6/src/include/edid.h@107 
PS6, Line 107: struct fb_info;
> I don't see why exposing the internals helps here. […]

idk, is <framebuffer_info.h> supposed to be a less exposed header than <edid.h>? Looks to me like they were both public APIs. In particular, you can't actually do anything useful with the result of set_vbe_mode_info_valid() unless you #include that one too.

It's just that in general I think our policy in coreboot (and most other projects I've seen) is to just include the header that declares the thing you need, and never forward-declare or double-declare stuff. It just seems messy for no real benefit.

https://review.coreboot.org/c/coreboot/+/39002/6/src/lib/edid_fill_fb.c 
File src/lib/edid_fill_fb.c:

https://review.coreboot.org/c/coreboot/+/39002/6/src/lib/edid_fill_fb.c@172 
PS6, Line 172: 	for (struct fb_info *i = list; i != NULL; i = i->next) {
> A later patch will make use of all registered framebuffers.

Ack