Re: [PATCH 1/2] wifi: mt76: mt7915: rework tx bytes/packets when WED is active

["lorenzo.bianconi@redhat.com" <lorenzo.bianconi@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4039 bytes --]

> On Tue, 2023-04-25 at 10:49 +0200, Lorenzo Bianconi wrote:
> > > From: Peter Chiu <chui-hao.chiu@mediatek.com>
> > > 
> > > 1. Mixed tx_byte calculation - need to handle both (non)binding
> > > packets
> > >    when WED is enabled.
> > > 2. PPDU based TxS can only reports MPDU counts whereas mac80211
> > > requires
> > >    MSDU unit(NL80211_STA_INFO_TX_PACKETS).
> > > 
> > > To solve above issues - switch to use TxS reporting for all tx_byte
> > > when
> > > WED is active and get MSDU Tx counts from WA statistic.
> > 
> > It seems to me we are doining multiple logic tasks in this patch. Can
> > you
> > please split them on multiple patches?
> > 
> 
> OK.
> > > 
> > > Note that mt7915 WA firmware only counts tx_packet for WED path, so
> > > driver needs to take care of host path.
> > > 
> > > Fixes: 43eaa3689507 ("wifi: mt76: add PPDU based TxS support for
> > > WED device")
> > > Co-developed-by: Ryder Lee <ryder.lee@mediatek.com>
> > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > > Signed-off-by: Peter Chiu <chui-hao.chiu@mediatek.com>
> > > ---
> > >  drivers/net/wireless/mediatek/mt76/mt76.h     |  2 +-
> > >  .../wireless/mediatek/mt76/mt76_connac_mac.c  |  8 +-
> > >  .../wireless/mediatek/mt76/mt76_connac_mcu.h  |  1 +
> > >  .../net/wireless/mediatek/mt76/mt7915/init.c  |  6 ++
> > >  .../net/wireless/mediatek/mt76/mt7915/main.c  |  6 +-
> > >  .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 74
> > > +++++++++++++++++--
> > >  .../net/wireless/mediatek/mt76/mt7915/mmio.c  | 30 +-------
> > >  .../wireless/mediatek/mt76/mt7915/mt7915.h    |  1 +
> > >  drivers/net/wireless/mediatek/mt76/tx.c       |  6 ++
> > >  9 files changed, 91 insertions(+), 43 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h
> > > b/drivers/net/wireless/mediatek/mt76/mt76.h
> > > index 6b07b8fafec2..0e9f4197213a 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> > > @@ -277,7 +277,7 @@ struct mt76_sta_stats {
> > >  	u64 tx_mcs[16];		/* mcs idx */
> > >  	u64 tx_bytes;
> > >  	/* WED TX */
> > > -	u32 tx_packets;
> > > +	u32 tx_packets;		/* unit: MSDU */
> > >  	u32 tx_retries;
> > >  	u32 tx_failed;
> > >  	/* WED RX */
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > > b/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > > index d39a3cc5e381..84985a989293 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > > @@ -521,9 +521,9 @@ void mt76_connac2_mac_write_txwi(struct
> > > mt76_dev *dev, __le32 *txwi,
> > >  		q_idx = wmm_idx * MT76_CONNAC_MAX_WMM_SETS +
> > >  			mt76_connac_lmac_mapping(skb_get_queue_mapping(
> > > skb));
> > >  
> > > -		/* counting non-offloading skbs */
> > > -		wcid->stats.tx_bytes += skb->len;
> > > -		wcid->stats.tx_packets++;
> > > +		/* mt7915 WA only counts WED path */
> > > +		if (mtk_wed_device_active(&dev->mmio.wed) &&
> > > is_mt7915(dev))
> > > +			wcid->stats.tx_packets++;
> > 
> > I think this will crash mt7921 usb/sdio drivers. It is enough to swap
> > the conditions since mt7915 supports mmio only mode.
> > 
> 
> OK.
> > >  	}
> > >  
> > >  	val = FIELD_PREP(MT_TXD0_TX_BYTES, skb->len + sz_txd) |
> > > @@ -610,8 +610,6 @@ bool mt76_connac2_mac_fill_txs(struct mt76_dev
> > > *dev, struct mt76_wcid *wcid,
> > >  		stats->tx_bytes +=
> > >  			le32_get_bits(txs_data[5],
> > > MT_TXS5_MPDU_TX_BYTE) -
> > >  			le32_get_bits(txs_data[7],
> > > MT_TXS7_MPDU_RETRY_BYTE);
> > > -		stats->tx_packets +=
> > > -			le32_get_bits(txs_data[5],
> > > MT_TXS5_MPDU_TX_CNT);
> > 
> > I think this will break accounting for mt7921, right?
> > 
> 
> Why? I don't think mt7921 supports WED. It should not go into this
> chunk.

ack, right. mt7921 just supports MT_TXS0_TXS_FORMAT <= 1.

Regards,
Lorenzo

> 
> > Regards,
> > Lorenzo
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]