diff mbox series

[V4,8/8] arm64: dts: qcom: sm4450: add camera, display and gpu clock controller

Message ID 20240611133752.2192401-9-quic_ajipan@quicinc.com (mailing list archive)
State Not Applicable, archived
Headers show
Series clk: qcom: Add support for DISPCC, CAMCC and GPUCC on SM4450 | expand

Commit Message

Ajit Pandey June 11, 2024, 1:37 p.m. UTC
Add device node for camera, display and graphics clock controller on
Qualcomm SM4450 platform.

Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm4450.dtsi | 38 ++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

Dmitry Baryshkov June 11, 2024, 3:47 p.m. UTC | #1
On Tue, Jun 11, 2024 at 07:07:52PM +0530, Ajit Pandey wrote:
> Add device node for camera, display and graphics clock controller on
> Qualcomm SM4450 platform.
> 
> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sm4450.dtsi | 38 ++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Konrad Dybcio June 13, 2024, 7:41 a.m. UTC | #2
On 6/11/24 15:37, Ajit Pandey wrote:
> Add device node for camera, display and graphics clock controller on
> Qualcomm SM4450 platform.
> 
> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
> ---

None of these nodes reference a power domain (which would usually be
CX/MX/MMCX). This way, the RPMhPDs will never be scaled.

The current upstream implementation only allows one power domain to be
scaled, but that's better than none (see other DTs for recent SoCs).

Konrad
Ajit Pandey July 3, 2024, 9:16 a.m. UTC | #3
On 6/13/2024 1:11 PM, Konrad Dybcio wrote:
> 
> 
> On 6/11/24 15:37, Ajit Pandey wrote:
>> Add device node for camera, display and graphics clock controller on
>> Qualcomm SM4450 platform.
>>
>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>> ---
> 
> None of these nodes reference a power domain (which would usually be
> CX/MX/MMCX). This way, the RPMhPDs will never be scaled.
> 
> The current upstream implementation only allows one power domain to be
> scaled, but that's better than none (see other DTs for recent SoCs).
> 
> Konrad

SM4450 doesn't support MMCX and CX/MX domains will remain active so
power-domains property is actually not required for SM4450 clock nodes.
Konrad Dybcio July 11, 2024, 9:55 a.m. UTC | #4
On 3.07.2024 11:16 AM, Ajit Pandey wrote:
> 
> 
> On 6/13/2024 1:11 PM, Konrad Dybcio wrote:
>>
>>
>> On 6/11/24 15:37, Ajit Pandey wrote:
>>> Add device node for camera, display and graphics clock controller on
>>> Qualcomm SM4450 platform.
>>>
>>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>>> ---
>>
>> None of these nodes reference a power domain (which would usually be
>> CX/MX/MMCX). This way, the RPMhPDs will never be scaled.
>>
>> The current upstream implementation only allows one power domain to be
>> scaled, but that's better than none (see other DTs for recent SoCs).
>>
>> Konrad
> 
> SM4450 doesn't support MMCX and CX/MX domains will remain active so
> power-domains property is actually not required for SM4450 clock nodes.

It's not only about them being active.. some PLLs require e.g. MX to be
at a certain level, or the system will be unstable

Konrad
Ajit Pandey July 12, 2024, 9:53 a.m. UTC | #5
On 7/11/2024 3:25 PM, Konrad Dybcio wrote:
> On 3.07.2024 11:16 AM, Ajit Pandey wrote:
>>
>>
>> On 6/13/2024 1:11 PM, Konrad Dybcio wrote:
>>>
>>>
>>> On 6/11/24 15:37, Ajit Pandey wrote:
>>>> Add device node for camera, display and graphics clock controller on
>>>> Qualcomm SM4450 platform.
>>>>
>>>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>>>> ---
>>>
>>> None of these nodes reference a power domain (which would usually be
>>> CX/MX/MMCX). This way, the RPMhPDs will never be scaled.
>>>
>>> The current upstream implementation only allows one power domain to be
>>> scaled, but that's better than none (see other DTs for recent SoCs).
>>>
>>> Konrad
>>
>> SM4450 doesn't support MMCX and CX/MX domains will remain active so
>> power-domains property is actually not required for SM4450 clock nodes.
> 
> It's not only about them being active.. some PLLs require e.g. MX to be
> at a certain level, or the system will be unstable
> 
> Konrad

