[PATCH] ath10k: Per-chain rssi should sum the secondary channels

[PATCH] ath10k: Per-chain rssi should sum the secondary channels

[greearb]

From: Ben Greear <greearb@candelatech.com>

This makes per-chain RSSI be more consistent between HT20, HT40, HT80.
Instead of doing precise log math for adding dbm, I did a rough estimate,
it seems to work good enough.

Tested on ath10k-ct 9984 firmware.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c  | 64 ++++++++++++++++++++---
 drivers/net/wireless/ath/ath10k/rx_desc.h |  3 +-
 2 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 13f652b622df..034d4ace228d 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1167,6 +1167,44 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
 	return true;
 }
 
+static int ath10k_sum_sigs_2(int a, int b) {
+	int diff;
+
+	if (b == 0x80)
+		return a;
+
+	if (a >= b) {
+		diff = a - b;
+		if (diff == 0)
+			return a + 3;
+		else if (diff == 1)
+			return a + 2;
+		else if (diff == 2)
+			return a + 1;
+		return a;
+	}
+	else {
+		diff = b - a;
+		if (diff == 0)
+			return b + 3;
+		else if (diff == 1)
+			return b + 2;
+		else if (diff == 2)
+			return b + 1;
+		return b;
+	}
+}
+
+static int ath10k_sum_sigs(int p20, int e20, int e40, int e80) {
+	/* Hacky attempt at summing dbm without resorting to log(10) business */
+	if (e40 != 0x80) {
+		return ath10k_sum_sigs_2(ath10k_sum_sigs_2(p20, e20), ath10k_sum_sigs_2(e40, e80));
+	}
+	else {
+		return ath10k_sum_sigs_2(p20, e20);
+	}
+}
+
 static void ath10k_htt_rx_h_signal(struct ath10k *ar,
 				   struct ieee80211_rx_status *status,
 				   struct htt_rx_desc *rxd)
@@ -1177,18 +1215,32 @@ static void ath10k_htt_rx_h_signal(struct ath10k *ar,
 		status->chains &= ~BIT(i);
 
 		if (rxd->ppdu_start.rssi_chains[i].pri20_mhz != 0x80) {
-			status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR +
-				rxd->ppdu_start.rssi_chains[i].pri20_mhz;
+			status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR
+				+ ath10k_sum_sigs(rxd->ppdu_start.rssi_chains[i].pri20_mhz,
+						  rxd->ppdu_start.rssi_chains[i].ext20_mhz,
+						  rxd->ppdu_start.rssi_chains[i].ext40_mhz,
+						  rxd->ppdu_start.rssi_chains[i].ext80_mhz);
+			//ath10k_warn(ar, "rx-h-sig, chain[%i] pri20: %d ext20: %d  ext40: %d  ext80: %d\n",
+			//	    i, rxd->ppdu_start.rssi_chains[i].pri20_mhz, rxd->ppdu_start.rssi_chains[i].ext20_mhz,
+			//	    rxd->ppdu_start.rssi_chains[i].ext40_mhz, rxd->ppdu_start.rssi_chains[i].ext80_mhz);
 
 			status->chains |= BIT(i);
 		}
 	}
 
 	/* FIXME: Get real NF */
-	status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
-			 rxd->ppdu_start.rssi_comb;
-	/* ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x  chain[0]: %d  chain[1]: %d  chan[2]: %d\n",
-                       status->signal, status->chains, status->chain_signal[0], status->chain_signal[1], status->chain_signal[2]); */
+	if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
+		status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
+			rxd->ppdu_start.rssi_comb_ht;
+	}
+	else {
+		status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
+			rxd->ppdu_start.rssi_comb;
+	}
+
+	//ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x  chain[0]: %d  chain[1]: %d  chain[2]: %d chain[3]: %d\n",
+	//	    status->signal, status->chains, status->chain_signal[0],
+	//	    status->chain_signal[1], status->chain_signal[2], status->chain_signal[3]);
 	status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h b/drivers/net/wireless/ath/ath10k/rx_desc.h
index dec1582005b9..6b44677474dd 100644
--- a/drivers/net/wireless/ath/ath10k/rx_desc.h
+++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
@@ -726,7 +726,8 @@ struct rx_ppdu_start {
 		u8 ext80_mhz;
 	} rssi_chains[4];
 	u8 rssi_comb;
-	__le16 rsvd0;
+	u8 rsvd0; /* first two bits are bandwidth, other 6 are reserved */
+	u8 rssi_comb_ht;
 	u8 info0; /* %RX_PPDU_START_INFO0_ */
 	__le32 info1; /* %RX_PPDU_START_INFO1_ */
 	__le32 info2; /* %RX_PPDU_START_INFO2_ */
-- 
2.20.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Sebastian Gottschall]

i see a issue in your patch for qca988x chipsets

+	if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
+		status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
+			rxd->ppdu_start.rssi_comb_ht;
+	}


this is always true for qca988x, but the field is not provided on these 
older chipsets. so signal reporting will be broken
i'm currently debugging in your code, but i already have seen that the 
values are wrong now for this chipset

Am 16.12.2019 um 23:07 schrieb greearb@candelatech.com:
> From: Ben Greear <greearb@candelatech.com>
>
> This makes per-chain RSSI be more consistent between HT20, HT40, HT80.
> Instead of doing precise log math for adding dbm, I did a rough estimate,
> it seems to work good enough.
>
> Tested on ath10k-ct 9984 firmware.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>   drivers/net/wireless/ath/ath10k/htt_rx.c  | 64 ++++++++++++++++++++---
>   drivers/net/wireless/ath/ath10k/rx_desc.h |  3 +-
>   2 files changed, 60 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 13f652b622df..034d4ace228d 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1167,6 +1167,44 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
>   	return true;
>   }
>   
> +static int ath10k_sum_sigs_2(int a, int b) {
> +	int diff;
> +
> +	if (b == 0x80)
> +		return a;
> +
> +	if (a >= b) {
> +		diff = a - b;
> +		if (diff == 0)
> +			return a + 3;
> +		else if (diff == 1)
> +			return a + 2;
> +		else if (diff == 2)
> +			return a + 1;
> +		return a;
> +	}
> +	else {
> +		diff = b - a;
> +		if (diff == 0)
> +			return b + 3;
> +		else if (diff == 1)
> +			return b + 2;
> +		else if (diff == 2)
> +			return b + 1;
> +		return b;
> +	}
> +}
> +
> +static int ath10k_sum_sigs(int p20, int e20, int e40, int e80) {
> +	/* Hacky attempt at summing dbm without resorting to log(10) business */
> +	if (e40 != 0x80) {
> +		return ath10k_sum_sigs_2(ath10k_sum_sigs_2(p20, e20), ath10k_sum_sigs_2(e40, e80));
> +	}
> +	else {
> +		return ath10k_sum_sigs_2(p20, e20);
> +	}
> +}
> +
>   static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>   				   struct ieee80211_rx_status *status,
>   				   struct htt_rx_desc *rxd)
> @@ -1177,18 +1215,32 @@ static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>   		status->chains &= ~BIT(i);
>   
>   		if (rxd->ppdu_start.rssi_chains[i].pri20_mhz != 0x80) {
> -			status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR +
> -				rxd->ppdu_start.rssi_chains[i].pri20_mhz;
> +			status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR
> +				+ ath10k_sum_sigs(rxd->ppdu_start.rssi_chains[i].pri20_mhz,
> +						  rxd->ppdu_start.rssi_chains[i].ext20_mhz,
> +						  rxd->ppdu_start.rssi_chains[i].ext40_mhz,
> +						  rxd->ppdu_start.rssi_chains[i].ext80_mhz);
> +			//ath10k_warn(ar, "rx-h-sig, chain[%i] pri20: %d ext20: %d  ext40: %d  ext80: %d\n",
> +			//	    i, rxd->ppdu_start.rssi_chains[i].pri20_mhz, rxd->ppdu_start.rssi_chains[i].ext20_mhz,
> +			//	    rxd->ppdu_start.rssi_chains[i].ext40_mhz, rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>   
>   			status->chains |= BIT(i);
>   		}
>   	}
>   
>   	/* FIXME: Get real NF */
> -	status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
> -			 rxd->ppdu_start.rssi_comb;
> -	/* ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x  chain[0]: %d  chain[1]: %d  chan[2]: %d\n",
> -                       status->signal, status->chains, status->chain_signal[0], status->chain_signal[1], status->chain_signal[2]); */
> +	if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
> +		status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
> +			rxd->ppdu_start.rssi_comb_ht;
> +	}
> +	else {
> +		status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
> +			rxd->ppdu_start.rssi_comb;
> +	}
> +
> +	//ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x  chain[0]: %d  chain[1]: %d  chain[2]: %d chain[3]: %d\n",
> +	//	    status->signal, status->chains, status->chain_signal[0],
> +	//	    status->chain_signal[1], status->chain_signal[2], status->chain_signal[3]);
>   	status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
>   }
>   
> diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h b/drivers/net/wireless/ath/ath10k/rx_desc.h
> index dec1582005b9..6b44677474dd 100644
> --- a/drivers/net/wireless/ath/ath10k/rx_desc.h
> +++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
> @@ -726,7 +726,8 @@ struct rx_ppdu_start {
>   		u8 ext80_mhz;
>   	} rssi_chains[4];
>   	u8 rssi_comb;
> -	__le16 rsvd0;
> +	u8 rsvd0; /* first two bits are bandwidth, other 6 are reserved */
> +	u8 rssi_comb_ht;
>   	u8 info0; /* %RX_PPDU_START_INFO0_ */
>   	__le32 info1; /* %RX_PPDU_START_INFO1_ */
>   	__le32 info2; /* %RX_PPDU_START_INFO2_ */

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Sebastian Gottschall]

result of my tests

on qca988x rxd->ppdu_start.rssi_comb_ht is always zero. so you need to 
add a additional check

