diff mbox series

[2/3] wifi: ath12k: add support for BA1024

Message ID 20231128025440.46988-3-quic_bqiang@quicinc.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series wifi: ath12k: some improvement to RX throughput | expand

Commit Message

Baochen Qiang Nov. 28, 2023, 2:54 a.m. UTC
Currently the maximum block ACK window size supported is 256.
This results in that, when connected to an AP which supports
larger BA sizes like BA512 or BA1024, only BA256 is
established, leading to a lower peak throughput.

So add support for BA1024, this is doen by allocating a larger
REO queue and advertising IEEE80211_MAX_AMPDU_BUF_EHT support
to MAC80211.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/dp.h       |  2 +-
 drivers/net/wireless/ath/ath12k/hal_desc.h | 28 ++++++++++++++++++++++
 drivers/net/wireless/ath/ath12k/hal_rx.c   | 11 ++++++---
 drivers/net/wireless/ath/ath12k/mac.c      |  2 +-
 4 files changed, 38 insertions(+), 5 deletions(-)

Comments

Jeff Johnson Nov. 28, 2023, 3:47 p.m. UTC | #1
On 11/27/2023 6:54 PM, Baochen Qiang wrote:
> Currently the maximum block ACK window size supported is 256.
> This results in that, when connected to an AP which supports
> larger BA sizes like BA512 or BA1024, only BA256 is
> established, leading to a lower peak throughput.
> 
> So add support for BA1024, this is doen by allocating a larger

nit: s/doen/done/

