Re: [PATCH V4 1/5] dt-bindings: firmware: Document bindings for QCOM SCMI Generic Extension

From: Dmitry Baryshkov
Date: Mon Oct 07 2024 - 14:06:50 EST

Next message: syzbot: "Re: [syzbot] [bcachefs?] general protection fault in bch2_alloc_read"
Previous message: Drew Fustini: "Re: [PATCH v2 3/3] pinctrl: th1520: Convert dt child node loop to scoped iterator"
In reply to: Sibi Sankar: "[PATCH V4 1/5] dt-bindings: firmware: Document bindings for QCOM SCMI Generic Extension"
Next in thread: Sibi Sankar: "[PATCH V4 2/5] firmware: arm_scmi: Add QCOM Generic Vendor Protocol documentation"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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>
> ---
>
> 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?

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?

> +
> + 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?

> +
> + 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?

> +
> + 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?

> + 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?

> + operating-points-v2 = <&memory0_monitor0_opp_table>;
> +
> + memory0_monitor0_opp_table: opp-table {

sensible names are better:

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'

> +
> + 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
>

--
With best wishes
Dmitry

Next message: syzbot: "Re: [syzbot] [bcachefs?] general protection fault in bch2_alloc_read"
Previous message: Drew Fustini: "Re: [PATCH v2 3/3] pinctrl: th1520: Convert dt child node loop to scoped iterator"
In reply to: Sibi Sankar: "[PATCH V4 1/5] dt-bindings: firmware: Document bindings for QCOM SCMI Generic Extension"
Next in thread: Sibi Sankar: "[PATCH V4 2/5] firmware: arm_scmi: Add QCOM Generic Vendor Protocol documentation"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]