Am 17.12.2019 um 13:02 schrieb Sebastian Gottschall:
> i see a issue in your patch for qca988x chipsets
>
> +    if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
> +            rxd->ppdu_start.rssi_comb_ht;
> +    }
>
>
> this is always true for qca988x, but the field is not provided on 
> these older chipsets. so signal reporting will be broken
> i'm currently debugging in your code, but i already have seen that the 
> values are wrong now for this chipset
>
> Am 16.12.2019 um 23:07 schrieb greearb@candelatech.com:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> This makes per-chain RSSI be more consistent between HT20, HT40, HT80.
>> Instead of doing precise log math for adding dbm, I did a rough 
>> estimate,
>> it seems to work good enough.
>>
>> Tested on ath10k-ct 9984 firmware.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>   drivers/net/wireless/ath/ath10k/htt_rx.c  | 64 ++++++++++++++++++++---
>>   drivers/net/wireless/ath/ath10k/rx_desc.h |  3 +-
>>   2 files changed, 60 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index 13f652b622df..034d4ace228d 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -1167,6 +1167,44 @@ static bool ath10k_htt_rx_h_channel(struct 
>> ath10k *ar,
>>       return true;
>>   }
>>   +static int ath10k_sum_sigs_2(int a, int b) {
>> +    int diff;
>> +
>> +    if (b == 0x80)
>> +        return a;
>> +
>> +    if (a >= b) {
>> +        diff = a - b;
>> +        if (diff == 0)
>> +            return a + 3;
>> +        else if (diff == 1)
>> +            return a + 2;
>> +        else if (diff == 2)
>> +            return a + 1;
>> +        return a;
>> +    }
>> +    else {
>> +        diff = b - a;
>> +        if (diff == 0)
>> +            return b + 3;
>> +        else if (diff == 1)
>> +            return b + 2;
>> +        else if (diff == 2)
>> +            return b + 1;
>> +        return b;
>> +    }
>> +}
>> +
>> +static int ath10k_sum_sigs(int p20, int e20, int e40, int e80) {
>> +    /* Hacky attempt at summing dbm without resorting to log(10) 
>> business */
>> +    if (e40 != 0x80) {
>> +        return ath10k_sum_sigs_2(ath10k_sum_sigs_2(p20, e20), 
>> ath10k_sum_sigs_2(e40, e80));
>> +    }
>> +    else {
>> +        return ath10k_sum_sigs_2(p20, e20);
>> +    }
>> +}
>> +
>>   static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>>                      struct ieee80211_rx_status *status,
>>                      struct htt_rx_desc *rxd)
>> @@ -1177,18 +1215,32 @@ static void ath10k_htt_rx_h_signal(struct 
>> ath10k *ar,
>>           status->chains &= ~BIT(i);
>>             if (rxd->ppdu_start.rssi_chains[i].pri20_mhz != 0x80) {
>> -            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR +
>> -                rxd->ppdu_start.rssi_chains[i].pri20_mhz;
>> +            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR
>> +                + 
>> ath10k_sum_sigs(rxd->ppdu_start.rssi_chains[i].pri20_mhz,
>> + rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>> + rxd->ppdu_start.rssi_chains[i].ext40_mhz,
>> + rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>> +            //ath10k_warn(ar, "rx-h-sig, chain[%i] pri20: %d ext20: 
>> %d  ext40: %d  ext80: %d\n",
>> +            //        i, rxd->ppdu_start.rssi_chains[i].pri20_mhz, 
>> rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>> +            // rxd->ppdu_start.rssi_chains[i].ext40_mhz, 
>> rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>                 status->chains |= BIT(i);
>>           }
>>       }
>>         /* FIXME: Get real NF */
>> -    status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>> -             rxd->ppdu_start.rssi_comb;
>> -    /* ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x chain[0]: 
>> %d  chain[1]: %d  chan[2]: %d\n",
>> -                       status->signal, status->chains, 
>> status->chain_signal[0], status->chain_signal[1], 
>> status->chain_signal[2]); */
>> +    if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>> +            rxd->ppdu_start.rssi_comb_ht;
>> +    }
>> +    else {
>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>> +            rxd->ppdu_start.rssi_comb;
>> +    }
>> +
>> +    //ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x chain[0]: 
>> %d  chain[1]: %d  chain[2]: %d chain[3]: %d\n",
>> +    //        status->signal, status->chains, status->chain_signal[0],
>> +    //        status->chain_signal[1], status->chain_signal[2], 
>> status->chain_signal[3]);
>>       status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
>>   }
>>   diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h 
>> b/drivers/net/wireless/ath/ath10k/rx_desc.h
>> index dec1582005b9..6b44677474dd 100644
>> --- a/drivers/net/wireless/ath/ath10k/rx_desc.h
>> +++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
>> @@ -726,7 +726,8 @@ struct rx_ppdu_start {
>>           u8 ext80_mhz;
>>       } rssi_chains[4];
>>       u8 rssi_comb;
>> -    __le16 rsvd0;
>> +    u8 rsvd0; /* first two bits are bandwidth, other 6 are reserved */
>> +    u8 rssi_comb_ht;
>>       u8 info0; /* %RX_PPDU_START_INFO0_ */
>>       __le32 info1; /* %RX_PPDU_START_INFO1_ */
>>       __le32 info2; /* %RX_PPDU_START_INFO2_ */
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Ben Greear]

On 12/17/2019 04:32 AM, Sebastian Gottschall wrote:
> result of my tests
>
> on qca988x rxd->ppdu_start.rssi_comb_ht is always zero. so you need to add a additional check
>
> Am 17.12.2019 um 13:02 schrieb Sebastian Gottschall:
>> i see a issue in your patch for qca988x chipsets
>>
>> +    if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>> +            rxd->ppdu_start.rssi_comb_ht;
>> +    }
>>
>>
>> this is always true for qca988x, but the field is not provided on these older chipsets. so signal reporting will be broken
>> i'm currently debugging in your code, but i already have seen that the values are wrong now for this chipset

Thanks for testing.  I'll add a check for 0 and ignore that value too.  That seem OK?

Were the per-chain values OK?

Thanks,
Ben

>>
>> Am 16.12.2019 um 23:07 schrieb greearb@candelatech.com:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> This makes per-chain RSSI be more consistent between HT20, HT40, HT80.
>>> Instead of doing precise log math for adding dbm, I did a rough estimate,
>>> it seems to work good enough.
>>>
>>> Tested on ath10k-ct 9984 firmware.
>>>
>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>> ---
>>>   drivers/net/wireless/ath/ath10k/htt_rx.c  | 64 ++++++++++++++++++++---
>>>   drivers/net/wireless/ath/ath10k/rx_desc.h |  3 +-
>>>   2 files changed, 60 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index 13f652b622df..034d4ace228d 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -1167,6 +1167,44 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
>>>       return true;
>>>   }
>>>   +static int ath10k_sum_sigs_2(int a, int b) {
>>> +    int diff;
>>> +
>>> +    if (b == 0x80)
>>> +        return a;
>>> +
>>> +    if (a >= b) {
>>> +        diff = a - b;
>>> +        if (diff == 0)
>>> +            return a + 3;
>>> +        else if (diff == 1)
>>> +            return a + 2;
>>> +        else if (diff == 2)
>>> +            return a + 1;
>>> +        return a;
>>> +    }
>>> +    else {
>>> +        diff = b - a;
>>> +        if (diff == 0)
>>> +            return b + 3;
>>> +        else if (diff == 1)
>>> +            return b + 2;
>>> +        else if (diff == 2)
>>> +            return b + 1;
>>> +        return b;
>>> +    }
>>> +}
>>> +
>>> +static int ath10k_sum_sigs(int p20, int e20, int e40, int e80) {
>>> +    /* Hacky attempt at summing dbm without resorting to log(10) business */
>>> +    if (e40 != 0x80) {
>>> +        return ath10k_sum_sigs_2(ath10k_sum_sigs_2(p20, e20), ath10k_sum_sigs_2(e40, e80));
>>> +    }
>>> +    else {
>>> +        return ath10k_sum_sigs_2(p20, e20);
>>> +    }
>>> +}
>>> +
>>>   static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>>>                      struct ieee80211_rx_status *status,
>>>                      struct htt_rx_desc *rxd)
>>> @@ -1177,18 +1215,32 @@ static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>>>           status->chains &= ~BIT(i);
>>>             if (rxd->ppdu_start.rssi_chains[i].pri20_mhz != 0x80) {
>>> -            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR +
>>> -                rxd->ppdu_start.rssi_chains[i].pri20_mhz;
>>> +            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR
>>> +                + ath10k_sum_sigs(rxd->ppdu_start.rssi_chains[i].pri20_mhz,
>>> + rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>>> + rxd->ppdu_start.rssi_chains[i].ext40_mhz,
>>> + rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>> +            //ath10k_warn(ar, "rx-h-sig, chain[%i] pri20: %d ext20: %d  ext40: %d  ext80: %d\n",
>>> +            //        i, rxd->ppdu_start.rssi_chains[i].pri20_mhz, rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>>> +            // rxd->ppdu_start.rssi_chains[i].ext40_mhz, rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>>                 status->chains |= BIT(i);
>>>           }
>>>       }
>>>         /* FIXME: Get real NF */
>>> -    status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>> -             rxd->ppdu_start.rssi_comb;
>>> -    /* ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x chain[0]: %d  chain[1]: %d  chan[2]: %d\n",
>>> -                       status->signal, status->chains, status->chain_signal[0], status->chain_signal[1], status->chain_signal[2]); */
>>> +    if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
>>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>> +            rxd->ppdu_start.rssi_comb_ht;
>>> +    }
>>> +    else {
>>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>> +            rxd->ppdu_start.rssi_comb;
>>> +    }
>>> +
>>> +    //ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x chain[0]: %d  chain[1]: %d  chain[2]: %d chain[3]: %d\n",
>>> +    //        status->signal, status->chains, status->chain_signal[0],
>>> +    //        status->chain_signal[1], status->chain_signal[2], status->chain_signal[3]);
>>>       status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
>>>   }
>>>   diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h b/drivers/net/wireless/ath/ath10k/rx_desc.h
>>> index dec1582005b9..6b44677474dd 100644
>>> --- a/drivers/net/wireless/ath/ath10k/rx_desc.h
>>> +++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
>>> @@ -726,7 +726,8 @@ struct rx_ppdu_start {
>>>           u8 ext80_mhz;
>>>       } rssi_chains[4];
>>>       u8 rssi_comb;
>>> -    __le16 rsvd0;
>>> +    u8 rsvd0; /* first two bits are bandwidth, other 6 are reserved */
>>> +    u8 rssi_comb_ht;
>>>       u8 info0; /* %RX_PPDU_START_INFO0_ */
>>>       __le32 info1; /* %RX_PPDU_START_INFO1_ */
>>>       __le32 info2; /* %RX_PPDU_START_INFO2_ */
>>
>> _______________________________________________
>> ath10k mailing list
>> ath10k@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/ath10k
>>
>

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Sebastian Gottschall]

