Re: [PATCH v4] dt-bindings: dvfs: Add support for generic performance domains

From: Sudeep Holla
Date: Thu Oct 21 2021 - 11:35:49 EST


On Thu, Oct 21, 2021 at 03:34:17PM +0200, Ulf Hansson wrote:
> On Wed, 20 Oct 2021 at 12:25, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:

[...]

> >
> > Looks like this can be used for devices but what about CPUs ?
>
> Yes, that should work. dev_pm_genpd_set_performance_state() takes a
> struct device* as an in-parameter.
>
> The struct device to use would typically be the one that you receive
> from dev_pm_domain_attach_by_name(). We already do this for some other
> cpufreq drivers, so this works fine.
>

I totally understand this is possible but the question is: is it really
necessary ?

> >
> > > >
> > > > I am happy to know if there are ways to support such systems with the
> > > > options you have mentioned above.
> > >
> > > As far as I understand, the "performance domains" DT bindings that
> > > $subject patch introduces, allows us to group devices into domains, to
> > > let them be "performance controlled" together. Right?
> > >
> >
> > Or independently. It doesn't matter.
> >
> > > Unless I am missing something, it looks like power domains DT bindings
> > > already offer this for us. Yes, certainly, the DT doc [1] needs an
> > > updated description to better explain this, but other than that we
> > > should be fine, don't you think?
> > >
> >
> > As I mentioned about, the main question is what if firmware doesn't
> > want to expose power domain details to OSPM like PC co-ordinated PSCI
> > idle states while it wants to either group CPUs or leave them as
> > individual in order to get per-CPU DVFS requests and aggregate them
> > in the firmware. It does something similar for idle states already.
>
> Yes, that can be modeled too.
>
> Just let each CPU node point to its own separate power-domain and also
> *don't* model the parent power-domain, instead leave this to be
> managed by the FW.
>

Why ? IMO it is unnecessary indirection and useless as we don't need the
genpd created for these for no useful reasons. All we need is to get the
domain id for the device and request the firmware passing that id to set
the performance. Creating a psuedo power domain with no power domain
info except the performance domain id and then creating genpd domains
for the same just because we can is something I am failing to understand
here. I agree if the CPU domains were real(as in what f/w or h/w represents),
then may be, again I really don't like them disconnected from real CPU
power domains just because perf and power are not 1:1. Creating one set
of genpd for power and another for performance is also not okay IMO for
reasons stated above.

--
Regards,
Sudeep