On 12 October 2018 at 20:16, Ulf Hansson <[email protected]> wrote: > On 12 October 2018 at 13:11, Viresh Kumar <[email protected]> wrote: >> Multiple generic power domains for a consumer device are supported with >> the help of virtual devices, which are created for each consumer device >> - genpd pair. These are the device structures which are attached to the >> power domain and are required by the OPP core to set the performance >> state of the genpd. >> >> The helpers added by this commit are required to be called once for each >> of these virtual devices. These are required only if multiple domains >> are available for a device, otherwise the actual device structure will >> be used instead by the OPP core. >> >> The new helpers also support the complex cases where the consumer device >> wouldn't always require all the domains. For example, a camera may >> require only one power domain during normal operations but two during >> high resolution operations. The consumer driver can call >> dev_pm_opp_put_genpd_device(high_resolution_genpd_dev) if it is >> currently operating in the normal mode and doesn't have any performance >> requirements from the genpd which manages high resolution power >> requirements. The consumer driver can later call >> dev_pm_opp_set_genpd_device(high_resolution_genpd_dev) once it switches >> back to the high resolution mode. >> >> The new helpers differ from other OPP set/put helpers as the new ones >> can be called with OPPs initialized for the table as we may need to call >> them on the fly because of the complex case explained above. For this >> reason it is possible that the genpd_device structure may be used in > > This is a bit unclear. What is really the genpd_device structure? > Could you please clarify that here?
It is the virtual device structures created by genpd. >> parallel while the new helpers are running and a new mutex is added to >> protect against that. We didn't use the existing opp_table->lock mutex >> as that is widely used in the OPP core and we will need a lock in the >> hotpath now, i.e. while changing OPP and we need to make sure there is >> not much contention while doing that. > > I not sure this needs to be considered as hotpath. I would be > surprised if changing genpd virtual devices for OPP, is something that > is going to be done frequently. Rather, this is more depending on the > use case, like the camera case you describe above. > > In other words, do you really need a new lock? My bad. I didn't explain it well. If you see a later patch where we started configuring the required OPPs, we use genpd_dev or virt-dev within opp_set_rate() which also needs this lock. And that is the hot path. -- viresh
