Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39002 )
Change subject: lib/edid_fill_fb: Support multiple framebuffers ......................................................................
Patch Set 12:
(4 comments)
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 30: /*
Any reason this now slipped into this patch? Seems to me like it belongs in CB:39430.
Done
https://review.coreboot.org/c/coreboot/+/39002/11/src/include/framebuffer_in... PS11, Line 37: uint16_t bpp)
Since the use of this is pretty specific to VESA OPROMs, maybe it would be better to just make it ta […]
Done
https://review.coreboot.org/c/coreboot/+/39002/11/src/include/framebuffer_in... PS11, Line 39: DIV_ROUND_UP(bpp, 8) / 8
If bpp is 24 this will be 3 but we need it to be 4. […]
24bpp is a valid pixel format. If a driver advertises 24bpp, but set up 32bpp (XRGB8) framebuffer that's a driver bug. I'll try to get rid of bpp next, and use an enum instead.
https://review.coreboot.org/c/coreboot/+/39002/11/src/include/framebuffer_in... PS11, Line 43: /* find first set bit */
We have an __ffs() function for this... but I don't think this is the right way to do this anyway. […]
Done