>> currently debugging in your code, but i already have seen that the 
>> values are wrong now for this chipset
>
> Thanks for testing.  I'll add a check for 0 and ignore that value 
> too.  That seem OK?
i tested already the 0 check and it works
>
> Were the per-chain values OK?
on 9984 i see no disadvantage so far. seem to work and the values look 
sane. i will do a side by side comparisation later this day on 9984
>
> Thanks,
> Ben
>
>>>
>>> Am 16.12.2019 um 23:07 schrieb greearb@candelatech.com:
>>>> From: Ben Greear <greearb@candelatech.com>
>>>>
>>>> This makes per-chain RSSI be more consistent between HT20, HT40, HT80.
>>>> Instead of doing precise log math for adding dbm, I did a rough 
>>>> estimate,
>>>> it seems to work good enough.
>>>>
>>>> Tested on ath10k-ct 9984 firmware.
>>>>
>>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>>> ---
>>>>   drivers/net/wireless/ath/ath10k/htt_rx.c  | 64 
>>>> ++++++++++++++++++++---
>>>>   drivers/net/wireless/ath/ath10k/rx_desc.h |  3 +-
>>>>   2 files changed, 60 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
>>>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> index 13f652b622df..034d4ace228d 100644
>>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> @@ -1167,6 +1167,44 @@ static bool ath10k_htt_rx_h_channel(struct 
>>>> ath10k *ar,
>>>>       return true;
>>>>   }
>>>>   +static int ath10k_sum_sigs_2(int a, int b) {
>>>> +    int diff;
>>>> +
>>>> +    if (b == 0x80)
>>>> +        return a;
>>>> +
>>>> +    if (a >= b) {
>>>> +        diff = a - b;
>>>> +        if (diff == 0)
>>>> +            return a + 3;
>>>> +        else if (diff == 1)
>>>> +            return a + 2;
>>>> +        else if (diff == 2)
>>>> +            return a + 1;
>>>> +        return a;
>>>> +    }
>>>> +    else {
>>>> +        diff = b - a;
>>>> +        if (diff == 0)
>>>> +            return b + 3;
>>>> +        else if (diff == 1)
>>>> +            return b + 2;
>>>> +        else if (diff == 2)
>>>> +            return b + 1;
>>>> +        return b;
>>>> +    }
>>>> +}
>>>> +
>>>> +static int ath10k_sum_sigs(int p20, int e20, int e40, int e80) {
>>>> +    /* Hacky attempt at summing dbm without resorting to log(10) 
>>>> business */
>>>> +    if (e40 != 0x80) {
>>>> +        return ath10k_sum_sigs_2(ath10k_sum_sigs_2(p20, e20), 
>>>> ath10k_sum_sigs_2(e40, e80));
>>>> +    }
>>>> +    else {
>>>> +        return ath10k_sum_sigs_2(p20, e20);
>>>> +    }
>>>> +}
>>>> +
>>>>   static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>>>>                      struct ieee80211_rx_status *status,
>>>>                      struct htt_rx_desc *rxd)
>>>> @@ -1177,18 +1215,32 @@ static void ath10k_htt_rx_h_signal(struct 
>>>> ath10k *ar,
>>>>           status->chains &= ~BIT(i);
>>>>             if (rxd->ppdu_start.rssi_chains[i].pri20_mhz != 0x80) {
>>>> -            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR +
>>>> - rxd->ppdu_start.rssi_chains[i].pri20_mhz;
>>>> +            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR
>>>> +                + 
>>>> ath10k_sum_sigs(rxd->ppdu_start.rssi_chains[i].pri20_mhz,
>>>> + rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>>>> + rxd->ppdu_start.rssi_chains[i].ext40_mhz,
>>>> + rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>>> +            //ath10k_warn(ar, "rx-h-sig, chain[%i] pri20: %d 
>>>> ext20: %d  ext40: %d  ext80: %d\n",
>>>> +            //        i, rxd->ppdu_start.rssi_chains[i].pri20_mhz, 
>>>> rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>>>> +            // rxd->ppdu_start.rssi_chains[i].ext40_mhz, 
>>>> rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>>>                 status->chains |= BIT(i);
>>>>           }
>>>>       }
>>>>         /* FIXME: Get real NF */
>>>> -    status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>>> -             rxd->ppdu_start.rssi_comb;
>>>> -    /* ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x 
>>>> chain[0]: %d  chain[1]: %d  chan[2]: %d\n",
>>>> -                       status->signal, status->chains, 
>>>> status->chain_signal[0], status->chain_signal[1], 
>>>> status->chain_signal[2]); */
>>>> +    if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
>>>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>>> +            rxd->ppdu_start.rssi_comb_ht;
>>>> +    }
>>>> +    else {
>>>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>>> +            rxd->ppdu_start.rssi_comb;
>>>> +    }
>>>> +
>>>> +    //ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x 
>>>> chain[0]: %d  chain[1]: %d  chain[2]: %d chain[3]: %d\n",
>>>> +    //        status->signal, status->chains, 
>>>> status->chain_signal[0],
>>>> +    //        status->chain_signal[1], status->chain_signal[2], 
>>>> status->chain_signal[3]);
>>>>       status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
>>>>   }
>>>>   diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h 
>>>> b/drivers/net/wireless/ath/ath10k/rx_desc.h
>>>> index dec1582005b9..6b44677474dd 100644
>>>> --- a/drivers/net/wireless/ath/ath10k/rx_desc.h
>>>> +++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
>>>> @@ -726,7 +726,8 @@ struct rx_ppdu_start {
>>>>           u8 ext80_mhz;
>>>>       } rssi_chains[4];
>>>>       u8 rssi_comb;
>>>> -    __le16 rsvd0;
>>>> +    u8 rsvd0; /* first two bits are bandwidth, other 6 are 
>>>> reserved */
>>>> +    u8 rssi_comb_ht;
>>>>       u8 info0; /* %RX_PPDU_START_INFO0_ */
>>>>       __le32 info1; /* %RX_PPDU_START_INFO1_ */
>>>>       __le32 info2; /* %RX_PPDU_START_INFO2_ */
>>>
>>> _______________________________________________
>>> ath10k mailing list
>>> ath10k@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/ath10k
>>>
>>
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Justin Capella]

I believe someone recently submitted a patch that defined noise floors
per band (2/5).

Can't say I'm a fan of the hacky code, in particular the if/else for
min/max // maybe abs(a-b)?

