Message ID | 1666128501-12364-3-git-send-email-quic_msinada@quicinc.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 38dfe775d0abf511341f37c1cb77b919a3ad410b |
Delegated to: | Kalle Valo |
Headers | show |
Series | wifi: ath11k: push MU-MIMO configurations to hardware | expand |
On Tue, Oct 18, 2022 at 11:28 PM Muna Sinada <quic_msinada@quicinc.com> wrote: > > In the previous behaviour only HE IE in management frames are changed > regarding MU-MIMO configurations and not in hardware. Adding push of > MU-MIMO configurations to the hardware as well. > > This patch is dependant on mac80211 patchset: > https://patchwork.kernel.org/project/linux-wireless/list/?series=683322&state=%2A&archive=both > > Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-00356-QCAHKSWPL_SILICONZ-1 > > Co-developed-by: Anilkumar Kolli <quic_akolli@quicinc.com> > Signed-off-by: Anilkumar Kolli <quic_akolli@quicinc.com> > Signed-off-by: Muna Sinada <quic_msinada@quicinc.com> > --- > drivers/net/wireless/ath/ath11k/mac.c | 199 +++++++++++++++++++++------------- > drivers/net/wireless/ath/ath11k/wmi.h | 3 + > 2 files changed, 129 insertions(+), 73 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c > index 6d83a178a891..83d4a565fe84 100644 > --- a/drivers/net/wireless/ath/ath11k/mac.c > +++ b/drivers/net/wireless/ath/ath11k/mac.c > @@ -2697,6 +2697,117 @@ static int ath11k_setup_peer_smps(struct ath11k *ar, struct ath11k_vif *arvif, > ath11k_smps_map[smps]); > } > > +static bool ath11k_mac_set_he_txbf_conf(struct ath11k_vif *arvif) > +{ > + struct ath11k *ar = arvif->ar; > + u32 param, value; > + int ret; > + > + if (!arvif->vif->bss_conf.he_support) > + return true; > + > + param = WMI_VDEV_PARAM_SET_HEMU_MODE; > + value = 0; > + if (arvif->vif->bss_conf.he_su_beamformer) { > + value |= FIELD_PREP(HE_MODE_SU_TX_BFER, HE_SU_BFER_ENABLE); > + if (arvif->vif->bss_conf.he_mu_beamformer && > + arvif->vdev_type == WMI_VDEV_TYPE_AP) > + value |= FIELD_PREP(HE_MODE_MU_TX_BFER, HE_MU_BFER_ENABLE); > + } > + > + if (arvif->vif->type != NL80211_IFTYPE_MESH_POINT) { > + value |= FIELD_PREP(HE_MODE_DL_OFDMA, HE_DL_MUOFDMA_ENABLE) | > + FIELD_PREP(HE_MODE_UL_OFDMA, HE_UL_MUOFDMA_ENABLE); > + > + if (arvif->vif->bss_conf.he_full_ul_mumimo) > + value |= FIELD_PREP(HE_MODE_UL_MUMIMO, HE_UL_MUMIMO_ENABLE); > + > + if (arvif->vif->bss_conf.he_su_beamformee) > + value |= FIELD_PREP(HE_MODE_SU_TX_BFEE, HE_SU_BFEE_ENABLE); > + } > + > + ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, param, value); > + if (ret) { > + ath11k_warn(ar->ab, "failed to set vdev %d HE MU mode: %d\n", > + arvif->vdev_id, ret); > + return false; > + } > + > + param = WMI_VDEV_PARAM_SET_HE_SOUNDING_MODE; > + value = FIELD_PREP(HE_VHT_SOUNDING_MODE, HE_VHT_SOUNDING_MODE_ENABLE) | > + FIELD_PREP(HE_TRIG_NONTRIG_SOUNDING_MODE, > + HE_TRIG_NONTRIG_SOUNDING_MODE_ENABLE); > + ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, > + param, value); > + if (ret) { > + ath11k_warn(ar->ab, "failed to set vdev %d sounding mode: %d\n", > + arvif->vdev_id, ret); > + return false; > + } > + return true; > +} > + > +static bool ath11k_mac_vif_recalc_sta_he_txbf(struct ath11k *ar, > + struct ieee80211_vif *vif, > + struct ieee80211_sta_he_cap *he_cap) > +{ > + struct ath11k_vif *arvif = (void *)vif->drv_priv; > + struct ieee80211_he_cap_elem he_cap_elem = {0}; > + struct ieee80211_sta_he_cap *cap_band = NULL; > + struct cfg80211_chan_def def; > + u32 param = WMI_VDEV_PARAM_SET_HEMU_MODE; > + u32 hemode = 0; > + int ret; > + > + if (!vif->bss_conf.he_support) > + return true; > + > + if (vif->type != NL80211_IFTYPE_STATION) > + return false; > + > + if (WARN_ON(ath11k_mac_vif_chan(vif, &def))) > + return false; > + > + if (def.chan->band == NL80211_BAND_2GHZ) > + cap_band = &ar->mac.iftype[NL80211_BAND_2GHZ][vif->type].he_cap; > + else > + cap_band = &ar->mac.iftype[NL80211_BAND_5GHZ][vif->type].he_cap; > + > + memcpy(&he_cap_elem, &cap_band->he_cap_elem, sizeof(he_cap_elem)); > + > + if (HECAP_PHY_SUBFME_GET(he_cap_elem.phy_cap_info)) { > + if (HECAP_PHY_SUBFMR_GET(he_cap->he_cap_elem.phy_cap_info)) > + hemode |= FIELD_PREP(HE_MODE_SU_TX_BFEE, HE_SU_BFEE_ENABLE); > + if (HECAP_PHY_MUBFMR_GET(he_cap->he_cap_elem.phy_cap_info)) > + hemode |= FIELD_PREP(HE_MODE_MU_TX_BFEE, HE_MU_BFEE_ENABLE); > + } > + > + if (vif->type != NL80211_IFTYPE_MESH_POINT) { > + hemode |= FIELD_PREP(HE_MODE_DL_OFDMA, HE_DL_MUOFDMA_ENABLE) | > + FIELD_PREP(HE_MODE_UL_OFDMA, HE_UL_MUOFDMA_ENABLE); > + > + if (HECAP_PHY_ULMUMIMO_GET(he_cap_elem.phy_cap_info)) > + if (HECAP_PHY_ULMUMIMO_GET(he_cap->he_cap_elem.phy_cap_info)) > + hemode |= FIELD_PREP(HE_MODE_UL_MUMIMO, > + HE_UL_MUMIMO_ENABLE); > + > + if (FIELD_GET(HE_MODE_MU_TX_BFEE, hemode)) > + hemode |= FIELD_PREP(HE_MODE_SU_TX_BFEE, HE_SU_BFEE_ENABLE); > + > + if (FIELD_GET(HE_MODE_MU_TX_BFER, hemode)) > + hemode |= FIELD_PREP(HE_MODE_SU_TX_BFER, HE_SU_BFER_ENABLE); > + } > + > + ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, param, hemode); > + if (ret) { > + ath11k_warn(ar->ab, "failed to submit vdev param txbf 0x%x: %d\n", > + hemode, ret); > + return false; > + } > + > + return true; > +} > + > static void ath11k_bss_assoc(struct ieee80211_hw *hw, > struct ieee80211_vif *vif, > struct ieee80211_bss_conf *bss_conf) > @@ -2707,6 +2818,7 @@ static void ath11k_bss_assoc(struct ieee80211_hw *hw, > struct ieee80211_sta *ap_sta; > struct ath11k_peer *peer; > bool is_auth = false; > + struct ieee80211_sta_he_cap he_cap; > int ret; > > lockdep_assert_held(&ar->conf_mutex); > @@ -2723,6 +2835,8 @@ static void ath11k_bss_assoc(struct ieee80211_hw *hw, > rcu_read_unlock(); > return; > } > + /* he_cap here is updated at assoc success for sta mode only */ > + he_cap = ap_sta->deflink.he_cap; > > ath11k_peer_assoc_prepare(ar, vif, ap_sta, &peer_arg, false); > > @@ -2751,6 +2865,12 @@ static void ath11k_bss_assoc(struct ieee80211_hw *hw, > return; > } > > + if (!ath11k_mac_vif_recalc_sta_he_txbf(ar, vif, &he_cap)) { > + ath11k_warn(ar->ab, "failed to recalc he txbf for vdev %i on bss %pM\n", > + arvif->vdev_id, bss_conf->bssid); > + return; > + } > + > WARN_ON(arvif->is_up); > > arvif->aid = vif->cfg.aid; > @@ -3200,6 +3320,8 @@ static void ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw, > ether_addr_copy(arvif->bssid, info->bssid); > > if (changed & BSS_CHANGED_BEACON_ENABLED) { > + if (info->enable_beacon) > + ath11k_mac_set_he_txbf_conf(arvif); > ath11k_control_beaconing(arvif, info); > > if (arvif->is_up && vif->bss_conf.he_support && > @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct ath11k *ar, > > he_cap_elem->mac_cap_info[1] &= > IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; > + he_cap_elem->phy_cap_info[0] &= > + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; > + he_cap_elem->phy_cap_info[0] &= > + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; Hi, This is causing a regression for us in OpenWrt at least on IPQ8074 but probably on all ath11k-supported HW. Cause 80+80 and 160MHz support bits are being cleared here so 160MHz is not being advertised after this patch. I fail to understand why are 80+80 and 160 MHz feature flags being cleared? Regards, Robert > > he_cap_elem->phy_cap_info[5] &= > ~IEEE80211_HE_PHY_CAP5_BEAMFORMEE_NUM_SND_DIM_UNDER_80MHZ_MASK; > @@ -6003,69 +6129,6 @@ ath11k_mac_setup_vdev_create_params(struct ath11k_vif *arvif, > } > } > > -static u32 > -ath11k_mac_prepare_he_mode(struct ath11k_pdev *pdev, u32 viftype) > -{ > - struct ath11k_pdev_cap *pdev_cap = &pdev->cap; > - struct ath11k_band_cap *cap_band = NULL; > - u32 *hecap_phy_ptr = NULL; > - u32 hemode = 0; > - > - if (pdev->cap.supported_bands & WMI_HOST_WLAN_2G_CAP) > - cap_band = &pdev_cap->band[NL80211_BAND_2GHZ]; > - else > - cap_band = &pdev_cap->band[NL80211_BAND_5GHZ]; > - > - hecap_phy_ptr = &cap_band->he_cap_phy_info[0]; > - > - hemode = FIELD_PREP(HE_MODE_SU_TX_BFEE, HE_SU_BFEE_ENABLE) | > - FIELD_PREP(HE_MODE_SU_TX_BFER, HECAP_PHY_SUBFMR_GET(hecap_phy_ptr)) | > - FIELD_PREP(HE_MODE_UL_MUMIMO, HECAP_PHY_ULMUMIMO_GET(hecap_phy_ptr)); > - > - /* TODO WDS and other modes */ > - if (viftype == NL80211_IFTYPE_AP) { > - hemode |= FIELD_PREP(HE_MODE_MU_TX_BFER, > - HECAP_PHY_MUBFMR_GET(hecap_phy_ptr)) | > - FIELD_PREP(HE_MODE_DL_OFDMA, HE_DL_MUOFDMA_ENABLE) | > - FIELD_PREP(HE_MODE_UL_OFDMA, HE_UL_MUOFDMA_ENABLE); > - } else { > - hemode |= FIELD_PREP(HE_MODE_MU_TX_BFEE, HE_MU_BFEE_ENABLE); > - } > - > - return hemode; > -} > - > -static int ath11k_set_he_mu_sounding_mode(struct ath11k *ar, > - struct ath11k_vif *arvif) > -{ > - u32 param_id, param_value; > - struct ath11k_base *ab = ar->ab; > - int ret = 0; > - > - param_id = WMI_VDEV_PARAM_SET_HEMU_MODE; > - param_value = ath11k_mac_prepare_he_mode(ar->pdev, arvif->vif->type); > - ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, > - param_id, param_value); > - if (ret) { > - ath11k_warn(ab, "failed to set vdev %d HE MU mode: %d param_value %x\n", > - arvif->vdev_id, ret, param_value); > - return ret; > - } > - param_id = WMI_VDEV_PARAM_SET_HE_SOUNDING_MODE; > - param_value = > - FIELD_PREP(HE_VHT_SOUNDING_MODE, HE_VHT_SOUNDING_MODE_ENABLE) | > - FIELD_PREP(HE_TRIG_NONTRIG_SOUNDING_MODE, > - HE_TRIG_NONTRIG_SOUNDING_MODE_ENABLE); > - ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, > - param_id, param_value); > - if (ret) { > - ath11k_warn(ab, "failed to set vdev %d HE MU mode: %d\n", > - arvif->vdev_id, ret); > - return ret; > - } > - return ret; > -} > - > static void ath11k_mac_op_update_vif_offload(struct ieee80211_hw *hw, > struct ieee80211_vif *vif) > { > @@ -6734,7 +6797,6 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif, > struct ath11k_base *ab = ar->ab; > struct wmi_vdev_start_req_arg arg = {}; > const struct cfg80211_chan_def *chandef = &ctx->def; > - int he_support = arvif->vif->bss_conf.he_support; > int ret = 0; > > lockdep_assert_held(&ar->conf_mutex); > @@ -6775,15 +6837,6 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif, > spin_lock_bh(&ab->base_lock); > arg.regdomain = ar->ab->dfs_region; > spin_unlock_bh(&ab->base_lock); > - > - if (he_support) { > - ret = ath11k_set_he_mu_sounding_mode(ar, arvif); > - if (ret) { > - ath11k_warn(ar->ab, "failed to set he mode vdev %i\n", > - arg.vdev_id); > - return ret; > - } > - } > } > > arg.channel.passive |= !!(chandef->chan->flags & IEEE80211_CHAN_NO_IR); > diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h > index 368b7755e800..7e1eea784eb5 100644 > --- a/drivers/net/wireless/ath/ath11k/wmi.h > +++ b/drivers/net/wireless/ath/ath11k/wmi.h > @@ -2895,8 +2895,11 @@ struct rx_reorder_queue_remove_params { > #define HE_DL_MUOFDMA_ENABLE 1 > #define HE_UL_MUOFDMA_ENABLE 1 > #define HE_DL_MUMIMO_ENABLE 1 > +#define HE_UL_MUMIMO_ENABLE 1 > #define HE_MU_BFEE_ENABLE 1 > #define HE_SU_BFEE_ENABLE 1 > +#define HE_MU_BFER_ENABLE 1 > +#define HE_SU_BFER_ENABLE 1 > > #define HE_VHT_SOUNDING_MODE_ENABLE 1 > #define HE_SU_MU_SOUNDING_MODE_ENABLE 1 > -- > 2.7.4 > > > -- > ath11k mailing list > ath11k@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/ath11k
Robert Marko <robert.marko@sartura.hr> writes: > On Tue, Oct 18, 2022 at 11:28 PM Muna Sinada <quic_msinada@quicinc.com> wrote: > >> >> In the previous behaviour only HE IE in management frames are changed >> regarding MU-MIMO configurations and not in hardware. Adding push of >> MU-MIMO configurations to the hardware as well. >> >> This patch is dependant on mac80211 patchset: >> https://patchwork.kernel.org/project/linux-wireless/list/?series=683322&state=%2A&archive=both >> >> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-00356-QCAHKSWPL_SILICONZ-1 >> >> Co-developed-by: Anilkumar Kolli <quic_akolli@quicinc.com> >> Signed-off-by: Anilkumar Kolli <quic_akolli@quicinc.com> >> Signed-off-by: Muna Sinada <quic_msinada@quicinc.com> [...] >> @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct ath11k *ar, >> >> he_cap_elem->mac_cap_info[1] &= >> IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; >> + he_cap_elem->phy_cap_info[0] &= >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; >> + he_cap_elem->phy_cap_info[0] &= >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; > > This is causing a regression for us in OpenWrt at least on IPQ8074 but > probably on all ath11k-supported HW. Cause 80+80 and 160MHz support > bits are being cleared here so 160MHz is not being advertised after > this patch. Oh man, not good. Robert, should we revert this patchset entirely? Of course it would be better if Muna can submit quickly a fix, but I'm not going to wait for long. The patchset is in wireless-next at the moment and the commits from the patchset are: a96f10422e74 wifi: ath11k: modify accessor macros to match index size 38dfe775d0ab wifi: ath11k: push MU-MIMO params from hostapd to hardware 8077c1bbbc28 wifi: ath11k: move HE MCS mapper to a separate function ebf82988f844 wifi: ath11k: generate rx and tx mcs maps for supported HE mcs > I fail to understand why are 80+80 and 160 MHz feature flags being cleared? Me neither. I missed that in my review.
On Wed, Mar 29, 2023 at 11:45 AM Kalle Valo <kvalo@kernel.org> wrote: > > Robert Marko <robert.marko@sartura.hr> writes: > > > On Tue, Oct 18, 2022 at 11:28 PM Muna Sinada <quic_msinada@quicinc.com> wrote: > > > >> > >> In the previous behaviour only HE IE in management frames are changed > >> regarding MU-MIMO configurations and not in hardware. Adding push of > >> MU-MIMO configurations to the hardware as well. > >> > >> This patch is dependant on mac80211 patchset: > >> https://patchwork.kernel.org/project/linux-wireless/list/?series=683322&state=%2A&archive=both > >> > >> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-00356-QCAHKSWPL_SILICONZ-1 > >> > >> Co-developed-by: Anilkumar Kolli <quic_akolli@quicinc.com> > >> Signed-off-by: Anilkumar Kolli <quic_akolli@quicinc.com> > >> Signed-off-by: Muna Sinada <quic_msinada@quicinc.com> > > [...] > > >> @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct ath11k *ar, > >> > >> he_cap_elem->mac_cap_info[1] &= > >> IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; > >> + he_cap_elem->phy_cap_info[0] &= > >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; > >> + he_cap_elem->phy_cap_info[0] &= > >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; > > > > This is causing a regression for us in OpenWrt at least on IPQ8074 but > > probably on all ath11k-supported HW. Cause 80+80 and 160MHz support > > bits are being cleared here so 160MHz is not being advertised after > > this patch. > > Oh man, not good. Robert, should we revert this patchset entirely? Of > course it would be better if Muna can submit quickly a fix, but I'm not > going to wait for long. I would prefer to see it get fixed, cause just removing the flag removal gets 160MHz working, but I am not sure about other flags as well. Regards, Robert > > The patchset is in wireless-next at the moment and the commits from the > patchset are: > > a96f10422e74 wifi: ath11k: modify accessor macros to match index size > 38dfe775d0ab wifi: ath11k: push MU-MIMO params from hostapd to hardware > 8077c1bbbc28 wifi: ath11k: move HE MCS mapper to a separate function > ebf82988f844 wifi: ath11k: generate rx and tx mcs maps for supported HE mcs > > > I fail to understand why are 80+80 and 160 MHz feature flags being cleared? > > Me neither. I missed that in my review. > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Robert Marko <robert.marko@sartura.hr> writes: > On Wed, Mar 29, 2023 at 11:45 AM Kalle Valo <kvalo@kernel.org> wrote: > >> >> @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct ath11k *ar, >> >> >> >> he_cap_elem->mac_cap_info[1] &= >> >> IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; >> >> + he_cap_elem->phy_cap_info[0] &= >> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; >> >> + he_cap_elem->phy_cap_info[0] &= >> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; >> > >> > This is causing a regression for us in OpenWrt at least on IPQ8074 but >> > probably on all ath11k-supported HW. Cause 80+80 and 160MHz support >> > bits are being cleared here so 160MHz is not being advertised after >> > this patch. >> >> Oh man, not good. Robert, should we revert this patchset entirely? Of >> course it would be better if Muna can submit quickly a fix, but I'm not >> going to wait for long. > > I would prefer to see it get fixed, cause just removing the flag > removal gets 160MHz working, but I am not sure about other flags as > well. Ok, let's try to get it fixed then. Muna, can you comment and send a fix ASAP?
Kalle Valo <kvalo@kernel.org> writes: > Robert Marko <robert.marko@sartura.hr> writes: > >> On Wed, Mar 29, 2023 at 11:45 AM Kalle Valo <kvalo@kernel.org> wrote: >> >>> >> @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct ath11k *ar, >>> >> >>> >> he_cap_elem->mac_cap_info[1] &= >>> >> IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; >>> >> + he_cap_elem->phy_cap_info[0] &= >>> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; >>> >> + he_cap_elem->phy_cap_info[0] &= >>> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; >>> > >>> > This is causing a regression for us in OpenWrt at least on IPQ8074 but >>> > probably on all ath11k-supported HW. Cause 80+80 and 160MHz support >>> > bits are being cleared here so 160MHz is not being advertised after >>> > this patch. >>> >>> Oh man, not good. Robert, should we revert this patchset entirely? Of >>> course it would be better if Muna can submit quickly a fix, but I'm not >>> going to wait for long. >> >> I would prefer to see it get fixed, cause just removing the flag >> removal gets 160MHz working, but I am not sure about other flags as >> well. > > Ok, let's try to get it fixed then. Muna, can you comment and send a fix > ASAP? No reply from Muna. I'll try the new bugbot to open a bug for this regression: bugbot on
Muna Sinada <quic_msinada@quicinc.com> writes: > Kalle Valo <kvalo@kernel.org> writes: >> Robert Marko <robert.marko@sartura.hr> writes: >> >>> On Wed, Mar 29, 2023 at 11:45 AM Kalle Valo <kvalo@kernel.org> wrote: >>> >>>> >> @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct >>>> >> ath11k *ar, >>>> >> >>>> >> he_cap_elem->mac_cap_info[1] &= >>>> >> >>>> >> IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; >>>> >> + he_cap_elem->phy_cap_info[0] &= >>>> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; >>>> >> + he_cap_elem->phy_cap_info[0] &= >>>> >> + >>>> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; >>>> > >>>> > This is causing a regression for us in OpenWrt at least on IPQ8074 >>>> > but probably on all ath11k-supported HW. Cause 80+80 and 160MHz >>>> > support bits are being cleared here so 160MHz is not being >>>> > advertised after this patch. >>>> >>>> Oh man, not good. Robert, should we revert this patchset entirely? >>>> Of course it would be better if Muna can submit quickly a fix, but >>>> I'm not going to wait for long. >>> >>> I would prefer to see it get fixed, cause just removing the flag >>> removal gets 160MHz working, but I am not sure about other flags as >>> well. >> >> Ok, let's try to get it fixed then. Muna, can you comment and send a >> fix ASAP? > >No reply from Muna. I'll try the new bugbot to open a bug for this regression: > >bugbot on I have created fix, tested it and pushed internally for review. @Kalle, how do I go about continuing the conversation on Bugzilla?
"Muna Sinada (QUIC)" <quic_msinada@quicinc.com> writes: > Muna Sinada <quic_msinada@quicinc.com> writes: >> Kalle Valo <kvalo@kernel.org> writes: >>> Robert Marko <robert.marko@sartura.hr> writes: >>> >>>> On Wed, Mar 29, 2023 at 11:45 AM Kalle Valo <kvalo@kernel.org> wrote: >>>> >>>>> >> @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct >>>>> >> ath11k *ar, >>>>> >> >>>>> >> he_cap_elem->mac_cap_info[1] &= >>>>> >> >>>>> >> IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; >>>>> >> + he_cap_elem->phy_cap_info[0] &= >>>>> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; >>>>> >> + he_cap_elem->phy_cap_info[0] &= >>>>> >> + >>>>> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; >>>>> > >>>>> > This is causing a regression for us in OpenWrt at least on IPQ8074 >>>>> > but probably on all ath11k-supported HW. Cause 80+80 and 160MHz >>>>> > support bits are being cleared here so 160MHz is not being >>>>> > advertised after this patch. >>>>> >>>>> Oh man, not good. Robert, should we revert this patchset entirely? >>>>> Of course it would be better if Muna can submit quickly a fix, but >>>>> I'm not going to wait for long. >>>> >>>> I would prefer to see it get fixed, cause just removing the flag >>>> removal gets 160MHz working, but I am not sure about other flags as >>>> well. >>> >>> Ok, let's try to get it fixed then. Muna, can you comment and send a >>> fix ASAP? >> >>No reply from Muna. I'll try the new bugbot to open a bug for this regression: >> >>bugbot on > > I have created fix, tested it and pushed internally for review. > > @Kalle, how do I go about continuing the conversation on Bugzilla? Better to continue the discussion on the list. I only created the bug on Bugzilla so that I don't forget this as I didn't hear anything from you.
Kalle Valo <kvalo@kernel.org> writes: > "Muna Sinada (QUIC)" <quic_msinada@quicinc.com> writes: > >> Muna Sinada <quic_msinada@quicinc.com> writes: >>> Kalle Valo <kvalo@kernel.org> writes: >>>> Robert Marko <robert.marko@sartura.hr> writes: >>>> >>>>> On Wed, Mar 29, 2023 at 11:45 AM Kalle Valo <kvalo@kernel.org> wrote: >>>>> >>>>>> >> @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct >>>>>> >> ath11k *ar, >>>>>> >> >>>>>> >> he_cap_elem->mac_cap_info[1] &= >>>>>> >> >>>>>> >> IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; >>>>>> >> + he_cap_elem->phy_cap_info[0] &= >>>>>> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; >>>>>> >> + he_cap_elem->phy_cap_info[0] &= >>>>>> >> + >>>>>> >> + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; >>>>>> > >>>>>> > This is causing a regression for us in OpenWrt at least on IPQ8074 >>>>>> > but probably on all ath11k-supported HW. Cause 80+80 and 160MHz >>>>>> > support bits are being cleared here so 160MHz is not being >>>>>> > advertised after this patch. >>>>>> >>>>>> Oh man, not good. Robert, should we revert this patchset entirely? >>>>>> Of course it would be better if Muna can submit quickly a fix, but >>>>>> I'm not going to wait for long. >>>>> >>>>> I would prefer to see it get fixed, cause just removing the flag >>>>> removal gets 160MHz working, but I am not sure about other flags as >>>>> well. >>>> >>>> Ok, let's try to get it fixed then. Muna, can you comment and send a >>>> fix ASAP? >>> >>>No reply from Muna. I'll try the new bugbot to open a bug for this regression: >>> >>>bugbot on >> >> I have created fix, tested it and pushed internally for review. >> >> @Kalle, how do I go about continuing the conversation on Bugzilla? > > Better to continue the discussion on the list. I only created the bug on > Bugzilla so that I don't forget this as I didn't hear anything from you. Muna now submitted a fix: https://patchwork.kernel.org/project/linux-wireless/patch/1681762947-13882-1-git-send-email-quic_msinada@quicinc.com/ Robert, can you please check it? And if it works a Tested-by would be nice.
> Muna now submitted a fix: > > https://patchwork.kernel.org/project/linux-wireless/patch/1681762947-13882-1-git-send-email-quic_msinada@quicinc.com/ > > Robert, can you please check it? And if it works a Tested-by would be > nice. Hi, I already saw it and gave it a quick test, for me it brings up 160MHz and I have requested some more testing from other community members. I will reply with my Tested-by tag. Thanks for fixing the issue. Regards, Robert
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 6d83a178a891..83d4a565fe84 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -2697,6 +2697,117 @@ static int ath11k_setup_peer_smps(struct ath11k *ar, struct ath11k_vif *arvif, ath11k_smps_map[smps]); } +static bool ath11k_mac_set_he_txbf_conf(struct ath11k_vif *arvif) +{ + struct ath11k *ar = arvif->ar; + u32 param, value; + int ret; + + if (!arvif->vif->bss_conf.he_support) + return true; + + param = WMI_VDEV_PARAM_SET_HEMU_MODE; + value = 0; + if (arvif->vif->bss_conf.he_su_beamformer) { + value |= FIELD_PREP(HE_MODE_SU_TX_BFER, HE_SU_BFER_ENABLE); + if (arvif->vif->bss_conf.he_mu_beamformer && + arvif->vdev_type == WMI_VDEV_TYPE_AP) + value |= FIELD_PREP(HE_MODE_MU_TX_BFER, HE_MU_BFER_ENABLE); + } + + if (arvif->vif->type != NL80211_IFTYPE_MESH_POINT) { + value |= FIELD_PREP(HE_MODE_DL_OFDMA, HE_DL_MUOFDMA_ENABLE) | + FIELD_PREP(HE_MODE_UL_OFDMA, HE_UL_MUOFDMA_ENABLE); + + if (arvif->vif->bss_conf.he_full_ul_mumimo) + value |= FIELD_PREP(HE_MODE_UL_MUMIMO, HE_UL_MUMIMO_ENABLE); + + if (arvif->vif->bss_conf.he_su_beamformee) + value |= FIELD_PREP(HE_MODE_SU_TX_BFEE, HE_SU_BFEE_ENABLE); + } + + ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, param, value); + if (ret) { + ath11k_warn(ar->ab, "failed to set vdev %d HE MU mode: %d\n", + arvif->vdev_id, ret); + return false; + } + + param = WMI_VDEV_PARAM_SET_HE_SOUNDING_MODE; + value = FIELD_PREP(HE_VHT_SOUNDING_MODE, HE_VHT_SOUNDING_MODE_ENABLE) | + FIELD_PREP(HE_TRIG_NONTRIG_SOUNDING_MODE, + HE_TRIG_NONTRIG_SOUNDING_MODE_ENABLE); + ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, + param, value); + if (ret) { + ath11k_warn(ar->ab, "failed to set vdev %d sounding mode: %d\n", + arvif->vdev_id, ret); + return false; + } + return true; +} + +static bool ath11k_mac_vif_recalc_sta_he_txbf(struct ath11k *ar, + struct ieee80211_vif *vif, + struct ieee80211_sta_he_cap *he_cap) +{ + struct ath11k_vif *arvif = (void *)vif->drv_priv; + struct ieee80211_he_cap_elem he_cap_elem = {0}; + struct ieee80211_sta_he_cap *cap_band = NULL; + struct cfg80211_chan_def def; + u32 param = WMI_VDEV_PARAM_SET_HEMU_MODE; + u32 hemode = 0; + int ret; + + if (!vif->bss_conf.he_support) + return true; + + if (vif->type != NL80211_IFTYPE_STATION) + return false; + + if (WARN_ON(ath11k_mac_vif_chan(vif, &def))) + return false; + + if (def.chan->band == NL80211_BAND_2GHZ) + cap_band = &ar->mac.iftype[NL80211_BAND_2GHZ][vif->type].he_cap; + else + cap_band = &ar->mac.iftype[NL80211_BAND_5GHZ][vif->type].he_cap; + + memcpy(&he_cap_elem, &cap_band->he_cap_elem, sizeof(he_cap_elem)); + + if (HECAP_PHY_SUBFME_GET(he_cap_elem.phy_cap_info)) { + if (HECAP_PHY_SUBFMR_GET(he_cap->he_cap_elem.phy_cap_info)) + hemode |= FIELD_PREP(HE_MODE_SU_TX_BFEE, HE_SU_BFEE_ENABLE); + if (HECAP_PHY_MUBFMR_GET(he_cap->he_cap_elem.phy_cap_info)) + hemode |= FIELD_PREP(HE_MODE_MU_TX_BFEE, HE_MU_BFEE_ENABLE); + } + + if (vif->type != NL80211_IFTYPE_MESH_POINT) { + hemode |= FIELD_PREP(HE_MODE_DL_OFDMA, HE_DL_MUOFDMA_ENABLE) | + FIELD_PREP(HE_MODE_UL_OFDMA, HE_UL_MUOFDMA_ENABLE); + + if (HECAP_PHY_ULMUMIMO_GET(he_cap_elem.phy_cap_info)) + if (HECAP_PHY_ULMUMIMO_GET(he_cap->he_cap_elem.phy_cap_info)) + hemode |= FIELD_PREP(HE_MODE_UL_MUMIMO, + HE_UL_MUMIMO_ENABLE); + + if (FIELD_GET(HE_MODE_MU_TX_BFEE, hemode)) + hemode |= FIELD_PREP(HE_MODE_SU_TX_BFEE, HE_SU_BFEE_ENABLE); + + if (FIELD_GET(HE_MODE_MU_TX_BFER, hemode)) + hemode |= FIELD_PREP(HE_MODE_SU_TX_BFER, HE_SU_BFER_ENABLE); + } + + ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, param, hemode); + if (ret) { + ath11k_warn(ar->ab, "failed to submit vdev param txbf 0x%x: %d\n", + hemode, ret); + return false; + } + + return true; +} + static void ath11k_bss_assoc(struct ieee80211_hw *hw, struct ieee80211_vif *vif, struct ieee80211_bss_conf *bss_conf) @@ -2707,6 +2818,7 @@ static void ath11k_bss_assoc(struct ieee80211_hw *hw, struct ieee80211_sta *ap_sta; struct ath11k_peer *peer; bool is_auth = false; + struct ieee80211_sta_he_cap he_cap; int ret; lockdep_assert_held(&ar->conf_mutex); @@ -2723,6 +2835,8 @@ static void ath11k_bss_assoc(struct ieee80211_hw *hw, rcu_read_unlock(); return; } + /* he_cap here is updated at assoc success for sta mode only */ + he_cap = ap_sta->deflink.he_cap; ath11k_peer_assoc_prepare(ar, vif, ap_sta, &peer_arg, false); @@ -2751,6 +2865,12 @@ static void ath11k_bss_assoc(struct ieee80211_hw *hw, return; } + if (!ath11k_mac_vif_recalc_sta_he_txbf(ar, vif, &he_cap)) { + ath11k_warn(ar->ab, "failed to recalc he txbf for vdev %i on bss %pM\n", + arvif->vdev_id, bss_conf->bssid); + return; + } + WARN_ON(arvif->is_up); arvif->aid = vif->cfg.aid; @@ -3200,6 +3320,8 @@ static void ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw, ether_addr_copy(arvif->bssid, info->bssid); if (changed & BSS_CHANGED_BEACON_ENABLED) { + if (info->enable_beacon) + ath11k_mac_set_he_txbf_conf(arvif); ath11k_control_beaconing(arvif, info); if (arvif->is_up && vif->bss_conf.he_support && @@ -5369,6 +5491,10 @@ static int ath11k_mac_copy_he_cap(struct ath11k *ar, he_cap_elem->mac_cap_info[1] &= IEEE80211_HE_MAC_CAP1_TF_MAC_PAD_DUR_MASK; + he_cap_elem->phy_cap_info[0] &= + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; + he_cap_elem->phy_cap_info[0] &= + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; he_cap_elem->phy_cap_info[5] &= ~IEEE80211_HE_PHY_CAP5_BEAMFORMEE_NUM_SND_DIM_UNDER_80MHZ_MASK; @@ -6003,69 +6129,6 @@ ath11k_mac_setup_vdev_create_params(struct ath11k_vif *arvif, } } -static u32 -ath11k_mac_prepare_he_mode(struct ath11k_pdev *pdev, u32 viftype) -{ - struct ath11k_pdev_cap *pdev_cap = &pdev->cap; - struct ath11k_band_cap *cap_band = NULL; - u32 *hecap_phy_ptr = NULL; - u32 hemode = 0; - - if (pdev->cap.supported_bands & WMI_HOST_WLAN_2G_CAP) - cap_band = &pdev_cap->band[NL80211_BAND_2GHZ]; - else - cap_band = &pdev_cap->band[NL80211_BAND_5GHZ]; - - hecap_phy_ptr = &cap_band->he_cap_phy_info[0]; - - hemode = FIELD_PREP(HE_MODE_SU_TX_BFEE, HE_SU_BFEE_ENABLE) | - FIELD_PREP(HE_MODE_SU_TX_BFER, HECAP_PHY_SUBFMR_GET(hecap_phy_ptr)) | - FIELD_PREP(HE_MODE_UL_MUMIMO, HECAP_PHY_ULMUMIMO_GET(hecap_phy_ptr)); - - /* TODO WDS and other modes */ - if (viftype == NL80211_IFTYPE_AP) { - hemode |= FIELD_PREP(HE_MODE_MU_TX_BFER, - HECAP_PHY_MUBFMR_GET(hecap_phy_ptr)) | - FIELD_PREP(HE_MODE_DL_OFDMA, HE_DL_MUOFDMA_ENABLE) | - FIELD_PREP(HE_MODE_UL_OFDMA, HE_UL_MUOFDMA_ENABLE); - } else { - hemode |= FIELD_PREP(HE_MODE_MU_TX_BFEE, HE_MU_BFEE_ENABLE); - } - - return hemode; -} - -static int ath11k_set_he_mu_sounding_mode(struct ath11k *ar, - struct ath11k_vif *arvif) -{ - u32 param_id, param_value; - struct ath11k_base *ab = ar->ab; - int ret = 0; - - param_id = WMI_VDEV_PARAM_SET_HEMU_MODE; - param_value = ath11k_mac_prepare_he_mode(ar->pdev, arvif->vif->type); - ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, - param_id, param_value); - if (ret) { - ath11k_warn(ab, "failed to set vdev %d HE MU mode: %d param_value %x\n", - arvif->vdev_id, ret, param_value); - return ret; - } - param_id = WMI_VDEV_PARAM_SET_HE_SOUNDING_MODE; - param_value = - FIELD_PREP(HE_VHT_SOUNDING_MODE, HE_VHT_SOUNDING_MODE_ENABLE) | - FIELD_PREP(HE_TRIG_NONTRIG_SOUNDING_MODE, - HE_TRIG_NONTRIG_SOUNDING_MODE_ENABLE); - ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, - param_id, param_value); - if (ret) { - ath11k_warn(ab, "failed to set vdev %d HE MU mode: %d\n", - arvif->vdev_id, ret); - return ret; - } - return ret; -} - static void ath11k_mac_op_update_vif_offload(struct ieee80211_hw *hw, struct ieee80211_vif *vif) { @@ -6734,7 +6797,6 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif, struct ath11k_base *ab = ar->ab; struct wmi_vdev_start_req_arg arg = {}; const struct cfg80211_chan_def *chandef = &ctx->def; - int he_support = arvif->vif->bss_conf.he_support; int ret = 0; lockdep_assert_held(&ar->conf_mutex); @@ -6775,15 +6837,6 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif, spin_lock_bh(&ab->base_lock); arg.regdomain = ar->ab->dfs_region; spin_unlock_bh(&ab->base_lock); - - if (he_support) { - ret = ath11k_set_he_mu_sounding_mode(ar, arvif); - if (ret) { - ath11k_warn(ar->ab, "failed to set he mode vdev %i\n", - arg.vdev_id); - return ret; - } - } } arg.channel.passive |= !!(chandef->chan->flags & IEEE80211_CHAN_NO_IR); diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h index 368b7755e800..7e1eea784eb5 100644 --- a/drivers/net/wireless/ath/ath11k/wmi.h +++ b/drivers/net/wireless/ath/ath11k/wmi.h @@ -2895,8 +2895,11 @@ struct rx_reorder_queue_remove_params { #define HE_DL_MUOFDMA_ENABLE 1 #define HE_UL_MUOFDMA_ENABLE 1 #define HE_DL_MUMIMO_ENABLE 1 +#define HE_UL_MUMIMO_ENABLE 1 #define HE_MU_BFEE_ENABLE 1 #define HE_SU_BFEE_ENABLE 1 +#define HE_MU_BFER_ENABLE 1 +#define HE_SU_BFER_ENABLE 1 #define HE_VHT_SOUNDING_MODE_ENABLE 1 #define HE_SU_MU_SOUNDING_MODE_ENABLE 1