Re: [PATCH V4 0/5] arm_scmi: vendors: Qualcomm Generic Vendor Extensions

[Cristian Marussi <cristian.marussi@arm.com>]

On Wed, Nov 06, 2024 at 01:55:33PM +0100, Johan Hovold wrote:
> On Mon, Oct 07, 2024 at 11:40:18AM +0530, Sibi Sankar wrote:
> > The QCOM SCMI vendor protocol provides a generic way of exposing a
> > number of Qualcomm SoC specific features (like memory bus scaling)
> > through a mixture of pre-determined algorithm strings and param_id
> > pairs hosted on the SCMI controller. Introduce a client driver that
> > uses the memlat algorithm string hosted on QCOM SCMI Vendor Protocol
> > to detect memory latency workloads and control frequency/level of
> > the various memory buses (DDR/LLCC/DDR_QOS).
> > 
> > QCOM SCMI Generic Vendor protocol background:
> > It was found that a lot of the vendor protocol used internally was
> > for debug/internal development purposes that would either be super
> > SoC specific or had to be disabled because of some features being
> > fused out during production. This lead to a large number of vendor
> > protocol numbers being quickly consumed and were never released
> > either. Using a generic vendor protocol with functionality abstracted
> > behind algorithm strings gave us the flexibility of allowing such
> > functionality exist during initial development/debugging while
> > still being able to expose functionality like memlat once they have
> > matured enough. The param-ids are certainly expected to act as ABI
> > for algorithms strings like MEMLAT.
> 
> I wanted to give this series a quick spin on the x1e80100 CRD, but ran
> into some issues.
> 
> First, I expected the drivers to be loaded automatically when built as
> modules, but that did not happen so something appears to be missing.
> 

Hi Johan,

so the SCMI stack is fully modularizable as of this release, i.e.

 - SCMI core (scmi-core + scmi-module)
 - SCMI transports (scmi_transport_{mailbox,virtio,smc,optee}
 - optional SCMI Vendor protos
 - Std and Vendor SCMI Drivers on top of protos

....on the other side the SCMI standard protocols are still embedded
in scmi-module (or builtin) as of now...

Even though, module usage is tracked by the core AND when an SCMI Vendor
driver tries to use protocol_X, it causes protocol_X to be initialized
(calling its protocol_init), there is NO auto-loading for SCMI Vendor
Protocols when bult as LKM...because there were really no ask till now
and this stuff is in general needed so very early dburing boot...so the
usecase of all these LKM modules is just debug/test as in your case
(or in mine frequently)....

...and I am NOT saying with this that is necessarily right or must be
stay like this...just explaining how it is now....

....anyway...it is mostly trivial to add vendor/protocols autoloading
transparently...today I was experimenting with a patch that triggers
autoloading based on a generic common alias pattern in the form
'scmi-protocol-0x<NN>' (with NN the specific protocol ID of course)
that triggers the loading as soon as the SCMI Vendor driver tries to
access the protocol during its probe...

....I will post it for the next cycle once it is clean.
(unless I am missing something else that you want to add...)

> Second, after loading the protocol and client drivers manually (in that
> order, shouldn't the client driver pull in the protocol?), I got:
> 
> 	scmi_module: Loaded SCMI Vendor Protocol 0x80 - Qualcomm  20000
> 	arm-scmi arm-scmi.0.auto: QCOM Generic Vendor Version 1.0
> 	scmi-qcom-generic-ext-memlat scmi_dev.5: error -EOPNOTSUPP: failed to configure common events
> 	scmi-qcom-generic-ext-memlat scmi_dev.5: probe with driver scmi-qcom-generic-ext-memlat failed with error -95
> 
> which seems to suggest that the firmware on my CRD does not support this
> feature. Is that the way this should be interpreted? And does that mean
> that non of the commercial laptops supports this either?

This seems like FW rejecting the command, maybe just only for the specific
Linux OSPM agent since it is not allowed to ask for that specific setup,
and only Sibi can shed a light here :D

...but this Vendor protocol, if I am not mistaken, AFAIU, uses a bunch
of "algo strings" coming from tokens it picks from DT and use thsoe as
params for the set_param() VendorProtocol ops...cannot be that a bad/missing
DT value could cause the FW to reject the command due to the params ?
(even if the command is supported)...

...just a guess ah... I have no real knowledge of this venndor proto...


Thanks,
Cristian