[PATCH] wcn36xx: Fix software-driven scan

[Loic Poulain]

Hi Bryan,

On Mon, 24 Aug 2020 at 03:45, Bryan O'Donoghue
<bryan.odonoghue at linaro.org> wrote:
>
> On 17/08/2020 17:01, Loic Poulain wrote:
> > For software-driven scan, rely on mac80211 software scan instead
> > of internal driver implementation. The internal implementation
> > cause connection trouble since it keep the antenna busy during
> > the entire scan duration, moreover it's only a passive scanning
> > (no probe request). Therefore, let mac80211 manages sw scan.
> >
> > Note: we fallback to software scan if firmware does not report
> > scan offload support or if we need to scan the 5Ghz band (currently
> > not supported by the offload scan...).
> >
> > Signed-off-by: Loic Poulain <loic.poulain at linaro.org>
> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org>
>
> OK.
>
> TL;DR included diff fixes the performance issue.
>
> I've poked about with the old scan, and this new scan and I've managed
> to find to find where the new scan is killing performance.
>
> Please see below.
>
> @@ -792,6 +792,7 @@ int wcn36xx_smd_finish_scan(struct wcn36xx *wcn,
>   {
>          struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
>          struct wcn36xx_hal_finish_scan_req_msg msg_body;
> +       u8 oper_channel;
>          int ret;
>
>          mutex_lock(&wcn->hal_mutex);
> @@ -802,7 +803,9 @@ int wcn36xx_smd_finish_scan(struct wcn36xx *wcn,
>                  /* Notify BSSID with null data packet */
>                  msg_body.notify = 1;
>                  msg_body.frame_type = 2;
> -               msg_body.oper_channel = channel;
> +               oper_channel = WCN36XX_HW_CHANNEL(wcn);
> +               if (oper_channel != channel)
> +                       msg_body.oper_channel = channel;

Is this condition ever satisfied? I mean, finish_scan is normally always
called with operating-channel as channel parameter.

>                  msg_body.scan_entry.bss_index[0] = vif_priv->bss_index;
>                  msg_body.scan_entry.active_bss_count = 1;
>          }
>
> Happily, I'm finding with this modification on top of your patch
>
> 1. Running a continuous Android scan
>     As in when you have the network scan window open
>     A simultaneous "iperf -c xxx.xxx.xxx.xxx -i 1 -t 1800"
> 2. Performance is as expected
>     I see throughput oscillate between 30 mbit and 100 mbit
>     - Android runs a scan every 15 seconds

With scanning app in the foreground, right?
While connected to an AP, Android 'should' only background scan if
RSSI becomes too low (for roaming).

>     - Software scan takes 11 seconds to complete
>     - So this is expected behavior
> 3. During quiescent periods we have the expected throughput
> 4. We gain from the additional stability the rest of your patch provides
>
> Specifically on #4 what I find with the old scan is that with some APs
> we can get a disassocation after a scan completes.
>
> My test-case AP for that behavior is a Ubiquiti Nano HD on a 5ghz frequency.
>
> Its funny that rewriting the operating channel in the finish_scan path
> should make a difference.
>
> All I can guess at here is that the PHY mode doesn't get updated correctly
>
> Loic, can you make sure you are happy with the change and V2 this patch?
>
> You can retain my SoB.
>
> ---
> bod