2016-11-02 14:12:23

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 00/10] rt2800: random fixes

Random fixes mostly related to HT performance.

Stanislaw Gruszka (10):
rt2800: correctly report MCS TX parameters
rt2800usb: do not wipe out USB_DMA_CFG settings
rt2800: OFDM rates are mandatory
rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE
rt2800: make ba_size depend on ampdu_factor
rt2800: correct AUTO_RSP_CFG
rt2800: correct TX_SW_CFG1 for 5592
rt2800: use RTS/CTS protection instead of CTS-to-self
rt2800: tune *_PROT_CFG parameters
rt2800: disable CCK rates on HT

drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 62 +++++++++++----------
drivers/net/wireless/ralink/rt2x00/rt2800usb.c | 5 +--
drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 15 ++++--
3 files changed, 43 insertions(+), 39 deletions(-)


2016-11-02 14:12:35

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 09/10] rt2800: tune *_PROT_CFG parameters

Use RTS/CTS protection for TXOP on all rates modes as default and
disable CCK rates (this cause performance problems).

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 57bfec4..8d35b2e 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -4771,9 +4771,9 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)

rt2800_register_read(rt2x00dev, MM20_PROT_CFG, &reg);
rt2x00_set_field32(&reg, MM20_PROT_CFG_PROTECT_RATE, 0x4004);
- rt2x00_set_field32(&reg, MM20_PROT_CFG_PROTECT_CTRL, 0);
+ rt2x00_set_field32(&reg, MM20_PROT_CFG_PROTECT_CTRL, 1);
rt2x00_set_field32(&reg, MM20_PROT_CFG_PROTECT_NAV_SHORT, 1);
- rt2x00_set_field32(&reg, MM20_PROT_CFG_TX_OP_ALLOW_CCK, 1);
+ rt2x00_set_field32(&reg, MM20_PROT_CFG_TX_OP_ALLOW_CCK, 0);
rt2x00_set_field32(&reg, MM20_PROT_CFG_TX_OP_ALLOW_OFDM, 1);
rt2x00_set_field32(&reg, MM20_PROT_CFG_TX_OP_ALLOW_MM20, 1);
rt2x00_set_field32(&reg, MM20_PROT_CFG_TX_OP_ALLOW_MM40, 0);
@@ -4784,9 +4784,9 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)

rt2800_register_read(rt2x00dev, MM40_PROT_CFG, &reg);
rt2x00_set_field32(&reg, MM40_PROT_CFG_PROTECT_RATE, 0x4084);
- rt2x00_set_field32(&reg, MM40_PROT_CFG_PROTECT_CTRL, 0);
+ rt2x00_set_field32(&reg, MM40_PROT_CFG_PROTECT_CTRL, 1);
rt2x00_set_field32(&reg, MM40_PROT_CFG_PROTECT_NAV_SHORT, 1);
- rt2x00_set_field32(&reg, MM40_PROT_CFG_TX_OP_ALLOW_CCK, 1);
+ rt2x00_set_field32(&reg, MM40_PROT_CFG_TX_OP_ALLOW_CCK, 0);
rt2x00_set_field32(&reg, MM40_PROT_CFG_TX_OP_ALLOW_OFDM, 1);
rt2x00_set_field32(&reg, MM40_PROT_CFG_TX_OP_ALLOW_MM20, 1);
rt2x00_set_field32(&reg, MM40_PROT_CFG_TX_OP_ALLOW_MM40, 1);
@@ -4797,9 +4797,9 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)

rt2800_register_read(rt2x00dev, GF20_PROT_CFG, &reg);
rt2x00_set_field32(&reg, GF20_PROT_CFG_PROTECT_RATE, 0x4004);
- rt2x00_set_field32(&reg, GF20_PROT_CFG_PROTECT_CTRL, 0);
+ rt2x00_set_field32(&reg, GF20_PROT_CFG_PROTECT_CTRL, 1);
rt2x00_set_field32(&reg, GF20_PROT_CFG_PROTECT_NAV_SHORT, 1);
- rt2x00_set_field32(&reg, GF20_PROT_CFG_TX_OP_ALLOW_CCK, 1);
+ rt2x00_set_field32(&reg, GF20_PROT_CFG_TX_OP_ALLOW_CCK, 0);
rt2x00_set_field32(&reg, GF20_PROT_CFG_TX_OP_ALLOW_OFDM, 1);
rt2x00_set_field32(&reg, GF20_PROT_CFG_TX_OP_ALLOW_MM20, 1);
rt2x00_set_field32(&reg, GF20_PROT_CFG_TX_OP_ALLOW_MM40, 0);
@@ -4810,9 +4810,9 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)

