2019-11-20 22:31:42

by Markus Theil

[permalink] [raw]
Subject: [PATCH v7 0/5] mt76: channel switch support for USB devices

This patch series adds channel switch support for mt76 usb interfaces.
When testing, I noticed that between 5 or 7 consecutive beacons had the
identical channel switch count set. After some debugging I found out,
that beacon copying over usb took far too long (up to 700ms for one call
of mt76x02u_pre_tbtt_work).

Therefore the first four patches speed up beacon copying and the last
patch enables channel switch support also for usb interfaces.

v7 (thanks to Stanislaw):
* fix mbss beacon settings
* fix compilation with latest upstream

v6:
* use min_t in mt76u_copy
* use round_up in mt76u_copy
* use additional copy for mmio beacon set again

v5 (thanks to Stanislaw):
* ommit empty mt76x2u_channel_switch_beacon
* copy txwi into beacon skb

v4:
* use multiple of 4 len for usb copy again

v3:
* fixed checkpatch errors

v2 (thanks for the comments Lorenzo):
* correctly track beacon data mask
* clean-ups
* make channel switch fn static (reported by kbuild test robot)

Markus Theil (5):
mt76: mt76x02: ommit beacon slot clearing
mt76: mt76x02: split beaconing
mt76: mt76x02: remove a copy call for usb speedup
mt76: speed up usb bulk copy
mt76: mt76x02: add channel switch support for usb interfaces

drivers/net/wireless/mediatek/mt76/mt76.h | 2 +-
.../wireless/mediatek/mt76/mt76x02_beacon.c | 73 +++++++------------
.../net/wireless/mediatek/mt76/mt76x02_mac.h | 1 +
.../net/wireless/mediatek/mt76/mt76x02_mmio.c | 5 ++
.../wireless/mediatek/mt76/mt76x02_usb_core.c | 12 +++
.../net/wireless/mediatek/mt76/mt76x02_util.c | 2 +-
drivers/net/wireless/mediatek/mt76/usb.c | 24 ++++--
7 files changed, 66 insertions(+), 53 deletions(-)

--
2.24.0



2019-11-20 22:31:45

by Markus Theil

[permalink] [raw]
Subject: [PATCH v7 1/5] mt76: mt76x02: ommit beacon slot clearing

mt76 hw does not send beacons from beacon slots, if the corresponding
bitmask is set accordingly. Therefore we can ommit clearing the beacon
memory. Clearing uses many usb calls, if usb drivers are used. These
calls unnecessarily slow down the beacon tasklet. Thanks to Stanislaw
Gruzska for pointing this out.

Signed-off-by: Markus Theil <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 8 --------
1 file changed, 8 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
index 4209209ac940..403866496640 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
@@ -58,8 +58,6 @@ __mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 bcn_idx,
dev->beacon_data_mask |= BIT(bcn_idx);
} else {
dev->beacon_data_mask &= ~BIT(bcn_idx);
- for (i = 0; i < beacon_len; i += 4)
- mt76_wr(dev, beacon_addr + i, 0);
}

mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
@@ -241,17 +239,11 @@ EXPORT_SYMBOL_GPL(mt76x02_enqueue_buffered_bc);

void mt76x02_init_beacon_config(struct mt76x02_dev *dev)
{
- int i;
-
mt76_clear(dev, MT_BEACON_TIME_CFG, (MT_BEACON_TIME_CFG_TIMER_EN |
MT_BEACON_TIME_CFG_TBTT_EN |
MT_BEACON_TIME_CFG_BEACON_TX));
mt76_set(dev, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE);
mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff);
-
- for (i = 0; i < 8; i++)
- mt76x02_mac_set_beacon(dev, i, NULL);
-
mt76x02_set_beacon_offsets(dev);
}
EXPORT_SYMBOL_GPL(mt76x02_init_beacon_config);
--
2.24.0


2019-11-20 22:31:47

