linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] arm64: dts: qcom: sc7180: Add Display Port dt node
@ 2020-06-18 23:21 Tanmay Shah
  2020-06-19  1:12 ` Stephen Boyd
  0 siblings, 1 reply; 3+ messages in thread
From: Tanmay Shah @ 2020-06-18 23:21 UTC (permalink / raw)
  To: swboyd, agross
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, dianders, aravindh, abhinavk,
	bjorn.andersson, Tanmay Shah

Enable DP driver for sc7180.

This change depends-on following series:
https://patchwork.freedesktop.org/series/78583/
and https://patchwork.freedesktop.org/patch/351990/

Changes in v2:

- Add assigned-clocks and assigned-clock-parents
- Remove cell-index and pixel_rcg
- Change compatible to qcom,sc7180-dp

Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 57 ++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 916401f7e87c..26fe623e3b0f 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2216,10 +2216,19 @@ ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
 
+					port@1 {
+						reg = <1>;
+						dpu_intf1_out: endpoint {
+							remote-endpoint =
+								 <&dsi0_in>;
+						};
+					};
+
 					port@0 {
 						reg = <0>;
-						dpu_intf1_out: endpoint {
-							remote-endpoint = <&dsi0_in>;
+						dpu_intf0_out: endpoint {
+							remote-endpoint =
+								 <&dp_in>;
 						};
 					};
 				};
@@ -2293,6 +2302,46 @@ dsi_phy: dsi-phy@ae94400 {
 			};
 		};
 
+		msm_dp: displayport-controller@ae90000{
+			status = "ok";
+			compatible = "qcom,sc7180-dp";
+
+			reg = <0 0xae90000 0 0x1400>;
+			reg-names = "dp_controller";
+
+			interrupt-parent = <&mdss>;
+			interrupts = <12 0>;
+
+			clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
+				 <&dispcc DISP_CC_MDSS_DP_LINK_CLK>,
+				 <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>,
+				 <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>;
+			clock-names = "core_aux", "ctrl_link",
+				      "ctrl_link_iface", "stream_pixel";
+			#clock-cells = <1>;
+			assigned-clocks = <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
+			assigned-clock-parents = <&msm_dp 1>;
+
+			data-lanes = <0 1>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					dp_in: endpoint {
+						remote-endpoint =
+							 <&dpu_intf0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					dp_out: endpoint { };
+				};
+			};
+		};
+
 		dispcc: clock-controller@af00000 {
 			compatible = "qcom,sc7180-dispcc";
 			reg = <0 0x0af00000 0 0x200000>;
@@ -2300,8 +2349,8 @@ dispcc: clock-controller@af00000 {
 				 <&gcc GCC_DISP_GPLL0_CLK_SRC>,
 				 <&dsi_phy 0>,
 				 <&dsi_phy 1>,
-				 <0>,
-				 <0>;
+				 <&msm_dp 0>,
+				 <&msm_dp 1>;
 			clock-names = "bi_tcxo",
 				      "gcc_disp_gpll0_clk_src",
 				      "dsi0_phy_pll_out_byteclk",
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] arm64: dts: qcom: sc7180: Add Display Port dt node
  2020-06-18 23:21 [PATCH v2] arm64: dts: qcom: sc7180: Add Display Port dt node Tanmay Shah
@ 2020-06-19  1:12 ` Stephen Boyd
  2020-06-19 21:55   ` tanmay
  0 siblings, 1 reply; 3+ messages in thread
From: Stephen Boyd @ 2020-06-19  1:12 UTC (permalink / raw)
  To: Tanmay Shah, agross
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, dianders, aravindh, abhinavk,
	bjorn.andersson, Tanmay Shah

Quoting Tanmay Shah (2020-06-18 16:21:13)
> Enable DP driver for sc7180.

Add DP device node on sc7180? This isn't a driver.

> 
> This change depends-on following series:
> https://patchwork.freedesktop.org/series/78583/
> and https://patchwork.freedesktop.org/patch/351990/
> 
> Changes in v2:
> 
> - Add assigned-clocks and assigned-clock-parents
> - Remove cell-index and pixel_rcg
> - Change compatible to qcom,sc7180-dp
> 
> Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 57 ++++++++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 916401f7e87c..26fe623e3b0f 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -2216,10 +2216,19 @@ ports {
>                                         #address-cells = <1>;
>                                         #size-cells = <0>;
>  
> +                                       port@1 {
> +                                               reg = <1>;
> +                                               dpu_intf1_out: endpoint {
> +                                                       remote-endpoint =
> +                                                                <&dsi0_in>;
> +                                               };
> +                                       };
> +
>                                         port@0 {
>                                                 reg = <0>;
> -                                               dpu_intf1_out: endpoint {
> -                                                       remote-endpoint = <&dsi0_in>;
> +                                               dpu_intf0_out: endpoint {
> +                                                       remote-endpoint =
> +                                                                <&dp_in>;
>                                                 };
>                                         };
>                                 };

I thought this wasn't supposed to change? At least according to the
binding it shouldn't be needed.

> @@ -2293,6 +2302,46 @@ dsi_phy: dsi-phy@ae94400 {
>                         };
>                 };
>  
> +               msm_dp: displayport-controller@ae90000{
> +                       status = "ok";

Please use status = "disabled";

> +                       compatible = "qcom,sc7180-dp";
> +
> +                       reg = <0 0xae90000 0 0x1400>;
> +                       reg-names = "dp_controller";
> +
> +                       interrupt-parent = <&mdss>;

Any reason why this isn't under the mdss node like the other display
device nodes?

> +                       interrupts = <12 0>;
> +
> +                       clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
> +                                <&dispcc DISP_CC_MDSS_DP_LINK_CLK>,
> +                                <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>,
> +                                <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>;
> +                       clock-names = "core_aux", "ctrl_link",
> +                                     "ctrl_link_iface", "stream_pixel";
> +                       #clock-cells = <1>;
> +                       assigned-clocks = <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
> +                       assigned-clock-parents = <&msm_dp 1>;
> +
> +                       data-lanes = <0 1>;

This can and should be left to the board files. At the SoC level my
understanding is that there are four lanes possible, so no need to
artificially limit it here.

> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               port@0 {
> +                                       reg = <0>;
> +                                       dp_in: endpoint {
> +                                               remote-endpoint =
> +                                                        <&dpu_intf0_out>;

I'd prefer these were on one line, regardless of the 80 character line
limit/suggestion.

> +                                       };
> +                               };
> +
> +                               port@1 {
> +                                       reg = <1>;
> +                                       dp_out: endpoint { };
> +                               };
> +                       };
> +               };
> +
>                 dispcc: clock-controller@af00000 {
>                         compatible = "qcom,sc7180-dispcc";
>                         reg = <0 0x0af00000 0 0x200000>;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] arm64: dts: qcom: sc7180: Add Display Port dt node
  2020-06-19  1:12 ` Stephen Boyd
