Received-SPF: pass (google.com: domain of linux-wireless+bounces-8970-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <4bdf84bf-0d6f-4391-a529-134ccde53aae@gmail.com>
Date: Thu, 13 Jun 2024 04:57:48 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] wifi: ath11k: fix ack signal strength calculation
To: Lingbo Kong <quic_lingbok@quicinc.com>, ath11k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
References: <20240611022550.59078-1-quic_lingbok@quicinc.com>
Content-Language: en-US
From: James Prestwood <prestwoj@gmail.com>
In-Reply-To: <20240611022550.59078-1-quic_lingbok@quicinc.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi Lingbo,

On 6/10/24 7:25 PM, Lingbo Kong wrote:
> Currently, the calculation of ack signal strength is incorrect.
>
> This is because before calculating the ack signal strength, ath11k need
> to determine whether the hardware and firmware support db2dbm. If the
> hardware and firmware support db2dbm, do not need to add noise floor,
> otherwise, need to add noise floor.

This oatch got me checking on some devices we have running ath11k 
(WCN6855). I don't see any "ack signal" property but I noticed the 
signal avg does not seem correct:

signal:      -55 [-62, -56] dBm
signal avg:    105 dBm

Every other device besides ath11k has a signal avg close to or equal to 
the signal. Do you think there is also an issue with this property? not 
just the ack signal?

Thanks,

James

>
> Besides, the value of ack_rssi passed by firmware to ath11k should be a
> signed number, so change its type to s8.
>
> After that, "iw wlan0 station dump" show the correct ack signal strength.
>
> Such as:
> root@CDCCSTEX0799733-LIN:~# iw wlp88s0 station dump
> Station 00:03:7f:12:df:df (on wlp88s0)
>          inactive time:  75 ms
>          rx bytes:       11599
>          rx packets:     99
>          tx bytes:       9029
>          tx packets:     81
>          tx retries:     4
>          tx failed:      0
>          rx drop misc:   2
>          signal:         -16 dBm
>          signal avg:     -24 dBm
>          tx bitrate:     1560.0 MBit/s VHT-MCS 9 80MHz VHT-NSS 4
>          tx duration:    9230 us
>          rx bitrate:     1560.0 MBit/s VHT-MCS 9 80MHz VHT-NSS 4
>          rx duration:    7201 us
>          last ack signal:-23 dBm
>          avg ack signal: -22 dBm
>
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
> ---
>   drivers/net/wireless/ath/ath11k/dp_tx.c  | 16 ++++++++++++----
>   drivers/net/wireless/ath/ath11k/dp_tx.h  |  4 ++--
>   drivers/net/wireless/ath/ath11k/hal_tx.h |  4 ++--
>   3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.c b/drivers/net/wireless/ath/ath11k/dp_tx.c
> index 21819b741701..8522c67baabf 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
> @@ -353,8 +353,12 @@ ath11k_dp_tx_htt_tx_complete_buf(struct ath11k_base *ab,
>   	if (ts->acked) {
>   		if (!(info->flags & IEEE80211_TX_CTL_NO_ACK)) {
>   			info->flags |= IEEE80211_TX_STAT_ACK;
> -			info->status.ack_signal = ATH11K_DEFAULT_NOISE_FLOOR +
> -						  ts->ack_rssi;
> +			info->status.ack_signal = ts->ack_rssi;
> +
> +			if (!test_bit(WMI_TLV_SERVICE_HW_DB2DBM_CONVERSION_SUPPORT,
> +				      ab->wmi_ab.svc_map))
> +				info->status.ack_signal += ATH11K_DEFAULT_NOISE_FLOOR;
> +
>   			info->status.flags |=
>   				IEEE80211_TX_STATUS_ACK_SIGNAL_VALID;
>   		} else {
> @@ -584,8 +588,12 @@ static void ath11k_dp_tx_complete_msdu(struct ath11k *ar,
>   	if (ts->status == HAL_WBM_TQM_REL_REASON_FRAME_ACKED &&
>   	    !(info->flags & IEEE80211_TX_CTL_NO_ACK)) {
>   		info->flags |= IEEE80211_TX_STAT_ACK;
> -		info->status.ack_signal = ATH11K_DEFAULT_NOISE_FLOOR +
> -					  ts->ack_rssi;
> +		info->status.ack_signal = ts->ack_rssi;
> +
> +		if (!test_bit(WMI_TLV_SERVICE_HW_DB2DBM_CONVERSION_SUPPORT,
> +			      ab->wmi_ab.svc_map))
> +			info->status.ack_signal += ATH11K_DEFAULT_NOISE_FLOOR;
> +
>   		info->status.flags |= IEEE80211_TX_STATUS_ACK_SIGNAL_VALID;
>   	}
>   
> diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.h b/drivers/net/wireless/ath/ath11k/dp_tx.h
> index 61be2265e09f..795fe3b8fa0d 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_tx.h
> +++ b/drivers/net/wireless/ath/ath11k/dp_tx.h
> @@ -1,7 +1,7 @@
>   /* SPDX-License-Identifier: BSD-3-Clause-Clear */
>   /*
>    * Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
> - * Copyright (c) 2021, 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2021, 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>    */
>   
>   #ifndef ATH11K_DP_TX_H
> @@ -13,7 +13,7 @@
>   struct ath11k_dp_htt_wbm_tx_status {
>   	u32 msdu_id;
>   	bool acked;
> -	int ack_rssi;
> +	s8 ack_rssi;
>   	u16 peer_id;
>   };
>   
> diff --git a/drivers/net/wireless/ath/ath11k/hal_tx.h b/drivers/net/wireless/ath/ath11k/hal_tx.h
> index c5e88364afe5..46d17abd808b 100644
> --- a/drivers/net/wireless/ath/ath11k/hal_tx.h
> +++ b/drivers/net/wireless/ath/ath11k/hal_tx.h
> @@ -1,7 +1,7 @@
>   /* SPDX-License-Identifier: BSD-3-Clause-Clear */
>   /*
>    * Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
> - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2022, 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>    */
>   
>   #ifndef ATH11K_HAL_TX_H
> @@ -54,7 +54,7 @@ struct hal_tx_info {
>   struct hal_tx_status {
>   	enum hal_wbm_rel_src_module buf_rel_source;
>   	enum hal_wbm_tqm_rel_reason status;
> -	u8 ack_rssi;
> +	s8 ack_rssi;
>   	u32 flags; /* %HAL_TX_STATUS_FLAGS_ */
>   	u32 ppdu_id;
>   	u8 try_cnt;
>
> base-commit: a116bf2be795eb1db75fa6a48aa85c397be001a6