Hi Lina, Apologies, I sent this reply before and automatically included an "IMPORTANT NOTICE" footer, please disregard that email, here's the same thing without the footer. On Thu, Aug 11, 2016 at 03:10:23PM -0600, Lina Iyer wrote: > On Wed, Aug 10 2016 at 12:09 -0600, Sudeep Holla wrote: > > > > > >On 10/08/16 17:40, Lina Iyer wrote: > >>Hi Sudeep, > >> > >>On Wed, Aug 10 2016 at 09:15 -0600, Sudeep Holla wrote: > >>>Hi Lina, > >>> > >>>I have few concerns mainly due to the lack of description and not the > >>>binding per say. > > > >[...] > > > >>It is pretty clear that CPUs cannot not define the domain idle states. > >>Domains define their own idle states. Just as you mention above. CPU is > >>just a single component in its domain. There may be other devices like > >>PMUs, Coresights etc that also may have a say in the idle state the > >>domain may be put in, when the devices are idle. As such, adding domain > >>idle states to the CPU's idle state property is not appropriate. > >> > > > >No I am not saying we need to add domain idle states to the CPU's idle > >state property. I am saying we need to remove cpu-idle-states or ignore > >it when PD is present. And get all the idle state information for PD. > > > >I am objecting the split we are creating across CPU and higher level > >power domains. And this binding document is incomplete as it skips all > >those details. We just need PD handle in CPU and no idle state > >information there. Create PD hierarchy and have all idle state > >information at one place. > > > Let me think about this a bit and see what I can come up with. > > >>Our kernel has runtime PM for devices and then there is CPUidle, both > >>are diverging without one knowing about the other. We have to start > >>unifying them inorder to have better holistic power management in the > >>SoC. To that regard, we have to start imagining CPUs as just another > >>device, albeit a special device. But for our purposes in determining > >>domain idle state, it will just be a device attached to the domain. > >> > > > >Absolutely agree on that. No arguments. I am asking to go a step ahead > >to include even cpu/core level power domains not just cluster/higher > >level domains. > > > >>>We need to have all the idle state information at one place and in this > >>>case PD seems more appropriate instead of splitting them across. > >>> > >>That approach isn't correct. Where will we put the idle states of other > >>devices that are also part of the domain? We are thinking about a model, > >>where every device defines its own idle states and we define > >>relationships between those idle states and their parents' idle states. > > > >Yes I understand. You confused me here. Won't that be one-to-one > >relationship ? If not, how is that dealt in the current bindings ? > > > >>Ofcourse, devices don't have idle states today, but that is something we > >>have been pondering over. > >> > > > >Yes we these binding should be easily extensible, I don't see any issue. > > > >>>We can also keep the code clean and not break compatibility. Whenever > >>>both PD and CPU contains idle-states, PD must take precedence. > >>> > >>Why? > >>The CPU and PD states are orthogonal. While the PD state is dependent on > >>the CPU state, the latter is not true. Devices determine their own > >>states. Based on the individual device states, we then determine the > >>state of the parent and bubble up on the hierarchy. > >> > > > >I may be missing something. Now with your example in the binding, if > >another device shares the cluster PD, can it have different idle states? > >If so how does it map ? > > > > > >In general whatever binding we come up must not just address OS > >coordinated mode. Also I was thinking to have better coverage in > >the description by having a bit more complex system like: > > > >cluster0 > > CLUSTER_RET(Retention) > > CLUSTER_PG(Power Gate) > > core0 > > CORE_RET > > CORE_PG > > core1 > > CORE_RET > > CORE_PG > > > >cluster1 > > CLUSTER_RET > > CLUSTER_PG > > core0 > > CORE_RET > > CORE_PG > > core1 > > CORE_RET > > CORE_PG > > > >Platform Co-ordinate supports the following states and we should > >be able to determine that from the binding: > > > >CORE_RET > >CORE_PG > >CORE_RET + CLUSTER_RET > > The problem that we have to sove here is knowing that CORE_RET + > CLUSTER_PG (hypothetically) an invalid combination. Kevin and > I debated it in the earlier RFC and we dont have a good way to solve > this generically for all devices. > This is interesting. I had been working on the assumption that a parent power domain cannot enter any idle state until its children were all in their deepest idle state. I now realise that it's easy to imagine platforms where this isn't the case. However, I don't understand how your current bindings solve this issue and why using domain-power-states for all states (i.e. ignoring cpu-idle-states and putting CPU idle states in the domain-idle-states of a per-CPU power domain - I believe this is what Sudeep is suggesting) makes it any more difficult. Could you link to this previous discussion you mentioned? I'm having trouble finding it (R.I.P Gmane). > >CORE_PG + CLUSTER_RET > >CORE_PG + CLUSTER_PG > > Cheers, Brendan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
