Message-ID: <9efc2082-d46d-5d57-d275-1cb47277f639@quicinc.com>
Date:   Mon, 27 Feb 2023 16:19:08 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v4 3/3] ath11k: Enable low power mode when WLAN is not
 active
Content-Language: en-US
To:     Kalle Valo <kvalo@kernel.org>
CC:     <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
References: <20230203060128.19625-1-quic_mpubbise@quicinc.com>
 <20230203060128.19625-4-quic_mpubbise@quicinc.com>
 <87cz5zruc6.fsf@kernel.org>
From:   Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
In-Reply-To: <87cz5zruc6.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2/24/2023 8:20 PM, Kalle Valo wrote:
> 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>
> 
> [...]
> 
>> +static int ath11k_ahb_core_start_ipq8074(struct ath11k_base *ab)
>> +{
>> +	/* TODO: Currently initializing the hardware/firmware only
>> +	 * during hardware recovery. Support to shutdown/turn-on
>> +	 * the hardware during Wi-Fi OFF/ON will be added later.
>> +	 */
>> +	if (!test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags))
>> +		return 0;
>> +
>> +	return ath11k_core_start_device(ab);
>> +}
>> +
>> +static void ath11k_ahb_core_stop_ipq8074(struct ath11k_base *ab)
>> +{
>> +	/* TODO: Currently stopping the hardware/firmware only
>> +	 * during driver unload. Support to shutdown/turn-on
>> +	 * the hardware during Wi-Fi OFF/ON will be added later.
>> +	 */
>> +	if (!test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags))
>> +		return;
>> +
>> +	return ath11k_core_stop_device(ab);
>> +}
> 
> Please clarify what Wi-Fi OFF/ON exactly means on these two comments,
> it's not clear for me.
>

  By Wi-Fi OFF/ON I mean is the bringing the last wlan interface down/up
which is nothing but the non-WoW suspend/resume.


> Also I want to mention that I suspect eventually we have to always power
> off the firmware during suspend to get hibernation working:
> 

This patch will power off the firmware for WCN6750. I'm not sure how to 
get that working for other ath11k devices.

Thanks,
Manikanta