[coreboot-gerrit] [M] Change in coreboot[master]: drivers/intel/gma: Hook up libgfxinit in romstage

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

Attention is currently required from: Bora Guvendik, Zhixing Ma, Anil Kumar K, Hannah Williams, Cliff Huang, Tarun Tuli, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons, Arthur Heymans.

Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70276 )

Change subject: drivers/intel/gma: Hook up libgfxinit in romstage
......................................................................

Patch Set 20:

(3 comments)

File src/drivers/intel/gma/Kconfig:

https://review.coreboot.org/c/coreboot/+/70276/comment/58c4b620_5751518e 
PS13, Line 95: SOC_INTEL_ALDERLAKE
> Let's keep it like this for now.

Ack

https://review.coreboot.org/c/coreboot/+/70276/comment/49dff4d1_d92d9830 
PS13, Line 103: Graphic MMIO address
Thanks Arthur and Subrata for the information and pointers. This is quite interesting. I never realized that these two are actually "mixed-up".
> Note: here in VGA text mode, we are using 0xb8000 address for video FB in VGA mode. The only purpose of GTTMMADR is to program the panel specific registers (within 2MB range).

The list of registers touched goes beyond this subset of five register of course 😊.

Thanks again.

File src/drivers/intel/gma/Kconfig:

https://review.coreboot.org/c/coreboot/+/70276/comment/e85d0bfb_97a57b51 
PS15, Line 105: If
              : 	  libgfxinit is used for both romstage and ramstage this
              : 	  address should be the same than the one allocated by the PCI
              : 	  resource allocator in ramstage.
> For now, you can tell coreboot to report BAR0 as a fixed resource. […]

Thanks for this great suggestion. I did not realize how flexible the PCI device/driver framework is in coreboot 😊.

Unfortunately, it does not work as-is because:
1. The PCI BAR0 is not necessarily set at this point in time.
2. `IORESOURCE_ASSIGNED` amd `IORESOURCE_FIXED` are mandatory but make the resource allocator to skip setting this resource.

I had to rework your suggestion as follow (and it does not look as good anymore):
```
    if (CONFIG(HWBASE_STATIC_MMIO)) {
    	struct resource *res_bar0 = find_resource(dev, PCI_BASE_ADDRESS_0);
    	res_bar0->base = CONFIG_GFX_GMA_DEFAULT_MMIO;
    	res_bar0->flags |= IORESOURCE_ASSIGNED;
    	pci_dev_set_resources(dev);
    	res_bar0->flags |= IORESOURCE_FIXED;
    }
```

Once I am done reworking all these patches, I'll try to identify when is the configuration lost. I bet this is during FSP-S.

I still think having global initialized variable would be the best the solution. I experimented a little bit on Friday. I have promising results but this is going to take a little while to have something merge-able.