2019-11-19 12:25:05

by Markus Theil

[permalink] [raw]
Subject: [PATCH v5 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.

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 | 62 ++++++-------------
.../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, 58 insertions(+), 50 deletions(-)

--
2.24.0



2019-11-19 12:25:05

by Markus Theil

[permalink] [raw]
Subject: [PATCH v5 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 | 44 +++++++------------
.../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, 26 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
index 403866496640..09013adae854 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
@@ -47,10 +47,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 +56,30 @@ __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;
}

-int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
- struct sk_buff *skb)
+void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
{
- bool force_update = false;
- int bcn_idx = 0;
int i;
+ int bcn_idx = 0;

- 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]);
- }
-
+ for (i = 0; i < hweight8(dev->mt76.beacon_mask); ++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);
+
+ 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)
+{
+ 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 +100,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 efa4ef945e35..e6e585c72f0d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -197,6 +197,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 dc773070481d..73c39d03b7f8 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 78dfc1e7f27b..8a2a90fb5663 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
@@ -179,6 +179,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);
@@ -191,6 +194,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-19 12:25:11

by Markus Theil

[permalink] [raw]
Subject: [PATCH v5 5/5] mt76: mt76x02: add channel switch support for usb interfaces

This patch enables channel switch support on mt76 usb interfaces.

Signed-off-by: Markus Theil <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c | 7 +++++++
drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
index 8a2a90fb5663..891825043117 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
@@ -182,6 +182,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
/* Prevent corrupt transmissions during update */
mt76_set(dev, MT_BCN_BYPASS_MASK, 0xffff);

+ mt76_csa_check(&dev->mt76);
+ if (dev->mt76.csa_complete) {
+ mt76_csa_finish(&dev->mt76);
+ goto out;
+ }
+
ieee80211_iterate_active_interfaces(mt76_hw(dev),
IEEE80211_IFACE_ITER_RESUME_ALL,
mt76x02_update_beacon_iter, dev);
@@ -196,6 +202,7 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)

mt76x02_mac_set_beacon_finish(dev);

+out:
mt76x02u_restart_pre_tbtt_timer(dev);
}

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 414b22399d93..3f95e5b24e1d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -174,7 +174,6 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
wiphy->reg_notifier = mt76x02_regd_notifier;
wiphy->iface_combinations = mt76x02_if_comb;
wiphy->n_iface_combinations = ARRAY_SIZE(mt76x02_if_comb);
- wiphy->flags |= WIPHY_FLAG_HAS_CHANNEL_SWITCH;

/* init led callbacks */
if (IS_ENABLED(CONFIG_MT76_LEDS)) {
@@ -184,6 +183,7 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
}
}

+ wiphy->flags |= WIPHY_FLAG_HAS_CHANNEL_SWITCH;
wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_VHT_IBSS);

hw->sta_data_size = sizeof(struct mt76x02_sta);
--
2.24.0


2019-11-19 12:25:16

by Markus Theil

[permalink] [raw]
Subject: [PATCH v5 4/5] mt76: speed up usb bulk copy

Use larger batches for usb copy to speed this operation up. Otherwise it
would be too slow for copying new beacons or broadcast frames over usb.
Assure, that always a multiple of 4 Bytes is copied, as outlined in
850e8f6fbd "mt76: round up length on mt76_wr_copy" from Felix Fietkau.

Signed-off-by: Markus Theil <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76.h | 2 +-
drivers/net/wireless/mediatek/mt76/usb.c | 24 +++++++++++++++++------
2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 8aec7ccf2d79..7a6f5d097a3d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -383,7 +383,7 @@ enum mt76u_out_ep {
struct mt76_usb {
struct mutex usb_ctrl_mtx;
union {
- u8 data[32];
+ u8 data[128];
__le32 reg_val;
};

diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 20c6fe510e9d..f1f67b0f8265 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -149,18 +149,30 @@ static void mt76u_copy(struct mt76_dev *dev, u32 offset,
const void *data, int len)
{
struct mt76_usb *usb = &dev->usb;
- const u32 *val = data;
- int i, ret;
+ const u8 *val = data;
+ int ret;
+ int current_batch_size;
+ int i = 0;
+
+ /* Assure that always a multiple of 4 bytes are copied,
+ * otherwise beacons can be corrupted.
+ * See: "mt76: round up length on mt76_wr_copy"
+ * Commit 850e8f6fbd5d0003b0
+ */
+ len = DIV_ROUND_UP(len, 4) * 4;

mutex_lock(&usb->usb_ctrl_mtx);
- for (i = 0; i < DIV_ROUND_UP(len, 4); i++) {
- put_unaligned(val[i], (u32 *)usb->data);
+ while (i < len) {
+ current_batch_size = min((int)sizeof(usb->data), len - i);
+ memcpy(usb->data, val + i, current_batch_size);
ret = __mt76u_vendor_request(dev, MT_VEND_MULTI_WRITE,
USB_DIR_OUT | USB_TYPE_VENDOR,
- 0, offset + i * 4, usb->data,
- sizeof(u32));
+ 0, offset + i, usb->data,
+ current_batch_size);
if (ret < 0)
break;
+
+ i += current_batch_size;
}
mutex_unlock(&usb->usb_ctrl_mtx);
}
--
2.24.0


2019-11-19 12:25:47

by Markus Theil

[permalink] [raw]
Subject: [PATCH v5 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-19 12:27:49

by Markus Theil

[permalink] [raw]
Subject: [PATCH v5 3/5] mt76: mt76x02: remove a copy call for usb speedup

This patch removes a mt76_wr_copy call from the beacon path to hw.
The skb which is used in this place gets therefore build with txwi
inside its data. For mt76 usb drivers, this saves one synchronuous
copy call over usb, which lets the beacon work complete faster.

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

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
index 09013adae854..a9df8f2d41b8 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
@@ -26,15 +26,15 @@ static int
mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb)
{
int beacon_len = dev->beacon_ops->slot_size;
- struct mt76x02_txwi txwi;
+ struct mt76x02_txwi *txwi;

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);
-
- mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
- offset += sizeof(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));

mt76_wr_copy(dev, offset, skb->data, skb->len);
return 0;
--
2.24.0


2019-11-19 14:47:43

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] mt76: mt76x02: remove a copy call for usb speedup

On Tue, Nov 19, 2019 at 01:24:08PM +0100, Markus Theil wrote:
> This patch removes a mt76_wr_copy call from the beacon path to hw.
> The skb which is used in this place gets therefore build with txwi
> inside its data. For mt76 usb drivers, this saves one synchronuous
> copy call over usb, which lets the beacon work complete faster.
>
> Signed-off-by: Markus Theil <[email protected]>
> ---
> drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
> index 09013adae854..a9df8f2d41b8 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
> @@ -26,15 +26,15 @@ static int
> mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb)
> {
> int beacon_len = dev->beacon_ops->slot_size;
> - struct mt76x02_txwi txwi;
> + struct mt76x02_txwi *txwi;
>
> 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);
> -
> - mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
> - offset += sizeof(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));

This require enough skb headroom for txwi . Mac80211 reserve headroom
for us according to hw->extra_tx_headroom . Currently we ask for extra
space for txwi only for usb in mt76x02_init_device().

So this code should be put under mt76_is_usb() condition or,
what I think is less preferred, reserve extra space also for mmio.

Stanislaw