[PATCH v7 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS

[luka.gejak@linux.dev]

From: Luka Gejak <luka.gejak@linux.dev>

The RTL8723BS reports free TX page counts that the generic 8051 path
reads back from the chip on every transfer, which is both slow over SDIO
and unreliable on this part: the register frequently reads back zero
while pages are in fact available. It also gates transmission on a free
count in the SDIO output queue, REG_SDIO_OQT_FREE_PG, which rtw88 does
not track at all. The vendor driver calls this the OQT free space and
never expands the acronym; the register holds the number of further
transfers the SDIO output queue can accept, and the chip discards
writes that arrive when it has run out.

Mirror the vendor driver and keep the per-queue and public page counts
in software, seeded at start and resynchronised from the chip only when
the cached counts say there is not enough room. Wait for a free output
queue entry before writing, and account for the pages consumed after a
successful transfer.

Transfers also have to be padded up to the SDIO block size for this
chip rather than using the generic alignment, so size the write
separately from the frame and zero the padding with __skb_pad(), which
also reallocates a cloned skb instead of writing into a buffer a clone
still shares.

The check, the output queue wait and the accounting are serialised by
a mutex. The TX worker and the H2C path reach this function
concurrently, and two writers that both pass the checks can otherwise
claim the same pages and output queue entry, after which the chip
silently discards whichever transfer arrives second. The vendor driver
avoids the same race by funnelling all transmission through one thread.

Measured on RTL8723BS hardware against an iperf3 server one hop behind
the AP, with the wlan0 byte counters as ground truth. On the generic
path the association completes but no data passes at all: TCP and UDP
both measure 0 bit/s in either direction. With this patch TCP is
25.3 Mbit/s up and 37.3 Mbit/s down, and UDP is 25.0 Mbit/s up at 0%
loss.

Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---

Notes:
    Changes in v7:
      - the free page check, the output queue wait and the accounting after
        the transfer are serialised by a mutex. The TX worker and the H2C
        path run concurrently, and two writers that both passed the checks
        could claim the same pages and output queue entry; the OQT refill
        could also read the register while the other writer was between its
        claim and its transfer. The chip silently discards the overcommitted
        transfer, which for H2C means a lost firmware command.
      - the CMD53 address is computed from skb->len again, as upstream does.
        Passing the aligned size changed the encoded transfer length for
        every other SDIO chip whenever sdio_align_size() padded; for the
        RTL8723BS the two encodings are the same value.
      - the padding is applied with __skb_pad() and only on the RTL8723BS
        path. The open coded version wrote into a cloned skb's shared buffer
        when the tailroom happened to be large enough, and it also ran on
        the generic path, giving other chips a new allocation and failure
        mode. __skb_pad() does not move skb->len, so the trim on the way out
        is gone too. It must not free the skb on failure, since one caller
        requeues it and the other frees it.
    
    Changes in v6: none.
    Changes in v5:
      - the commit message now says what OQT is, as far as the vendor driver
        reveals it.
      - rtw_sdio_8723bs_sync_free_txpg() returns whether the chip reported
        anything and is the only caller of _store_free_txpg(), and
        _init_free_txpg() returns an error rather than nothing. That found a
        real bug: the public pool size was acq_pg_num minus the reserved
        queues with no check, so a chip coming up with no transmit page
        allocation would underflow a u16 and leave about 65000 free pages.
        It is now rtw_sdio_8723bs_pubq_num(), shared with the queue page
        allocation repair path, and it fails cleanly.
      - the output queue wait is bounded by a jiffies deadline rather than a
        loop count, so RTW_SDIO_OQT_TIMEOUT_MS is really milliseconds.
      - rtw_sdio_8723bs_check_rqpn() returns an error instead of silently
        doing nothing when the pool cannot cover the reserved queues, and
        rtw_sdio_start() propagates it. Both early returns are explained.

 drivers/net/wireless/realtek/rtw88/sdio.c | 297 +++++++++++++++++++++-
 drivers/net/wireless/realtek/rtw88/sdio.h |  12 +
 2 files changed, 300 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 5b40d74b16ee..9777900f9bb8 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -20,6 +20,7 @@
 #include "tx.h"
 
 #define RTW_SDIO_INDIRECT_RW_RETRIES			50
+#define RTW_SDIO_OQT_TIMEOUT_MS				1000
 
 static bool rtw_sdio_is_bus_addr(u32 addr)
 {
@@ -548,12 +549,157 @@ static int rtw_sdio_read_port(struct rtw_dev *rtwdev, u8 *buf, size_t count)
 	return ret;
 }
 
+/*
+ * The cached free page counters are a fast path hint only. They are written
+ * from the single threaded TX work and, for H2C and reserved page writes,
+ * from process context, so they are atomic_t; whenever they claim there is
+ * not enough room they are resynchronised from the chip before the caller
+ * gives up, which also absorbs a lost update.
+ */
+static void rtw_sdio_8723bs_store_free_txpg(struct rtw_dev *rtwdev,
+					    u32 free_txpg)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	atomic_set(&rtwsdio->free_pg_high,
+		   u32_get_bits(free_txpg, BIT_FREE_TXPG_HIGH));
+	atomic_set(&rtwsdio->free_pg_normal,
+		   u32_get_bits(free_txpg, BIT_FREE_TXPG_NORMAL));
+	atomic_set(&rtwsdio->free_pg_low,
+		   u32_get_bits(free_txpg, BIT_FREE_TXPG_LOW));
+	atomic_set(&rtwsdio->free_pg_pub,
+		   u32_get_bits(free_txpg, BIT_FREE_TXPG_PUB));
+}
+
+/*
+ * Refresh the cached counters from the chip. Returns false when the chip
+ * reports no free pages at all, which means the counts cannot be trusted
+ * and the caller has to decide what to do instead.
+ */
+static bool rtw_sdio_8723bs_sync_free_txpg(struct rtw_dev *rtwdev)
+{
+	u32 free_txpg = rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+
+	if (!free_txpg)
+		return false;
+
+	rtw_sdio_8723bs_store_free_txpg(rtwdev, free_txpg);
+
+	return true;
+}
+
+/*
+ * Size of the public page pool: whatever the transmit page allocation has
+ * left once the per queue pools are taken out. Fails if the allocation
+ * cannot cover the reserved queues, since the remainder would underflow and
+ * there would be no sensible pool to hand out.
+ */
+static int rtw_sdio_8723bs_pubq_num(struct rtw_dev *rtwdev, u16 *pubq_num)
+{
+	const struct rtw_page_table *pg_tbl = &rtwdev->chip->page_table[0];
+	u16 acq_pg_num = rtwdev->fifo.acq_pg_num;
+	u16 reserved_num;
+
+	reserved_num = pg_tbl->hq_num + pg_tbl->lq_num + pg_tbl->nq_num +
+		       pg_tbl->exq_num + pg_tbl->gapq_num;
+	if (acq_pg_num <= reserved_num) {
+		rtw_err(rtwdev,
+			"no transmit pages left for the public queue: %u of %u reserved\n",
+			reserved_num, acq_pg_num);
+		return -EINVAL;
+	}
+
+	*pubq_num = acq_pg_num - reserved_num;
+
+	return 0;
+}
+
+static int rtw_sdio_8723bs_init_free_txpg(struct rtw_dev *rtwdev)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	const struct rtw_page_table *pg_tbl;
+	u16 pubq_num;
+	int ret;
+
+	/* Seed from the page table when the chip has nothing to report yet. */
+	if (!rtw_sdio_8723bs_sync_free_txpg(rtwdev)) {
+		ret = rtw_sdio_8723bs_pubq_num(rtwdev, &pubq_num);
+		if (ret)
+			return ret;
+
+		pg_tbl = &rtwdev->chip->page_table[0];
+		atomic_set(&rtwsdio->free_pg_high, pg_tbl->hq_num);
+		atomic_set(&rtwsdio->free_pg_normal, pg_tbl->nq_num);
+		atomic_set(&rtwsdio->free_pg_low, pg_tbl->lq_num);
+		atomic_set(&rtwsdio->free_pg_pub, pubq_num);
+	}
+
+	atomic_set(&rtwsdio->tx_oqt_free,
+		   rtw_read8(rtwdev, REG_SDIO_OQT_FREE_PG));
+
+	return 0;
+}
+
+/*
+ * Sum of the queue's dedicated counter and the public pool, clamped at zero:
+ * a lost update between the check below and rtw_sdio_8723bs_consume_txpg()
+ * can briefly drive a counter negative, and letting that wrap would hide the
+ * shortage instead of triggering a resync from the chip.
+ */
+static unsigned int rtw_sdio_8723bs_pages_free(struct rtw_dev *rtwdev,
+					       atomic_t *dedicated)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	int free;
+
+	free = atomic_read(dedicated) + atomic_read(&rtwsdio->free_pg_pub);
+
+	return free > 0 ? free : 0;
+}
+
+static atomic_t *rtw_sdio_8723bs_free_txpg(struct rtw_dev *rtwdev, u8 queue)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	switch (queue) {
+	case RTW_TX_QUEUE_VI:
+		return &rtwsdio->free_pg_normal;
+	case RTW_TX_QUEUE_BE:
+	case RTW_TX_QUEUE_BK:
+		return &rtwsdio->free_pg_low;
+	case RTW_TX_QUEUE_BCN:
+	case RTW_TX_QUEUE_H2C:
+	case RTW_TX_QUEUE_HI0:
+	case RTW_TX_QUEUE_MGMT:
+	case RTW_TX_QUEUE_VO:
+		return &rtwsdio->free_pg_high;
+	default:
+		return NULL;
+	}
+}
+
 static int rtw_sdio_check_free_txpg(struct rtw_dev *rtwdev, u8 queue,
 				    size_t count)
 {
 	unsigned int pages_free, pages_needed;
 
-	if (rtw_chip_wcpu_8051(rtwdev)) {
+	if (rtw_is_8723bs(rtwdev)) {
+		atomic_t *dedicated;
+
+		dedicated = rtw_sdio_8723bs_free_txpg(rtwdev, queue);
+		if (!dedicated) {
+			rtw_warn(rtwdev, "Unknown mapping for queue %u\n", queue);
+			return -EINVAL;
+		}
+
+		pages_free = rtw_sdio_8723bs_pages_free(rtwdev, dedicated);
+		pages_needed = DIV_ROUND_UP(count, rtwdev->chip->page_size);
+		if (pages_needed <= pages_free)
+			return 0;
+
+		rtw_sdio_8723bs_sync_free_txpg(rtwdev);
+		pages_free = rtw_sdio_8723bs_pages_free(rtwdev, dedicated);
+	} else if (rtw_chip_wcpu_8051(rtwdev)) {
 		u32 free_txpg;
 
 		free_txpg = rtw_sdio_read32(rtwdev, REG_SDIO_FREE_TXPG);
@@ -632,10 +778,56 @@ static int rtw_sdio_check_free_txpg(struct rtw_dev *rtwdev, u8 queue,
 	return 0;
 }
 
+static int rtw_sdio_8723bs_wait_tx_oqt(struct rtw_dev *rtwdev)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	unsigned long deadline;
+	u8 free;
+
+	if (atomic_add_unless(&rtwsdio->tx_oqt_free, -1, 0))
+		return 0;
+
+	deadline = jiffies + msecs_to_jiffies(RTW_SDIO_OQT_TIMEOUT_MS);
+	do {
+		free = rtw_read8(rtwdev, REG_SDIO_OQT_FREE_PG);
+		if (free) {
+			atomic_set(&rtwsdio->tx_oqt_free, free - 1);
+			return 0;
+		}
+		usleep_range(1000, 2000);
+	} while (time_before(jiffies, deadline));
+
+	return -EBUSY;
+}
+
+static void rtw_sdio_8723bs_consume_txpg(struct rtw_dev *rtwdev, u8 queue,
+					 unsigned int pages)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	atomic_t *dedicated;
+	unsigned int taken;
+	int free;
+
+	dedicated = rtw_sdio_8723bs_free_txpg(rtwdev, queue);
+	if (!dedicated)
+		return;
+
+	free = atomic_read(dedicated);
+	taken = min_t(unsigned int, pages, free > 0 ? free : 0);
+	atomic_sub(taken, dedicated);
+
+	pages -= taken;
+	if (pages && atomic_sub_return(pages, &rtwsdio->free_pg_pub) < 0)
+		atomic_set(&rtwsdio->free_pg_pub, 0);
+}
+
 static int rtw_sdio_write_port(struct rtw_dev *rtwdev, struct sk_buff *skb,
 			       enum rtw_tx_queue_type queue)
 {
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	bool rtl8723bs = rtw_is_8723bs(rtwdev);
+	unsigned int pages;
+	size_t write_size;
 	bool bus_claim;
 	size_t txsize;
 	u32 txaddr;
@@ -645,31 +837,73 @@ static int rtw_sdio_write_port(struct rtw_dev *rtwdev, struct sk_buff *skb,
 	if (!txaddr)
 		return -EINVAL;
 
-	txsize = sdio_align_size(rtwsdio->sdio_func, skb->len);
+	if (rtl8723bs) {
+		txsize = round_up(skb->len, 4);
+		write_size = txsize > RTW_SDIO_BLOCK_SIZE ?
+			     round_up(txsize, RTW_SDIO_BLOCK_SIZE) : txsize;
+
+		/*
+		 * __skb_pad() zeroes the padding without moving skb->len and
+		 * reallocates when the skb is cloned or short on tailroom,
+		 * so the padding can never land in a buffer a clone still
+		 * shares. It must not free the skb on failure: both callers
+		 * still own it, one requeues it and the other frees it.
+		 */
+		if (write_size > skb->len) {
+			ret = __skb_pad(skb, write_size - skb->len, false);
+			if (ret)
+				return ret;
+		}
+	} else {
+		txsize = sdio_align_size(rtwsdio->sdio_func, skb->len);
+		write_size = txsize;
+	}
+
+	/*
+	 * The free page check, the output queue wait and the accounting
+	 * after the transfer must not interleave with another writer: the
+	 * TX worker and the H2C path run concurrently, and two writers that
+	 * both pass the checks can claim the same pages and output queue
+	 * entry, after which the chip silently discards whichever transfer
+	 * arrives second.
+	 */
+	if (rtl8723bs)
+		mutex_lock(&rtwsdio->tx_credit_lock);
 
 	ret = rtw_sdio_check_free_txpg(rtwdev, queue, txsize);
 	if (ret)
-		return ret;
+		goto out_unlock;
+
+	if (rtl8723bs) {
+		ret = rtw_sdio_8723bs_wait_tx_oqt(rtwdev);
+		if (ret)
+			goto out_unlock;
+	}
 
 	if (!IS_ALIGNED((unsigned long)skb->data, RTW_SDIO_DATA_PTR_ALIGN))
 		rtw_warn(rtwdev, "Got unaligned SKB in %s() for queue %u\n",
 			 __func__, queue);
 
 	bus_claim = rtw_sdio_bus_claim_needed(rtwsdio);
-
 	if (bus_claim)
 		sdio_claim_host(rtwsdio->sdio_func);
-
-	ret = sdio_memcpy_toio(rtwsdio->sdio_func, txaddr, skb->data, txsize);
-
+	ret = sdio_memcpy_toio(rtwsdio->sdio_func, txaddr, skb->data,
+			       write_size);
 	if (bus_claim)
 		sdio_release_host(rtwsdio->sdio_func);
 
-	if (ret)
+	if (ret) {
 		rtw_warn(rtwdev,
 			 "Failed to write %zu byte(s) to SDIO port 0x%08x",
-			 txsize, txaddr);
+			 write_size, txaddr);
+	} else if (rtl8723bs) {
+		pages = DIV_ROUND_UP(txsize, rtwdev->chip->page_size);
+		rtw_sdio_8723bs_consume_txpg(rtwdev, queue, pages);
+	}
 
+out_unlock:
+	if (rtl8723bs)
+		mutex_unlock(&rtwsdio->tx_credit_lock);
 	return ret;
 }
 
@@ -749,8 +983,51 @@ static int rtw_sdio_setup(struct rtw_dev *rtwdev)
 	return 0;
 }
 
