mbox series

[0/4] wifi: rtl8xxxu: A few improvements

Message ID 56712d04-1505-2cbb-b6ac-3af4e73de108@gmail.com
Headers show
Series wifi: rtl8xxxu: A few improvements | expand

Message

Bitterblue Smith Sept. 18, 2022, 12:26 p.m. UTC
I found these problems in my quest to make rtl8xxxu as fast as the
rtl8188fu driver. With these changes at least the download speed is
almost as good. The upload speed gets better but it's not there yet,
possibly because there is no MSDU aggregation.

Bitterblue Smith (4):
  wifi: rtl8xxxu: gen2: Turn on the rate control
  wifi: rtl8xxxu: gen2: Enable 40 MHz channel width
  wifi: rtl8xxxu: Fix AIFS written to REG_EDCA_*_PARAM
  wifi: rtl8xxxu: Improve rtl8xxxu_queue_select

 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  6 +-
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 92 ++++++++++++++++---
 2 files changed, 82 insertions(+), 16 deletions(-)

Comments

Jes Sorensen Sept. 25, 2022, 4:53 p.m. UTC | #1
On 9/18/22 08:35, Bitterblue Smith wrote:
> Inform the firmware when connecting to a network. This makes the
> firmware enable the rate control, which makes the upload faster.
> 
> Don't inform the firmware when disconnecting from a network, because
> that makes reconnecting impossible for some reason:

Have you dug through the vendor driver to see what it does here?

Thanks,
Jes


> wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 1/3)
> wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 2/3)
> wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 3/3)
> wlp0s20f0u3: authentication with 90:55:de:__:__:__ timed out
> 
> Not informing the firmware about disconnecting doesn't seem to have
> any downside.
> 
> Tested only with RTL8188FU. Someone should test the other gen2 chips:
> RTL8723BU and RTL8192EU.
> 
> Fixes: c59f13bbead4 ("rtl8xxxu: Work around issue with 8192eu and 8723bu devices not reconn…")
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c   | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 32ab6ed5b9b6..7978d5dcc826 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4471,25 +4471,34 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
>  void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
>  				  u8 macid, bool connect)
>  {
> -#ifdef RTL8XXXU_GEN2_REPORT_CONNECT
>  	/*
>  	 * Barry Day reports this causes issues with 8192eu and 8723bu
>  	 * devices reconnecting. The reason for this is unclear, but
>  	 * until it is better understood, leave the code in place but
>  	 * disabled, so it is not lost.
>  	 */
> +	/*
> +	 * It also affects 8188FU. However, without this the rate control
> +	 * is not on. Probably it only enables the rate control when it
> +	 * knows it's connected to a network.
> +	 *
> +	 * Hack: don't report the disconnect. This way the rate control
> +	 * is on and reconnecting also works. TODO Test 8192EU and 8723BU.
> +	 */
>  	struct h2c_cmd h2c;
>  
>  	memset(&h2c, 0, sizeof(struct h2c_cmd));
>  
>  	h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT;
> -	if (connect)
> +	if (connect) {
>  		h2c.media_status_rpt.parm |= BIT(0);
> +	/*
>  	else
>  		h2c.media_status_rpt.parm &= ~BIT(0);
> +	*/
>  
> -	rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
> -#endif
> +		rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
> +	}
>  }
>  
>  void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
Jes Sorensen Sept. 25, 2022, 4:56 p.m. UTC | #2
On 9/18/22 08:42, Bitterblue Smith wrote:
> ieee80211_tx_queue_params.aifs is not supposed to be written directly
> to the REG_EDCA_*_PARAM registers. Instead process it like the vendor
> drivers do. It's kinda hacky but it works.
> 
> This change boosts the download speed and makes it more stable.
> 
> Tested with RTL8188FU but all the other supported chips should also
> benefit.
> 
> Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>

Acked-by: Jes Sorensen <jes@trained-monkey.org>

Jes
Kalle Valo Sept. 26, 2022, 9:24 a.m. UTC | #3
Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:

> ieee80211_tx_queue_params.aifs is not supposed to be written directly
> to the REG_EDCA_*_PARAM registers. Instead process it like the vendor
> drivers do. It's kinda hacky but it works.
>
> This change boosts the download speed and makes it more stable.
>
> Tested with RTL8188FU but all the other supported chips should also
> benefit.
>
> Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>

[...]

