<p>Patrick Georgi has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/22740">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">device/pciexp_device: Set values numerically instead of as bitmask<br><br>As noted on linux-pci, we have a weird way to handling "value" and<br>"scale" fields that are supposed to contain numerical values: we encode<br>them as a bitfield.<br>Instead define the two fields (offset and mask) and use numbers.<br><br>Another issue, not fixed in this CL, is that we write hard-coded values<br>while these fields really need to contain the max() of acceptable delays<br>of the downstream devices. That way the controller can decide whether or<br>not to enter a deeper power management state. It's noted as a TODO.<br><br>Change-Id: I895b9fe2ee438d3958c2d787e70a84d73eaa49d2<br>Signed-off-by: Patrick Georgi <pgeorgi@google.com><br>Found-by: Bjorn Helgaas <bhelgaas@google.com><br>---<br>M src/device/pciexp_device.c<br>M src/include/device/pciexp.h<br>2 files changed, 18 insertions(+), 4 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/22740/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/src/device/pciexp_device.c b/src/device/pciexp_device.c<br>index a177204..c020159 100644<br>--- a/src/device/pciexp_device.c<br>+++ b/src/device/pciexp_device.c<br>@@ -277,8 +277,14 @@<br>   pci_update_config32(root, root_cap + 0x0c , 0xffffff04,<br>               (endp_power_on_value << 3) | (power_on_scale));<br> <br>-     pci_update_config32(root, root_cap + 0x08, ~0xe3ff0000,<br>-              (1 << 21) | (1 << 23) | (1 << 30));<br>+        /* TODO: 0xa0, 2 are values that work on some chipsets but really<br>+     * should be determined dynamically by looking at downstream devices.<br>+         */<br>+  pci_update_config32(root, root_cap + 0x08,<br>+           ~(ASPM_LTR_L12_THRESHOLD_VALUE_MASK |<br>+                        ASPM_LTR_L12_THRESHOLD_SCALE_MASK),<br>+          (0xa0 << ASPM_LTR_L12_THRESHOLD_VALUE_OFFSET) |<br>+                (2 << ASPM_LTR_L12_THRESHOLD_SCALE_OFFSET));<br> <br>         pci_update_config32(root, root_cap + 0x08, ~0x1f,<br>             L1SubStateSupport);<br>@@ -287,8 +293,11 @@<br>             pci_update_config32(dev_t, end_cap + 0x0c , 0xffffff04,<br>                       (endp_power_on_value << 3) | (power_on_scale));<br> <br>-             pci_update_config32(dev_t, end_cap + 0x08, ~0xe3ff0000,<br>-                      (1 << 21) | (1 << 23) | (1 << 30));<br>+                pci_update_config32(dev_t, end_cap + 0x08,<br>+                   ~(ASPM_LTR_L12_THRESHOLD_VALUE_MASK |<br>+                                ASPM_LTR_L12_THRESHOLD_SCALE_MASK),<br>+                  (0xa0 << ASPM_LTR_L12_THRESHOLD_VALUE_OFFSET) |<br>+                        (2 << ASPM_LTR_L12_THRESHOLD_SCALE_OFFSET));<br> <br>                 pci_update_config32(dev_t, end_cap + 0x08, ~0x1f,<br>                     L1SubStateSupport);<br>diff --git a/src/include/device/pciexp.h b/src/include/device/pciexp.h<br>index f3df1a5..e7f2120 100644<br>--- a/src/include/device/pciexp.h<br>+++ b/src/include/device/pciexp.h<br>@@ -9,6 +9,11 @@<br>    PCIE_ASPM_BOTH = 3,<br> };<br> <br>+#define ASPM_LTR_L12_THRESHOLD_VALUE_OFFSET 16<br>+#define ASPM_LTR_L12_THRESHOLD_VALUE_MASK (0x3ff << ASPM_LTR_L12_THRESHOLD_VALUE_OFFSET)<br>+#define ASPM_LTR_L12_THRESHOLD_SCALE_OFFSET 29<br>+#define ASPM_LTR_L12_THRESHOLD_SCALE_MASK (0x7 << ASPM_LTR_L12_THRESHOLD_SCALE_OFFSET)<br>+<br> void pciexp_scan_bus(struct bus *bus, unsigned int min_devfn,<br>                           unsigned int max_devfn);<br> <br></pre><p>To view, visit <a href="https://review.coreboot.org/22740">change 22740</a>. To unsubscribe, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/22740"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I895b9fe2ee438d3958c2d787e70a84d73eaa49d2 </div>
<div style="display:none"> Gerrit-Change-Number: 22740 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Patrick Georgi <pgeorgi@google.com> </div>