On Mon, Apr 07, 2025 at 07:33:31PM +0100, Sergey Dolgov wrote:
> Dear Bjorn,
>
> both attached.
Thank you very much. Most of the differences are that the
LTR1.2_Threshold values are increased with 7afeb84d14ea ("PCI/ASPM:
Correct LTR_L1.2_THRESHOLD computation"):
02:00.0 PCI bridge: Intel Corporation JHL7540 Thunderbolt 3 Bridge [Titan Ridge 4C 2018]
Bus: primary=02, secondary=03, subordinate=6c, sec-latency=0
Capabilities: [a00 v1] L1 PM Substates
- T_CommonMode=40us LTR1.2_Threshold=65536ns
+ T_CommonMode=40us LTR1.2_Threshold=90112ns
6d:00.0 Non-Volatile memory controller: Intel Corporation Optane NVME SSD H10 with Solid State Storage
Capabilities: [180 v1] L1 PM Substates
- T_CommonMode=0us LTR1.2_Threshold=3145728ns
+ T_CommonMode=0us LTR1.2_Threshold=3211264ns
70:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. RTS525A PCI Express Card Reader
Capabilities: [160 v1] L1 PM Substates
- T_CommonMode=0us LTR1.2_Threshold=98304ns
+ T_CommonMode=0us LTR1.2_Threshold=126976ns
In addition, I wonder if there's something wrong with link training
because the 00:1d.0 - 6d:00.0 link claims training is in progress, and
the 00:1d.2 - 6e:00.0 link is only x1 when it should be x2:
00:1d.0 PCI bridge: Intel Corporation Cannon Lake PCH PCI Express Root Port #9
LnkSta: Speed 8GT/s, Width x2
- TrErr- Train- SlotClk+ DLActive+ BWMgmt- ABWMgmt-
+ TrErr- Train+ SlotClk+ DLActive+ BWMgmt- ABWMgmt-
00:1d.2 PCI bridge: Intel Corporation Cannon Lake PCH PCI Express Root Port #11
- LnkSta: Speed 8GT/s, Width x2
+ LnkSta: Speed 8GT/s, Width x1
I don't see the problem right off, so could you please add the patch
below and collect the dmesg logs again?
Bjorn
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 29fcb0689a91..bd9322bde53a 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -647,12 +647,19 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
val2 = FIELD_GET(PCI_L1SS_CAP_CM_RESTORE_TIME, child_l1ss_cap);
t_common_mode = max(val1, val2);
+ pci_info(child, "parent CMRT %#04x child CMRT %#04x\n", val1, val2);
+
/* Choose the greater of the two Port T_POWER_ON times */
val1 = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_VALUE, parent_l1ss_cap);
scale1 = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_SCALE, parent_l1ss_cap);
val2 = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_VALUE, child_l1ss_cap);
scale2 = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_SCALE, child_l1ss_cap);
+ pci_info(child, "parent T_POWER_ON %#04x usec (val %#02x scale %x)\n",
+ calc_l12_pwron(parent, scale1, val1), val1, scale1);
+ pci_info(child, "child T_POWER_ON %#04x usec (val %#02x scale %x)\n",
+ calc_l12_pwron(child, scale2, val2), val2, scale2);
+
if (calc_l12_pwron(parent, scale1, val1) >
calc_l12_pwron(child, scale2, val2)) {
ctl2 |= FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_SCALE, scale1) |
@@ -675,7 +682,11 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
* least 4us.
*/
l1_2_threshold = 2 + 4 + t_common_mode + t_power_on;
+ pci_info(child, "t_common_mode %#04x t_power_on %#04x l1_2_threshold %#04x\n",
+ t_common_mode, t_power_on, l1_2_threshold);
encode_l12_threshold(l1_2_threshold, &scale, &value);
+ pci_info(child, "encoded LTR_L1.2_THRESHOLD value %#04x scale %x\n",
+ value, scale);
ctl1 |= FIELD_PREP(PCI_L1SS_CTL1_CM_RESTORE_TIME, t_common_mode) |
FIELD_PREP(PCI_L1SS_CTL1_LTR_L12_TH_VALUE, value) |
FIELD_PREP(PCI_L1SS_CTL1_LTR_L12_TH_SCALE, scale);
@@ -686,6 +697,11 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL1, &cctl1);
pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL2, &cctl2);
+ pci_info(child, "L1SS_CTL1 %#08x (parent %#08x child %08x)\n",
+ ctl1, pctl1, cctl1);
+ pci_info(child, "L1SS_CTL2 %#08x (parent %#08x child %08x)\n",
+ ctl2, pctl2, cctl2);
+
if (ctl1 == pctl1 && ctl1 == cctl1 &&
ctl2 == pctl2 && ctl2 == cctl2)
return;
@@ -703,14 +719,27 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
PCI_L1SS_CTL1_L1_2_MASK, 0);
}
+ pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, &pctl1);
+ pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL1, &cctl1);
+ pci_info(child, "L1SS_CTL1 parent %#08x child %#08x (L1.2 disabled)\n",
+ pctl1, cctl1);
+
/* Program T_POWER_ON times in both ports */
pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL2, ctl2);
pci_write_config_dword(child, child->l1ss + PCI_L1SS_CTL2, ctl2);
+ pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL2, &pctl2);
+ pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL2, &cctl2);
+ pci_info(child, "L1SS_CTL2 parent %#08x child %#08x (T_POWER_ON updated)\n",
+ pctl2, cctl2);
+
/* Program Common_Mode_Restore_Time in upstream device */
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
PCI_L1SS_CTL1_CM_RESTORE_TIME, ctl1);
+ pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, &pctl1);
+ pci_info(child, "L1SS_CTL1 parent %#08x (CMRT updated)\n", pctl1);
+
/* Program LTR_L1.2_THRESHOLD time in both ports */
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
PCI_L1SS_CTL1_LTR_L12_TH_VALUE |
@@ -721,6 +750,11 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
PCI_L1SS_CTL1_LTR_L12_TH_SCALE,
ctl1);
+ pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, &pctl1);
+ pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL1, &cctl1);
+ pci_info(child, "L1SS_CTL1 parent %#08x child %#08x (LTR_L1.2_THRESHOLD updated)\n",
+ pctl1, cctl1);
+
if (pl1_2_enables || cl1_2_enables) {
pci_clear_and_set_config_dword(parent,
parent->l1ss + PCI_L1SS_CTL1, 0,
@@ -729,6 +763,11 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
child->l1ss + PCI_L1SS_CTL1, 0,
cl1_2_enables);
}
+
+ pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, &pctl1);
+ pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL1, &cctl1);
+ pci_info(child, "L1SS_CTL1 parent %#08x child %#08x (L1.2 restored)\n",
+ pctl1, cctl1);
}
static void aspm_l1ss_init(struct pcie_link_state *link)
