Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4f95f560-7f09-48d2-9121-5bee5c4201ab@quicinc.com>
Date:   Mon, 29 Aug 2022 10:58:45 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH 0/4] ath11k: Enable low power mode when WLAN is not active
Content-Language: en-US
To:     "Limonciello, Mario" <mario.limonciello@amd.com>,
        <ath11k@lists.infradead.org>
CC:     <linux-wireless@vger.kernel.org>
References: <20220720134959.15688-1-quic_mpubbise@quicinc.com>
 <ad99d238-dc1a-3233-fc6b-0cd49e428903@amd.com>
 <6daefd60-dcbc-06e1-8091-ea225690edf8@amd.com>
 <ecce8764-1760-4617-49aa-c111305ebd55@quicinc.com>
 <7e00cc36-3a07-92e2-c60e-a3c8939b407e@amd.com>
From:   Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
In-Reply-To: <7e00cc36-3a07-92e2-c60e-a3c8939b407e@amd.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 8/25/2022 10:46 PM, Limonciello, Mario wrote:
> On 8/25/2022 06:24, Manikanta Pubbisetty wrote:
>> On 7/23/2022 5:39 PM, Limonciello, Mario wrote:
>>> On 7/22/2022 17:00, Limonciello, Mario wrote:
>>>> On 7/20/2022 08:49, Manikanta Pubbisetty wrote:
>>>>> 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 on a battery operated device. Same is the case with non-WoW
>>>>> suspend, chip will not be put into low power mode when the system is
>>>>> suspended resulting in higher battery drain.
>>>>>
>>>>> Send QMI MODE OFF command to firmware during WiFi OFF to put device
>>>>> into low power mode.
>>>>>
>>>>> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1
>>>>>
>>>>> Manikanta Pubbisetty (4):
>>>>>    ath11k: Fix double free issue during SRNG deinit
>>>>>    ath11k: Move hardware initialization logic to start()
>>>>>    ath11k: Enable low power mode when WLAN is not active
>>>>>    ath11k: Fix failed to parse regulatory event print
>>>>>
>>>>>   drivers/net/wireless/ath/ath11k/core.c | 237 
>>>>> ++++++++++++++++++-------
>>>>>   drivers/net/wireless/ath/ath11k/core.h |   8 +-
>>>>>   drivers/net/wireless/ath/ath11k/hal.c  |   1 +
>>>>>   drivers/net/wireless/ath/ath11k/mac.c  |  33 ++--
>>>>>   drivers/net/wireless/ath/ath11k/reg.c  |   2 +
>>>>>   5 files changed, 189 insertions(+), 92 deletions(-)
>>>>>
>>>>
>>>> This series looked potentially promising to me for a problem that 
>>>> I'm seeing on a notebook failing to sleep where WLAN_WAKE is 
>>>> asserted on the WCN6855 on resume even though WoW wasn't set at 
>>>> all.  This is problematic as it causes a spurious wake while SUT is 
>>>> reading from the EC since two IRQs are now active and the kernel 
>>>> wakes from that.
>>>> Removing the WCN6855 from the system it doesn't happen.
>>>>
>>>> I figured I'd give it a spin to see if it improved things.
>>>> I applied the series on top of 5.19-rc7 and it applied cleanly but I 
>>>> get timeouts on wlan card init (and of course suspend fails now too).
>>>>
>>>> Here's the mhi/ath11k_pci snippets:
>>>>
>>>> [    2.864110] ath11k_pci 0000:01:00.0: BAR 0: assigned [mem 
>>>> 0xb4000000-0xb41fffff 64bit]
>>>> [    2.864901] ath11k_pci 0000:01:00.0: MSI vectors: 32
>>>> [    2.864912] ath11k_pci 0000:01:00.0: wcn6855 hw2.1
>>>> [    3.106892] mhi mhi0: Requested to power ON
>>>> [    3.107054] mhi mhi0: Power on setup success
>>>> [    3.198178] mhi mhi0: Wait for device to enter SBL or Mission mode
>>>> [    3.971019] ath11k_pci 0000:01:00.0: chip_id 0x2 chip_family 0xb 
>>>> board_id 0xff soc_id 0x400c0210
>>>> [    3.971025] ath11k_pci 0000:01:00.0: fw_version 0x11080bbb 
>>>> fw_build_timestamp 2021-12-16 03:42 fw_build_id 
>>>> WLAN.HSP.1.1-03003-QCAHSPSWPL_V1_V2_SILICONZ_LITE-2
>>>> [    4.290328] ath11k_pci 0000:01:00.0 wlp1s0: renamed from wlan0
>>>> [    4.308760] ath11k_pci 0000:01:00.0: Failed to set the requested 
>>>> Country regulatory setting
>>>> [    4.309028] ath11k_pci 0000:01:00.0: Failed to set the requested 
>>>> Country regulatory setting
>>>> [   14.386201] ath11k_pci 0000:01:00.0: qmi failed wlan ini request, 
>>>> err = -110
>>>> [   14.386210] ath11k_pci 0000:01:00.0: qmi failed to send wlan fw 
>>>> ini:-110
>>>> [   14.386214] ath11k_pci 0000:01:00.0: failed to send firmware 
>>>> start: -110
>>>> [   14.386243] ath11k_pci 0000:01:00.0: failed to start firmware: -110
>>>> [   14.386266] ath11k_pci 0000:01:00.0: failed to setup device: -110
>>>> [   14.386300] ath11k_pci 0000:01:00.0: failed to start device : -110
>>>>
>>>> And so - on.
>>>> Is it functionally dependent on other patches in linux-next or 
>>>> another tree I won't see in 5.19?
>>>
>>> I had a try with linux-next and your series but I reproduce the same 
>>> failure I described above.
>>>
>>> ee3a066e4f4e (HEAD) ath11k: Fix failed to parse regulatory event print
>>> 4f850f1e26dc ath11k: Enable low power mode when WLAN is not active
>>> e320c8ef9841 ath11k: Move hardware initialization logic to start()
>>> dbc26036c1ef ath11k: Fix double free issue during SRNG deinit
>>> 18c107a1f120 (tag: next-20220722, linux-next/master) Add linux-next 
>>> specific files for 20220722
>>>
>>>
>>
>> Sorry for late reply and thanks for reporting the bug.
>> After several rounds of discussions with the concerned teams on the 
>> failure that you have reported, we have root caused the issue.
>>
> 
> Thanks for getting back to me and confirming that.
> 
>> The changes that I have posted will not work on other targets like 
>> WCN6855 as is. It requires additional changes and therefore lots of 
>> testing.
> 
> Well glad I reported it and saved you a regression!
> 
>>
>> For now, I have posted a V2 enabling the "low power mode" design only on
>> WCN6750. I have started working on enabling the logic for other 
>> chipsets also. Once the changes are well tested, I'll post a separate 
>> patch to enable it on WCN6855. It might take a little more time.
>>
> 
> Will low power mode on WCN6855 also require F/W changes, or you expect 
> that it will be able to be done entirely in the kernel driver?

It should most likely be a driver change.

> 
> I am eager to see if low power mode can fix my existing mentioned issue. 
>   If it can you may CC me when they go out and I'm happy to test these 
> patches too when you have them ready.
> 

Sure, thanks!!