With active I mean CX/MX rails will be default running at minimum level 
required for clock controllers. Adding power-domains property for CX/MX 
rails is like a redundant code as that will also scale such rails at 
default specified minimum level only. Also we hadn't added such property 
for other targets DT nodes to scale up CX/MX at minimum level.
Konrad Dybcio July 12, 2024, 12:22 p.m. UTC | #6
On 12.07.2024 11:53 AM, Ajit Pandey wrote:
> 
> 
> On 7/11/2024 3:25 PM, Konrad Dybcio wrote:
>> On 3.07.2024 11:16 AM, Ajit Pandey wrote:
>>>
>>>
>>> On 6/13/2024 1:11 PM, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 6/11/24 15:37, Ajit Pandey wrote:
>>>>> Add device node for camera, display and graphics clock controller on
>>>>> Qualcomm SM4450 platform.
>>>>>
>>>>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>>>>> ---
>>>>
>>>> None of these nodes reference a power domain (which would usually be
>>>> CX/MX/MMCX). This way, the RPMhPDs will never be scaled.
>>>>
>>>> The current upstream implementation only allows one power domain to be
>>>> scaled, but that's better than none (see other DTs for recent SoCs).
>>>>
>>>> Konrad
>>>
>>> SM4450 doesn't support MMCX and CX/MX domains will remain active so
>>> power-domains property is actually not required for SM4450 clock nodes.
>>
>> It's not only about them being active.. some PLLs require e.g. MX to be
>> at a certain level, or the system will be unstable
>>
>> Konrad
> 
> With active I mean CX/MX rails will be default running at minimum level required for clock controllers. Adding power-domains property for CX/MX rails is like a redundant code as that will also scale such rails at default specified minimum level only. Also we hadn't added such property for other targets DT nodes to scale up CX/MX at minimum level.

What I mean here is that, the minimum level may not be enough. In such case
you would also add a required-opps = <&handle_to_rpmhpd_opp_level>

Konrad
Ajit Pandey July 12, 2024, 12:31 p.m. UTC | #7
On 7/12/2024 5:52 PM, Konrad Dybcio wrote:
> On 12.07.2024 11:53 AM, Ajit Pandey wrote:
>>
>>
>> On 7/11/2024 3:25 PM, Konrad Dybcio wrote:
>>> On 3.07.2024 11:16 AM, Ajit Pandey wrote:
>>>>
>>>>
>>>> On 6/13/2024 1:11 PM, Konrad Dybcio wrote:
>>>>>
>>>>>
>>>>> On 6/11/24 15:37, Ajit Pandey wrote:
>>>>>> Add device node for camera, display and graphics clock controller on
>>>>>> Qualcomm SM4450 platform.
>>>>>>
>>>>>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>>>>>> ---
>>>>>
>>>>> None of these nodes reference a power domain (which would usually be
>>>>> CX/MX/MMCX). This way, the RPMhPDs will never be scaled.
>>>>>
>>>>> The current upstream implementation only allows one power domain to be
>>>>> scaled, but that's better than none (see other DTs for recent SoCs).
>>>>>
>>>>> Konrad
>>>>
>>>> SM4450 doesn't support MMCX and CX/MX domains will remain active so
>>>> power-domains property is actually not required for SM4450 clock nodes.
>>>
>>> It's not only about them being active.. some PLLs require e.g. MX to be
>>> at a certain level, or the system will be unstable
>>>
>>> Konrad
>>
>> With active I mean CX/MX rails will be default running at minimum level required for clock controllers. Adding power-domains property for CX/MX rails is like a redundant code as that will also scale such rails at default specified minimum level only. Also we hadn't added such property for other targets DT nodes to scale up CX/MX at minimum level.
> 
> What I mean here is that, the minimum level may not be enough. In such case
> you would also add a required-opps = <&handle_to_rpmhpd_opp_level>
> 
> Konrad
> 

