| Message ID | 20220616173947.21901-1-jesus.manzano@galgus.net |
|---|---|
| State | New |
| Headers | show |
| Series | ath11k: fix number of VHT beamformee spatial streams | expand |
Hi Csaba, It happens the same to me, HE is also affected and not fixed by this patch because it is treated in a different place in the code than where the VHT capabilities are set. If this patch is correct and is accepted I would like to fix the HE part and upstream it too. Ps: thanks for your work in the OpenWRT forum in the Xiaomi AX3600 thread. Very useful all these months. Regards, Jesus El 20/6/22 a las 19:35, Sipos Csaba escribió: > Dear Jesus, > > I wanted to ask you if you are sure if this is just a VHT issue, and > HE is not affected? On my IPQ8074 board with 4 antennas for 5GHz, I > have this IW output: > > HE Iftypes: AP > HE MAC Capabilities (0x000d9a181040): > +HTC HE Supported > TWT Responder > Dynamic BA Fragementation Level: 1 > BSR > Broadcast TWT > OM Control > Maximum A-MPDU Length Exponent: 3 > RX Control Frame to MultiBSS > A-MSDU in A-MPDU > OM Control UL MU Data Disable RX > HE PHY Capabilities: (0x1c604c887fdb839c010c00): > HE40/HE80/5GHz > HE160/5GHz > HE160/HE80+80/5GHz > LDPC Coding in Payload > HE SU PPDU with 1x HE-LTF and 0.8us GI > STBC Tx <= 80MHz > STBC Rx <= 80MHz > Full Bandwidth UL MU-MIMO > DCM Max Constellation Rx: 1 > SU Beamformer > SU Beamformee > MU Beamformer > Beamformee STS <= 80Mhz: 7 > Beamformee STS > 80Mhz: 3 > Sounding Dimensions <= 80Mhz: 3 > Sounding Dimensions > 80Mhz: 3 > Ng = 16 SU Feedback > Ng = 16 MU Feedback > Codebook Size SU Feedback > Codebook Size MU Feedback > PPE Threshold Present > HE SU PPDU & HE PPDU 4x HE-LTF 0.8us GI > Max NC: 3 > STBC Rx > 80MHz > HE ER SU PPDU 4x HE-LTF 0.8us GI > TX 1024-QAM > RX 1024-QAM > HE RX MCS and NSS set <= 80 MHz > 1 streams: MCS 0-11 > 2 streams: MCS 0-11 > 3 streams: MCS 0-11 > 4 streams: MCS 0-11 > 5 streams: not supported > 6 streams: not supported > 7 streams: not supported > 8 streams: not supported > HE TX MCS and NSS set <= 80 MHz > 1 streams: MCS 0-11 > 2 streams: MCS 0-11 > 3 streams: MCS 0-11 > 4 streams: MCS 0-11 > 5 streams: not supported > 6 streams: not supported > 7 streams: not supported > 8 streams: not supported > HE RX MCS and NSS set 160 MHz > 1 streams: MCS 0-11 > 2 streams: MCS 0-11 > 3 streams: not supported > 4 streams: not supported > 5 streams: not supported > 6 streams: not supported > 7 streams: not supported > 8 streams: not supported > HE TX MCS and NSS set 160 MHz > 1 streams: MCS 0-11 > 2 streams: MCS 0-11 > 3 streams: not supported > 4 streams: not supported > 5 streams: not supported > 6 streams: not supported > 7 streams: not supported > 8 streams: not supported > > As you can see I have Beamformee STS <= 80Mhz: 7 under HE PHY > Capabilities, not sure if that is correct. > > Using WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1 firmware. > > Thanks for your answer! > > Regards, > Csaba > > Jesus Fernandez Manzano <jesus.manzano@galgus.net> ezt írta (időpont: > 2022. jún. 16., Cs, 20:21): > > The number of spatial streams used when acting as a beamformee in VHT > mode are reported by the firmware as 7 (8 sts - 1) both in IPQ6018 and > IPQ8074 which respectively have 2 and 4 sts each. So the firmware > should > report 1 (2 - 1) and 3 (4 - 1). > > Fix this by checking that the number of VHT beamformee sts reported by > the firmware is not greater than the number of receiving antennas - 1. > The fix is based on the same approach used in this same function for > sanitizing the number of sounding dimensions reported by the firmware. > > Without this change, acting as a beamformee in VHT mode is not working > properly. > > Tested-on: IPQ6018 hw1.0 AHB > WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1 > Tested-on: IPQ8074 hw2.0 AHB > WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1 > > Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax > devices") > Signed-off-by: Jesus Fernandez Manzano <jesus.manzano@galgus.net> > --- > drivers/net/wireless/ath/ath11k/mac.c | 25 ++++++++++++++++++++----- > 1 file changed, 20 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/mac.c > b/drivers/net/wireless/ath/ath11k/mac.c > index 42d2e8cf8125..7109ca4f166d 100644 > --- a/drivers/net/wireless/ath/ath11k/mac.c > +++ b/drivers/net/wireless/ath/ath11k/mac.c > @@ -4950,6 +4950,8 @@ static int ath11k_mac_set_txbf_conf(struct > ath11k_vif *arvif) > if (vht_cap & (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE)) { > nsts = vht_cap & > IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; > nsts >>= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; > + if (nsts > (ar->num_rx_chains - 1)) > + nsts = ar->num_rx_chains - 1; > value |= SM(nsts, WMI_TXBF_STS_CAP_OFFSET); > } > > @@ -4990,7 +4992,7 @@ static int ath11k_mac_set_txbf_conf(struct > ath11k_vif *arvif) > static void ath11k_set_vht_txbf_cap(struct ath11k *ar, u32 *vht_cap) > { > bool subfer, subfee; > - int sound_dim = 0; > + int sound_dim = 0, nsts = 0; > > subfer = !!(*vht_cap & > (IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE)); > subfee = !!(*vht_cap & > (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE)); > @@ -5000,6 +5002,11 @@ static void ath11k_set_vht_txbf_cap(struct > ath11k *ar, u32 *vht_cap) > subfer = false; > } > > + if (ar->num_rx_chains < 2) { > + *vht_cap &= > ~(IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE); > + subfee = false; > + } > + > /* If SU Beaformer is not set, then disable MU Beamformer > Capability */ > if (!subfer) > *vht_cap &= > ~(IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE); > @@ -5012,7 +5019,9 @@ static void ath11k_set_vht_txbf_cap(struct > ath11k *ar, u32 *vht_cap) > sound_dim >>= IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_SHIFT; > *vht_cap &= ~IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_MASK; > > - /* TODO: Need to check invalid STS and Sound_dim values > set by FW? */ > + nsts = (*vht_cap & IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK); > + nsts >>= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; > + *vht_cap &= ~IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; > > /* Enable Sounding Dimension Field only if SU BF is enabled */ > if (subfer) { > @@ -5024,9 +5033,15 @@ static void ath11k_set_vht_txbf_cap(struct > ath11k *ar, u32 *vht_cap) > *vht_cap |= sound_dim; > } > > - /* Use the STS advertised by FW unless SU Beamformee is > not supported*/ > - if (!subfee) > - *vht_cap &= ~(IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK); > + /* Enable Beamformee STS Field only if SU BF is enabled */ > + if (subfee) { > + if (nsts > (ar->num_rx_chains - 1)) > + nsts = ar->num_rx_chains - 1; > + > + nsts <<= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; > + nsts &= IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; > + *vht_cap |= nsts; > + } > } > > static struct ieee80211_sta_vht_cap > -- > 2.25.1 >
Hi Jesus, Oh, I am nobody, the masterminds are clearly Robert and Christian (Ansuel), I just try to use my radio access network knowledge and common sense to try and dissect issues like this. I am not sure if you noticed, that when you do uplink heavy tests on AX, in the current state uplink power is grossly fluctuating reported by the driver, while the RF environment is static. I am not sure if this issue with BF will fix it, but sure it sounds plausible. It is also unknown how the driver calculates RX power per client: does it uses some sort of reference signal, or the calculated power depends on traffic volume? Regards, Csaba Jesús Fernández Manzano <jesus.manzano@galgus.net> ezt írta (időpont: 2022. jún. 21., K, 10:29): > > Hi Csaba, > > It happens the same to me, HE is also affected and not fixed by this > patch because it is treated in a different place in the code than where > the VHT capabilities are set. If this patch is correct and is accepted I > would like to fix the HE part and upstream it too. > > Ps: thanks for your work in the OpenWRT forum in the Xiaomi AX3600 > thread. Very useful all these months. > > Regards, > Jesus > > El 20/6/22 a las 19:35, Sipos Csaba escribió: > > Dear Jesus, > > > > I wanted to ask you if you are sure if this is just a VHT issue, and > > HE is not affected? On my IPQ8074 board with 4 antennas for 5GHz, I > > have this IW output: > > > > HE Iftypes: AP > > HE MAC Capabilities (0x000d9a181040): > > +HTC HE Supported > > TWT Responder > > Dynamic BA Fragementation Level: 1 > > BSR > > Broadcast TWT > > OM Control > > Maximum A-MPDU Length Exponent: 3 > > RX Control Frame to MultiBSS > > A-MSDU in A-MPDU > > OM Control UL MU Data Disable RX > > HE PHY Capabilities: (0x1c604c887fdb839c010c00): > > HE40/HE80/5GHz > > HE160/5GHz > > HE160/HE80+80/5GHz > > LDPC Coding in Payload > > HE SU PPDU with 1x HE-LTF and 0.8us GI > > STBC Tx <= 80MHz > > STBC Rx <= 80MHz > > Full Bandwidth UL MU-MIMO > > DCM Max Constellation Rx: 1 > > SU Beamformer > > SU Beamformee > > MU Beamformer > > Beamformee STS <= 80Mhz: 7 > > Beamformee STS > 80Mhz: 3 > > Sounding Dimensions <= 80Mhz: 3 > > Sounding Dimensions > 80Mhz: 3 > > Ng = 16 SU Feedback > > Ng = 16 MU Feedback > > Codebook Size SU Feedback > > Codebook Size MU Feedback > > PPE Threshold Present > > HE SU PPDU & HE PPDU 4x HE-LTF 0.8us GI > > Max NC: 3 > > STBC Rx > 80MHz > > HE ER SU PPDU 4x HE-LTF 0.8us GI > > TX 1024-QAM > > RX 1024-QAM > > HE RX MCS and NSS set <= 80 MHz > > 1 streams: MCS 0-11 > > 2 streams: MCS 0-11 > > 3 streams: MCS 0-11 > > 4 streams: MCS 0-11 > > 5 streams: not supported > > 6 streams: not supported > > 7 streams: not supported > > 8 streams: not supported > > HE TX MCS and NSS set <= 80 MHz > > 1 streams: MCS 0-11 > > 2 streams: MCS 0-11 > > 3 streams: MCS 0-11 > > 4 streams: MCS 0-11 > > 5 streams: not supported > > 6 streams: not supported > > 7 streams: not supported > > 8 streams: not supported > > HE RX MCS and NSS set 160 MHz > > 1 streams: MCS 0-11 > > 2 streams: MCS 0-11 > > 3 streams: not supported > > 4 streams: not supported > > 5 streams: not supported > > 6 streams: not supported > > 7 streams: not supported > > 8 streams: not supported > > HE TX MCS and NSS set 160 MHz > > 1 streams: MCS 0-11 > > 2 streams: MCS 0-11 > > 3 streams: not supported > > 4 streams: not supported > > 5 streams: not supported > > 6 streams: not supported > > 7 streams: not supported > > 8 streams: not supported > > > > As you can see I have Beamformee STS <= 80Mhz: 7 under HE PHY > > Capabilities, not sure if that is correct. > > > > Using WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1 firmware. > > > > Thanks for your answer! > > > > Regards, > > Csaba > > > > Jesus Fernandez Manzano <jesus.manzano@galgus.net> ezt írta (időpont: > > 2022. jún. 16., Cs, 20:21): > > > > The number of spatial streams used when acting as a beamformee in VHT > > mode are reported by the firmware as 7 (8 sts - 1) both in IPQ6018 and > > IPQ8074 which respectively have 2 and 4 sts each. So the firmware > > should > > report 1 (2 - 1) and 3 (4 - 1). > > > > Fix this by checking that the number of VHT beamformee sts reported by > > the firmware is not greater than the number of receiving antennas - 1. > > The fix is based on the same approach used in this same function for > > sanitizing the number of sounding dimensions reported by the firmware. > > > > Without this change, acting as a beamformee in VHT mode is not working > > properly. > > > > Tested-on: IPQ6018 hw1.0 AHB > > WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1 > > Tested-on: IPQ8074 hw2.0 AHB > > WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1 > > > > Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax > > devices") > > Signed-off-by: Jesus Fernandez Manzano <jesus.manzano@galgus.net> > > --- > > drivers/net/wireless/ath/ath11k/mac.c | 25 ++++++++++++++++++++----- > > 1 file changed, 20 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/wireless/ath/ath11k/mac.c > > b/drivers/net/wireless/ath/ath11k/mac.c > > index 42d2e8cf8125..7109ca4f166d 100644 > > --- a/drivers/net/wireless/ath/ath11k/mac.c > > +++ b/drivers/net/wireless/ath/ath11k/mac.c > > @@ -4950,6 +4950,8 @@ static int ath11k_mac_set_txbf_conf(struct > > ath11k_vif *arvif) > > if (vht_cap & (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE)) { > > nsts = vht_cap & > > IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; > > nsts >>= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; > > + if (nsts > (ar->num_rx_chains - 1)) > > + nsts = ar->num_rx_chains - 1; > > value |= SM(nsts, WMI_TXBF_STS_CAP_OFFSET); > > } > > > > @@ -4990,7 +4992,7 @@ static int ath11k_mac_set_txbf_conf(struct > > ath11k_vif *arvif) > > static void ath11k_set_vht_txbf_cap(struct ath11k *ar, u32 *vht_cap) > > { > > bool subfer, subfee; > > - int sound_dim = 0; > > + int sound_dim = 0, nsts = 0; > > > > subfer = !!(*vht_cap & > > (IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE)); > > subfee = !!(*vht_cap & > > (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE)); > > @@ -5000,6 +5002,11 @@ static void ath11k_set_vht_txbf_cap(struct > > ath11k *ar, u32 *vht_cap) > > subfer = false; > > } > > > > + if (ar->num_rx_chains < 2) { > > + *vht_cap &= > > ~(IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE); > > + subfee = false; > > + } > > + > > /* If SU Beaformer is not set, then disable MU Beamformer > > Capability */ > > if (!subfer) > > *vht_cap &= > > ~(IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE); > > @@ -5012,7 +5019,9 @@ static void ath11k_set_vht_txbf_cap(struct > > ath11k *ar, u32 *vht_cap) > > sound_dim >>= IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_SHIFT; > > *vht_cap &= ~IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_MASK; > > > > - /* TODO: Need to check invalid STS and Sound_dim values > > set by FW? */ > > + nsts = (*vht_cap & IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK); > > + nsts >>= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; > > + *vht_cap &= ~IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; > > > > /* Enable Sounding Dimension Field only if SU BF is enabled */ > > if (subfer) { > > @@ -5024,9 +5033,15 @@ static void ath11k_set_vht_txbf_cap(struct > > ath11k *ar, u32 *vht_cap) > > *vht_cap |= sound_dim; > > } > > > > - /* Use the STS advertised by FW unless SU Beamformee is > > not supported*/ > > - if (!subfee) > > - *vht_cap &= ~(IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK); > > + /* Enable Beamformee STS Field only if SU BF is enabled */ > > + if (subfee) { > > + if (nsts > (ar->num_rx_chains - 1)) > > + nsts = ar->num_rx_chains - 1; > > + > > + nsts <<= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; > > + nsts &= IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; > > + *vht_cap |= nsts; > > + } > > } > > > > static struct ieee80211_sta_vht_cap > > -- > > 2.25.1 > > >
El 21/6/22 a las 14:06, Sipos Csaba escribió: > I am not sure if you noticed, that when you do uplink heavy tests on > AX, in the current state uplink power is grossly fluctuating reported > by the driver, while the RF environment is static. I am not sure if > this issue with BF will fix it, but sure it sounds plausible. It is > also unknown how the driver calculates RX power per client: does it > uses some sort of reference signal, or the calculated power depends on > traffic volume? Hi Csaba, Yes I noticed the same issue in heavy iperf3 uplink tests. But that was happening to me with firmware 2.4, now with 2.5 and the latest board-2.bin it works like a charm. Maybe you need some more patches in ath11k? I am using a combination of upstream patches from recent kernels and some qca patches. If you want, we can discuss the topic outside the mailing list to not spam other people. Regards, Jesus.
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 42d2e8cf8125..7109ca4f166d 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -4950,6 +4950,8 @@ static int ath11k_mac_set_txbf_conf(struct ath11k_vif *arvif) if (vht_cap & (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE)) { nsts = vht_cap & IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; nsts >>= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; + if (nsts > (ar->num_rx_chains - 1)) + nsts = ar->num_rx_chains - 1; value |= SM(nsts, WMI_TXBF_STS_CAP_OFFSET); } @@ -4990,7 +4992,7 @@ static int ath11k_mac_set_txbf_conf(struct ath11k_vif *arvif) static void ath11k_set_vht_txbf_cap(struct ath11k *ar, u32 *vht_cap) { bool subfer, subfee; - int sound_dim = 0; + int sound_dim = 0, nsts = 0; subfer = !!(*vht_cap & (IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE)); subfee = !!(*vht_cap & (IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE)); @@ -5000,6 +5002,11 @@ static void ath11k_set_vht_txbf_cap(struct ath11k *ar, u32 *vht_cap) subfer = false; } + if (ar->num_rx_chains < 2) { + *vht_cap &= ~(IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE); + subfee = false; + } + /* If SU Beaformer is not set, then disable MU Beamformer Capability */ if (!subfer) *vht_cap &= ~(IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE); @@ -5012,7 +5019,9 @@ static void ath11k_set_vht_txbf_cap(struct ath11k *ar, u32 *vht_cap) sound_dim >>= IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_SHIFT; *vht_cap &= ~IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_MASK; - /* TODO: Need to check invalid STS and Sound_dim values set by FW? */ + nsts = (*vht_cap & IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK); + nsts >>= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; + *vht_cap &= ~IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; /* Enable Sounding Dimension Field only if SU BF is enabled */ if (subfer) { @@ -5024,9 +5033,15 @@ static void ath11k_set_vht_txbf_cap(struct ath11k *ar, u32 *vht_cap) *vht_cap |= sound_dim; } - /* Use the STS advertised by FW unless SU Beamformee is not supported*/ - if (!subfee) - *vht_cap &= ~(IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK); + /* Enable Beamformee STS Field only if SU BF is enabled */ + if (subfee) { + if (nsts > (ar->num_rx_chains - 1)) + nsts = ar->num_rx_chains - 1; + + nsts <<= IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT; + nsts &= IEEE80211_VHT_CAP_BEAMFORMEE_STS_MASK; + *vht_cap |= nsts; + } } static struct ieee80211_sta_vht_cap
The number of spatial streams used when acting as a beamformee in VHT mode are reported by the firmware as 7 (8 sts - 1) both in IPQ6018 and IPQ8074 which respectively have 2 and 4 sts each. So the firmware should report 1 (2 - 1) and 3 (4 - 1). Fix this by checking that the number of VHT beamformee sts reported by the firmware is not greater than the number of receiving antennas - 1. The fix is based on the same approach used in this same function for sanitizing the number of sounding dimensions reported by the firmware. Without this change, acting as a beamformee in VHT mode is not working properly. Tested-on: IPQ6018 hw1.0 AHB WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1 Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1 Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") Signed-off-by: Jesus Fernandez Manzano <jesus.manzano@galgus.net> --- drivers/net/wireless/ath/ath11k/mac.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)