Message ID | 20191216220747.887-1-greearb@candelatech.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Kalle Valo |
Headers | show |
Series | ath10k: Per-chain rssi should sum the secondary channels | expand |
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_ */
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 >
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 >> >
>> 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 >>> >> >
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 > >>> > >> > >
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 >
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.
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
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
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
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
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
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 >
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 >> >
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 >>> >> >
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 > >>> > >> > >
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 >>>>>
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
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 >>>>> >>>> >>> >
> 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 >>>>>> >>>>> >>>> >> >
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_ */