Apologies, but could you please elaborate the use-case where minimum 
level isn't enough ? I guess for clock controllers configuration min 
level of CX/MX would be suffice, client will anyhow scale such rails to 
higher levels depending on their use-case.
Konrad Dybcio July 12, 2024, 12:40 p.m. UTC | #8
On 12.07.2024 2:31 PM, Ajit Pandey wrote:
> 
> 
> On 7/12/2024 5:52 PM, Konrad Dybcio wrote:
>> On 12.07.2024 11:53 AM, Ajit Pandey wrote:
>>>
>>>
>>> On 7/11/2024 3:25 PM, Konrad Dybcio wrote:
>>>> On 3.07.2024 11:16 AM, Ajit Pandey wrote:
>>>>>
>>>>>
>>>>> On 6/13/2024 1:11 PM, Konrad Dybcio wrote:
>>>>>>
>>>>>>
>>>>>> On 6/11/24 15:37, Ajit Pandey wrote:
>>>>>>> Add device node for camera, display and graphics clock controller on
>>>>>>> Qualcomm SM4450 platform.
>>>>>>>
>>>>>>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>>>>>>> ---
>>>>>>
>>>>>> None of these nodes reference a power domain (which would usually be
>>>>>> CX/MX/MMCX). This way, the RPMhPDs will never be scaled.
>>>>>>
>>>>>> The current upstream implementation only allows one power domain to be
>>>>>> scaled, but that's better than none (see other DTs for recent SoCs).
>>>>>>
>>>>>> Konrad
>>>>>
>>>>> SM4450 doesn't support MMCX and CX/MX domains will remain active so
>>>>> power-domains property is actually not required for SM4450 clock nodes.
>>>>
>>>> It's not only about them being active.. some PLLs require e.g. MX to be
>>>> at a certain level, or the system will be unstable
>>>>
>>>> Konrad
>>>
>>> With active I mean CX/MX rails will be default running at minimum level required for clock controllers. Adding power-domains property for CX/MX rails is like a redundant code as that will also scale such rails at default specified minimum level only. Also we hadn't added such property for other targets DT nodes to scale up CX/MX at minimum level.
>>
>> What I mean here is that, the minimum level may not be enough. In such case
>> you would also add a required-opps = <&handle_to_rpmhpd_opp_level>
>>
>> Konrad
>>
> 
> Apologies, but could you please elaborate the use-case where minimum level isn't enough ? I guess for clock controllers configuration min level of CX/MX would be suffice, client will anyhow scale such rails to higher levels depending on their use-case.

The main issue here is with PLLs within the clock controllers. Nobody
votes for them. It's an unsolved problem and we currently work around
cases where it's necessary by requiring that (with runtime pm, so when
there's active consumers of the clock controller) the attached power
domain is at >= SOME_LEVEL

