<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>