| Message ID | 20240702191440.2161623-2-quic_sibis@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm_scmi: vendors: ARM SCMI Qualcomm Vendor Protocol | expand |
On Wed, Jul 03, 2024 at 12:44:37AM +0530, Sibi Sankar wrote: > Document the various memory buses that can be monitored and scaled by the > memory latency governor hosted by the ARM SCMI QCOM Vendor protocol v1.0. > > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> > --- Hi Sibi, this series got a bit neglected...my bad...a few comments down below. > > Adding a reg property in scmi-memlat.yaml seems incorrect/superfluous > but without it I see the following errors: > > Err Logs: > protocol@80: 'reg' does not match any of the regexes: '^memory-[0-9]$', 'pinctrl-[0-9]+' > protocol@80: Unevaluated properties are not allowed ('memory-0', 'memory-1', 'memory-2' were unexpected) > > v2: > * Drop container dvfs memlat container node. [Rob] > * Move scmi-memlat.yaml to protocol level given that a lot of vendors might end up > using the same protocol number. [Rob] > * Replace qcom,cpulist with the standard "cpus" property. [Rob] > * Fix up compute-type/ipm-ceil required. [Rob] > ...so there has been a lot of work around Vendor protos recently (as you have seen) and especially around the way we define the DT bindings to have multiple vendor protocols coexist with the same overlapping numbers. (the code-level coexistence is already in place as you've seen...) I think some sort of agreement on HOW to render this in the bindings side was reached around a series from NXP...not sure if I am missing something here but this commit from Peng/NXP (if you have not seen it already): https://lore.kernel.org/linux-arm-kernel/20240621-imx95-bbm-misc-v2-v5-2-b85a6bf778cb@nxp.com/ ...it is a good example of how you can define your vendor specific part in a vendor specific binding files and then just add a single $ref line in the core binding Documentation/devicetree/bindings/firmware/arm,scmi.yaml (and that has been successfully reviewd...) Moreover, in that same series from Peng/NXP you could have a look at https://lore.kernel.org/linux-arm-kernel/20240621-imx95-bbm-misc-v2-v5-1-b85a6bf778cb@nxp.com/ that adds the Documentation for their Vendor protocols. Beside the final location in the tree for such docs, which is a detail we can settle later on our side too, I think that patch is a good example of the kind of vendor-protos Documentation Sudeep is expecting. > .../bindings/firmware/arm,scmi.yaml | 15 ++ > .../bindings/soc/qcom/qcom,scmi-memlat.yaml | 242 ++++++++++++++++++ > include/dt-bindings/soc/qcom,scmi-vendor.h | 22 ++ > 3 files changed, 279 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml > create mode 100644 include/dt-bindings/soc/qcom,scmi-vendor.h > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > index 4d823f3b1f0e..a4022682e5ca 100644 > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > @@ -284,6 +284,21 @@ properties: > required: > - reg > > + protocol@80: > + type: object > + allOf: > + - $ref: '#/$defs/protocol-node' > + - $ref: /schemas/soc/qcom/qcom,scmi-memlat.yaml# > + > + unevaluatedProperties: false > + > + properties: > + reg: > + const: 0x80 > + > + required: > + - reg > + ..here you should be able to just plant your $ref without redefining the protocol@80 > additionalProperties: false > > $defs: > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml > new file mode 100644 > index 000000000000..915a6bf5697f > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml > @@ -0,0 +1,242 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soc/qcom/qcom,scmi-memlat.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm SCMI Memory Bus nodes > + > +maintainers: > + - Sibi Sankar <quic_sibis@quicinc.com> > + > +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). > + ...and instead here you will define your protocols, compliant with the main protocol-node def and any specific vendor sub-properties that you should need.... ...the above example from NXP is probably more clear than any attempt of mine to explain this :P Thanks, Cristian
On 7/9/24 22:02, Cristian Marussi wrote: > On Wed, Jul 03, 2024 at 12:44:37AM +0530, Sibi Sankar wrote: >> Document the various memory buses that can be monitored and scaled by the >> memory latency governor hosted by the ARM SCMI QCOM Vendor protocol v1.0. >> >> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> Hey Christian, Thanks for taking time to review the series :) >> --- > > Hi Sibi, > > this series got a bit neglected...my bad...a few comments down below. > >> >> Adding a reg property in scmi-memlat.yaml seems incorrect/superfluous >> but without it I see the following errors: >> >> Err Logs: >> protocol@80: 'reg' does not match any of the regexes: '^memory-[0-9]$', 'pinctrl-[0-9]+' >> protocol@80: Unevaluated properties are not allowed ('memory-0', 'memory-1', 'memory-2' were unexpected) >> >> v2: >> * Drop container dvfs memlat container node. [Rob] >> * Move scmi-memlat.yaml to protocol level given that a lot of vendors might end up >> using the same protocol number. [Rob] >> * Replace qcom,cpulist with the standard "cpus" property. [Rob] >> * Fix up compute-type/ipm-ceil required. [Rob] >> > > ...so there has been a lot of work around Vendor protos recently (as you > have seen) and especially around the way we define the DT bindings to have > multiple vendor protocols coexist with the same overlapping numbers. > (the code-level coexistence is already in place as you've seen...) > > I think some sort of agreement on HOW to render this in the bindings > side was reached around a series from NXP...not sure if I am missing something > here but this commit from Peng/NXP (if you have not seen it already): > https://lore.kernel.org/linux-arm-kernel/20240621-imx95-bbm-misc-v2-v5-2-b85a6bf778cb@nxp.com/ > > ...it is a good example of how you can define your vendor specific part in > a vendor specific binding files and then just add a single $ref line in > the core binding Documentation/devicetree/bindings/firmware/arm,scmi.yaml > (and that has been successfully reviewd...) > > Moreover, in that same series from Peng/NXP you could have a look at > https://lore.kernel.org/linux-arm-kernel/20240621-imx95-bbm-misc-v2-v5-1-b85a6bf778cb@nxp.com/ > > that adds the Documentation for their Vendor protocols. > Beside the final location in the tree for such docs, which is a detail > we can settle later on our side too, I think that patch is a good example > of the kind of vendor-protos Documentation Sudeep is expecting. > > >> .../bindings/firmware/arm,scmi.yaml | 15 ++ >> .../bindings/soc/qcom/qcom,scmi-memlat.yaml | 242 ++++++++++++++++++ >> include/dt-bindings/soc/qcom,scmi-vendor.h | 22 ++ >> 3 files changed, 279 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml >> create mode 100644 include/dt-bindings/soc/qcom,scmi-vendor.h >> >> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml >> index 4d823f3b1f0e..a4022682e5ca 100644 >> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml >> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml >> @@ -284,6 +284,21 @@ properties: >> required: >> - reg >> >> + protocol@80: >> + type: object >> + allOf: >> + - $ref: '#/$defs/protocol-node' >> + - $ref: /schemas/soc/qcom/qcom,scmi-memlat.yaml# >> + >> + unevaluatedProperties: false >> + >> + properties: >> + reg: >> + const: 0x80 >> + >> + required: >> + - reg >> + > > ..here you should be able to just plant your $ref without redefining the > protocol@80 > >> additionalProperties: false >> >> $defs: >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml >> new file mode 100644 >> index 000000000000..915a6bf5697f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml >> @@ -0,0 +1,242 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/soc/qcom/qcom,scmi-memlat.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm SCMI Memory Bus nodes >> + >> +maintainers: >> + - Sibi Sankar <quic_sibis@quicinc.com> >> + >> +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). >> + > > ...and instead here you will define your protocols, compliant with the > main protocol-node def and any specific vendor sub-properties that you > should need.... > > ...the above example from NXP is probably more clear than any attempt of mine > to explain this :P ack, will adhere to the same in the next re-spin. > > Thanks, > Cristian >
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml index 4d823f3b1f0e..a4022682e5ca 100644 --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml @@ -284,6 +284,21 @@ properties: required: - reg + protocol@80: + type: object + allOf: + - $ref: '#/$defs/protocol-node' + - $ref: /schemas/soc/qcom/qcom,scmi-memlat.yaml# + + unevaluatedProperties: false + + properties: + reg: + const: 0x80 + + required: + - reg + additionalProperties: false $defs: diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml new file mode 100644 index 000000000000..915a6bf5697f --- /dev/null +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml @@ -0,0 +1,242 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/qcom/qcom,scmi-memlat.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm SCMI Memory Bus nodes + +maintainers: + - Sibi Sankar <quic_sibis@quicinc.com> + +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: + reg: + maxItems: 1 + +patternProperties: + '^memory-[0-9]$': + type: object + description: + The list of all memory buses that can be monitored and scaled by the + memory latency governor running on the SCMI controller. + + unevaluatedProperties: false + + 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 + + freq-table-hz: + items: + items: + - description: Minimum frequency of the memory bus in Hz + - description: Maximum frequency of the memory bus in Hz + + 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 + + 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). + + 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: false + +examples: + - | + #include <dt-bindings/soc/qcom,scmi-vendor.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 { + qcom,ipm-ceil = <20000000>; + cpus = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 + &CPU8 &CPU9 &CPU10 &CPU11>; + operating-points-v2 = <&memory0_monitor0_opp_table>; + + memory0_monitor0_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>; + + 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/soc/qcom,scmi-vendor.h b/include/dt-bindings/soc/qcom,scmi-vendor.h new file mode 100644 index 000000000000..7ae8d8d5623b --- /dev/null +++ b/include/dt-bindings/soc/qcom,scmi-vendor.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 */
Document the various memory buses that can be monitored and scaled by the memory latency governor hosted by the ARM SCMI QCOM Vendor protocol v1.0. Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> --- Adding a reg property in scmi-memlat.yaml seems incorrect/superfluous but without it I see the following errors: Err Logs: protocol@80: 'reg' does not match any of the regexes: '^memory-[0-9]$', 'pinctrl-[0-9]+' protocol@80: Unevaluated properties are not allowed ('memory-0', 'memory-1', 'memory-2' were unexpected) v2: * Drop container dvfs memlat container node. [Rob] * Move scmi-memlat.yaml to protocol level given that a lot of vendors might end up using the same protocol number. [Rob] * Replace qcom,cpulist with the standard "cpus" property. [Rob] * Fix up compute-type/ipm-ceil required. [Rob] .../bindings/firmware/arm,scmi.yaml | 15 ++ .../bindings/soc/qcom/qcom,scmi-memlat.yaml | 242 ++++++++++++++++++ include/dt-bindings/soc/qcom,scmi-vendor.h | 22 ++ 3 files changed, 279 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml create mode 100644 include/dt-bindings/soc/qcom,scmi-vendor.h