Skip to content

Commit

Permalink
rtw88: don't hold all IRQs disabled for PS operations
Browse files Browse the repository at this point in the history
This driver generally only needs to ensure that
(a) it doesn't try to process TX interrupts at the same time as
    power-save operations (and similar)
(b) the device interrupt gets disabled while we're still handling the
    last set of interrupts

For (a), all the operations (e.g., PS transitions, packet handling)
happens in non-atomic contexts (e.g., threaded IRQ).

For (b), we only need mutual exclusion for brief sections (i.e., while
we're actually manipulating the interrupt mask/status).

So, we can introduce a separate lock for handling (b), disabling IRQs
while we do it. For (a), we can demote the locking to BH only, now that
(b) (the only steps done in atomic context) and that has its own lock.

This helps reduce the amount of time this driver spends with IRQs off.
Notably, transitioning out of power-save modes can take >3 milliseconds,
and this transition is done under the protection of 'irq_lock'.

Signed-off-by: Brian Norris <[email protected]>
Signed-off-by: Yan-Hsuan Chuang <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
  • Loading branch information
computersforpeace authored and Kalle Valo committed Mar 23, 2020
1 parent 53efdc9 commit 57fb39e
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 22 deletions.
54 changes: 33 additions & 21 deletions drivers/net/wireless/realtek/rtw88/pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ static int rtw_pci_init(struct rtw_dev *rtwdev)
rtwpci->irq_mask[3] = IMR_H2CDOK |
0;
spin_lock_init(&rtwpci->irq_lock);
spin_lock_init(&rtwpci->hwirq_lock);
ret = rtw_pci_init_trx_ring(rtwdev);

return ret;
Expand Down Expand Up @@ -472,19 +473,35 @@ static void rtw_pci_reset_trx_ring(struct rtw_dev *rtwdev)
static void rtw_pci_enable_interrupt(struct rtw_dev *rtwdev,
struct rtw_pci *rtwpci)
{
unsigned long flags;

spin_lock_irqsave(&rtwpci->hwirq_lock, flags);

rtw_write32(rtwdev, RTK_PCI_HIMR0, rtwpci->irq_mask[0]);
rtw_write32(rtwdev, RTK_PCI_HIMR1, rtwpci->irq_mask[1]);
rtw_write32(rtwdev, RTK_PCI_HIMR3, rtwpci->irq_mask[3]);
rtwpci->irq_enabled = true;

spin_unlock_irqrestore(&rtwpci->hwirq_lock, flags);
}

static void rtw_pci_disable_interrupt(struct rtw_dev *rtwdev,
struct rtw_pci *rtwpci)
{
unsigned long flags;

spin_lock_irqsave(&rtwpci->hwirq_lock, flags);

if (!rtwpci->irq_enabled)
goto out;

rtw_write32(rtwdev, RTK_PCI_HIMR0, 0);
rtw_write32(rtwdev, RTK_PCI_HIMR1, 0);
rtw_write32(rtwdev, RTK_PCI_HIMR3, 0);
rtwpci->irq_enabled = false;

out:
spin_unlock_irqrestore(&rtwpci->hwirq_lock, flags);
}

static void rtw_pci_dma_reset(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci)
Expand Down Expand Up @@ -520,24 +537,22 @@ static void rtw_pci_dma_release(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci)
static int rtw_pci_start(struct rtw_dev *rtwdev)
{
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
unsigned long flags;

spin_lock_irqsave(&rtwpci->irq_lock, flags);
spin_lock_bh(&rtwpci->irq_lock);
rtw_pci_enable_interrupt(rtwdev, rtwpci);
spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
spin_unlock_bh(&rtwpci->irq_lock);

return 0;
}

static void rtw_pci_stop(struct rtw_dev *rtwdev)
{
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
unsigned long flags;

spin_lock_irqsave(&rtwpci->irq_lock, flags);
spin_lock_bh(&rtwpci->irq_lock);
rtw_pci_disable_interrupt(rtwdev, rtwpci);
rtw_pci_dma_release(rtwdev, rtwpci);
spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
spin_unlock_bh(&rtwpci->irq_lock);
}

static void rtw_pci_deep_ps_enter(struct rtw_dev *rtwdev)
Expand Down Expand Up @@ -590,17 +605,16 @@ static void rtw_pci_deep_ps_leave(struct rtw_dev *rtwdev)
static void rtw_pci_deep_ps(struct rtw_dev *rtwdev, bool enter)
{
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
unsigned long flags;

spin_lock_irqsave(&rtwpci->irq_lock, flags);
spin_lock_bh(&rtwpci->irq_lock);

if (enter && !test_bit(RTW_FLAG_LEISURE_PS_DEEP, rtwdev->flags))
rtw_pci_deep_ps_enter(rtwdev);

if (!enter && test_bit(RTW_FLAG_LEISURE_PS_DEEP, rtwdev->flags))
rtw_pci_deep_ps_leave(rtwdev);

spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
spin_unlock_bh(&rtwpci->irq_lock);
}

static u8 ac_to_hwq[] = {
Expand Down Expand Up @@ -683,7 +697,6 @@ static int rtw_pci_xmit(struct rtw_dev *rtwdev,
u8 *pkt_desc;
struct rtw_pci_tx_buffer_desc *buf_desc;
u32 bd_idx;
unsigned long flags;

ring = &rtwpci->tx_rings[queue];

Expand Down Expand Up @@ -720,7 +733,7 @@ static int rtw_pci_xmit(struct rtw_dev *rtwdev,
tx_data->dma = dma;
tx_data->sn = pkt_info->sn;

spin_lock_irqsave(&rtwpci->irq_lock, flags);
spin_lock_bh(&rtwpci->irq_lock);

rtw_pci_deep_ps_leave(rtwdev);
skb_queue_tail(&ring->queue, skb);
Expand All @@ -738,7 +751,7 @@ static int rtw_pci_xmit(struct rtw_dev *rtwdev,
reg_bcn_work |= BIT_PCI_BCNQ_FLAG;
rtw_write8(rtwdev, RTK_PCI_TXBD_BCN_WORK, reg_bcn_work);
}
spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
spin_unlock_bh(&rtwpci->irq_lock);

return 0;
}
Expand Down Expand Up @@ -961,6 +974,10 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
static void rtw_pci_irq_recognized(struct rtw_dev *rtwdev,
struct rtw_pci *rtwpci, u32 *irq_status)
{
unsigned long flags;

spin_lock_irqsave(&rtwpci->hwirq_lock, flags);

irq_status[0] = rtw_read32(rtwdev, RTK_PCI_HISR0);
irq_status[1] = rtw_read32(rtwdev, RTK_PCI_HISR1);
irq_status[3] = rtw_read32(rtwdev, RTK_PCI_HISR3);
Expand All @@ -970,17 +987,15 @@ static void rtw_pci_irq_recognized(struct rtw_dev *rtwdev,
rtw_write32(rtwdev, RTK_PCI_HISR0, irq_status[0]);
rtw_write32(rtwdev, RTK_PCI_HISR1, irq_status[1]);
rtw_write32(rtwdev, RTK_PCI_HISR3, irq_status[3]);

spin_unlock_irqrestore(&rtwpci->hwirq_lock, flags);
}

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;

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
*
Expand All @@ -990,8 +1005,6 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq, void *dev)
* a new HISR flag is set.
*/
rtw_pci_disable_interrupt(rtwdev, rtwpci);
out:
spin_unlock(&rtwpci->irq_lock);

return IRQ_WAKE_THREAD;
}
Expand All @@ -1000,10 +1013,9 @@ 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);
spin_lock_bh(&rtwpci->irq_lock);
rtw_pci_irq_recognized(rtwdev, rtwpci, irq_status);

if (irq_status[0] & IMR_MGNTDOK)
Expand All @@ -1025,7 +1037,7 @@ static irqreturn_t rtw_pci_interrupt_threadfn(int irq, void *dev)

/* all of the jobs for this interrupt have been done */
rtw_pci_enable_interrupt(rtwdev, rtwpci);
spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
spin_unlock_bh(&rtwpci->irq_lock);

return IRQ_HANDLED;
}
Expand Down
4 changes: 3 additions & 1 deletion drivers/net/wireless/realtek/rtw88/pci.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,9 @@ struct rtw_pci_rx_ring {
struct rtw_pci {
struct pci_dev *pdev;

/* used for pci interrupt */
/* Used for PCI interrupt. */
spinlock_t hwirq_lock;
/* Used for PCI TX queueing. */
spinlock_t irq_lock;
u32 irq_mask[4];
bool irq_enabled;
Expand Down

0 comments on commit 57fb39e

Please sign in to comment.