rt2800_register_read(rt2x00dev, GF40_PROT_CFG, &reg);
rt2x00_set_field32(&reg, GF40_PROT_CFG_PROTECT_RATE, 0x4084);
- rt2x00_set_field32(&reg, GF40_PROT_CFG_PROTECT_CTRL, 0);
+ rt2x00_set_field32(&reg, GF40_PROT_CFG_PROTECT_CTRL, 1);
rt2x00_set_field32(&reg, GF40_PROT_CFG_PROTECT_NAV_SHORT, 1);
- rt2x00_set_field32(&reg, GF40_PROT_CFG_TX_OP_ALLOW_CCK, 1);
+ rt2x00_set_field32(&reg, GF40_PROT_CFG_TX_OP_ALLOW_CCK, 0);
rt2x00_set_field32(&reg, GF40_PROT_CFG_TX_OP_ALLOW_OFDM, 1);
rt2x00_set_field32(&reg, GF40_PROT_CFG_TX_OP_ALLOW_MM20, 1);
rt2x00_set_field32(&reg, GF40_PROT_CFG_TX_OP_ALLOW_MM40, 1);
--
1.7.1

2016-11-14 08:44:25

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE

On Sat, Nov 05, 2016 at 01:55:14PM +0100, Mathias Kresin wrote:
> 02.11.2016 15:10, Stanislaw Gruszka:
> >We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on
> >rt2800_init_registers().
> >
> >Signed-off-by: Stanislaw Gruszka <[email protected]>
> >---
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
> >
> >diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >index feceb13..9ecdc4c 100644
> >--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >@@ -6756,7 +6756,6 @@ int rt2800_enable_radio(struct rt2x00_dev *rt2x00dev)
> > rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
> > rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 1);
> > rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 1);
> >- rt2x00_set_field32(&reg, WPDMA_GLO_CFG_WP_DMA_BURST_SIZE, 2);
>
> More a notice than a potential issue since I don't have much
> knowledge about the driver/chip internals.
>
> But WPDMA_GLO_CFG_WP_DMA_BURST_SIZE in rt2800_init_registers() is
> set conditionally for rt2x00_is_usb(rt2x00dev), where this one is
> set unconditionally. Not sure if this change has side effects.

Default HW setting of WP_DMA_BURTS_SIZE is 2, hence patch does not
change behaviour on PCI devices. However I looked at RT3290 and
RT5592 PCI vendor drivers and there this value is initialized to 3.
So I think we can remove is_usb condition in rt2800_init_registers().

Stanislaw

2016-11-14 09:45:34

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

Hi

