diff mbox series

[2/2] arm64: dts: freescale: Add LVDS overlay for TQMa8MPxL

Message ID 20221208090842.2869374-2-alexander.stein@ew.tq-group.com (mailing list archive)
State New, archived
Headers show
Series [1/2] arm64: dts: imx8mp: Add LCDIF2 & LDB nodes | expand

Commit Message

Alexander Stein Dec. 8, 2022, 9:08 a.m. UTC 
This adds an overlay for the supported LVDS display tianma tm070jvhg33.
The display timings have to be specified explicitly. Using the typical pixel
clock, the LDB clock can not be configured as the 7-fold of that.
By setting pixel clock to 74.25 MHz, LDB can be configured to exactly
519.75 MHz.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
To be hold I'm unsure where to list the overlays. I checked other overlays
and opted to added them at the end of imx8mp section.

 arch/arm64/boot/dts/freescale/Makefile        |  4 ++
 .../imx8mp-tqma8mpql-mba8mpxl-lvds.dtso       | 61 +++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso

Comments

Richard Leitner Dec. 20, 2022, 8:48 a.m. UTC | #1 
Hi Alexander,

thanks for the series.

On Thu, Dec 08, 2022 at 10:08:42AM +0100, Alexander Stein wrote:
> This adds an overlay for the supported LVDS display tianma tm070jvhg33.
> The display timings have to be specified explicitly. Using the typical pixel
> clock, the LDB clock can not be configured as the 7-fold of that.
> By setting pixel clock to 74.25 MHz, LDB can be configured to exactly
> 519.75 MHz.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> To be hold I'm unsure where to list the overlays. I checked other overlays
> and opted to added them at the end of imx8mp section.
> 
>  arch/arm64/boot/dts/freescale/Makefile        |  4 ++
>  .../imx8mp-tqma8mpql-mba8mpxl-lvds.dtso       | 61 +++++++++++++++++++
>  2 files changed, 65 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso

...

> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso
> new file mode 100644
> index 0000000000000..ea44d605342ba
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
> +/*
> + * Copyright (c) 2022 TQ-Systems GmbH <linux@ew.tq-group.com>,
> + * D-82229 Seefeld, Germany.
> + * Author: Alexander Stein
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&{/} {
> +	compatible = "tq,imx8mp-tqma8mpql-mba8mpxl", "tq,imx8mp-tqma8mpql", "fsl,imx8mp";
> +};
> +
> +&backlight_lvds {
> +	status = "okay";
> +};
> +
> +&display {
> +	compatible = "tianma,tm070jvhg33";
> +	status = "okay";
> +
> +	panel-timing {
> +		clock-frequency = <74250000>;
> +		hactive = <1280>;
> +		vactive = <800>;
> +		hfront-porch = <64>;
> +		hback-porch = <5>;
> +		hsync-len = <1>;
> +		vfront-porch = <40>;
> +		vback-porch = <2>;
> +		vsync-len = <1>;
> +		de-active = <1>;
> +	};
> +
> +	port {
> +		panel_in_lvds0: endpoint {
> +			remote-endpoint = <&ldb_lvds_ch0>;
> +		};
> +	};
> +};
> +
> +&lcdif2 {
> +	status = "okay";
> +};
> +
> +&lvds_bridge {
> +	status = "okay";
> +
> +	ports {
> +		port@1 {
> +			ldb_lvds_ch0: endpoint {
> +				remote-endpoint = <&panel_in_lvds0>;
> +			};
> +		};
> +	};
> +};

Wouldn't it be possible and easier to read if this was &ldb_lvds_ch0
only instead of the "whole" &lvds_bridge?

&ldb_lvds_ch0 {
	remote-endpoint = <&panel_in_lvds0>;
};

> +
> +&pwm2 {
> +	status = "okay";
> +};
> -- 
> 2.34.1
> 

regards;rl
Alexander Stein Jan. 2, 2023, 9:07 a.m. UTC | #2 
Hello Richard,

Am Dienstag, 20. Dezember 2022, 09:48:45 CET schrieb Richard Leitner:
> Hi Alexander,
> 
> thanks for the series.
> 
> On Thu, Dec 08, 2022 at 10:08:42AM +0100, Alexander Stein wrote:
> > This adds an overlay for the supported LVDS display tianma tm070jvhg33.
> > The display timings have to be specified explicitly. Using the typical
> > pixel clock, the LDB clock can not be configured as the 7-fold of that.
> > By setting pixel clock to 74.25 MHz, LDB can be configured to exactly
> > 519.75 MHz.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > To be hold I'm unsure where to list the overlays. I checked other overlays
> > and opted to added them at the end of imx8mp section.
> > 
> >  arch/arm64/boot/dts/freescale/Makefile        |  4 ++
> >  .../imx8mp-tqma8mpql-mba8mpxl-lvds.dtso       | 61 +++++++++++++++++++
> >  2 files changed, 65 insertions(+)
> >  create mode 100644
> >  arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso
> ...
> 
> > diff --git
> > a/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso
> > b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso new
> > file mode 100644
> > index 0000000000000..ea44d605342ba
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso
> > @@ -0,0 +1,61 @@
> > +// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
> > +/*
> > + * Copyright (c) 2022 TQ-Systems GmbH <linux@ew.tq-group.com>,
> > + * D-82229 Seefeld, Germany.
> > + * Author: Alexander Stein
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +&{/} {
> > +	compatible = "tq,imx8mp-tqma8mpql-mba8mpxl", "tq,imx8mp-tqma8mpql",
> > "fsl,imx8mp"; +};
> > +
> > +&backlight_lvds {
> > +	status = "okay";
> > +};
> > +
> > +&display {
> > +	compatible = "tianma,tm070jvhg33";
> > +	status = "okay";
> > +
> > +	panel-timing {
> > +		clock-frequency = <74250000>;
> > +		hactive = <1280>;
> > +		vactive = <800>;
> > +		hfront-porch = <64>;
> > +		hback-porch = <5>;
> > +		hsync-len = <1>;
> > +		vfront-porch = <40>;
> > +		vback-porch = <2>;
> > +		vsync-len = <1>;
> > +		de-active = <1>;
> > +	};
> > +
> > +	port {
> > +		panel_in_lvds0: endpoint {
> > +			remote-endpoint = <&ldb_lvds_ch0>;
> > +		};
> > +	};
> > +};
> > +
> > +&lcdif2 {
> > +	status = "okay";
> > +};
> > +
> > +&lvds_bridge {
> > +	status = "okay";
> > +
> > +	ports {
> > +		port@1 {
> > +			ldb_lvds_ch0: endpoint {
> > +				remote-endpoint = <&panel_in_lvds0>;
> > +			};
> > +		};
> > +	};
> > +};
> 
> Wouldn't it be possible and easier to read if this was &ldb_lvds_ch0
> only instead of the "whole" &lvds_bridge?
> 
> &ldb_lvds_ch0 {
> 	remote-endpoint = <&panel_in_lvds0>;
> };

Yes it is possible and easier to read. Unfortunately dtbs_check will raise the 
follwing warnings when doing so:
> arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtbo: Warning 
(graph_port): /fragment@4: graph port node name should be 'port'
> ../arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso:
47.15-49.3: Warning (graph_endpoint): /fragment@4/__overlay__: graph endpoint 
node name should be 'endpoint'
> ../arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso:
47.15-49.3: Warning (graph_endpoint): /fragment@4/__overlay__: graph 
connection to node '/fragment@2/__overlay__/port/endpoint' is not 
bidirectional

I'm not sure which way is the preferred/correct one or if this is just an 
issue in dtc, raising the warnings.

Best regards,
Alexander

> > +
> > +&pwm2 {
> > +	status = "okay";
> > +};
> 
> regards;rl
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index ef6f364eaa183..9eedee68413b2 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -97,6 +97,10 @@  dtb-$(CONFIG_ARCH_MXC) += imx8mp-verdin-nonwifi-dahlia.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-verdin-nonwifi-dev.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-verdin-wifi-dahlia.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-verdin-wifi-dev.dtb
+
+imx8mp-tqma8mpql-mba8mpxl-lvds-dtbs += imx8mp-tqma8mpql-mba8mpxl.dtb imx8mp-tqma8mpql-mba8mpxl-lvds.dtbo
+dtb-$(CONFIG_ARCH_MXC) += imx8mp-tqma8mpql-mba8mpxl-lvds.dtb
+
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-kontron-pitx-imx8m.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso
new file mode 100644
index 0000000000000..ea44d605342ba
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl-lvds.dtso
@@ -0,0 +1,61 @@ 
+// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
+/*
+ * Copyright (c) 2022 TQ-Systems GmbH <linux@ew.tq-group.com>,
+ * D-82229 Seefeld, Germany.
+ * Author: Alexander Stein
+ */
+
+/dts-v1/;
+/plugin/;
+
+&{/} {
+	compatible = "tq,imx8mp-tqma8mpql-mba8mpxl", "tq,imx8mp-tqma8mpql", "fsl,imx8mp";
+};
+
+&backlight_lvds {
+	status = "okay";
+};
+
+&display {
+	compatible = "tianma,tm070jvhg33";
+	status = "okay";
+
+	panel-timing {
+		clock-frequency = <74250000>;
+		hactive = <1280>;
+		vactive = <800>;
+		hfront-porch = <64>;
+		hback-porch = <5>;
+		hsync-len = <1>;
+		vfront-porch = <40>;
+		vback-porch = <2>;
+		vsync-len = <1>;
+		de-active = <1>;
+	};
+
+	port {
+		panel_in_lvds0: endpoint {
+			remote-endpoint = <&ldb_lvds_ch0>;
+		};
+	};
+};
+
+&lcdif2 {
+	status = "okay";
+};
+
+&lvds_bridge {
+	status = "okay";
+
+	ports {
+		port@1 {
+			ldb_lvds_ch0: endpoint {
+				remote-endpoint = <&panel_in_lvds0>;
+			};
+		};
+	};
+};
+
+&pwm2 {
+	status = "okay";
+};