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

[Jérémy Compostella (Code Review)]

Attention is currently required from: Alok Agarwal, Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Paul Menzel, Pranava Y N, Subrata Banik, Vikrant L Jadeja.

Jérémy Compostella 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 14:

(5 comments)

Patchset:

PS11:
> What’s the difference to the implementation for other SoC types?

The UPD are being used a bit differently, especially at the moment. `VgaInitControl`, for instance, used to be a bit field.

Commit Message:

https://review.coreboot.org/c/coreboot/+/85454/comment/79f47875_60a21f57?usp... :
PS11, Line 12: initialization
> Fits on the line above.

Indeed, I aligned my

https://review.coreboot.org/c/coreboot/+/85454/comment/8da3c729_53ce5132?usp... :
PS11, Line 47: The SOC_INTEL_PANTHERLAKE_SIGN_OF_LIFE flag also selects the LZ4
             : compression algorithm for the Video BIOS Tables (VBT), as LZMA
             : decompression is not available by default during the romstage
             : phase. Integrating LZMA support would increase the romstage binary
             : size by an amount greater than the reduction achieved by compressing
             : the VBT binary using LZMA.
> Please reflow for 72 characters per line.

Why 72-character ? According to https://doc.coreboot.org/contributing/gerrit_guidelines.html#commit-message-... this is up to 75-character.

File src/soc/intel/pantherlake/Kconfig:

https://review.coreboot.org/c/coreboot/+/85454/comment/4802956f_c7688aae?usp... :
PS11, Line 420: SOC_INTEL_PANTHERLAKE_SIGN_OF_LIFE
> can you please rely on https://review.coreboot.org/c/coreboot/+/83770 Kconfig.

Done

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

https://review.coreboot.org/c/coreboot/+/85454/comment/55475f79_6afb4e0b?usp... :
PS11, Line 375:
> how about someone still using CSE sync in romstage ? (like non-CrOS usecase). […]

This is not supported in the current design. I raised the point in December.
I requested a mechanism for the bootloader (coreboot) to control VGA exit. Once available, I will push a new CL and update this function.