Re: [PATCH 2/3] wifi: mt76: mt792x: Add TX page pool support for IOMMU-enabled systems

["Eason Lai (賴易聖)" <Eason.Lai@mediatek.com>]

On Thu, 2026-07-09 at 10:27 +0200, Lorenzo Bianconi wrote:
> On Jul 09, Eason Lai wrote:
> > From: Eason Lai <Eason.Lai@mediatek.com>
> > 
> > Add support for TX page pool allocation in mt792x devices when
> > IOMMU is
> > enabled. This optimization reduces DMA mapping overhead by
> > pre-allocating and reusing page pool buffers for TX operations.
> 
> Can you please provide some test results here? In particular
> regarding the
> resulting throughput.
> 

On MT8196, the HE160 TCP UL peak throuhgput could drop below 1.4Gbps
when IOMMU is enabled. With this series, the throughput reaches the
expected performance (>1.8G bps).

> > 
> > Key changes:
> > - Export DMA helper functions and ops for mt792x usage
> >   - Add tx_prealloc_enabled flag to track page pool state
> >     - Implement mt792x-specific DMA queue operations with TX page
> > pool
> >       - Create page pools per TX queue when IOMMU is detected
> >         - Handle page pool buffer cleanup in both success and error
> >           paths
> > - Add proper skip_unmap flag handling for page pool buffers
> > 
> > The page pool path is used for linear skbs without fragments,
> > falling
> > back to standard DMA mapping for complex skb structures.
> > 
> > Signed-off-by: Eason Lai <Eason.Lai@mediatek.com>
> > ---
> 
> [...]
> 
> >  
> > -static void
> > +void
> >  mt76_dma_kick_queue(struct mt76_dev *dev, struct mt76_queue *q)
> >  {
> >  	wmb();
> > @@ -409,6 +417,7 @@ mt76_dma_kick_queue(struct mt76_dev *dev,
> > struct mt76_queue *q)
> >  	else
> >  		Q_WRITE(q, cpu_idx, q->head);
> >  }
> > +EXPORT_SYMBOL_GPL(mt76_dma_kick_queue);
> 
> this is already available in mt76_queue_ops struct.
> 

Thanks. I'll use mt76_queue_kick() in v2.

