[PATCH V4] PM / Domains: Remove intermediate states from the power off sequence

[Rafael J. Wysocki]

On Thursday, July 09, 2015 08:05:32 AM Lina Iyer wrote:
> On Wed, Jul 08 2015 at 13:56 -0600, Kevin Hilman wrote:
> >Ulf Hansson <ulf.hansson at linaro.org> writes:
> >
> >> Genpd's ->runtime_suspend() (assigned to pm_genpd_runtime_suspend())
> >> doesn't immediately walk the hierarchy of ->runtime_suspend() callbacks.
> >> Instead, pm_genpd_runtime_suspend() calls pm_genpd_poweroff() which
> >> postpones that until *all* the devices in the genpd are runtime suspended.
> >>
> >> When pm_genpd_poweroff() discovers that the last device in the genpd is
> >> about to be runtime suspended, it calls __pm_genpd_save_device() for *all*
> >> the devices in the genpd sequentially. Furthermore,
> >> __pm_genpd_save_device() invokes the ->start() callback, walks the
> >> hierarchy of the ->runtime_suspend() callbacks and invokes the ->stop()
> >> callback. This causes a "thundering herd" problem.
> >>
> >> Let's address this issue by having pm_genpd_runtime_suspend() immediately
> >> walk the hierarchy of the ->runtime_suspend() callbacks, instead of
> >> postponing that to the power off sequence via pm_genpd_poweroff(). If the
> >> selected ->runtime_suspend() callback doesn't return an error code, call
> >> pm_genpd_poweroff() to see if it's feasible to also power off the PM
> >> domain.
> >>
> >> Adopting this change enables us to simplify parts of the code in genpd,
> >> for example the locking mechanism. Additionally, it gives some positive
> >> side effects, as described below.
> >>
> >> i)
> >> One device's ->runtime_resume() latency is no longer affected by other
> >> devices' latencies in a genpd.
> >>
> >> The complexity genpd has to support the option to abort the power off
> >> sequence suffers from latency issues. More precisely, a device that is
> >> requested to be runtime resumed, may end up waiting for
> >> __pm_genpd_save_device() to complete its operations for *another* device.
> >> That's because pm_genpd_poweroff() can't confirm an abort request while it
> >> waits for __pm_genpd_save_device() to return.
> >>
> >> As this patch removes the intermediate states in pm_genpd_poweroff() while
> >> powering off the PM domain, we no longer need the ability to abort that
> >> sequence.
> >>
> >> ii)
> >> Make pm_runtime[_status]_suspended() reliable when used with genpd.
> >>
> >> Until the last device in a genpd becomes idle, pm_genpd_runtime_suspend()
> >> will return 0 without actually walking the hierarchy of the
> >> ->runtime_suspend() callbacks. However, by returning 0 the runtime PM core
> >> considers the device as runtime_suspended, so
> >> pm_runtime[_status]_suspended() will return true, even though the device
> >> isn't (yet) runtime suspended.
> >>
> >> After this patch, since pm_genpd_runtime_suspend() immediately walks the
> >> hierarchy of the ->runtime_suspend() callbacks,
> >> pm_runtime[_status]_suspended() will accurately reflect the status of the
> >> device.
> >>
> >> iii)
> >> Enable fine-grained PM through runtime PM callbacks in drivers/subsystems.
> >>
> >> There are currently cases were drivers/subsystems implements runtime PM
> >> callbacks to deploy fine-grained PM (e.g. gate clocks, move pinctrl to
> >> power-save state, etc.). While using the genpd, pm_genpd_runtime_suspend()
> >> postpones invoking these callbacks until *all* the devices in the genpd
> >> are runtime suspended. In essence, one runtime resumed device prevents
> >> fine-grained PM for other devices within the same genpd.
> >>
> >> After this patch, since pm_genpd_runtime_suspend() immediately walks the
> >> hierarchy of the ->runtime_suspend() callbacks, fine-grained PM is enabled
> >> throughout all the levels of runtime PM callbacks.
> >>
> >> iiii)
> >> Enable fine-grained PM for IRQ safe devices
> >>
> >> Per the definition for an IRQ safe device, its runtime PM callbacks must
> >> be able to execute in atomic context. In the path while genpd walks the
> >> hierarchy of the ->runtime_suspend() callbacks for the device, it uses a
> >> mutex. Therefore, genpd prevents that path to be executed for IRQ safe
> >> devices.
> >>
> >> As this patch changes pm_genpd_runtime_suspend() to immediately walk the
> >> hierarchy of the ->runtime_suspend() callbacks and without needing to use
> >> a mutex, fine-grained PM is enabled throughout all the levels of runtime
> >> PM callbacks for IRQ safe devices.
> >>
> >> Unfortunately this patch also comes with a drawback, as described in the
> >> summary below.
> >>
> >> Driver's/subsystem's runtime PM callbacks may be invoked even when the
> >> genpd hasn't actually powered off the PM domain, potentially introducing
> >> unnecessary latency.
> >>
> >> However, in most cases, saving/restoring register contexts for devices are
> >> typically fast operations or can be optimized in device specific ways
> >> (e.g. shadow copies of register contents in memory, device-specific checks
> >> to see if context has been lost before restoring context, etc.).
> >>
> >> Still, in some cases the driver/subsystem may suffer from latency if
> >> runtime PM is used in a very fine-grained manner (e.g. for each IO request
> >> or xfer). To prevent that extra overhead, the driver/subsystem may deploy
> >> the runtime PM autosuspend feature.
> >>
> >> Signed-off-by: Ulf Hansson <ulf.hansson at linaro.org>
> >
> >Reviewed-by: Kevin Hilman <khilman at linaro.org>
> >
> >I believe Geert and Lina have both tested earlier versions, but would be
> >nice to see their Tested-by for this one.
> >
> 
> I tested verssion 4. Did not see any issues with genpd suspend/resume.
> 
> Tested-by: Lina Iyer <lina.iyer at linaro.org>

OK, I'm queing this patch up for 4.3, thanks everybody!


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.