[PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU

Akhil P Oommen posted 3 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU
Posted by Akhil P Oommen 1 month, 2 weeks ago
Update GPU node to include acd level values.

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index a36076e3c56b..e6c500480eb1 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -3323,60 +3323,69 @@ zap-shader {
 			};
 
 			gpu_opp_table: opp-table {
-				compatible = "operating-points-v2";
+				compatible = "operating-points-v2-adreno";
 
 				opp-1100000000 {
 					opp-hz = /bits/ 64 <1100000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;
 					opp-peak-kBps = <16500000>;
+					qcom,opp-acd-level = <0xa82a5ffd>;
 				};
 
 				opp-1000000000 {
 					opp-hz = /bits/ 64 <1000000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_TURBO>;
 					opp-peak-kBps = <14398438>;
+					qcom,opp-acd-level = <0xa82b5ffd>;
 				};
 
 				opp-925000000 {
 					opp-hz = /bits/ 64 <925000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_NOM_L1>;
 					opp-peak-kBps = <14398438>;
+					qcom,opp-acd-level = <0xa82b5ffd>;
 				};
 
 				opp-800000000 {
 					opp-hz = /bits/ 64 <800000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
 					opp-peak-kBps = <12449219>;
+					qcom,opp-acd-level = <0xa82c5ffd>;
 				};
 
 				opp-744000000 {
 					opp-hz = /bits/ 64 <744000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_SVS_L2>;
 					opp-peak-kBps = <10687500>;
+					qcom,opp-acd-level = <0x882e5ffd>;
 				};
 
 				opp-687000000 {
 					opp-hz = /bits/ 64 <687000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
 					opp-peak-kBps = <8171875>;
+					qcom,opp-acd-level = <0x882e5ffd>;
 				};
 
 				opp-550000000 {
 					opp-hz = /bits/ 64 <550000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
 					opp-peak-kBps = <6074219>;
+					qcom,opp-acd-level = <0xc0285ffd>;
 				};
 
 				opp-390000000 {
 					opp-hz = /bits/ 64 <390000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
 					opp-peak-kBps = <3000000>;
+					qcom,opp-acd-level = <0xc0285ffd>;
 				};
 
 				opp-300000000 {
 					opp-hz = /bits/ 64 <300000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
 					opp-peak-kBps = <2136719>;
+					qcom,opp-acd-level = <0xc02b5ffd>;
 				};
 			};
 		};