if (e40 != 0x80) { // whats this case about?

Are there reasons to not use log?




On Tue, Dec 17, 2019 at 7:59 AM Sebastian Gottschall
<s.gottschall@newmedia-net.de> wrote:
>
>
> >> currently debugging in your code, but i already have seen that the
> >> values are wrong now for this chipset
> >
> > Thanks for testing.  I'll add a check for 0 and ignore that value
> > too.  That seem OK?
> i tested already the 0 check and it works
> >
> > Were the per-chain values OK?
> on 9984 i see no disadvantage so far. seem to work and the values look
> sane. i will do a side by side comparisation later this day on 9984
> >
> > Thanks,
> > Ben
> >
> >>>
> >>> Am 16.12.2019 um 23:07 schrieb greearb@candelatech.com:
> >>>> From: Ben Greear <greearb@candelatech.com>
> >>>>
> >>>> This makes per-chain RSSI be more consistent between HT20, HT40, HT80.
> >>>> Instead of doing precise log math for adding dbm, I did a rough
> >>>> estimate,
> >>>> it seems to work good enough.
> >>>>
> >>>> Tested on ath10k-ct 9984 firmware.
> >>>>
> >>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
> >>>> ---
> >>>>   drivers/net/wireless/ath/ath10k/htt_rx.c  | 64
> >>>> ++++++++++++++++++++---
> >>>>   drivers/net/wireless/ath/ath10k/rx_desc.h |  3 +-
> >>>>   2 files changed, 60 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
> >>>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
> >>>> index 13f652b622df..034d4ace228d 100644
> >>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> >>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> >>>> @@ -1167,6 +1167,44 @@ static bool ath10k_htt_rx_h_channel(struct
> >>>> ath10k *ar,
> >>>>       return true;
> >>>>   }
> >>>>   +static int ath10k_sum_sigs_2(int a, int b) {
> >>>> +    int diff;
> >>>> +
> >>>> +    if (b == 0x80)
> >>>> +        return a;
> >>>> +
> >>>> +    if (a >= b) {
> >>>> +        diff = a - b;
> >>>> +        if (diff == 0)
> >>>> +            return a + 3;
> >>>> +        else if (diff == 1)
> >>>> +            return a + 2;
> >>>> +        else if (diff == 2)
> >>>> +            return a + 1;
> >>>> +        return a;
> >>>> +    }
> >>>> +    else {
> >>>> +        diff = b - a;
> >>>> +        if (diff == 0)
> >>>> +            return b + 3;
> >>>> +        else if (diff == 1)
> >>>> +            return b + 2;
> >>>> +        else if (diff == 2)
> >>>> +            return b + 1;
> >>>> +        return b;
> >>>> +    }
> >>>> +}
> >>>> +
> >>>> +static int ath10k_sum_sigs(int p20, int e20, int e40, int e80) {
> >>>> +    /* Hacky attempt at summing dbm without resorting to log(10)
> >>>> business */
> >>>> +    if (e40 != 0x80) {
> >>>> +        return ath10k_sum_sigs_2(ath10k_sum_sigs_2(p20, e20),
> >>>> ath10k_sum_sigs_2(e40, e80));
> >>>> +    }
> >>>> +    else {
> >>>> +        return ath10k_sum_sigs_2(p20, e20);
> >>>> +    }
> >>>> +}
> >>>> +
> >>>>   static void ath10k_htt_rx_h_signal(struct ath10k *ar,
> >>>>                      struct ieee80211_rx_status *status,
> >>>>                      struct htt_rx_desc *rxd)
> >>>> @@ -1177,18 +1215,32 @@ static void ath10k_htt_rx_h_signal(struct
> >>>> ath10k *ar,
> >>>>           status->chains &= ~BIT(i);
> >>>>             if (rxd->ppdu_start.rssi_chains[i].pri20_mhz != 0x80) {
> >>>> -            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR +
> >>>> - rxd->ppdu_start.rssi_chains[i].pri20_mhz;
> >>>> +            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR
> >>>> +                +
> >>>> ath10k_sum_sigs(rxd->ppdu_start.rssi_chains[i].pri20_mhz,
> >>>> + rxd->ppdu_start.rssi_chains[i].ext20_mhz,
> >>>> + rxd->ppdu_start.rssi_chains[i].ext40_mhz,
> >>>> + rxd->ppdu_start.rssi_chains[i].ext80_mhz);
> >>>> +            //ath10k_warn(ar, "rx-h-sig, chain[%i] pri20: %d
> >>>> ext20: %d  ext40: %d  ext80: %d\n",
> >>>> +            //        i, rxd->ppdu_start.rssi_chains[i].pri20_mhz,
> >>>> rxd->ppdu_start.rssi_chains[i].ext20_mhz,
> >>>> +            // rxd->ppdu_start.rssi_chains[i].ext40_mhz,
> >>>> rxd->ppdu_start.rssi_chains[i].ext80_mhz);
> >>>>                 status->chains |= BIT(i);
> >>>>           }
> >>>>       }
> >>>>         /* FIXME: Get real NF */
> >>>> -    status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
> >>>> -             rxd->ppdu_start.rssi_comb;
> >>>> -    /* ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x
> >>>> chain[0]: %d  chain[1]: %d  chan[2]: %d\n",
> >>>> -                       status->signal, status->chains,
> >>>> status->chain_signal[0], status->chain_signal[1],
> >>>> status->chain_signal[2]); */
> >>>> +    if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
> >>>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
> >>>> +            rxd->ppdu_start.rssi_comb_ht;
> >>>> +    }
> >>>> +    else {
> >>>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
> >>>> +            rxd->ppdu_start.rssi_comb;
> >>>> +    }
> >>>> +
> >>>> +    //ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x
> >>>> chain[0]: %d  chain[1]: %d  chain[2]: %d chain[3]: %d\n",
> >>>> +    //        status->signal, status->chains,
> >>>> status->chain_signal[0],
> >>>> +    //        status->chain_signal[1], status->chain_signal[2],
> >>>> status->chain_signal[3]);
> >>>>       status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
> >>>>   }
> >>>>   diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h
> >>>> b/drivers/net/wireless/ath/ath10k/rx_desc.h
> >>>> index dec1582005b9..6b44677474dd 100644
> >>>> --- a/drivers/net/wireless/ath/ath10k/rx_desc.h
> >>>> +++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
> >>>> @@ -726,7 +726,8 @@ struct rx_ppdu_start {
> >>>>           u8 ext80_mhz;
> >>>>       } rssi_chains[4];
> >>>>       u8 rssi_comb;
> >>>> -    __le16 rsvd0;
> >>>> +    u8 rsvd0; /* first two bits are bandwidth, other 6 are
> >>>> reserved */
> >>>> +    u8 rssi_comb_ht;
> >>>>       u8 info0; /* %RX_PPDU_START_INFO0_ */
> >>>>       __le32 info1; /* %RX_PPDU_START_INFO1_ */
> >>>>       __le32 info2; /* %RX_PPDU_START_INFO2_ */
> >>>
> >>> _______________________________________________
> >>> ath10k mailing list
> >>> ath10k@lists.infradead.org
> >>> http://lists.infradead.org/mailman/listinfo/ath10k
> >>>
> >>
> >

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Ben Greear]

On 12/17/19 8:23 AM, Justin Capella wrote:
> I believe someone recently submitted a patch that defined noise floors
> per band (2/5).

I looked at using the real noise floor.  Our radio was reporting a noise floor of around -102,
where the hard-coded default is -95.  This of course would make the reported RSSI lower by 7db
in that case.  I am not sure that is correct.

If this were to be implemented that way, then the firmware would have to be queried for
the noise floor in a better way than it is currently done.  So, I am not planning to work on
that soon.

Someone could post-process RSSI based on the reported noise floor if they want to adjust
the values in user-space, for isntance.

> Can't say I'm a fan of the hacky code, in particular the if/else for
> min/max // maybe abs(a-b)?

I like open coded stuff.  I'm more concerned that maybe the math could
be improved, but it seems to work pretty well in our testing.

Either way, please comment inline so that it is more obvious exactly
what code you are talking about.

> 
> if (e40 != 0x80) { // whats this case about?

0x80 means 'value is not valid'.  I can add a comment about that.

> 
> Are there reasons to not use log?

I don't want to use log in the rx path, it would very likely decrease
rx performance, especially on lower powered systems.


Thanks,
Ben

> 
> 
> 
> 
> On Tue, Dec 17, 2019 at 7:59 AM Sebastian Gottschall
> <s.gottschall@newmedia-net.de> wrote:
>>
>>
>>>> currently debugging in your code, but i already have seen that the
>>>> values are wrong now for this chipset
>>>
>>> Thanks for testing.  I'll add a check for 0 and ignore that value
>>> too.  That seem OK?
>> i tested already the 0 check and it works
>>>
>>> Were the per-chain values OK?
>> on 9984 i see no disadvantage so far. seem to work and the values look
>> sane. i will do a side by side comparisation later this day on 9984
>>>
>>> Thanks,
>>> Ben
>>>
>>>>>
>>>>> Am 16.12.2019 um 23:07 schrieb greearb@candelatech.com:
>>>>>> From: Ben Greear <greearb@candelatech.com>
>>>>>>
>>>>>> This makes per-chain RSSI be more consistent between HT20, HT40, HT80.
>>>>>> Instead of doing precise log math for adding dbm, I did a rough
>>>>>> estimate,
>>>>>> it seems to work good enough.
>>>>>>
>>>>>> Tested on ath10k-ct 9984 firmware.
>>>>>>
>>>>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>>>>> ---
>>>>>>    drivers/net/wireless/ath/ath10k/htt_rx.c  | 64
>>>>>> ++++++++++++++++++++---
>>>>>>    drivers/net/wireless/ath/ath10k/rx_desc.h |  3 +-
>>>>>>    2 files changed, 60 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>>>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>>>> index 13f652b622df..034d4ace228d 100644
>>>>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>>>> @@ -1167,6 +1167,44 @@ static bool ath10k_htt_rx_h_channel(struct
>>>>>> ath10k *ar,
>>>>>>        return true;
>>>>>>    }
>>>>>>    +static int ath10k_sum_sigs_2(int a, int b) {
>>>>>> +    int diff;
>>>>>> +
>>>>>> +    if (b == 0x80)
>>>>>> +        return a;
>>>>>> +
>>>>>> +    if (a >= b) {
>>>>>> +        diff = a - b;
>>>>>> +        if (diff == 0)
>>>>>> +            return a + 3;
>>>>>> +        else if (diff == 1)
>>>>>> +            return a + 2;
>>>>>> +        else if (diff == 2)
>>>>>> +            return a + 1;
>>>>>> +        return a;
>>>>>> +    }
>>>>>> +    else {
>>>>>> +        diff = b - a;
>>>>>> +        if (diff == 0)
>>>>>> +            return b + 3;
>>>>>> +        else if (diff == 1)
>>>>>> +            return b + 2;
>>>>>> +        else if (diff == 2)
>>>>>> +            return b + 1;
>>>>>> +        return b;
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +static int ath10k_sum_sigs(int p20, int e20, int e40, int e80) {
>>>>>> +    /* Hacky attempt at summing dbm without resorting to log(10)
>>>>>> business */
>>>>>> +    if (e40 != 0x80) {
>>>>>> +        return ath10k_sum_sigs_2(ath10k_sum_sigs_2(p20, e20),
>>>>>> ath10k_sum_sigs_2(e40, e80));
>>>>>> +    }
>>>>>> +    else {
>>>>>> +        return ath10k_sum_sigs_2(p20, e20);
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>>    static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>>>>>>                       struct ieee80211_rx_status *status,
>>>>>>                       struct htt_rx_desc *rxd)
>>>>>> @@ -1177,18 +1215,32 @@ static void ath10k_htt_rx_h_signal(struct
>>>>>> ath10k *ar,
>>>>>>            status->chains &= ~BIT(i);
>>>>>>              if (rxd->ppdu_start.rssi_chains[i].pri20_mhz != 0x80) {
>>>>>> -            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR +
>>>>>> - rxd->ppdu_start.rssi_chains[i].pri20_mhz;
>>>>>> +            status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR
>>>>>> +                +
>>>>>> ath10k_sum_sigs(rxd->ppdu_start.rssi_chains[i].pri20_mhz,
>>>>>> + rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>>>>>> + rxd->ppdu_start.rssi_chains[i].ext40_mhz,
>>>>>> + rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>>>>> +            //ath10k_warn(ar, "rx-h-sig, chain[%i] pri20: %d
>>>>>> ext20: %d  ext40: %d  ext80: %d\n",
>>>>>> +            //        i, rxd->ppdu_start.rssi_chains[i].pri20_mhz,
>>>>>> rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>>>>>> +            // rxd->ppdu_start.rssi_chains[i].ext40_mhz,
>>>>>> rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>>>>>                  status->chains |= BIT(i);
>>>>>>            }
>>>>>>        }
>>>>>>          /* FIXME: Get real NF */
>>>>>> -    status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>>>>> -             rxd->ppdu_start.rssi_comb;
>>>>>> -    /* ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x
>>>>>> chain[0]: %d  chain[1]: %d  chan[2]: %d\n",
>>>>>> -                       status->signal, status->chains,
>>>>>> status->chain_signal[0], status->chain_signal[1],
>>>>>> status->chain_signal[2]); */
>>>>>> +    if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
>>>>>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>>>>> +            rxd->ppdu_start.rssi_comb_ht;
>>>>>> +    }
>>>>>> +    else {
>>>>>> +        status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>>>>> +            rxd->ppdu_start.rssi_comb;
>>>>>> +    }
>>>>>> +
>>>>>> +    //ath10k_warn(ar, "rx-h-sig, signal: %d  chains: 0x%x
>>>>>> chain[0]: %d  chain[1]: %d  chain[2]: %d chain[3]: %d\n",
>>>>>> +    //        status->signal, status->chains,
>>>>>> status->chain_signal[0],
>>>>>> +    //        status->chain_signal[1], status->chain_signal[2],
>>>>>> status->chain_signal[3]);
>>>>>>        status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
>>>>>>    }
>>>>>>    diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h
>>>>>> b/drivers/net/wireless/ath/ath10k/rx_desc.h
>>>>>> index dec1582005b9..6b44677474dd 100644
>>>>>> --- a/drivers/net/wireless/ath/ath10k/rx_desc.h
>>>>>> +++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
>>>>>> @@ -726,7 +726,8 @@ struct rx_ppdu_start {
>>>>>>            u8 ext80_mhz;
>>>>>>        } rssi_chains[4];
>>>>>>        u8 rssi_comb;
>>>>>> -    __le16 rsvd0;
>>>>>> +    u8 rsvd0; /* first two bits are bandwidth, other 6 are
>>>>>> reserved */
>>>>>> +    u8 rssi_comb_ht;
>>>>>>        u8 info0; /* %RX_PPDU_START_INFO0_ */
>>>>>>        __le32 info1; /* %RX_PPDU_START_INFO1_ */
>>>>>>        __le32 info2; /* %RX_PPDU_START_INFO2_ */
>>>>>
>>>>> _______________________________________________
>>>>> ath10k mailing list
>>>>> ath10k@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/ath10k
>>>>>
>>>>
>>>
> 
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Tom Psyborg]

On 17/12/2019, Ben Greear <greearb@candelatech.com> wrote:
> On 12/17/19 8:23 AM, Justin Capella wrote:
>> I believe someone recently submitted a patch that defined noise floors
>> per band (2/5).
>
> I looked at using the real noise floor.  Our radio was reporting a noise
> floor of around -102,
> where the hard-coded default is -95.  This of course would make the reported
> RSSI lower by 7db
> in that case.  I am not sure that is correct.
>

Hi

I am getting similar NF values with all my ath10k devices, I thought
default was changed since ath9k from -95 to -115 just like in the
vendor driver? There were some discussions about it on mailing list.
On some channels (5Ghz) the value goes down to about -107, even saw
-110 once.

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Adrian Chadd]