Konrad
Taniya Das July 16, 2024, 8:39 a.m. UTC | #9
On 7/12/2024 6:10 PM, Konrad Dybcio wrote:
> On 12.07.2024 2:31 PM, Ajit Pandey wrote:
>>
>>
>> On 7/12/2024 5:52 PM, Konrad Dybcio wrote:
>>> On 12.07.2024 11:53 AM, Ajit Pandey wrote:
>>>>
>>>>
>>>> On 7/11/2024 3:25 PM, Konrad Dybcio wrote:
>>>>> On 3.07.2024 11:16 AM, Ajit Pandey wrote:
>>>>>>
>>>>>>
>>>>>> On 6/13/2024 1:11 PM, Konrad Dybcio wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 6/11/24 15:37, Ajit Pandey wrote:
>>>>>>>> Add device node for camera, display and graphics clock controller on
>>>>>>>> Qualcomm SM4450 platform.
>>>>>>>>
>>>>>>>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>>>>>>>> ---
>>>>>>>
>>>>>>> None of these nodes reference a power domain (which would usually be
>>>>>>> CX/MX/MMCX). This way, the RPMhPDs will never be scaled.
>>>>>>>
>>>>>>> The current upstream implementation only allows one power domain to be
>>>>>>> scaled, but that's better than none (see other DTs for recent SoCs).
>>>>>>>
>>>>>>> Konrad
>>>>>>
>>>>>> SM4450 doesn't support MMCX and CX/MX domains will remain active so
>>>>>> power-domains property is actually not required for SM4450 clock nodes.
>>>>>
>>>>> It's not only about them being active.. some PLLs require e.g. MX to be
>>>>> at a certain level, or the system will be unstable
>>>>>
>>>>> Konrad
>>>>
>>>> With active I mean CX/MX rails will be default running at minimum level required for clock controllers. Adding power-domains property for CX/MX rails is like a redundant code as that will also scale such rails at default specified minimum level only. Also we hadn't added such property for other targets DT nodes to scale up CX/MX at minimum level.
>>>
>>> What I mean here is that, the minimum level may not be enough. In such case
>>> you would also add a required-opps = <&handle_to_rpmhpd_opp_level>
>>>
>>> Konrad
>>>
>>
>> Apologies, but could you please elaborate the use-case where minimum level isn't enough ? I guess for clock controllers configuration min level of CX/MX would be suffice, client will anyhow scale such rails to higher levels depending on their use-case.
> 
> The main issue here is with PLLs within the clock controllers. Nobody
> votes for them. It's an unsolved problem and we currently work around
> cases where it's necessary by requiring that (with runtime pm, so when
> there's active consumers of the clock controller) the attached power
> domain is at >= SOME_LEVEL
> 
> Konrad

Konrad, this target (SM4450) have all the PLLs connected to CX/MX(again 
this is not collapsible). At boot the RPMHPD driver would keep the rails 
at minimum level and which is good to operate for the clock controller. 
I do not see currently this requirement you pose here specifically for 
SM4450.

As part of the PLL requirement within clock controller, this is 
definitely a requirement which we plan to RFC soon. There are 
discussions already in progress on how to handle this requirement.
Konrad Dybcio July 16, 2024, 10:39 a.m. UTC | #10
On 16.07.2024 10:39 AM, Taniya Das wrote:
> 
> 
> On 7/12/2024 6:10 PM, Konrad Dybcio wrote:
>> On 12.07.2024 2:31 PM, Ajit Pandey wrote:
>>>
>>>
>>> On 7/12/2024 5:52 PM, Konrad Dybcio wrote:
>>>> On 12.07.2024 11:53 AM, Ajit Pandey wrote:
>>>>>
>>>>>
>>>>> On 7/11/2024 3:25 PM, Konrad Dybcio wrote:
>>>>>> On 3.07.2024 11:16 AM, Ajit Pandey wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 6/13/2024 1:11 PM, Konrad Dybcio wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 6/11/24 15:37, Ajit Pandey wrote:
>>>>>>>>> Add device node for camera, display and graphics clock controller on
>>>>>>>>> Qualcomm SM4450 platform.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>>>>>>>>> ---
>>>>>>>>
>>>>>>>> None of these nodes reference a power domain (which would usually be
>>>>>>>> CX/MX/MMCX). This way, the RPMhPDs will never be scaled.
>>>>>>>>
>>>>>>>> The current upstream implementation only allows one power domain to be
>>>>>>>> scaled, but that's better than none (see other DTs for recent SoCs).
>>>>>>>>
>>>>>>>> Konrad
>>>>>>>
>>>>>>> SM4450 doesn't support MMCX and CX/MX domains will remain active so
>>>>>>> power-domains property is actually not required for SM4450 clock nodes.
>>>>>>
>>>>>> It's not only about them being active.. some PLLs require e.g. MX to be
>>>>>> at a certain level, or the system will be unstable
>>>>>>
>>>>>> Konrad
>>>>>
>>>>> With active I mean CX/MX rails will be default running at minimum level required for clock controllers. Adding power-domains property for CX/MX rails is like a redundant code as that will also scale such rails at default specified minimum level only. Also we hadn't added such property for other targets DT nodes to scale up CX/MX at minimum level.
>>>>
>>>> What I mean here is that, the minimum level may not be enough. In such case
>>>> you would also add a required-opps = <&handle_to_rpmhpd_opp_level>
>>>>
>>>> Konrad
>>>>
>>>
>>> Apologies, but could you please elaborate the use-case where minimum level isn't enough ? I guess for clock controllers configuration min level of CX/MX would be suffice, client will anyhow scale such rails to higher levels depending on their use-case.
>>
>> The main issue here is with PLLs within the clock controllers. Nobody
>> votes for them. It's an unsolved problem and we currently work around
>> cases where it's necessary by requiring that (with runtime pm, so when
>> there's active consumers of the clock controller) the attached power
>> domain is at >= SOME_LEVEL
>>
>> Konrad
> 
> Konrad, this target (SM4450) have all the PLLs connected to CX/MX(again this is not collapsible). At boot the RPMHPD driver would keep the rails at minimum level and which is good to operate for the clock controller. I do not see currently this requirement you pose here specifically for SM4450.
> 
> As part of the PLL requirement within clock controller, this is definitely a requirement which we plan to RFC soon. There are discussions already in progress on how to handle this requirement.

