Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35516 )
Change subject: device/pci: Enable full 16-bit VGA port i/o decoding ......................................................................
Patch Set 3:
(7 comments)
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.
It could be that even when implemented for PCIe root ports, CTL_VGA bits for those bridges have no […]
Done
https://review.coreboot.org/c/coreboot/+/35516/1/src/device/device.c File src/device/device.c:
https://review.coreboot.org/c/coreboot/+/35516/1/src/device/device.c@403 PS1, Line 403: The legacy PCI decodes : * only 10 bits, uses 0x100 - 0x3ff.
I wonder if the comment was supposed to say 'legacy ISA' decodes only 10 bits? […]
Done
https://review.coreboot.org/c/coreboot/+/35516/1/src/device/device.c@417 PS1, Line 417: }
Well.. it only took 16 years to notice? […]
Done
https://review.coreboot.org/c/coreboot/+/35516/1/src/device/device.c@794 PS1, Line 794: /* VGA is first add-on card or the only onboard VGA. */
The code here is full of weird assumptions, the comment only adds to it... […]
Done
https://review.coreboot.org/c/coreboot/+/35516/2/src/device/device.c File src/device/device.c:
https://review.coreboot.org/c/coreboot/+/35516/2/src/device/device.c@799 PS2, Line 799:
Um, I guess it's best if you add it with your patch (wasn't sure if you want it […]
Done
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@807 PS1, Line 807: bridge_ctrl = pci_read_config16(bus->dev, PCI_BRIDGE_CONTROL);
If upstream bridge supports CTL_VGA16, it does not matter when downstream (including cardbus) brid […]
Done
https://review.coreboot.org/c/coreboot/+/35516/1/src/include/device/pci_def.... File src/include/device/pci_def.h:
https://review.coreboot.org/c/coreboot/+/35516/1/src/include/device/pci_def.... PS1, Line 174: #define PCI_CB_BRIDGE_CTL_VGA 0x08
well, "matches" means we can assume the bit to be reserved for now... […]
Done