Using scmi performance domains and scmi power domains together

[Ben Horgan]

On 1/23/24 10:33, Sudeep Holla wrote:
> (+ 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 at 11 {
>>          reg = <0x11>;
>>          #power-domain-cells = <1>;
>> };
>>
>> scmi_dvfs: protocol at 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 😄.

My motivation here is simply trying to avoid having hacky custom changes 
for our platform in either linux or any other drivers. I started looking 
at this before the recent changes to make using the scmi perf domain 
from linux for devices proper and was under the misapprehension that 
there was even more work to be done to make this even better. Naively, I 
thought as a platform device driver can work with a single power domain 
using either the scmi_perf_domain driver or the scmi_power_domain driver 
and they use separate callbacks that a way to use them together was in 
the works. Getting a NACK on this means that I now know that the missing 
piece is in the driver and we can get the work done there.

>
>> B. Combine on every pair
>>
>> scmi_pm_perf: protocol at 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 at 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.
Good to know. I will get this fixed in our platform.
>
> Unless I hear strong technical reasons to this approach other than simplicity,
> I am inclined towards opposing this proposal.
>