On Tue, Feb 21, 2023 at 7:09 PM Heiner Kallweit <hkallweit1@xxxxxxxxx> wrote: > > On 21.02.2023 03:38, Kai-Heng Feng wrote: > > ASPM L1/L1.1 gets enabled based on [0], but ASPM L1.1 was actually > > disabled too [1]. > > > > So also disable L1.1 for better compatibility. > > > > [0] https://bugs.launchpad.net/bugs/1942830 > > [1] https://git.launchpad.net/~canonical-kernel/ubuntu/+source/linux-oem/+git/focal/commit/?id=c9b3736de48fd419d6699045d59a0dd1041da014 > > > These references are about problems with L1.2 (which is disabled > per default in mainline). They don't allow any statement about whether > L1.1 is problematic too (and under which circumstances). > At least on my system with RTL8168h there's no problem with L1.1 > when running iperf. There are some systems have performance issue with L1.1 too. But since the series will disable chip-side ASPM during NAPI poll, maybe we can keep both L1.1 and L1.2 enabled? Kai-Heng > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > > --- > > v8: > > - New patch. > > > > drivers/net/ethernet/realtek/r8169_main.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > > index 45147a1016bec..1c949822661ae 100644 > > --- a/drivers/net/ethernet/realtek/r8169_main.c > > +++ b/drivers/net/ethernet/realtek/r8169_main.c > > @@ -5224,13 +5224,13 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > > > > /* Disable ASPM L1 as that cause random device stop working > > * problems as well as full system hangs for some PCIe devices users. > > - * Chips from RTL8168h partially have issues with L1.2, but seem > > - * to work fine with L1 and L1.1. > > + * Chips from RTL8168h partially have issues with L1.1 and L1.2, but > > + * seem to work fine with L1. > > */ > > if (rtl_aspm_is_safe(tp)) > > rc = 0; > > else if (tp->mac_version >= RTL_GIGA_MAC_VER_46) > > - rc = pci_disable_link_state(pdev, PCIE_LINK_STATE_L1_2); > > + rc = pci_disable_link_state(pdev, PCIE_LINK_STATE_L1_1 | PCIE_LINK_STATE_L1_2); > > else > > rc = pci_disable_link_state(pdev, PCIE_LINK_STATE_L1); > > tp->aspm_manageable = !rc; >
