| Message ID | c75931ac-7208-4200-9ca1-821629cf5e28@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3cb4f534bac010258b2688395c2f13459a932be9 |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Revert "PCI/ASPM: Disable only ASPM_STATE_L1 when driver, disables L1" | expand |
On Wed, Oct 11, 2023 at 09:36:40AM +0200, Heiner Kallweit wrote: > This reverts commit fb097dcd5a28c0a2325632405c76a66777a6bed9. > > After the referenced commit we may see L1 sub-states being active > unexpectedly. Following scenario as an example: > r8169 disables L1 because of known hardware issues on a number of > systems. Implicitly L1.1 and L1.2 are disabled too. > On my system L1 and L1.1 work fine, but L1.2 causes missed > rx packets. Therefore I write 1 to aspm_l1_1. > This removes ASPM_STATE_L1 from the disabled modes and therefore > unexpectedly enables also L1.2. So return to the old behavior. > > Link: https://lore.kernel.org/linux-pci/20231002151452.GA560499@bhelgaas/ > Cc: stable@vger.kernel.org > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to pci/aspm for v6.7, thanks, Heiner! > --- > Splitted the first version of the patch according to the linked discussion. > --- > drivers/pci/pcie/aspm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 1bf630059264..530c3bb5708c 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -1059,7 +1059,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) > -- > 2.42.0 >
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 1bf630059264..530c3bb5708c 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -1059,7 +1059,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)
This reverts commit fb097dcd5a28c0a2325632405c76a66777a6bed9. After the referenced commit we may see L1 sub-states being active unexpectedly. Following scenario as an example: r8169 disables L1 because of known hardware issues on a number of systems. Implicitly L1.1 and L1.2 are disabled too. On my system L1 and L1.1 work fine, but L1.2 causes missed rx packets. Therefore I write 1 to aspm_l1_1. This removes ASPM_STATE_L1 from the disabled modes and therefore unexpectedly enables also L1.2. So return to the old behavior. Link: https://lore.kernel.org/linux-pci/20231002151452.GA560499@bhelgaas/ Cc: stable@vger.kernel.org Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- Splitted the first version of the patch according to the linked discussion. --- drivers/pci/pcie/aspm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)