This is a note to let you know that I've just added the patch titled
Revert "PCI/ASPM: Disable only ASPM_STATE_L1 when driver, disables L1"
to the 4.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
revert-pci-aspm-disable-only-aspm_state_l1-when-driv.patch
and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
commit 8b08bc1854f27aad6e9a7e1cadcaf24e39ec15c4
Author: Heiner Kallweit <hkallweit1@xxxxxxxxx>
Date: Wed Oct 11 09:36:40 2023 +0200
Revert "PCI/ASPM: Disable only ASPM_STATE_L1 when driver, disables L1"
[ Upstream commit 3cb4f534bac010258b2688395c2f13459a932be9 ]
This reverts commit fb097dcd5a28c0a2325632405c76a66777a6bed9.
After fb097dcd5a28 ("PCI/ASPM: Disable only ASPM_STATE_L1 when driver
disables L1"), disabling L1 via pci_disable_link_state(PCIE_LINK_STATE_L1),
then enabling one substate, e.g., L1.1, via sysfs actually enables *all*
the substates.
For example, r8169 disables L1 because of hardware issues on a number of
systems, which implicitly disables the L1.1 and L1.2 substates.
On some systems, L1 and L1.1 work fine, but L1.2 causes missed rx packets.
Enabling L1.1 via the sysfs "aspm_l1_1" attribute unexpectedly enables L1.2
as well as L1.1.
After fb097dcd5a28, pci_disable_link_state(PCIE_LINK_STATE_L1) adds only
ASPM_L1 (but not any of the L1.x substates) to the "aspm_disable" mask:
--- Before fb097dcd5a28
+++ After fb097dcd5a28
# r8169 disables L1:
pci_disable_link_state(PCIE_LINK_STATE_L1)
- disable |= ASPM_L1 | ASPM_L1_1 | ASPM_L1_2 | ... # disable L1, L1.x
+ disable |= ASPM_L1 # disable L1 only
# write "1" to sysfs "aspm_l1_1" attribute:
l1_1_aspm
aspm_attr_store_common(state = ASPM_L1_1)
disable &= ~ASPM_L1_1 # enable L1.1
if (state & (ASPM_L1_1 | ...)) # if enabling any substate
disable &= ~ASPM_L1 # enable L1
# final state:
- disable = ASPM_L1_2 | ... # L1, L1.1 enabled; L1.2 disabled
+ disable = 0 # L1, L1.1, L1.2 all enabled
Enabling an L1.x substate removes the substate and L1 from the
"aspm_disable" mask. After fb097dcd5a28, the substates were not added to
the mask when disabling L1, so enabling one substate implicitly enables all
of them.
Revert fb097dcd5a28 so enabling one substate doesn't enable the others.
Link: https://lore.kernel.org/r/c75931ac-7208-4200-9ca1-821629cf5e28@xxxxxxxxx
Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
[bhelgaas: work through example in commit log]
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 8db6a9084a12a..8f934c88dcd76 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1110,7 +1110,8 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
if (state & PCIE_LINK_STATE_L0S)
link->aspm_disable |= ASPM_STATE_L0S;
if (state & PCIE_LINK_STATE_L1)
- link->aspm_disable |= ASPM_STATE_L1;
+ /* L1 PM substates require L1 */
+ link->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
if (state & PCIE_LINK_STATE_L1_1)
link->aspm_disable |= ASPM_STATE_L1_1;
if (state & PCIE_LINK_STATE_L1_2)
