[PATCH rtw-next] wifi: rtw89: usb: shorten the RX aggregation hold on RTL8922AU

[PATCH rtw-next] wifi: rtw89: usb: shorten the RX aggregation hold on RTL8922AU

[Devin Wittmayer]

The RTL8922AU holds received frames for up to a millisecond before
handing them to the host, a value carried over from the RTL8852CU.
That adds up to a millisecond of round trip, and under load at 160 MHz
the access point drops to lower rates and download suffers.

Shorten the hold on this chip. Upload is unchanged, and so is download
on a USB 2 port, where the bus is the limit. The cost is some extra CPU
at the same load.

RTL8922AU on USB 3 at 160 MHz, power save off for the round trip:

                         x86, 6 GHz    x86, 5 GHz    Pi 5, 6 GHz
                        before after  before after  before after
  TCP down, 1 flow        613  1002     579   948     586   999  Mbps
  TCP down, 8 flows       656  1187     629  1080     605  1108  Mbps
  round trip             2.33  1.29    2.25  1.18    1.44  0.88  ms
  busiest core, UDP 200M  8.4  12.8     8.3  12.9     2.3   2.8  %

Fixes: 32cee8a9bb66 ("wifi: rtw89: usb: Enable RX aggregation for RTL8922AU")
Reported-by: Nick Morrow <morrownr@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Devin Wittmayer <lucid_duck@justthetip.ca>
---
Ping-Ke, Bitterblue pointed me to you, since the 32 came from Realtek.
Realtek's 8852CU driver uses it as the default and also has a
small-packet mode that sets the hold to 1, the value this patch uses.
If the longer default is there for a reason these tests would miss,
please let me know, thank you.

 drivers/net/wireless/realtek/rtw89/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c
index dbad42c17761..3dc4c97bba50 100644
--- a/drivers/net/wireless/realtek/rtw89/usb.c
+++ b/drivers/net/wireless/realtek/rtw89/usb.c
@@ -821,7 +821,7 @@ static void rtw89_usb_rx_agg_cfg_v3(struct rtw89_dev *rtwdev)
 {
 	const u32 rxagg_0 = FIELD_PREP_CONST(B_BE_RXAGG_0_EN, 1) |
 			    FIELD_PREP_CONST(B_BE_RXAGG_0_NUM_TH, 255) |
-			    FIELD_PREP_CONST(B_BE_RXAGG_0_TIME_32US_TH, 32) |
+			    FIELD_PREP_CONST(B_BE_RXAGG_0_TIME_32US_TH, 1) |
 			    FIELD_PREP_CONST(B_BE_RXAGG_0_BUF_SZ_1K, 20);
 
 	rtw89_write32(rtwdev, R_BE_RXAGG_0_V1, rxagg_0);
-- 
2.55.0

RE: [PATCH rtw-next] wifi: rtw89: usb: shorten the RX aggregation hold on RTL8922AU

[Ping-Ke Shih]

Devin Wittmayer <lucid_duck@justthetip.ca> wrote:
> The RTL8922AU holds received frames for up to a millisecond before
> handing them to the host, a value carried over from the RTL8852CU.
> That adds up to a millisecond of round trip, and under load at 160 MHz
> the access point drops to lower rates and download suffers.
> 
> Shorten the hold on this chip. Upload is unchanged, and so is download
> on a USB 2 port, where the bus is the limit. 

Can I know the number of USB 2 port? Just curious, not blocking item.

> The cost is some extra CPU at the same load.

I think the major reason people want RX aggregation is that a chip is
operating on a low-end CPU. 

> 
> RTL8922AU on USB 3 at 160 MHz, power save off for the round trip:
> 
>                          x86, 6 GHz    x86, 5 GHz    Pi 5, 6 GHz
>                         before after  before after  before after
>   TCP down, 1 flow        613  1002     579   948     586   999  Mbps
>   TCP down, 8 flows       656  1187     629  1080     605  1108  Mbps
>   round trip             2.33  1.29    2.25  1.18    1.44  0.88  ms
>   busiest core, UDP 200M  8.4  12.8     8.3  12.9     2.3   2.8  %
> 
> Fixes: 32cee8a9bb66 ("wifi: rtw89: usb: Enable RX aggregation for RTL8922AU")
> Reported-by: Nick Morrow <morrownr@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Devin Wittmayer <lucid_duck@justthetip.ca>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

> ---
> Ping-Ke, Bitterblue pointed me to you, since the 32 came from Realtek.
> Realtek's 8852CU driver uses it as the default and also has a
> small-packet mode that sets the hold to 1, the value this patch uses.
> If the longer default is there for a reason these tests would miss,
> please let me know, thank you.

I can't find small-packet mode you mentioned, please share a link. 

As I know the reason of longer default can benefit low-end CPU. 
By the way, the TX aggregation is pending, because our experiments
show no gain after the patch, so I don't send the patch. That means
current platform is much better than what years ago.

Ping-Ke