On Mon, Nov 14, 2016 at 08:41:57PM +1100, Julian Calaby wrote:
> > - rt2x00_set_field32(&reg, MAX_LEN_CFG_MAX_PSDU, 1);
> > + rt2x00_set_field32(&reg, MAX_LEN_CFG_MAX_PSDU, 3;
>
> You're missing a closing parenthesis, so it isn't going to work unless
> it's added back in.

Thanks for the notice, hopefully Mathias can fix that.

Stanislaw

2016-11-05 12:57:00

by Mathias Kresin

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

02.11.2016 15:11, Stanislaw Gruszka:
> We can calculate BA window size (max number of pending frames not
> yet block acked) of remote station using Maximum A-MPDU length factor
> for that station.
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>
> ---
> drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 15 ++++++++++-----
> 1 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> index 68b620b..9da89e3 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> @@ -305,14 +305,19 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
> struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
> struct ieee80211_tx_rate *txrate = &tx_info->control.rates[0];
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> - struct rt2x00_sta *sta_priv = NULL;
> + u8 ba_size = 0;
>
> if (sta) {
> - txdesc->u.ht.mpdu_density =
> - sta->ht_cap.ampdu_density;
> + struct rt2x00_sta *sta_priv = sta_to_rt2x00_sta(sta);
>
> - sta_priv = sta_to_rt2x00_sta(sta);
> + txdesc->u.ht.mpdu_density = sta->ht_cap.ampdu_density;
> txdesc->u.ht.wcid = sta_priv->wcid;
> +
> + if (!(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)) {
> + ba_size = IEEE80211_MIN_AMPDU_BUF;
> + ba_size <<= sta->ht_cap.ampdu_factor;
> + ba_size = min_t(int, 63, ba_size - 1);
> + }
> }
>
> /*
> @@ -345,7 +350,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
> return;
> }
>
> - txdesc->u.ht.ba_size = 7; /* FIXME: What value is needed? */
> + txdesc->u.ht.ba_size = ba_size;
>
> /*
> * Only one STBC stream is supported for now.
>

Having this patch applied, the throughput on a vgv7510kw22 (RT3062F) in
AP mode using LEDE head is decreased by somewhat around 10 Mbits/sec.
I'm using iperf3 for throughput tests and having this patch reverted the
throughout is back to 80 Mbits/sec.

When bringing down the wifi interface the following messages are logged
with the patch applied:

[ 281.738373] ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue
2 failed to flush
[ 281.906380] ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue
2 failed to flush

Mathias

2016-11-02 14:12:32

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 07/10] rt2800: correct TX_SW_CFG1 for 5592

Those TX_SW_CFG1 values are used in vendor driver.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index ff4a7c3..812f8e7 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -4670,11 +4670,14 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
0x00000000);
}
} else if (rt2x00_rt(rt2x00dev, RT5390) ||
- rt2x00_rt(rt2x00dev, RT5392) ||
- rt2x00_rt(rt2x00dev, RT5592)) {
+ rt2x00_rt(rt2x00dev, RT5392)) {
rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000404);
rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x00080606);
rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x00000000);
+ } else if (rt2x00_rt(rt2x00dev, RT5592)) {
+ rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000404);
+ rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x00000000);
+ rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x00000000);
} else {
rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000000);
rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x00080606);
--
1.7.1

2016-11-14 08:46:56

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

On Sat, Nov 05, 2016 at 01:56:58PM +0100, Mathias Kresin wrote:
> 02.11.2016 15:11, Stanislaw Gruszka:
> >
> >- sta_priv = sta_to_rt2x00_sta(sta);
> >+ txdesc->u.ht.mpdu_density = sta->ht_cap.ampdu_density;
> > txdesc->u.ht.wcid = sta_priv->wcid;
> >+
> >+ if (!(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)) {
> >+ ba_size = IEEE80211_MIN_AMPDU_BUF;
> >+ ba_size <<= sta->ht_cap.ampdu_factor;
> >+ ba_size = min_t(int, 63, ba_size - 1);
> >+ }
> > }
> >
> > /*
> >@@ -345,7 +350,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
> > return;
> > }
> >
> >- txdesc->u.ht.ba_size = 7; /* FIXME: What value is needed? */
> >+ txdesc->u.ht.ba_size = ba_size;
> >
> > /*
> > * Only one STBC stream is supported for now.
> >
>
> Having this patch applied, the throughput on a vgv7510kw22 (RT3062F)
> in AP mode using LEDE head is decreased by somewhat around 10
> Mbits/sec. I'm using iperf3 for throughput tests and having this
> patch reverted the throughout is back to 80 Mbits/sec.
>
> When bringing down the wifi interface the following messages are
> logged with the patch applied:
>
> [ 281.738373] ieee80211 phy0: rt2x00queue_flush_queue: Warning -
> Queue 2 failed to flush
> [ 281.906380] ieee80211 phy0: rt2x00queue_flush_queue: Warning -
> Queue 2 failed to flush

Could you check below patch and see if it helps? If it does not,
could you printk sta->ht_cap.ampdu_density and ba_size values
and provide them here.

Thanks
Stanislaw

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 2515702..72c7948 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -4707,7 +4707,7 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
rt2x00_rt_rev_lt(rt2x00dev, RT3070, REV_RT3070E))
rt2x00_set_field32(&reg, MAX_LEN_CFG_MAX_PSDU, 2);
else
- rt2x00_set_field32(&reg, MAX_LEN_CFG_MAX_PSDU, 1);
+ rt2x00_set_field32(&reg, MAX_LEN_CFG_MAX_PSDU, 3;
rt2x00_set_field32(&reg, MAX_LEN_CFG_MIN_PSDU, 0);
rt2x00_set_field32(&reg, MAX_LEN_CFG_MIN_MPDU, 0);
rt2800_register_write(rt2x00dev, MAX_LEN_CFG, reg);

2016-11-14 12:44:44

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

On Mon, Nov 14, 2016 at 09:45:36AM +0100, Stanislaw Gruszka wrote:
> Could you check below patch and see if it helps? If it does not,
> could you printk sta->ht_cap.ampdu_density and ba_size values
> and provide them here.

Actually please print parameters from below patch. I think ba_size
should be based on per TID buf_size instead of ampdu_factor, in case
STA has buf size different for some TIDs.

Also adding Felix to cc since my orginal patch:
http://marc.info/?l=linux-wireless&m=147809595316289&w=2
was shamelessly stolen from mt76 driver. Perhaps Felix could provide
us some expertise.

Thanks
Stanislaw

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 2515702..35bc38c 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -7950,6 +7950,8 @@ int rt2800_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
struct rt2x00_sta *sta_priv = (struct rt2x00_sta *)sta->drv_priv;
int ret = 0;

+ printk("action %d sta %pM tid %u buf_size %u ampdu_factor %u\n", params->action, sta->addr, params->tid, params->buf_size, sta->ht_cap.ampdu_factor);
+
/*
* Don't allow aggregation for stations the hardware isn't aware
* of because tx status reports for frames to an unknown station

2016-11-16 08:10:55

by Mathias Kresin

[permalink] [raw]
Subject: Re: [PATCH 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE

14.11.2016 09:42, Stanislaw Gruszka:
> On Sat, Nov 05, 2016 at 01:55:14PM +0100, Mathias Kresin wrote:
>> 02.11.2016 15:10, Stanislaw Gruszka:
>>> We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on
>>> rt2800_init_registers().
>>>
>>> Signed-off-by: Stanislaw Gruszka <[email protected]>
>>> ---
>>> drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 1 -
>>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
>>> index feceb13..9ecdc4c 100644
>>> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
>>> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
>>> @@ -6756,7 +6756,6 @@ int rt2800_enable_radio(struct rt2x00_dev *rt2x00dev)
>>> rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
>>> rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 1);
>>> rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 1);
>>> - rt2x00_set_field32(&reg, WPDMA_GLO_CFG_WP_DMA_BURST_SIZE, 2);
>>
>> More a notice than a potential issue since I don't have much
>> knowledge about the driver/chip internals.
>>
>> But WPDMA_GLO_CFG_WP_DMA_BURST_SIZE in rt2800_init_registers() is
>> set conditionally for rt2x00_is_usb(rt2x00dev), where this one is
>> set unconditionally. Not sure if this change has side effects.
>
> Default HW setting of WP_DMA_BURTS_SIZE is 2, hence patch does not
> change behaviour on PCI devices. However I looked at RT3290 and
> RT5592 PCI vendor drivers and there this value is initialized to 3.
> So I think we can remove is_usb condition in rt2800_init_registers().

Shouldn't one of the explanations (depending on where you decide to set
WP_DMA_BURTS_SIZE) go into the commit message? As said in my last mail,
without further explanation it looks like introducing a bug.

Mathias

2016-11-16 11:57:30

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE

We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on
rt2800_init_registers() for USB devices. For PCI devices we will use
HW default setting, which is 2, so patch does not change behaviour
on PCI devices.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
v1 -> v2 Changelog fixes

drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index feceb13..9ecdc4c 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -6756,7 +6756,6 @@ int rt2800_enable_radio(struct rt2x00_dev *rt2x00dev)
rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 1);
rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 1);
- rt2x00_set_field32(&reg, WPDMA_GLO_CFG_WP_DMA_BURST_SIZE, 2);
rt2x00_set_field32(&reg, WPDMA_GLO_CFG_TX_WRITEBACK_DONE, 1);
rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);

--
1.7.1

2016-11-05 12:55:22

by Mathias Kresin

[permalink] [raw]
Subject: Re: [PATCH 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE

02.11.2016 15:10, Stanislaw Gruszka:
> We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on
> rt2800_init_registers().
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>
> ---
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index feceb13..9ecdc4c 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -6756,7 +6756,6 @@ int rt2800_enable_radio(struct rt2x00_dev *rt2x00dev)
> rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
> rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 1);
> rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 1);
> - rt2x00_set_field32(&reg, WPDMA_GLO_CFG_WP_DMA_BURST_SIZE, 2);

More a notice than a potential issue since I don't have much knowledge
about the driver/chip internals.

But WPDMA_GLO_CFG_WP_DMA_BURST_SIZE in rt2800_init_registers() is set
conditionally for rt2x00_is_usb(rt2x00dev), where this one is set
unconditionally. Not sure if this change has side effects.

Mathias

2016-11-02 14:12:36

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 10/10] rt2800: disable CCK rates on HT

Sending frames in CCK rates on HT can cause performance problems.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 8d35b2e..2515702 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -7475,7 +7475,6 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
/*
* Initialize all hw fields.
*/
- ieee80211_hw_set(rt2x00dev->hw, SUPPORTS_HT_CCK_RATES);
ieee80211_hw_set(rt2x00dev->hw, REPORTS_TX_ACK_STATUS);
ieee80211_hw_set(rt2x00dev->hw, AMPDU_AGGREGATION);
ieee80211_hw_set(rt2x00dev->hw, PS_NULLFUNC_STACK);
--
1.7.1

2016-11-02 14:12:24

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 01/10] rt2800: correctly report MCS TX parameters

