| Message ID | c03390ce-34c2-42dd-9bd6-b231bb1f2fae@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Ping-Ke Shih |
| Headers | show |
| Series | [1/4] wifi: rtw88: Init RX burst length for 8822cu/8822bu/8821cu | expand |
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > Init RX burst length according to the USB speed. > > This is needed in order to make USB RX aggregation work. > > Tested with RTL8811CU. Having a throughput after this change would be better. > > Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > --- > I would mention in the commit message what BIT_DMA_BURST_CNT, > BIT_DMA_MODE, and BIT_DROP_DATA_EN are doing, but I don't know. That will be helpful to other developers. Please put them in second paragraph. [...] > +static void rtw8821cu_init_burst_pkt_len(struct rtw_dev *rtwdev) > +{ > + u8 rxdma, burst_size; > + > + rxdma = BIT_DMA_BURST_CNT | BIT_DMA_MODE; > + > + if (rtw_read8(rtwdev, REG_SYS_CFG2 + 3) == 0x20) > + burst_size = BIT_DMA_BURST_SIZE_1024; > + else if ((rtw_read8(rtwdev, REG_USB_USBSTAT) & 0x3) == 0x1) > + burst_size = BIT_DMA_BURST_SIZE_512; > + else > + burst_size = BIT_DMA_BURST_SIZE_64; > + > + u8p_replace_bits(&rxdma, burst_size, BIT_DMA_BURST_SIZE); > + > + rtw_write8(rtwdev, REG_RXDMA_MODE, rxdma); > + rtw_write16_set(rtwdev, REG_TXDMA_OFFSET_CHK, BIT_DROP_DATA_EN); > +} > + All use the same setup. Can we move it to usb.c? Maybe rtw_usb_interface_cfg() is a good place? (still exclude untested chips.)
On 30/07/2024 06:57, Ping-Ke Shih wrote: > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: >> Init RX burst length according to the USB speed. >> >> This is needed in order to make USB RX aggregation work. >> >> Tested with RTL8811CU. > > Having a throughput after this change would be better. > >> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> >> --- >> I would mention in the commit message what BIT_DMA_BURST_CNT, >> BIT_DMA_MODE, and BIT_DROP_DATA_EN are doing, but I don't know. > > That will be helpful to other developers. Please put them in second paragraph. > > [...] > >> +static void rtw8821cu_init_burst_pkt_len(struct rtw_dev *rtwdev) >> +{ >> + u8 rxdma, burst_size; >> + >> + rxdma = BIT_DMA_BURST_CNT | BIT_DMA_MODE; >> + >> + if (rtw_read8(rtwdev, REG_SYS_CFG2 + 3) == 0x20) >> + burst_size = BIT_DMA_BURST_SIZE_1024; >> + else if ((rtw_read8(rtwdev, REG_USB_USBSTAT) & 0x3) == 0x1) >> + burst_size = BIT_DMA_BURST_SIZE_512; >> + else >> + burst_size = BIT_DMA_BURST_SIZE_64; >> + >> + u8p_replace_bits(&rxdma, burst_size, BIT_DMA_BURST_SIZE); >> + >> + rtw_write8(rtwdev, REG_RXDMA_MODE, rxdma); >> + rtw_write16_set(rtwdev, REG_TXDMA_OFFSET_CHK, BIT_DROP_DATA_EN); >> +} >> + > > All use the same setup. > Can we move it to usb.c? Maybe rtw_usb_interface_cfg() is a good place? > (still exclude untested chips.) > rtw_usb_interface_cfg() is a good place. I will move it there. The other chips will complicate it a bit, but that's okay. RTL8723DU doesn't check for USB 3, of course: if (rtwusb->udev->speed == USB_SPEED_HIGH) burst_size = BIT_DMA_BURST_SIZE_512; else burst_size = BIT_DMA_BURST_SIZE_64; RTL8821AU/RTL8812AU: if (chip->id == RTW_CHIP_TYPE_8821A) speedvalue = BIT(7); else speedvalue = rtw_read8(rtwdev, REG_SYS_CFG2 + 3); if (speedvalue & BIT(7)) { /* USB 2/1.1 Mode */ temp = rtw_read8(rtwdev, 0xfe17); if (((temp >> 4) & 0x03) == 0) burst_size = BIT_DMA_BURST_SIZE_512; else burst_size = BIT_DMA_BURST_SIZE_64; } else { /* USB3 Mode */ burst_size = BIT_DMA_BURST_SIZE_1024; } RTL8814AU: speedvalue = rtw_read8(rtwdev, REG_SYS_CFG2 + 3); if (speedvalue & BIT(7)) { /* USB 2/1.1 Mode */ if (rtwusb->udev->speed == USB_SPEED_HIGH) burst_size = BIT_DMA_BURST_SIZE_512; else burst_size = BIT_DMA_BURST_SIZE_64; } else { /* USB3 Mode */ burst_size = BIT_DMA_BURST_SIZE_1024; } I don't understand why we can't just check rtwusb->udev->speed instead of reading various registers. Then they could all use the same code. (By the way, RTL8821AU/RTL8812AU is ready now. I will send the patches after this patch set is sorted out. There are about 16 smaller patches to prepare things, and then the new driver.)
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > On 30/07/2024 06:57, Ping-Ke Shih wrote: > > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > >> Init RX burst length according to the USB speed. > >> > >> This is needed in order to make USB RX aggregation work. > >> > >> Tested with RTL8811CU. > > > > Having a throughput after this change would be better. > > > >> > >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > >> --- > >> I would mention in the commit message what BIT_DMA_BURST_CNT, > >> BIT_DMA_MODE, and BIT_DROP_DATA_EN are doing, but I don't know. > > > > That will be helpful to other developers. Please put them in second paragraph. > > > > [...] > > > >> +static void rtw8821cu_init_burst_pkt_len(struct rtw_dev *rtwdev) > >> +{ > >> + u8 rxdma, burst_size; > >> + > >> + rxdma = BIT_DMA_BURST_CNT | BIT_DMA_MODE; > >> + > >> + if (rtw_read8(rtwdev, REG_SYS_CFG2 + 3) == 0x20) > >> + burst_size = BIT_DMA_BURST_SIZE_1024; > >> + else if ((rtw_read8(rtwdev, REG_USB_USBSTAT) & 0x3) == 0x1) > >> + burst_size = BIT_DMA_BURST_SIZE_512; > >> + else > >> + burst_size = BIT_DMA_BURST_SIZE_64; > >> + > >> + u8p_replace_bits(&rxdma, burst_size, BIT_DMA_BURST_SIZE); > >> + > >> + rtw_write8(rtwdev, REG_RXDMA_MODE, rxdma); > >> + rtw_write16_set(rtwdev, REG_TXDMA_OFFSET_CHK, BIT_DROP_DATA_EN); > >> +} > >> + > > > > All use the same setup. > > Can we move it to usb.c? Maybe rtw_usb_interface_cfg() is a good place? > > (still exclude untested chips.) > > > > rtw_usb_interface_cfg() is a good place. I will move it there. > The other chips will complicate it a bit, but that's okay. Maybe we can have init_burst_pkt_len_{v1, v2, v3, ...}, but... > > I don't understand why we can't just check rtwusb->udev->speed > instead of reading various registers. Then they could all use > the same code. It seems to be better to just use rtwusb->udev->speed. I ask USB team but no clear answer, but they prefer rtwusb->udev->speed as well. > > (By the way, RTL8821AU/RTL8812AU is ready now. I will send > the patches after this patch set is sorted out. There are about > 16 smaller patches to prepare things, and then the new driver.) Nice work!
diff --git a/drivers/net/wireless/realtek/rtw88/reg.h b/drivers/net/wireless/realtek/rtw88/reg.h index e7b24465f549..788e450d30d2 100644 --- a/drivers/net/wireless/realtek/rtw88/reg.h +++ b/drivers/net/wireless/realtek/rtw88/reg.h @@ -322,6 +322,12 @@ #define REG_RXDMA_DPR 0x028C #define REG_RXDMA_MODE 0x0290 #define BIT_DMA_MODE BIT(1) +#define BIT_DMA_BURST_CNT GENMASK(3, 2) +#define BIT_DMA_BURST_SIZE GENMASK(5, 4) +#define BIT_DMA_BURST_SIZE_64 2 +#define BIT_DMA_BURST_SIZE_512 1 +#define BIT_DMA_BURST_SIZE_1024 0 + #define REG_RXPKTNUM 0x02B0 #define REG_INT_MIG 0x0304 @@ -703,6 +709,8 @@ #define REG_IGN_GNTBT4 0x4160 +#define REG_USB_USBSTAT 0xFE11 + #define RF_MODE 0x00 #define RF_MODOPT 0x01 #define RF_WLINT 0x01 diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c index 526e8de77b3e..55b6fe874710 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c @@ -204,6 +204,25 @@ static void rtw8821c_phy_set_param(struct rtw_dev *rtwdev) rtw8821c_phy_bf_init(rtwdev); } +static void rtw8821cu_init_burst_pkt_len(struct rtw_dev *rtwdev) +{ + u8 rxdma, burst_size; + + rxdma = BIT_DMA_BURST_CNT | BIT_DMA_MODE; + + if (rtw_read8(rtwdev, REG_SYS_CFG2 + 3) == 0x20) + burst_size = BIT_DMA_BURST_SIZE_1024; + else if ((rtw_read8(rtwdev, REG_USB_USBSTAT) & 0x3) == 0x1) + burst_size = BIT_DMA_BURST_SIZE_512; + else + burst_size = BIT_DMA_BURST_SIZE_64; + + u8p_replace_bits(&rxdma, burst_size, BIT_DMA_BURST_SIZE); + + rtw_write8(rtwdev, REG_RXDMA_MODE, rxdma); + rtw_write16_set(rtwdev, REG_TXDMA_OFFSET_CHK, BIT_DROP_DATA_EN); +} + static int rtw8821c_mac_init(struct rtw_dev *rtwdev) { u32 value32; @@ -261,6 +280,9 @@ static int rtw8821c_mac_init(struct rtw_dev *rtwdev) rtw_write32(rtwdev, REG_WMAC_OPTION_FUNCTION + 8, WLAN_MAC_OPT_FUNC2); rtw_write8(rtwdev, REG_WMAC_OPTION_FUNCTION + 4, WLAN_MAC_OPT_NORM_FUNC1); + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB) + rtw8821cu_init_burst_pkt_len(rtwdev); + return 0; } diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822b.c b/drivers/net/wireless/realtek/rtw88/rtw8822b.c index 6edb17aea90e..0949eaa2b6c1 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8822b.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8822b.c @@ -237,6 +237,25 @@ static void rtw8822b_phy_set_param(struct rtw_dev *rtwdev) #define WLAN_NAV_CFG (WLAN_RDG_NAV | (WLAN_TXOP_NAV << 16)) #define WLAN_RX_TSF_CFG (WLAN_CCK_RX_TSF | (WLAN_OFDM_RX_TSF) << 8) +static void rtw8822bu_init_burst_pkt_len(struct rtw_dev *rtwdev) +{ + u8 rxdma, burst_size; + + rxdma = BIT_DMA_BURST_CNT | BIT_DMA_MODE; + + if (rtw_read8(rtwdev, REG_SYS_CFG2 + 3) == 0x20) + burst_size = BIT_DMA_BURST_SIZE_1024; + else if ((rtw_read8(rtwdev, REG_USB_USBSTAT) & 0x3) == 0x1) + burst_size = BIT_DMA_BURST_SIZE_512; + else + burst_size = BIT_DMA_BURST_SIZE_64; + + u8p_replace_bits(&rxdma, burst_size, BIT_DMA_BURST_SIZE); + + rtw_write8(rtwdev, REG_RXDMA_MODE, rxdma); + rtw_write16_set(rtwdev, REG_TXDMA_OFFSET_CHK, BIT_DROP_DATA_EN); +} + static int rtw8822b_mac_init(struct rtw_dev *rtwdev) { u32 value32; @@ -284,6 +303,9 @@ static int rtw8822b_mac_init(struct rtw_dev *rtwdev) rtw_write8_set(rtwdev, REG_SND_PTCL_CTRL, BIT_DIS_CHK_VHTSIGB_CRC); + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB) + rtw8822bu_init_burst_pkt_len(rtwdev); + return 0; } diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822c.c b/drivers/net/wireless/realtek/rtw88/rtw8822c.c index e265a35184ab..2a90a879196b 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8822c.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.c @@ -2001,6 +2001,25 @@ static void rtw8822c_phy_set_param(struct rtw_dev *rtwdev) #define MAC_CLK_SPEED 80 /* 80M */ #define EFUSE_PCB_INFO_OFFSET 0xCA +static void rtw8822cu_init_burst_pkt_len(struct rtw_dev *rtwdev) +{ + u8 rxdma, burst_size; + + rxdma = BIT_DMA_BURST_CNT | BIT_DMA_MODE; + + if (rtw_read8(rtwdev, REG_SYS_CFG2 + 3) == 0x20) + burst_size = BIT_DMA_BURST_SIZE_1024; + else if ((rtw_read8(rtwdev, REG_USB_USBSTAT) & 0x3) == 0x1) + burst_size = BIT_DMA_BURST_SIZE_512; + else + burst_size = BIT_DMA_BURST_SIZE_64; + + u8p_replace_bits(&rxdma, burst_size, BIT_DMA_BURST_SIZE); + + rtw_write8(rtwdev, REG_RXDMA_MODE, rxdma); + rtw_write16_set(rtwdev, REG_TXDMA_OFFSET_CHK, BIT_DROP_DATA_EN); +} + static int rtw8822c_mac_init(struct rtw_dev *rtwdev) { u8 value8; @@ -2127,6 +2146,9 @@ static int rtw8822c_mac_init(struct rtw_dev *rtwdev) /* Interrupt migration configuration */ rtw_write32(rtwdev, REG_INT_MIG, WLAN_MAC_INT_MIG_CFG); + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB) + rtw8822cu_init_burst_pkt_len(rtwdev); + return 0; }
Init RX burst length according to the USB speed. This is needed in order to make USB RX aggregation work. Tested with RTL8811CU. Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> --- I would mention in the commit message what BIT_DMA_BURST_CNT, BIT_DMA_MODE, and BIT_DROP_DATA_EN are doing, but I don't know. --- drivers/net/wireless/realtek/rtw88/reg.h | 8 +++++++ drivers/net/wireless/realtek/rtw88/rtw8821c.c | 22 +++++++++++++++++++ drivers/net/wireless/realtek/rtw88/rtw8822b.c | 22 +++++++++++++++++++ drivers/net/wireless/realtek/rtw88/rtw8822c.c | 22 +++++++++++++++++++ 4 files changed, 74 insertions(+)