2023-06-27 22:27:28

by Abhinav Kumar

[permalink] [raw]
Subject: Question on platform_suspend_ops::suspend_again() callback

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


2023-07-06 21:02:22

by Abhinav Kumar

[permalink] [raw]
Subject: Re: Question on platform_suspend_ops::suspend_again() callback

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