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: <7197a514-5568-2089-f038-2625b88d7e2e@quicinc.com>
Date:   Tue, 27 Jun 2023 15:19:35 -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
Content-Language: en-US
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>
From:   Abhinav Kumar <quic_abhinavk@quicinc.com>
Subject: Question on platform_suspend_ops::suspend_again() callback
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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