[RFC,7/7] arm64: dts: qcom: x1e80100: Enable LLCC/DDR dvfs

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

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
>