[PATCH 0/4] Improve some Adreno Device Tree nodes

[PATCH 0/4] Improve some Adreno Device Tree nodes

[Konrad Dybcio]

There are a couple of issues with some of the existing Adreno DT nodes:
- opp-level is used wrongly instead of required-opps
- the correct power domain for scaling is not always selected
- some frequencies are missing

And up until now, the power domain was not even scaled at all, until
the introduction of commit "Enable optional icc voting from OPP tables".

This series tries to address all of these. 8916 and 8996 didn't explode,
8998 could use some testing.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (4):
      arm64: dts: qcom: msm8998: Improve GPU OPP table
      arm64: dts: qcom: msm8996: Pass VDDMX to gpu in power-domains
      arm64: dts: qcom: msm8996: Improve GPU OPP table
      arm64: dts: qcom: msm8916: Improve GPU OPP table

 arch/arm64/boot/dts/qcom/msm8916.dtsi | 22 ++++++++++++++++----
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 29 +++++++++++++++++++-------
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 39 +++++++++++++++--------------------
 3 files changed, 57 insertions(+), 33 deletions(-)
---
base-commit: 198925fae644b0099b66fac1d972721e6e563b17
change-id: 20230329-topic-adreno_opp-7e5214a3e632

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@linaro.org>

[PATCH 1/4] arm64: dts: qcom: msm8998: Improve GPU OPP table

[Konrad Dybcio]

