| Message ID | 20190503072146.49999-2-chiu@endlessm.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | Improve TX performance of RTL8723BU on rtl8xxxu driver | expand |
Chris Chiu <chiu@endlessm.com> writes: > Add wireless mode, signal strength level, and rate table index > to tell the firmware that we need to adjust the tx rate bitmap > accordingly. Please read Developer's Certificate of Origin and add Signed-off-by to the commit logs: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#signed-off-by_missing
Hi Chris, Thanks for this! Some suggestions below, although let me know if any don't make sense. On Fri, May 3, 2019 at 3:22 PM Chris Chiu <chiu@endlessm.com> wrote: > > Add wireless mode, signal strength level, and rate table index > to tell the firmware that we need to adjust the tx rate bitmap > accordingly. > --- > .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 45 +++++++++++++++++++ > .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 45 ++++++++++++++++++- > 2 files changed, 89 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h > index 8828baf26e7b..771f58aa7cae 100644 > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h > @@ -1195,6 +1195,50 @@ struct rtl8723bu_c2h { > > struct rtl8xxxu_fileops; > > +/*mlme related.*/ > +enum wireless_mode { > + WIRELESS_MODE_UNKNOWN = 0, > + //Sub-Element Run these patches through checkpatch.pl, it'll have some suggestions to bring the coding style in line, for example not using // style comments. > + WIRELESS_MODE_B = BIT(0), // tx: cck only , rx: cck only, hw: cck > + WIRELESS_MODE_G = BIT(1), // tx: ofdm only, rx: ofdm & cck, hw: cck & ofdm > + WIRELESS_MODE_A = BIT(2), // tx: ofdm only, rx: ofdm only, hw: ofdm only > + WIRELESS_MODE_N_24G = BIT(3), // tx: MCS only, rx: MCS & cck, hw: MCS & cck > + WIRELESS_MODE_N_5G = BIT(4), // tx: MCS only, rx: MCS & ofdm, hw: ofdm only > + WIRELESS_AUTO = BIT(5), > + WIRELESS_MODE_AC = BIT(6), > + WIRELESS_MODE_MAX = (WIRELESS_MODE_B|WIRELESS_MODE_G|WIRELESS_MODE_A|WIRELESS_MODE_N_24G|WIRELESS_MODE_N_5G|WIRELESS_MODE_AC), > +}; > + > +/* from rtlwifi/wifi.h */ > +enum ratr_table_mode_new { > + RATEID_IDX_BGN_40M_2SS = 0, > + RATEID_IDX_BGN_40M_1SS = 1, > + RATEID_IDX_BGN_20M_2SS_BN = 2, > + RATEID_IDX_BGN_20M_1SS_BN = 3, > + RATEID_IDX_GN_N2SS = 4, > + RATEID_IDX_GN_N1SS = 5, > + RATEID_IDX_BG = 6, > + RATEID_IDX_G = 7, > + RATEID_IDX_B = 8, > + RATEID_IDX_VHT_2SS = 9, > + RATEID_IDX_VHT_1SS = 10, > + RATEID_IDX_MIX1 = 11, > + RATEID_IDX_MIX2 = 12, > + RATEID_IDX_VHT_3SS = 13, > + RATEID_IDX_BGN_3SS = 14, > +}; > + > +#define RTL8XXXU_RATR_STA_INIT 0 > +#define RTL8XXXU_RATR_STA_HIGH 1 > +#define RTL8XXXU_RATR_STA_MID 2 > +#define RTL8XXXU_RATR_STA_LOW 3 > + > +struct rtl8xxxu_rate_adaptive { > + u16 wireless_mode; > + u8 ratr_index; > + u8 rssi_level; /* INIT, HIGH, MIDDLE, LOW */ > +} __packed; It would be better/cleaner to avoid storing data in per-device structures if at all possible. For wireless_mode, I think you should just call rtl8xxxu_wireless_mode() every time from rtl8723b_refresh_rate_mask(). The work done there is simple (i.e. it's not expensive to call) and then we avoid having to store data (which might become stale etc). For ratr_index, I believe you can just make it a parameter passed to rtl8xxxu_gen2_update_rate_mask which is the only consumer of this variable. The two callsites (rtl8xxxu_bss_info_changed and rtl8723b_refresh_rate_mask) already know which value they want to be used. rssi_level is the one that you probably do want to store, since I see the logic in patch 2 - if the rssi level hasn't changed then you don't need to set the rate mask again, and that's a good idea since it reduces bus traffic. You could move this into rtl8xxxu_priv rather than having its own separate structure. After making those changes it might even make sense to collapse this all into a single patch; you can judge!
On Thu, 2019-05-09 at 16:11 +0800, Daniel Drake wrote: > On Fri, May 3, 2019 at 3:22 PM Chris Chiu <chiu@endlessm.com> wrote: > > Add wireless mode, signal strength level, and rate table index > > to tell the firmware that we need to adjust the tx rate bitmap > > accordingly. [] > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h [] > > +/*mlme related.*/ > > +enum wireless_mode { > > + WIRELESS_MODE_UNKNOWN = 0, > > + //Sub-Element > > Run these patches through checkpatch.pl, it'll have some suggestions > to bring the coding style in line, for example not using // style > comments. just fyi: checkpatch ignores // comments since 2016 (new in 2019: unless you add --ignore=c99_comment_tolerance) These are the relevant checkpatch commits: In 2016, commit dadf680de3c2 ("checkpatch: allow c99 style // comments") In 2019, commit 98005e8c743f ("checkpatch: allow reporting C99 style comments")
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h index 8828baf26e7b..771f58aa7cae 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h @@ -1195,6 +1195,50 @@ struct rtl8723bu_c2h { struct rtl8xxxu_fileops; +/*mlme related.*/ +enum wireless_mode { + WIRELESS_MODE_UNKNOWN = 0, + //Sub-Element + WIRELESS_MODE_B = BIT(0), // tx: cck only , rx: cck only, hw: cck + WIRELESS_MODE_G = BIT(1), // tx: ofdm only, rx: ofdm & cck, hw: cck & ofdm + WIRELESS_MODE_A = BIT(2), // tx: ofdm only, rx: ofdm only, hw: ofdm only + WIRELESS_MODE_N_24G = BIT(3), // tx: MCS only, rx: MCS & cck, hw: MCS & cck + WIRELESS_MODE_N_5G = BIT(4), // tx: MCS only, rx: MCS & ofdm, hw: ofdm only + WIRELESS_AUTO = BIT(5), + WIRELESS_MODE_AC = BIT(6), + WIRELESS_MODE_MAX = (WIRELESS_MODE_B|WIRELESS_MODE_G|WIRELESS_MODE_A|WIRELESS_MODE_N_24G|WIRELESS_MODE_N_5G|WIRELESS_MODE_AC), +}; + +/* from rtlwifi/wifi.h */ +enum ratr_table_mode_new { + RATEID_IDX_BGN_40M_2SS = 0, + RATEID_IDX_BGN_40M_1SS = 1, + RATEID_IDX_BGN_20M_2SS_BN = 2, + RATEID_IDX_BGN_20M_1SS_BN = 3, + RATEID_IDX_GN_N2SS = 4, + RATEID_IDX_GN_N1SS = 5, + RATEID_IDX_BG = 6, + RATEID_IDX_G = 7, + RATEID_IDX_B = 8, + RATEID_IDX_VHT_2SS = 9, + RATEID_IDX_VHT_1SS = 10, + RATEID_IDX_MIX1 = 11, + RATEID_IDX_MIX2 = 12, + RATEID_IDX_VHT_3SS = 13, + RATEID_IDX_BGN_3SS = 14, +}; + +#define RTL8XXXU_RATR_STA_INIT 0 +#define RTL8XXXU_RATR_STA_HIGH 1 +#define RTL8XXXU_RATR_STA_MID 2 +#define RTL8XXXU_RATR_STA_LOW 3 + +struct rtl8xxxu_rate_adaptive { + u16 wireless_mode; + u8 ratr_index; + u8 rssi_level; /* INIT, HIGH, MIDDLE, LOW */ +} __packed; + struct rtl8xxxu_priv { struct ieee80211_hw *hw; struct usb_device *udev; @@ -1299,6 +1343,7 @@ struct rtl8xxxu_priv { u8 pi_enabled:1; u8 no_pape:1; u8 int_buf[USB_INTR_CONTENT_LENGTH]; + struct rtl8xxxu_rate_adaptive ra_info; }; struct rtl8xxxu_rx_urb { diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index 039e5ca9d2e4..360e9bd837e5 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -4345,7 +4345,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv, h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff; h2c.ramask.arg = 0x80; - h2c.b_macid_cfg.data1 = 0; + h2c.b_macid_cfg.data1 = priv->ra_info.ratr_index; if (sgi) h2c.b_macid_cfg.data1 |= BIT(7); @@ -4485,6 +4485,43 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg) rtl8xxxu_write8(priv, REG_INIRTS_RATE_SEL, rate_idx); } +static u16 +rtl8xxxu_wireless_mode(struct ieee80211_hw *hw, struct ieee80211_sta *sta) +{ + u16 network_type = WIRELESS_MODE_UNKNOWN; + u32 rate_mask; + + rate_mask = (sta->supp_rates[0] & 0xfff) | + (sta->ht_cap.mcs.rx_mask[0] << 12) | + (sta->ht_cap.mcs.rx_mask[0] << 20); + + if (hw->conf.chandef.chan->band == NL80211_BAND_5GHZ) + { + if (sta->vht_cap.vht_supported) + network_type = WIRELESS_MODE_AC; + else if (sta->ht_cap.ht_supported) + network_type = WIRELESS_MODE_N_5G; + + network_type |= WIRELESS_MODE_A; + } + else + { + if (sta->vht_cap.vht_supported) + network_type = WIRELESS_MODE_AC; + else if (sta->ht_cap.ht_supported) + network_type = WIRELESS_MODE_N_24G; + + if (sta->supp_rates[0] <= 0xf) + network_type |= WIRELESS_MODE_B; + else if (sta->supp_rates[0] & 0xf) + network_type |= (WIRELESS_MODE_B | WIRELESS_MODE_G); + else + network_type |= WIRELESS_MODE_G; + } + + return network_type; +} + static void rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif, struct ieee80211_bss_conf *bss_conf, u32 changed) @@ -4503,6 +4540,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif, if (bss_conf->assoc) { u32 ramask; int sgi = 0; + struct rtl8xxxu_rate_adaptive *ra; rcu_read_lock(); sta = ieee80211_find_sta(vif, bss_conf->bssid); @@ -4527,6 +4565,11 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif, sgi = 1; rcu_read_unlock(); + ra = &priv->ra_info; + ra->wireless_mode = rtl8xxxu_wireless_mode(hw, sta); + ra->ratr_index = RATEID_IDX_BGN_40M_2SS; + ra->rssi_level = RTL8XXXU_RATR_STA_INIT; + priv->fops->update_rate_mask(priv, ramask, sgi); rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);