Re: [RFC 2/2] firmware: arm_scmi: Add SCMI QTI Memlat vendor protocol

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Sudeep,

Thanks for taking time to review the series.

On 11/3/22 16:07, Sudeep Holla wrote:
On Thu, Nov 03, 2022 at 10:24:44AM +0000, Sudeep Holla wrote:
On Thu, Nov 03, 2022 at 10:28:32AM +0530, Sibi Sankar wrote:
Add support for the SCMI QTI memlat (memory latency) vendor protocol.
The QTI memlat vendor protocol takes in several tuneables including the
IPM ratio (Instructions Per Miss), bus bandwidth requirements and PMU
maps to enable frequency scaling of various buses (L3/LLCC/DDR) performed
by the memory latency governor running on the CPUSS Control Processor.

Signed-off-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx>
---
  drivers/firmware/arm_scmi/Kconfig              |  10 +
  drivers/firmware/arm_scmi/Makefile             |   1 +
  drivers/firmware/arm_scmi/qcom_memlat_vendor.c | 269 +++++++++++++++++++++++++
  include/linux/scmi_protocol.h                  |  36 ++++
  4 files changed, 316 insertions(+)
  create mode 100644 drivers/firmware/arm_scmi/qcom_memlat_vendor.c

diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index a14f65444b35..814a3fc37dc1 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -136,6 +136,16 @@ config ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE
endif #ARM_SCMI_PROTOCOL +config QTI_SCMI_MEMLAT_PROTOCOL
+	tristate "Qualcomm Technologies, Inc. SCMI MEMLAT vendor Protocol"
+	depends on ARM_SCMI_PROTOCOL && QCOM_CPUCP_MBOX

If you have set the transport correctly, there should be no need for any
such dependency.

ack


+	help
+	  The SCMI QTI memlat vendor protocol adds support for the frequency
+	  scaling of buses (L3/LLCC/DDR) by the QTI HW memlat governor running
+	  on the CPUSS Control Processor (CPUCP).
+
+	  Say Y here if you want to build this driver.
+

I don't think it is scalable to have a config option for each vendor+protocol
Kconfig. IMO just one config for all qcom vendor protocol please.

ditto



Sorry pressed send too early before I could write the main part :(.
Can you please also add the driver using this protocol in the next revision.
What framework does that fit in ? Devfreq ? I am very much interested in
that as it helps in distributing the responsibility across these correctly.
I think that could be one of the reason I don't like all the information
dump you have in the DT binding proposed in the provider node. It needs to
move out but in order to understand where to, we need full picture here.
So please provide the same.


As of now it would just be a simple SoC platform driver which passes on
the tuneables and starts the memlat governor on the SCP firmware. I'll
include it in the next re-spin.


Also it doesn't hurt to describe in detail: what theses "several tuneables"
are and where are they expected to arrive from or targeted to ?

ack

Is CPUSS Control Processor responsible for CPU DVFS or not ?
Does it just control DVFS of L3/LLCC and DDR or is there a bigger list ?

CPUSS CP is responsible for CPU DVFS but it's not done through
SCMI (it's done through the qcom-cpufreq-hw driver). Memory latency
protocol is meant to control the DVFS of memory buses. So it could
be one or all of them and the supported list for that particular
SoC was meant to be passed through the qcom,bus-type array as
described in the bindings.

- Sibi

All these information matters as your current DT proposal seem to be
tightly coupled with only few of these.

--
Regards,
Sudeep



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux