RE: [PATCH] wifi: rtw88: usb: Further limit the TX aggregation

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

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> Currently the number of frames sent to the chip in a single USB Request
> Block is limited only by the size of the TX buffer, which is 20 KiB.
> Testing reveals that as many as 13 frames get aggregated. This is more
> than what any of the chips would like to receive. RTL8822CU, RTL8822BU,
> and RTL8821CU want at most 3 frames, and RTL8723DU wants only 1 frame
> per URB.
> 
> RTL8723DU in particular reliably malfunctions during a speed test. All
> traffic seems to stop. Pinging the AP no longer works.
> 
> Fix this problem by limiting the number of frames sent to the chip in a
> single URB according to what each chip likes.
> 
> Also configure RTL8822CU, RTL8822BU, and RTL8821CU to expect 3 frames
> per URB.
> 
> RTL8703B may or may not be found in USB devices. Declare that it wants
> only 1 frame per URB, just in case.
> 
> Tested with RTL8723DU and RTL8811CU.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
>  drivers/net/wireless/realtek/rtw88/mac.c      | 12 ++++++++++++
>  drivers/net/wireless/realtek/rtw88/main.h     |  2 ++
>  drivers/net/wireless/realtek/rtw88/reg.h      |  1 +
>  drivers/net/wireless/realtek/rtw88/rtw8703b.c |  1 +
>  drivers/net/wireless/realtek/rtw88/rtw8723d.c |  1 +
>  drivers/net/wireless/realtek/rtw88/rtw8821c.c |  1 +
>  drivers/net/wireless/realtek/rtw88/rtw8822b.c |  1 +
>  drivers/net/wireless/realtek/rtw88/rtw8822c.c |  1 +
>  drivers/net/wireless/realtek/rtw88/usb.c      |  4 +++-
>  9 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
> index 0dba8aae7716..fca90759c268 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac.c
> @@ -1186,6 +1186,7 @@ static int __priority_queue_cfg(struct rtw_dev *rtwdev,
>  {
>         const struct rtw_chip_info *chip = rtwdev->chip;
>         struct rtw_fifo_conf *fifo = &rtwdev->fifo;
> +       u8 val8;
> 
>         rtw_write16(rtwdev, REG_FIFOPAGE_INFO_1, pg_tbl->hq_num);
>         rtw_write16(rtwdev, REG_FIFOPAGE_INFO_2, pg_tbl->lq_num);
> @@ -1201,6 +1202,17 @@ static int __priority_queue_cfg(struct rtw_dev *rtwdev,
>         rtw_write16(rtwdev, REG_FIFOPAGE_CTRL_2 + 2, fifo->rsvd_boundary);
>         rtw_write16(rtwdev, REG_BCNQ1_BDNY_V1, fifo->rsvd_boundary);
>         rtw_write32(rtwdev, REG_RXFF_BNDY, chip->rxff_size - C2H_PKT_BUF - 1);
> +
> +       if (rtwdev->hci.type == RTW_HCI_TYPE_USB) {
> +               val8 = rtw_read8(rtwdev, REG_AUTO_LLT_V1);
> +               u8p_replace_bits(&val8, chip->usb_tx_agg_desc_num,
> +                                BIT_MASK_BLK_DESC_NUM);
> +               rtw_write8(rtwdev, REG_AUTO_LLT_V1, val8);

rtw_write8_mask(rtwdev, REG_AUTO_LLT_V1, BIT_MASK_BLK_DESC_NUM, chip->usb_tx_agg_desc_num);

> +
> +               rtw_write8(rtwdev, REG_AUTO_LLT_V1 + 3, chip->usb_tx_agg_desc_num);
> +               rtw_write8_set(rtwdev, REG_TXDMA_OFFSET_CHK + 1, BIT(1));
> +       }
> +
>         rtw_write8_set(rtwdev, REG_AUTO_LLT_V1, BIT_AUTO_INIT_LLT_V1);
> 
>         if (!check_hw_ready(rtwdev, REG_AUTO_LLT_V1, BIT_AUTO_INIT_LLT_V1, 0))
> diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
> index 49894331f7b4..49a3fd4fb7dc 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.h
> +++ b/drivers/net/wireless/realtek/rtw88/main.h
> @@ -1197,6 +1197,8 @@ struct rtw_chip_info {
>         u16 fw_fifo_addr[RTW_FW_FIFO_MAX];
>         const struct rtw_fwcd_segs *fwcd_segs;
> 
> +       u8 usb_tx_agg_desc_num;
> +

Please keep order of field and instance declaration, like rtw8703b_hw_spec.

>         u8 default_1ss_tx_path;
> 
>         bool path_div_supported;
> diff --git a/drivers/net/wireless/realtek/rtw88/reg.h b/drivers/net/wireless/realtek/rtw88/reg.h
> index b122f226924b..572651b3aa4b 100644
> --- a/drivers/net/wireless/realtek/rtw88/reg.h
> +++ b/drivers/net/wireless/realtek/rtw88/reg.h
> @@ -270,6 +270,7 @@
>  #define BIT_MASK_BCN_HEAD_1_V1 0xfff
>  #define REG_AUTO_LLT_V1                0x0208
>  #define BIT_AUTO_INIT_LLT_V1   BIT(0)
> +#define BIT_MASK_BLK_DESC_NUM  0xf0

0xf0 --> GENMASK(7, 4)

>  #define REG_DWBCN0_CTRL                0x0208
>  #define BIT_BCN_VALID          BIT(16)
>  #define REG_TXDMA_OFFSET_CHK   0x020C
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8703b.c
> b/drivers/net/wireless/realtek/rtw88/rtw8703b.c
> index 8919f9e11f03..222608de33cd 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw8703b.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.c
> @@ -2013,6 +2013,7 @@ const struct rtw_chip_info rtw8703b_hw_spec = {
>         .tx_stbc = false,
>         .max_power_index = 0x3f,
>         .ampdu_density = IEEE80211_HT_MPDU_DENSITY_16,
> +       .usb_tx_agg_desc_num = 1, /* Not sure if this chip has USB interface */

The position to declare this field is very different from others. 

Not sure when we messed up the order, but please don't feel free to add one,
keep the order. 

If you have time, could you help to align all of them across chips?
If not, I will take my time to do that after your patch.