Kyösti Mälkki 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 16:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/Makefile.inc File src/device/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/Makefile.inc@51 PS15, Line 51: ramstage-$(CONFIG_BOOTSPLASH) += ../lib/bootsplash.c You can just modify lib/Makefile.inc instead, jpeg.c is probably there already.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... PS15, Line 225: int mode_info_valid; I think you can declare those static, and do:
const vbe_mode_info_t *vbe_mode_info(void) { if (!mode_info_valid) return NULL; return &mode_info; }
The name mode_info is sort of ugly to have in global namespace.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/yabel/vbe... PS15, Line 722: } Like before.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@77... PS15, Line 772: #if CONFIG(BOOTSPLASH) I would move the details to lib/bootsplash.c leaving only following here:
if (CONFIG(BOOTSPLASH)) show_bootsplash();
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@78... PS15, Line 788: timestamp_add_now(TS_OPROM_END); Would be more correct to render image after TS_OPROM_END.
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h@105 PS15, Line 105: #if CONFIG(FRAMEBUFFER_SET_VESA_MODE) Generally such guards are only used when it does not build without them.
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h@107 PS15, Line 107: int vbe_mode_info_valid(void); There was suggestion to return vbe_mode_info_t pointer instead.