Attention is currently required from: Tim Wawrzynczak, Angel Pons, Alexander Couzens, Patrick Rudolph. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51332 )
Change subject: soc/intel/skylake: detect if IGD is present ......................................................................
Patch Set 7:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51332/comment/e62979fc_c8e822ab PS7, Line 10: FSP-S will hang indefinitely. explain how this is actually fixed, please
https://review.coreboot.org/c/coreboot/+/51332/comment/2c9e2cf7_28dfdea2 PS7, Line 13: x11ssh-ln4f you can add x11ssm-f, too, here. I've verified the bug and your fix there
File src/soc/intel/skylake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/51332/comment/a895598c_b2b0af67 PS7, Line 119: igd = pci_read_config16(SA_DEVFN_IGD, PCI_VENDOR_ID) == PCI_VENDOR_ID_INTEL; just a side note: actually, the check isn't required here, since FSP checks if IGD is fused disabled and skips graphics init
https://review.coreboot.org/c/coreboot/+/51332/comment/75568120_0abdc8ca PS7, Line 117: dev = pcidev_path_on_root(SA_DEVFN_IGD); : if (dev && dev->enabled) { : igd = pci_read_config16(SA_DEVFN_IGD, PCI_VENDOR_ID) == PCI_VENDOR_ID_INTEL; : this is used twice - here and in systemagent.c. what about moving it to a helper function to not repeat yourself? :)
File src/soc/intel/skylake/romstage/systemagent.c:
https://review.coreboot.org/c/coreboot/+/51332/comment/5d0c45c8_149950eb PS7, Line 31: : if (igd_dev && igd_dev->enabled && : pci_read_config16(SA_DEVFN_IGD, PCI_VENDOR_ID) == PCI_VENDOR_ID_INTEL) : sa_set_mch_bar(&soc_gfxvt_mmio_descriptor, 1); : there's another occurence of this in src/soc/intel/skylake/systemagent.c, which probably should be guarded as well