+/*
+ * Reprogram the queue page allocation if the chip came up with none. This is
+ * a repair path, not part of the normal start sequence: a non-zero free page
+ * count means the allocation latched during power on and must be left alone,
+ * and without a transmit page pool there is nothing to divide up either.
+ */
+static int rtw_sdio_8723bs_check_rqpn(struct rtw_dev *rtwdev)
+{
+	const struct rtw_chip_info *chip = rtwdev->chip;
+	struct rtw_fifo_conf *fifo = &rtwdev->fifo;
+	const struct rtw_page_table *pg_tbl;
+	u32 free_txpg;
+	u16 pubq_num;
+	int ret;
+
+	free_txpg = rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+	if (free_txpg || !fifo->acq_pg_num)
+		return 0;
+
+	ret = rtw_sdio_8723bs_pubq_num(rtwdev, &pubq_num);
+	if (ret)
+		return ret;
+
+	pg_tbl = &chip->page_table[0];
+	rtw_write32(rtwdev, REG_RQPN_NPQ,
+		    BIT_RQPN_NE(pg_tbl->nq_num, pg_tbl->exq_num));
+	rtw_write32(rtwdev, REG_RQPN,
+		    BIT_RQPN_HLP(pg_tbl->hq_num, pg_tbl->lq_num, pubq_num));
+
+	return 0;
+}
+
 static int rtw_sdio_start(struct rtw_dev *rtwdev)
 {
+	if (rtw_is_8723bs(rtwdev)) {
+		int ret = rtw_sdio_8723bs_check_rqpn(rtwdev);
+
+		if (ret)
+			return ret;
+
+		ret = rtw_sdio_8723bs_init_free_txpg(rtwdev);
+		if (ret)
+			return ret;
+	}
+
 	rtw_sdio_enable_rx_aggregation(rtwdev);
 	rtw_sdio_enable_interrupt(rtwdev);
 
@@ -1294,6 +1571,8 @@ static int rtw_sdio_init_tx(struct rtw_dev *rtwdev)
 		return -ENOMEM;
 	}
 