We should only set IEEE80211_HT_MCS_TX_RX_DIF when TX and RX MCS sets
are not equal, i.e. when number of tx streams is different than
number of RX streams.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 23 +++++++++++++----------
1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index bf3f0a3..aab59f6 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -7464,7 +7464,7 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
char *default_power1;
char *default_power2;
char *default_power3;
- unsigned int i;
+ unsigned int i, tx_chains, rx_chains;
u32 reg;

/*
@@ -7589,21 +7589,24 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HT_CAP_SGI_20 |
IEEE80211_HT_CAP_SGI_40;

- if (rt2x00dev->default_ant.tx_chain_num >= 2)
+ tx_chains = rt2x00dev->default_ant.tx_chain_num;
+ rx_chains = rt2x00dev->default_ant.rx_chain_num;
+
+ if (tx_chains >= 2)
spec->ht.cap |= IEEE80211_HT_CAP_TX_STBC;

- spec->ht.cap |= rt2x00dev->default_ant.rx_chain_num <<
- IEEE80211_HT_CAP_RX_STBC_SHIFT;
+ spec->ht.cap |= rx_chains << IEEE80211_HT_CAP_RX_STBC_SHIFT;

spec->ht.ampdu_factor = 3;
spec->ht.ampdu_density = 4;
- spec->ht.mcs.tx_params =
- IEEE80211_HT_MCS_TX_DEFINED |
- IEEE80211_HT_MCS_TX_RX_DIFF |
- ((rt2x00dev->default_ant.tx_chain_num - 1) <<
- IEEE80211_HT_MCS_TX_MAX_STREAMS_SHIFT);
+ spec->ht.mcs.tx_params = IEEE80211_HT_MCS_TX_DEFINED;
+ if (tx_chains != rx_chains) {
+ spec->ht.mcs.tx_params |= IEEE80211_HT_MCS_TX_RX_DIFF;
+ spec->ht.mcs.tx_params |=
+ (tx_chains - 1) << IEEE80211_HT_MCS_TX_MAX_STREAMS_SHIFT;
+ }

- switch (rt2x00dev->default_ant.rx_chain_num) {
+ switch (rx_chains) {
case 3:
spec->ht.mcs.rx_mask[2] = 0xff;
case 2:
--
1.7.1

2016-11-16 15:54:25

by Mathias Kresin

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

2016-11-16 13:02 GMT+01:00 Stanislaw Gruszka <[email protected]>:
> On Wed, Nov 16, 2016 at 09:07:00AM +0100, Mathias Kresin wrote:
>> Here are the results of the requested tests. Please keep in mind, I'm not in
>> a lab environment:
>>
>> LEDE head
>> connect
>> action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
>> action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
>> action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
>> action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
>
> No problem here - buf_size corresponds to ampdu_factor.
>
>> ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
>> to flush
>> ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
>> to flush
>
> I think we do not give device enough time to post AMPDU consisted
> with bigger amount of frames. If we want to increase ba_size we will
> need also some other changes in the driver. Anyway I already request
> Kalle to drop this patch. I assume other patches do not cause
> regression for you, correct?


Correct. Just 05/10 caused issues.

2016-11-16 08:07:03

by Mathias Kresin

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

14.11.2016 13:43, Stanislaw Gruszka:
> On Mon, Nov 14, 2016 at 09:45:36AM +0100, Stanislaw Gruszka wrote:
>> Could you check below patch and see if it helps? If it does not,
>> could you printk sta->ht_cap.ampdu_density and ba_size values
>> and provide them here.
>
> Actually please print parameters from below patch. I think ba_size
> should be based on per TID buf_size instead of ampdu_factor, in case
> STA has buf size different for some TIDs.
>
> Also adding Felix to cc since my orginal patch:
> http://marc.info/?l=linux-wireless&m=147809595316289&w=2
> was shamelessly stolen from mt76 driver. Perhaps Felix could provide
> us some expertise.
>
> Thanks
> Stanislaw
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index 2515702..35bc38c 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -7950,6 +7950,8 @@ int rt2800_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> struct rt2x00_sta *sta_priv = (struct rt2x00_sta *)sta->drv_priv;
> int ret = 0;
>
> + printk("action %d sta %pM tid %u buf_size %u ampdu_factor %u\n", params->action, sta->addr, params->tid, params->buf_size, sta->ht_cap.ampdu_factor);
> +
> /*
> * Don't allow aggregation for stations the hardware isn't aware
> * of because tx status reports for frames to an unknown station
>

Here are the results of the requested tests. Please keep in mind, I'm
not in a lab environment:

LEDE head
connect
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3

wireless (iperf client) to wired (iperf server)
Interval Transfer Bandwidth
0.00-60.01 sec 544 MBytes 76.1 Mbits/sec sender
0.00-60.01 sec 544 MBytes 76.1 Mbits/sec receiver

wired (iperf client) to wireless (iperf server)
Interval Transfer Bandwidth Retr
0.00-60.00 sec 609 MBytes 85.1 Mbits/sec 96 sender
0.00-60.00 sec 606 MBytes 84.8 Mbits/sec receiver

on interface down
action 4 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3

LEDE + vanilla driver (without LEDE rt2x00 patches)
connect
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3

wireless (iperf client) to wired (iperf server)
Interval Transfer Bandwidth
0.00-60.02 sec 522 MBytes 73.0 Mbits/sec sender
0.00-60.02 sec 522 MBytes 73.0 Mbits/sec receiver

wired (iperf client) to wireless (iperf server)
Interval Transfer Bandwidth Retr
0.00-60.00 sec 583 MBytes 81.5 Mbits/sec 104 sender
0.00-60.00 sec 581 MBytes 81.2 Mbits/sec receiver

on interface down
action 4 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3

LEDE + vanilla driver + patchset
connect
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3

wireless (iperf client) to wired (iperf server)
Interval Transfer Bandwidth
0.00-60.02 sec 377 MBytes 52.7 Mbits/sec sender
0.00-60.02 sec 377 MBytes 52.6 Mbits/sec receive

on interface down
action 4 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3
ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
to flush
ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
to flush

wired (iperf client) to wireless (iperf server)
* not reliable reproducible stalls/reconnects:
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
* one time I've seen this:
ieee80211 phy0: rt2x00lib_rxdone_read_signal: Warning - Frame
received with unrecognized signal, mode=0x0001,
signal=0x010c, type=4
* sometimes the same rt2x00queue_flush_queue warning on interface
down

Interval Transfer Bandwidth Retr
0.00-60.00 sec 576 MBytes 80.6 Mbits/sec 447 sender
0.00-60.00 sec 574 MBytes 80.2 Mbits/sec receiver

on interface down
action 4 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3

LEDE + vanilla driver + patchset + increased max psdu
connect
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3

wireless (iperf client) to wired (iperf server)
Interval Transfer Bandwidth
0.00-60.02 sec 366 MBytes 51.1 Mbits/sec sender
0.00-60.02 sec 365 MBytes 51.1 Mbits/sec receiver

wired (iperf client) to wireless (iperf server)
Interval Transfer Bandwidth Retr
0.00-60.00 sec 569 MBytes 79.6 Mbits/sec 471 sender
0.00-60.00 sec 566 MBytes 79.1 Mbits/sec receiver

on interface down
action 4 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3
ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
to flush
ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
to flush

2016-11-02 14:12:29

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

We can calculate BA window size (max number of pending frames not
yet block acked) of remote station using Maximum A-MPDU length factor
for that station.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index 68b620b..9da89e3 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -305,14 +305,19 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
struct ieee80211_tx_rate *txrate = &tx_info->control.rates[0];
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
- struct rt2x00_sta *sta_priv = NULL;
+ u8 ba_size = 0;

if (sta) {
- txdesc->u.ht.mpdu_density =
- sta->ht_cap.ampdu_density;
+ struct rt2x00_sta *sta_priv = sta_to_rt2x00_sta(sta);

- sta_priv = sta_to_rt2x00_sta(sta);
+ txdesc->u.ht.mpdu_density = sta->ht_cap.ampdu_density;
txdesc->u.ht.wcid = sta_priv->wcid;
+
+ if (!(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)) {
+ ba_size = IEEE80211_MIN_AMPDU_BUF;
+ ba_size <<= sta->ht_cap.ampdu_factor;
+ ba_size = min_t(int, 63, ba_size - 1);
+ }
}

/*
@@ -345,7 +350,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
return;
}

- txdesc->u.ht.ba_size = 7; /* FIXME: What value is needed? */
+ txdesc->u.ht.ba_size = ba_size;

