Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35516 )
Change subject: device/pci: Ensure full 16-bit VGA port i/o decoding ......................................................................
Patch Set 1:
(2 comments)
So this would definitely break CardBus VGA adapters (if the bridges comply with their spec)... do we mind? I'd rather just mention it in the commit message.
https://review.coreboot.org/c/coreboot/+/35516/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35516/1//COMMIT_MSG@17 PS1, Line 17: these days, so nobody noticed.
Looks like this bit is only defined when upstream is PCIe and downstream is PCI/PCI-X/AGP(?), given […]
It's often reserved, read zero (including PC Card 8.0). It's in PCI-to-PCI bridge 1.2 and in PCIe-to-PCI/PCI-X 1.0.
Fun fact: VGA Enable (bit 3) seems to be only mandatory in the PC Card spec ;) so far, we just assume that it's present.
With slots, I guess you mean root ports? Yes, they are covered by the PCIe base spec. aaaaaaand, they only added the bits in 4.0 xD Anyway, if we'd have passed more than a decade without legacy VGA, we'd have noticed.
https://review.coreboot.org/c/coreboot/+/35516/1/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/35516/1/src/device/pci_device.c@822 PS1, Line 822: bridge_ctrl = pci_read_config16(bus->dev, PCI_BRIDGE_CONTROL);
Write with CTL_VGA=0,CTL_VGA16=1 is strange/unexpected. […]
Ooff, I assumed the other case, that hardware might mind if we change VGA16 while VGA is off... I don't think this can be solved without time travel and tighter specs.