On Wed, Jun 29, 2016 at 1:50 PM, Krishna Chaitanya
[off-list ref] wrote:

On Wed, Jun 29, 2016 at 1:41 PM, Karl Beldan [off-list ref] wrote:

quoted

On Mon, Jun 27, 2016 at 9:53 AM, Chaitanya TK [off-list ref]
wrote:

quoted

From: Chaitanya T K <redacted>

If peer support reception of STBC and LDPC, enable them for better
performance.

Signed-off-by: Chaitanya TK <redacted>
---
 include/linux/ieee80211.h          |    1 +
 net/mac80211/rc80211_minstrel_ht.c |   25 +++++++++++++++++--------
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index b118744..4d01130 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h

@@ -1550,6 +1550,7 @@ struct ieee80211_vht_operation {
 #define IEEE80211_VHT_CAP_RXSTBC_3                             0x00000300
 #define IEEE80211_VHT_CAP_RXSTBC_4                             0x00000400
 #define IEEE80211_VHT_CAP_RXSTBC_MASK                          0x00000700
+#define IEEE80211_VHT_CAP_RXSTBC_SHIFT                         8
 #define IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE

0x00000800
 #define IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE
0x00001000
 #define IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT                  13

diff --git a/net/mac80211/rc80211_minstrel_ht.c

b/net/mac80211/rc80211_minstrel_ht.c
index 30fbabf..e2fcdea 100644

--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c

@@ -1166,13 +1166,14 @@ minstrel_ht_update_caps(void *priv, struct

ieee80211_supported_band *sband,
        struct minstrel_ht_sta_priv *msp = priv_sta;
        struct minstrel_ht_sta *mi = &msp->ht;
        struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
-       u16 sta_cap = sta->ht_cap.cap;
+       u16 ht_cap = sta->ht_cap.cap;
        struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
        int use_vht;
        int n_supported = 0;
        int ack_dur;
        int stbc;
        int i;
+       bool ldpc;

        /* fall back to the old minstrel for legacy stations */
        if (!sta->ht_cap.ht_supported)

@@ -1210,16 +1211,24 @@ minstrel_ht_update_caps(void *priv, struct

ieee80211_supported_band *sband,
        }
        mi->sample_tries = 4;

-       /* TODO tx_flags for vht - ATM the RC API is not fine-grained
enough */
        if (!use_vht) {
-               stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
+               stbc = (ht_cap & IEEE80211_HT_CAP_RX_STBC) >>
                        IEEE80211_HT_CAP_RX_STBC_SHIFT;
-               mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;

-               if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
-                       mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
+               if (ht_cap & IEEE80211_HT_CAP_LDPC_CODING)
+                       ldpc = true;
+       } else {
+               stbc = (vht_cap->cap & IEEE80211_VHT_CAP_RXSTBC_MASK) >>
+                       IEEE80211_VHT_CAP_RXSTBC_SHIFT;
+
+               if (vht_cap->cap & IEEE80211_VHT_CAP_RXLDPC)
+                       ldpc = true;
        }

+       mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
+       if (ldpc)
+               mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
+

Here you are using ldpc uninitialized.


As for the idea, ldpc and stbc support/enable in HT and VHT being
independant and tx_flags being shared by all the info.rates, you cannot use
it as you are trying to (e.g. you might end up asking lower layers to send
stbc/vht with a hw only supporting stbc/ht or/and trying to send stbc/vht
with inappropriate advertised vht caps).

The design in minstrel is it checks only for peers capabilities and it doesn't
check local hw capabilities (except for those in MCS_GROUPS), so minstrel
just informs the we can use STBC/LDPC, its HW's decision whether to
actually use or not.

quoted

This stems from the STBC/LDPC CTL flags being a mac80211_tx_info_flags, this
is what should be changed.

I understand the problem here, if all rates are same then its no problem.
But if some rates are HT and some are VHT, then we might end up in problem.

To solve this, we need to move this to mac80211_rate_control_flags so that
we can convey per rate info.

Karl/Johannes,

Any thoughts on this? This impacts all drivers, so it would be good
to finalize on design before submitting patches.