Add a newline before the first OPP subnode, remove useless
opp-supported-hw (there's only a single speed bin anyway) and replace
opp-level with required-opps to make sure the power domain level is
actually set, as opp-level is not the right property for this..
Furthermore, correct the levels that were incorrect before (confirmed
against downstream).

Round off frequencies that had uneven fluff on the last two digits.

To top if off, leave a note that we should really be scaling the
VDD GFX power domain coming from CPR4, which is not yet supported.
Scaling MX is still very important though and can be considered
valid for the time being - it's better if we scale at one of
two voltage rails than if we scaled none..

Fixes: 87cd46d68aea ("arm64: dts: qcom: msm8998: Configure Adreno GPU and related IOMMU")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 39 +++++++++++++++--------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 09b222f363c2..11952f9ed9ae 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1396,51 +1396,46 @@ adreno_gpu: gpu@5000000 {
 			interrupts = <0 300 IRQ_TYPE_LEVEL_HIGH>;
 			iommus = <&adreno_smmu 0>;
 			operating-points-v2 = <&gpu_opp_table>;
+			/* TODO: also scale VDDGFX with CPR4 */
 			power-domains = <&rpmpd MSM8998_VDDMX>;
 			status = "disabled";
 
 			gpu_opp_table: opp-table {
 				compatible = "operating-points-v2";
-				opp-710000097 {
-					opp-hz = /bits/ 64 <710000097>;
-					opp-level = <RPM_SMD_LEVEL_TURBO>;
-					opp-supported-hw = <0xff>;
+
+				opp-710000000 {
+					opp-hz = /bits/ 64 <710000000>;
+					required-opps = <&rpmpd_opp_turbo>;
 				};
 
-				opp-670000048 {
-					opp-hz = /bits/ 64 <670000048>;
-					opp-level = <RPM_SMD_LEVEL_NOM_PLUS>;
-					opp-supported-hw = <0xff>;
+				opp-670000000 {
+					opp-hz = /bits/ 64 <670000000>;
+					required-opps = <&rpmpd_opp_turbo>;
 				};
 
-				opp-596000097 {
-					opp-hz = /bits/ 64 <596000097>;
-					opp-level = <RPM_SMD_LEVEL_NOM>;
-					opp-supported-hw = <0xff>;
+				opp-596000000 {
+					opp-hz = /bits/ 64 <596000000>;
+					required-opps = <&rpmpd_opp_nom>;
 				};
 
-				opp-515000097 {
-					opp-hz = /bits/ 64 <515000097>;
-					opp-level = <RPM_SMD_LEVEL_SVS_PLUS>;
-					opp-supported-hw = <0xff>;
+				opp-515000000 {
+					opp-hz = /bits/ 64 <515000000>;
+					required-opps = <&rpmpd_opp_nom>;
 				};
 
 				opp-414000000 {
 					opp-hz = /bits/ 64 <414000000>;
-					opp-level = <RPM_SMD_LEVEL_SVS>;
-					opp-supported-hw = <0xff>;
+					required-opps = <&rpmpd_opp_svs>;
 				};
 
 				opp-342000000 {
 					opp-hz = /bits/ 64 <342000000>;
-					opp-level = <RPM_SMD_LEVEL_LOW_SVS>;
-					opp-supported-hw = <0xff>;
+					required-opps = <&rpmpd_opp_svs>;
 				};
 
 				opp-257000000 {
 					opp-hz = /bits/ 64 <257000000>;
-					opp-level = <RPM_SMD_LEVEL_MIN_SVS>;
-					opp-supported-hw = <0xff>;
+					required-opps = <&rpmpd_opp_svs>;
 				};
 			};
 		};

-- 
2.40.0

[PATCH 2/4] arm64: dts: qcom: msm8996: Pass VDDMX to gpu in power-domains

[Konrad Dybcio]

Since power-domains is used for perf_level pm_genpd scaling, it's only
fitting that we pass a power domain that's actually supposed to be
scaled (and not only turned on/off) to the GPU. While we don't quite
support CPR3 yet, the next best thing we can do is pass VDDMX, so that
we're at least guaranteed a reasonable vote on the memory side of
things. Do so and leave a note mentioning CPR3 PD should be used here
instead when support is added.

Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions")
Fixes: 3f65d51e9e22 ("arm64: dts: qcom: msm8996: Make GPU node control GPU_GX GDSC")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 4661a556772e..4dd37f72e018 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -1228,7 +1228,8 @@ gpu: gpu@b00000 {
 			interconnects = <&bimc MASTER_GRAPHICS_3D &bimc SLAVE_EBI_CH0>;
 			interconnect-names = "gfx-mem";
 
-			power-domains = <&mmcc GPU_GX_GDSC>;
+			/* TODO: also scale VDDGFX with CPR3 */
+			power-domains = <&rpmpd MSM8996_VDDMX>;
 			iommus = <&adreno_smmu 0>;
 
 			nvmem-cells = <&speedbin_efuse>;
@@ -2254,7 +2255,13 @@ adreno_smmu: iommu@b40000 {
 				 <&mmcc GPU_AHB_CLK>;
 			clock-names = "bus", "iface";
 
-			power-domains = <&mmcc GPU_GDSC>;
+			/*
+			 * We need both GPU_GDSC and GPU_GX_GDSC to be on, but the
+			 * power-domains entry under gpu is occupied by the scaled
+			 * voltage domain. Since GPU_GDSC is a parent of GX_GDSC,
+			 * we can simply pass GX here to turn them both on!
+			 */
+			power-domains = <&mmcc GPU_GX_GDSC>;
 		};
 
 		venus: video-codec@c00000 {

-- 
2.40.0

Re: [PATCH 2/4] arm64: dts: qcom: msm8996: Pass VDDMX to gpu in power-domains

[Dmitry Baryshkov]

On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> Since power-domains is used for perf_level pm_genpd scaling, it's only
> fitting that we pass a power domain that's actually supposed to be
> scaled (and not only turned on/off) to the GPU. While we don't quite
> support CPR3 yet, the next best thing we can do is pass VDDMX, so that
> we're at least guaranteed a reasonable vote on the memory side of
> things. Do so and leave a note mentioning CPR3 PD should be used here
> instead when support is added.
>
> Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions")
> Fixes: 3f65d51e9e22 ("arm64: dts: qcom: msm8996: Make GPU node control GPU_GX GDSC")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 4661a556772e..4dd37f72e018 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -1228,7 +1228,8 @@ gpu: gpu@b00000 {
>                         interconnects = <&bimc MASTER_GRAPHICS_3D &bimc SLAVE_EBI_CH0>;
>                         interconnect-names = "gfx-mem";
>
> -                       power-domains = <&mmcc GPU_GX_GDSC>;
> +                       /* TODO: also scale VDDGFX with CPR3 */
> +                       power-domains = <&rpmpd MSM8996_VDDMX>;
>                         iommus = <&adreno_smmu 0>;
>
>                         nvmem-cells = <&speedbin_efuse>;
> @@ -2254,7 +2255,13 @@ adreno_smmu: iommu@b40000 {
>                                  <&mmcc GPU_AHB_CLK>;
>                         clock-names = "bus", "iface";
>
> -                       power-domains = <&mmcc GPU_GDSC>;
> +                       /*
> +                        * We need both GPU_GDSC and GPU_GX_GDSC to be on, but the
> +                        * power-domains entry under gpu is occupied by the scaled
> +                        * voltage domain. Since GPU_GDSC is a parent of GX_GDSC,
> +                        * we can simply pass GX here to turn them both on!
> +                        */

Ugh. You can pass GX and VDDMX to the gpu. Let me send the patch
supporting that.

> +                       power-domains = <&mmcc GPU_GX_GDSC>;
>                 };
>
>                 venus: video-codec@c00000 {
>
> --
> 2.40.0
>


-- 
With best wishes
Dmitry

[PATCH 3/4] arm64: dts: qcom: msm8996: Improve GPU OPP table

[Konrad Dybcio]

Remove the self-explanatory comment about opp-supported-hw contents,
add required-opps to ensure reasonable power domain levels are voted
for (currently we've been piggybacking off of miracles and MDP votes)
and add newlines between each subnode.

Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 4dd37f72e018..62ad30e94f40 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -1244,37 +1244,45 @@ gpu: gpu@b00000 {
 			gpu_opp_table: opp-table {
 				compatible = "operating-points-v2";
 
-				/*
-				 * 624Mhz is only available on speed bins 0 and 3.
-				 * 560Mhz is only available on speed bins 0, 2 and 3.
-				 * All the rest are available on all bins of the hardware.
-				 */
 				opp-624000000 {
 					opp-hz = /bits/ 64 <624000000>;
+					required-opps = <&rpmpd_opp_turbo>;
 					opp-supported-hw = <0x09>;
 				};
+
 				opp-560000000 {
 					opp-hz = /bits/ 64 <560000000>;
+					required-opps = <&rpmpd_opp_turbo>;
 					opp-supported-hw = <0x0d>;
 				};
+
 				opp-510000000 {
 					opp-hz = /bits/ 64 <510000000>;
+					required-opps = <&rpmpd_opp_nom>;
 					opp-supported-hw = <0xff>;
 				};
+
 				opp-401800000 {
 					opp-hz = /bits/ 64 <401800000>;
+					required-opps = <&rpmpd_opp_nom>;
 					opp-supported-hw = <0xff>;
 				};
+
 				opp-315000000 {
 					opp-hz = /bits/ 64 <315000000>;
+					required-opps = <&rpmpd_opp_svs>;
 					opp-supported-hw = <0xff>;
 				};
+
 				opp-214000000 {
 					opp-hz = /bits/ 64 <214000000>;
+					required-opps = <&rpmpd_opp_svs>;
 					opp-supported-hw = <0xff>;
 				};
+
 				opp-133000000 {
 					opp-hz = /bits/ 64 <133000000>;
+					required-opps = <&rpmpd_opp_svs>;
 					opp-supported-hw = <0xff>;
 				};
 			};

-- 
2.40.0

Re: [PATCH 3/4] arm64: dts: qcom: msm8996: Improve GPU OPP table

[Dmitry Baryshkov]

On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> Remove the self-explanatory comment about opp-supported-hw contents,
> add required-opps to ensure reasonable power domain levels are voted
> for (currently we've been piggybacking off of miracles and MDP votes)
> and add newlines between each subnode.

I'm not sure this is 100% correct. The values that you add are correct
for the voltage scaling case. However, based on the vendor kernel
sources I think that MX should only be scaled if the voltage is scaled
too. I might be wrong here.

>
> Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 4dd37f72e018..62ad30e94f40 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 {
>                         gpu_opp_table: opp-table {
>                                 compatible = "operating-points-v2";
>
> -                               /*
> -                                * 624Mhz is only available on speed bins 0 and 3.
> -                                * 560Mhz is only available on speed bins 0, 2 and 3.
> -                                * All the rest are available on all bins of the hardware.
> -                                */
>                                 opp-624000000 {
>                                         opp-hz = /bits/ 64 <624000000>;
> +                                       required-opps = <&rpmpd_opp_turbo>;
>                                         opp-supported-hw = <0x09>;
>                                 };
> +
>                                 opp-560000000 {
>                                         opp-hz = /bits/ 64 <560000000>;
> +                                       required-opps = <&rpmpd_opp_turbo>;
>                                         opp-supported-hw = <0x0d>;
>                                 };
> +
>                                 opp-510000000 {
>                                         opp-hz = /bits/ 64 <510000000>;
> +                                       required-opps = <&rpmpd_opp_nom>;
>                                         opp-supported-hw = <0xff>;
>                                 };
> +
>                                 opp-401800000 {
>                                         opp-hz = /bits/ 64 <401800000>;
> +                                       required-opps = <&rpmpd_opp_nom>;
>                                         opp-supported-hw = <0xff>;
>                                 };
> +
>                                 opp-315000000 {
>                                         opp-hz = /bits/ 64 <315000000>;
> +                                       required-opps = <&rpmpd_opp_svs>;
>                                         opp-supported-hw = <0xff>;
>                                 };
> +
>                                 opp-214000000 {
>                                         opp-hz = /bits/ 64 <214000000>;
> +                                       required-opps = <&rpmpd_opp_svs>;
>                                         opp-supported-hw = <0xff>;
>                                 };
> +
>                                 opp-133000000 {
>                                         opp-hz = /bits/ 64 <133000000>;
> +                                       required-opps = <&rpmpd_opp_svs>;
>                                         opp-supported-hw = <0xff>;
>                                 };
>                         };
>
> --
> 2.40.0
>


-- 
With best wishes
Dmitry

Re: [PATCH 3/4] arm64: dts: qcom: msm8996: Improve GPU OPP table

[Konrad Dybcio]

On 29.03.2023 23:32, Dmitry Baryshkov wrote:
> On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> Remove the self-explanatory comment about opp-supported-hw contents,
>> add required-opps to ensure reasonable power domain levels are voted
>> for (currently we've been piggybacking off of miracles and MDP votes)
>> and add newlines between each subnode.
> 
> I'm not sure this is 100% correct. The values that you add are correct
> for the voltage scaling case. However, based on the vendor kernel
> sources I think that MX should only be scaled if the voltage is scaled
> too. I might be wrong here.
MX must be >= CX (and GX), so this should bring no harm.

(citation needed, but that seems to hold true..)

Konrad
> 
>>
>> Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++-----
>>  1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> index 4dd37f72e018..62ad30e94f40 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 {
>>                         gpu_opp_table: opp-table {
>>                                 compatible = "operating-points-v2";
>>
>> -                               /*
>> -                                * 624Mhz is only available on speed bins 0 and 3.
>> -                                * 560Mhz is only available on speed bins 0, 2 and 3.
>> -                                * All the rest are available on all bins of the hardware.
>> -                                */
>>                                 opp-624000000 {
>>                                         opp-hz = /bits/ 64 <624000000>;
>> +                                       required-opps = <&rpmpd_opp_turbo>;
>>                                         opp-supported-hw = <0x09>;
>>                                 };
>> +
>>                                 opp-560000000 {
>>                                         opp-hz = /bits/ 64 <560000000>;
>> +                                       required-opps = <&rpmpd_opp_turbo>;
>>                                         opp-supported-hw = <0x0d>;
>>                                 };
>> +
>>                                 opp-510000000 {
>>                                         opp-hz = /bits/ 64 <510000000>;
>> +                                       required-opps = <&rpmpd_opp_nom>;
>>                                         opp-supported-hw = <0xff>;
>>                                 };
>> +
>>                                 opp-401800000 {
>>                                         opp-hz = /bits/ 64 <401800000>;
>> +                                       required-opps = <&rpmpd_opp_nom>;
>>                                         opp-supported-hw = <0xff>;
>>                                 };
>> +
>>                                 opp-315000000 {
>>                                         opp-hz = /bits/ 64 <315000000>;
>> +                                       required-opps = <&rpmpd_opp_svs>;
>>                                         opp-supported-hw = <0xff>;
>>                                 };
>> +
>>                                 opp-214000000 {
>>                                         opp-hz = /bits/ 64 <214000000>;
>> +                                       required-opps = <&rpmpd_opp_svs>;
>>                                         opp-supported-hw = <0xff>;
>>                                 };
>> +
>>                                 opp-133000000 {
>>                                         opp-hz = /bits/ 64 <133000000>;
>> +                                       required-opps = <&rpmpd_opp_svs>;
>>                                         opp-supported-hw = <0xff>;
>>                                 };
>>                         };
>>
>> --
>> 2.40.0
>>
> 
> 

Re: [PATCH 3/4] arm64: dts: qcom: msm8996: Improve GPU OPP table

[Konrad Dybcio]

On 30.03.2023 12:57, Konrad Dybcio wrote:
> 
> 
> On 29.03.2023 23:32, Dmitry Baryshkov wrote:
>> On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>
>>> Remove the self-explanatory comment about opp-supported-hw contents,
>>> add required-opps to ensure reasonable power domain levels are voted
>>> for (currently we've been piggybacking off of miracles and MDP votes)
>>> and add newlines between each subnode.
>>
>> I'm not sure this is 100% correct. The values that you add are correct
>> for the voltage scaling case. However, based on the vendor kernel
>> sources I think that MX should only be scaled if the voltage is scaled
>> too. I might be wrong here.
> MX must be >= CX (and GX), so this should bring no harm.
> 
> (citation needed, but that seems to hold true..)
With that in mind, would you ack these patches Dmitry?

Konrad
> 
> Konrad
>>
>>>
>>> Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions")
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++-----
>>>  1 file changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>>> index 4dd37f72e018..62ad30e94f40 100644
>>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>>> @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 {
>>>                         gpu_opp_table: opp-table {
>>>                                 compatible = "operating-points-v2";
>>>
>>> -                               /*
>>> -                                * 624Mhz is only available on speed bins 0 and 3.
>>> -                                * 560Mhz is only available on speed bins 0, 2 and 3.
>>> -                                * All the rest are available on all bins of the hardware.
>>> -                                */
>>>                                 opp-624000000 {
>>>                                         opp-hz = /bits/ 64 <624000000>;
>>> +                                       required-opps = <&rpmpd_opp_turbo>;
>>>                                         opp-supported-hw = <0x09>;
>>>                                 };
>>> +
>>>                                 opp-560000000 {
>>>                                         opp-hz = /bits/ 64 <560000000>;
>>> +                                       required-opps = <&rpmpd_opp_turbo>;
>>>                                         opp-supported-hw = <0x0d>;
>>>                                 };
>>> +
>>>                                 opp-510000000 {
>>>                                         opp-hz = /bits/ 64 <510000000>;
>>> +                                       required-opps = <&rpmpd_opp_nom>;
>>>                                         opp-supported-hw = <0xff>;
>>>                                 };
>>> +
>>>                                 opp-401800000 {
>>>                                         opp-hz = /bits/ 64 <401800000>;
>>> +                                       required-opps = <&rpmpd_opp_nom>;
>>>                                         opp-supported-hw = <0xff>;
>>>                                 };
>>> +
>>>                                 opp-315000000 {
>>>                                         opp-hz = /bits/ 64 <315000000>;
>>> +                                       required-opps = <&rpmpd_opp_svs>;
>>>                                         opp-supported-hw = <0xff>;
>>>                                 };
>>> +
>>>                                 opp-214000000 {
>>>                                         opp-hz = /bits/ 64 <214000000>;
>>> +                                       required-opps = <&rpmpd_opp_svs>;
>>>                                         opp-supported-hw = <0xff>;
>>>                                 };
>>> +
>>>                                 opp-133000000 {
>>>                                         opp-hz = /bits/ 64 <133000000>;
>>> +                                       required-opps = <&rpmpd_opp_svs>;
>>>                                         opp-supported-hw = <0xff>;
>>>                                 };
>>>                         };
>>>
>>> --
>>> 2.40.0
>>>
>>
>>

Re: [PATCH 3/4] arm64: dts: qcom: msm8996: Improve GPU OPP table

[Dmitry Baryshkov]

On Fri, 26 May 2023 at 16:30, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 30.03.2023 12:57, Konrad Dybcio wrote:
> >
> >
> > On 29.03.2023 23:32, Dmitry Baryshkov wrote:
> >> On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>>
> >>> Remove the self-explanatory comment about opp-supported-hw contents,
> >>> add required-opps to ensure reasonable power domain levels are voted
> >>> for (currently we've been piggybacking off of miracles and MDP votes)
> >>> and add newlines between each subnode.
> >>
> >> I'm not sure this is 100% correct. The values that you add are correct
> >> for the voltage scaling case. However, based on the vendor kernel
> >> sources I think that MX should only be scaled if the voltage is scaled
> >> too. I might be wrong here.
> > MX must be >= CX (and GX), so this should bring no harm.
> >
> > (citation needed, but that seems to hold true..)
> With that in mind, would you ack these patches Dmitry?

I remember that we should not overwolt the memory, it might cause
memory cells degradation. So MX >= CX & MX <= CX + delta.

I have pinged Jordan if he can provide feedback on my a530/540 voltage
programming attempt ([1]), but got no response from him up to now.

[1] https://git.linaro.org/people/dmitry.baryshkov/kernel.git/log/?h=msm8996-upstream

I will try doing more experiments with the mentioned branch. Maybe it
breaks because of the missing MX vote or because of something
suchalike.
I'm 80% sure in the voltage level (which were generated by the CPR3 on
my db820c board) and 95% sure in the programming sequence.

>
> Konrad
> >
> > Konrad
> >>
> >>>
> >>> Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions")
> >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++-----
> >>>  1 file changed, 13 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> >>> index 4dd37f72e018..62ad30e94f40 100644
> >>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> >>> @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 {
> >>>                         gpu_opp_table: opp-table {
> >>>                                 compatible = "operating-points-v2";
> >>>
> >>> -                               /*
> >>> -                                * 624Mhz is only available on speed bins 0 and 3.
> >>> -                                * 560Mhz is only available on speed bins 0, 2 and 3.
> >>> -                                * All the rest are available on all bins of the hardware.
> >>> -                                */
> >>>                                 opp-624000000 {
> >>>                                         opp-hz = /bits/ 64 <624000000>;
> >>> +                                       required-opps = <&rpmpd_opp_turbo>;
> >>>                                         opp-supported-hw = <0x09>;
> >>>                                 };
> >>> +
> >>>                                 opp-560000000 {
> >>>                                         opp-hz = /bits/ 64 <560000000>;
> >>> +                                       required-opps = <&rpmpd_opp_turbo>;
> >>>                                         opp-supported-hw = <0x0d>;
> >>>                                 };
> >>> +
> >>>                                 opp-510000000 {
> >>>                                         opp-hz = /bits/ 64 <510000000>;
> >>> +                                       required-opps = <&rpmpd_opp_nom>;
> >>>                                         opp-supported-hw = <0xff>;
> >>>                                 };
> >>> +
> >>>                                 opp-401800000 {
> >>>                                         opp-hz = /bits/ 64 <401800000>;
> >>> +                                       required-opps = <&rpmpd_opp_nom>;
> >>>                                         opp-supported-hw = <0xff>;
> >>>                                 };
> >>> +
> >>>                                 opp-315000000 {
> >>>                                         opp-hz = /bits/ 64 <315000000>;
> >>> +                                       required-opps = <&rpmpd_opp_svs>;
> >>>                                         opp-supported-hw = <0xff>;
> >>>                                 };
> >>> +
> >>>                                 opp-214000000 {
> >>>                                         opp-hz = /bits/ 64 <214000000>;
> >>> +                                       required-opps = <&rpmpd_opp_svs>;
> >>>                                         opp-supported-hw = <0xff>;
> >>>                                 };
> >>> +
> >>>                                 opp-133000000 {
> >>>                                         opp-hz = /bits/ 64 <133000000>;
> >>> +                                       required-opps = <&rpmpd_opp_svs>;
> >>>                                         opp-supported-hw = <0xff>;
> >>>                                 };
> >>>                         };
> >>>
> >>> --
> >>> 2.40.0
> >>>
> >>
> >>



-- 
With best wishes
Dmitry

Re: [PATCH 3/4] arm64: dts: qcom: msm8996: Improve GPU OPP table

[Konrad Dybcio]

On 26.05.2023 15:30, Konrad Dybcio wrote:
> 
> 
> On 30.03.2023 12:57, Konrad Dybcio wrote:
>>
>>
>> On 29.03.2023 23:32, Dmitry Baryshkov wrote:
>>> On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>
>>>> Remove the self-explanatory comment about opp-supported-hw contents,
>>>> add required-opps to ensure reasonable power domain levels are voted
>>>> for (currently we've been piggybacking off of miracles and MDP votes)
>>>> and add newlines between each subnode.
>>>
>>> I'm not sure this is 100% correct. The values that you add are correct
>>> for the voltage scaling case. However, based on the vendor kernel
>>> sources I think that MX should only be scaled if the voltage is scaled
>>> too. I might be wrong here.
>> MX must be >= CX (and GX), so this should bring no harm.
>>
>> (citation needed, but that seems to hold true..)
> With that in mind, would you ack these patches Dmitry?
Bump?

Konrad

[PATCH 4/4] arm64: dts: qcom: msm8916: Improve GPU OPP table

[Konrad Dybcio]

Add required-opps to ensure reasonable power domain levels are voted
for (currently we've been piggybacking off of miracles and MDP
votes), add missing frequencies and add newlines between each
subnode.

Fixes: 61b83be9117c ("arm64: dts: qcom: msm8916: Add gpu support")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 639b158b4fbe..13bd0c647c1d 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1287,18 +1287,31 @@ gpu@1c00000 {
 			    <&gcc GCC_BIMC_GFX_CLK>,
 			    <&gcc GCC_BIMC_GPU_CLK>,
 			    <&gcc GFX3D_CLK_SRC>;
-			power-domains = <&gcc OXILI_GDSC>;
+			power-domains = <&rpmpd MSM8916_VDDCX>;
 			operating-points-v2 = <&gpu_opp_table>;
 			iommus = <&gpu_iommu 1>, <&gpu_iommu 2>;
 
 			gpu_opp_table: opp-table {
 				compatible = "operating-points-v2";
 
+				opp-27000000 {
+					opp-hz = /bits/ 64 <27000000>;
+					required-opps = <&rpmpd_opp_svs_krait>;
+				};
+
+				opp-200000000 {
+					opp-hz = /bits/ 64 <200000000>;
+					required-opps = <&rpmpd_opp_svs_soc>;
+				};
+
+				opp-310000000 {
+					opp-hz = /bits/ 64 <310000000>;
+					required-opps = <&rpmpd_opp_nom>;
+				};
+
 				opp-400000000 {
 					opp-hz = /bits/ 64 <400000000>;
-				};
-				opp-19200000 {
-					opp-hz = /bits/ 64 <19200000>;
+					required-opps = <&rpmpd_opp_super_turbo>;
 				};
 			};
 		};
@@ -1368,6 +1381,7 @@ gpu_iommu: iommu@1f08000 {
 			clocks = <&gcc GCC_SMMU_CFG_CLK>,
 				 <&gcc GCC_GFX_TCU_CLK>;
 			clock-names = "iface", "bus";
+			power-domains = <&gcc OXILI_GDSC>;
 			qcom,iommu-secure-id = <18>;
 
 			/* GFX3D_USER */

-- 
2.40.0