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 22:
(7 comments)
https://review.coreboot.org/c/coreboot/+/39002/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39002/21//COMMIT_MSG@14 PS21, Line 14: +
But this isn't code? IMHO, the `+` here would work like a hyphen on compound adjectives, e.g. […]
Done
https://review.coreboot.org/c/coreboot/+/39002/21//COMMIT_MSG@15 PS21, Line 15: +
add spaces around +
Done
https://review.coreboot.org/c/coreboot/+/39002/21//COMMIT_MSG@17 PS21, Line 17: the advertisement : of
advertise
Done
https://review.coreboot.org/c/coreboot/+/39002/21/src/include/framebuffer_in... File src/include/framebuffer_info.h:
https://review.coreboot.org/c/coreboot/+/39002/21/src/include/framebuffer_in... PS21, Line 2: /* This file is part of the coreboot project. */
Please drop
Done
https://review.coreboot.org/c/coreboot/+/39002/21/src/lib/edid_fill_fb.c File src/lib/edid_fill_fb.c:
https://review.coreboot.org/c/coreboot/+/39002/21/src/lib/edid_fill_fb.c@96 PS21, Line 96: physical_address : fb_addr,
This is the old syntax for struct initializers. New syntax would be: […]
Done
https://review.coreboot.org/c/coreboot/+/39002/21/src/lib/edid_fill_fb.c@105 PS21, Line 105: 0
Is this supposed to be the same as `red_mask_pos` ?
no, done
https://review.coreboot.org/c/coreboot/+/39002/21/src/lib/edid_fill_fb.c@167 PS21, Line 167: //TODO: Add support for advertising all framebuffers in this list
TODO
That's fixed in the following patch. See CB:39005 for details.