From 9adcbfe486cb37ecbf38167ac13a502326f5863c Mon Sep 17 00:00:00 2001 From: Patrick Georgi Date: Tue, 5 Dec 2017 16:36:30 -0500 Subject: [PATCH] device/pciexp_device: Set values numerically instead of as bitmask As noted on linux-pci, we have a weird way to handling "value" and "scale" fields that are supposed to contain numerical values: we encode them as a bitfield. Instead define the two fields (offset and mask) and use numbers. Another issue, not fixed in this CL, is that we write hard-coded values while these fields really need to contain the max() of acceptable delays of the downstream devices. That way the controller can decide whether or not to enter a deeper power management state. It's noted as a TODO. Change-Id: I895b9fe2ee438d3958c2d787e70a84d73eaa49d2 Signed-off-by: Patrick Georgi Found-by: Bjorn Helgaas Reviewed-on: https://review.coreboot.org/22740 Tested-by: build bot (Jenkins) Reviewed-by: Stefan Reinauer --- src/device/pciexp_device.c | 17 +++++++++++++---- src/include/device/pciexp.h | 5 +++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/device/pciexp_device.c b/src/device/pciexp_device.c index a1772047bb..c020159b0e 100644 --- a/src/device/pciexp_device.c +++ b/src/device/pciexp_device.c @@ -277,8 +277,14 @@ static void pciexp_L1_substate_commit(device_t root, device_t dev, pci_update_config32(root, root_cap + 0x0c , 0xffffff04, (endp_power_on_value << 3) | (power_on_scale)); - pci_update_config32(root, root_cap + 0x08, ~0xe3ff0000, - (1 << 21) | (1 << 23) | (1 << 30)); + /* TODO: 0xa0, 2 are values that work on some chipsets but really + * should be determined dynamically by looking at downstream devices. + */ + pci_update_config32(root, root_cap + 0x08, + ~(ASPM_LTR_L12_THRESHOLD_VALUE_MASK | + ASPM_LTR_L12_THRESHOLD_SCALE_MASK), + (0xa0 << ASPM_LTR_L12_THRESHOLD_VALUE_OFFSET) | + (2 << ASPM_LTR_L12_THRESHOLD_SCALE_OFFSET)); pci_update_config32(root, root_cap + 0x08, ~0x1f, L1SubStateSupport); @@ -287,8 +293,11 @@ static void pciexp_L1_substate_commit(device_t root, device_t dev, pci_update_config32(dev_t, end_cap + 0x0c , 0xffffff04, (endp_power_on_value << 3) | (power_on_scale)); - pci_update_config32(dev_t, end_cap + 0x08, ~0xe3ff0000, - (1 << 21) | (1 << 23) | (1 << 30)); + pci_update_config32(dev_t, end_cap + 0x08, + ~(ASPM_LTR_L12_THRESHOLD_VALUE_MASK | + ASPM_LTR_L12_THRESHOLD_SCALE_MASK), + (0xa0 << ASPM_LTR_L12_THRESHOLD_VALUE_OFFSET) | + (2 << ASPM_LTR_L12_THRESHOLD_SCALE_OFFSET)); pci_update_config32(dev_t, end_cap + 0x08, ~0x1f, L1SubStateSupport); diff --git a/src/include/device/pciexp.h b/src/include/device/pciexp.h index f3df1a5f27..e7f212092e 100644 --- a/src/include/device/pciexp.h +++ b/src/include/device/pciexp.h @@ -9,6 +9,11 @@ enum aspm_type { PCIE_ASPM_BOTH = 3, }; +#define ASPM_LTR_L12_THRESHOLD_VALUE_OFFSET 16 +#define ASPM_LTR_L12_THRESHOLD_VALUE_MASK (0x3ff << ASPM_LTR_L12_THRESHOLD_VALUE_OFFSET) +#define ASPM_LTR_L12_THRESHOLD_SCALE_OFFSET 29 +#define ASPM_LTR_L12_THRESHOLD_SCALE_MASK (0x7 << ASPM_LTR_L12_THRESHOLD_SCALE_OFFSET) + void pciexp_scan_bus(struct bus *bus, unsigned int min_devfn, unsigned int max_devfn);