RE: [PATCH rtw-next] wifi: rtw88: pci: recover from a stalled TX ring

[Ping-Ke Shih]

Abdurrahman Karadag <abdurrahmankaradag19@xxxxxxxxx> wrote:

[...]

> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index 66d2e5f51..f9875cb07 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -787,6 +787,72 @@ static void rtw_pci_tx_kick_off_queue(struct rtw_dev *rtwdev,
>         spin_unlock_bh(&rtwpci->irq_lock);
>  }
> 
> +/* The hardware read pointer of a TX ring can stop advancing while the driver

First line of comment block should be empty. We don't use special style for
networking subsystem anymroe.

> + * keeps queueing descriptors. Once the ring is full, rtw_pci_tx_write() stops
> + * the mac80211 queue, and because the only wake up is inside the completion
> + * loop of rtw_pci_tx_isr(), which does not run while the read pointer is
> + * frozen, the queue would stay stopped forever. Detect a ring that is not
> + * draining and kick it off again.
> + */
> +#define RTW_PCI_TX_STALL_LIMIT 3
> +
> +static void rtw_pci_tx_stall_check(struct rtw_dev *rtwdev)
> +{
> +       struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
> +       struct rtw_pci_tx_ring *ring;
> +       enum rtw_tx_queue_type queue;
> +       u32 bd_idx, cur_rp, wp;
> +       bool pending, kick, warn, stopped;
> +
> +       for (queue = 0; queue < RTK_MAX_TX_QUEUE_NUM; queue++) {
> +               /* BCN is the reserved page and H2C is managed by the
> +                * firmware, neither is flow controlled through mac80211
> +                */
> +               if (queue == RTW_TX_QUEUE_BCN || queue == RTW_TX_QUEUE_H2C)
> +                       continue;
> +
> +               ring = &rtwpci->tx_rings[queue];
> +               kick = false;
> +               warn = false;
> +
> +               spin_lock_bh(&rtwpci->irq_lock);
> +
> +               /* nothing in flight, do not touch the device */
> +               if (skb_queue_empty(&ring->queue) && !ring->queue_stopped) {
> +                       ring->stall_cnt = 0;
> +                       ring->stall_warned = false;
> +                       spin_unlock_bh(&rtwpci->irq_lock);
> +                       continue;
> +               }
> +
> +               bd_idx = rtw_read32(rtwdev, rtw_pci_tx_queue_idx_addr[queue]);
> +               cur_rp = (bd_idx >> 16) & TRX_BD_IDX_MASK;
> +               wp = ring->r.wp;
> +               pending = ring->queue_stopped || cur_rp != wp;
> +
> +               if (pending && cur_rp == ring->last_rp) {
> +                       if (++ring->stall_cnt >= RTW_PCI_TX_STALL_LIMIT) {
> +                               ring->stall_cnt = 0;
> +                               kick = true;
> +                               warn = !ring->stall_warned;
> +                               ring->stall_warned = true;
> +                       }
> +               } else {
> +                       ring->stall_cnt = 0;
> +                       ring->stall_warned = false;
> +               }
> +               stopped = ring->queue_stopped;
> +               ring->last_rp = cur_rp;

Does software pointer cause driver doesn't restart queue?
Or without changes of this chunk, the stuck can be resolved?

> +               spin_unlock_bh(&rtwpci->irq_lock);
> +
> +               if (warn)
> +                       rtw_warn(rtwdev, "TX queue %d stalled (rp %u wp %u%s), kicking\n",
> +                                queue, cur_rp, wp, stopped ? ", stopped" : "");
> +               if (kick)
> +                       rtw_pci_tx_kick_off_queue(rtwdev, queue);

I mean just doing this kick is enough to resolve the stuck?

> +       }
> +}
> +
>  static void rtw_pci_tx_kick_off(struct rtw_dev *rtwdev)
>  {
>         struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;