Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39454 )
Change subject: soc/intel/cannonlake: Steal no memory for disabled IGD ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39454/8/src/soc/intel/cannonlake/ro... File src/soc/intel/cannonlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39454/8/src/soc/intel/cannonlake/ro... PS8, Line 38: (dev && (pci_read_config16(SA_DEV_IGD, PCI_VENDOR_ID) == 0xffff))) {
Why is this extra read needed?
IMHO a device can be enabled in the devicetree but physically not be there. Correct me if I am wrong. If the CPU has no IGD the read will return 0xffff. Otherwise the FSP would crash if the device is enabled in the devicetree but the CPU has not IGD.
https://review.coreboot.org/c/coreboot/+/39454/8/src/soc/intel/cannonlake/ro... PS8, Line 38: dev
Isn’t that guaranteed by the `!dev` in the beginning?
See other message.
https://review.coreboot.org/c/coreboot/+/39454/8/src/soc/intel/cannonlake/ro... PS8, Line 163: tconfig->PanelPowerEnable = 0;
This is not mentioned in the commit message? Any URL for the FSP bug report?
https://github.com/IntelFsp/FSP/issues/49
I can add it to the commit message - But I doubt that this will be fixed.