[PATCH rebased 0/2] rtw88: pci: interrupt routine improvement

[PATCH rebased 0/2] rtw88: pci: interrupt routine improvement

[yhchuang]

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

This patch set includes two patches to improve PCI interrupt routine.
One is to reduce HW IRQ time, the other is to enable MSI.

The patches can be found at:
https://patchwork.kernel.org/patch/11114043/
https://patchwork.kernel.org/patch/11126007/

They were rebased properly to resolve conflicts.


Jian-Hong Pan (1):
  rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ

Yu-Yen Ting (1):
  rtw88: pci: enable MSI interrupt

 drivers/net/wireless/realtek/rtw88/pci.c | 70 +++++++++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH rebased 1/2] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ

[yhchuang]

From: Jian-Hong Pan <jian-hong@endlessm.com>

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>
Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 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 00ef229..955dd6c 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.7.4

Re: [PATCH rebased 1/2] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ

[Kalle Valo]

<yhchuang@realtek.com> wrote:

> From: Jian-Hong Pan <jian-hong@endlessm.com>
> 
> 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>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

2 patches applied to wireless-drivers-next.git, thanks.

b3d07736b30a rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ
79066903454b rtw88: pci: enable MSI interrupt

-- 
https://patchwork.kernel.org/patch/11127455/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH rebased 2/2] rtw88: pci: enable MSI interrupt

[yhchuang]

From: Yu-Yen Ting <steventing@realtek.com>

MSI interrupt should be enabled on certain platform.

Add a module parameter disable_msi to disable MSI interrupt,
driver will then use legacy interrupt instead.

One could rebind the PCI device, probe() will pick up the
new value of the module parameter. Such as:

    echo '0000:01:00.0' > /sys/bus/pci/drivers/rtw_pci/unbind
    echo '0000:01:00.0' > /sys/bus/pci/drivers/rtw_pci/bind

Tested-by: Ján Veselý <jano.vesely@gmail.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Daniel Drake <drake@endlessm.com>
Signed-off-by: Yu-Yen Ting <steventing@realtek.com>
Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 48 ++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 955dd6c..3fdb52a 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -11,6 +11,10 @@
 #include "fw.h"
 #include "debug.h"
 
+static bool rtw_disable_msi;
+module_param_named(disable_msi, rtw_disable_msi, bool, 0644);
+MODULE_PARM_DESC(disable_msi, "Set Y to disable MSI interrupt support");
+
 static u32 rtw_pci_tx_queue_idx_addr[] = {
 	[RTW_TX_QUEUE_BK]	= RTK_PCI_TXBD_IDX_BKQ,
 	[RTW_TX_QUEUE_BE]	= RTK_PCI_TXBD_IDX_BEQ,
@@ -873,6 +877,11 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq, void *dev)
 
 	/* disable RTW PCI interrupt to avoid more interrupts before the end of
 	 * thread function
+	 *
+	 * disable HIMR here to also avoid new HISR flag being raised before
+	 * the HISRs have been Write-1-cleared for MSI. If not all of the HISRs
+	 * are cleared, the edge-triggered interrupt will not be generated when
+	 * a new HISR flag is set.
 	 */
 	rtw_pci_disable_interrupt(rtwdev, rtwpci);
 out:
@@ -1116,6 +1125,38 @@ static struct rtw_hci_ops rtw_pci_ops = {
 	.write_data_h2c = rtw_pci_write_data_h2c,
 };
 
