05.11.2020 18:22, Dmitry Osipenko пишет: > 05.11.2020 12:45, Ulf Hansson пишет: > ... >> I need some more time to review this, but just a quick check found a >> few potential issues... > > Thank you for starting the review! I'm pretty sure it will take a couple > revisions until all the questions will be resolved :) > >> The "core-supply", that you specify as a regulator for each >> controller's device node, is not the way we describe power domains. >> Instead, it seems like you should register a power-domain provider >> (with the help of genpd) and implement the ->set_performance_state() >> callback for it. Each device node should then be hooked up to this >> power-domain, rather than to a "core-supply". For DT bindings, please >> have a look at Documentation/devicetree/bindings/power/power-domain.yaml >> and Documentation/devicetree/bindings/power/power_domain.txt. >> >> In regards to the "sync state" problem (preventing to change >> performance states until all consumers have been attached), this can >> then be managed by the genpd provider driver instead. > > I'll need to take a closer look at GENPD, thank you for the suggestion. > > Sounds like a software GENPD driver which manages clocks and voltages > could be a good idea, but it also could be an unnecessary > over-engineering. Let's see.. > Hello Ulf and all, I took a detailed look at the GENPD and tried to implement it. Here is what was found: 1. GENPD framework doesn't aggregate performance requests from the attached devices. This means that if deviceA requests performance state 10 and then deviceB requests state 3, then framework will set domain's state to 3 instead of 10. https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/base/power/domain.c#L376 2. GENPD framework has a sync() callback in the genpd.domain structure, but this callback isn't allowed to be used by the GENPD implementation. The GENPD framework always overrides that callback for its own needs. Hence GENPD doesn't allow to solve the bootstrapping state-synchronization problem in a nice way. https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/base/power/domain.c#L2606 3. Tegra doesn't have a dedicated hardware power-controller for the core domain, instead there is only an external voltage regulator. Hence we will need to create a phony device-tree node for the virtual power domain, which is probably a wrong thing to do. === Perhaps it should be possible to create some hacks to work around bullets 2 and 3 in order to achieve what we need for DVFS on Tegra, but bullet 1 isn't solvable without changing how the GENPD core works. Altogether, the GENPD in its current form is a wrong abstraction for a system-wide DVFS in a case where multiple devices share power domain and this domain is a voltage regulator. The regulator framework is the correct abstraction in this case for today.