> >  
> >  static void
> >  mt76_dma_tx_cleanup(struct mt76_dev *dev, struct mt76_queue *q,
> > bool flush)
> > @@ -1152,7 +1161,7 @@ mt76_dma_init(struct mt76_dev *dev,
> >  	return 0;
> >  }
> >  
> > -static const struct mt76_queue_ops mt76_dma_ops = {
> > +const struct mt76_queue_ops mt76_dma_ops = {
> >  	.init = mt76_dma_init,
> >  	.alloc = mt76_dma_alloc_queue,
> >  	.reset_q = mt76_dma_queue_reset,
> > @@ -1164,6 +1173,7 @@ static const struct mt76_queue_ops
> > mt76_dma_ops = {
> >  	.rx_reset = mt76_dma_rx_reset,
> >  	.kick = mt76_dma_kick_queue,
> >  };
> > +EXPORT_SYMBOL_GPL(mt76_dma_ops);
> >  
> >  void mt76_dma_attach(struct mt76_dev *dev)
> >  {
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h
> > b/drivers/net/wireless/mediatek/mt76/mt76.h
> > index 927c21536f4e..25e24fa36eca 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> > @@ -957,6 +957,8 @@ struct mt76_dev {
> >  	int tx_dma_idx[4];
> >  	enum mt76_hwrro_mode hwrro_mode;
> >  
> > +	bool tx_prealloc_enabled;
> > +
> >  	struct mt76_worker tx_worker;
> >  	struct napi_struct tx_napi;
> >  
> > @@ -1785,7 +1787,16 @@ mt76_tx_status_get_hw(struct mt76_dev *dev,
> > struct sk_buff *skb)
> >  
> >  void mt76_put_txwi(struct mt76_dev *dev, struct mt76_txwi_cache
> > *t);
> >  void mt76_put_rxwi(struct mt76_dev *dev, struct mt76_txwi_cache
> > *t);
> > +struct mt76_txwi_cache *mt76_get_txwi(struct mt76_dev *dev);
> >  struct mt76_txwi_cache *mt76_get_rxwi(struct mt76_dev *dev);
> > +int mt76_dma_add_buf(struct mt76_dev *dev, struct mt76_queue *q,
> > +		     struct mt76_queue_buf *buf, int nbufs, u32
> > info,
> > +		     struct sk_buff *skb, void *txwi);
> > +void mt76_dma_tx_cleanup_idx(struct mt76_dev *dev, struct
> > mt76_queue *q, int idx,
> > +			     struct mt76_queue_entry *prev_e);
> > +void mt76_dma_kick_queue(struct mt76_dev *dev, struct mt76_queue
> > *q);
> > +void mt76_dma_sync_idx(struct mt76_dev *dev, struct mt76_queue
> > *q);
> > +extern const struct mt76_queue_ops mt76_dma_ops;
> >  void mt76_free_pending_rxwi(struct mt76_dev *dev);
> >  void mt76_rx_complete(struct mt76_dev *dev, struct sk_buff_head
> > *frames,
> >  		      struct napi_struct *napi);
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > b/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > index 0339e2e7ab60..d4b36b0832b8 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > @@ -250,6 +250,12 @@ void mt76_connac_txp_skb_unmap(struct mt76_dev
> > *dev,
> >  {
> >  	struct mt76_connac_txp_common *txp;
> >  
> > +	if (t->page_pool_buf) {
> > +		mt76_put_page_pool_buf(t->page_pool_buf, false);
> > +		t->page_pool_buf = NULL;
> > +		return;
> > +	}
> > +
> >  	txp = mt76_connac_txwi_to_txp(dev, t);
> >  	if (is_mt76_fw_txp(dev))
> >  		mt76_connac_txp_skb_unmap_fw(dev, &txp->fw);
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> > b/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> > index c4161754c01d..3d69d8c67dea 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> > @@ -214,7 +214,7 @@ static int mt7925_dma_init(struct mt792x_dev
> > *dev)
> >  {
> >  	int ret;
> >  
> > -	mt76_dma_attach(&dev->mt76);
> > +	mt792x_dma_attach(&dev->mt76);
> >  
> >  	ret = mt792x_dma_disable(dev, true);
> >  	if (ret)
> > @@ -263,6 +263,8 @@ static int mt7925_dma_init(struct mt792x_dev
> > *dev)
> >  			  mt792x_poll_tx);
> >  	napi_enable(&dev->mt76.tx_napi);
> >  
> > +	mt792x_dma_tx_page_pool_init(dev);
> > +
> >  	return mt792x_dma_enable(dev);
> >  }
> >  
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt792x.h
> > b/drivers/net/wireless/mediatek/mt76/mt792x.h
> > index 4ff93f2cd624..7ac0318dc249 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt792x.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt792x.h
> > @@ -418,8 +418,10 @@ void mt792x_sta_statistics(struct ieee80211_hw
> > *hw,
> >  			   struct station_info *sinfo);
> >  void mt792x_set_coverage_class(struct ieee80211_hw *hw, int
> > radio_idx,
> >  			       s16 coverage_class);
> > +void mt792x_dma_attach(struct mt76_dev *dev);
> >  void mt792x_dma_cleanup(struct mt792x_dev *dev);
> >  int mt792x_dma_enable(struct mt792x_dev *dev);
> > +int mt792x_dma_tx_page_pool_init(struct mt792x_dev *dev);
> >  int mt792x_wpdma_reset(struct mt792x_dev *dev, bool force);
> >  int mt792x_wpdma_reinit_cond(struct mt792x_dev *dev);
> >  int mt792x_dma_disable(struct mt792x_dev *dev, bool force);
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt792x_dma.c
> > b/drivers/net/wireless/mediatek/mt76/mt792x_dma.c
> > index 002aece857b2..b341f1cb3ce0 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt792x_dma.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt792x_dma.c
> > @@ -3,6 +3,9 @@
> >  
> >  #include <linux/module.h>
> >  #include <linux/firmware.h>
> > +#include <linux/iommu.h>
> > +#include <net/page_pool/helpers.h>
> > +#include <linux/of.h>
> >  
> >  #include "mt792x.h"
> >  #include "dma.h"
> > @@ -266,6 +269,323 @@ int mt792x_wpdma_reinit_cond(struct
> > mt792x_dev *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(mt792x_wpdma_reinit_cond);
> >  
> > +static int mt792x_create_tx_page_pool(struct mt76_dev *dev, struct
> > mt76_queue *q)
> > +{
> > +	struct page_pool_params pp_params = {
> > +		.order = 0,
> > +		.flags = 0,
> > +		.nid = NUMA_NO_NODE,
> > +		.dev = dev->dma_dev,
> > +	};
> > +
> > +	if (!q || !dev->tx_prealloc_enabled)
> > +		return 0;
> > +
> > +	if (!mt76_is_mmio(dev))
> > +		return 0;
> 
> do you really need this check? you already check it in
> mt792x_dma_tx_page_pool_init().
> 

OK, I will remove it in v2.

> > +
> > +	pp_params.pool_size = 256;
> > +	pp_params.flags |= PP_FLAG_DMA_MAP;
> > +	pp_params.dma_dir = DMA_BIDIRECTIONAL;
> > +	pp_params.max_len = PAGE_SIZE;
> > +	pp_params.offset = 0;
> > +
> > +	q->page_pool = page_pool_create(&pp_params);
> > +	if (IS_ERR(q->page_pool)) {
> > +		int err = PTR_ERR(q->page_pool);
> > +
> > +		q->page_pool = NULL;
> > +		dev_warn(dev->dev, "Failed to create TX page pool
> > for queue %d (err=%d)\n",
> > +			 q->hw_idx, err);
> > +		return 0;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int mt792x_dma_tx_page_pool_init(struct mt792x_dev *dev)
> > +{
> > +	struct mt76_dev *mdev = &dev->mt76;
> > +	int i, ret, pool_count = 0;
> > +
> > +	if (!iommu_get_domain_for_dev(mdev->dma_dev))
> > +		return 0;
> > +
> > +	if (!mt76_is_mmio(mdev))
> > +		return 0;
> > +
> > +	mdev->tx_prealloc_enabled = true;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(mdev->phy.q_tx); i++) {
> > +		struct mt76_queue *q = mdev->phy.q_tx[i];
> > +
> > +		if (!q)
> > +			continue;
> > +
> > +		ret = mt792x_create_tx_page_pool(mdev, q);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (q->page_pool)
> > +			pool_count++;
> > +	}
> > +
> > +	if (pool_count > 0)
> > +		dev_info(mdev->dev,
> > +			 "IOMMU enabled, created %d TX page
> > pools\n", pool_count);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(mt792x_dma_tx_page_pool_init);
> > +
> > +static void mt792x_dma_tx_page_pool_cleanup(struct mt792x_dev
> > *dev)
> > +{
> > +	struct mt76_dev *mdev = &dev->mt76;
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(mdev->phy.q_tx); i++) {
> > +		struct mt76_queue *q = mdev->phy.q_tx[i];
> > +
> > +		if (!q || !q->page_pool)
> > +			continue;
> > +
> > +		page_pool_destroy(q->page_pool);
> > +		q->page_pool = NULL;
> > +	}
> > +
> > +	mdev->tx_prealloc_enabled = false;
> > +}
> > +
> > +static void *
> > +mt792x_dma_tx_alloc_page_pool_buf(struct mt76_dev *dev, struct
> > mt76_queue *q,
> > +				  struct sk_buff *skb, dma_addr_t
> > *dma_addr,
> > +				  int *buf_len)
> > +{
> > +	struct page *page;
> > +	void *buf;
> > +	int len;
> > +	u32 offset;
> > +
> > +	if (!q->page_pool || !dev->tx_prealloc_enabled)
> > +		return NULL;
> > +
> > +	len = skb_headlen(skb);
> > +	if (len > PAGE_SIZE)
> > +		return NULL;
> > +
> > +	buf = mt76_get_page_pool_buf(q, &offset, len);
> > +	if (!buf)
> > +		return NULL;
> > +
> > +	page = virt_to_head_page(buf);
> > +	*dma_addr = page_pool_get_dma_addr(page) + offset;
> > +	if (unlikely(!*dma_addr)) {
> > +		dev_warn_ratelimited(dev->dev, "Page pool returned
> > NULL DMA address\n");
> > +		mt76_put_page_pool_buf(buf, false);
> > +		return NULL;
> > +	}
> > +
> > +	*buf_len = len;
> > +
> > +	dma_sync_single_for_cpu(dev->dma_dev, *dma_addr, len,
> > DMA_TO_DEVICE);
> > +	skb_copy_from_linear_data(skb, buf, len);
> 
> I would like to see some results here since usually it is not a good
> idea to
> copy all the packets (even if it is just the linear part).
> 

The results are shown above.

> > +	dma_sync_single_for_device(dev->dma_dev, *dma_addr, len,
> > DMA_TO_DEVICE);
> > +
> > +	return buf;
> > +}
> > +
> > +static int
> > +mt792x_dma_tx_queue_skb(struct mt76_phy *phy, struct mt76_queue
> > *q,
> > +			enum mt76_txq_id qid, struct sk_buff *skb,
> > +			struct mt76_wcid *wcid, struct
> > ieee80211_sta *sta)
> > +{
> > +	struct ieee80211_tx_status status = {
> > +		.sta = sta,
> > +	};
> > +	struct mt76_tx_info tx_info = {
> > +		.skb = skb,
> > +	};
> > +	struct mt76_dev *dev = phy->dev;
> > +	struct ieee80211_hw *hw;
> > +	int len, n = 0, ret = -ENOMEM;
> > +	struct mt76_txwi_cache *t;
> > +	struct sk_buff *iter;
> > +	dma_addr_t addr;
> > +	u8 *txwi;
> > +
> > +	if (test_bit(MT76_RESET, &phy->state))
> > +		goto free_skb;
> > +
> > +	t = mt76_get_txwi(dev);
> > +	if (!t)
> > +		goto free_skb;
> > +
> > +	txwi = mt76_get_txwi_ptr(dev, t);
> > +
> > +	skb->prev = NULL;
> > +	skb->next = NULL;
> > +	if (dev->drv->drv_flags & MT_DRV_TX_ALIGNED4_SKBS)
> > +		mt76_insert_hdr_pad(skb);
> > +
> > +	len = skb_headlen(skb);
> > +
> > +	if (dev->tx_prealloc_enabled && q->page_pool &&
> > +	    !skb_has_frag_list(skb) && !skb_shinfo(skb)->nr_frags)
> > {
> > +		void *buf;
> > +		int pp_len;
> > +
> > +		buf = mt792x_dma_tx_alloc_page_pool_buf(dev, q,
> > skb, &addr, &pp_len);
> > +		if (buf) {
> > +			t->page_pool_buf = buf;
> > +			len = pp_len;
> > +
> > +			tx_info.buf[n].addr = t->dma_addr;
> > +			tx_info.buf[n++].len = dev->drv-
> > >txwi_size;
> > +			tx_info.buf[n].addr = addr;
> > +			tx_info.buf[n].len = len;
> > +			tx_info.buf[n].skip_unmap = true;
> > +			n++;
> > +
> > +			goto skip_dma_map;
> > +		}
> > +	}
> > +
> > +	addr = dma_map_single(dev->dma_dev, skb->data, len,
> > DMA_TO_DEVICE);
> > +	if (unlikely(dma_mapping_error(dev->dma_dev, addr)))
> > +		goto free;
> > +
> > +	t->page_pool_buf = NULL;
> > +
> > +	tx_info.buf[n].addr = t->dma_addr;
> > +	tx_info.buf[n++].len = dev->drv->txwi_size;
> > +	tx_info.buf[n].addr = addr;
> > +	tx_info.buf[n++].len = len;
> > +
> > +skip_dma_map:
> > +	skb_walk_frags(skb, iter) {
> > +		if (n == ARRAY_SIZE(tx_info.buf))
> > +			goto unmap;
> > +
> > +		addr = dma_map_single(dev->dma_dev, iter->data,
> > iter->len,
> > +				      DMA_TO_DEVICE);
> > +		if (unlikely(dma_mapping_error(dev->dma_dev,
> > addr)))
> > +			goto unmap;
> > +
> > +		tx_info.buf[n].addr = addr;
> > +		tx_info.buf[n].skip_unmap = false;
> > +		tx_info.buf[n++].len = iter->len;
> > +	}
> > +	tx_info.nbuf = n;
> > +
> > +	if (q->queued + (tx_info.nbuf + 1) / 2 >= q->ndesc - 1) {
> > +		ret = -ENOMEM;
> > +		goto unmap;
> > +	}
> > +
> > +	dma_sync_single_for_cpu(dev->dma_dev, t->dma_addr, dev-
> > >drv->txwi_size,
> > +				DMA_TO_DEVICE);
> > +	ret = dev->drv->tx_prepare_skb(dev, txwi, qid, wcid, sta,
> > &tx_info);
> > +	dma_sync_single_for_device(dev->dma_dev, t->dma_addr, dev-
> > >drv->txwi_size,
> > +				   DMA_TO_DEVICE);
> > +	if (ret < 0)
> > +		goto unmap;
> > +
> > +	return mt76_dma_add_buf(dev, q, tx_info.buf, tx_info.nbuf,
> > +				tx_info.info, tx_info.skb, t);
> > +
> > +unmap:
> > +	for (n--; n > 0; n--) {
> > +		if (!tx_info.buf[n].skip_unmap)
> > +			dma_unmap_single(dev->dma_dev,
> > tx_info.buf[n].addr,
> > +					 tx_info.buf[n].len,
> > DMA_TO_DEVICE);
> > +	}
> > +
> > +	if (t->page_pool_buf) {
> > +		mt76_put_page_pool_buf(t->page_pool_buf, false);
> > +		t->page_pool_buf = NULL;
> > +	}
> > +
> > +free:
> > +#ifdef CONFIG_NL80211_TESTMODE
> > +	if (mt76_is_testmode_skb(dev, skb, &hw)) {
> > +		struct mt76_phy *phy = hw->priv;
> > +
> > +		if (tx_info.skb == phy->test.tx_skb)
> > +			phy->test.tx_done--;
> > +	}
> > +#endif
> > +
> > +	mt76_put_txwi(dev, t);
> > +
> > +free_skb:
> > +	status.skb = tx_info.skb;
> > +	hw = mt76_tx_status_get_hw(dev, tx_info.skb);
> > +	spin_lock_bh(&dev->rx_lock);
> > +	ieee80211_tx_status_ext(hw, &status);
> > +	spin_unlock_bh(&dev->rx_lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static void
> > +mt792x_dma_tx_cleanup(struct mt76_dev *dev, struct mt76_queue *q,
> > bool flush)
> 
> this is almost a copy of mt76_dma_tx_cleanup(). Can we do something
> better
> here?
> 

Yes, it's minor enough that there's no need to split the logic into a
separate mt792x_dma_tx_cleanup() function. I will make the change in
mt76_dma_tx_cleanup() in v2. 
 
> > +{
> > +	struct mt76_queue_entry entry;
> > +	int last;
> > +
> > +	if (!q || !q->ndesc)
> > +		return;
> > +
> > +	spin_lock_bh(&q->cleanup_lock);
> > +	if (flush)
> > +		last = -1;
> > +	else
> > +		last = Q_READ(q, dma_idx);
> > +
> > +	while (q->queued > 0 && q->tail != last) {
> > +		mt76_dma_tx_cleanup_idx(dev, q, q->tail, &entry);
> > +		mt76_npu_txdesc_cleanup(q, q->tail);
> > +		mt76_queue_tx_complete(dev, q, &entry);
> > +
> > +		if (entry.txwi) {
> > +			if (entry.skb && entry.txwi-
> > >page_pool_buf) {
> > +				mt76_put_page_pool_buf(entry.txwi-
> > >page_pool_buf, false);
> > +				entry.txwi->page_pool_buf = NULL;
> > +			}
> > +
> > +			if (!(dev->drv->drv_flags &
> > MT_DRV_TXWI_NO_FREE))
> > +				mt76_put_txwi(dev, entry.txwi);
> > +		}
> > +
> > +		if (!flush && q->tail == last)
> > +			last = Q_READ(q, dma_idx);
> > +	}
> > +	spin_unlock_bh(&q->cleanup_lock);
> > +
> > +	if (flush) {
> > +		spin_lock_bh(&q->lock);
> > +		mt76_dma_sync_idx(dev, q);
> > +		mt76_dma_kick_queue(dev, q);
> > +		spin_unlock_bh(&q->lock);
> > +	}
> > +
> > +	if (!q->queued)
> > +		wake_up(&dev->tx_wait);
> > +}
> > +
> > +static struct mt76_queue_ops mt792x_queue_ops;
> > +
> > +void mt792x_dma_attach(struct mt76_dev *dev)
> > +{
> > +	mt792x_queue_ops = mt76_dma_ops;
> 
> can you please take a look to mt7615_mmio_probe()?
> 
> Regards,
> Lorenzo
> 

Yes, I've referenced it and will make this change in v2 if you're OK
with it.

    void mt792x_dma_attach(struct mt76_dev *dev)
    {
    -    mt702x_queue_ops = mt76_dma_ops;
    +    mt76_dma_attach(dev);
    +    mt792x_queue_ops = *dev->queue_ops;
    +    mt792x_queue_ops.tx_queue_skb = mt792x_dma_tx_queue_skb;
    -    mt792x_queue_ops.tx_cleanup = mt792x_dma_tx_cleanup;
         dev->queue_ops = &mt792x_queue_ops;
    }

Thanks,
Eason

> > +
> > +	mt792x_queue_ops.tx_queue_skb = mt792x_dma_tx_queue_skb;
> > +	mt792x_queue_ops.tx_cleanup = mt792x_dma_tx_cleanup;
> > +
> > +	dev->queue_ops = &mt792x_queue_ops;
> > +}
> > +EXPORT_SYMBOL_GPL(mt792x_dma_attach);
> > +
> >  int mt792x_dma_disable(struct mt792x_dev *dev, bool force)
> >  {
> >  	/* disable WFDMA0 */
> > @@ -326,6 +646,8 @@ void mt792x_dma_cleanup(struct mt792x_dev *dev)
> >  		 MT_WFDMA0_RST_LOGIC_RST);
> >  
> >  	mt76_dma_cleanup(&dev->mt76);
> > +
> > +	mt792x_dma_tx_page_pool_cleanup(dev);
> >  }
> >  EXPORT_SYMBOL_GPL(mt792x_dma_cleanup);
> >  
> > -- 
> > 2.45.2
> >