<p>Aamir Bohra has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/21868">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">src/device: Update LTR configuration scheme<br><br>This patch moves out LTR programming under L1 substate<br>to pchexp_tune_device function, as substate programming<br>and LTR programming are independent.<br><br>LTR programming scheme is updated to check LTR is enabled<br>and supported for entire link and only then LTR for device<br>needs to be enabled to comply with PCI base specification<br>(rev 3.1a, section 6.18). And remove explicit LTR enable<br>programming for root device under L1 substate.<br><br>BRANCH=none<br>BUG=b:66722364<br>TEST=Verify LTR is configured for end point device only<br>when all parent devices in link has LTR enabled and max<br>snoop latency gets configured.<br><br>Change-Id: I6be99c3b590c1457adf88bc1b40f128fcade3fbe<br>Signed-off-by: Aamir Bohra <aamir.bohra@intel.com><br>---<br>M src/device/pciexp_device.c<br>M src/include/device/pci_def.h<br>2 files changed, 43 insertions(+), 9 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/21868/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 40c3f8c..e3859d8 100644<br>--- a/src/device/pciexp_device.c<br>+++ b/src/device/pciexp_device.c<br>@@ -149,16 +149,49 @@<br> root->ops->ops_pci->set_L1_ss_latency(dev, cap + 4);<br> }<br> <br>-static void pciexp_enable_ltr(device_t dev)<br>+static unsigned int pciexp_is_ltrenable(device_t dev)<br> {<br> unsigned int cap;<br> cap = pci_find_capability(dev, PCI_CAP_ID_PCIE);<br>- if(!cap) {<br>+ if (cap && pci_read_config16(dev, cap + PCI_EXP_DEV_CAP2_OFFSET) &<br>+ 1 << 11 && pci_read_config16(dev, cap<br>+ +PCI__EXP_DEV_CTL_STS2_CAP_OFFSET) & 1 << 10)<br>+ return 1;<br>+<br>+ return 0;<br>+}<br>+<br>+static void pciexp_enable_ltr(device_t dev)<br>+{<br>+ unsigned int cap;<br>+ device_t root;<br>+<br>+ cap = pci_find_capability(dev, PCI_CAP_ID_PCIE);<br>+ /* Check if capibility pointer is valid and dev ltrms is enabled*/<br>+ if (!cap && pci_read_config16(dev, cap + PCI_EXP_DEV_CAP2_OFFSET) &<br>+ 1 << 11) {<br> printk(BIOS_INFO, "Failed to enable LTR for dev = %s\n",<br> dev_path(dev));<br> return;<br> }<br>- pci_update_config32(dev, cap + 0x28, ~(1 << 10), 1 << 10);<br>+ for (root = dev->bus->dev; root->path.type != DEVICE_PATH_DOMAIN;<br>+ root = root->bus->dev) {<br>+ if (pciexp_is_ltrenable(root))<br>+ continue;<br>+ break;<br>+ }<br>+ if (root->path.type == DEVICE_PATH_DOMAIN) {<br>+ pci_update_config32(dev, cap +<br>+ PCI__EXP_DEV_CTL_STS2_CAP_OFFSET,<br>+ ~(1 << 10), 1 << 10);<br>+ pciexp_config_max_latency(dev->bus->dev, dev);<br>+ }<br>+ else<br>+ printk(BIOS_INFO,<br>+ " Failed to enable LTR for dev %s"<br>+ "as link ltr support is disabled\n",<br>+ dev_path(dev));<br>+ return;<br> }<br> <br> static unsigned char pciexp_L1_substate_cal(device_t dev, unsigned int endp_cap,<br>@@ -232,8 +265,6 @@<br> printk(BIOS_INFO, "Power On Value = 0x%x, Power On Scale = 0x%x\n",<br> endp_power_on_value, power_on_scale);<br> <br>- pciexp_enable_ltr(root);<br>-<br> pci_update_config32(root, root_cap + 0x08, ~0xff00,<br> (comm_mode_rst_time << 8));<br> <br>@@ -255,10 +286,6 @@<br> <br> pci_update_config32(dev_t, end_cap + 0x08, ~0x1f,<br> L1SubStateSupport);<br>-<br>- pciexp_enable_ltr(dev_t);<br>-<br>- pciexp_config_max_latency(root, dev_t);<br> }<br> }<br> <br>@@ -391,6 +418,9 @@<br> if (IS_ENABLED(CONFIG_PCIEXP_L1_SUB_STATE))<br> pciexp_config_L1_sub_state(root, dev);<br> <br>+ /* Check for LTR support and enable */<br>+ pciexp_enable_ltr(dev);<br>+<br> /* Check for and enable ASPM */<br> if (IS_ENABLED(CONFIG_PCIEXP_ASPM))<br> pciexp_enable_aspm(root, root_cap, dev, cap);<br>diff --git a/src/include/device/pci_def.h b/src/include/device/pci_def.h<br>index 1674ee1..a7e032c 100644<br>--- a/src/include/device/pci_def.h<br>+++ b/src/include/device/pci_def.h<br>@@ -373,6 +373,10 @@<br> #define PCI_EXP_DEVCAP 4 /* Device capabilities */<br> #define PCI_EXP_DEVCAP_PAYLOAD 0x07 /* Max_Payload_Size */<br> #define PCI_EXP_DEVCAP_PHANTOM 0x18 /* Phantom functions */<br>+/* Device Capabilities 2 offset */<br>+#define PCI_EXP_DEV_CAP2_OFFSET 0x24<br>+/* Device Control Status 2 offset*/<br>+#define PCI__EXP_DEV_CTL_STS2_CAP_OFFSET 0x28<br> #define PCI_EXP_DEVCAP_EXT_TAG 0x20 /* Extended tags */<br> #define PCI_EXP_DEVCAP_L0S 0x1c0 /* L0s Acceptable Latency */<br> #define PCI_EXP_DEVCAP_L1 0xe00 /* L1 Acceptable Latency */<br></pre><p>To view, visit <a href="https://review.coreboot.org/21868">change 21868</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/21868"/><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: I6be99c3b590c1457adf88bc1b40f128fcade3fbe </div>
<div style="display:none"> Gerrit-Change-Number: 21868 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Aamir Bohra <aamir.bohra@intel.com> </div>