* [PATCH v3] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE
@ 2021-12-15 6:55 Kai-Heng Feng
2021-12-15 7:15 ` Jian-Hong Pan
0 siblings, 1 reply; 5+ messages in thread
From: Kai-Heng Feng @ 2021-12-15 6:55 UTC (permalink / raw)
To: tony0620emma, pkshih
Cc: jian-hong, Kai-Heng Feng, Kalle Valo, David S. Miller,
Jakub Kicinski, Po-Hao Huang, Brian Norris, linux-wireless,
netdev, linux-kernel
Many Intel based platforms face system random freeze after commit
9e2fd29864c5 ("rtw88: add napi support").
The commit itself shouldn't be the culprit. My guess is that the 8821CE
only leaves ASPM L1 for a short period when IRQ is raised. Since IRQ is
masked during NAPI polling, the PCIe link stays at L1 and makes RX DMA
extremely slow. Eventually the RX ring becomes messed up:
[ 1133.194697] rtw_8821ce 0000:02:00.0: pci bus timeout, check dma status
Since the 8821CE hardware may fail to leave ASPM L1, manually do it in
the driver to resolve the issue.
Fixes: 9e2fd29864c5 ("rtw88: add napi support")
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=215131
BugLink: https://bugs.launchpad.net/bugs/1927808
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v3:
- Move the module parameter to be part of private struct.
- Ensure link_usage never goes below zero.
v2:
- Add default value for module parameter.
drivers/net/wireless/realtek/rtw88/pci.c | 61 ++++++++----------------
drivers/net/wireless/realtek/rtw88/pci.h | 2 +
2 files changed, 21 insertions(+), 42 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index a7a6ebfaa203c..08cf66141889b 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -2,7 +2,6 @@
/* Copyright(c) 2018-2019 Realtek Corporation
*/
-#include <linux/dmi.h>
#include <linux/module.h>
#include <linux/pci.h>
#include "main.h"
@@ -1409,7 +1408,11 @@ static void rtw_pci_link_ps(struct rtw_dev *rtwdev, bool enter)
* throughput. This is probably because the ASPM behavior slightly
* varies from different SOC.
*/
- if (rtwpci->link_ctrl & PCI_EXP_LNKCTL_ASPM_L1)
+ if (!(rtwpci->link_ctrl & PCI_EXP_LNKCTL_ASPM_L1))
+ return;
+
+ if ((enter && atomic_dec_if_positive(&rtwpci->link_usage) == 0) ||
+ (!enter && atomic_inc_return(&rtwpci->link_usage) == 1))
rtw_pci_aspm_set(rtwdev, enter);
}
@@ -1658,6 +1661,9 @@ static int rtw_pci_napi_poll(struct napi_struct *napi, int budget)
priv);
int work_done = 0;
+ if (rtwpci->rx_no_aspm)
+ rtw_pci_link_ps(rtwdev, false);
+
while (work_done < budget) {
u32 work_done_once;
@@ -1681,6 +1687,8 @@ static int rtw_pci_napi_poll(struct napi_struct *napi, int budget)
if (rtw_pci_get_hw_rx_ring_nr(rtwdev, rtwpci))
napi_schedule(napi);
}
+ if (rtwpci->rx_no_aspm)
+ rtw_pci_link_ps(rtwdev, true);
return work_done;
}
@@ -1702,50 +1710,13 @@ static void rtw_pci_napi_deinit(struct rtw_dev *rtwdev)
netif_napi_del(&rtwpci->napi);
}
-enum rtw88_quirk_dis_pci_caps {
- QUIRK_DIS_PCI_CAP_MSI,
- QUIRK_DIS_PCI_CAP_ASPM,
-};
-
-static int disable_pci_caps(const struct dmi_system_id *dmi)
-{
- uintptr_t dis_caps = (uintptr_t)dmi->driver_data;
-
- if (dis_caps & BIT(QUIRK_DIS_PCI_CAP_MSI))
- rtw_disable_msi = true;
- if (dis_caps & BIT(QUIRK_DIS_PCI_CAP_ASPM))
- rtw_pci_disable_aspm = true;
-
- return 1;
-}
-
-static const struct dmi_system_id rtw88_pci_quirks[] = {
- {
- .callback = disable_pci_caps,
- .ident = "Protempo Ltd L116HTN6SPW",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Protempo Ltd"),
- DMI_MATCH(DMI_PRODUCT_NAME, "L116HTN6SPW"),
- },
- .driver_data = (void *)BIT(QUIRK_DIS_PCI_CAP_ASPM),
- },
- {
- .callback = disable_pci_caps,
- .ident = "HP HP Pavilion Laptop 14-ce0xxx",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "HP"),
- DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion Laptop 14-ce0xxx"),
- },
- .driver_data = (void *)BIT(QUIRK_DIS_PCI_CAP_ASPM),
- },
- {}
-};
-
int rtw_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
+ struct pci_dev *bridge = pci_upstream_bridge(pdev);
struct ieee80211_hw *hw;
struct rtw_dev *rtwdev;
+ struct rtw_pci *rtwpci;
int drv_data_size;
int ret;
@@ -1763,6 +1734,9 @@ int rtw_pci_probe(struct pci_dev *pdev,
rtwdev->hci.ops = &rtw_pci_ops;
rtwdev->hci.type = RTW_HCI_TYPE_PCIE;
+ rtwpci = (struct rtw_pci *)rtwdev->priv;
+ atomic_set(&rtwpci->link_usage, 1);
+
ret = rtw_core_init(rtwdev);
if (ret)
goto err_release_hw;
@@ -1791,7 +1765,10 @@ int rtw_pci_probe(struct pci_dev *pdev,
goto err_destroy_pci;
}
- dmi_check_system(rtw88_pci_quirks);
+ /* Disable PCIe ASPM L1 while doing NAPI poll for 8821CE */
+ if (pdev->device == 0xc821 && bridge->vendor == PCI_VENDOR_ID_INTEL)
+ rtwpci->rx_no_aspm = true;
+
rtw_pci_phy_cfg(rtwdev);
ret = rtw_register_hw(rtwdev, hw);
diff --git a/drivers/net/wireless/realtek/rtw88/pci.h b/drivers/net/wireless/realtek/rtw88/pci.h
index 66f78eb7757c5..0c37efd8c66fa 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.h
+++ b/drivers/net/wireless/realtek/rtw88/pci.h
@@ -223,6 +223,8 @@ struct rtw_pci {
struct rtw_pci_tx_ring tx_rings[RTK_MAX_TX_QUEUE_NUM];
struct rtw_pci_rx_ring rx_rings[RTK_MAX_RX_QUEUE_NUM];
u16 link_ctrl;
+ atomic_t link_usage;
+ bool rx_no_aspm;
DECLARE_BITMAP(flags, NUM_OF_RTW_PCI_FLAGS);
void __iomem *mmap;
--
2.33.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE
2021-12-15 6:55 [PATCH v3] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE Kai-Heng Feng
@ 2021-12-15 7:15 ` Jian-Hong Pan
2021-12-15 8:03 ` Pkshih
0 siblings, 1 reply; 5+ messages in thread
From: Jian-Hong Pan @ 2021-12-15 7:15 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: Yan-Hsuan Chuang, Ping-Ke Shih, Jian-Hong Pan, Kalle Valo,
David S. Miller, Jakub Kicinski, Po-Hao Huang, Brian Norris,
linux-wireless, Linux Netdev List, Linux Kernel
Tried to apply this patch for testing. But it seems conflicting to
commit c81edb8dddaa "rtw88: add quirk to disable pci caps on HP 250 G7
Notebook PC" in wireless-drivers-next repo.
Jian-Hong Pan
Kai-Heng Feng <kai.heng.feng@canonical.com> 於 2021年12月15日 週三 下午2:55寫道：
>
> Many Intel based platforms face system random freeze after commit
> 9e2fd29864c5 ("rtw88: add napi support").
>
> The commit itself shouldn't be the culprit. My guess is that the 8821CE
> only leaves ASPM L1 for a short period when IRQ is raised. Since IRQ is
> masked during NAPI polling, the PCIe link stays at L1 and makes RX DMA
> extremely slow. Eventually the RX ring becomes messed up:
> [ 1133.194697] rtw_8821ce 0000:02:00.0: pci bus timeout, check dma status
>
> Since the 8821CE hardware may fail to leave ASPM L1, manually do it in
> the driver to resolve the issue.
>
> Fixes: 9e2fd29864c5 ("rtw88: add napi support")
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=215131
> BugLink: https://bugs.launchpad.net/bugs/1927808
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v3:
> - Move the module parameter to be part of private struct.
> - Ensure link_usage never goes below zero.
>
> v2:
> - Add default value for module parameter.
>
> drivers/net/wireless/realtek/rtw88/pci.c | 61 ++++++++----------------
> drivers/net/wireless/realtek/rtw88/pci.h | 2 +
> 2 files changed, 21 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index a7a6ebfaa203c..08cf66141889b 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -2,7 +2,6 @@
> /* Copyright(c) 2018-2019 Realtek Corporation
> */
>
> -#include <linux/dmi.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> #include "main.h"
> @@ -1409,7 +1408,11 @@ static void rtw_pci_link_ps(struct rtw_dev *rtwdev, bool enter)
> * throughput. This is probably because the ASPM behavior slightly
> * varies from different SOC.
> */
> - if (rtwpci->link_ctrl & PCI_EXP_LNKCTL_ASPM_L1)
> + if (!(rtwpci->link_ctrl & PCI_EXP_LNKCTL_ASPM_L1))
> + return;
> +
> + if ((enter && atomic_dec_if_positive(&rtwpci->link_usage) == 0) ||
> + (!enter && atomic_inc_return(&rtwpci->link_usage) == 1))
> rtw_pci_aspm_set(rtwdev, enter);
> }
>
> @@ -1658,6 +1661,9 @@ static int rtw_pci_napi_poll(struct napi_struct *napi, int budget)
> priv);
> int work_done = 0;
>
> + if (rtwpci->rx_no_aspm)
> + rtw_pci_link_ps(rtwdev, false);
> +
> while (work_done < budget) {
> u32 work_done_once;
>
> @@ -1681,6 +1687,8 @@ static int rtw_pci_napi_poll(struct napi_struct *napi, int budget)
> if (rtw_pci_get_hw_rx_ring_nr(rtwdev, rtwpci))
> napi_schedule(napi);
> }
> + if (rtwpci->rx_no_aspm)
> + rtw_pci_link_ps(rtwdev, true);
>
> return work_done;
> }
> @@ -1702,50 +1710,13 @@ static void rtw_pci_napi_deinit(struct rtw_dev *rtwdev)
> netif_napi_del(&rtwpci->napi);
> }
>
> -enum rtw88_quirk_dis_pci_caps {
> - QUIRK_DIS_PCI_CAP_MSI,
> - QUIRK_DIS_PCI_CAP_ASPM,
> -};
> -
> -static int disable_pci_caps(const struct dmi_system_id *dmi)
> -{
> - uintptr_t dis_caps = (uintptr_t)dmi->driver_data;
> -
> - if (dis_caps & BIT(QUIRK_DIS_PCI_CAP_MSI))
> - rtw_disable_msi = true;
> - if (dis_caps & BIT(QUIRK_DIS_PCI_CAP_ASPM))
> - rtw_pci_disable_aspm = true;
> -
> - return 1;
> -}
> -
> -static const struct dmi_system_id rtw88_pci_quirks[] = {
> - {
> - .callback = disable_pci_caps,
> - .ident = "Protempo Ltd L116HTN6SPW",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Protempo Ltd"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "L116HTN6SPW"),
> - },
> - .driver_data = (void *)BIT(QUIRK_DIS_PCI_CAP_ASPM),
> - },
> - {
> - .callback = disable_pci_caps,
> - .ident = "HP HP Pavilion Laptop 14-ce0xxx",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "HP"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion Laptop 14-ce0xxx"),
> - },
> - .driver_data = (void *)BIT(QUIRK_DIS_PCI_CAP_ASPM),
> - },
> - {}
> -};
> -
> int rtw_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> {
> + struct pci_dev *bridge = pci_upstream_bridge(pdev);
> struct ieee80211_hw *hw;
> struct rtw_dev *rtwdev;
> + struct rtw_pci *rtwpci;
> int drv_data_size;
> int ret;
>
> @@ -1763,6 +1734,9 @@ int rtw_pci_probe(struct pci_dev *pdev,
> rtwdev->hci.ops = &rtw_pci_ops;
> rtwdev->hci.type = RTW_HCI_TYPE_PCIE;
>
> + rtwpci = (struct rtw_pci *)rtwdev->priv;
> + atomic_set(&rtwpci->link_usage, 1);
> +
> ret = rtw_core_init(rtwdev);
> if (ret)
> goto err_release_hw;
> @@ -1791,7 +1765,10 @@ int rtw_pci_probe(struct pci_dev *pdev,
> goto err_destroy_pci;
> }
>
> - dmi_check_system(rtw88_pci_quirks);
> + /* Disable PCIe ASPM L1 while doing NAPI poll for 8821CE */
> + if (pdev->device == 0xc821 && bridge->vendor == PCI_VENDOR_ID_INTEL)
> + rtwpci->rx_no_aspm = true;
> +
> rtw_pci_phy_cfg(rtwdev);
>
> ret = rtw_register_hw(rtwdev, hw);
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.h b/drivers/net/wireless/realtek/rtw88/pci.h
> index 66f78eb7757c5..0c37efd8c66fa 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.h
> +++ b/drivers/net/wireless/realtek/rtw88/pci.h
> @@ -223,6 +223,8 @@ struct rtw_pci {
> struct rtw_pci_tx_ring tx_rings[RTK_MAX_TX_QUEUE_NUM];
> struct rtw_pci_rx_ring rx_rings[RTK_MAX_RX_QUEUE_NUM];
> u16 link_ctrl;
> + atomic_t link_usage;
> + bool rx_no_aspm;
> DECLARE_BITMAP(flags, NUM_OF_RTW_PCI_FLAGS);
>
> void __iomem *mmap;
> --
> 2.33.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v3] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE
2021-12-15 7:15 ` Jian-Hong Pan
@ 2021-12-15 8:03 ` Pkshih
2021-12-15 10:06 ` Jian-Hong Pan
0 siblings, 1 reply; 5+ messages in thread
From: Pkshih @ 2021-12-15 8:03 UTC (permalink / raw)
To: Jian-Hong Pan, Kai-Heng Feng
Cc: Yan-Hsuan Chuang, Jian-Hong Pan, Kalle Valo, David S. Miller,
Jakub Kicinski, Bernie Huang, Brian Norris, linux-wireless,
Linux Netdev List, Linux Kernel
> -----Original Message-----
> From: Jian-Hong Pan <jhp@endlessos.org>
> Sent: Wednesday, December 15, 2021 3:16 PM
>
> Tried to apply this patch for testing. But it seems conflicting to
> commit c81edb8dddaa "rtw88: add quirk to disable pci caps on HP 250 G7
> Notebook PC" in wireless-drivers-next repo.
>
I fix the conflict manually, and the ASPM setting is expected.
--
Ping-Ke
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE
2021-12-15 8:03 ` Pkshih
@ 2021-12-15 10:06 ` Jian-Hong Pan
2021-12-15 11:45 ` Kai-Heng Feng
0 siblings, 1 reply; 5+ messages in thread
From: Jian-Hong Pan @ 2021-12-15 10:06 UTC (permalink / raw)
To: Pkshih
Cc: Kai-Heng Feng, Yan-Hsuan Chuang, Jian-Hong Pan, Kalle Valo,
David S. Miller, Jakub Kicinski, Bernie Huang, Brian Norris,
linux-wireless, Linux Netdev List, Linux Kernel
Pkshih <pkshih@realtek.com> 於 2021年12月15日 週三 下午4:03寫道：
>
>
> > -----Original Message-----
> > From: Jian-Hong Pan <jhp@endlessos.org>
> > Sent: Wednesday, December 15, 2021 3:16 PM
> >
> > Tried to apply this patch for testing. But it seems conflicting to
> > commit c81edb8dddaa "rtw88: add quirk to disable pci caps on HP 250 G7
> > Notebook PC" in wireless-drivers-next repo.
> >
>
> I fix the conflict manually, and the ASPM setting is expected.
Yes! Same from my side.
Jian-Hong Pan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE
2021-12-15 10:06 ` Jian-Hong Pan
@ 2021-12-15 11:45 ` Kai-Heng Feng
0 siblings, 0 replies; 5+ messages in thread
From: Kai-Heng Feng @ 2021-12-15 11:45 UTC (permalink / raw)
To: Jian-Hong Pan
Cc: Pkshih, Yan-Hsuan Chuang, Kalle Valo, David S. Miller,
Jakub Kicinski, Bernie Huang, Brian Norris, linux-wireless,
Linux Netdev List, Linux Kernel
On Wed, Dec 15, 2021 at 6:07 PM Jian-Hong Pan <jhp@endlessos.org> wrote:
>
> Pkshih <pkshih@realtek.com> 於 2021年12月15日 週三 下午4:03寫道：
> >
> >
> > > -----Original Message-----
> > > From: Jian-Hong Pan <jhp@endlessos.org>
> > > Sent: Wednesday, December 15, 2021 3:16 PM
> > >
> > > Tried to apply this patch for testing. But it seems conflicting to
> > > commit c81edb8dddaa "rtw88: add quirk to disable pci caps on HP 250 G7
> > > Notebook PC" in wireless-drivers-next repo.
> > >
> >
> > I fix the conflict manually, and the ASPM setting is expected.
>
> Yes! Same from my side.
Sorry for that, I used the wrong tree.
Will send a new one.
Kai-Heng
>
> Jian-Hong Pan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-15 11:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-15 6:55 [PATCH v3] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE Kai-Heng Feng
2021-12-15 7:15 ` Jian-Hong Pan
2021-12-15 8:03 ` Pkshih
2021-12-15 10:06 ` Jian-Hong Pan
2021-12-15 11:45 ` Kai-Heng Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).