Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47238 )
Change subject: device/pciexp: Allow ASPM on bridge devices ......................................................................
device/pciexp: Allow ASPM on bridge devices
The device acceptable latency field is only valid for 'endpoints', but not for bridge devices. Set the maximum acceptable latency on such devices to allow ASPM being enabled if supported on both sides.
Allows the PCIe link on bridge devices to go into L0s/L1.
This allows the package to enter a deeper sleep state when all links are idle.
WARNING: This might cause issues on PCIe bridge devices that don't properly support ASPM. In addition it might decrease performance.
Change-Id: I277efe0bd1448ee8bff633428aa729aeedf04e28 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/device/pciexp_device.c 1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/47238/1
diff --git a/src/device/pciexp_device.c b/src/device/pciexp_device.c index 8d4bb98..b13318a 100644 --- a/src/device/pciexp_device.c +++ b/src/device/pciexp_device.c @@ -368,8 +368,20 @@ if (endp->disable_pcie_aspm) return;
- /* Get endpoint device capabilities for acceptable limits */ - devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP); + const uint16_t xcap = pci_read_config16(endp, endp_cap + PCI_EXP_FLAGS); + const uint8_t type = (xcap & PCI_EXP_FLAGS_TYPE) >> 4; + + /* + * PCI_EXP_DEVCAP_L0S and PCI_EXP_DEVCAP_L1 are only valid for PCIe endpoints. + * Refer to "PCI Express Base Specification Revision 2.0" Chapter 7.8.3 + */ + if (type != PCI_EXP_TYPE_ENDPOINT && type != PCI_EXP_TYPE_LEG_END) { + /* Set no limit in acceptable latency */ + devcap = (0x7 << 6) | (0x7 << 9); + } else { + /* Get endpoint device capabilities for acceptable limits */ + devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP); + }
/* Enable L0s if it is within endpoint acceptable limit */ ok_latency = (devcap & PCI_EXP_DEVCAP_L0S) >> 6;
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47238 )
Change subject: device/pciexp: Allow ASPM on bridge devices ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/47238/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47238/1//COMMIT_MSG@9 PS1, Line 9: The device acceptable latency field is only valid for 'endpoints', but not Please wrap at 72 characters
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47238
to look at the new patch set (#2).
Change subject: device/pciexp: Allow ASPM on bridge devices ......................................................................
device/pciexp: Allow ASPM on bridge devices
The device acceptable latency field is only valid for 'endpoints', but not for bridge devices. Set the maximum acceptable latency on such devices to allow ASPM being enabled if supported on both sides.
Allows the PCIe link on bridge devices to go into L0s/L1.
This allows the package to enter a deeper sleep state when all links are idle.
WARNING: This might cause issues on PCIe bridge devices that doesn't properly support ASPM. In addition it might decrease performance.
Change-Id: I277efe0bd1448ee8bff633428aa729aeedf04e28 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/device/pciexp_device.c 1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/47238/2
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47238?usp=email )
Change subject: device/pciexp: Allow ASPM on bridge devices ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Patrick Rudolph (Code Review)
-
Stefan Reinauer (Code Review)