/*
* Only one STBC stream is supported for now.
--
1.7.1

2016-11-17 06:29:18

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE

Stanislaw Gruszka <[email protected]> writes:

> We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on
> rt2800_init_registers() for USB devices. For PCI devices we will use
> HW default setting, which is 2, so patch does not change behaviour
> on PCI devices.
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>
> ---
> v1 -> v2 Changelog fixes

Please resend the whole patchset. For me cherry picking patches
individually like this is difficult and especially error prone.

--
Kalle Valo

2016-11-02 14:12:28

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE

We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on
rt2800_init_registers().

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index feceb13..9ecdc4c 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -6756,7 +6756,6 @@ int rt2800_enable_radio(struct rt2x00_dev *rt2x00dev)
rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 1);
rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 1);
- rt2x00_set_field32(&reg, WPDMA_GLO_CFG_WP_DMA_BURST_SIZE, 2);
rt2x00_set_field32(&reg, WPDMA_GLO_CFG_TX_WRITEBACK_DONE, 1);
rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);

--
1.7.1

2016-11-02 14:12:33

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 08/10] rt2800: use RTS/CTS protection instead of CTS-to-self

Change default to RTS/CTS protection. This has a cost of transmitting
one more control frame (RTS) however protect us from traffic from
hidden node.

On station mode will use CTS-to-self if AP will configure that
for the network.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 812f8e7..57bfec4 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1621,7 +1621,7 @@ static void rt2800_config_ht_opmode(struct rt2x00_dev *rt2x00dev,
* => Protect all HT40 transmissions.
*/
mm20_mode = gf20_mode = 0;
- mm40_mode = gf40_mode = 2;
+ mm40_mode = gf40_mode = 1;

