diff mbox series

arm64: dts: sdm845: Add display nodes to MTP dts

Message ID 1541187725-24325-1-git-send-email-jsanka@codeaurora.org (mailing list archive)
State Not Applicable, archived
Headers show
Series arm64: dts: sdm845: Add display nodes to MTP dts | expand

Commit Message

Jeykumar Sankaran Nov. 2, 2018, 7:42 p.m. UTC
Add mdss, dsi, dsi_phy, dsi pinctrl  and truly nt35597 panel nodes to
sdm845 MTP board dtsi.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 124 ++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

Comments

Doug Anderson Nov. 2, 2018, 8:33 p.m. UTC | #1
Hi,

On Fri, Nov 2, 2018 at 12:42 PM Jeykumar Sankaran <jsanka@codeaurora.org> wrote:
>
> Add mdss, dsi, dsi_phy, dsi pinctrl  and truly nt35597 panel nodes to
> sdm845 MTP board dtsi.
>
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 124 ++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)

A few nits below about trying to keep the sort ordering right in this
file.  I'm not an expert on device tree properties for display, but
other than these nits things here look good to me.

NOTE also that I'd probably put all 3 of your recent patches in a
3-part series since they all depend on each other, don't they?  AKA
I'd to see next time:

[PATCH v4 1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
[PATCH v4 2/3] arm64: dts: sdm845: Add dsi pinctrl nodes
[PATCH v4 3/3] arm64: dts: sdm845: Add display nodes to MTP dts

...you can call them all v4 just to make it easier to track...


> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 6d651f3..6e98ae8 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -8,6 +8,7 @@
>  /dts-v1/;
>
>  #include "sdm845.dtsi"
> +#include <dt-bindings/gpio/gpio.h>

Generally includes should have < includes above " includes.

...also: please rebase atop Andy's tree.  Then you'll have the line:

#include <dt-bindings/regulator/qcom,rpmh-regulator.h>

..."gpio" is alphabetically before "regulator" so your include should
be _above_ that one.

>
>  / {
>         model = "Qualcomm Technologies, Inc. SDM845 MTP";
> @@ -22,6 +23,113 @@
>         };
>  };
>
> +&dsi0 {
> +       status = "okay";
> +       qcom,dual-dsi-mode;
> +       qcom,master-dsi;
> +       qcom,sync-dual-dsi;
> +
> +       vdda-supply = <&vdda_mipi_dsi0_1p2>;
> +
> +       panel@0 {
> +               compatible = "truly,nt35597-2K-display";
> +               reg = <0>;
> +
> +               vdda-supply = <&vreg_l14a_1p88>;
> +               vdispp-supply = <&lab_regulator>;
> +               vdispn-supply = <&ibb_regulator>;
> +
> +               pinctrl-names = "default", "suspend";
> +               pinctrl-0 = <&dpu_dsi_active>;
> +               pinctrl-1 = <&dpu_dsi_suspend>;
> +
> +               reset-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;
> +               mode-gpios = <&tlmm 52 GPIO_ACTIVE_HIGH>;
> +
> +               display-timings {
> +                       timing0: timing-0 {
> +                               /* originally
> +                                * 268316160 Mhz,
> +                                * but value below fits
> +                                * better w/ downstream
> +                                */
> +                               clock-frequency = <268316138>;
> +                               hactive = <1440>;
> +                               vactive = <2560>;
> +                               hfront-porch = <200>;
> +                               hback-porch = <64>;
> +                               hsync-len = <32>;
> +                               vfront-porch = <8>;
> +                               vback-porch = <7>;
> +                               vsync-len = <1>;
> +                       };
> +               };
> +
> +               ports {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       port@0 {
> +                               reg = <0>;
> +                               panel0_in: endpoint {
> +                                       remote-endpoint = <&dsi0_out>;
> +                               };
> +                       };
> +
> +                       port@1 {
> +                               reg = <1>;
> +                               panel1_in: endpoint {
> +                                       remote-endpoint = <&dsi1_out>;
> +                               };
> +                       };
> +               };
> +       };
> +
> +       ports {
> +               port@1 {
> +                       endpoint {
> +                               remote-endpoint = <&panel0_in>;
> +                               data-lanes = <0 1 2 3>;
> +                       };
> +               };
> +       };
> +};
> +
> +&dsi0_phy {
> +       status = "okay";
> +       vdds-supply = <&vdda_mipi_dsi0_pll>;
> +};
> +
> +&dsi1 {
> +       status = "okay";
> +
> +       qcom,dual-dsi-mode;
> +       qcom,sync-dual-dsi;
> +
> +       vdda-supply = <&vdda_mipi_dsi1_1p2>;
> +
> +       ports {
> +               port@1 {
> +                       endpoint {
> +                               remote-endpoint = <&panel1_in>;
> +                               data-lanes = <0 1 2 3>;
> +                       };
> +               };
> +       };
> +};
> +
> +&dsi1_phy {
> +       status = "okay";
> +       vdds-supply = <&vdda_mipi_dsi1_pll>;
> +};
> +
> +&mdss {
> +       status = "okay";
> +};
> +
> +&mdss_mdp {
> +       status = "okay";
> +};

