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 15:
(8 comments)
https://review.coreboot.org/c/coreboot/+/39002/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39002/14//COMMIT_MSG@17 PS14, Line 17: goal is
goals are
Done
https://review.coreboot.org/c/coreboot/+/39002/14//COMMIT_MSG@17 PS14, Line 17: advertisment
advertis*e*ment
Done
https://review.coreboot.org/c/coreboot/+/39002/14//COMMIT_MSG@18 PS14, Line 18: indepent
independent?
Done
https://review.coreboot.org/c/coreboot/+/39002/14//COMMIT_MSG@22 PS14, Line 22: it's
its
Done
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.
Done
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@78 PS14, Line 78: bits_per_pixel = bpp_mask;
I'm not confident about this one. Why not return NULL? If the fields […]
Done. What's the definition of bits_per_pixel? does it count gaps, too?
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
Done
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.
Done