On Tue, Mar 13, 2012 at 5:28 PM, Colin Cross [off-list ref] wrote:

On Tue, Mar 13, 2012 at 4:52 PM, Kevin Hilman [off-list ref] wrote:

quoted

Hi Colin,

On 12/21/2011 01:09 AM, Colin Cross wrote:

quoted

To use coupled cpuidle states, a cpuidle driver must:

[...]

quoted

    Provide a struct cpuidle_state.enter function for each state
    that affects multiple cpus.  This function is guaranteed to be
    called on all cpus at approximately the same time.  The driver
    should ensure that the cpus all abort together if any cpu tries
    to abort once the function is called.

I've discoved the last sentence above is crucial, and in order to catch
all the corner cases I found it useful to have the struct
cpuidle_coupled in cpuidle.h so that the driver can check ready_count
itself (patch below, on top of $SUBJECT series.)

ready_count is an internal state of core coupled code, and will change
significantly in the next version of the patches.  Drivers cannot
depend on it.

quoted

As you know, on OMAP4, when entering the coupled state, CPU0 has to wait
for CPU1 to enter its low power state before entering itself.  The first
pass at implementing this was to just spin waiting for the powerdomain
of CPU1 to hit off.  That works... most of the time.

If CPU1 wakes up immediately (or before CPU0 starts checking), or more
likely, fails to hit the low-power state because of other hardware
"conditions", CPU0 will end up stuck in the loop waiting for CPU1.

To solve this, in addition to checking the power state of CPU1, I also
check if (coupled->ready_count != cpumask_weight(&coupled->alive_coupled_cpus)).
If true, it means that CPU1 has already exited/aborted so CPU0 had
better abort as well.

Checking the ready_count seemed like an easy way to do this, but did you
have any other mechanisms in mind for CPUs to communicate that they've
exited/aborted?

Why not set a flag from CPU1 when it exits the low power state, and
have CPU0 spin on the powerdomain register or the flag?  You can then
use the parallel barrier function to ensure both cpus have seen the
flag and reset it to 0 before returning.

I realized the parallel barrier helper was not included in the patch
set I posted, it will be in the next patch set.  Short version, no
caller to cpuidle_coupled_parallel_barrier will return before all cpus
in the coupled set have called it.  It allows you to resynchronize the
cpus after an abort to ensure they have all seen the abort flag before
clearing it and returning, leaving everything in the correct state for
the next idle attempt.