RE: [PATCH v2] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE
flat view
From: Pkshih <pkshih@realtek.com>
Date: 2021-12-14 05:59:34
Also in:
lkml, netdev
quoted hunk ↗ jump to hunk
-----Original Message----- From: Kai-Heng Feng <redacted> Sent: Tuesday, December 14, 2021 1:33 PM To: tony0620emma@gmail.com; Pkshih <pkshih@realtek.com> Cc: jian-hong@endlessm.com; Kai-Heng Feng <redacted>; Kalle Valo [off-list ref]; David S. Miller [off-list ref]; Jakub Kicinski [off-list ref]; Bernie Huang [off-list ref]; Brian Norris [off-list ref]; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org Subject: [PATCH v2] rtw88: Disable PCIe ASPM while doing NAPI poll on 8821CE 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 <redacted> --- v2: - Add default value for module parameter. drivers/net/wireless/realtek/rtw88/pci.c | 74 ++++++++---------------- drivers/net/wireless/realtek/rtw88/pci.h | 1 + 2 files changed, 24 insertions(+), 51 deletions(-)diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c index 3b367c9085eba..4ab75ac2500e9 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"@@ -16,10 +15,13 @@ static bool rtw_disable_msi; static bool rtw_pci_disable_aspm; +static int rtw_rx_aspm = -1; module_param_named(disable_msi, rtw_disable_msi, bool, 0644); module_param_named(disable_aspm, rtw_pci_disable_aspm, bool, 0644); +module_param_named(rx_aspm, rtw_rx_aspm, int, 0444); MODULE_PARM_DESC(disable_msi, "Set Y to disable MSI interrupt support"); MODULE_PARM_DESC(disable_aspm, "Set Y to disable PCI ASPM support"); +MODULE_PARM_DESC(rx_aspm, "Use PCIe ASPM for RX (0=disable, 1=enable, -1=default)"); static u32 rtw_pci_tx_queue_idx_addr[] = { [RTW_TX_QUEUE_BK] = RTK_PCI_TXBD_IDX_BKQ,@@ -1409,7 +1411,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_return(&rtwpci->link_usage) == 0) || + (!enter && atomic_inc_return(&rtwpci->link_usage) == 1)) rtw_pci_aspm_set(rtwdev, enter); }
I found calling pci_link_ps isn't always symmetric, so we need to reset ref_cnt at pci_start() like below, or we can't enter rtw_pci_aspm_set() anymore. The negative flow I face is ifup -> connect AP -> ifdown -> ifup (ref_cnt isn't expected now).
@@ -582,6 +582,8 @@ static int rtw_pci_start(struct rtw_dev *rtwdev) rtw_pci_napi_start(rtwdev); spin_lock_bh(&rtwpci->irq_lock); + atomic_set(&rtwpci->link_usage, 1); rtwpci->running = true; rtw_pci_enable_interrupt(rtwdev, rtwpci, false); spin_unlock_bh(&rtwpci->irq_lock); --
Ping-Ke