-- 
2.45.2
Re: [PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU
Posted by Krzysztof Kozlowski 1 month, 1 week ago
On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote:
> Update GPU node to include acd level values.
> 
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index a36076e3c56b..e6c500480eb1 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -3323,60 +3323,69 @@ zap-shader {
>  			};
>  
>  			gpu_opp_table: opp-table {
> -				compatible = "operating-points-v2";
> +				compatible = "operating-points-v2-adreno";

This nicely breaks all existing users of this DTS. Sorry, no. We are way
past initial bringup/development. One year past.

Best regards,
Krzysztof
Re: [PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU
Posted by Akhil P Oommen 1 month, 1 week ago
On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote:
> On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote:
> > Update GPU node to include acd level values.
> > 
> > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > index a36076e3c56b..e6c500480eb1 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > @@ -3323,60 +3323,69 @@ zap-shader {
> >  			};
> >  
> >  			gpu_opp_table: opp-table {
> > -				compatible = "operating-points-v2";
> > +				compatible = "operating-points-v2-adreno";
> 
> This nicely breaks all existing users of this DTS. Sorry, no. We are way
> past initial bringup/development. One year past.

It is not obvious to me how it breaks backward compatibility. Could you
please elaborate a bit? I am aware that drivers should be backward
compatible with DT, but not the other way. Are we talking about kernels other
than Linux?

Also, does including "operating-points-v2" too here help?

-Akhil.

> 
> Best regards,
> Krzysztof
>
Re: [PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU
Posted by Krzysztof Kozlowski 1 month, 1 week ago
On 15/10/2024 21:35, Akhil P Oommen wrote:
> On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote:
>> On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote:
>>> Update GPU node to include acd level values.
>>>
>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++-
>>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> index a36076e3c56b..e6c500480eb1 100644
>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> @@ -3323,60 +3323,69 @@ zap-shader {
>>>  			};
>>>  
>>>  			gpu_opp_table: opp-table {
>>> -				compatible = "operating-points-v2";
>>> +				compatible = "operating-points-v2-adreno";
>>
>> This nicely breaks all existing users of this DTS. Sorry, no. We are way
>> past initial bringup/development. One year past.
> 
> It is not obvious to me how it breaks backward compatibility. Could you

I did not say "backward compatibility". I said existing users.

> please elaborate a bit? I am aware that drivers should be backward
> compatible with DT, but not the other way. Are we talking about kernels other
> than Linux?
> 

Boot OpenBSD with new DTS. Previously: worked fine. Now: works less fine.

We had exact talk about this during LPC.

> Also, does including "operating-points-v2" too here help?

Fallback? Yes, assuming these are compatible. Not much is explained in
the commit msg, except duplicating diff. That's not what the commit msg
is for.


Best regards,
Krzysztof
Re: [PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU
Posted by Akhil P Oommen 1 month, 1 week ago
On Wed, Oct 16, 2024 at 09:50:04AM +0200, Krzysztof Kozlowski wrote:
> On 15/10/2024 21:35, Akhil P Oommen wrote:
> > On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote:
> >> On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote:
> >>> Update GPU node to include acd level values.
> >>>
> >>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++-
> >>>  1 file changed, 10 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>> index a36076e3c56b..e6c500480eb1 100644
> >>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>> @@ -3323,60 +3323,69 @@ zap-shader {
> >>>  			};
> >>>  
> >>>  			gpu_opp_table: opp-table {
> >>> -				compatible = "operating-points-v2";
> >>> +				compatible = "operating-points-v2-adreno";
> >>
> >> This nicely breaks all existing users of this DTS. Sorry, no. We are way
> >> past initial bringup/development. One year past.

How do I identify when devicetree is considered stable? An arbitrary
time period doesn't sound like a good idea. Is there a general consensus
on this?

X1E chipset is still considered under development at least till the end of this
year, right?

> > 
> > It is not obvious to me how it breaks backward compatibility. Could you
> 
> I did not say "backward compatibility". I said existing users.
> 
> > please elaborate a bit? I am aware that drivers should be backward
> > compatible with DT, but not the other way. Are we talking about kernels other
> > than Linux?
> > 
> 
> Boot OpenBSD with new DTS. Previously: worked fine. Now: works less fine.
> 
> We had exact talk about this during LPC.
> 
> > Also, does including "operating-points-v2" too here help?
> 
> Fallback? Yes, assuming these are compatible. Not much is explained in
> the commit msg, except duplicating diff. That's not what the commit msg
> is for.

Okay. We can keep the fallback compatible string.

-Akhil.

> 
> 
> Best regards,
> Krzysztof
>
Re: [PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU
Posted by Krzysztof Kozlowski 1 month, 1 week ago
On 17/10/2024 08:12, Akhil P Oommen wrote:
> On Wed, Oct 16, 2024 at 09:50:04AM +0200, Krzysztof Kozlowski wrote:
>> On 15/10/2024 21:35, Akhil P Oommen wrote:
>>> On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote:
>>>> On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote:
>>>>> Update GPU node to include acd level values.
>>>>>
>>>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>>>>> ---
>>>>>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++-
>>>>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>>> index a36076e3c56b..e6c500480eb1 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>>> @@ -3323,60 +3323,69 @@ zap-shader {
>>>>>  			};
>>>>>  
>>>>>  			gpu_opp_table: opp-table {
>>>>> -				compatible = "operating-points-v2";
>>>>> +				compatible = "operating-points-v2-adreno";
>>>>
>>>> This nicely breaks all existing users of this DTS. Sorry, no. We are way
>>>> past initial bringup/development. One year past.
> 
> How do I identify when devicetree is considered stable? An arbitrary
> time period doesn't sound like a good idea. Is there a general consensus
> on this?
> 
> X1E chipset is still considered under development at least till the end of this
> year, right?

Stable could be when people already get their consumer/final product
with it. I got some weeks ago Lenovo T14s laptop and since yesterday
working fine with Ubuntu:
https://discourse.ubuntu.com/t/ubuntu-24-10-concept-snapdragon-x-elite/48800

All chipsets are under development, even old SM8450, but we avoid
breaking it while doing that.



Best regards,
Krzysztof
Re: [PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU
Posted by Akhil P Oommen 1 month, 1 week ago
On Thu, Oct 17, 2024 at 09:05:50AM +0200, Krzysztof Kozlowski wrote:
> On 17/10/2024 08:12, Akhil P Oommen wrote:
> > On Wed, Oct 16, 2024 at 09:50:04AM +0200, Krzysztof Kozlowski wrote:
> >> On 15/10/2024 21:35, Akhil P Oommen wrote:
> >>> On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote:
> >>>> On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote:
> >>>>> Update GPU node to include acd level values.
> >>>>>
> >>>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> >>>>> ---
> >>>>>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++-
> >>>>>  1 file changed, 10 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>>>> index a36076e3c56b..e6c500480eb1 100644
> >>>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>>>> @@ -3323,60 +3323,69 @@ zap-shader {
> >>>>>  			};
> >>>>>  
> >>>>>  			gpu_opp_table: opp-table {
> >>>>> -				compatible = "operating-points-v2";
> >>>>> +				compatible = "operating-points-v2-adreno";
> >>>>
> >>>> This nicely breaks all existing users of this DTS. Sorry, no. We are way
> >>>> past initial bringup/development. One year past.
> > 
> > How do I identify when devicetree is considered stable? An arbitrary
> > time period doesn't sound like a good idea. Is there a general consensus
> > on this?
> > 
> > X1E chipset is still considered under development at least till the end of this
> > year, right?
> 
> Stable could be when people already get their consumer/final product
> with it. I got some weeks ago Lenovo T14s laptop and since yesterday
> working fine with Ubuntu:
> https://discourse.ubuntu.com/t/ubuntu-24-10-concept-snapdragon-x-elite/48800
> 
> All chipsets are under development, even old SM8450, but we avoid
> breaking it while doing that.
> 
I still have questions about the practicality especially in IoT/Auto chipsets,
but I will try to get it clarified when I face them.

I will go ahead and send out the v2 series addressing the suggestions.

-Akhil.

> 
> 
> Best regards,
> Krzysztof
>