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 17:
(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.
THanks nice! thanks
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: […]
Using it! Thanks a lot!
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.
Done
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: […]
I disagree, as lib/bootsplash should be generic without the dependency of handling vesa related things.
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.
I agree. Changed
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.
Okay. Thanks
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.
Will use the function approach instead.