Re: [PATCH 1/2] PM / Domains: Add GENPD_FLAG_SUSPEND_ON flag

[Sibi Sankar]

On 2020-08-17 14:14, Ulf Hansson wrote:
> On Thu, 13 Aug 2020 at 19:26, Sibi Sankar <sibis@xxxxxxxxxxxxxx> wrote:
> > On 2020-08-13 18:04, Ulf Hansson wrote:
> > > On Wed, 12 Aug 2020 at 19:03, Sibi Sankar <sibis@xxxxxxxxxxxxxx> wrote:
> > >>
> > >> Uffe,
> > >> Thanks for taking time to review the
> > >> series!
> > >>
> > >> On 2020-08-12 15:15, Ulf Hansson wrote:
> > >> > On Tue, 11 Aug 2020 at 21:03, Sibi Sankar <sibis@xxxxxxxxxxxxxx> wrote:
> > >> >>
> > >> >> This is for power domains which needs to stay powered on for suspend
> > >> >> but can be powered on/off as part of runtime PM. This flag is aimed at
> > >> >> power domains coupled to remote processors which enter suspend states
> > >> >> independent to that of the application processor. Such power domains
> > >> >> are turned off only on remote processor crash/shutdown.
> > >> >
> > >> > As Kevin also requested, please elaborate more on the use case.
> > >> >
> > >> > Why exactly must the PM domain stay powered on during system suspend?
> > >> > Is there a wakeup configured that needs to be managed - or is there a
> > >> > co-processor/FW behaviour that needs to be obeyed to?
> > >>
> > >> Yes this is a co-processor behavior that
> > >> needs to be obeyed. Specifically application
> > >> processor notifies the Always on Subsystem
> > >> (AOSS) that a particular co-processor is up
> > >> using the power domains exposed by AOSS QMP
> > >> driver. AOSS uses this information to wait
> > >> for the co-processors to suspend before
> > >> starting its sleep sequence. The application
> > >> processor powers off these power domains only
> > >> if the co-processor has crashed or powered
> > >> off.
> > >
> > > Thanks for clarifying!
> > >
> > > Although, can you please elaborate a bit more on the actual use case?
> > > What are the typical co-processor and what drivers are involved in
> > > managing it?
> >
> > The co-processors using the power domains
> > exposed by qcom_aoss driver are modem,
> > audio dsp, compute dsp managed using
> > qcom_q6v5_mss and qcom_q6v5_pas driver.
> >
> > >
> > > As you may know, runtime PM becomes disabled during system suspend of
> > > a device. Which means, if the driver tries to power off the
> > > coprocessor (via calling pm_runtime_put() for example), somewhere in
> > > the system suspend phase of the corresponding device, its attached PM
> > > domain stays powered on when managed by genpd.
> >
> > The drivers aren't really expected
> > do anything during suspend/resume
> > pretty much because the co-processors
> > enter low-power modes independent to
> > that of the application processor. On
> > co-processor crash the remoteproc core
> > does a pm_stay_awake followed by a
> > pm_relax after crash recovery.
>
> Okay, thanks again for clarifying. You have convinced me about the
> need for a new flag to cope with these use cases.
>
> Would you mind updating the commit message with some of the
> information you just provided?
>
> Additionally, to make it clear that the flag should be used to keep
> the PM domain powered on during system suspend, but only if it's
> already powered on - please rename the flag to GENPD_FLAG_NO_SUSPEND,
> and update the corresponding description of it in the header file.

Thanks, naming it ^^ makes more sense :)

https://lore.kernel.org/lkml/340a7aafcf0301ff3158a4e211992041@xxxxxxxxxxxxxx/

Also we wouldn't want to power on
runtime suspended power domains with
the NO_SUSPEND flag set, on resume as
explained ^^. Do you agree with that
as well?

> [...]
>
> Kind regards
> Uffe

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.