Attention is currently required from: Andrey Petrov, Matt DeVillier, Ronak Kanabar, Subrata Banik, Tim Van Patten.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82121?usp=email )
Change subject: drivers/intel/fsp2_0: Release bmp_logo during READY_TO_BOOT stage ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
what if any platform decide to not call into fsp notify phases like intel platform starting from adl? in that case, we won't be releasing the bmp logo?
notify.c is enabled in ramstage for all platforms. FSP notify phase is skipped at run-time for ADL and MTL. Even there, bmp logo buffer is released since the release operation is done outside the fsp_notify.
can we do the bmp logo release as part of the finalize phase rather trying to bind it against fsp callbacks?
Each SoC has soc_finalize callback in Intel platforms whereas AMD has a common soc_finalize callback. This scatters bmp_release_logo throughout the code-base and there are chances where we may forget to release it on certain SoCs.
How about registering a callback in silicon_init.c that gets called during BS_PAYLOAD_LOAD as follows:
``` static void release_logo(void *arg_unused) { if (CONFIG(BMP_LOGO)) bmp_release_logo(); } BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_LOAD, BS_ON_EXIT, release_logo, NULL); ```
This keeps it contained within FSP related code-base and easy to follow through in the future.