On Tue, 17 Dec 2019 at 10:29, Tom Psyborg <pozega.tomislav@gmail.com> wrote:
>
> On 17/12/2019, Ben Greear <greearb@candelatech.com> wrote:
> > On 12/17/19 8:23 AM, Justin Capella wrote:
> >> I believe someone recently submitted a patch that defined noise floors
> >> per band (2/5).
> >
> > I looked at using the real noise floor.  Our radio was reporting a noise
> > floor of around -102,
> > where the hard-coded default is -95.  This of course would make the reported
> > RSSI lower by 7db
> > in that case.  I am not sure that is correct.
> >
>
> Hi
>
> I am getting similar NF values with all my ath10k devices, I thought
> default was changed since ath9k from -95 to -115 just like in the
> vendor driver? There were some discussions about it on mailing list.
> On some channels (5Ghz) the value goes down to about -107, even saw
> -110 once.

IIRC they're /not/ calibrated dBm values. They're just internal noise
floor calibration values derived from the internal ADC reads when you
kick off a NF cal. Then everything is relative to this NF value.

This is one of the many, many reasons things get gunked up when you
start seeing higher noise floor values; since a lot of things
internally are/were related to what the currently calibrated NF value
is, a too-high NF sample value would result in quite exciting things
in the PHY like "this channel never drops below CCA"...




-adrian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Ben Greear]

On 12/17/19 10:29 AM, Tom Psyborg wrote:
> On 17/12/2019, Ben Greear <greearb@candelatech.com> wrote:
>> On 12/17/19 8:23 AM, Justin Capella wrote:
>>> I believe someone recently submitted a patch that defined noise floors
>>> per band (2/5).
>>
>> I looked at using the real noise floor.  Our radio was reporting a noise
>> floor of around -102,
>> where the hard-coded default is -95.  This of course would make the reported
>> RSSI lower by 7db
>> in that case.  I am not sure that is correct.
>>
> 
> Hi
> 
> I am getting similar NF values with all my ath10k devices, I thought
> default was changed since ath9k from -95 to -115 just like in the
> vendor driver? There were some discussions about it on mailing list.
> On some channels (5Ghz) the value goes down to about -107, even saw
> -110 once.
> 

If you use ath9k and ath10k on same channel/environment, do you see similar
RSSI reported (especially with the ath10k patch I just posted)?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Tom Psyborg]

On 17/12/2019, Ben Greear <greearb@candelatech.com> wrote:
> On 12/17/19 10:29 AM, Tom Psyborg wrote:
>> On 17/12/2019, Ben Greear <greearb@candelatech.com> wrote:
>>> On 12/17/19 8:23 AM, Justin Capella wrote:
>>>> I believe someone recently submitted a patch that defined noise floors
>>>> per band (2/5).
>>>
>>> I looked at using the real noise floor.  Our radio was reporting a noise
>>> floor of around -102,
>>> where the hard-coded default is -95.  This of course would make the
>>> reported
>>> RSSI lower by 7db
>>> in that case.  I am not sure that is correct.
>>>
>>
>> Hi
>>
>> I am getting similar NF values with all my ath10k devices, I thought
>> default was changed since ath9k from -95 to -115 just like in the
>> vendor driver? There were some discussions about it on mailing list.
>> On some channels (5Ghz) the value goes down to about -107, even saw
>> -110 once.
>>
>
> If you use ath9k and ath10k on same channel/environment, do you see similar
> RSSI reported (especially with the ath10k patch I just posted)?
>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>
>

Nope. RSSI values are quite different between two cards. Applying this
patch also made no difference on ath10k card, but this might be due to
the fact the network wasn't setup (so no data passed through) in order
to keep tx rx rates at 6 Mbps.

First I put AR9462 card in Archer C7 and connect to Litebeam5AC (CH36,VHT80)

commands output from archer:

iwinfo:

signal: -65dBm noise: -94dBm txpower: 14dBm

iw wlan1 station dump:

signal: -65 [-69, -67] dBm
signal avg: -65 [-68, -67] dBm

with the default 14dBm power the RSSI in Litebeam was -72dBm

Then I put QCA9880 in archer and connected to the same AP:

commands output from archer:

iwinfo:

signal: -58dBm noise: -102dBm txpower: 23dBm

iw wlan0 station dump:

signal: -59 [-65, -64, -62] dBm
signal avg: -58 [-65, -63, -62] dBm

RSSI of the card reported in Litebeam -63dBm (very inconsistent
between wifi restarts on archer, -65, -67 even -69 dBm, and no
software reboot would restore card's full power, only cold boot)

next I lowered the power of the card to 14dBm to match AR9462

iwinfo:

signal: -57 dBm noise: -101 dBm txpower: 14 dBm

iw wlan0 station dump

signal: -57 [-62, -62, -61] dBm
signal avg: -57 [-62, -62, -61] dBm

now the RSSI in litebeam dropped to approx. -75dBm

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Tom Psyborg]

also noticed now that the noise floor changes with signal strength as
described in this bug report:
https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html

after wifi restart

iwinfo:

signal: -59dBm noise: -108dBm

then goes to

signal: -52dBm noise: -103dBm

and finally drops to

signal: -59dBm noise: -103dBm

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Ben Greear]

On 12/17/19 3:37 PM, Tom Psyborg wrote:
> also noticed now that the noise floor changes with signal strength as
> described in this bug report:
> https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html
> 
> after wifi restart
> 
> iwinfo:
> 
> signal: -59dBm noise: -108dBm
> 
> then goes to
> 
> signal: -52dBm noise: -103dBm
> 
> and finally drops to
> 
> signal: -59dBm noise: -103dBm
> 

The problem with debugging this sort of stuff is that you need an RF scope
to determine whether signal power of transmitter is changing or receiver
is reporting stuff weirdly.

If you are comparing against ath9k, probably you need to force your ath10k station to do /n only
(or change your AP to do /n only) so that you can be comparing similar MCS rates.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Sebastian Gottschall]

i dont know what you want to compare here.

1. you compare 2 different wifi chipsets. both have different 
sensititivy and overall output power spec

2. both have different amount of antenna chains. which does make a 
difference in input sensitivity

3. the patch ben made has no effect on qca9880 chipsets. it only takes 
effect on 10.4 based chipsets like 9984


about noise floors in general. noise floors of -108 are bogus. there is 
a physical limit a noise level can be.
since drivers like ath9k are doing a cyclic calibration, the noise value 
might indeed change. but this calibration is
not running in realtime. its cyclic. i'm not aware if chipsets like 
qca988x are going the same way, but since qca988x
has sime similaries with ath9k chipsets unlike the newer 9984 variants, 
it could be. the 30 seconds mentioned
in the bug report fits to my expectations of the early noisefloor 
calibration which has a short delay and after success
turning to use a long delay. anyway. in this early calibration phase 
signals might change and will stabilize after. this isnt a issue
since your connection will work anyway even if it might take a little 
bit longer if you have poor signal levels

@ben. am i wrong or what do think?

Sebastian

Am 18.12.2019 um 00:37 schrieb Tom Psyborg:
> also noticed now that the noise floor changes with signal strength as
> described in this bug report:
> https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html
>
> after wifi restart
>
> iwinfo:
>
> signal: -59dBm noise: -108dBm
>
> then goes to
>
> signal: -52dBm noise: -103dBm
>
> and finally drops to
>
> signal: -59dBm noise: -103dBm
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Ben Greear]

On 12/17/2019 06:12 PM, Sebastian Gottschall wrote:
> i dont know what you want to compare here.
>
> 1. you compare 2 different wifi chipsets. both have different sensititivy and overall output power spec
>
> 2. both have different amount of antenna chains. which does make a difference in input sensitivity
>
> 3. the patch ben made has no effect on qca9880 chipsets. it only takes effect on 10.4 based chipsets like 9984

The part of my patch that sums secondary frequencies should apply to wave-1 as well, but I have
not verified that yet.