> +static void rtl8xxxu_set_aifs(struct rtl8xxxu_priv *priv, u8 slot_time)
> +{
> +	u32 reg_edca_param[IEEE80211_NUM_ACS] = {
> +		[IEEE80211_AC_VO] = REG_EDCA_VO_PARAM,
> +		[IEEE80211_AC_VI] = REG_EDCA_VI_PARAM,
> +		[IEEE80211_AC_BE] = REG_EDCA_BE_PARAM,
> +		[IEEE80211_AC_BK] = REG_EDCA_BK_PARAM,
> +	};

static const?
Bitterblue Smith Sept. 27, 2022, 7:49 p.m. UTC | #4
On 25/09/2022 19:53, Jes Sorensen wrote:
> On 9/18/22 08:35, Bitterblue Smith wrote:
>> Inform the firmware when connecting to a network. This makes the
>> firmware enable the rate control, which makes the upload faster.
>>
>> Don't inform the firmware when disconnecting from a network, because
>> that makes reconnecting impossible for some reason:
> 
> Have you dug through the vendor driver to see what it does here?
> 
> Thanks,
> Jes
> 

I hadn't investigated, but since you asked :) I looked into it today.

The vendor driver doesn't do anything weird. Our report_connect
function *should* work.

And it turns out it does work! I restored the original form of the
function to test something and reconnecting worked. I couldn't
reproduce the problem anymore. Not much has changed in rtl8xxxu since
the last time I tried this, so it was easy to find the reason: fixing
the queue selection [0] fixed the reconnecting problem. Before, it was
sending the auth attempts using queue 0x7 (TXDESC_QUEUE_VO). With the
queue selection fix it uses queue 0x12 (TXDESC_QUEUE_MGNT). Perhaps
queue 0x7 is not functional when the firmware knows it's not connected
to a network?

I guess I have to send a different patch for this now.

[0] https://lore.kernel.org/linux-wireless/7fa4819a-4f20-b2af-b7a6-8ee01ac49295@gmail.com/
Kalle Valo Sept. 28, 2022, 8:39 a.m. UTC | #5
Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:

> On 25/09/2022 19:53, Jes Sorensen wrote:
>> On 9/18/22 08:35, Bitterblue Smith wrote:
>>> Inform the firmware when connecting to a network. This makes the
>>> firmware enable the rate control, which makes the upload faster.
>>>
>>> Don't inform the firmware when disconnecting from a network, because
>>> that makes reconnecting impossible for some reason:
>> 
>> Have you dug through the vendor driver to see what it does here?
>> 
>> Thanks,
>> Jes
>> 
>
> I hadn't investigated, but since you asked :) I looked into it today.
>
> The vendor driver doesn't do anything weird. Our report_connect
> function *should* work.
>
> And it turns out it does work! I restored the original form of the
> function to test something and reconnecting worked. I couldn't
> reproduce the problem anymore. Not much has changed in rtl8xxxu since
> the last time I tried this, so it was easy to find the reason: fixing
> the queue selection [0] fixed the reconnecting problem. Before, it was
> sending the auth attempts using queue 0x7 (TXDESC_QUEUE_VO). With the
> queue selection fix it uses queue 0x12 (TXDESC_QUEUE_MGNT). Perhaps
> queue 0x7 is not functional when the firmware knows it's not connected
> to a network?
>
> I guess I have to send a different patch for this now.

So what should I do with this patchset? Can I take patches 2-4?
Bitterblue Smith Sept. 28, 2022, 9:52 a.m. UTC | #6
On 28/09/2022 11:39, Kalle Valo wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:
> 
>> On 25/09/2022 19:53, Jes Sorensen wrote:
>>> On 9/18/22 08:35, Bitterblue Smith wrote:
>>>> Inform the firmware when connecting to a network. This makes the
>>>> firmware enable the rate control, which makes the upload faster.
>>>>
>>>> Don't inform the firmware when disconnecting from a network, because
>>>> that makes reconnecting impossible for some reason:
>>>
>>> Have you dug through the vendor driver to see what it does here?
>>>
>>> Thanks,
>>> Jes
>>>
>>
>> I hadn't investigated, but since you asked :) I looked into it today.
>>
>> The vendor driver doesn't do anything weird. Our report_connect
>> function *should* work.
>>
>> And it turns out it does work! I restored the original form of the
>> function to test something and reconnecting worked. I couldn't
>> reproduce the problem anymore. Not much has changed in rtl8xxxu since
>> the last time I tried this, so it was easy to find the reason: fixing
>> the queue selection [0] fixed the reconnecting problem. Before, it was
>> sending the auth attempts using queue 0x7 (TXDESC_QUEUE_VO). With the
>> queue selection fix it uses queue 0x12 (TXDESC_QUEUE_MGNT). Perhaps
>> queue 0x7 is not functional when the firmware knows it's not connected
>> to a network?
>>
>> I guess I have to send a different patch for this now.
> 
> So what should I do with this patchset? Can I take patches 2-4?
> 

Yes, please. They're all independent.
Kalle Valo Sept. 28, 2022, 11:07 a.m. UTC | #7
Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:

> On 28/09/2022 11:39, Kalle Valo wrote:
>
>> Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:
>> 
>>> I guess I have to send a different patch for this now.
>> 
>> So what should I do with this patchset? Can I take patches 2-4?
>
> Yes, please. They're all independent.

Very good, so I'll then take patches 2-4. I'll drop patch 1 and wait for
the next version.