Received-SPF: pass (google.com: domain of linux-wireless+bounces-8971-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <39f55daa-788a-4e67-884b-948570655eb8@quicinc.com>
Date: Thu, 13 Jun 2024 20:08:09 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] wifi: ath11k: fix ack signal strength calculation
To: James Prestwood <prestwoj@gmail.com>, <ath11k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20240611022550.59078-1-quic_lingbok@quicinc.com>
 <4bdf84bf-0d6f-4391-a529-134ccde53aae@gmail.com>
Content-Language: en-US
From: Lingbo Kong <quic_lingbok@quicinc.com>
In-Reply-To: <4bdf84bf-0d6f-4391-a529-134ccde53aae@gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


On 2024/6/13 19:57, James Prestwood wrote:
> 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
> 

Hi,james,

This is indeed a issue, but I have solved it. It's just that this patch 
has not been applied to ath.git. you can apply the following patch.

https://patchwork.kernel.org/project/linux-wireless/patch/20240309121129.5379-1-quic_lingbok@quicinc.com/

best regards
Lingbo Kong


>>
>> 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