[S] Change in coreboot[main]: soc/intel/pantherlake: Display Sign-of-Life during memory training

[Subrata Banik (Code Review)]

Attention is currently required from: Alok Agarwal, Anil Kumar K, Intel coreboot Reviewers, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Paul Menzel, Pranava Y N, Vikrant L Jadeja.

Subrata Banik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/85454?usp=email )

Change subject: soc/intel/pantherlake: Display Sign-of-Life during memory training
......................................................................

Patch Set 21:

(1 comment)

File src/soc/intel/pantherlake/romstage/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/85454/comment/a4a5d1ad_8950f7c4?usp... :
PS21, Line 387: 	m_cfg->VgaMessage = (efi_uintn_t)ux_locales_get_text(UX_LOCALE_MSG_MEMORY_TRAINING);
> Combining the comment from a similar discussion here: 
> https://review.coreboot.org/c/coreboot/+/86452/comment/84dc3ced_2fc8ae13/
>
> I acknowledge there are 2 problems which are coming in the way of common code:
> UPD name changes during each generation of SoCs
> New UPD requirement for ESOL in the future.
>
> Those problems can be addressed through one or more of the following:
> fill_base_esol_fsp_params() that are common/base to all SoCs. We can provide the default implementation in the common code. We can communicate to Intel and ask them to not change the name for these UPDs. If the UPD names change even after that, then we can provide an override to fill_base_esol_fsp_params() in the SoC specific way.
> Add soc_fill_additional_esol_fsp_params() to support new UPDs. The default implementation does nothing, but the SoC specific UPDs can be filled by the concerned code. Again we can do this when we have to cross that bridge.
> Versioning scheme as suggested earlier by Julius.
>
> Even if the suggested improvements does not stand the test of time, they will definitely reduce code and logic duplication

There are at least two angles to this request to create common code using eSOL:

1. Julius wants to leverage the code between MTL and PTL because the number of UPDs is the same in both eSOL implementations. Also, the names look the same at this point.

2. Karthik wants to avoid using #if/#endif guards in each SoC code base that implements an override for platform_display_early_shutdown_notification. Instead, he suggests using common code like ux_inform_user_of_poweroff_operation, which takes an FSP UPD pointer and updates the required UPDs. Currently, the SoC override might exist in each SoC and circle back to the #1 angle.

There are few reasons why we can't implement a common layer:

In the IA common code model, we intentionally avoided making common code based on FSP-UPD because the FSP team never guaranteed to use the same UPD names across multiple SOC generations.
    - UPD names can change
    - New UPDs may be added
    - UPDs can be divided based on SoC IP block, like `VgaInit`, which is now part of `m_cfg->VgaInit`, can be implemented in FSP as a nested data structure like `m_cfg->PreMemDisplay->VgaInit`.

this idea of implementing common code based on the UPD variable names:

- The common block might need more overrides or #if/#endif guards to handle different SoC variations.
- Creating a folder structure as suggested by Julius could solve the problem, but it wouldn't be any better than the existing proposal, where each SoC layer has its dedicated `platform_display_early_shutdown_notification` API implementation.
- We might end up with multiple versions of the common block (e.g., `fsp_esol_2024`, `fsp_esol_2025`, `fsp_esol_2026`, etc.) as it evolves each year, which would require us to move unnecessary folders into the common block.
- The volatile SoC block was never designed to be moved into IA common code.
- If the guideline is to implement common code based on UPD name commonality, there are many better candidates than just eSOL.

Overall, I believe that trying to common things based on UPD variable names is not a good idea, and I would strongly oppose it.

As suggested by Jeremy, there are ample code blocks that exist into each SoC block which can be revisited to make it common. I would like to emphasize that need rather using common code for eSOL.