Ok, if it works, let's keep it as is until that RFC

Konrad
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm4450.dtsi b/arch/arm64/boot/dts/qcom/sm4450.dtsi
index 9c9919e78fbd..1e05cd00b635 100644
--- a/arch/arm64/boot/dts/qcom/sm4450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm4450.dtsi
@@ -4,7 +4,10 @@ 
  */
 
 #include <dt-bindings/clock/qcom,rpmh.h>
+#include <dt-bindings/clock/qcom,sm4450-camcc.h>
+#include <dt-bindings/clock/qcom,sm4450-dispcc.h>
 #include <dt-bindings/clock/qcom,sm4450-gcc.h>
+#include <dt-bindings/clock/qcom,sm4450-gpucc.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
@@ -422,6 +425,41 @@  tcsr_mutex: hwlock@1f40000 {
 			#hwlock-cells = <1>;
 		};
 
+		gpucc: clock-controller@3d90000 {
+			compatible = "qcom,sm4450-gpucc";
+			reg = <0x0 0x03d90000 0x0 0xa000>;
+			clocks = <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_GPU_GPLL0_CLK_SRC>,
+				 <&gcc GCC_GPU_GPLL0_DIV_CLK_SRC>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			#power-domain-cells = <1>;
+		};
+
+		camcc: clock-controller@ade0000 {
+			compatible = "qcom,sm4450-camcc";
+			reg = <0x0 0x0ade0000 0x0 0x20000>;
+			clocks = <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_CAMERA_AHB_CLK>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			#power-domain-cells = <1>;
+		};
+
+		dispcc: clock-controller@af00000 {
+			compatible = "qcom,sm4450-dispcc";
+			reg = <0x0 0x0af00000 0x0 0x20000>;
+			clocks = <&rpmhcc RPMH_CXO_CLK>,
+				 <&rpmhcc RPMH_CXO_CLK_A>,
+				 <&gcc GCC_DISP_AHB_CLK>,
+				 <&sleep_clk>,
+				 <0>,
+				 <0>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			#power-domain-cells = <1>;
+		};
+
 		pdc: interrupt-controller@b220000 {
 			compatible = "qcom,sm4450-pdc", "qcom,pdc";
 			reg = <0 0x0b220000 0 0x30000>, <0 0x174000f0 0 0x64>;