+static int rtw_pci_request_irq(struct rtw_dev *rtwdev, struct pci_dev *pdev)
+{
+	unsigned int flags = PCI_IRQ_LEGACY;
+	int ret;
+
+	if (!rtw_disable_msi)
+		flags |= PCI_IRQ_MSI;
+
+	ret = pci_alloc_irq_vectors(pdev, 1, 1, flags);
+	if (ret < 0) {
+		rtw_err(rtwdev, "failed to alloc PCI irq vectors\n");
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(rtwdev->dev, pdev->irq,
+					rtw_pci_interrupt_handler,
+					rtw_pci_interrupt_threadfn,
+					IRQF_SHARED, KBUILD_MODNAME, rtwdev);
+	if (ret) {
+		rtw_err(rtwdev, "failed to request irq %d\n", ret);
+		pci_free_irq_vectors(pdev);
+	}
+
+	return ret;
+}
+
+static void rtw_pci_free_irq(struct rtw_dev *rtwdev, struct pci_dev *pdev)
+{
+	devm_free_irq(rtwdev->dev, pdev->irq, rtwdev);
+	pci_free_irq_vectors(pdev);
+}
+
 static int rtw_pci_probe(struct pci_dev *pdev,
 			 const struct pci_device_id *id)
 {
@@ -1170,10 +1211,7 @@ static int rtw_pci_probe(struct pci_dev *pdev,
 		goto err_destroy_pci;
 	}
 
-	ret = devm_request_threaded_irq(rtwdev->dev, pdev->irq,
-					rtw_pci_interrupt_handler,
-					rtw_pci_interrupt_threadfn,
-					IRQF_SHARED, KBUILD_MODNAME, rtwdev);
+	ret = rtw_pci_request_irq(rtwdev, pdev);
 	if (ret) {
 		ieee80211_unregister_hw(hw);
 		goto err_destroy_pci;
@@ -1212,7 +1250,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);
-	devm_free_irq(rtwdev->dev, rtwpci->pdev->irq, rtwdev);
+	rtw_pci_free_irq(rtwdev, pdev);
 	rtw_core_deinit(rtwdev);
 	ieee80211_free_hw(hw);
 }
-- 
2.7.4

Re: [PATCH rebased 0/2] rtw88: pci: interrupt routine improvement

[Kalle Valo]

<yhchuang@realtek.com> writes:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>
> This patch set includes two patches to improve PCI interrupt routine.
> One is to reduce HW IRQ time, the other is to enable MSI.
>
> The patches can be found at:
> https://patchwork.kernel.org/patch/11114043/
> https://patchwork.kernel.org/patch/11126007/
>
> They were rebased properly to resolve conflicts.

In the future, please use version markings ("v2", "v3" etc) in the
Subject and not something like "rebased". This makes my life easier as I
can immeaditely see what is the latest version I should take.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

RE: [PATCH rebased 0/2] rtw88: pci: interrupt routine improvement

[Tony Chuang]

> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> 
> <yhchuang@realtek.com> writes:
> 
> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >
> > This patch set includes two patches to improve PCI interrupt routine.
> > One is to reduce HW IRQ time, the other is to enable MSI.
> >
> > The patches can be found at:
> > https://patchwork.kernel.org/patch/11114043/
> > https://patchwork.kernel.org/patch/11126007/
> >
> > They were rebased properly to resolve conflicts.
> 
> In the future, please use version markings ("v2", "v3" etc) in the
> Subject and not something like "rebased". This makes my life easier as I
> can immeaditely see what is the latest version I should take.
> 

Oh, I just combine two patches with different version markings.
And I don't know how to write a proper subject in this case, maybe
I shouldn't use cover latter?

Yan-Hsuan

Re: [PATCH rebased 0/2] rtw88: pci: interrupt routine improvement

[Kalle Valo]

Tony Chuang <yhchuang@realtek.com> writes:

>> From: Kalle Valo [mailto:kvalo@codeaurora.org]
>> 
>> <yhchuang@realtek.com> writes:
>> 
>> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>> >
>> > This patch set includes two patches to improve PCI interrupt routine.
>> > One is to reduce HW IRQ time, the other is to enable MSI.
>> >
>> > The patches can be found at:
>> > https://patchwork.kernel.org/patch/11114043/
>> > https://patchwork.kernel.org/patch/11126007/
>> >
>> > They were rebased properly to resolve conflicts.
>> 
>> In the future, please use version markings ("v2", "v3" etc) in the
>> Subject and not something like "rebased". This makes my life easier as I
>> can immeaditely see what is the latest version I should take.
>> 
>
> Oh, I just combine two patches with different version markings.
> And I don't know how to write a proper subject in this case, maybe
> I shouldn't use cover latter?

The cover letter was helpful, please continue using that. IIRC the
separate patches were v3 and v4, so using v5 would have been clear for
me. But no need to resend because of this, just trying to streamline the
process and optimise my time :)

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

RE: [PATCH rebased 0/2] rtw88: pci: interrupt routine improvement

[Tony Chuang]

> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> 
> Tony Chuang <yhchuang@realtek.com> writes:
> 
> >> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> >>
> >> <yhchuang@realtek.com> writes:
> >>
> >> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >> >
> >> > This patch set includes two patches to improve PCI interrupt routine.
> >> > One is to reduce HW IRQ time, the other is to enable MSI.
> >> >
> >> > The patches can be found at:
> >> > https://patchwork.kernel.org/patch/11114043/
> >> > https://patchwork.kernel.org/patch/11126007/
> >> >
> >> > They were rebased properly to resolve conflicts.
> >>
> >> In the future, please use version markings ("v2", "v3" etc) in the
> >> Subject and not something like "rebased". This makes my life easier as I
> >> can immeaditely see what is the latest version I should take.
> >>
> >
> > Oh, I just combine two patches with different version markings.
> > And I don't know how to write a proper subject in this case, maybe
> > I shouldn't use cover latter?
> 
> The cover letter was helpful, please continue using that. IIRC the
> separate patches were v3 and v4, so using v5 would have been clear for
> me. But no need to resend because of this, just trying to streamline the
> process and optimise my time :)
> 

Sure! Next time I know I should pick the largest number.

Thanks,
Yan-Hsuan