On 11-07-24, 13:05, Ulf Hansson wrote: > On Thu, 11 Jul 2024 at 12:31, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > > > On Thu, 11 Jul 2024 at 05:13, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote: > > > > > > On 10-07-24, 15:51, Ulf Hansson wrote: > > > > I think this should work, but in this case we seem to need a similar > > > > thing for dev_pm_opp_set_rate(). > > > > > > We don't go to that path for genpd's I recall. Do we ? For genpd's, > > > since there is no freq, we always call _set_opp(). > > > > You are right! Although, maybe it's still preferred to do it in > > _set_opp() as it looks like the code would be more consistent? No? Since the function already accepted a flag, it was very easier to just reuse it without. > No matter how we do this, we end up enforcing OPPs for genpds. > > It means that we may be requesting the same performance-level that we > have already requested for the device. Of course genpd manages this, > but it just seems a bit in-efficient to mee. Or maybe this isn't a big > deal as consumer drivers should end up doing this anyway? Normally I won't expect a consumer driver to do this check and so was the opp core handling that. But for genpd's we need to make this inefficient to not miss a vote. The problem is at another level though. Normally for any other device, like CPU, there is one vote for the entire range of devices supported by the OPP table. For example all CPUs of a cluster will share an OPP table (and they do dvfs together), and you call set_opp() for any of the CPU, we will go and change the OPP. There is no per-device vote. This whole design is broken in case of genpd, since you are expecting a separate vote per device. Ideally, each device should have had its own copy of the OPP table, but it is messy in case of genpd and to make it all work nicely, we may have to choose this inefficient way of doing it :( -- viresh