| Message ID | 20190826070827.1436-1-jian-hong@endlessm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v4] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ | expand |
> From: Jian-Hong Pan > Subject: [PATCH v4] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ > > There is a mass of jobs between spin lock and unlock in the hardware > IRQ which will occupy much time originally. To make system work more > efficiently, this patch moves the jobs to the soft IRQ (bottom half) to > reduce the time in hardware IRQ. > > Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com> > --- > v2: > Change the spin_lock_irqsave/unlock_irqrestore to spin_lock/unlock in > rtw_pci_interrupt_handler. Because the interrupts are already disabled > in the hardware interrupt handler. > > v3: > Extend the spin lock protecting area for the TX path in > rtw_pci_interrupt_threadfn by Realtek's suggestion > > v4: > Remove the WiFi running check in rtw_pci_interrupt_threadfn to avoid AP > connection failed by Realtek's suggestion. > > drivers/net/wireless/realtek/rtw88/pci.c | 32 +++++++++++++++++++----- > 1 file changed, 26 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c > b/drivers/net/wireless/realtek/rtw88/pci.c > index 00ef229552d5..955dd6c6fb57 100644 > --- a/drivers/net/wireless/realtek/rtw88/pci.c > +++ b/drivers/net/wireless/realtek/rtw88/pci.c > @@ -866,12 +866,29 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq, > void *dev) > { > struct rtw_dev *rtwdev = dev; > struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv; > - u32 irq_status[4]; > > spin_lock(&rtwpci->irq_lock); > if (!rtwpci->irq_enabled) > goto out; > > + /* disable RTW PCI interrupt to avoid more interrupts before the end of > + * thread function > + */ > + rtw_pci_disable_interrupt(rtwdev, rtwpci); > +out: > + spin_unlock(&rtwpci->irq_lock); > + > + return IRQ_WAKE_THREAD; > +} > + > +static irqreturn_t rtw_pci_interrupt_threadfn(int irq, void *dev) > +{ > + struct rtw_dev *rtwdev = dev; > + struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv; > + unsigned long flags; > + u32 irq_status[4]; > + > + spin_lock_irqsave(&rtwpci->irq_lock, flags); > rtw_pci_irq_recognized(rtwdev, rtwpci, irq_status); > > if (irq_status[0] & IMR_MGNTDOK) > @@ -891,8 +908,9 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq, > void *dev) > if (irq_status[0] & IMR_ROK) > rtw_pci_rx_isr(rtwdev, rtwpci, RTW_RX_QUEUE_MPDU); > > -out: > - spin_unlock(&rtwpci->irq_lock); > + /* all of the jobs for this interrupt have been done */ > + rtw_pci_enable_interrupt(rtwdev, rtwpci); > + spin_unlock_irqrestore(&rtwpci->irq_lock, flags); > > return IRQ_HANDLED; > } > @@ -1152,8 +1170,10 @@ static int rtw_pci_probe(struct pci_dev *pdev, > goto err_destroy_pci; > } > > - ret = request_irq(pdev->irq, &rtw_pci_interrupt_handler, > - IRQF_SHARED, KBUILD_MODNAME, rtwdev); > + ret = devm_request_threaded_irq(rtwdev->dev, pdev->irq, > + rtw_pci_interrupt_handler, > + rtw_pci_interrupt_threadfn, > + IRQF_SHARED, KBUILD_MODNAME, rtwdev); > if (ret) { > ieee80211_unregister_hw(hw); > goto err_destroy_pci; > @@ -1192,7 +1212,7 @@ static void rtw_pci_remove(struct pci_dev *pdev) > rtw_pci_disable_interrupt(rtwdev, rtwpci); > rtw_pci_destroy(rtwdev, pdev); > rtw_pci_declaim(rtwdev, pdev); > - free_irq(rtwpci->pdev->irq, rtwdev); > + devm_free_irq(rtwdev->dev, rtwpci->pdev->irq, rtwdev); > rtw_core_deinit(rtwdev); > ieee80211_free_hw(hw); > } > -- > 2.20.1 Now it works fine with MSI interrupt enabled. But this patch is conflicting with MSI interrupt patch. Is there a better way we can make Kalle apply them more smoothly? I can rebase them and submit both if you're OK. Yan-Hsuan
Tony Chuang <yhchuang@realtek.com> writes: >> From: Jian-Hong Pan >> Subject: [PATCH v4] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ >> >> There is a mass of jobs between spin lock and unlock in the hardware >> IRQ which will occupy much time originally. To make system work more >> efficiently, this patch moves the jobs to the soft IRQ (bottom half) to >> reduce the time in hardware IRQ. >> >> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com> > > Now it works fine with MSI interrupt enabled. > > But this patch is conflicting with MSI interrupt patch. > Is there a better way we can make Kalle apply them more smoothly? > I can rebase them and submit both if you're OK. Yeah, submitting all the MSI patches in the same patchset is the easiest approach. That way they apply cleanly to wireless-drivers-next.
Kalle Valo <kvalo@codeaurora.org> 於 2019年9月2日 週一 下午8:18寫道： > > Tony Chuang <yhchuang@realtek.com> writes: > > >> From: Jian-Hong Pan > >> Subject: [PATCH v4] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ > >> > >> There is a mass of jobs between spin lock and unlock in the hardware > >> IRQ which will occupy much time originally. To make system work more > >> efficiently, this patch moves the jobs to the soft IRQ (bottom half) to > >> reduce the time in hardware IRQ. > >> > >> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com> > > > > Now it works fine with MSI interrupt enabled. > > > > But this patch is conflicting with MSI interrupt patch. > > Is there a better way we can make Kalle apply them more smoothly? > > I can rebase them and submit both if you're OK. The rebase work is appreciated. Thank you, Jian-Hong Pan > Yeah, submitting all the MSI patches in the same patchset is the easiest > approach. That way they apply cleanly to wireless-drivers-next. > > -- > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> From: Jian-Hong Pan [mailto:jian-hong@endlessm.com] > > > > > Tony Chuang <yhchuang@realtek.com> writes: > > > > >> From: Jian-Hong Pan > > >> Subject: [PATCH v4] rtw88: pci: Move a mass of jobs in hw IRQ to soft > IRQ > > >> > > >> There is a mass of jobs between spin lock and unlock in the hardware > > >> IRQ which will occupy much time originally. To make system work more > > >> efficiently, this patch moves the jobs to the soft IRQ (bottom half) to > > >> reduce the time in hardware IRQ. > > >> > > >> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com> > > > > > > Now it works fine with MSI interrupt enabled. > > > > > > But this patch is conflicting with MSI interrupt patch. > > > Is there a better way we can make Kalle apply them more smoothly? > > > I can rebase them and submit both if you're OK. > > The rebase work is appreciated. > Rebased and sent. Please check it and see if I've done anything wrong :) https://patchwork.kernel.org/cover/11127453/ Thanks, Yan-Hsuan
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c index 00ef229552d5..955dd6c6fb57 100644 --- a/drivers/net/wireless/realtek/rtw88/pci.c +++ b/drivers/net/wireless/realtek/rtw88/pci.c @@ -866,12 +866,29 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq, void *dev) { struct rtw_dev *rtwdev = dev; struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv; - u32 irq_status[4]; spin_lock(&rtwpci->irq_lock); if (!rtwpci->irq_enabled) goto out; + /* disable RTW PCI interrupt to avoid more interrupts before the end of + * thread function + */ + rtw_pci_disable_interrupt(rtwdev, rtwpci); +out: + spin_unlock(&rtwpci->irq_lock); + + return IRQ_WAKE_THREAD; +} + +static irqreturn_t rtw_pci_interrupt_threadfn(int irq, void *dev) +{ + struct rtw_dev *rtwdev = dev; + struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv; + unsigned long flags; + u32 irq_status[4]; + + spin_lock_irqsave(&rtwpci->irq_lock, flags); rtw_pci_irq_recognized(rtwdev, rtwpci, irq_status); if (irq_status[0] & IMR_MGNTDOK) @@ -891,8 +908,9 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq, void *dev) if (irq_status[0] & IMR_ROK) rtw_pci_rx_isr(rtwdev, rtwpci, RTW_RX_QUEUE_MPDU); -out: - spin_unlock(&rtwpci->irq_lock); + /* all of the jobs for this interrupt have been done */ + rtw_pci_enable_interrupt(rtwdev, rtwpci); + spin_unlock_irqrestore(&rtwpci->irq_lock, flags); return IRQ_HANDLED; } @@ -1152,8 +1170,10 @@ static int rtw_pci_probe(struct pci_dev *pdev, goto err_destroy_pci; } - ret = request_irq(pdev->irq, &rtw_pci_interrupt_handler, - IRQF_SHARED, KBUILD_MODNAME, rtwdev); + ret = devm_request_threaded_irq(rtwdev->dev, pdev->irq, + rtw_pci_interrupt_handler, + rtw_pci_interrupt_threadfn, + IRQF_SHARED, KBUILD_MODNAME, rtwdev); if (ret) { ieee80211_unregister_hw(hw); goto err_destroy_pci; @@ -1192,7 +1212,7 @@ static void rtw_pci_remove(struct pci_dev *pdev) rtw_pci_disable_interrupt(rtwdev, rtwpci); rtw_pci_destroy(rtwdev, pdev); rtw_pci_declaim(rtwdev, pdev); - free_irq(rtwpci->pdev->irq, rtwdev); + devm_free_irq(rtwdev->dev, rtwpci->pdev->irq, rtwdev); rtw_core_deinit(rtwdev); ieee80211_free_hw(hw); }
There is a mass of jobs between spin lock and unlock in the hardware IRQ which will occupy much time originally. To make system work more efficiently, this patch moves the jobs to the soft IRQ (bottom half) to reduce the time in hardware IRQ. Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com> --- v2: Change the spin_lock_irqsave/unlock_irqrestore to spin_lock/unlock in rtw_pci_interrupt_handler. Because the interrupts are already disabled in the hardware interrupt handler. v3: Extend the spin lock protecting area for the TX path in rtw_pci_interrupt_threadfn by Realtek's suggestion v4: Remove the WiFi running check in rtw_pci_interrupt_threadfn to avoid AP connection failed by Realtek's suggestion. drivers/net/wireless/realtek/rtw88/pci.c | 32 +++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-)