Re: [Update 2x][RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Thursday, May 24, 2012, Kevin Hilman wrote:
> Hi Rafael,
> 
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > On some systems there are CPU cores located in the same power
> > domains as I/O devices.  Then, power can only be removed from the
> > domain if all I/O devices in it are not in use and the CPU core
> > is idle.  Add preliminary support for that to the generic PM domains
> > framework.
> >
> > First, the platform is expected to provide a cpuidle driver with one
> > extra state designated for use with the generic PM domains code.
> > This state should be initially disabled and its exit_latency value
> > should be set to whatever time is needed to bring up the CPU core
> > itself after restoring power to it, not including the domain's
> > power on latency.  Its .enter() callback should point to a procedure
> > that will remove power from the domain containing the CPU core at
> > the end of the CPU power transition.
> >
> > The remaining characteristics of the extra cpuidle state, referred to
> > as the "domain" cpuidle state below, (e.g. power usage, target
> > residency) should be populated in accordance with the properties of
> > the hardware.
> >
> > Next, the platform should execute genpd_attach_cpuidle() on the PM
> > domain containing the CPU core.  That will cause the generic PM
> > domains framework to treat that domain in a special way such that:
> >
> >  * When all devices in the domain have been suspended and it is about
> >    to be turned off, the states of the devices will be saved, but
> >    power will not be removed from the domain.  Instead, the "domain"
> >    cpuidle state will be enabled so that power can be removed from
> >    the domain when the CPU core is idle and the state has been chosen
> >    as the target by the cpuidle governor.
> >
> >  * When the first I/O device in the domain is resumed and
> >    __pm_genpd_poweron(() is called for the first time after
> >    power has been removed from the domain, the "domain" cpuidle
> >    state will be disabled to avoid subsequent surprise power removals
> >    via cpuidle.
> 
> This looks like a good approach.  I like that it keeps a pretty clean
> separation between CPUidle and PM domains.
> 
> My only comment would be that the recalc of the exit_latency should be
> described a bit more.  Specifically, I'm not sure why it's adjused at
> every genpd poweron.  At first I thought it was just supposed to be
> adjusted upon attach, then adjusted back on detatch, but with the recalc
> also in every poweron, I'm a little confused.  Care to clarify?

The problem is that the PM domains code measures the time it takes to
power off a domain and updates its power on latency parameter if the
measured time is greater.  This is done for PM QoS to operate on realistic
numbers (most of the time at least).

Of course, this also affects the CPU wakeup latency if the wakeup involves
turning a domain on.

Thanks,
Rafael