linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Sudeep Holla <sudeep.holla@arm.com>
To: Ben Horgan <Ben.Horgan@arm.com>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, nd <nd@arm.com>,
	Vishnu Banavath <Vishnu.Banavath@arm.com>,
	Florent Tomasin <Florent.Tomasin@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Cristian Marussi <cristian.marussi@arm.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>
Subject: Re: Using scmi performance domains and scmi power domains together
Date: Tue, 23 Jan 2024 10:33:44 +0000	[thread overview]
Message-ID: <Za-WCHwfpf7dXJ1Z@bogus> (raw)
In-Reply-To: <VE1PR08MB476848A0895993EAC92AF77B91752@VE1PR08MB4768.eurprd08.prod.outlook.com>

(+ Cristian)

On Mon, Jan 22, 2024 at 08:27:03PM +0000, Ben Horgan wrote:
> Hi,
>
> I've been looking at adding support in total compute, an arm reference
> platform, to control the gpu operating points and gpu power on/off via scmi.
> This was previously done for the juno platform but involved hacks. I would
> like to make sure this is cleaner going forward.
>
> For device driver simplicity it would be good if a device with a single
> power domain and a single performance domain could just use a single PM
> domain.

Do you have any other technical reason for this other than simplicity ?
We can't always have to so simple when managing to support wide variety
of platforms with standards like SCMI.

You need to justify why it is hard for the driver if there are 2 genpd
domains associated with a device(power and perf genpds).

> Using a single PM domain means this can be on the platform device
> and you don't need to create virtual devices. The drivers scmi_pm_domain and
> scmi_perf_domain both initialize a separate 'struct generic_pm_domain genpd'
> for each of the corresponding scmi domains. Possibly, there could be some
> way to bring these together under a single genpd domain. Possible options
> are:
> 
> A. Parent power domains with a helper driver that just uses an empty genpd
> domain as the child of both the genpd performance domain and the genpd power
> domain.
> B. Combine the scmi_pm_domain and scmi_perf_domain driver and create a
> 'struct generic_pm_domain genpd' for every pair of power domain and
> performance domain.

This is purely software implementation and expect no change in the firmware
(DT) representation of these domains and association with the device.

> C. Combine the scmi_pm_domain and scmi_perf_domain driver but only create
> the 'struct generic_pm_domain genpd' for the power domain combinations that
> are used.

Not possible unless the specification assures the power domain and the
performance domain IDs match.

> D. Keep things as they are and use separate PM domains for performance and
> power when using scmi.

+1, it was designed this way to ensure it addresses all the possible
implementations using SCMI.

>
> Examples of possible ways of expressing these options in the device tree,
> the scmi performance domain is 3 and the scmi power domain is 8.
>
> A.
>
> scmi_devpd: protocol@11 {
>         reg = <0x11>;
>         #power-domain-cells = <1>;
> };
>
> scmi_dvfs: protocol@13 {
>         reg = <0x13>;
>         #power-domain-cells = <1>;
> };
>
> perf_and_performance: perf_and_performance {
>         power-domain-names = "perf", "power";
>         power-domain = <&scmi_dvfs 3>, <&scmi_devpd 8>;
> };
>
> my_device : my_device  {
>         power-domain = <&perf_and_performance>
> };
>

NACK as I mentioned, we need to keep DT representation as minimal as
possible, adding nodes for this virtual domain is a no go IMO. Just
use the existing binding to create this virtual genpd at which point you
may realise handling 2 genpd in the driver may not be so hard 😄.

> B. Combine on every pair
>
> scmi_pm_perf: protocol@11_13 {
>         reg = <0x11>, <0x13>;
>         #power-domain-cells = <2>;
> };
>
> my_device : my_device {
>         power-domain = <&scmi_pm_perf 8 3>
> };
>

Again big fat NACK as above. No change in the binding to make it confusing.

> C. Combine on used pairs
>
> scmi_pm_perf: protocol@11_13 {
>         reg = <0x11>, <0x13>;
>         #power-domain-cells = <2>;
>        used-domains = <8, 3>, <9, 4>;
> };
>
> my_device : my_device {
>         power-domain = <&scmi_pm_perf 8 3>
> };
>

At this point I give up and will just say I would expect no change in the
DT bindings to achieve whatever you are terming as "simple" here. We are
not going to add any bindings to make it easy or simple for OS to implement
it's policy.

> It seems wasteful that the scmi_pm_domain sets up and makes scmi calls for
> all possibly usable domains at start up even those that aren't controllable
> by linux. E.g. cpus may use scmi power domain controlled via psci.
>


Not an OS issue. If the power domain is purely controlled by PSCI agent, why
is it even present to OS as SCMI power domain. We have examples where it
is correctly presented as PSCI power domain. So this issue doesn't exist,
fix the SCMI platform firmware. It needs to present per agent view correctly
and not present a global system view to all the agents.

Unless I hear strong technical reasons to this approach other than simplicity,
I am inclined towards opposing this proposal.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-01-23 10:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-22 20:27 Using scmi performance domains and scmi power domains together Ben Horgan
2024-01-23 10:33 ` Sudeep Holla [this message]
2024-01-24  9:55   ` Ben Horgan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Za-WCHwfpf7dXJ1Z@bogus \
    --to=sudeep.holla@arm.com \
    --cc=Ben.Horgan@arm.com \
    --cc=Florent.Tomasin@arm.com \
    --cc=Vishnu.Banavath@arm.com \
    --cc=cristian.marussi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nd@arm.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).