Re: [PATCH rtw-next 3/4] wifi: rtw88: Set AMPDU factor to hardware

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 19/03/2025 02:28, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> On 18/03/2025 04:06, Ping-Ke Shih wrote:
>>> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>>>> On 17/03/2025 05:01, Ping-Ke Shih wrote:
>>>>> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>>>>>>
>>>>>> Tell the chip the maximum AMPDU size supported by the AP. This greatly
>>>>>> improves the TX speed of RTL8814AU in the 2.4 GHz band. Before: ~90
>>>>>> Mbps. After: ~300 Mbps.
>>>>>>
>>>>>> Add this configuration for all the chips, even if it only has an effect
>>>>>> on RTL8814AU in my tests. Surely they all need this.
>>>>>
>>>>> The hardware default value of REG_AMPDU_MAX_LENGTH is 0xffff (unlimited)
>>>>> for most chips. It seems like RTL8812A/RTL8821A are also exceptions, so
>>>>> at power on function they do
>>>>>     rtw_write32(rtwdev, REG_AMPDU_MAX_LENGTH, 0xffffffff);
>>>>>
>>>>> I feel RTL8814A has similar setting, so maybe you can just add similar
>>>>> stuff.
>>>>>
>>>>> By the way, the AMPDU is controlled by TX descriptor basically:
>>>>>       pkt_info->ampdu_factor = ampdu_factor;
>>>>>       pkt_info->ampdu_density = ampdu_density;
>>>>>       pkt_info->ampdu_en = ampdu_en;
>>>>>
>>>>> Since you didn't change this part at all, I still feel setting
>>>>> REG_AMPDU_MAX_LENGTH to 0xffffffff can fix low throughput problem.
>>>>>
>>>>
>>>> I tried 0xffffffff just now and it doesn't work. It's the same with
>>>> both of my routers. They advertise a maximum AMPDU size of 64 K.
>>>> I can't just set it to 0xffff either, because then the upload speed
>>>> in the 5 GHz band suffers a lot. The dual band router advertises a
>>>> maximum AMPDU size of 256 K in the 5 GHz band so it gets a value of
>>>> 0x3ffff.
>>>
>>> Not sure if 0xffffffff is a special value. Since this is a limit of
>>> AMPDU length, you can set a constant large value such as 0x3ffff you
>>> have tested. Is there special case it can't handle?
>>>
>>>
>>
>> 0x3ffff is not good for the 2.4 GHz band. The upload is only ~90 Mbps
>> with both of the routers I tested. Same with 0x1ffff. Only 0xffff
>> works well for them.
> 
> Have you checked the packets in the air? How about their difference?
> Intuitively larger REG_AMPDU_MAX_LENGTH would be better.
> 

I checked today. With 0xffffffff I see 29% retransmission rate. With
0xffff the retransmission rate is only 8.6%.

>>
>> 0xffff is too little for the 5 GHz band. The upload speed is ~200 Mbps
>> less than with 0x3ffff.
>>
>> I guess if you really don't want this patch I can hardcode 0xffff and
>> 0x3ffff in rtw8814a_switch_band(). I just don't know if all access
>> points will be happy with that.
> 
> Initially I wanted to simply this patch, because changing REG_AMPDU_MAX_LENGTH
> for other chips without testing is risky. With your tests, the behavior of
> REG_AMPDU_MAX_LENGTH works not fully expected, so I suspect the risk
> is even higher. 
> 
> Therefore, I would like limit this change to RTL8814A. Though hardcode proposal
> is not sure workable for all AP, we also don't know this patch works for all
> AP. Anyway this proposal is fine to me if we don't have other ideas.
> 
> 
All right, I will limit this only to RTL8814A.