[PATCH v3 02/15] dt/bindings: Update binding for PM domain idle states

[Sudeep Holla]

On 16/08/16 09:41, Brendan Jackman wrote:
> Hi Lina,
> On Mon, Aug 15, 2016 at 04:40:14PM -0600, Lina Iyer wrote:
>> On Mon, Aug 15 2016 at 10:14 -0600, Sudeep Holla wrote:
[,,,]


>>>
>>> Yes even ACPI has indices to solve this.
>>>
>>>>> 2. Something similar to (1) but without index instead phandles.
>>>>>
>>>>
>>>> The problem is when you have non-CPU devices in the device tree and
>>>> since they do not have a way to represent states like CPU, we did not
>>>> have a clear path to that. Hence we punted that to later. Whatever we
>>>> do, we should solve it for a generic PM domain, not just CPU domains.
>>>>
>>>
>>> Yes bindings defined here should be applicable for devices to, but only
>>> CPU's will have this hierarchy while the devices need not bother about
>>> hierarchy. However the parent power domain can ever the state which is
>>> least common denominator of all it's children power domain. That's my
>>> understanding. No?
>>>
>
>
> Are you saying that the parent can enter the shallowest idle state that all its
> children are in (I.e if all its children are in "retention" then it can enter
> "retention")? I don't know what the reality is on existing platforms but it
> doesn't sound like 100% safe assumption to make.

I was referring to non-CPU/device power states above. For CPU we do need
a mechanism in place to indicate the dependency.

> Also I don't think you can
> necessarily correlate idle states at different domain levels - i.e. here we've
> matched up the idea of "retention" at core level with that of "retention" at
> cluster level. I may have misunderstood you there..
>

Correct for CPUs. For normal devices and their power domains, it could
straight forward. E.g if many devices are at-least at state D1(few may
be at state D2 or above), the parent can enter D1.(D0-runnning and D1-D3
are low power states in the above example)

>> That is correct. But say if all the CPUs choose CORE_RET + CLUSTER_PG,
>> which is invalid and the firmware has to ignore it and does CORE_RET +
>> CLUSTER_RET instead, then Linux may have an inconsistent view of the
>> state selection.
>>

1. First, CORE_RET + CLUSTER_PG should not be registered as valid idle
    state.
2. We do have inconsistent view already for platform co-ordinated idle
    In-fact it could happen even with OSC mode I believe. Platform can
    always demote the state, so OS can never get the exact view unless it
    queries the firmware for that explicitly(e.g. PSCI_STATS)

>
> Perhaps a better starting point would be to go with the assumption that a parent
> PD can only enter any idle state once its children are in their deepest idle
> states.
>
> So in the example above we'd end up with
>
> CORE_RET
> CORE_PG
> CORE_PG + CLUSTER_RET
> CORE_PG + CLUSTER_PG
>

Yes this assumption seems good enough to me. At-least no invalid
combination is ensured.

> (Missing out on CORE_RET + CLUSTER_RET, even though that's a valid combination
> from the hardware's perspective)
>

Yes, if it's a real issue then we need proper bindings to deal with
that. Otherwise we can manage without the extra information.

> Then a later addition to the bindings as discussed above could enable the
> possibility of those combinations to be expressed.
>

Seems feasible solution to me, but better to make this explicit in the
binding and check with few others. It looks fair enough assumption IMO.

-- 
-- 
Regards,
Sudeep