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 14:
(5 comments)
LGTM after outstanding comments
https://review.coreboot.org/c/coreboot/+/39002/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39002/4//COMMIT_MSG@13 PS4, Line 13: for a single graphics driver there's no change in functionality.
The goal is building a framework to fully use multiple displays at the same time. […]
Ack
https://review.coreboot.org/c/coreboot/+/39002/14/src/include/edid.h File src/include/edid.h:
https://review.coreboot.org/c/coreboot/+/39002/14/src/include/edid.h@95 PS14, Line 95: #include <framebuffer_info.h> At the top with the other includes, please.
https://review.coreboot.org/c/coreboot/+/39002/11/src/include/framebuffer_in... File src/include/framebuffer_info.h:
https://review.coreboot.org/c/coreboot/+/39002/11/src/include/framebuffer_in... PS11, Line 39: DIV_ROUND_UP(bpp, 8) / 8
Started cleaning up the 24bpp mess here: CB:40798 […]
Hmm... okay, I think you're right, the bootsplash code actually seems to be written with true 24bpp in mind. It's just the code in set_vbe_mode_info_valid() that has always been wrong then (where case 32 and case 24 map to the same thing). It looks like no platform actually sets bpp to 24 so we're probably good there (all the ones that don't explicitly call edid_set_framebuffer_bits_per_pixel() use the 32 default hardcoded in the EDID decoder, I think).
https://review.coreboot.org/c/coreboot/+/39002/14/src/lib/edid_fill_fb.c File src/lib/edid_fill_fb.c:
https://review.coreboot.org/c/coreboot/+/39002/14/src/lib/edid_fill_fb.c@116 PS14, Line 116: fb_new_framebuffer_info() Comment slightly out of date
https://review.coreboot.org/c/coreboot/+/39002/14/src/lib/edid_fill_fb.c@131 PS14, Line 131: bits_per_pixel Needs to be hardcoded to 32 until you fix the FIXME above.