On 10/7/24 23:36, Dmitry Baryshkov wrote:
On Mon, Oct 07, 2024 at 11:40:19AM GMT, Sibi Sankar wrote:
Document the various memory buses that can be monitored and scaled by
the memory latency governor hosted by the QCOM SCMI Generic Extension
Protocol v1.0.
Signed-off-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx>
Hey Dmitry,
Thanks for taking time to review the series!
---
v3:
* Restructure the bindings to mimic IMX [Christian]
.../bindings/firmware/arm,scmi.yaml | 1 +
.../bindings/firmware/qcom,scmi-memlat.yaml | 246 ++++++++++++++++++
.../dt-bindings/firmware/qcom,scmi-memlat.h | 22 ++
3 files changed, 269 insertions(+)
create mode 100644 Documentation/devicetree/bindings/firmware/qcom,scmi-memlat.yaml
create mode 100644 include/dt-bindings/firmware/qcom,scmi-memlat.h
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 54d7d11bfed4..1d405f429168 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -24,6 +24,7 @@ description: |
anyOf:
- $ref: /schemas/firmware/nxp,imx95-scmi.yaml
+ - $ref: /schemas/firmware/qcom,scmi-memlat.yaml
properties:
$nodename:
diff --git a/Documentation/devicetree/bindings/firmware/qcom,scmi-memlat.yaml b/Documentation/devicetree/bindings/firmware/qcom,scmi-memlat.yaml
new file mode 100644
index 000000000000..0e8ea6dacd6a
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/qcom,scmi-memlat.yaml
@@ -0,0 +1,246 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/qcom,scmi-memlat.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SCMI Memory Bus nodes
+
+maintainers:
+ - Sibi Sankar <quic_sibis@xxxxxxxxxxx>
+
+description:
+ This binding describes the various memory buses that can be monitored and scaled
+ by memory latency governor running on the CPU Control Processor (SCMI controller).
+
+properties:
+ protocol@80:
+ $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ const: 0x80
+
+ patternProperties:
+ '^memory-[0-9]$':
+ type: object
+ unevaluatedProperties: false
+ description:
+ The list of all memory buses that can be monitored and scaled by the
+ memory latency governor running on the SCMI controller.
+
+ properties:
+ qcom,memory-type:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2]
+ description: |
+ Memory Bus Identifier
+ 0 = QCOM_MEM_TYPE_DDR
+ 1 = QCOM_MEM_TYPE_LLCC
+ 2 = QCOM_MEM_TYPE_DDR_QOS
I'm sorry if this has been discussed and frowned upon, but can you
squash memory type into device node?
I don't think anyone had any strong opinions on how the
nodes is to be named. We went with a generic node name since
it could accomodate multiple instances of llcc or ddr in the
future. We didn't want it be named ddr-0/ddr-1 and so on. So
I'll continue to stick with the current naming unless you have
a strong reason other than readability.
protocol@80 {
ddr {
};
llcc {
};
ddr-qos {
};
};
+
+ freq-table-hz:
+ items:
+ items:
+ - description: Minimum frequency of the memory bus in Hz
+ - description: Maximum frequency of the memory bus in Hz
Does it make sense for the DDR-QOS type? Can we hardcode those values
and drop freq-table-hz from the DDR-QOS node?
Also, can we drop this completely by adding one extra OPP entry with the
minimum memory bus frequency?
the map table doesn't necessarily list all the supported
frequencies. It was made that way so that the table is flexible
enough that it doesn't have to be changed a lot across platforms.
Hence a need for a separate property to list min/max freq.
+
+ patternProperties:
+ '^monitor-[0-9]$':
+ type: object
+ unevaluatedProperties: false
+ description:
+ The list of all monitors detecting the memory latency bound workloads using
+ various counters.
+
+ properties:
+ qcom,compute-type:
+ description:
+ Monitors of type compute perform bus dvfs based on a rudimentary CPU
+ frequency to memory frequency map.
+ type: boolean
This seems to be redundant. If there is no qcom,ipm-ceil property, then
it's qcom,compute-type, isn't it?
ack
+
+ qcom,ipm-ceil:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Monitors having this property perform bus dvfs based on the same
+ rudimentary table but the scaling is performed only if the calculated
+ IPM (Instruction Per Misses) exceeds the given ceiling.
+
+ cpus:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description:
+ Should be a list of phandles to CPU nodes (as described in
+ Documentation/devicetree/bindings/arm/cpus.yaml).
Which CPU nodes? I see that the examples list all CPUs here. Do we
really need them?
This observation is only valid for X1E where all the cpus have
identical freq charecteristics. Even with this case we need to
list them to handle cases where cpus gets disabled by the bootloader
on lower cored X1E parts i.e. we use this to figure out the actual
physical mask.
+
+ operating-points-v2: true
+ opp-table:
+ type: object
+
+ required:
+ - cpus
+ - operating-points-v2
+
+ oneOf:
+ - required: [ 'qcom,compute-type' ]
+ - required: [ 'qcom,ipm-ceil' ]
+
+ required:
+ - qcom,memory-type
+ - freq-table-hz
+
+additionalProperties: true
+
+examples:
+ - |
+ #include <dt-bindings/firmware/qcom,scmi-memlat.h>
+
+ firmware {
+ scmi {
+ compatible = "arm,scmi";
+ mboxes = <&cpucp_mbox 0>, <&cpucp_mbox 2>;
+ mbox-names = "tx", "rx";
+ shmem = <&cpu_scp_lpri0>, <&cpu_scp_lpri1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ protocol@80 {
+ reg = <0x80>;
+
+ memory-0 {
+ qcom,memory-type = <QCOM_MEM_TYPE_DDR>;
+ freq-table-hz = /bits/ 64 <200000000 4224000000>;
+
+ monitor-0 {
Hmm. Can we say that each memory type can have at most one IPM and one
compute aka "passive" memlat monitor? Does it make sense to use them as
node names and drop the extra monitor-M names?
Again this observation is valid only for X1E where the cpu freq
across cpu's are identical across clusters and is not true for
other mobile SoCs. So each memory can have more than 2 monitors
i.e. atleast one active/passibe monitor for each cluster.
+ qcom,ipm-ceil = <20000000>;
+ cpus = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7
+ &CPU8 &CPU9 &CPU10 &CPU11>;
Are CPU lists different between monitors? Can they be different? Can
they be different between different memory types?
same explanation.
+ operating-points-v2 = <&memory0_monitor0_opp_table>;
+
+ memory0_monitor0_opp_table: opp-table {
sensible names are better:
I think I just picked these names up from a cpufreq table upstream.
ddr_ipm_opp_table: opp-table {
};
+ compatible = "operating-points-v2";
+
+ opp-999000000 {
+ opp-hz = /bits/ 64 <999000000 547000000>;
+ };
+
+ opp-1440000000 {
+ opp-hz = /bits/ 64 <1440000000 768000000>;
+ };
+
+ opp-1671000000 {
+ opp-hz = /bits/ 64 <1671000000 1555000000>;
+ };
+
+ opp-2189000000 {
+ opp-hz = /bits/ 64 <2189000000 2092000000>;
+ };
+
+ opp-2516000000 {
+ opp-hz = /bits/ 64 <2516000000 3187000000>;
+ };
+
+ opp-3860000000 {
+ opp-hz = /bits/ 64 <3860000000 4224000000>;
+ };
+ };
+ };
+
+ monitor-1 {
+ qcom,compute-type;
+ cpus = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7
+ &CPU8 &CPU9 &CPU10 &CPU11>;
+ operating-points-v2 = <&memory0_monitor1_opp_table>;
+
+ memory0_monitor1_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-1440000000 {
+ opp-hz = /bits/ 64 <1440000000 200000000>;
+ };
+
+ opp-2189000000 {
+ opp-hz = /bits/ 64 <2189000000 768000000>;
+ };
+
+ opp-2516000000 {
+ opp-hz = /bits/ 64 <2516000000 1555000000>;
+ };
+
+ opp-3860000000 {
+ opp-hz = /bits/ 64 <3860000000 4224000000>;
+ };
+ };
+ };
+ };
+
+ memory-1 {
+ qcom,memory-type = <QCOM_MEM_TYPE_LLCC>;
+ freq-table-hz = /bits/ 64 <300000000 1067000000>;
+
+ monitor-0 {
+ qcom,ipm-ceil = <20000000>;
+ cpus = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7
+ &CPU8 &CPU9 &CPU10 &CPU11>;
+ operating-points-v2 = <&memory1_monitor0_opp_table>;
+
+ memory1_monitor0_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-999000000 {
+ opp-hz = /bits/ 64 <999000000 300000000>;
+ };
+
+ opp-1440000000 {
+ opp-hz = /bits/ 64 <1440000000 466000000>;
+ };
+
+ opp-1671000000 {
+ opp-hz = /bits/ 64 <1671000000 600000000>;
+ };
+
+ opp-2189000000 {
+ opp-hz = /bits/ 64 <2189000000 806000000>;
+ };
+
+ opp-2516000000 {
+ opp-hz = /bits/ 64 <2516000000 933000000>;
+ };
+
+ opp-3860000000 {
+ opp-hz = /bits/ 64 <3860000000 1066000000>;
+ };
+ };
+ };
+ };
+
+ memory-2 {
+ qcom,memory-type = <QCOM_MEM_TYPE_DDR_QOS>;
+ freq-table-hz = /bits/ 64 <QCOM_DDR_LEVEL_AUTO QCOM_DDR_LEVEL_PERF>;
This is definitely not 'frequency of the memory bys in Hz'
ack, will change it.
-Sibi
+
+ monitor-0 {
+ qcom,ipm-ceil = <20000000>;
+ cpus = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7
+ &CPU8 &CPU9 &CPU10 &CPU11>;
+ operating-points-v2 = <&memory2_monitor0_opp_table>;
+
+ memory2_monitor0_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-2189000000 {
+ opp-hz = /bits/ 64 <2189000000>;
+ opp-level = <QCOM_DDR_LEVEL_AUTO>;
+ };
+
+ opp-3860000000 {
+ opp-hz = /bits/ 64 <3860000000>;
+ opp-level = <QCOM_DDR_LEVEL_PERF>;
+ };
+ };
+ };
+ };
+ };
+ };
+ };
diff --git a/include/dt-bindings/firmware/qcom,scmi-memlat.h b/include/dt-bindings/firmware/qcom,scmi-memlat.h
new file mode 100644
index 000000000000..7ae8d8d5623b
--- /dev/null
+++ b/include/dt-bindings/firmware/qcom,scmi-memlat.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+#ifndef __DT_BINDINGS_QCOM_SCMI_VENDOR_H
+#define __DT_BINDINGS_QCOM_SCMI_VENDOR
+
+/* Memory IDs */
+#define QCOM_MEM_TYPE_DDR 0x0
+#define QCOM_MEM_TYPE_LLCC 0x1
+#define QCOM_MEM_TYPE_DDR_QOS 0x2
+
+/*
+ * QCOM_MEM_TYPE_DDR_QOS supports the following states.
+ *
+ * %QCOM_DDR_LEVEL_AUTO: DDR operates with LPM enabled
+ * %QCOM_DDR_LEVEL_PERF: DDR operates with LPM disabled
+ */
+#define QCOM_DDR_LEVEL_AUTO 0x0
+#define QCOM_DDR_LEVEL_PERF 0x1
+
+#endif /* __DT_BINDINGS_QCOM_SCMI_VENDOR_H */
--
2.34.1
