Re: [PATCH v8 01/34] opp: Add dev_pm_opp_sync() helper

Viresh Kumar
2021-08-20T05:18:51Z

On 19-08-21, 16:55, Ulf Hansson wrote:
> Right, that sounds reasonable.
>
> We already have pm_genpd_opp_to_performance_state() which translates
> an OPP to a performance state. This function invokes the
> ->opp_to_performance_state() for a genpd. Maybe we need to allow a
> genpd to not have ->opp_to_performance_state() callback assigned
> though, but continue up in the hierarchy to see if the parent has the
> callback assigned, to make this work for Tegra?
>
> Perhaps we should add an API dev_pm_genpd_opp_to_performance_state(),
> allowing us to pass the device instead of the genpd. But that's a
> minor thing.

I am not concerned a lot about how it gets implemented, and am not
sure as well, as I haven't looked into these details since sometime.
Any reasonable thing will be accepted, as simple as that.

> Finally, the precondition to use the above, is to first get a handle
> to an OPP table. This is where I am struggling to find a generic
> solution, because I guess that would be platform or even consumer
> driver specific for how to do this. And at what point should we do
> this?

Hmm, I am not very clear with the whole picture at this point of time.

Dmitry, can you try to frame a sequence of events/calls/etc that will
define what kind of devices we are looking at here, and how this can
be made to work ?

> > > Viresh, please take a look at what I did in [1]. Maybe it could be done
> > > in another way.
> >
> > I looked into this and looked like too much trouble. The
> > implementation needs to be simple. I am not sure I understand all the
> > problems you faced while doing that, would be better to start with a
> > simpler implementation of get_performance_state() kind of API for
> > genpd, after the domain is attached and its OPP table is initialized.
> >
> > Note, that the OPP table isn't required to be fully initialized for
> > the device at this point, we can parse the DT as well if needed be.
>
> Sure, but as I indicated above, you need some kind of input data to
> figure out what OPP table to pick, before you can translate that into
> a performance state. Is that always the clock rate, for example?

Eventually it can be clock, bandwidth, or pstate of anther genpd, not
sure what all we are looking for now. It should be just clock right
now as far as I can imagine :)

> Perhaps, we should start with adding a dev_pm_opp_get_from_rate() or
> what do you think? Do you have other suggestions?

We already have similar APIs, so that won't be a problem. We also have
a mechanism inside the OPP core, frequency based, which is used to
guess the current OPP. Maybe we can enhance and use that directly
here.

--
viresh