[coreboot-gerrit] [XS] Change in coreboot[main]: drivers/intel/fsp2_0: Release bmp_logo during READY_TO_BOOT stage

[Subrata Banik (Code Review)]

Attention is currently required from: Andrey Petrov, Karthik Ramasubramanian, Matt DeVillier, Ronak Kanabar, Tim Van Patten.

Subrata Banik 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 is a more reasonable approach to me.
> This keeps it contained within FSP related code-base and easy to follow through in the future.