@ 2020-06-19 21:55   ` tanmay
  0 siblings, 0 replies; 3+ messages in thread
From: tanmay @ 2020-06-19 21:55 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: agross, devicetree, linux-kernel, linux-arm-msm, dri-devel,
	freedreno, seanpaul, robdclark, dianders, aravindh, abhinavk,
	bjorn.andersson

Thanks Stephen for reviews.
I will post new change addressing your all comments.

On 2020-06-18 18:12, Stephen Boyd wrote:
> Quoting Tanmay Shah (2020-06-18 16:21:13)
>> Enable DP driver for sc7180.
> 
> Add DP device node on sc7180? This isn't a driver.
> 
Done.
>> 
>> This change depends-on following series:
>> https://patchwork.freedesktop.org/series/78583/
>> and https://patchwork.freedesktop.org/patch/351990/
>> 
>> Changes in v2:
>> 
>> - Add assigned-clocks and assigned-clock-parents
>> - Remove cell-index and pixel_rcg
>> - Change compatible to qcom,sc7180-dp
>> 
>> Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 57 
>> ++++++++++++++++++++++++++--
>>  1 file changed, 53 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> index 916401f7e87c..26fe623e3b0f 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -2216,10 +2216,19 @@ ports {
>>                                         #address-cells = <1>;
>>                                         #size-cells = <0>;
>> 
>> +                                       port@1 {
>> +                                               reg = <1>;
>> +                                               dpu_intf1_out: 
>> endpoint {
>> +                                                       
>> remote-endpoint =
>> +                                                                
>> <&dsi0_in>;
>> +                                               };
>> +                                       };
>> +
>>                                         port@0 {
>>                                                 reg = <0>;
>> -                                               dpu_intf1_out: 
>> endpoint {
>> -                                                       
>> remote-endpoint = <&dsi0_in>;
>> +                                               dpu_intf0_out: 
>> endpoint {
>> +                                                       
>> remote-endpoint =
>> +                                                                
>> <&dp_in>;
>>                                                 };
>>                                         };
>>                                 };
> 
> I thought this wasn't supposed to change? At least according to the
> binding it shouldn't be needed.
> 
Thanks. Yes it should be port@2 as per new dpu-sc7180.yaml bindings in 
below patch.
https://patchwork.freedesktop.org/patch/371087/?series=78583&rev=1

>> @@ -2293,6 +2302,46 @@ dsi_phy: dsi-phy@ae94400 {
>>                         };
>>                 };
>> 
>> +               msm_dp: displayport-controller@ae90000{
>> +                       status = "ok";
> 
> Please use status = "disabled";
> 
Ok got it. Board's DT will enable it.

>> +                       compatible = "qcom,sc7180-dp";
>> +
>> +                       reg = <0 0xae90000 0 0x1400>;
>> +                       reg-names = "dp_controller";
>> +
>> +                       interrupt-parent = <&mdss>;
> 
> Any reason why this isn't under the mdss node like the other display
> device nodes?
> 
Done. Moved msm_dp node under mdss.
>> +                       interrupts = <12 0>;
>> +
>> +                       clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
>> +                                <&dispcc DISP_CC_MDSS_DP_LINK_CLK>,
>> +                                <&dispcc 
>> DISP_CC_MDSS_DP_LINK_INTF_CLK>,
>> +                                <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>;
>> +                       clock-names = "core_aux", "ctrl_link",
>> +                                     "ctrl_link_iface", 
>> "stream_pixel";
>> +                       #clock-cells = <1>;
>> +                       assigned-clocks = <&dispcc 
>> DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
>> +                       assigned-clock-parents = <&msm_dp 1>;
>> +
>> +                       data-lanes = <0 1>;
> 
> This can and should be left to the board files. At the SoC level my
> understanding is that there are four lanes possible, so no need to
> artificially limit it here.
> 
Removed from here. Driver changes I will be posting accordingly.
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +                               port@0 {
>> +                                       reg = <0>;
>> +                                       dp_in: endpoint {
>> +                                               remote-endpoint =
>> +                                                        
>> <&dpu_intf0_out>;
> 
> I'd prefer these were on one line, regardless of the 80 character line
> limit/suggestion.
> 
ok done.
>> +                                       };
>> +                               };
>> +
>> +                               port@1 {
>> +                                       reg = <1>;
>> +                                       dp_out: endpoint { };
>> +                               };
>> +                       };
>> +               };
>> +
>>                 dispcc: clock-controller@af00000 {
>>                         compatible = "qcom,sc7180-dispcc";
>>                         reg = <0 0x0af00000 0 0x200000>;

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-06-19 21:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-18 23:21 [PATCH v2] arm64: dts: qcom: sc7180: Add Display Port dt node Tanmay Shah
2020-06-19  1:12 ` Stephen Boyd
2020-06-19 21:55   ` tanmay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).