[M] Change in coreboot[master]: soc/intel/alderlake: Add romstage early graphics support

[Subrata Banik (Code Review)]

Attention is currently required from: Bora Guvendik, Anil Kumar K, Hannah Williams, Cliff Huang, Jérémy Compostella, Paul Menzel, Angel Pons, Arthur Heymans, Tarun Tuli, Nico Huber, Michał Żygowski, Swift Geek (Sebastian Grzywna), Elyes Haouas.

Subrata Banik 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 46:

(2 comments)

File src/soc/intel/alderlake/Kconfig:

https://review.coreboot.org/c/coreboot/+/70303/comment/76acf29b_adc37c83 
PS42, Line 211: 0xfa000000
> I understand but the change you have recommended, libgfinit does not run anymore in both romstage and ramstage while before your suggestion it was working.
>
> Let me level set one more time by sending you again the following summary as I feel like your question revealed that you may have missed one point.
>
> The  `CONFIG_GFX_GMA_DEFAULT_MMIO` configuration flag is originally a configuration flag consumed by the `libgfxinit` and it is used when the libhwbase driver is used.
>
> The `libhwbase` has two MMIO drivers:
> **static**: a base address must be provided at compile time (cf. `CONFIG_HWBASE_STATIC_MMIO`)
> **dynamic**: the driver get the information from the PCI device BAR0 (cf. `HWBASE_DYNAMIC_MMIO`)
>
> The most flexible and convenient driver is the **dynamic** one. Unfortunately, its compilation for `libgfxinit` generates global initialized variables which are not supported in cache-as-ram (cf. `src/arch/x86/car.ld`).
>
> Hence, we have to use the **static** driver in `romstage`. Since this `CONFIG_GFX_GMA_DEFAULT_MMIO` is also consumed by `libgfxinit/libhwbase`, if we use `libgfxinit` in both `romstage` and `ramstage`, it has to be set to a value which is working for both `romstage` and `ramstage`. Also we do not have `libhwbase` separated configuration flag between `romstage` and `ramstage` and as a result both `romstage` and `ramstage` version of `libhwbase` have to use the same driver.
>
> The problem with using a temporary MMIO region as you suggested is that it makes the `romstage` + `ramstage libgfxinit` use-case complicated to support because FSPs reset BAR 0 and more importantly the coreboot PCI resource allocator is setting another one (`0x81000000`).
>
> I raised this concern a month ago on the coreboot mailing list and I got only one feedback which was to use a simple Kconfig to supply this address statically and it is what I have implemented. The nice thing is I did not have to introduce any new configuration flag as `CONFIG_GFX_GMA_DEFAULT_MMIO` is already mandatory for `libgfxinit` with the **static** `libhwbase` driver. I am just extending its usage to other places in coreboot.
>
> The simplest way to keep it simple is to supply the same MMIO region than the one the PCI resource allocator allocates at ramstage.
>
> If we use another temporary one, some hack will have to be introduced in `soc/intel/common/block/graphics/graphics.c`, to switch temporarily to this other MMIO region.
>
> I already exposed this issue on the mailing list nearly two month ago and I unfortunately did not get tons of feedback. I am glad you are also looking at it now :)
>
> So this dynamic driver already exist but if we cannot use it in romstage as we do not have global dynamic support.
> > This is expected, we can't blindly assign any address (from future aka ramstage) while we are still on romstage.
>
> This is not really blindly as this is specified by a platform specific configuration flag.
> > We don't have SRAM on intel platform.
>
> I don't get your point here. Why are talking about SRAM ?

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

https://review.coreboot.org/c/coreboot/+/70303/comment/9f7cfa4b_742d94a0 
PS46, Line 427: 	m_cfg->GttMmAdr = CONFIG_GFX_GMA_DEFAULT_MMIO;
for user without LIBGFXINIT config being selected this value could be 0.

https://review.coreboot.org/c/coreboot/+/70303/46/src/soc/intel/alderlake/Kc...

ideally u can use the config to guard and override the value otherwise relying on FSP default.