Please keep these nodes sorted alphabetically even if it means you
don't keep all the display nodes together.  Thus "mdss" and "mdss_mdp"
should be below "i2c10" and above "qupv3_id_1".


> +
>  &i2c10 {
>         status = "okay";
>         clock-frequency = <400000>;
> @@ -58,3 +166,19 @@
>                 bias-pull-up;
>         };
>  };
> +
> +&dpu_dsi_active {
> +       pinconf {
> +               pins = "gpio6", "gpio52";
> +               drive-strength = <8>;
> +               bias-disable;
> +       };
> +};
> +
> +&dpu_dsi_suspend {
> +       pinconf {
> +               pins = "gpio6", "gpio52";
> +               drive-strength = <2>;
> +               bias-pull-down;
> +       };
> +};

These two things should sorted alphabetically in the "PINCTRL -
additions to nodes defined in sdm845.dtsi" section.  So please place
them both above the "qup_i2c10_default" node.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index 6d651f3..6e98ae8 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -8,6 +8,7 @@ 
 /dts-v1/;
 
 #include "sdm845.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "Qualcomm Technologies, Inc. SDM845 MTP";
@@ -22,6 +23,113 @@ 
 	};
 };
 
+&dsi0 {
+	status = "okay";
+	qcom,dual-dsi-mode;
+	qcom,master-dsi;
+	qcom,sync-dual-dsi;
+
+	vdda-supply = <&vdda_mipi_dsi0_1p2>;
+
+	panel@0 {
+		compatible = "truly,nt35597-2K-display";
+		reg = <0>;
+
+		vdda-supply = <&vreg_l14a_1p88>;
+		vdispp-supply = <&lab_regulator>;
+		vdispn-supply = <&ibb_regulator>;
+
+		pinctrl-names = "default", "suspend";
+		pinctrl-0 = <&dpu_dsi_active>;
+		pinctrl-1 = <&dpu_dsi_suspend>;
+
+		reset-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;
+		mode-gpios = <&tlmm 52 GPIO_ACTIVE_HIGH>;
+
+		display-timings {
+			timing0: timing-0 {
+				/* originally
+				 * 268316160 Mhz,
+				 * but value below fits
+				 * better w/ downstream
+				 */
+				clock-frequency = <268316138>;
+				hactive = <1440>;
+				vactive = <2560>;
+				hfront-porch = <200>;
+				hback-porch = <64>;
+				hsync-len = <32>;
+				vfront-porch = <8>;
+				vback-porch = <7>;
+				vsync-len = <1>;
+			};
+		};
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			port@0 {
+				reg = <0>;
+				panel0_in: endpoint {
+					remote-endpoint = <&dsi0_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				panel1_in: endpoint {
+					remote-endpoint = <&dsi1_out>;
+				};
+			};
+		};
+	};
+
+	ports {
+		port@1 {
+			endpoint {
+				remote-endpoint = <&panel0_in>;
+				data-lanes = <0 1 2 3>;
+			};
+		};
+	};
+};
+
+&dsi0_phy {
+	status = "okay";
+	vdds-supply = <&vdda_mipi_dsi0_pll>;
+};
+
+&dsi1 {
+	status = "okay";
+
+	qcom,dual-dsi-mode;
+	qcom,sync-dual-dsi;
+
+	vdda-supply = <&vdda_mipi_dsi1_1p2>;
+
+	ports {
+		port@1 {
+			endpoint {
+				remote-endpoint = <&panel1_in>;
+				data-lanes = <0 1 2 3>;
+			};
+		};
+	};
+};
+
+&dsi1_phy {
+	status = "okay";
+	vdds-supply = <&vdda_mipi_dsi1_pll>;
+};
+
+&mdss {
+	status = "okay";
+};
+
+&mdss_mdp {
+	status = "okay";
+};
+
 &i2c10 {
 	status = "okay";
 	clock-frequency = <400000>;
@@ -58,3 +166,19 @@ 
 		bias-pull-up;
 	};
 };
+
+&dpu_dsi_active {
+	pinconf {
+		pins = "gpio6", "gpio52";
+		drive-strength = <8>;
+		bias-disable;
+	};
+};
+
+&dpu_dsi_suspend {
+	pinconf {
+		pins = "gpio6", "gpio52";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+};