| Message ID | 323190ee-5b88-4d37-bad0-b721cdfead1a@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: > > Enable USB RX aggregation when there is at least 1 Mbps RX or TX > traffic, otherwise disable it. > > USB RX aggregation improves the RX speed on certain ARM systems, like > the NanoPi NEO Core2. With RTL8811CU, before: 28 Mbps, after: 231 Mbps. > > The official drivers for these chips use the same logic for SDIO, but > for some reason rtw88_sdio always enables RX aggregation, so this patch > only toggles aggregation for USB devices. > > RTL8703B is likely not found in USB devices, and RTL8723DU doesn't like > aggregation. Please explicitly set .rx_aggregation = NULL to these two chips, so we know these two chips don't have this feature. > > Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > --- > drivers/net/wireless/realtek/rtw88/main.c | 18 +++++++++++---- > drivers/net/wireless/realtek/rtw88/main.h | 1 + > drivers/net/wireless/realtek/rtw88/rtw8821c.c | 23 +++++++++++++++++++ > drivers/net/wireless/realtek/rtw88/rtw8822b.c | 23 +++++++++++++++++++ > drivers/net/wireless/realtek/rtw88/rtw8822c.c | 23 +++++++++++++++++++ > 5 files changed, 84 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c > index 9d9d33a4a503..b3a089b4f707 100644 > --- a/drivers/net/wireless/realtek/rtw88/main.c > +++ b/drivers/net/wireless/realtek/rtw88/main.c > @@ -210,8 +210,10 @@ static void rtw_watch_dog_work(struct work_struct *work) > struct rtw_dev *rtwdev = container_of(work, struct rtw_dev, > watch_dog_work.work); > struct rtw_traffic_stats *stats = &rtwdev->stats; > + const struct rtw_chip_info *chip = rtwdev->chip; > struct rtw_watch_dog_iter_data data = {}; > bool busy_traffic = test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags); > + u32 tx_unicast_shift, rx_unicast_shift; > bool ps_active; > > mutex_lock(&rtwdev->mutex); > @@ -236,13 +238,21 @@ static void rtw_watch_dog_work(struct work_struct *work) > else > ps_active = false; > > - ewma_tp_add(&stats->tx_ewma_tp, > - (u32)(stats->tx_unicast >> RTW_TP_SHIFT)); > - ewma_tp_add(&stats->rx_ewma_tp, > - (u32)(stats->rx_unicast >> RTW_TP_SHIFT)); > + tx_unicast_shift = stats->tx_unicast >> RTW_TP_SHIFT; > + rx_unicast_shift = stats->rx_unicast >> RTW_TP_SHIFT; {tx,rx}_unicast_mbps because 'shift' is to get Mbps. > + > + ewma_tp_add(&stats->tx_ewma_tp, tx_unicast_shift); > + ewma_tp_add(&stats->rx_ewma_tp, rx_unicast_shift); > stats->tx_throughput = ewma_tp_read(&stats->tx_ewma_tp); > stats->rx_throughput = ewma_tp_read(&stats->rx_ewma_tp); > > + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB && chip->ops->rx_aggregation) { > + if (tx_unicast_shift < 1 && rx_unicast_shift < 1) > + chip->ops->rx_aggregation(rtwdev, false); > + else > + chip->ops->rx_aggregation(rtwdev, true); > + } Move this chunk to a function with arguments {tx,rx}_unicast_mbps. The function name might be something like rtw_dynamic_usb_rx_aggregation(). > + > /* reset tx/rx statictics */ > stats->tx_unicast = 0; > stats->rx_unicast = 0; > diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h > index 9d21637cf5d5..65bedd1668cc 100644 > --- a/drivers/net/wireless/realtek/rtw88/main.h > +++ b/drivers/net/wireless/realtek/rtw88/main.h > @@ -888,6 +888,7 @@ struct rtw_chip_ops { > void (*fill_txdesc_checksum)(struct rtw_dev *rtwdev, > struct rtw_tx_pkt_info *pkt_info, > u8 *txdesc); > + void (*rx_aggregation)(struct rtw_dev *rtwdev, bool enable); > > /* for coex */ > void (*coex_set_init)(struct rtw_dev *rtwdev); > diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c > b/drivers/net/wireless/realtek/rtw88/rtw8821c.c > index 55b6fe874710..3efdb41f22c5 100644 > --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c > +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c > @@ -1276,6 +1276,28 @@ static void rtw8821c_fill_txdesc_checksum(struct rtw_dev *rtwdev, > fill_txdesc_checksum_common(txdesc, 16); > } > > +static void rtw8821c_rx_aggregation(struct rtw_dev *rtwdev, bool enable) > +{ > + u8 size, timeout; > + u16 val16; > + > + rtw_write32_set(rtwdev, REG_RXDMA_AGG_PG_TH, BIT_EN_PRE_CALC); > + rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN); > + rtw_write8_clr(rtwdev, REG_RXDMA_AGG_PG_TH + 3, BIT(7)); > + > + if (enable) { > + size = 0x5; > + timeout = 0x20; > + } else { > + size = 0x0; > + timeout = 0x1; > + } > + 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); > +} > + All use the same settings. Move this to rtw_usb_rx_aggregation() called by rtw_dynamic_usb_rx_aggregation().
On 30/07/2024 08:47, Ping-Ke Shih wrote: > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: >> >> Enable USB RX aggregation when there is at least 1 Mbps RX or TX >> traffic, otherwise disable it. >> >> USB RX aggregation improves the RX speed on certain ARM systems, like >> the NanoPi NEO Core2. With RTL8811CU, before: 28 Mbps, after: 231 Mbps. >> >> The official drivers for these chips use the same logic for SDIO, but >> for some reason rtw88_sdio always enables RX aggregation, so this patch >> only toggles aggregation for USB devices. >> >> RTL8703B is likely not found in USB devices, and RTL8723DU doesn't like >> aggregation. > > Please explicitly set .rx_aggregation = NULL to these two chips, so > we know these two chips don't have this feature. > >> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> >> --- >> drivers/net/wireless/realtek/rtw88/main.c | 18 +++++++++++---- >> drivers/net/wireless/realtek/rtw88/main.h | 1 + >> drivers/net/wireless/realtek/rtw88/rtw8821c.c | 23 +++++++++++++++++++ >> drivers/net/wireless/realtek/rtw88/rtw8822b.c | 23 +++++++++++++++++++ >> drivers/net/wireless/realtek/rtw88/rtw8822c.c | 23 +++++++++++++++++++ >> 5 files changed, 84 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c >> index 9d9d33a4a503..b3a089b4f707 100644 >> --- a/drivers/net/wireless/realtek/rtw88/main.c >> +++ b/drivers/net/wireless/realtek/rtw88/main.c >> @@ -210,8 +210,10 @@ static void rtw_watch_dog_work(struct work_struct *work) >> struct rtw_dev *rtwdev = container_of(work, struct rtw_dev, >> watch_dog_work.work); >> struct rtw_traffic_stats *stats = &rtwdev->stats; >> + const struct rtw_chip_info *chip = rtwdev->chip; >> struct rtw_watch_dog_iter_data data = {}; >> bool busy_traffic = test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags); >> + u32 tx_unicast_shift, rx_unicast_shift; >> bool ps_active; >> >> mutex_lock(&rtwdev->mutex); >> @@ -236,13 +238,21 @@ static void rtw_watch_dog_work(struct work_struct *work) >> else >> ps_active = false; >> >> - ewma_tp_add(&stats->tx_ewma_tp, >> - (u32)(stats->tx_unicast >> RTW_TP_SHIFT)); >> - ewma_tp_add(&stats->rx_ewma_tp, >> - (u32)(stats->rx_unicast >> RTW_TP_SHIFT)); >> + tx_unicast_shift = stats->tx_unicast >> RTW_TP_SHIFT; >> + rx_unicast_shift = stats->rx_unicast >> RTW_TP_SHIFT; > > {tx,rx}_unicast_mbps because 'shift' is to get Mbps. > >> + >> + ewma_tp_add(&stats->tx_ewma_tp, tx_unicast_shift); >> + ewma_tp_add(&stats->rx_ewma_tp, rx_unicast_shift); >> stats->tx_throughput = ewma_tp_read(&stats->tx_ewma_tp); >> stats->rx_throughput = ewma_tp_read(&stats->rx_ewma_tp); >> >> + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB && chip->ops->rx_aggregation) { >> + if (tx_unicast_shift < 1 && rx_unicast_shift < 1) >> + chip->ops->rx_aggregation(rtwdev, false); >> + else >> + chip->ops->rx_aggregation(rtwdev, true); >> + } > > Move this chunk to a function with arguments {tx,rx}_unicast_mbps. > The function name might be something like rtw_dynamic_usb_rx_aggregation(). > >> + >> /* reset tx/rx statictics */ >> stats->tx_unicast = 0; >> stats->rx_unicast = 0; >> diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h >> index 9d21637cf5d5..65bedd1668cc 100644 >> --- a/drivers/net/wireless/realtek/rtw88/main.h >> +++ b/drivers/net/wireless/realtek/rtw88/main.h >> @@ -888,6 +888,7 @@ struct rtw_chip_ops { >> void (*fill_txdesc_checksum)(struct rtw_dev *rtwdev, >> struct rtw_tx_pkt_info *pkt_info, >> u8 *txdesc); >> + void (*rx_aggregation)(struct rtw_dev *rtwdev, bool enable); >> >> /* for coex */ >> void (*coex_set_init)(struct rtw_dev *rtwdev); >> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c >> b/drivers/net/wireless/realtek/rtw88/rtw8821c.c >> index 55b6fe874710..3efdb41f22c5 100644 >> --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c >> +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c >> @@ -1276,6 +1276,28 @@ static void rtw8821c_fill_txdesc_checksum(struct rtw_dev *rtwdev, >> fill_txdesc_checksum_common(txdesc, 16); >> } >> >> +static void rtw8821c_rx_aggregation(struct rtw_dev *rtwdev, bool enable) >> +{ >> + u8 size, timeout; >> + u16 val16; >> + >> + rtw_write32_set(rtwdev, REG_RXDMA_AGG_PG_TH, BIT_EN_PRE_CALC); >> + rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN); >> + rtw_write8_clr(rtwdev, REG_RXDMA_AGG_PG_TH + 3, BIT(7)); >> + >> + if (enable) { >> + size = 0x5; >> + timeout = 0x20; >> + } else { >> + size = 0x0; >> + timeout = 0x1; >> + } >> + 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); >> +} >> + > > All use the same settings. Move this to rtw_usb_rx_aggregation() called by > rtw_dynamic_usb_rx_aggregation(). > These three chips use the same settings. RTL8821AU/RTL8812AU will be a bit different: static void rtw8821au_rx_aggregation(struct rtw_dev *rtwdev, bool enable) { struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev); u8 rxagg_usb_size, rxagg_usb_timeout; u16 val16; if (rtwusb->udev->speed == USB_SPEED_SUPER) { rxagg_usb_size = 0x7; rxagg_usb_timeout = 0x1a; } else { rxagg_usb_size = 0x5; rxagg_usb_timeout = 0x20; } if (!enable) { rxagg_usb_size = 0x0; rxagg_usb_timeout = 0x1; } val16 = (rxagg_usb_timeout << 8) | rxagg_usb_size; rtw_write16(rtwdev, REG_RXDMA_AGG_PG_TH, val16); rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN); } And RTL8814AU will be more different. I suppose they can be moved to the same place, like the burst stuff. Maybe struct rtw_hci_ops should have a new member called rx_aggregation, in case someone decides to make the SDIO driver use dynamic RX aggregation as well?
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > I suppose they can be moved to the same place, like the burst > stuff. Maybe struct rtw_hci_ops should have a new member called > rx_aggregation, in case someone decides to make the SDIO driver > use dynamic RX aggregation as well? Not sure. We can just leave NULL or dummy function for SDIO temporarily.
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c index 9d9d33a4a503..b3a089b4f707 100644 --- a/drivers/net/wireless/realtek/rtw88/main.c +++ b/drivers/net/wireless/realtek/rtw88/main.c @@ -210,8 +210,10 @@ static void rtw_watch_dog_work(struct work_struct *work) struct rtw_dev *rtwdev = container_of(work, struct rtw_dev, watch_dog_work.work); struct rtw_traffic_stats *stats = &rtwdev->stats; + const struct rtw_chip_info *chip = rtwdev->chip; struct rtw_watch_dog_iter_data data = {}; bool busy_traffic = test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags); + u32 tx_unicast_shift, rx_unicast_shift; bool ps_active; mutex_lock(&rtwdev->mutex); @@ -236,13 +238,21 @@ static void rtw_watch_dog_work(struct work_struct *work) else ps_active = false; - ewma_tp_add(&stats->tx_ewma_tp, - (u32)(stats->tx_unicast >> RTW_TP_SHIFT)); - ewma_tp_add(&stats->rx_ewma_tp, - (u32)(stats->rx_unicast >> RTW_TP_SHIFT)); + tx_unicast_shift = stats->tx_unicast >> RTW_TP_SHIFT; + rx_unicast_shift = stats->rx_unicast >> RTW_TP_SHIFT; + + ewma_tp_add(&stats->tx_ewma_tp, tx_unicast_shift); + ewma_tp_add(&stats->rx_ewma_tp, rx_unicast_shift); stats->tx_throughput = ewma_tp_read(&stats->tx_ewma_tp); stats->rx_throughput = ewma_tp_read(&stats->rx_ewma_tp); + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB && chip->ops->rx_aggregation) { + if (tx_unicast_shift < 1 && rx_unicast_shift < 1) + chip->ops->rx_aggregation(rtwdev, false); + else + chip->ops->rx_aggregation(rtwdev, true); + } + /* reset tx/rx statictics */ stats->tx_unicast = 0; stats->rx_unicast = 0; diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h index 9d21637cf5d5..65bedd1668cc 100644 --- a/drivers/net/wireless/realtek/rtw88/main.h +++ b/drivers/net/wireless/realtek/rtw88/main.h @@ -888,6 +888,7 @@ struct rtw_chip_ops { void (*fill_txdesc_checksum)(struct rtw_dev *rtwdev, struct rtw_tx_pkt_info *pkt_info, u8 *txdesc); + void (*rx_aggregation)(struct rtw_dev *rtwdev, bool enable); /* for coex */ void (*coex_set_init)(struct rtw_dev *rtwdev); diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c index 55b6fe874710..3efdb41f22c5 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c @@ -1276,6 +1276,28 @@ static void rtw8821c_fill_txdesc_checksum(struct rtw_dev *rtwdev, fill_txdesc_checksum_common(txdesc, 16); } +static void rtw8821c_rx_aggregation(struct rtw_dev *rtwdev, bool enable) +{ + u8 size, timeout; + u16 val16; + + rtw_write32_set(rtwdev, REG_RXDMA_AGG_PG_TH, BIT_EN_PRE_CALC); + rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN); + rtw_write8_clr(rtwdev, REG_RXDMA_AGG_PG_TH + 3, BIT(7)); + + if (enable) { + size = 0x5; + timeout = 0x20; + } else { + size = 0x0; + timeout = 0x1; + } + 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); +} + static struct rtw_pwr_seq_cmd trans_carddis_to_cardemu_8821c[] = { {0x0086, RTW_PWR_CUT_ALL_MSK, @@ -1724,6 +1746,7 @@ static struct rtw_chip_ops rtw8821c_ops = { .set_gid_table = rtw_bf_set_gid_table, .cfg_csi_rate = rtw_bf_cfg_csi_rate, .fill_txdesc_checksum = rtw8821c_fill_txdesc_checksum, + .rx_aggregation = rtw8821c_rx_aggregation, .coex_set_init = rtw8821c_coex_cfg_init, .coex_set_ant_switch = rtw8821c_coex_cfg_ant_switch, diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822b.c b/drivers/net/wireless/realtek/rtw88/rtw8822b.c index 0949eaa2b6c1..52bcdf3cf043 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8822b.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8822b.c @@ -1638,6 +1638,28 @@ static void rtw8822b_fill_txdesc_checksum(struct rtw_dev *rtwdev, fill_txdesc_checksum_common(txdesc, words); } +static void rtw8822b_rx_aggregation(struct rtw_dev *rtwdev, bool enable) +{ + u8 size, timeout; + u16 val16; + + rtw_write32_set(rtwdev, REG_RXDMA_AGG_PG_TH, BIT_EN_PRE_CALC); + rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN); + rtw_write8_clr(rtwdev, REG_RXDMA_AGG_PG_TH + 3, BIT(7)); + + if (enable) { + size = 0x5; + timeout = 0x20; + } else { + size = 0x0; + timeout = 0x1; + } + 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); +} + static const struct rtw_pwr_seq_cmd trans_carddis_to_cardemu_8822b[] = { {0x0086, RTW_PWR_CUT_ALL_MSK, @@ -2214,6 +2236,7 @@ static struct rtw_chip_ops rtw8822b_ops = { .adaptivity_init = rtw8822b_adaptivity_init, .adaptivity = rtw8822b_adaptivity, .fill_txdesc_checksum = rtw8822b_fill_txdesc_checksum, + .rx_aggregation = rtw8822b_rx_aggregation, .coex_set_init = rtw8822b_coex_cfg_init, .coex_set_ant_switch = rtw8822b_coex_cfg_ant_switch, diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822c.c b/drivers/net/wireless/realtek/rtw88/rtw8822c.c index 2a90a879196b..9d3ed8992133 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8822c.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.c @@ -4612,6 +4612,28 @@ static void rtw8822c_fill_txdesc_checksum(struct rtw_dev *rtwdev, fill_txdesc_checksum_common(txdesc, words); } +static void rtw8822c_rx_aggregation(struct rtw_dev *rtwdev, bool enable) +{ + u8 size, timeout; + u16 val16; + + rtw_write32_set(rtwdev, REG_RXDMA_AGG_PG_TH, BIT_EN_PRE_CALC); + rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN); + rtw_write8_clr(rtwdev, REG_RXDMA_AGG_PG_TH + 3, BIT(7)); + + if (enable) { + size = 0x5; + timeout = 0x20; + } else { + size = 0x0; + timeout = 0x1; + } + 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); +} + static const struct rtw_pwr_seq_cmd trans_carddis_to_cardemu_8822c[] = { {0x0086, RTW_PWR_CUT_ALL_MSK, @@ -5036,6 +5058,7 @@ static struct rtw_chip_ops rtw8822c_ops = { .config_tx_path = rtw8822c_config_tx_path, .config_txrx_mode = rtw8822c_config_trx_mode, .fill_txdesc_checksum = rtw8822c_fill_txdesc_checksum, + .rx_aggregation = rtw8822c_rx_aggregation, .coex_set_init = rtw8822c_coex_cfg_init, .coex_set_ant_switch = NULL,
Enable USB RX aggregation when there is at least 1 Mbps RX or TX traffic, otherwise disable it. USB RX aggregation improves the RX speed on certain ARM systems, like the NanoPi NEO Core2. With RTL8811CU, before: 28 Mbps, after: 231 Mbps. The official drivers for these chips use the same logic for SDIO, but for some reason rtw88_sdio always enables RX aggregation, so this patch only toggles aggregation for USB devices. RTL8703B is likely not found in USB devices, and RTL8723DU doesn't like aggregation. Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> --- drivers/net/wireless/realtek/rtw88/main.c | 18 +++++++++++---- drivers/net/wireless/realtek/rtw88/main.h | 1 + drivers/net/wireless/realtek/rtw88/rtw8821c.c | 23 +++++++++++++++++++ drivers/net/wireless/realtek/rtw88/rtw8822b.c | 23 +++++++++++++++++++ drivers/net/wireless/realtek/rtw88/rtw8822c.c | 23 +++++++++++++++++++ 5 files changed, 84 insertions(+), 4 deletions(-)