by Markus Theil

[permalink] [raw]
Subject: [PATCH v7 2/5] mt76: mt76x02: split beaconing

Sending beacons to the hardware always happens in batches. In order to
speed up beacon processing on usb devices, this patch splits out common
code an calls it only once (mt76x02_mac_set_beacon_prepare,
mt76x02_mac_set_beacon_finish). Making this split breaks beacon
enabling/disabling per vif. This is fixed by adding a call to set the
bypass mask, if beaconing should be disabled for a vif. Otherwise the
beacon is send after the next beacon interval.

The code is also adapted for the mmio part of the driver, but should not
have any performance implication there.

Signed-off-by: Markus Theil <[email protected]>
---
.../wireless/mediatek/mt76/mt76x02_beacon.c | 63 ++++++++-----------
.../net/wireless/mediatek/mt76/mt76x02_mac.h | 1 +
.../net/wireless/mediatek/mt76/mt76x02_mmio.c | 5 ++
.../wireless/mediatek/mt76/mt76x02_usb_core.c | 5 ++
4 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
index 403866496640..50e68af63d4f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
@@ -31,10 +31,22 @@ mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb)
if (WARN_ON_ONCE(beacon_len < skb->len + sizeof(struct mt76x02_txwi)))
return -ENOSPC;

- mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len);
+ /* USB devices already reserve enough skb headroom for txwi's. This
+ * helps to save slow copies over USB.
+ */
+ if (mt76_is_usb(&dev->mt76)) {
+ struct mt76x02_txwi *txwi;
+
+ mt76_insert_hdr_pad(skb);
+ txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi));
+ mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len);
+ skb_push(skb, sizeof(*txwi));
+ } else {
+ struct mt76x02_txwi txwi;

- mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
- offset += sizeof(txwi);
+ mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
+ offset += sizeof(txwi);
+ }

mt76_wr_copy(dev, offset, skb->data, skb->len);
return 0;
@@ -47,10 +59,6 @@ __mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 bcn_idx,
int beacon_len = dev->beacon_ops->slot_size;
int beacon_addr = MT_BEACON_BASE + (beacon_len * bcn_idx);
int ret = 0;
- int i;
-
- /* Prevent corrupt transmissions during update */
- mt76_set(dev, MT_BCN_BYPASS_MASK, BIT(bcn_idx));

if (skb) {
ret = mt76x02_write_beacon(dev, beacon_addr, skb);
@@ -60,41 +68,23 @@ __mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 bcn_idx,
dev->beacon_data_mask &= ~BIT(bcn_idx);
}

- mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
-
return ret;
}

+void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
+{
+ mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N,
+ hweight8(dev->beacon_data_mask) - 1);
+ mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
+}
+EXPORT_SYMBOL_GPL(mt76x02_mac_set_beacon_finish);
+
int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
struct sk_buff *skb)
{
- bool force_update = false;
- int bcn_idx = 0;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(dev->beacons); i++) {
- if (vif_idx == i) {
- force_update = !!dev->beacons[i] ^ !!skb;
- dev_kfree_skb(dev->beacons[i]);
- dev->beacons[i] = skb;
- __mt76x02_mac_set_beacon(dev, bcn_idx, skb);
- } else if (force_update && dev->beacons[i]) {
- __mt76x02_mac_set_beacon(dev, bcn_idx,
- dev->beacons[i]);
- }
-
- bcn_idx += !!dev->beacons[i];
- }
-
- for (i = bcn_idx; i < ARRAY_SIZE(dev->beacons); i++) {
- if (!(dev->beacon_data_mask & BIT(i)))
- break;
-
- __mt76x02_mac_set_beacon(dev, i, NULL);
- }
-
- mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N,
- bcn_idx - 1);
+ dev_kfree_skb(dev->beacons[vif_idx]);
+ dev->beacons[vif_idx] = skb;
+ __mt76x02_mac_set_beacon(dev, vif_idx, skb);
return 0;
}
EXPORT_SYMBOL_GPL(mt76x02_mac_set_beacon);
@@ -115,6 +105,7 @@ void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
} else {
dev->mt76.beacon_mask &= ~BIT(mvif->idx);
mt76x02_mac_set_beacon(dev, mvif->idx, NULL);
+ mt76_set(dev, MT_BCN_BYPASS_MASK, BIT(mvif->idx));
}

