Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4d676b7a-3200-2585-3421-393e899a4da6@quicinc.com>
Date:   Thu, 6 Jul 2023 13:50:50 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: Question on platform_suspend_ops::suspend_again() callback
Content-Language: en-US
From:   Abhinav Kumar <quic_abhinavk@quicinc.com>
To:     "Rafael J. Wysocki" <rafael@kernel.org>, <pavel@ucw.cz>
CC:     <len.brown@intel.com>, <linux-pm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, <quic_travitej@quicinc.com>,
        <quic_pdhaval@quicinc.com>,
        Nagamalleswararao Ganji <nganji@quicinc.com>,
        <ulf.hansson@linaro.org>, Viresh Kumar <viresh.kumar@linaro.org>,
        "Bjorn Andersson" <quic_bjorande@quicinc.com>
References: <7197a514-5568-2089-f038-2625b88d7e2e@quicinc.com>
In-Reply-To: <7197a514-5568-2089-f038-2625b88d7e2e@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Rafael / Pavel

Gentle reminder on this  ....

Thanks

Abhinav

On 6/27/2023 3:19 PM, Abhinav Kumar wrote:
> Hi Rafael / Pavel
> 
> I work with the QC display team and help to co-maintain the MSM display 
> sub-system.
> 
> I had a question about the suspend_again() callback in the 
> platform_suspend_ops
> 
> Typical pm_suspend() callflow is like below:
> 
> pm_suspend()
> 
>      ->pm_suspend()
>          ->enter_state()
>              -> suspend_prepare()
>                  -> suspend_freeze_processes()
>                   (freeze all userspace processes)
>              -> suspend_devices_and_enter()
>                  -> dpm_suspend_start()
>                  -> suspend_enter()
>                      -> s2idle_loop
>                      -> dpm_resume_early()
>                      (after breaking out of loop)
> 
>              -> suspend_finish()
>                  -> suspend_thaw_processes()
>                    (unfreeze all userspace processes)
> 
> 
> Now, lets say while we are in s2idle() (typically in the display idle 
> screen use-cases) and then an interrupt fires. We will then break out of 
> the loop and also unfreeze() all usermode processes unless
> we have a suspend_again() callback 
> https://github.com/torvalds/linux/blob/master/kernel/power/suspend.c#L512 due to the suspend_thaw_processes().
> 
> (ignoring the error and wakeup conditions for this discussion)
> 
>  From our profiling it takes about 300ms to wakeup these usermode 
> processes on android (it can vary based on how many processes were 
> running, which OS etc).
> 
> But on an average we can can continue to remain idle for about 300ms 
> more if can skip the usermode process wakeup.
> 
>  From the documentation of the suspend_again() also, it seems that 
> defining this callback would ensure we can address the interrupt and go 
> back to the s2idle instead of waking up usermode processes and trying 
> again after that.
> 
>   * @suspend_again: Returns whether the system should suspend again 
> (true) or
>   *      not (false). If the platform wants to poll sensors or execute some
>   *      code during suspended without invoking userspace and most of 
> devices,
>   *      suspend_again callback is the place assuming that 
> periodic-wakeup or
>   *      alarm-wakeup is already setup. This allows to execute some 
> codes while
>   *      being kept suspended in the view of userland and devices.
>   *
> 
> Questions to you:
> 
> 1) We do not see anyone using this suspend_again() op currently. Has 
> there been any discussion about this before or are there any existing 
> patches in the list which demonstrate a usage? If so, can you pls point 
> us to those?
> 
> 2) If there are no such patches, if we go ahead and add a 
> suspend_again() callback and post it for review, can it be considered 
> for acceptance?
> 
> 3) If suspend_again() callback is not the way to go, can you please 
> suggest an alternative of how we can avoid wakeup of userspace process.
> 
> Just want to make sure that there are no reasons which we are not aware 
> of before attempting this.
> 
> Thanks
> 
> Abhinav