break;
case IEEE80211_HT_OP_MODE_PROTECTION_NONMEMBER:
@@ -1644,7 +1644,7 @@ static void rt2800_config_ht_opmode(struct rt2x00_dev *rt2x00dev,
* Legacy STAs are present
* => Protect all HT transmissions.
*/
- mm20_mode = mm40_mode = gf20_mode = gf40_mode = 2;
+ mm20_mode = mm40_mode = gf20_mode = gf40_mode = 1;

/*
* If erp protection is needed we have to protect HT
@@ -1660,7 +1660,7 @@ static void rt2800_config_ht_opmode(struct rt2x00_dev *rt2x00dev,

/* check for STAs not supporting greenfield mode */
if (any_sta_nongf)
- gf20_mode = gf40_mode = 2;
+ gf20_mode = gf40_mode = 1;

/* Update HT protection config */
rt2800_register_read(rt2x00dev, MM20_PROT_CFG, &reg);
--
1.7.1

2016-11-16 12:06:24

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

On Wed, Nov 16, 2016 at 09:07:00AM +0100, Mathias Kresin wrote:
> Here are the results of the requested tests. Please keep in mind, I'm not in
> a lab environment:
>
> LEDE head
> connect
> action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
> action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
> action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
> action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3

No problem here - buf_size corresponds to ampdu_factor.

> ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
> to flush
> ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
> to flush

I think we do not give device enough time to post AMPDU consisted
with bigger amount of frames. If we want to increase ba_size we will
need also some other changes in the driver. Anyway I already request
Kalle to drop this patch. I assume other patches do not cause
regression for you, correct?

Thanks for testing.
Stanislaw

2016-11-14 12:47:06

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

On Wed, Nov 02, 2016 at 03:11:00PM +0100, Stanislaw Gruszka wrote:
> We can calculate BA window size (max number of pending frames not
> yet block acked) of remote station using Maximum A-MPDU length factor
> for that station.
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>

Kalle, please drop this patch from the set, it requires some rework.

Thanks
Stanislaw


> ---
> drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 15 ++++++++++-----
> 1 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> index 68b620b..9da89e3 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> @@ -305,14 +305,19 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
> struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
> struct ieee80211_tx_rate *txrate = &tx_info->control.rates[0];
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> - struct rt2x00_sta *sta_priv = NULL;
> + u8 ba_size = 0;
>
> if (sta) {
> - txdesc->u.ht.mpdu_density =
> - sta->ht_cap.ampdu_density;
> + struct rt2x00_sta *sta_priv = sta_to_rt2x00_sta(sta);
>
> - sta_priv = sta_to_rt2x00_sta(sta);
> + txdesc->u.ht.mpdu_density = sta->ht_cap.ampdu_density;
> txdesc->u.ht.wcid = sta_priv->wcid;
> +
> + if (!(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)) {
> + ba_size = IEEE80211_MIN_AMPDU_BUF;
> + ba_size <<= sta->ht_cap.ampdu_factor;
> + ba_size = min_t(int, 63, ba_size - 1);
> + }
> }
>
> /*
> @@ -345,7 +350,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
> return;
> }
>
> - txdesc->u.ht.ba_size = 7; /* FIXME: What value is needed? */
> + txdesc->u.ht.ba_size = ba_size;
>
> /*
> * Only one STBC stream is supported for now.
> --
> 1.7.1
>

2016-11-16 13:59:41

by Kalle Valo

[permalink] [raw]
Subject: Re: [05/10] rt2800: make ba_size depend on ampdu_factor

Stanislaw Gruszka <[email protected]> wrote:
> We can calculate BA window size (max number of pending frames not
> yet block acked) of remote station using Maximum A-MPDU length factor
> for that station.
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>

Dropping as requested by Stanislaw.

Patch set to Changes Requested.

--
https://patchwork.kernel.org/patch/9409241/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2016-11-02 14:12:26

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 03/10] rt2800: OFDM rates are mandatory

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index aab59f6..feceb13 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1707,7 +1707,7 @@ void rt2800_config_erp(struct rt2x00_dev *rt2x00dev, struct rt2x00lib_erp *erp,

if (changed & BSS_CHANGED_BASIC_RATES) {
rt2800_register_write(rt2x00dev, LEGACY_BASIC_RATE,
- erp->basic_rates);
+ 0xff0 | erp->basic_rates);
rt2800_register_write(rt2x00dev, HT_BASIC_RATE, 0x00008003);
}

--
1.7.1

2016-11-02 14:12:30

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 06/10] rt2800: correct AUTO_RSP_CFG

Initialize AUTO_RSP_CFG register to similar value as vendor driver does.

Do not set BAC_ACK_POLICY based on short preamble setting, those are
unrelated.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 9ecdc4c..ff4a7c3 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1691,8 +1691,6 @@ void rt2800_config_erp(struct rt2x00_dev *rt2x00dev, struct rt2x00lib_erp *erp,

if (changed & BSS_CHANGED_ERP_PREAMBLE) {
rt2800_register_read(rt2x00dev, AUTO_RSP_CFG, &reg);
- rt2x00_set_field32(&reg, AUTO_RSP_CFG_BAC_ACK_POLICY,
- !!erp->short_preamble);
rt2x00_set_field32(&reg, AUTO_RSP_CFG_AR_PREAMBLE,
!!erp->short_preamble);
rt2800_register_write(rt2x00dev, AUTO_RSP_CFG, reg);
@@ -4735,9 +4733,9 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
rt2800_register_read(rt2x00dev, AUTO_RSP_CFG, &reg);
rt2x00_set_field32(&reg, AUTO_RSP_CFG_AUTORESPONDER, 1);
rt2x00_set_field32(&reg, AUTO_RSP_CFG_BAC_ACK_POLICY, 1);
- rt2x00_set_field32(&reg, AUTO_RSP_CFG_CTS_40_MMODE, 0);
+ rt2x00_set_field32(&reg, AUTO_RSP_CFG_CTS_40_MMODE, 1);
rt2x00_set_field32(&reg, AUTO_RSP_CFG_CTS_40_MREF, 0);
- rt2x00_set_field32(&reg, AUTO_RSP_CFG_AR_PREAMBLE, 1);
+ rt2x00_set_field32(&reg, AUTO_RSP_CFG_AR_PREAMBLE, 0);
rt2x00_set_field32(&reg, AUTO_RSP_CFG_DUAL_CTS_EN, 0);
rt2x00_set_field32(&reg, AUTO_RSP_CFG_ACK_CTS_PSM_BIT, 0);
rt2800_register_write(rt2x00dev, AUTO_RSP_CFG, reg);
--
1.7.1

2016-11-02 14:12:26

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 02/10] rt2800usb: do not wipe out USB_DMA_CFG settings

We should not reset USB_DMA_CFG on rt2800usb_init_registers() as this
function is called indirectly from rt2800_enable_radio(). If we
do so, we wipe out USB_DMA_CFG settings from rt2800usb_enable_radio().

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800usb.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800usb.c b/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
index 4b0bb6b..9f61293 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
@@ -341,8 +341,6 @@ static int rt2800usb_init_registers(struct rt2x00_dev *rt2x00dev)
rt2x00_set_field32(&reg, MAC_SYS_CTRL_RESET_BBP, 1);
rt2x00usb_register_write(rt2x00dev, MAC_SYS_CTRL, reg);

- rt2x00usb_register_write(rt2x00dev, USB_DMA_CFG, 0x00000000);
-
rt2x00usb_vendor_request_sw(rt2x00dev, USB_DEVICE_MODE, 0,
USB_MODE_RESET, REGISTER_TIMEOUT);

@@ -353,12 +351,11 @@ static int rt2800usb_init_registers(struct rt2x00_dev *rt2x00dev)

static int rt2800usb_enable_radio(struct rt2x00_dev *rt2x00dev)
{
- u32 reg;
+ u32 reg = 0;

if (unlikely(rt2800_wait_wpdma_ready(rt2x00dev)))
return -EIO;

- rt2x00usb_register_read(rt2x00dev, USB_DMA_CFG, &reg);
rt2x00_set_field32(&reg, USB_DMA_CFG_PHY_CLEAR, 0);
rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_AGG_EN, 0);
rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_AGG_TIMEOUT, 128);
--
1.7.1

2016-11-14 09:42:18

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor

Hi Stainslaw,

On Mon, Nov 14, 2016 at 7:45 PM, Stanislaw Gruszka <[email protected]> wrote:
> On Sat, Nov 05, 2016 at 01:56:58PM +0100, Mathias Kresin wrote:
>> 02.11.2016 15:11, Stanislaw Gruszka:
>> >
>> >- sta_priv = sta_to_rt2x00_sta(sta);
>> >+ txdesc->u.ht.mpdu_density = sta->ht_cap.ampdu_density;
>> > txdesc->u.ht.wcid = sta_priv->wcid;
>> >+
>> >+ if (!(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)) {
>> >+ ba_size = IEEE80211_MIN_AMPDU_BUF;
>> >+ ba_size <<= sta->ht_cap.ampdu_factor;
>> >+ ba_size = min_t(int, 63, ba_size - 1);
>> >+ }
>> > }
>> >
>> > /*
>> >@@ -345,7 +350,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
>> > return;
>> > }
>> >
>> >- txdesc->u.ht.ba_size = 7; /* FIXME: What value is needed? */
>> >+ txdesc->u.ht.ba_size = ba_size;
>> >
>> > /*
>> > * Only one STBC stream is supported for now.
>> >
>>
>> Having this patch applied, the throughput on a vgv7510kw22 (RT3062F)
>> in AP mode using LEDE head is decreased by somewhat around 10
>> Mbits/sec. I'm using iperf3 for throughput tests and having this
>> patch reverted the throughout is back to 80 Mbits/sec.
>>
>> When bringing down the wifi interface the following messages are
>> logged with the patch applied:
>>
>> [ 281.738373] ieee80211 phy0: rt2x00queue_flush_queue: Warning -
>> Queue 2 failed to flush
>> [ 281.906380] ieee80211 phy0: rt2x00queue_flush_queue: Warning -
>> Queue 2 failed to flush
>
> Could you check below patch and see if it helps? If it does not,
> could you printk sta->ht_cap.ampdu_density and ba_size values
> and provide them here.
>
> Thanks
> Stanislaw
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index 2515702..72c7948 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -4707,7 +4707,7 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
> rt2x00_rt_rev_lt(rt2x00dev, RT3070, REV_RT3070E))
> rt2x00_set_field32(&reg, MAX_LEN_CFG_MAX_PSDU, 2);
> else
> - rt2x00_set_field32(&reg, MAX_LEN_CFG_MAX_PSDU, 1);
> + rt2x00_set_field32(&reg, MAX_LEN_CFG_MAX_PSDU, 3;

You're missing a closing parenthesis, so it isn't going to work unless
it's added back in.

> rt2x00_set_field32(&reg, MAX_LEN_CFG_MIN_PSDU, 0);
> rt2x00_set_field32(&reg, MAX_LEN_CFG_MIN_MPDU, 0);
> rt2800_register_write(rt2x00dev, MAX_LEN_CFG, reg);

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/