From: Kalle Valo <kvalo@kernel.org>
To: Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
Cc: <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v4 3/3] ath11k: Enable low power mode when WLAN is not active
Date: Fri, 24 Feb 2023 16:46:31 +0200 [thread overview]
Message-ID: <87h6vbruig.fsf@kernel.org> (raw)
In-Reply-To: <20230203060128.19625-4-quic_mpubbise@quicinc.com> (Manikanta Pubbisetty's message of "Fri, 3 Feb 2023 11:31:28 +0530")
Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes:
> Currently, WLAN chip is powered once during driver probe and is kept
> ON (powered) always even when WLAN is not active; keeping the chip
> powered ON all the time will consume extra power which is not
> desirable for a battery operated device. Same is the case with non-WoW
> suspend, chip will never be put into low power mode when the system is
> suspended resulting in higher battery drain.
>
> As per the recommendation, sending a PDEV suspend WMI command followed
> by a QMI MODE OFF command will cease all WLAN activity and put the device
> in low power mode. When WLAN interfaces are brought up, sending a QMI
> MISSION MODE command would be sufficient to bring the chip out of low
> power. This is a better approach than doing hif_power_down()/hif_power_up()
> for every WiFi ON/OFF sequence since the turnaround time for entry/exit of
> low power mode is much less. Overhead is just the time taken for sending
> QMI MODE OFF & QMI MISSION MODE commands instead of going through the
> entire chip boot & QMI init sequence.
>
> Currently the changes are applicable only for WCN6750. This can be
> extended to other targets with a future patch.
>
> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16
>
> Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
This is still crashing for me every time with WCN6855 on a NUC x86
device when I rmmod ath11k. Interestingly enough QCA6390 on a Dell XPS
13 9310 does not crash.
I investigated the crash more, the crash happens in
ath11k_dp_process_rxdma_err() on this line:
srng = &ab->hal.srng_list[err_ring->ring_id];
Here are the debug messages before the crash (first and last are my
own messages):
[ 226.766111] rmmod ath11k_pci
[ 227.003678] ath11k_pci 0000:06:00.0: txpower from firmware NaN, reported -2147483648 dBm
[ 227.082283] ath11k_pci 0000:06:00.0: qmi wifi fw del server
[ 227.195760] ath11k_pci 0000:06:00.0: cookie:0x0
[ 227.195843] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x15b894d
[ 227.216022] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x0
[ 227.216086] ath11k_pci 0000:06:00.0: soc reset cause:0
[ 227.236170] ath11k_pci 0000:06:00.0: MHISTATUS 0xff04
[ 227.270816] ath11k_pci 0000:06:00.0: ext irq:167
[ 227.271231] ath11k_dp_process_rxdma_err() 4187 ab ffff888145520000 err_ring 00000000000001d0
So we get irq 167 which is:
167: 0 0 0 0 0 0 0 0 IR-PCI-MSI-0000:06:00.0 14-edge DP_EXT_IRQ
But in ath11k_pcic_ext_interrupt_handler() ATH11K_FLAG_EXT_IRQ_ENABLED
is still enabled so the irq is processed:
if (!test_bit(ATH11K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags))
return IRQ_HANDLED;
It looks like that, after applying this patch 3, whenever
ath11k_pci_remove() is called we are not calling
ath11k_hif_irq_disable() anymore. I checked that without patch 3
ath11k_hif_irq_disable() is always called. So this patch is definitely
breaking something fundamental, but I ran out of time to invetigate
further. I hope this still helps.
Do note I have concerns about this patchset, it just changes quite a lot
of the driver logic and I'm worried what else this breaks. Also we
should definitely test with another AHB device like IPQ8074, this
patchset needs extensive testing.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
WARNING: multiple messages have this Message-ID (diff)
From: Kalle Valo <kvalo@kernel.org>
To: Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
Cc: <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v4 3/3] ath11k: Enable low power mode when WLAN is not active
Date: Fri, 24 Feb 2023 16:46:31 +0200 [thread overview]
Message-ID: <87h6vbruig.fsf@kernel.org> (raw)
In-Reply-To: <20230203060128.19625-4-quic_mpubbise@quicinc.com> (Manikanta Pubbisetty's message of "Fri, 3 Feb 2023 11:31:28 +0530")
Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes:
> Currently, WLAN chip is powered once during driver probe and is kept
> ON (powered) always even when WLAN is not active; keeping the chip
> powered ON all the time will consume extra power which is not
> desirable for a battery operated device. Same is the case with non-WoW
> suspend, chip will never be put into low power mode when the system is
> suspended resulting in higher battery drain.
>
> As per the recommendation, sending a PDEV suspend WMI command followed
> by a QMI MODE OFF command will cease all WLAN activity and put the device
> in low power mode. When WLAN interfaces are brought up, sending a QMI
> MISSION MODE command would be sufficient to bring the chip out of low
> power. This is a better approach than doing hif_power_down()/hif_power_up()
> for every WiFi ON/OFF sequence since the turnaround time for entry/exit of
> low power mode is much less. Overhead is just the time taken for sending
> QMI MODE OFF & QMI MISSION MODE commands instead of going through the
> entire chip boot & QMI init sequence.
>
> Currently the changes are applicable only for WCN6750. This can be
> extended to other targets with a future patch.
>
> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16
>
> Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
This is still crashing for me every time with WCN6855 on a NUC x86
device when I rmmod ath11k. Interestingly enough QCA6390 on a Dell XPS
13 9310 does not crash.
I investigated the crash more, the crash happens in
ath11k_dp_process_rxdma_err() on this line:
srng = &ab->hal.srng_list[err_ring->ring_id];
Here are the debug messages before the crash (first and last are my
own messages):
[ 226.766111] rmmod ath11k_pci
[ 227.003678] ath11k_pci 0000:06:00.0: txpower from firmware NaN, reported -2147483648 dBm
[ 227.082283] ath11k_pci 0000:06:00.0: qmi wifi fw del server
[ 227.195760] ath11k_pci 0000:06:00.0: cookie:0x0
[ 227.195843] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x15b894d
[ 227.216022] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x0
[ 227.216086] ath11k_pci 0000:06:00.0: soc reset cause:0
[ 227.236170] ath11k_pci 0000:06:00.0: MHISTATUS 0xff04
[ 227.270816] ath11k_pci 0000:06:00.0: ext irq:167
[ 227.271231] ath11k_dp_process_rxdma_err() 4187 ab ffff888145520000 err_ring 00000000000001d0
So we get irq 167 which is:
167: 0 0 0 0 0 0 0 0 IR-PCI-MSI-0000:06:00.0 14-edge DP_EXT_IRQ
But in ath11k_pcic_ext_interrupt_handler() ATH11K_FLAG_EXT_IRQ_ENABLED
is still enabled so the irq is processed:
if (!test_bit(ATH11K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags))
return IRQ_HANDLED;
It looks like that, after applying this patch 3, whenever
ath11k_pci_remove() is called we are not calling
ath11k_hif_irq_disable() anymore. I checked that without patch 3
ath11k_hif_irq_disable() is always called. So this patch is definitely
breaking something fundamental, but I ran out of time to invetigate
further. I hope this still helps.
Do note I have concerns about this patchset, it just changes quite a lot
of the driver logic and I'm worried what else this breaks. Also we
should definitely test with another AHB device like IPQ8074, this
patchset needs extensive testing.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
next prev parent reply other threads:[~2023-02-24 14:46 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-03 6:01 [PATCH v4 0/3] Enable low power mode when WLAN is not active Manikanta Pubbisetty
2023-02-03 6:01 ` Manikanta Pubbisetty
2023-02-03 6:01 ` [PATCH v4 1/3] ath11k: Fix double free issue during SRNG deinit Manikanta Pubbisetty
2023-02-03 6:01 ` Manikanta Pubbisetty
2023-02-03 6:01 ` [PATCH v4 2/3] ath11k: Move hardware initialization logic to start() Manikanta Pubbisetty
2023-02-03 6:01 ` Manikanta Pubbisetty
2023-02-03 6:01 ` [PATCH v4 3/3] ath11k: Enable low power mode when WLAN is not active Manikanta Pubbisetty
2023-02-03 6:01 ` Manikanta Pubbisetty
2023-02-24 14:46 ` Kalle Valo [this message]
2023-02-24 14:46 ` Kalle Valo
2023-02-27 10:42 ` Manikanta Pubbisetty
2023-02-27 10:42 ` Manikanta Pubbisetty
2023-03-13 7:19 ` Manikanta Pubbisetty
2023-03-13 7:19 ` Manikanta Pubbisetty
2023-02-24 14:50 ` Kalle Valo
2023-02-24 14:50 ` Kalle Valo
2023-02-27 10:49 ` Manikanta Pubbisetty
2023-02-27 10:49 ` Manikanta Pubbisetty
2023-02-27 11:59 ` Kalle Valo
2023-02-27 11:59 ` Kalle Valo
2023-02-24 14:54 ` [PATCH v4 0/3] " Kalle Valo
2023-02-24 14:54 ` Kalle Valo
2023-02-27 10:49 ` Manikanta Pubbisetty
2023-02-27 10:49 ` Manikanta Pubbisetty
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87h6vbruig.fsf@kernel.org \
--to=kvalo@kernel.org \
--cc=ath11k@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=quic_mpubbise@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.