Re: [PATCH 2/2] wifi: rtw88: usb: Enable RX aggregation for 8821au/8812au

[Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>]

On 08/11/2024 04:40, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>>>
>> USB RX aggregation improves the RX speed on certain ARM systems, like
>> the NanoPi NEO Core2. With RTL8811AU, before: 30 Mbps, after: 224 Mbps.
>>
>> The out-of-tree driver uses aggregation size of 7 in USB 3 mode, but
>> that doesn't work here. rtw88 advertises support for receiving AMSDU
>> in AMPDU, so the AP sends larger frames, up to ~5100 bytes. With a size
>> of 7 RTL8812AU frequently tries to aggregate more frames than will fit
>> in 32768 bytes. Use a size of 6 instead.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
>> ---
>>  drivers/net/wireless/realtek/rtw88/usb.c | 30 ++++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
>> index 752bca05b9af..9172af63500b 100644
>> --- a/drivers/net/wireless/realtek/rtw88/usb.c
>> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
>> @@ -790,6 +790,32 @@ static void rtw_usb_dynamic_rx_agg_v1(struct rtw_dev *rtwdev, bool enable)
>>         rtw_write16(rtwdev, REG_RXDMA_AGG_PG_TH, val16);
>>  }
>>
>> +static void rtw_usb_dynamic_rx_agg_v2(struct rtw_dev *rtwdev, bool enable)
>> +{
>> +       struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
>> +       u8 size, timeout;
>> +       u16 val16;
>> +
> 
> How about a shortcut?
> 
> if (!enable) {
> 	size = 0x0;
> 	timeout = 0x1;
> 
> 	goto rx_agg;
> }
> 
>> +       if (rtwusb->udev->speed == USB_SPEED_SUPER) {
>> +               size = 0x6;
>> +               timeout = 0x1a;
>> +       } else {
>> +               size = 0x5;
>> +               timeout = 0x20;
>> +       }
>> +
>> +       if (!enable) {
>> +               size = 0x0;
>> +               timeout = 0x1;
>> +       }
>> +
> 
> rx_agg:
> 
>> +       val16 = u16_encode_bits(size, BIT_RXDMA_AGG_PG_TH) |
>> +               u16_encode_bits(timeout, BIT_DMA_AGG_TO_V1);
>> +
>> +       rtw_write16(rtwdev, REG_RXDMA_AGG_PG_TH, val16);
>> +       rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN);
>> +}

Hmm, I don't like it. What about this?

	if (!enable) {
		size = 0x0;
		timeout = 0x1;
	} else if (rtwusb->udev->speed == USB_SPEED_SUPER) {
		size = 0x6;
		timeout = 0x1a;
	} else {
		size = 0x5;
		timeout = 0x20;
	}