> about noise floors in general. noise floors of -108 are bogus. there is a physical limit a noise level can be.
> since drivers like ath9k are doing a cyclic calibration, the noise value might indeed change. but this calibration is
> not running in realtime. its cyclic. i'm not aware if chipsets like qca988x are going the same way, but since qca988x
> has sime similaries with ath9k chipsets unlike the newer 9984 variants, it could be. the 30 seconds mentioned
> in the bug report fits to my expectations of the early noisefloor calibration which has a short delay and after success
> turning to use a long delay. anyway. in this early calibration phase signals might change and will stabilize after. this isnt a issue
> since your connection will work anyway even if it might take a little bit longer if you have poor signal levels
>
> @ben. am i wrong or what do think?

I don't know enough about how the noise floor calculations are done or how the apply to settings
to know the answer.

I will be happy in general if ath10k wave-1, wave-2, and ath9k report similar RSSI for similar
setups.

If you look at the tx-rate-power table in ath10k, for instance, you can see different MCS are transmitted
at different signal levels.  So, some change from initial conditions might be because higher MCS is
being transmitted after rate-ctrl scales up?

Lots of moving parts...

Thanks,
Ben

>
> Sebastian
>
> Am 18.12.2019 um 00:37 schrieb Tom Psyborg:
>> also noticed now that the noise floor changes with signal strength as
>> described in this bug report:
>> https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html
>>
>> after wifi restart
>>
>> iwinfo:
>>
>> signal: -59dBm noise: -108dBm
>>
>> then goes to
>>
>> signal: -52dBm noise: -103dBm
>>
>> and finally drops to
>>
>> signal: -59dBm noise: -103dBm
>>
>

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Sebastian Gottschall]

Am 18.12.2019 um 03:37 schrieb Ben Greear:
>
>
> On 12/17/2019 06:12 PM, Sebastian Gottschall wrote:
>> i dont know what you want to compare here.
>>
>> 1. you compare 2 different wifi chipsets. both have different 
>> sensititivy and overall output power spec
>>
>> 2. both have different amount of antenna chains. which does make a 
>> difference in input sensitivity
>>
>> 3. the patch ben made has no effect on qca9880 chipsets. it only 
>> takes effect on 10.4 based chipsets like 9984
>
> The part of my patch that sums secondary frequencies should apply to 
> wave-1 as well, but I have
> not verified that yet.
yeah. right. sorry i was just looking at total signal sum which uses 
rssi_comb_ht
>
>
>> about noise floors in general. noise floors of -108 are bogus. there 
>> is a physical limit a noise level can be.
>> since drivers like ath9k are doing a cyclic calibration, the noise 
>> value might indeed change. but this calibration is
>> not running in realtime. its cyclic. i'm not aware if chipsets like 
>> qca988x are going the same way, but since qca988x
>> has sime similaries with ath9k chipsets unlike the newer 9984 
>> variants, it could be. the 30 seconds mentioned
>> in the bug report fits to my expectations of the early noisefloor 
>> calibration which has a short delay and after success
>> turning to use a long delay. anyway. in this early calibration phase 
>> signals might change and will stabilize after. this isnt a issue
>> since your connection will work anyway even if it might take a little 
>> bit longer if you have poor signal levels
>>
>> @ben. am i wrong or what do think?
>
> I don't know enough about how the noise floor calculations are done or 
> how the apply to settings
> to know the answer.
>
> I will be happy in general if ath10k wave-1, wave-2, and ath9k report 
> similar RSSI for similar
> setups.
that will not work. you compare different chipsets and depending on the 
implementation by the card vendor
rf sensitivity can be very diffent. the same goes for output power. some 
vendors are using additional rf amps
for enhancing output power (ubiquiti is best example here). this these 
amps also may have influence to sensitivity.
on these cards you set 10 db output power, but in fact it outputs 18 db. 
so there is a bias offset on these cards or devices. (the offset is 
depending on the device model)

what you measure is what the chip receives, but not what was lost on the 
pcb layout. (or was even generated in case of noise)
and when it comes to calibration data. correct would be if each 
individual card is calibrated before shipment. in reality manufactures
are doing calibration on a single reference card and clone it on all 
following cards to save time. the result depends on day or week of 
production
and current position of the moon and sun. errors of +- 2 db are common 
here. (this is not a fact for all card or device vendors)

>
> If you look at the tx-rate-power table in ath10k, for instance, you 
> can see different MCS are transmitted
> at different signal levels.  So, some change from initial conditions 
> might be because higher MCS is
> being transmitted after rate-ctrl scales up?
yes. this is modulation related. as higher the rate goes as lower the 
power will be. thats princible of QAM.
and the rate control itself isnt signal but error rate based. so high 
packet loss triggers the rate control to lower the rate which results
in increased output power and vice versa. but as mentioned. at card 
startup a noise floor calibration starts which may succeed or fail.
if it succeeds it will turn into a long delay phase. so cyclic 
calibration. the calibration time is exactly 30 seconds (minimum) and if 
it fails it can
exceed to 60 seconds. after that time it will sleep for 300 seconds and 
will check for recalibration conditions. (there are rules like high 
noise floor changes etc.)
a recalibration is also triggered at channel changes  and if chipset 
temperature changes at a certain level.
from what i have seen the procedure in the qca9880 firmware is exactly 
the same as in ath9k.
anyway. while this calibration is running, the signal and noise floor 
might be unstable or even bogus until this is finished and rate control 
might not be optimal
under stress conditions like long range links with low signals. with 
standard wifi usage you should not notice it that much since signal to 
noise ratio is high enough anyway


>
> Lots of moving parts...
>
> Thanks,
> Ben
>
>>
>> Sebastian
>>
>> Am 18.12.2019 um 00:37 schrieb Tom Psyborg:
>>> also noticed now that the noise floor changes with signal strength as
>>> described in this bug report:
>>> https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html
>>>
>>> after wifi restart
>>>
>>> iwinfo:
>>>
>>> signal: -59dBm noise: -108dBm
>>>
>>> then goes to
>>>
>>> signal: -52dBm noise: -103dBm
>>>
>>> and finally drops to
>>>
>>> signal: -59dBm noise: -103dBm
>>>
>>
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Justin Capella]

Don't mean to steal your thread here, but since it's being discussed--
is there something that can be done to provide more accurate/precise
data? Use of the default is widespread so not a reason to hold back
the patch imo, but with a proposed pcap-ng capture information block
they would become more accessible and maybe there will be increased
interest in real values.

Anyway to fill out IEEE80211_RADIOTAP_DBM_ANT{SIGNAL,NOISE}?

I recall from another thread that there isn't currently periodic
calibration but the floor could change with environment too.

On Tue, Dec 17, 2019 at 8:05 PM Sebastian Gottschall
<s.gottschall@newmedia-net.de> wrote:
>
>
> Am 18.12.2019 um 03:37 schrieb Ben Greear:
> >
> >
> > On 12/17/2019 06:12 PM, Sebastian Gottschall wrote:
> >> i dont know what you want to compare here.
> >>
> >> 1. you compare 2 different wifi chipsets. both have different
> >> sensititivy and overall output power spec
> >>
> >> 2. both have different amount of antenna chains. which does make a
> >> difference in input sensitivity
> >>
> >> 3. the patch ben made has no effect on qca9880 chipsets. it only
> >> takes effect on 10.4 based chipsets like 9984
> >
> > The part of my patch that sums secondary frequencies should apply to
> > wave-1 as well, but I have
> > not verified that yet.
> yeah. right. sorry i was just looking at total signal sum which uses
> rssi_comb_ht
> >
> >
> >> about noise floors in general. noise floors of -108 are bogus. there
> >> is a physical limit a noise level can be.
> >> since drivers like ath9k are doing a cyclic calibration, the noise
> >> value might indeed change. but this calibration is
> >> not running in realtime. its cyclic. i'm not aware if chipsets like
> >> qca988x are going the same way, but since qca988x
> >> has sime similaries with ath9k chipsets unlike the newer 9984
> >> variants, it could be. the 30 seconds mentioned
> >> in the bug report fits to my expectations of the early noisefloor
> >> calibration which has a short delay and after success
> >> turning to use a long delay. anyway. in this early calibration phase
> >> signals might change and will stabilize after. this isnt a issue
> >> since your connection will work anyway even if it might take a little
> >> bit longer if you have poor signal levels
> >>
> >> @ben. am i wrong or what do think?
> >
> > I don't know enough about how the noise floor calculations are done or
> > how the apply to settings
> > to know the answer.
> >
> > I will be happy in general if ath10k wave-1, wave-2, and ath9k report
> > similar RSSI for similar
> > setups.
> that will not work. you compare different chipsets and depending on the
> implementation by the card vendor
> rf sensitivity can be very diffent. the same goes for output power. some
> vendors are using additional rf amps
> for enhancing output power (ubiquiti is best example here). this these
> amps also may have influence to sensitivity.
> on these cards you set 10 db output power, but in fact it outputs 18 db.
> so there is a bias offset on these cards or devices. (the offset is
> depending on the device model)
>
> what you measure is what the chip receives, but not what was lost on the
> pcb layout. (or was even generated in case of noise)
> and when it comes to calibration data. correct would be if each
> individual card is calibrated before shipment. in reality manufactures
> are doing calibration on a single reference card and clone it on all
> following cards to save time. the result depends on day or week of
> production
> and current position of the moon and sun. errors of +- 2 db are common
> here. (this is not a fact for all card or device vendors)
>
> >
> > If you look at the tx-rate-power table in ath10k, for instance, you
> > can see different MCS are transmitted
> > at different signal levels.  So, some change from initial conditions
> > might be because higher MCS is
> > being transmitted after rate-ctrl scales up?
> yes. this is modulation related. as higher the rate goes as lower the
> power will be. thats princible of QAM.
> and the rate control itself isnt signal but error rate based. so high
> packet loss triggers the rate control to lower the rate which results
> in increased output power and vice versa. but as mentioned. at card
> startup a noise floor calibration starts which may succeed or fail.
> if it succeeds it will turn into a long delay phase. so cyclic
> calibration. the calibration time is exactly 30 seconds (minimum) and if
> it fails it can
> exceed to 60 seconds. after that time it will sleep for 300 seconds and
> will check for recalibration conditions. (there are rules like high
> noise floor changes etc.)
> a recalibration is also triggered at channel changes  and if chipset
> temperature changes at a certain level.
> from what i have seen the procedure in the qca9880 firmware is exactly
> the same as in ath9k.
> anyway. while this calibration is running, the signal and noise floor
> might be unstable or even bogus until this is finished and rate control
> might not be optimal
> under stress conditions like long range links with low signals. with
> standard wifi usage you should not notice it that much since signal to
> noise ratio is high enough anyway
>
>
> >
> > Lots of moving parts...
> >
> > Thanks,
> > Ben
> >
> >>
> >> Sebastian
> >>
> >> Am 18.12.2019 um 00:37 schrieb Tom Psyborg:
> >>> also noticed now that the noise floor changes with signal strength as
> >>> described in this bug report:
> >>> https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html
> >>>
> >>> after wifi restart
> >>>
> >>> iwinfo:
> >>>
> >>> signal: -59dBm noise: -108dBm
> >>>
> >>> then goes to
> >>>
> >>> signal: -52dBm noise: -103dBm
> >>>
> >>> and finally drops to
> >>>
> >>> signal: -59dBm noise: -103dBm
> >>>
> >>
> >

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Sebastian Gottschall]