if (!!old_mask == !!dev->mt76.beacon_mask)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
index 7d946aa77182..26853f7acba9 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -205,6 +205,7 @@ int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
struct sk_buff *skb);
void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
struct ieee80211_vif *vif, bool enable);
+void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev);

void mt76x02_edcca_init(struct mt76x02_dev *dev);
#endif
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
index 4e2371c926d8..644bc8c284cc 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
@@ -24,10 +24,15 @@ static void mt76x02_pre_tbtt_tasklet(unsigned long arg)

mt76x02_resync_beacon_timer(dev);

+ /* Prevent corrupt transmissions during update */
+ mt76_set(dev, MT_BCN_BYPASS_MASK, 0xffff);
+
ieee80211_iterate_active_interfaces_atomic(mt76_hw(dev),
IEEE80211_IFACE_ITER_RESUME_ALL,
mt76x02_update_beacon_iter, dev);

+ mt76x02_mac_set_beacon_finish(dev);
+
mt76_csa_check(&dev->mt76);

if (dev->mt76.csa_complete)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
index d03d3c8e296c..0b3437a30b02 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
@@ -208,6 +208,9 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)

mt76x02_resync_beacon_timer(dev);

+ /* Prevent corrupt transmissions during update */
+ mt76_set(dev, MT_BCN_BYPASS_MASK, 0xffff);
+
ieee80211_iterate_active_interfaces(mt76_hw(dev),
IEEE80211_IFACE_ITER_RESUME_ALL,
mt76x02_update_beacon_iter, dev);
@@ -220,6 +223,8 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
mt76x02_mac_set_beacon(dev, i, skb);
}

+ mt76x02_mac_set_beacon_finish(dev);
+
mt76x02u_restart_pre_tbtt_timer(dev);
}

--
2.24.0


2019-11-21 12:33:03

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] mt76: mt76x02: split beaconing

On Wed, Nov 20, 2019 at 11:28:23PM +0100, Markus Theil wrote:
> Sending beacons to the hardware always happens in batches. In order to
> speed up beacon processing on usb devices, this patch splits out common
> code an calls it only once (mt76x02_mac_set_beacon_prepare,
> mt76x02_mac_set_beacon_finish). Making this split breaks beacon
> enabling/disabling per vif. This is fixed by adding a call to set the
> bypass mask, if beaconing should be disabled for a vif. Otherwise the
> beacon is send after the next beacon interval.
>
> The code is also adapted for the mmio part of the driver, but should not
> have any performance implication there.
<snip>
> + */
> + if (mt76_is_usb(&dev->mt76)) {
> + struct mt76x02_txwi *txwi;
> +
> + mt76_insert_hdr_pad(skb);
> + txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi));
> + mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len);
> + skb_push(skb, sizeof(*txwi));
> + } else {
> + struct mt76x02_txwi txwi;
>
> - mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
> - offset += sizeof(txwi);
> + mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
> + offset += sizeof(txwi);
> + }

You merged another patch into this one. Please keep them separated.

> +void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
> +{
> + mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N,
> + hweight8(dev->beacon_data_mask) - 1);
> + mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
> +}

Well, this code still does not look quite right. At least it is not
compatible what the BCN_BYPASS_MASK description said in the manual.

I think the code need serious testing on multi-bss (USB support up
to 2 AP bssids) and with broadcast/multicast with some PS stations
on the network.

In particular adding second bssid and remove first one should
be checked.

Stanislaw