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

[Ping-Ke Shih <pkshih@realtek.com>]

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.

> 
> 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.