Am 18.12.2019 um 09:05 schrieb Justin Capella:
> Don't mean to steal your thread here, but since it's being discussed--
> is there something that can be done to provide more accurate/precise
> data? Use of the default is widespread so not a reason to hold back
> the patch imo, but with a proposed pcap-ng capture information block
> they would become more accessible and maybe there will be increased
> interest in real values.
>
> Anyway to fill out IEEE80211_RADIOTAP_DBM_ANT{SIGNAL,NOISE}?
>
> I recall from another thread that there isn't currently periodic
> calibration but the floor could change with environment too.
there is periodic calibration. this is done by the chipset firmware 
according to my
review of the firmware code last night. without periodic calibration the 
chipset is likelly to turn
unstable over time and may also hang. the periodic calibration is 
triggerd by temperature changes
(if change is more than 30 degrees celsius) and various baseband hanging 
checks. in addition its still periodic
thats the case for 10.4 and also for 10.2 based firmwares. (i checked 
qca998x and qca9984 firmware codes)
>
> On Tue, Dec 17, 2019 at 8:05 PM Sebastian Gottschall
> <s.gottschall@newmedia-net.de> wrote:
>>
>> Am 18.12.2019 um 03:37 schrieb Ben Greear:
>>>
>>> On 12/17/2019 06:12 PM, Sebastian Gottschall wrote:
>>>> i dont know what you want to compare here.
>>>>
>>>> 1. you compare 2 different wifi chipsets. both have different
>>>> sensititivy and overall output power spec
>>>>
>>>> 2. both have different amount of antenna chains. which does make a
>>>> difference in input sensitivity
>>>>
>>>> 3. the patch ben made has no effect on qca9880 chipsets. it only
>>>> takes effect on 10.4 based chipsets like 9984
>>> The part of my patch that sums secondary frequencies should apply to
>>> wave-1 as well, but I have
>>> not verified that yet.
>> yeah. right. sorry i was just looking at total signal sum which uses
>> rssi_comb_ht
>>>
>>>> about noise floors in general. noise floors of -108 are bogus. there
>>>> is a physical limit a noise level can be.
>>>> since drivers like ath9k are doing a cyclic calibration, the noise
>>>> value might indeed change. but this calibration is
>>>> not running in realtime. its cyclic. i'm not aware if chipsets like
>>>> qca988x are going the same way, but since qca988x
>>>> has sime similaries with ath9k chipsets unlike the newer 9984
>>>> variants, it could be. the 30 seconds mentioned
>>>> in the bug report fits to my expectations of the early noisefloor
>>>> calibration which has a short delay and after success
>>>> turning to use a long delay. anyway. in this early calibration phase
>>>> signals might change and will stabilize after. this isnt a issue
>>>> since your connection will work anyway even if it might take a little
>>>> bit longer if you have poor signal levels
>>>>
>>>> @ben. am i wrong or what do think?
>>> I don't know enough about how the noise floor calculations are done or
>>> how the apply to settings
>>> to know the answer.
>>>
>>> I will be happy in general if ath10k wave-1, wave-2, and ath9k report
>>> similar RSSI for similar
>>> setups.
>> that will not work. you compare different chipsets and depending on the
>> implementation by the card vendor
>> rf sensitivity can be very diffent. the same goes for output power. some
>> vendors are using additional rf amps
>> for enhancing output power (ubiquiti is best example here). this these
>> amps also may have influence to sensitivity.
>> on these cards you set 10 db output power, but in fact it outputs 18 db.
>> so there is a bias offset on these cards or devices. (the offset is
>> depending on the device model)
>>
>> what you measure is what the chip receives, but not what was lost on the
>> pcb layout. (or was even generated in case of noise)
>> and when it comes to calibration data. correct would be if each
>> individual card is calibrated before shipment. in reality manufactures
>> are doing calibration on a single reference card and clone it on all
>> following cards to save time. the result depends on day or week of
>> production
>> and current position of the moon and sun. errors of +- 2 db are common
>> here. (this is not a fact for all card or device vendors)
>>
>>> If you look at the tx-rate-power table in ath10k, for instance, you
>>> can see different MCS are transmitted
>>> at different signal levels.  So, some change from initial conditions
>>> might be because higher MCS is
>>> being transmitted after rate-ctrl scales up?
>> yes. this is modulation related. as higher the rate goes as lower the
>> power will be. thats princible of QAM.
>> and the rate control itself isnt signal but error rate based. so high
>> packet loss triggers the rate control to lower the rate which results
>> in increased output power and vice versa. but as mentioned. at card
>> startup a noise floor calibration starts which may succeed or fail.
>> if it succeeds it will turn into a long delay phase. so cyclic
>> calibration. the calibration time is exactly 30 seconds (minimum) and if
>> it fails it can
>> exceed to 60 seconds. after that time it will sleep for 300 seconds and
>> will check for recalibration conditions. (there are rules like high
>> noise floor changes etc.)
>> a recalibration is also triggered at channel changes  and if chipset
>> temperature changes at a certain level.
>> from what i have seen the procedure in the qca9880 firmware is exactly
>> the same as in ath9k.
>> anyway. while this calibration is running, the signal and noise floor
>> might be unstable or even bogus until this is finished and rate control
>> might not be optimal
>> under stress conditions like long range links with low signals. with
>> standard wifi usage you should not notice it that much since signal to
>> noise ratio is high enough anyway
>>
>>
>>> Lots of moving parts...
>>>
>>> Thanks,
>>> Ben
>>>
>>>> Sebastian
>>>>
>>>> Am 18.12.2019 um 00:37 schrieb Tom Psyborg:
>>>>> also noticed now that the noise floor changes with signal strength as
>>>>> described in this bug report:
>>>>> https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html
>>>>>
>>>>> after wifi restart
>>>>>
>>>>> iwinfo:
>>>>>
>>>>> signal: -59dBm noise: -108dBm
>>>>>
>>>>> then goes to
>>>>>
>>>>> signal: -52dBm noise: -103dBm
>>>>>
>>>>> and finally drops to
>>>>>
>>>>> signal: -59dBm noise: -103dBm
>>>>>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Ben Greear]

On 12/18/2019 12:05 AM, Justin Capella wrote:
> Don't mean to steal your thread here, but since it's being discussed--
> is there something that can be done to provide more accurate/precise
> data? Use of the default is widespread so not a reason to hold back
> the patch imo, but with a proposed pcap-ng capture information block
> they would become more accessible and maybe there will be increased
> interest in real values.

It would take some large effort up and down the stack, but we could potentially
report the raw data for the secondary frequencies.  Probably that is of so little
use for the general user that it is not worth the effort.

You could just uncomment the printk in my patch if you are curious, or perhaps
add some debugfs API if you wanted to get at lots of data with run-time config
change.

>
> Anyway to fill out IEEE80211_RADIOTAP_DBM_ANT{SIGNAL,NOISE}?

Per-antenna rssi is already in wireshark capture for ath10k-ct.  I'm pretty
sure it is working in upstream ath10k too.

> I recall from another thread that there isn't currently periodic
> calibration but the floor could change with environment too.

I don't think it is correct to say periodic calibration does not happen with
ath10k.  Maybe very old wave-1 firmware has some issues, but recent stuff appears
to work.  I do see reported noise floor changing on 9984.

Thanks,
Ben

