| Message ID | 20240117173458.2312669-8-quic_sibis@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | firmware: arm_scmi: Qualcomm Vendor Protocol | expand |
On 1/17/24 18:34, Sibi Sankar wrote: > Enable LLCC/DDR dvfs through the Qualcomm's SCMI vendor protocol. > > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> > --- > arch/arm64/boot/dts/qcom/x1e80100.dtsi | 48 ++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > index 6856a206f7fc..3dc6f32fbb4c 100644 > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > @@ -329,6 +329,54 @@ scmi_dvfs: protocol@13 { > reg = <0x13>; > #clock-cells = <1>; > }; > + > + scmi_vendor: protocol@80 { > + reg = <0x80>; > + > + memlat { > + #address-cells = <1>; > + #size-cells = <0>; > + > + memory@0 { > + reg = <0x0>; /* Memory Type DDR */ I'm not sure reg is the best property to (ab)use.. You could very well define a new one, like qcom,memory type, then the subnodes could look like: memory-0 { qcom,memory-type = <QCOM_MEM_TYPE_DDR>; [...] }; > + freq-table-khz = <200000 4224000>; > + > + monitor-0 { > + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; I fail to see the usefulness in checking which CPUs make use of the same DRAM or LLC pool. If that's something that may not be obvious in future designs like on dual-socket x86 servers, I think it can be deferred until then and for now, AFAIU you can just unconditionally assume all CPUs count. > + qcom,cpufreq-memfreq-tbl = < 999000 547000 >, > + < 1440000 768000 >, > + < 1671000 1555000 >, > + < 2189000 2092000 >, > + < 2156000 3187000 >, > + < 3860000 4224000 >; I.. can't seem to think of a future where this doesn't explode. When you release a different bin/SKU/fuse config of this SoC where the CPU frequencies are different, this will likely also need to be updated. We don't want that manual cruft in the devicetree. Since both previously cpufreq-hw and now cpufreq-scmi generally operate on levels that map to some frequencies in the firmware, could it be bound to that instead? Konrad
On Wed, 17 Jan 2024 at 19:37, Sibi Sankar <quic_sibis@quicinc.com> wrote: > > Enable LLCC/DDR dvfs through the Qualcomm's SCMI vendor protocol. Could you please post DT bindings? > > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> > --- > arch/arm64/boot/dts/qcom/x1e80100.dtsi | 48 ++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > index 6856a206f7fc..3dc6f32fbb4c 100644 > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > @@ -329,6 +329,54 @@ scmi_dvfs: protocol@13 { > reg = <0x13>; > #clock-cells = <1>; > }; > + > + scmi_vendor: protocol@80 { > + reg = <0x80>; > + > + memlat { This doesn't look like a generic node name. > + #address-cells = <1>; > + #size-cells = <0>; > + > + memory@0 { > + reg = <0x0>; /* Memory Type DDR */ > + freq-table-khz = <200000 4224000>; > + > + monitor-0 { > + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; > + qcom,cpufreq-memfreq-tbl = < 999000 547000 >, > + < 1440000 768000 >, > + < 1671000 1555000 >, > + < 2189000 2092000 >, > + < 2156000 3187000 >, > + < 3860000 4224000 >; These tables should be rewritten as OPP tables. > + }; > + > + monitor-1 { > + qcom,compute-mon; > + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; > + qcom,cpufreq-memfreq-tbl = < 1440000 200000 >, > + < 2189000 768000 >, > + < 2156000 1555000 >, > + < 3860000 2092000 >; > + }; > + }; > + > + memory@1 { > + reg = <0x1>; /* Memory Type LLCC */ > + freq-table-khz = <300000 1067000>; > + > + monitor-0 { > + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; > + qcom,cpufreq-memfreq-tbl = < 999000 300000 >, > + < 1440000 466000 >, > + < 1671000 600000 >, > + < 2189000 806000 >, > + < 2156000 933000 >, > + < 3860000 1066000 >; > + }; > + }; > + }; > + }; > }; > }; > > -- > 2.34.1 > > -- With best wishes Dmitry
On 1/18/24 02:17, Dmitry Baryshkov wrote: > On Wed, 17 Jan 2024 at 19:37, Sibi Sankar <quic_sibis@quicinc.com> wrote: >> >> Enable LLCC/DDR dvfs through the Qualcomm's SCMI vendor protocol. > > Could you please post DT bindings? ack, will include it in the next re-spin. > >> >> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 48 ++++++++++++++++++++++++++ >> 1 file changed, 48 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi >> index 6856a206f7fc..3dc6f32fbb4c 100644 >> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi >> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi >> @@ -329,6 +329,54 @@ scmi_dvfs: protocol@13 { >> reg = <0x13>; >> #clock-cells = <1>; >> }; >> + >> + scmi_vendor: protocol@80 { >> + reg = <0x80>; >> + >> + memlat { > > This doesn't look like a generic node name. > >> + #address-cells = <1>; >> + #size-cells = <0>; > >> + >> + memory@0 { >> + reg = <0x0>; /* Memory Type DDR */ >> + freq-table-khz = <200000 4224000>; >> + >> + monitor-0 { >> + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; > > > >> + qcom,cpufreq-memfreq-tbl = < 999000 547000 >, >> + < 1440000 768000 >, >> + < 1671000 1555000 >, >> + < 2189000 2092000 >, >> + < 2156000 3187000 >, >> + < 3860000 4224000 >; > > These tables should be rewritten as OPP tables. > > >> + }; >> + >> + monitor-1 { >> + qcom,compute-mon; >> + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; >> + qcom,cpufreq-memfreq-tbl = < 1440000 200000 >, >> + < 2189000 768000 >, >> + < 2156000 1555000 >, >> + < 3860000 2092000 >; >> + }; >> + }; >> + >> + memory@1 { >> + reg = <0x1>; /* Memory Type LLCC */ >> + freq-table-khz = <300000 1067000>; >> + >> + monitor-0 { >> + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; >> + qcom,cpufreq-memfreq-tbl = < 999000 300000 >, >> + < 1440000 466000 >, >> + < 1671000 600000 >, >> + < 2189000 806000 >, >> + < 2156000 933000 >, >> + < 3860000 1066000 >; >> + }; >> + }; >> + }; >> + }; >> }; >> }; >> >> -- >> 2.34.1 >> >> > > > -- > With best wishes > Dmitry
On 1/18/24 02:08, Konrad Dybcio wrote: > > > On 1/17/24 18:34, Sibi Sankar wrote: >> Enable LLCC/DDR dvfs through the Qualcomm's SCMI vendor protocol. >> >> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 48 ++++++++++++++++++++++++++ >> 1 file changed, 48 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi >> b/arch/arm64/boot/dts/qcom/x1e80100.dtsi >> index 6856a206f7fc..3dc6f32fbb4c 100644 >> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi >> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi >> @@ -329,6 +329,54 @@ scmi_dvfs: protocol@13 { >> reg = <0x13>; >> #clock-cells = <1>; >> }; >> + >> + scmi_vendor: protocol@80 { >> + reg = <0x80>; >> + >> + memlat { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + memory@0 { >> + reg = <0x0>; /* Memory Type DDR */ > > I'm not sure reg is the best property to (ab)use.. I'm ok with introducing a custom property as well. I went ahead with reg mainly because the overall structure looked similar to audio apr. > > You could very well define a new one, like qcom,memory type, > then the subnodes could look like: > > memory-0 { > qcom,memory-type = <QCOM_MEM_TYPE_DDR>; > [...] > }; > >> + freq-table-khz = <200000 4224000>; >> + >> + monitor-0 { >> + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 >> &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; > > I fail to see the usefulness in checking which CPUs make use of > the same DRAM or LLC pool. If that's something that may not be > obvious in future designs like on dual-socket x86 servers, > I think it can be deferred until then and for now, AFAIU you > can just unconditionally assume all CPUs count. we list all the cpus here because on X1E they are identical and have the same cpu frequency to memory frequency mapping. But doesn't really apply to other SoCs in general. But dropping this would mean that driver assumes a table applies to all cpus by default. > >> + qcom,cpufreq-memfreq-tbl = < 999000 >> 547000 >, >> + < 1440000 768000 >, >> + < 1671000 1555000 >, >> + < 2189000 2092000 >, >> + < 2156000 3187000 >, >> + < 3860000 4224000 >; > > I.. can't seem to think of a future where this doesn't explode. Not really ... You can already see a more or less standard table being used across various skus on older SoCs that uses memlat running from the kernel downstream. So that should count for something. > > When you release a different bin/SKU/fuse config of this SoC where > the CPU frequencies are different, this will likely also need to be > updated. We don't want that manual cruft in the devicetree. Also unlike cpufreq map, if you notice this table doesn't list all possible cpu frequencies but list broad ranges instead. This way the table rarely needs updates unless we want to scale to max llcc/ddr at a lower CPU frequency for a particular SKU. > > Since both previously cpufreq-hw and now cpufreq-scmi generally > operate on levels that map to some frequencies in the firmware, > could it be bound to that instead? At this point the only decision is whether the table lies in dt or in the driver. But driver wouldn't even have a way to distinguish between various skus so the dt looks like the only option. -Sibi > > Konrad >
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi index 6856a206f7fc..3dc6f32fbb4c 100644 --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi @@ -329,6 +329,54 @@ scmi_dvfs: protocol@13 { reg = <0x13>; #clock-cells = <1>; }; + + scmi_vendor: protocol@80 { + reg = <0x80>; + + memlat { + #address-cells = <1>; + #size-cells = <0>; + + memory@0 { + reg = <0x0>; /* Memory Type DDR */ + freq-table-khz = <200000 4224000>; + + monitor-0 { + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; + qcom,cpufreq-memfreq-tbl = < 999000 547000 >, + < 1440000 768000 >, + < 1671000 1555000 >, + < 2189000 2092000 >, + < 2156000 3187000 >, + < 3860000 4224000 >; + }; + + monitor-1 { + qcom,compute-mon; + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; + qcom,cpufreq-memfreq-tbl = < 1440000 200000 >, + < 2189000 768000 >, + < 2156000 1555000 >, + < 3860000 2092000 >; + }; + }; + + memory@1 { + reg = <0x1>; /* Memory Type LLCC */ + freq-table-khz = <300000 1067000>; + + monitor-0 { + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3 &CPU4 &CPU5 &CPU6 &CPU7 &CPU8 &CPU9 &CPU10 &CPU11>; + qcom,cpufreq-memfreq-tbl = < 999000 300000 >, + < 1440000 466000 >, + < 1671000 600000 >, + < 2189000 806000 >, + < 2156000 933000 >, + < 3860000 1066000 >; + }; + }; + }; + }; }; };
Enable LLCC/DDR dvfs through the Qualcomm's SCMI vendor protocol. Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> --- arch/arm64/boot/dts/qcom/x1e80100.dtsi | 48 ++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)