> REO queue and advertising IEEE80211_MAX_AMPDU_BUF_EHT support
> to MAC80211.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
> 
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/dp.h       |  2 +-
>  drivers/net/wireless/ath/ath12k/hal_desc.h | 28 ++++++++++++++++++++++
>  drivers/net/wireless/ath/ath12k/hal_rx.c   | 11 ++++++---
>  drivers/net/wireless/ath/ath12k/mac.c      |  2 +-
>  4 files changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/dp.h b/drivers/net/wireless/ath/ath12k/dp.h
> index 61f765432516..50db1403ebce 100644
> --- a/drivers/net/wireless/ath/ath12k/dp.h
> +++ b/drivers/net/wireless/ath/ath12k/dp.h
> @@ -145,7 +145,7 @@ struct ath12k_pdev_dp {
>  
>  #define DP_RX_HASH_ENABLE	1 /* Enable hash based Rx steering */
>  
> -#define DP_BA_WIN_SZ_MAX	256
> +#define DP_BA_WIN_SZ_MAX	1024
>  
>  #define DP_TCL_NUM_RING_MAX	4
>  
> diff --git a/drivers/net/wireless/ath/ath12k/hal_desc.h b/drivers/net/wireless/ath/ath12k/hal_desc.h
> index ec204939e50c..f12977aa6afe 100644
> --- a/drivers/net/wireless/ath/ath12k/hal_desc.h
> +++ b/drivers/net/wireless/ath/ath12k/hal_desc.h
> @@ -2517,6 +2517,34 @@ struct hal_reo_update_rx_queue {
>  	__le32 pn[4];
>  } __packed;
>  
> +struct hal_rx_reo_queue_1k {
> +	struct hal_desc_header desc_hdr;
> +	__le32 rx_bitmap_319_288;

are these individual bitmap members ever directly referenced? seems it
would make more sense to have simply:
	__le32 rx_bitmap_1023_288[23]

this would align with struct hal_rx_reo_queue which defines:
	__le32 rx_bitmap[9]

> +	__le32 rx_bitmap_351_320;
> +	__le32 rx_bitmap_383_352;
> +	__le32 rx_bitmap_415_384;
> +	__le32 rx_bitmap_447_416;
> +	__le32 rx_bitmap_479_448;
> +	__le32 rx_bitmap_511_480;
> +	__le32 rx_bitmap_543_512;
> +	__le32 rx_bitmap_575_544;
> +	__le32 rx_bitmap_607_576;
> +	__le32 rx_bitmap_639_608;
> +	__le32 rx_bitmap_671_640;
> +	__le32 rx_bitmap_703_672;
> +	__le32 rx_bitmap_735_704;
> +	__le32 rx_bitmap_767_736;
> +	__le32 rx_bitmap_799_768;
> +	__le32 rx_bitmap_831_800;
> +	__le32 rx_bitmap_863_832;
> +	__le32 rx_bitmap_895_864;
> +	__le32 rx_bitmap_927_896;
> +	__le32 rx_bitmap_959_928;
> +	__le32 rx_bitmap_991_960;
> +	__le32 rx_bitmap_1023_992;
> +	__le32 reserved[8];
> +} __packed;
> +
>  #define HAL_REO_UNBLOCK_CACHE_INFO0_UNBLK_CACHE		BIT(0)
>  #define HAL_REO_UNBLOCK_CACHE_INFO0_RESOURCE_IDX	GENMASK(2, 1)
>  
> diff --git a/drivers/net/wireless/ath/ath12k/hal_rx.c b/drivers/net/wireless/ath/ath12k/hal_rx.c
> index f6afbd8196bf..6fa874a93d3a 100644
> --- a/drivers/net/wireless/ath/ath12k/hal_rx.c
> +++ b/drivers/net/wireless/ath/ath12k/hal_rx.c
> @@ -688,23 +688,28 @@ void ath12k_hal_reo_update_rx_reo_queue_status(struct ath12k_base *ab,
>  
>  u32 ath12k_hal_reo_qdesc_size(u32 ba_window_size, u8 tid)
>  {
> -	u32 num_ext_desc;
> +	u32 num_ext_desc, num_1k_desc = 0;
>  
>  	if (ba_window_size <= 1) {
>  		if (tid != HAL_DESC_REO_NON_QOS_TID)
>  			num_ext_desc = 1;
>  		else
>  			num_ext_desc = 0;
> +
>  	} else if (ba_window_size <= 105) {
>  		num_ext_desc = 1;
>  	} else if (ba_window_size <= 210) {
>  		num_ext_desc = 2;
> -	} else {
> +	} else if (ba_window_size <= 256) {
>  		num_ext_desc = 3;
> +	} else {
> +		num_ext_desc = 10;
> +		num_1k_desc = 1;
>  	}
>  
>  	return sizeof(struct hal_rx_reo_queue) +
> -		(num_ext_desc * sizeof(struct hal_rx_reo_queue_ext));
> +		(num_ext_desc * sizeof(struct hal_rx_reo_queue_ext)) +
> +		(num_1k_desc * sizeof(struct hal_rx_reo_queue_1k));
>  }
>  
>  void ath12k_hal_reo_qdesc_setup(struct hal_rx_reo_queue *qdesc,
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index fc0d14ea328e..3cfb17f71aa6 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -7474,7 +7474,7 @@ static int __ath12k_mac_register(struct ath12k *ar)
>  	ar->hw->queues = ATH12K_HW_MAX_QUEUES;
>  	ar->hw->wiphy->tx_queue_len = ATH12K_QUEUE_LEN;
>  	ar->hw->offchannel_tx_hw_queue = ATH12K_HW_MAX_QUEUES - 1;
> -	ar->hw->max_rx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF_HE;
> +	ar->hw->max_rx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF_EHT;
>  
>  	ar->hw->vif_data_size = sizeof(struct ath12k_vif);
>  	ar->hw->sta_data_size = sizeof(struct ath12k_sta);

Are any related changes needed to struct hal_reo_get_queue_stats_status?
Or to ath12k_hal_reo_status_queue_stats()?
There I see the 256-bit bitmap being dumped -- do you need to dump the
1024-bit bitmap? Is there a mechanism which allows that?
Baochen Qiang Nov. 29, 2023, 1:52 a.m. UTC | #2
On 11/28/2023 11:47 PM, Jeff Johnson wrote:
> On 11/27/2023 6:54 PM, Baochen Qiang wrote:
>> Currently the maximum block ACK window size supported is 256.
>> This results in that, when connected to an AP which supports
>> larger BA sizes like BA512 or BA1024, only BA256 is
>> established, leading to a lower peak throughput.
>>
>> So add support for BA1024, this is doen by allocating a larger
> 
> nit: s/doen/done/
> 
>> REO queue and advertising IEEE80211_MAX_AMPDU_BUF_EHT support
>> to MAC80211.
>>
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>>
>> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/dp.h       |  2 +-
>>   drivers/net/wireless/ath/ath12k/hal_desc.h | 28 ++++++++++++++++++++++
>>   drivers/net/wireless/ath/ath12k/hal_rx.c   | 11 ++++++---
>>   drivers/net/wireless/ath/ath12k/mac.c      |  2 +-
>>   4 files changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/dp.h b/drivers/net/wireless/ath/ath12k/dp.h
>> index 61f765432516..50db1403ebce 100644
>> --- a/drivers/net/wireless/ath/ath12k/dp.h
>> +++ b/drivers/net/wireless/ath/ath12k/dp.h
>> @@ -145,7 +145,7 @@ struct ath12k_pdev_dp {
>>   
>>   #define DP_RX_HASH_ENABLE	1 /* Enable hash based Rx steering */
>>   
>> -#define DP_BA_WIN_SZ_MAX	256
>> +#define DP_BA_WIN_SZ_MAX	1024
>>   
>>   #define DP_TCL_NUM_RING_MAX	4
>>   
>> diff --git a/drivers/net/wireless/ath/ath12k/hal_desc.h b/drivers/net/wireless/ath/ath12k/hal_desc.h
>> index ec204939e50c..f12977aa6afe 100644
>> --- a/drivers/net/wireless/ath/ath12k/hal_desc.h
>> +++ b/drivers/net/wireless/ath/ath12k/hal_desc.h
>> @@ -2517,6 +2517,34 @@ struct hal_reo_update_rx_queue {
>>   	__le32 pn[4];
>>   } __packed;
>>   
>> +struct hal_rx_reo_queue_1k {
>> +	struct hal_desc_header desc_hdr;
>> +	__le32 rx_bitmap_319_288;
> 
> are these individual bitmap members ever directly referenced? seems it
> would make more sense to have simply:
> 	__le32 rx_bitmap_1023_288[23]
> 
> this would align with struct hal_rx_reo_queue which defines:
> 	__le32 rx_bitmap[9]
> 
>> +	__le32 rx_bitmap_351_320;
>> +	__le32 rx_bitmap_383_352;
>> +	__le32 rx_bitmap_415_384;
>> +	__le32 rx_bitmap_447_416;
>> +	__le32 rx_bitmap_479_448;
>> +	__le32 rx_bitmap_511_480;
>> +	__le32 rx_bitmap_543_512;
>> +	__le32 rx_bitmap_575_544;
>> +	__le32 rx_bitmap_607_576;
>> +	__le32 rx_bitmap_639_608;
>> +	__le32 rx_bitmap_671_640;
>> +	__le32 rx_bitmap_703_672;
>> +	__le32 rx_bitmap_735_704;
>> +	__le32 rx_bitmap_767_736;
>> +	__le32 rx_bitmap_799_768;
>> +	__le32 rx_bitmap_831_800;
>> +	__le32 rx_bitmap_863_832;
>> +	__le32 rx_bitmap_895_864;
>> +	__le32 rx_bitmap_927_896;
>> +	__le32 rx_bitmap_959_928;
>> +	__le32 rx_bitmap_991_960;
>> +	__le32 rx_bitmap_1023_992;
>> +	__le32 reserved[8];
>> +} __packed;
>> +
>>   #define HAL_REO_UNBLOCK_CACHE_INFO0_UNBLK_CACHE		BIT(0)
>>   #define HAL_REO_UNBLOCK_CACHE_INFO0_RESOURCE_IDX	GENMASK(2, 1)
>>   
>> diff --git a/drivers/net/wireless/ath/ath12k/hal_rx.c b/drivers/net/wireless/ath/ath12k/hal_rx.c
>> index f6afbd8196bf..6fa874a93d3a 100644
>> --- a/drivers/net/wireless/ath/ath12k/hal_rx.c
>> +++ b/drivers/net/wireless/ath/ath12k/hal_rx.c
>> @@ -688,23 +688,28 @@ void ath12k_hal_reo_update_rx_reo_queue_status(struct ath12k_base *ab,
>>   
>>   u32 ath12k_hal_reo_qdesc_size(u32 ba_window_size, u8 tid)
>>   {
>> -	u32 num_ext_desc;
>> +	u32 num_ext_desc, num_1k_desc = 0;
>>   
>>   	if (ba_window_size <= 1) {
>>   		if (tid != HAL_DESC_REO_NON_QOS_TID)
>>   			num_ext_desc = 1;
>>   		else
>>   			num_ext_desc = 0;
>> +
>>   	} else if (ba_window_size <= 105) {
>>   		num_ext_desc = 1;
>>   	} else if (ba_window_size <= 210) {
>>   		num_ext_desc = 2;
>> -	} else {
>> +	} else if (ba_window_size <= 256) {
>>   		num_ext_desc = 3;
>> +	} else {
>> +		num_ext_desc = 10;
>> +		num_1k_desc = 1;
>>   	}
>>   
>>   	return sizeof(struct hal_rx_reo_queue) +
>> -		(num_ext_desc * sizeof(struct hal_rx_reo_queue_ext));
>> +		(num_ext_desc * sizeof(struct hal_rx_reo_queue_ext)) +
>> +		(num_1k_desc * sizeof(struct hal_rx_reo_queue_1k));
>>   }
>>   
>>   void ath12k_hal_reo_qdesc_setup(struct hal_rx_reo_queue *qdesc,
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index fc0d14ea328e..3cfb17f71aa6 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -7474,7 +7474,7 @@ static int __ath12k_mac_register(struct ath12k *ar)
>>   	ar->hw->queues = ATH12K_HW_MAX_QUEUES;
>>   	ar->hw->wiphy->tx_queue_len = ATH12K_QUEUE_LEN;
>>   	ar->hw->offchannel_tx_hw_queue = ATH12K_HW_MAX_QUEUES - 1;
>> -	ar->hw->max_rx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF_HE;
>> +	ar->hw->max_rx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF_EHT;
>>   
>>   	ar->hw->vif_data_size = sizeof(struct ath12k_vif);
>>   	ar->hw->sta_data_size = sizeof(struct ath12k_sta);
> 
> Are any related changes needed to struct hal_reo_get_queue_stats_status?
> Or to ath12k_hal_reo_status_queue_stats()?
> There I see the 256-bit bitmap being dumped -- do you need to dump the
> 1024-bit bitmap? Is there a mechanism which allows that?
I don't think it's possible. As you can see, struct 
hal_reo_get_queue_stats_status is directly extracted from REO_STATUS 
ring, which is queued by firmware/HW. That is to say firmware/HW doesn't 
upload other bitmap info, except for the first 256 bits, to host, so 
host has no way to dump them.
> 
>
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath12k/dp.h b/drivers/net/wireless/ath/ath12k/dp.h
index 61f765432516..50db1403ebce 100644
--- a/drivers/net/wireless/ath/ath12k/dp.h
+++ b/drivers/net/wireless/ath/ath12k/dp.h
@@ -145,7 +145,7 @@  struct ath12k_pdev_dp {
 
 #define DP_RX_HASH_ENABLE	1 /* Enable hash based Rx steering */
 
-#define DP_BA_WIN_SZ_MAX	256
+#define DP_BA_WIN_SZ_MAX	1024
 
 #define DP_TCL_NUM_RING_MAX	4
 
diff --git a/drivers/net/wireless/ath/ath12k/hal_desc.h b/drivers/net/wireless/ath/ath12k/hal_desc.h
index ec204939e50c..f12977aa6afe 100644
--- a/drivers/net/wireless/ath/ath12k/hal_desc.h
+++ b/drivers/net/wireless/ath/ath12k/hal_desc.h
@@ -2517,6 +2517,34 @@  struct hal_reo_update_rx_queue {
 	__le32 pn[4];
 } __packed;
 
+struct hal_rx_reo_queue_1k {
+	struct hal_desc_header desc_hdr;
+	__le32 rx_bitmap_319_288;
+	__le32 rx_bitmap_351_320;
+	__le32 rx_bitmap_383_352;
+	__le32 rx_bitmap_415_384;
+	__le32 rx_bitmap_447_416;
+	__le32 rx_bitmap_479_448;
+	__le32 rx_bitmap_511_480;
+	__le32 rx_bitmap_543_512;
+	__le32 rx_bitmap_575_544;
+	__le32 rx_bitmap_607_576;
+	__le32 rx_bitmap_639_608;
+	__le32 rx_bitmap_671_640;
+	__le32 rx_bitmap_703_672;
+	__le32 rx_bitmap_735_704;
+	__le32 rx_bitmap_767_736;
+	__le32 rx_bitmap_799_768;
+	__le32 rx_bitmap_831_800;
+	__le32 rx_bitmap_863_832;
+	__le32 rx_bitmap_895_864;
+	__le32 rx_bitmap_927_896;
+	__le32 rx_bitmap_959_928;
+	__le32 rx_bitmap_991_960;
+	__le32 rx_bitmap_1023_992;
+	__le32 reserved[8];
+} __packed;
+
 #define HAL_REO_UNBLOCK_CACHE_INFO0_UNBLK_CACHE		BIT(0)
 #define HAL_REO_UNBLOCK_CACHE_INFO0_RESOURCE_IDX	GENMASK(2, 1)
 
diff --git a/drivers/net/wireless/ath/ath12k/hal_rx.c b/drivers/net/wireless/ath/ath12k/hal_rx.c
index f6afbd8196bf..6fa874a93d3a 100644
--- a/drivers/net/wireless/ath/ath12k/hal_rx.c
+++ b/drivers/net/wireless/ath/ath12k/hal_rx.c
@@ -688,23 +688,28 @@  void ath12k_hal_reo_update_rx_reo_queue_status(struct ath12k_base *ab,
 
 u32 ath12k_hal_reo_qdesc_size(u32 ba_window_size, u8 tid)
 {
-	u32 num_ext_desc;
+	u32 num_ext_desc, num_1k_desc = 0;
 
 	if (ba_window_size <= 1) {
 		if (tid != HAL_DESC_REO_NON_QOS_TID)
 			num_ext_desc = 1;
 		else
 			num_ext_desc = 0;
+
 	} else if (ba_window_size <= 105) {
 		num_ext_desc = 1;
 	} else if (ba_window_size <= 210) {
 		num_ext_desc = 2;
-	} else {
+	} else if (ba_window_size <= 256) {
 		num_ext_desc = 3;
+	} else {
+		num_ext_desc = 10;
+		num_1k_desc = 1;
 	}
 
 	return sizeof(struct hal_rx_reo_queue) +
-		(num_ext_desc * sizeof(struct hal_rx_reo_queue_ext));
+		(num_ext_desc * sizeof(struct hal_rx_reo_queue_ext)) +
+		(num_1k_desc * sizeof(struct hal_rx_reo_queue_1k));
 }
 
 void ath12k_hal_reo_qdesc_setup(struct hal_rx_reo_queue *qdesc,
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index fc0d14ea328e..3cfb17f71aa6 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -7474,7 +7474,7 @@  static int __ath12k_mac_register(struct ath12k *ar)
 	ar->hw->queues = ATH12K_HW_MAX_QUEUES;
 	ar->hw->wiphy->tx_queue_len = ATH12K_QUEUE_LEN;
 	ar->hw->offchannel_tx_hw_queue = ATH12K_HW_MAX_QUEUES - 1;
-	ar->hw->max_rx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF_HE;
+	ar->hw->max_rx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF_EHT;
 
 	ar->hw->vif_data_size = sizeof(struct ath12k_vif);
 	ar->hw->sta_data_size = sizeof(struct ath12k_sta);