Search Linux Wireless

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx:
From: Ben Greear <greearb@xxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxx>
---
   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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/ath10k




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



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




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux