[M] Change in coreboot[master]: soc/intel/alderlake: Add romstage early graphics support
Attention is currently required from: Bora Guvendik, Anil Kumar K, Hannah Williams, Cliff Huang, Paul Menzel, Angel Pons, Arthur Heymans, Tarun Tuli, Nico Huber, Michał Żygowski, Subrata Banik, Swift Geek (Sebastian Grzywna), Elyes Haouas.
Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70303 )
Change subject: soc/intel/alderlake: Add romstage early graphics support ......................................................................
Patch Set 44: Code-Review+1
(6 comments)
File src/soc/intel/alderlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/70303/comment/f9b1c95d_2248bfc3 PS33, Line 209: if EARLY_GFX_GMA : : config GFX_GMA_DEFAULT_MMIO : default 0x81000000 : : endif # EARLY_GFX_GMA
Would this work? […]
Done
File src/soc/intel/alderlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/70303/comment/f0fbebb1_7fdd3d62 PS42, Line 211: 0xfa000000
- It isn't directly straightforward - I agree. […]
Done
https://review.coreboot.org/c/coreboot/+/70303/comment/3ed4f82d_7868c449 PS42, Line 211: 0xfa000000
this is from the logs we had dumped in romstage sometime back […]
Done
File src/soc/intel/alderlake/romstage/graphics.c:
https://review.coreboot.org/c/coreboot/+/70303/comment/437a54bd_f0f55853 PS33, Line 21: /* Attempt to unlock memory */ : wrmsr(MSR_UNLOCK_MEMORY, msrval);
Probably but outside this patch. […]
Done
https://review.coreboot.org/c/coreboot/+/70303/comment/ec7f6b3e_6ce219b4 PS33, Line 37: unlock_memory();
Also, have you thought of a scenario where user still would like to keep TXT support enabled and don't wish to disable it (like what the code is doing now) but would like to show a video msg. Seems like both are exclusive to each other.
The Alder Lake / Raptor Lake FSP-M seems to unconditionally unlock memory. Looks like this use-case is already going to be conflicting with FSP-M.
What is the value of reading TPM base address and unlock the LT memory on the SKUs where TXT itself is fuse disable. Shouldn't we save some cycle there by adding a check?
Good question: I don't know which one of the extra cpuid or the blank MSR unlock is going to take more cycles. I am under the impression this should not make a major difference though.
File src/soc/intel/alderlake/romstage/graphics.c:
https://review.coreboot.org/c/coreboot/+/70303/comment/8e4e8fd7_4cf1f27c PS39, Line 20: read8p(TPM_BASE_ADDRESS);
@Subrata, I also went through the FSP code and I experimented around it to arrive to the point we ar […]
Done
-
Jérémy Compostella (Code Review)