Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7bcb7489-aec1-e20a-44d3-3ba303e5d86f@gmail.com>
Date:   Tue, 3 Jan 2023 22:12:04 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.3
Subject: Re: [PATCH 2/2] wifi: rtl8xxxu: Use a longer retry limit of 48
To:     Ping-Ke Shih <pkshih@realtek.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc:     Jes Sorensen <Jes.Sorensen@gmail.com>
References: <6fcaaead-876c-68d1-a049-f1e7f7ff81e7@gmail.com>
 <55813ec6-b99f-dd25-a1e2-7af9e3be3117@gmail.com>
 <20da2e492e1e4c6ab00ddc53e7e0a79c@realtek.com>
Content-Language: en-US
From:   Bitterblue Smith <rtl8821cerfe2@gmail.com>
In-Reply-To: <20da2e492e1e4c6ab00ddc53e7e0a79c@realtek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/01/2023 10:13, Ping-Ke Shih wrote:
> 
>> -----Original Message-----
>> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> Sent: Saturday, December 31, 2022 12:54 AM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
>> Subject: [PATCH 2/2] wifi: rtl8xxxu: Use a longer retry limit of 48
>>
>> The Realtek rate control algorithm goes back and forth a lot between
>> the highest and the lowest rate it's allowed to use. This is due to
>> a lot of frames being dropped because the retry limits set by
>> IEEE80211_CONF_CHANGE_RETRY_LIMITS are too low. (Experimentally, they
>> are 4 for long frames and 7 for short frames.)
>>
>> The vendor drivers hardcode the value 48 for both retry limits (for
>> station mode), which makes dropped frames very rare and thus the rate
>> control is more stable.
> 
> I have similar feeling with Realtek 802.11ac chips as well, but I didn't
> dig further years ago. Maybe, it could have TX EVM problem.
> 
>>
>> Because most Realtek chips handle the rate control in the firmware,
>> which can't be modified, ignore the limits set by
>> IEEE80211_CONF_CHANGE_RETRY_LIMITS and use the value 48 (set during
>> chip initialisation), same as the vendor drivers.
>>
>> Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
> 
> No sure if "Fixes" is suitable to this patch, because original looks well but
> bad performance in real.
> 

Mostly I care about the Fixes tag because it gets the patch into the
stable kernels, but I can use the Cc tag for that instead.

>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> 
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> 
>> ---
>>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 ---------
>>  1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index f5fd3c448587..9bca5e837583 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -6184,7 +6184,6 @@ static int rtl8xxxu_config(struct ieee80211_hw *hw, u32 changed)
>>  {
>>  	struct rtl8xxxu_priv *priv = hw->priv;
>>  	struct device *dev = &priv->udev->dev;
>> -	u16 val16;
>>  	int ret = 0, channel;
>>  	bool ht40;
>>
>> @@ -6194,14 +6193,6 @@ static int rtl8xxxu_config(struct ieee80211_hw *hw, u32 changed)
>>  			 __func__, hw->conf.chandef.chan->hw_value,
>>  			 changed, hw->conf.chandef.width);
>>
>> -	if (changed & IEEE80211_CONF_CHANGE_RETRY_LIMITS) {
>> -		val16 = ((hw->conf.long_frame_max_tx_count <<
>> -			  RETRY_LIMIT_LONG_SHIFT) & RETRY_LIMIT_LONG_MASK) |
>> -			((hw->conf.short_frame_max_tx_count <<
>> -			  RETRY_LIMIT_SHORT_SHIFT) & RETRY_LIMIT_SHORT_MASK);
>> -		rtl8xxxu_write16(priv, REG_RETRY_LIMIT, val16);
>> -	}
>> -
>>  	if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
>>  		switch (hw->conf.chandef.width) {
>>  		case NL80211_CHAN_WIDTH_20_NOHT:
>> --
>> 2.38.0
>>
>> ------Please consider the environment before printing this e-mail.