Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 9 Jun 2023 10:40:05 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Ulf Hansson <ulf.hansson@linaro.org>
Cc:     Sudeep Holla <sudeep.holla@arm.com>,
        Cristian Marussi <cristian.marussi@arm.com>,
        Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Nikunj Kela <nkela@quicinc.com>,
        Prasad Sodagudi <psodagud@quicinc.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 13/16] OPP: Extend dev_pm_opp_data with OPP provider
 support
Message-ID: <20230609051005.fxuvp4ilcjvhhrkc@vireshk-i7>
References: <20230607124628.157465-1-ulf.hansson@linaro.org>
 <20230607124628.157465-14-ulf.hansson@linaro.org>
 <20230608053446.ngoxh7zo7drnr32z@vireshk-i7>
 <CAPDyKFqUWhdmctKRpQoqwZ2nsx_P2FiWvLzfm_d39QdECWoytA@mail.gmail.com>
 <20230608104540.tykxtvwhoyogthw5@vireshk-i7>
 <CAPDyKFrbpm0W1Hdv+85MqMAC2-UbPOE6qp26L0UvVF0sSL2ayA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAPDyKFrbpm0W1Hdv+85MqMAC2-UbPOE6qp26L0UvVF0sSL2ayA@mail.gmail.com>
Precedence: bulk

On 08-06-23, 13:45, Ulf Hansson wrote:
> Okay, if I understand your point you want to avoid creating OPPs for
> each device, but rather coupling them with the genpd provider's OPP
> table. Right?

Not exactly :)

> Note that, there is no such thing as a "required opp" in the SCMI
> performance protocol case. A device is compatible to use all of the
> OPPs that its corresponding SCMI performance domain provides. Should
> we rename the required opp things in the OPP core to better reflect
> this too?

Not really :)

> That said, we still need to be able to add OPPs dynamically when not
> based on DT. The difference would be that we add the OPPs when
> initializing the genpd provider instead of when attaching the devices.
> In other words, we still need something along the lines of the new
> dev_pm_opp_add_dynamic() API that $subject series is introducing, I
> think.

That's fine.

> Moreover, to tie the consumer device's OPP table to their genpd
> provider's OPP table (call it required-opp or whatever), we need
> another OPP helper function that we can call from the genpd provider's
> ->attach_dev() callback. Similarly, we need to be able to remove this
> connection when genpd's ->detach_dev() callback is invoked. I will
> think of something here.

Something like set/link/config_required_opp()..

> Finally, I want to point out that there is work going on in parallel
> with this, that is adding performance state support for the ACPI PM
> domain. The ACPI PM domain, isn't a genpd provider but implements it's
> own PM domain. The important point is, that it will have its own
> variant of the dev_pm_genpd_set_performance_state() that we may need
> to call from the OPP library.

Okay.

Let me explain how structures are linked currently with help of below (badly
made) drawing. The 'level' fields are only set for Genpd's OPP entries and not
devices. The genpd OPP table normally has only this information, where it
presents all the possible level values, separate OPP for each of them.

Now the devices don't have `level` set in their OPP table, DT or in C
structures. All they have are links for required OPPs, which help us find the
required level or performance state for a particular genpd.

  +-----------------+                    +------------------+
  |  Device A       |                    | GENPD OPP Table  |
  +-----------------+  required-opps     +------------------+   required-opps
  |OPP1: freq: x1   +--------------------+ OPP1:            +--------------+
  +-----------------+                    |      level: 1    |              |
  |OPP2: freq: y1   +-----------+        +------------------+              |
  +-----------------+           |        | OPP2:            +---------+    |
  |OPP3: freq: z1   +--------+  +--------+      level: 2    |         |    |
  +-----------------+        |           +------------------+         |    |
                             |           | OPP3:            +--+      |    |
                             |           |      level: 3    |  |      |    |
                             |           +------------------+  |      |    |
  +-----------------+        |           | OPP4:            |  |      |    |
  |  Device B       |        +-----------+      level: 4    |  |      |    |
  +-----------------+                    +------------------+  |      |    |
  |OPP1: freq: x2   +------------------------------------------+      |    |
  +-----------------+                                                 |    |
  |OPP2: freq: y2   +-------------------------------------------------+    |
  +-----------------+                                                      |
  |OPP3: freq: z2   +------------------------------------------------------+
  +-----------------+


What I am asking you to do now is, create an OPP table for the Genpd first, with
OPPs for each possible level. Now the Genpd layer creates the OPP table for
Device A, where it won't fill the levels, but all other fields and then use a
new API to add required OPPs for the device's OPP, which will point into Genpd's
OPP entries. And with that the existing code will work fine without any other
modifications.

Does this help ?

--

viresh