Attention is currently required from: Alok Agarwal, Anil Kumar K, Intel coreboot Reviewers, Jayvik Desai, Julius Werner, 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/9a968aef_41f61910?usp... : PS21, Line 387: m_cfg->VgaMessage = (efi_uintn_t)ux_locales_get_text(UX_LOCALE_MSG_MEMORY_TRAINING);
Would https://review.coreboot.org/c/coreboot/+/86532 be acceptable ?
I'm planning to take step approach here by implementing https://review.coreboot.org/c/coreboot/+/86509/1 and then move the SoC UX into basecode. Now we would have two kinds of UX implementation (for ADL using libgfxinit and MTL onwards using uGOP).
Note, we need some special handling like https://review.coreboot.org/c/coreboot/+/86509/1/src/soc/intel/pantherlake/r... to avoid overlapping text message due to low-battery w/ MRC update in pipeline
At this moment, I would prefer to land the SoC changes as is and take the refactoring with upcoming CLs. let me you @julius has any concerns
Additionally, moving common things based on FSP-M UPD names is a terrible idea because there's no guarantee these names will stay the same across future SoC generations. Any change in the UPD name would prevent us from using this common layer of code. The FSP team can't guarantee they'll keep using the same UPD names.
I would strongly recommend to keep UPDs inside SoC layer and use helper implementation like UX.c/.h to allow sharing the code across different use case like low-battery, updates etc.
``` m_cfg->VgaInitControl = vga_init_control; m_cfg->VbtPtr = (efi_uintn_t)vbt; m_cfg->VbtSize = vbt_size; m_cfg->LidStatus = CONFIG(VBOOT_LID_SWITCH) ? get_lid_switch() : CONFIG(RUN_FSP_GOP); m_cfg->VgaMessage = (efi_uintn_t)ux_locales_get_text(UX_LOCALE_MSG_MEMORY_TRAINING); } ```