Re: [PATCH 3/4] PM / Domains: Allow runtime PM during system PM phases

[Kevin Hilman <khilman@baylibre.com>]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> The PM core disables runtime PM at __device_suspend_late() before it calls
> a system PM "late" callback for the device. When resuming the device,
> after the corresponding "early" callback has been invoked, it re-enables
> runtime PM.
>
> By changing genpd to conform to this behaviour, the device no longer have
> to be unconditionally runtime resumed from genpd's ->prepare() callback. In
> most cases that avoids unnecessary operations.

...avoids the unnecessary, and energy-wasting operaions of runtime
resuming devices that have nothing to do, only to runtime suspend them
shortly after.

> As runtime PM then isn't disabled/enabled by genpd, the subsystem/driver
> can rely on the generic behaviour from PM core. Consequentially runtime PM
> is allowed in more phases of system PM than before.

OK.  I like this change a lot, but it the changelog doesn't go into
describing how this gets around the problems that were being worked
around by having genpd disable runtime PM in the first place.  This
changelog should describe how this appraoch deals with those problems,
or why they don't exist anymore.

> Although, because of this change and due to that genpd powers on the PM
> domain unconditionally in the system PM resume "noirq" phase, it could
> potentially cause a PM domain to stay powered on even if it's unused after
> the system has resumed. To avoid this, let's schedule a power off work
> when genpd's system PM ->complete() callback has been invoked for the last
> device in the PM domain.

OK.

> Another issue that arises due to this change in genpd, concerns those
> platforms/PM domains that makes use of genpd's device ->stop|start()
> callbacks. In these scenarios, the corresponding subsystem/driver needs to
> invoke pm_runtime_force_suspend() from a system PM suspend callback to
> allow genpd's ->runtime_suspend() to be invoked for an active device, else
> genpd can't "stop" a device that is "started".
>
> The subsystem/driver also needs to invoke pm_runtime_force_resume()
> in a system PM resume callback, to restore the runtime PM state for the
> device and to re-enable runtime PM.
>
> Currently not all involved subsystem/drivers makes use of
> pm_runtime_force_suspend|resume() accordingly. Therefore, let's invoke
> pm_runtime_force_suspend|resume() from genpd's "noirq" system PM
> callbacks, in cases when the ->stop|start() callbacks are being used.
>
> In this way, devices are "stoped" during suspend and "started" during
> resume, even in those cases when the subsystem/driver don't call
> pm_runtime_force_suspend|resume() themselves.

IMO, this last part might be better dealt with as a separate patch, so
it's clear which parts are for the main genpd change, and which parts
fixup the fallout.

On a related note, are there many genpd drivers using dev_ops->start and
->stop out there?  A quick grep didn't find anything other than the
pm_clk stuff.

Kevin