Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 11:
(5 comments)
Thanks for the review. Move to lib/bootsplash.c for good.
https://review.coreboot.org/c/coreboot/+/34537/10/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/10/src/device/oprom/realmode/... PS10, Line 359: (unsigned char *)mode_info.vesa.phys_base_ptr;
Either this should have le32_to_cpu() or the two le16_to_cpu() above should be removed, for consiste […]
I agree. Changed to le32_to_cpu
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 58: #if CONFIG(BOOTSPLASH)
No guards required. But I would appreciate a more thorough fix.
Removed. Could you explain briefly, if you have the time, so i can improve in the future?
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 750: #if CONFIG(BOOTSPLASH)
This is what I consider the most "broken" part calling for attention. […]
Ahh! Now i get what you mean. Hopefully I did exactly that :)
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 758: int ret = oprom_set_bootsplash(framebuffer, x_resolution, y_resolution, fb_resolution)
Missing ; at end of line?
Yep. Thanks
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 760: DEBUG_PRINTF_VBE("Could not find bootsplash.jpg\n");
Strongly recommend braces here. Probably does not build currently.
Done