>
> On Tue, Dec 17, 2019 at 8:05 PM Sebastian Gottschall
> <s.gottschall@newmedia-net.de> wrote:
>>
>>
>> Am 18.12.2019 um 03:37 schrieb Ben Greear:
>>>
>>>
>>> On 12/17/2019 06:12 PM, Sebastian Gottschall wrote:
>>>> i dont know what you want to compare here.
>>>>
>>>> 1. you compare 2 different wifi chipsets. both have different
>>>> sensititivy and overall output power spec
>>>>
>>>> 2. both have different amount of antenna chains. which does make a
>>>> difference in input sensitivity
>>>>
>>>> 3. the patch ben made has no effect on qca9880 chipsets. it only
>>>> takes effect on 10.4 based chipsets like 9984
>>>
>>> The part of my patch that sums secondary frequencies should apply to
>>> wave-1 as well, but I have
>>> not verified that yet.
>> yeah. right. sorry i was just looking at total signal sum which uses
>> rssi_comb_ht
>>>
>>>
>>>> about noise floors in general. noise floors of -108 are bogus. there
>>>> is a physical limit a noise level can be.
>>>> since drivers like ath9k are doing a cyclic calibration, the noise
>>>> value might indeed change. but this calibration is
>>>> not running in realtime. its cyclic. i'm not aware if chipsets like
>>>> qca988x are going the same way, but since qca988x
>>>> has sime similaries with ath9k chipsets unlike the newer 9984
>>>> variants, it could be. the 30 seconds mentioned
>>>> in the bug report fits to my expectations of the early noisefloor
>>>> calibration which has a short delay and after success
>>>> turning to use a long delay. anyway. in this early calibration phase
>>>> signals might change and will stabilize after. this isnt a issue
>>>> since your connection will work anyway even if it might take a little
>>>> bit longer if you have poor signal levels
>>>>
>>>> @ben. am i wrong or what do think?
>>>
>>> I don't know enough about how the noise floor calculations are done or
>>> how the apply to settings
>>> to know the answer.
>>>
>>> I will be happy in general if ath10k wave-1, wave-2, and ath9k report
>>> similar RSSI for similar
>>> setups.
>> that will not work. you compare different chipsets and depending on the
>> implementation by the card vendor
>> rf sensitivity can be very diffent. the same goes for output power. some
>> vendors are using additional rf amps
>> for enhancing output power (ubiquiti is best example here). this these
>> amps also may have influence to sensitivity.
>> on these cards you set 10 db output power, but in fact it outputs 18 db.
>> so there is a bias offset on these cards or devices. (the offset is
>> depending on the device model)
>>
>> what you measure is what the chip receives, but not what was lost on the
>> pcb layout. (or was even generated in case of noise)
>> and when it comes to calibration data. correct would be if each
>> individual card is calibrated before shipment. in reality manufactures
>> are doing calibration on a single reference card and clone it on all
>> following cards to save time. the result depends on day or week of
>> production
>> and current position of the moon and sun. errors of +- 2 db are common
>> here. (this is not a fact for all card or device vendors)
>>
>>>
>>> If you look at the tx-rate-power table in ath10k, for instance, you
>>> can see different MCS are transmitted
>>> at different signal levels.  So, some change from initial conditions
>>> might be because higher MCS is
>>> being transmitted after rate-ctrl scales up?
>> yes. this is modulation related. as higher the rate goes as lower the
>> power will be. thats princible of QAM.
>> and the rate control itself isnt signal but error rate based. so high
>> packet loss triggers the rate control to lower the rate which results
>> in increased output power and vice versa. but as mentioned. at card
>> startup a noise floor calibration starts which may succeed or fail.
>> if it succeeds it will turn into a long delay phase. so cyclic
>> calibration. the calibration time is exactly 30 seconds (minimum) and if
>> it fails it can
>> exceed to 60 seconds. after that time it will sleep for 300 seconds and
>> will check for recalibration conditions. (there are rules like high
>> noise floor changes etc.)
>> a recalibration is also triggered at channel changes  and if chipset
>> temperature changes at a certain level.
>> from what i have seen the procedure in the qca9880 firmware is exactly
>> the same as in ath9k.
>> anyway. while this calibration is running, the signal and noise floor
>> might be unstable or even bogus until this is finished and rate control
>> might not be optimal
>> under stress conditions like long range links with low signals. with
>> standard wifi usage you should not notice it that much since signal to
>> noise ratio is high enough anyway
>>
>>
>>>
>>> Lots of moving parts...
>>>
>>> Thanks,
>>> Ben
>>>
>>>>
>>>> Sebastian
>>>>
>>>> Am 18.12.2019 um 00:37 schrieb Tom Psyborg:
>>>>> also noticed now that the noise floor changes with signal strength as
>>>>> described in this bug report:
>>>>> https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html
>>>>>
>>>>> after wifi restart
>>>>>
>>>>> iwinfo:
>>>>>
>>>>> signal: -59dBm noise: -108dBm
>>>>>
>>>>> then goes to
>>>>>
>>>>> signal: -52dBm noise: -103dBm
>>>>>
>>>>> and finally drops to
>>>>>
>>>>> signal: -59dBm noise: -103dBm
>>>>>
>>>>
>>>
>

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Sebastian Gottschall]

> I don't think it is correct to say periodic calibration does not 
> happen with
> ath10k.  Maybe very old wave-1 firmware has some issues, but recent 
> stuff appears
> to work.  I do see reported noise floor changing on 9984.
like on qca998x i expect it to change at least every 300 seconds. thats 
the calibration intervall for 988x. for 9984 i need
to check if its different
>
> Thanks,
> Ben
>
>>
>> On Tue, Dec 17, 2019 at 8:05 PM Sebastian Gottschall
>> <s.gottschall@newmedia-net.de> wrote:
>>>
>>>
>>> Am 18.12.2019 um 03:37 schrieb Ben Greear:
>>>>
>>>>
>>>> On 12/17/2019 06:12 PM, Sebastian Gottschall wrote:
>>>>> i dont know what you want to compare here.
>>>>>
>>>>> 1. you compare 2 different wifi chipsets. both have different
>>>>> sensititivy and overall output power spec
>>>>>
>>>>> 2. both have different amount of antenna chains. which does make a
>>>>> difference in input sensitivity
>>>>>
>>>>> 3. the patch ben made has no effect on qca9880 chipsets. it only
>>>>> takes effect on 10.4 based chipsets like 9984
>>>>
>>>> The part of my patch that sums secondary frequencies should apply to
>>>> wave-1 as well, but I have
>>>> not verified that yet.
>>> yeah. right. sorry i was just looking at total signal sum which uses
>>> rssi_comb_ht
>>>>
>>>>
>>>>> about noise floors in general. noise floors of -108 are bogus. there
>>>>> is a physical limit a noise level can be.
>>>>> since drivers like ath9k are doing a cyclic calibration, the noise
>>>>> value might indeed change. but this calibration is
>>>>> not running in realtime. its cyclic. i'm not aware if chipsets like
>>>>> qca988x are going the same way, but since qca988x
>>>>> has sime similaries with ath9k chipsets unlike the newer 9984
>>>>> variants, it could be. the 30 seconds mentioned
>>>>> in the bug report fits to my expectations of the early noisefloor
>>>>> calibration which has a short delay and after success
>>>>> turning to use a long delay. anyway. in this early calibration phase
>>>>> signals might change and will stabilize after. this isnt a issue
>>>>> since your connection will work anyway even if it might take a little
>>>>> bit longer if you have poor signal levels
>>>>>
>>>>> @ben. am i wrong or what do think?
>>>>
>>>> I don't know enough about how the noise floor calculations are done or
>>>> how the apply to settings
>>>> to know the answer.
>>>>
>>>> I will be happy in general if ath10k wave-1, wave-2, and ath9k report
>>>> similar RSSI for similar
>>>> setups.
>>> that will not work. you compare different chipsets and depending on the
>>> implementation by the card vendor
>>> rf sensitivity can be very diffent. the same goes for output power. 
>>> some
>>> vendors are using additional rf amps
>>> for enhancing output power (ubiquiti is best example here). this these
>>> amps also may have influence to sensitivity.
>>> on these cards you set 10 db output power, but in fact it outputs 18 
>>> db.
>>> so there is a bias offset on these cards or devices. (the offset is
>>> depending on the device model)
>>>
>>> what you measure is what the chip receives, but not what was lost on 
>>> the
>>> pcb layout. (or was even generated in case of noise)
>>> and when it comes to calibration data. correct would be if each
>>> individual card is calibrated before shipment. in reality manufactures
>>> are doing calibration on a single reference card and clone it on all
>>> following cards to save time. the result depends on day or week of
>>> production
>>> and current position of the moon and sun. errors of +- 2 db are common
>>> here. (this is not a fact for all card or device vendors)
>>>
>>>>
>>>> If you look at the tx-rate-power table in ath10k, for instance, you
>>>> can see different MCS are transmitted
>>>> at different signal levels.  So, some change from initial conditions
>>>> might be because higher MCS is
>>>> being transmitted after rate-ctrl scales up?
>>> yes. this is modulation related. as higher the rate goes as lower the
>>> power will be. thats princible of QAM.
>>> and the rate control itself isnt signal but error rate based. so high
>>> packet loss triggers the rate control to lower the rate which results
>>> in increased output power and vice versa. but as mentioned. at card
>>> startup a noise floor calibration starts which may succeed or fail.
>>> if it succeeds it will turn into a long delay phase. so cyclic
>>> calibration. the calibration time is exactly 30 seconds (minimum) 
>>> and if
>>> it fails it can
>>> exceed to 60 seconds. after that time it will sleep for 300 seconds and
>>> will check for recalibration conditions. (there are rules like high
>>> noise floor changes etc.)
>>> a recalibration is also triggered at channel changes  and if chipset
>>> temperature changes at a certain level.
>>> from what i have seen the procedure in the qca9880 firmware is exactly
>>> the same as in ath9k.
>>> anyway. while this calibration is running, the signal and noise floor
>>> might be unstable or even bogus until this is finished and rate control
>>> might not be optimal
>>> under stress conditions like long range links with low signals. with
>>> standard wifi usage you should not notice it that much since signal to
>>> noise ratio is high enough anyway
>>>
>>>
>>>>
>>>> Lots of moving parts...
>>>>
>>>> Thanks,
>>>> Ben
>>>>
>>>>>
>>>>> Sebastian
>>>>>
>>>>> Am 18.12.2019 um 00:37 schrieb Tom Psyborg:
>>>>>> also noticed now that the noise floor changes with signal 
>>>>>> strength as
>>>>>> described in this bug report:
>>>>>> https://www.mail-archive.com/ath10k@lists.infradead.org/msg11553.html 
>>>>>>
>>>>>>
>>>>>> after wifi restart
>>>>>>
>>>>>> iwinfo:
>>>>>>
>>>>>> signal: -59dBm noise: -108dBm
>>>>>>
>>>>>> then goes to
>>>>>>
>>>>>> signal: -52dBm noise: -103dBm
>>>>>>
>>>>>> and finally drops to
>>>>>>
>>>>>> signal: -59dBm noise: -103dBm
>>>>>>
>>>>>
>>>>
>>
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels

[Tom Psyborg]

On 18/12/2019, Sebastian Gottschall <s.gottschall@newmedia-net.de> wrote:
>
> Am 18.12.2019 um 03:37 schrieb Ben Greear:
>>
>>
>> On 12/17/2019 06:12 PM, Sebastian Gottschall wrote:
>>> i dont know what you want to compare here.
>>>
>>> 1. you compare 2 different wifi chipsets. both have different
>>> sensititivy and overall output power spec
>>>
>>> 2. both have different amount of antenna chains. which does make a
>>> difference in input sensitivity
>>>

both were connecting to 2x2 AP. 3x3 card should disable 3rd chain in
that case but driver doesn't do that yet.

> anyway. while this calibration is running, the signal and noise floor
> might be unstable or even bogus until this is finished and rate control
> might not be optimal
> under stress conditions like long range links with low signals.

i've noticed noise level switching between -105 and 0 on some high
5ghz channels between 2 litebeams (not very long range, less than 5km)
while signal levels are in -65 - -75dBm range

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k