[PATCH 1/2] ath10k: change beamformee VHT STS capability

[PATCH 1/2] ath10k: change beamformee VHT STS capability

[Michal Kazior]

From: Bartosz Markowski <bartosz.markowski@tieto.com>

All beamformee supporting chips have the ability to support
VHT NDP in up to 4 STSs. Change the published beamformee
STS cap accordingly to 3 as it should be Nsts-1.

This makes it possible to actually make full use
of. e.g. a 4SS MU-MIMO capable AP.

Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 64674c955d44..54ced30f5364 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4074,7 +4074,7 @@ static int ath10k_mac_set_txbf_conf(struct ath10k_vif *arvif)
 
 	if (ar->vht_cap_info & (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE |
 				IEEE80211_VHT_CAP_MU_BEAMFORMEE_CAPABLE))
-		value |= SM((ar->num_rf_chains - 1), WMI_TXBF_STS_CAP_OFFSET);
+		value |= SM(3, WMI_TXBF_STS_CAP_OFFSET);
 
 	if (ar->vht_cap_info & (IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE |
 				IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE))
@@ -6804,7 +6804,7 @@ static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
 
 	if (ar->vht_cap_info & (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE |
 				IEEE80211_VHT_CAP_MU_BEAMFORMEE_CAPABLE)) {
-		val = ar->num_rf_chains - 1;
+		val = 3;
 		val <<= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT;
 		val &= IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK;
 
-- 
2.1.4

[PATCH 2/2] ath10k: fix mu-mimo rx status reporting

[Michal Kazior]

MU-MIMO Rx involves different interpretation of
the VHT-SIG-A compared to SU-MIMO.

The incorrect interpretation led ath10k to report
VHT MCS values greater than 9 which subsequently
prompted mac80211 to drop such frames. This
effectively broke Rx with MU-MIMO in many cases
and manifested with a kernel warning in the log
which looked like this:

  [   14.552520] WARNING: CPU: 2 PID: 0 at net/mac80211/rx.c:3578 ieee80211_rx+0x26c/0x940 [mac80211]()
  [   14.552522] Rate marked as a VHT rate but data is invalid: MCS: 10, NSS: 2
  ... call trace follows ...

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 1b7a04366256..8ba8fa9004cf 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -643,6 +643,8 @@ struct amsdu_subframe_hdr {
 	__be16 len;
 } __packed;
 
+#define GROUP_ID_IS_SU_MIMO(x) ((x) == 0 || (x) == 63)
+
 static void ath10k_htt_rx_h_rates(struct ath10k *ar,
 				  struct ieee80211_rx_status *status,
 				  struct htt_rx_desc *rxd)
@@ -650,6 +652,7 @@ static void ath10k_htt_rx_h_rates(struct ath10k *ar,
 	struct ieee80211_supported_band *sband;
 	u8 cck, rate, bw, sgi, mcs, nss;
 	u8 preamble = 0;
+	u8 group_id;
 	u32 info1, info2, info3;
 
 	info1 = __le32_to_cpu(rxd->ppdu_start.info1);
@@ -692,10 +695,27 @@ static void ath10k_htt_rx_h_rates(struct ath10k *ar,
 	case HTT_RX_VHT_WITH_TXBF:
 		/* VHT-SIG-A1 in info2, VHT-SIG-A2 in info3
 		   TODO check this */
-		mcs = (info3 >> 4) & 0x0F;
-		nss = ((info2 >> 10) & 0x07) + 1;
 		bw = info2 & 3;
 		sgi = info3 & 1;
+		group_id = (info2 >> 4) & 0x3F;
+
+		if (GROUP_ID_IS_SU_MIMO(group_id)) {
+			mcs = (info3 >> 4) & 0x0F;
+			nss = ((info2 >> 10) & 0x07) + 1;
+		} else {
+			/* Hardware doesn't decode VHT-SIG-B into Rx descriptor
+			 * so it's impossible to decode MCS. Also since
+			 * firmware consumes Group Id Management frames host
+			 * has no knowledge regarding group/user position
+			 * mapping so it's impossible to pick the correct Nsts
+			 * from VHT-SIG-A1.
+			 *
+			 * Bandwidth and SGI are valid so report the rateinfo
+			 * on best-effort basis.
+			 */
+			mcs = 0;
+			nss = 1;
+		}
 
 		status->rate_idx = mcs;
 		status->vht_nss = nss;
-- 
2.1.4

Re: [PATCH 1/2] ath10k: change beamformee VHT STS capability

[Bartosz Markowski]

On 27 August 2015 at 14:47, Michal Kazior <michal.kazior@tieto.com> wrote:
> From: Bartosz Markowski <bartosz.markowski@tieto.com>
>
> All beamformee supporting chips have the ability to support
> VHT NDP in up to 4 STSs. Change the published beamformee
> STS cap accordingly to 3 as it should be Nsts-1.
>
> This makes it possible to actually make full use
> of. e.g. a 4SS MU-MIMO capable AP.
>
> Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---

Kalle, please drop this one for now. It looks like we need to discuss
about this a bit more still.

>  drivers/net/wireless/ath/ath10k/mac.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 64674c955d44..54ced30f5364 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -4074,7 +4074,7 @@ static int ath10k_mac_set_txbf_conf(struct ath10k_vif *arvif)
>
>         if (ar->vht_cap_info & (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE |
>                                 IEEE80211_VHT_CAP_MU_BEAMFORMEE_CAPABLE))
> -               value |= SM((ar->num_rf_chains - 1), WMI_TXBF_STS_CAP_OFFSET);
> +               value |= SM(3, WMI_TXBF_STS_CAP_OFFSET);
>
>         if (ar->vht_cap_info & (IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE |
>                                 IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE))
> @@ -6804,7 +6804,7 @@ static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
>
>         if (ar->vht_cap_info & (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE |
>                                 IEEE80211_VHT_CAP_MU_BEAMFORMEE_CAPABLE)) {
> -               val = ar->num_rf_chains - 1;
> +               val = 3;
>                 val <<= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT;
>                 val &= IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK;
>
> --
> 2.1.4
>



-- 
Bartosz

Re: [PATCH 1/2] ath10k: change beamformee VHT STS capability

[Bartosz Markowski]

On 28 August 2015 at 14:56, Bartosz Markowski
<bartosz.markowski@tieto.com> wrote:
> On 27 August 2015 at 14:47, Michal Kazior <michal.kazior@tieto.com> wrote:
>> From: Bartosz Markowski <bartosz.markowski@tieto.com>
>>
>> All beamformee supporting chips have the ability to support
>> VHT NDP in up to 4 STSs. Change the published beamformee
>> STS cap accordingly to 3 as it should be Nsts-1.
>>
>> This makes it possible to actually make full use
>> of. e.g. a 4SS MU-MIMO capable AP.
>>
>> Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> ---
>
> Kalle, please drop this one for now. It looks like we need to discuss
> about this a bit more still.

I have sent a follow up on this, splitting it into 2 patches (vht nsts
and sounding dimensions), addressing the concerns raised about the
lack of firmware passing the needed bits in vht_cap_info.

Bartosz

Re: [PATCH 1/2] ath10k: change beamformee VHT STS capability

[Kalle Valo]

Bartosz Markowski <bartosz.markowski@tieto.com> writes:

> On 28 August 2015 at 14:56, Bartosz Markowski
> <bartosz.markowski@tieto.com> wrote:
>> On 27 August 2015 at 14:47, Michal Kazior <michal.kazior@tieto.com> wrote:
>>> From: Bartosz Markowski <bartosz.markowski@tieto.com>
>>>
>>> All beamformee supporting chips have the ability to support
>>> VHT NDP in up to 4 STSs. Change the published beamformee
>>> STS cap accordingly to 3 as it should be Nsts-1.
>>>
>>> This makes it possible to actually make full use
>>> of. e.g. a 4SS MU-MIMO capable AP.
>>>
>>> Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>> ---
>>
>> Kalle, please drop this one for now. It looks like we need to discuss
>> about this a bit more still.
>
> I have sent a follow up on this, splitting it into 2 patches (vht nsts
> and sounding dimensions), addressing the concerns raised about the
> lack of firmware passing the needed bits in vht_cap_info.

Ok, patch 1 dropped. I assume patch 2 is still valid.

-- 
Kalle Valo

Re: [PATCH 1/2] ath10k: change beamformee VHT STS capability

[Bartosz Markowski]

On 2 September 2015 at 15:55, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Bartosz Markowski <bartosz.markowski@tieto.com> writes:
>
>> On 28 August 2015 at 14:56, Bartosz Markowski
>> <bartosz.markowski@tieto.com> wrote:
>>> On 27 August 2015 at 14:47, Michal Kazior <michal.kazior@tieto.com> wrote:
>>>> From: Bartosz Markowski <bartosz.markowski@tieto.com>
>>>>
>>>> All beamformee supporting chips have the ability to support
>>>> VHT NDP in up to 4 STSs. Change the published beamformee
>>>> STS cap accordingly to 3 as it should be Nsts-1.
>>>>
>>>> This makes it possible to actually make full use
>>>> of. e.g. a 4SS MU-MIMO capable AP.
>>>>
>>>> Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
>>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>>> ---
>>>
>>> Kalle, please drop this one for now. It looks like we need to discuss
>>> about this a bit more still.
>>
>> I have sent a follow up on this, splitting it into 2 patches (vht nsts
>> and sounding dimensions), addressing the concerns raised about the
>> lack of firmware passing the needed bits in vht_cap_info.
>
> Ok, patch 1 dropped. I assume patch 2 is still valid.

correct.

-- 
Bartosz

Re: [PATCH 2/2] ath10k: fix mu-mimo rx status reporting

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> MU-MIMO Rx involves different interpretation of
> the VHT-SIG-A compared to SU-MIMO.
>
> The incorrect interpretation led ath10k to report
> VHT MCS values greater than 9 which subsequently
> prompted mac80211 to drop such frames. This
> effectively broke Rx with MU-MIMO in many cases
> and manifested with a kernel warning in the log
> which looked like this:
>
>   [   14.552520] WARNING: CPU: 2 PID: 0 at net/mac80211/rx.c:3578 ieee80211_rx+0x26c/0x940 [mac80211]()
>   [   14.552522] Rate marked as a VHT rate but data is invalid: MCS: 10, NSS: 2
>   ... call trace follows ...
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Thanks, patch 2 applied.

-- 
Kalle Valo