Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38214 )
Change subject: asus/am1i-a: fix the VGA-related PCI resource allocation problems ......................................................................
Patch Set 4:
(3 comments)
Please mention in the commit message what this change does (not what symptoms it fixes). I assume it lowers the MMIO hole from 0xe0000000 to 0xd0000000. Is it just that?
Assuming it is:
* You should check MTRR allocations before/after the change. If it runs out of registers, that can be bad for performance (depending on whether the CPU supports PAT). * A modern platform should support memory remapping (i.e. can address the DRAM behind the hole above 4GiB). In that case, you can add a safety margin at no cost, e.g. 0xc0000000, or even 0x80000000. * Why is this handled at the mainboard level? * Please also mention in the commit message that this is about large MMIO resources. Not really VGA specific.
https://review.coreboot.org/c/coreboot/+/38214/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38214/3//COMMIT_MSG@11 PS3, Line 11: It does not bring any downsides if there is no discrete VGA adapter.
Done.
For the record, this is how breaking at 75 chars looks like in a narrow Gerrit view:
asus/am1i-a: fix the VGA-related PCI resource allocation problems
This workaround helps to avoid the PCI resource allocation problems which happen if a discrete VGA adapter (i.e. AMD HD6670) is plugged into AM1I- A board. It doesn't bring any downsides if there is no discrete VGA adapter.
The standard is 72 chars.
https://review.coreboot.org/c/coreboot/+/38214/3//COMMIT_MSG@14 PS3, Line 14: IO You are referring to MMIO, right? IO is easily confused with i/o ports. Especially when you talk about VGA resources.
https://review.coreboot.org/c/coreboot/+/38214/3/src/mainboard/asus/am1i-a/O... File src/mainboard/asus/am1i-a/OemCustomize.c:
https://review.coreboot.org/c/coreboot/+/38214/3/src/mainboard/asus/am1i-a/O... PS3, Line 152: InitPost->MemConfig.BottomIo = 0xD0; Is this about MMIO? is the value shifted by 24 bit?