+	mutex_init(&rtwsdio->tx_credit_lock);
+
 	for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
 		skb_queue_head_init(&rtwsdio->tx_queue[i]);
 	rtwsdio->tx_handler_data = kmalloc_obj(*rtwsdio->tx_handler_data);
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
index 457e8b02380e..f43f1c6309b7 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.h
+++ b/drivers/net/wireless/realtek/rtw88/sdio.h
@@ -86,6 +86,10 @@
 #define REG_SDIO_OQT_FREE_PG			(SDIO_LOCAL_OFFSET + 0x001E)
 /* Free Tx Buffer Page */
 #define REG_SDIO_FREE_TXPG			(SDIO_LOCAL_OFFSET + 0x0020)
+#define BIT_FREE_TXPG_HIGH			GENMASK(7, 0)
+#define BIT_FREE_TXPG_NORMAL			GENMASK(15, 8)
+#define BIT_FREE_TXPG_LOW			GENMASK(23, 16)
+#define BIT_FREE_TXPG_PUB			GENMASK(31, 24)
 /* HCI Current Power Mode 1 */
 #define REG_SDIO_HCPWM1				(SDIO_LOCAL_OFFSET + 0x0024)
 /* HCI Current Power Mode 2 */
@@ -159,6 +163,14 @@ struct rtw_sdio {
 	struct workqueue_struct *txwq;
 	struct rtw_sdio_work_data *tx_handler_data;
 	struct sk_buff_head tx_queue[RTK_MAX_TX_QUEUE_NUM];
+
+	atomic_t free_pg_high;
+	atomic_t free_pg_normal;
+	atomic_t free_pg_low;
+	atomic_t free_pg_pub;
+	atomic_t tx_oqt_free;
+	/* one writer at a time between the credit check and the accounting */
+	struct mutex tx_credit_lock;
 };
 
 extern const struct dev_pm_ops rtw_sdio_pm_ops;
-- 
2.53.0