Skip to contentSkip to conversation

ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

PATCH New christopher.spinrath at rwth-aachen.de 3 participants 10 messages Dec 2, 2016 – Jan 23, 2017 4 messages not in the archive

Subsystems (2)
Device tree bindings Arm i.MX and Layerscape
lore
Share & tools
lore

christopher.spinrath at rwth-aachen.de

author
#0 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

From: Christopher Spinrath <[email protected]>

Apart from the already enabled Designware HDMI port, the Utilite Pro has a second display pipeline which has the following shape:

IPU1 DI0 --> Parallel display --> tfp410 rgb24 to DVI encoder

                                --> HDMI connector.

Enable support for it.

In addition, since this pipeline is hardwired to IPU1, sever the link between IPU1 and the SoC-internal Designware HDMI encoder forcing the latter to be connected to IPU2 instead of IPU1. Otherwise, it is not possible to drive both displays at high resolution due to the bandwidth limitations of a single IPU.

Signed-off-by Christopher Spinrath <[email protected]>

---

Hi all,

the removal of the link between IPU1 and the Designware HDMI encoder is the result of a discussion I had with Philipp Zabel:

https://lists.freedesktop.org/archives/dri-devel/2016-November/125399.html .

Altough it is not possible to connect anything else to IPU1 on the Utilite, this approach has at least one disadvantage: if the resolution is low enough such that a single IPU can handle both displays then muxing both displays to IPU1 would reduce the power consumption.

However, IMHO omitting the link IPU1 <--> DW HDMI is still the preferrable solution since I'm not aware of any OS/driver that is capable of switching IPUs or can handle the bandwidth limitation in a sane way. In particular, Linux is unusable when both displays are supposed to be driven at high resolution and both muxing options for the DW HDMI are available (this is not a userspace issue; the system becomes almost unresponsive as soon as the kernel sets the initial resolution).

Cheers,
Christopher

P.S.: this patch depends on the tfp410 bridge driver which has recently been merged into drm-next.

 arch/arm/boot/dts/imx6q-utilite-pro.dts | 115 ++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +115 -0
index 2200994..69bdd82 100644
@@ -59,6 +59,33 @@
 		rtc1 = &snvs_rtc;
 	};
 
+	encoder {
+		compatible = "ti,tfp410";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+
+				tfp410_in: endpoint {
+					remote-endpoint = <&parallel_display_out>;
+				};
+			};
+
+			port at 1 {
+				reg = <1>;
+
+				tfp410_out: endpoint {
+					remote-endpoint = <&hdmi_connector_in>;
+				};
+			};
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";

@@ -72,6 +99,19 @@
 		};
 	};
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+
+		type = "a";
+		ddc-i2c-bus = <&i2c_dvi_ddc>;
+
+		port {
+			hdmi_connector_in: endpoint {
+				remote-endpoint = <&tfp410_out>;
+			};
+		};
+	};
+
 	i2cmux {
 		compatible = "i2c-mux-gpio";
 		pinctrl-names = "default";

@@ -105,8 +145,46 @@
 			#size-cells = <0>;
 		};
 	};
+
+	parallel-display {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu1>;
+
+		interface-pix-fmt = "rgb24";
+
+		port at 0 {
+			reg = <0>;
+
+			parallel_display_in: endpoint {
+				remote-endpoint = <&ipu1_di0_disp0>;
+			};
+		};
+
+		port at 1 {
+			reg = <1>;
+
+			parallel_display_out: endpoint {
+				remote-endpoint = <&tfp410_in>;
+			};
+		};
+	};
 };
 
+/*
+ * A single IPU is not able to drive both display interfaces available on the
+ * Utilite Pro at high resolution due to its bandwidth limitation. Since the
+ * tfp410 encoder is wired up to IPU1, sever the link between IPU1 and the
+ * SoC-internal Designware HDMI encoder forcing the latter to be connected to
+ * IPU2 instead of IPU1.
+ */
+/delete-node/&ipu1_di0_hdmi;
+/delete-node/&hdmi_mux_0;
+/delete-node/&ipu1_di1_hdmi;
+/delete-node/&hdmi_mux_1;
+
 &hdmi {
 	ddc-i2c-bus = <&i2c2>;
 	status = "okay";

@@ -151,6 +229,39 @@
 		>;
 	};
 
+	pinctrl_ipu1: ipu1grp {
+		fsl,pins = <
+			MX6QDL_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK 0x38
+			MX6QDL_PAD_DI0_PIN15__IPU1_DI0_PIN15       0x38
+			MX6QDL_PAD_DI0_PIN2__IPU1_DI0_PIN02        0x38
+			MX6QDL_PAD_DI0_PIN3__IPU1_DI0_PIN03        0x38
+			MX6QDL_PAD_DISP0_DAT0__IPU1_DISP0_DATA00   0x38
+			MX6QDL_PAD_DISP0_DAT1__IPU1_DISP0_DATA01   0x38
+			MX6QDL_PAD_DISP0_DAT2__IPU1_DISP0_DATA02   0x38
+			MX6QDL_PAD_DISP0_DAT3__IPU1_DISP0_DATA03   0x38
+			MX6QDL_PAD_DISP0_DAT4__IPU1_DISP0_DATA04   0x38
+			MX6QDL_PAD_DISP0_DAT5__IPU1_DISP0_DATA05   0x38
+			MX6QDL_PAD_DISP0_DAT6__IPU1_DISP0_DATA06   0x38
+			MX6QDL_PAD_DISP0_DAT7__IPU1_DISP0_DATA07   0x38
+			MX6QDL_PAD_DISP0_DAT8__IPU1_DISP0_DATA08   0x38
+			MX6QDL_PAD_DISP0_DAT9__IPU1_DISP0_DATA09   0x38
+			MX6QDL_PAD_DISP0_DAT10__IPU1_DISP0_DATA10  0x38
+			MX6QDL_PAD_DISP0_DAT11__IPU1_DISP0_DATA11  0x38
+			MX6QDL_PAD_DISP0_DAT12__IPU1_DISP0_DATA12  0x38
+			MX6QDL_PAD_DISP0_DAT13__IPU1_DISP0_DATA13  0x38
+			MX6QDL_PAD_DISP0_DAT14__IPU1_DISP0_DATA14  0x38
+			MX6QDL_PAD_DISP0_DAT15__IPU1_DISP0_DATA15  0x38
+			MX6QDL_PAD_DISP0_DAT16__IPU1_DISP0_DATA16  0x38
+			MX6QDL_PAD_DISP0_DAT17__IPU1_DISP0_DATA17  0x38
+			MX6QDL_PAD_DISP0_DAT18__IPU1_DISP0_DATA18  0x38
+			MX6QDL_PAD_DISP0_DAT19__IPU1_DISP0_DATA19  0x38
+			MX6QDL_PAD_DISP0_DAT20__IPU1_DISP0_DATA20  0x38
+			MX6QDL_PAD_DISP0_DAT21__IPU1_DISP0_DATA21  0x38
+			MX6QDL_PAD_DISP0_DAT22__IPU1_DISP0_DATA22  0x38
+			MX6QDL_PAD_DISP0_DAT23__IPU1_DISP0_DATA23  0x38
+		>;
+	};
+
 	pinctrl_uart2: uart2grp {
 		fsl,pins = <
 			MX6QDL_PAD_GPIO_7__UART2_TX_DATA 0x1b0b1

@@ -194,6 +305,10 @@
 	};
 };
 
+&ipu1_di0_disp0 {
+	remote-endpoint = <&parallel_display_in>;
+};
+
 &pcie {
 	pcie at 0,0 {
 		reg = <0x000000 0 0 0 0>;
Signature · 1 line
2.10.2
#1 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

Quoted 9 lines · christopher.spinrath at rwth-aachen.de

From: Christopher Spinrath <[email protected]>

Apart from the already enabled Designware HDMI port, the Utilite Pro has a second display pipeline which has the following shape:

IPU1 DI0 --> Parallel display --> tfp410 rgb24 to DVI encoder

                                --> HDMI connector.

Enable support for it.

Quoted 7 lines

In addition, since this pipeline is hardwired to IPU1, sever the link between IPU1 and the SoC-internal Designware HDMI encoder forcing the latter to be connected to IPU2 instead of IPU1. Otherwise, it is not possible to drive both displays at high resolution due to the bandwidth limitations of a single IPU.

Signed-off-by Christopher Spinrath <[email protected]>

@Philipp, can you help review the changes?

Quoted 22 lines

---

Hi all,

the removal of the link between IPU1 and the Designware HDMI encoder is the result of a discussion I had with Philipp Zabel:

https://lists.freedesktop.org/archives/dri-devel/2016-November/125399.html .

Altough it is not possible to connect anything else to IPU1 on the Utilite, this approach has at least one disadvantage: if the resolution is low enough such that a single IPU can handle both displays then muxing both displays to IPU1 would reduce the power consumption.

However, IMHO omitting the link IPU1 <--> DW HDMI is still the preferrable solution since I'm not aware of any OS/driver that is capable of switching IPUs or can handle the bandwidth limitation in a sane way. In particular, Linux is unusable when both displays are supposed to be driven at high resolution and both muxing options for the DW HDMI are available (this is not a userspace issue; the system becomes almost unresponsive as soon as the kernel sets the initial resolution).

Quoted 5 lines

Cheers,
Christopher

P.S.: this patch depends on the tfp410 bridge driver which has recently been merged into drm-next.

v4.10-rc1 has the driver, so the dependency is gone now, I guess.

Quoted 43 lines
 arch/arm/boot/dts/imx6q-utilite-pro.dts | 115 ++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +27 -0
index 2200994..69bdd82 100644
@@ -59,6 +59,33 @@
 		rtc1 = &snvs_rtc;
 	};
 
+	encoder {
+		compatible = "ti,tfp410";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+
+				tfp410_in: endpoint {
+					remote-endpoint = <&parallel_display_out>;
+				};
+			};
+
+			port at 1 {
+				reg = <1>;
+
+				tfp410_out: endpoint {
+					remote-endpoint = <&hdmi_connector_in>;
+				};
+			};
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";

arch/arm/boot/dts/imx6q-utilite-pro.dts line 104

Quoted 6 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +3 -0
@@ -72,6 +99,19 @@
 		};
 	};
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+

The newline is unnecessary.

Quoted 15 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +10 -0
+		type = "a";
+		ddc-i2c-bus = <&i2c_dvi_ddc>;
+
+		port {
+			hdmi_connector_in: endpoint {
+				remote-endpoint = <&tfp410_out>;
+			};
+		};
+	};
+
 	i2cmux {
 		compatible = "i2c-mux-gpio";
 		pinctrl-names = "default";

@@ -105,8 +145,46 @@
 			#size-cells = <0>;

arch/arm/boot/dts/imx6q-utilite-pro.dts line 155

Quoted 10 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +8 -0
 		};
 	};
+
+	parallel-display {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu1>;
+

Ditto

I can fix them up if I get a Reviewed-by tag from Philipp on this version.

Shawn

Quoted 89 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +67 -0
+		interface-pix-fmt = "rgb24";
+
+		port at 0 {
+			reg = <0>;
+
+			parallel_display_in: endpoint {
+				remote-endpoint = <&ipu1_di0_disp0>;
+			};
+		};
+
+		port at 1 {
+			reg = <1>;
+
+			parallel_display_out: endpoint {
+				remote-endpoint = <&tfp410_in>;
+			};
+		};
+	};
 };
 
+/*
+ * A single IPU is not able to drive both display interfaces available on the
+ * Utilite Pro at high resolution due to its bandwidth limitation. Since the
+ * tfp410 encoder is wired up to IPU1, sever the link between IPU1 and the
+ * SoC-internal Designware HDMI encoder forcing the latter to be connected to
+ * IPU2 instead of IPU1.
+ */
+/delete-node/&ipu1_di0_hdmi;
+/delete-node/&hdmi_mux_0;
+/delete-node/&ipu1_di1_hdmi;
+/delete-node/&hdmi_mux_1;
+
 &hdmi {
 	ddc-i2c-bus = <&i2c2>;
 	status = "okay";

@@ -151,6 +229,39 @@
 		>;
 	};
 
+	pinctrl_ipu1: ipu1grp {
+		fsl,pins = <
+			MX6QDL_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK 0x38
+			MX6QDL_PAD_DI0_PIN15__IPU1_DI0_PIN15       0x38
+			MX6QDL_PAD_DI0_PIN2__IPU1_DI0_PIN02        0x38
+			MX6QDL_PAD_DI0_PIN3__IPU1_DI0_PIN03        0x38
+			MX6QDL_PAD_DISP0_DAT0__IPU1_DISP0_DATA00   0x38
+			MX6QDL_PAD_DISP0_DAT1__IPU1_DISP0_DATA01   0x38
+			MX6QDL_PAD_DISP0_DAT2__IPU1_DISP0_DATA02   0x38
+			MX6QDL_PAD_DISP0_DAT3__IPU1_DISP0_DATA03   0x38
+			MX6QDL_PAD_DISP0_DAT4__IPU1_DISP0_DATA04   0x38
+			MX6QDL_PAD_DISP0_DAT5__IPU1_DISP0_DATA05   0x38
+			MX6QDL_PAD_DISP0_DAT6__IPU1_DISP0_DATA06   0x38
+			MX6QDL_PAD_DISP0_DAT7__IPU1_DISP0_DATA07   0x38
+			MX6QDL_PAD_DISP0_DAT8__IPU1_DISP0_DATA08   0x38
+			MX6QDL_PAD_DISP0_DAT9__IPU1_DISP0_DATA09   0x38
+			MX6QDL_PAD_DISP0_DAT10__IPU1_DISP0_DATA10  0x38
+			MX6QDL_PAD_DISP0_DAT11__IPU1_DISP0_DATA11  0x38
+			MX6QDL_PAD_DISP0_DAT12__IPU1_DISP0_DATA12  0x38
+			MX6QDL_PAD_DISP0_DAT13__IPU1_DISP0_DATA13  0x38
+			MX6QDL_PAD_DISP0_DAT14__IPU1_DISP0_DATA14  0x38
+			MX6QDL_PAD_DISP0_DAT15__IPU1_DISP0_DATA15  0x38
+			MX6QDL_PAD_DISP0_DAT16__IPU1_DISP0_DATA16  0x38
+			MX6QDL_PAD_DISP0_DAT17__IPU1_DISP0_DATA17  0x38
+			MX6QDL_PAD_DISP0_DAT18__IPU1_DISP0_DATA18  0x38
+			MX6QDL_PAD_DISP0_DAT19__IPU1_DISP0_DATA19  0x38
+			MX6QDL_PAD_DISP0_DAT20__IPU1_DISP0_DATA20  0x38
+			MX6QDL_PAD_DISP0_DAT21__IPU1_DISP0_DATA21  0x38
+			MX6QDL_PAD_DISP0_DAT22__IPU1_DISP0_DATA22  0x38
+			MX6QDL_PAD_DISP0_DAT23__IPU1_DISP0_DATA23  0x38
+		>;
+	};
+
 	pinctrl_uart2: uart2grp {
 		fsl,pins = <
 			MX6QDL_PAD_GPIO_7__UART2_TX_DATA 0x1b0b1

@@ -194,6 +305,10 @@
 	};
 };
 
+&ipu1_di0_disp0 {
+	remote-endpoint = <&parallel_display_in>;
+};
+
 &pcie {
 	pcie at 0,0 {
 		reg = <0x000000 0 0 0 0>;
Signature · 1 line
2.10.2

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#3 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

Hi Shawn,

thanks for the review!

Quoted 42 lines · Shawn Guo
Quoted 9 lines · christopher.spinrath at rwth-aachen.de

From: Christopher Spinrath <[email protected]>

Apart from the already enabled Designware HDMI port, the Utilite Pro has a second display pipeline which has the following shape:

IPU1 DI0 --> Parallel display --> tfp410 rgb24 to DVI encoder

                                --> HDMI connector.

Enable support for it.

Quoted 7 lines

In addition, since this pipeline is hardwired to IPU1, sever the link between IPU1 and the SoC-internal Designware HDMI encoder forcing the latter to be connected to IPU2 instead of IPU1. Otherwise, it is not possible to drive both displays at high resolution due to the bandwidth limitations of a single IPU.

Signed-off-by Christopher Spinrath <[email protected]>

@Philipp, can you help review the changes?

Quoted 22 lines

---

Hi all,

the removal of the link between IPU1 and the Designware HDMI encoder is the result of a discussion I had with Philipp Zabel:

https://lists.freedesktop.org/archives/dri-devel/2016-November/125399.html .

Altough it is not possible to connect anything else to IPU1 on the Utilite, this approach has at least one disadvantage: if the resolution is low enough such that a single IPU can handle both displays then muxing both displays to IPU1 would reduce the power consumption.

However, IMHO omitting the link IPU1 <--> DW HDMI is still the preferrable solution since I'm not aware of any OS/driver that is capable of switching IPUs or can handle the bandwidth limitation in a sane way. In particular, Linux is unusable when both displays are supposed to be driven at high resolution and both muxing options for the DW HDMI are available (this is not a userspace issue; the system becomes almost unresponsive as soon as the kernel sets the initial resolution).

Quoted 7 lines
Quoted 5 lines

Cheers,
Christopher

P.S.: this patch depends on the tfp410 bridge driver which has recently been merged into drm-next.

v4.10-rc1 has the driver, so the dependency is gone now, I guess.

That's correct.

Quoted 80 lines
Quoted 43 lines
 arch/arm/boot/dts/imx6q-utilite-pro.dts | 115 ++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +27 -0
index 2200994..69bdd82 100644
@@ -59,6 +59,33 @@
 		rtc1 = &snvs_rtc;
 	};
 
+	encoder {
+		compatible = "ti,tfp410";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+
+				tfp410_in: endpoint {
+					remote-endpoint = <&parallel_display_out>;
+				};
+			};
+
+			port at 1 {
+				reg = <1>;
+
+				tfp410_out: endpoint {
+					remote-endpoint = <&hdmi_connector_in>;
+				};
+			};
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";

arch/arm/boot/dts/imx6q-utilite-pro.dts line 104

Quoted 6 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +3 -0
@@ -72,6 +99,19 @@
 		};
 	};
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+

The newline is unnecessary.

Quoted 15 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +10 -0
+		type = "a";
+		ddc-i2c-bus = <&i2c_dvi_ddc>;
+
+		port {
+			hdmi_connector_in: endpoint {
+				remote-endpoint = <&tfp410_out>;
+			};
+		};
+	};
+
 	i2cmux {
 		compatible = "i2c-mux-gpio";
 		pinctrl-names = "default";

@@ -105,8 +145,46 @@
 			#size-cells = <0>;

arch/arm/boot/dts/imx6q-utilite-pro.dts line 155

Quoted 10 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +8 -0
 		};
 	};
+
+	parallel-display {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu1>;
+

Ditto

Quoted 2 lines

I can fix them up if I get a Reviewed-by tag from Philipp on this version.

Thanks. I'll bear that in mind for future patches (and, of course, I will fix them up myself if I have to send a v2).

Cheers,
Christopher

Quoted 91 lines

Shawn

Quoted 89 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +67 -1
+		interface-pix-fmt = "rgb24";
+
+		port at 0 {
+			reg = <0>;
+
+			parallel_display_in: endpoint {
+				remote-endpoint = <&ipu1_di0_disp0>;
+			};
+		};
+
+		port at 1 {
+			reg = <1>;
+
+			parallel_display_out: endpoint {
+				remote-endpoint = <&tfp410_in>;
+			};
+		};
+	};
 };
 
+/*
+ * A single IPU is not able to drive both display interfaces available on the
+ * Utilite Pro at high resolution due to its bandwidth limitation. Since the
+ * tfp410 encoder is wired up to IPU1, sever the link between IPU1 and the
+ * SoC-internal Designware HDMI encoder forcing the latter to be connected to
+ * IPU2 instead of IPU1.
+ */
+/delete-node/&ipu1_di0_hdmi;
+/delete-node/&hdmi_mux_0;
+/delete-node/&ipu1_di1_hdmi;
+/delete-node/&hdmi_mux_1;
+
 &hdmi {
 	ddc-i2c-bus = <&i2c2>;
 	status = "okay";

@@ -151,6 +229,39 @@
 		>;
 	};
 
+	pinctrl_ipu1: ipu1grp {
+		fsl,pins = <
+			MX6QDL_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK 0x38
+			MX6QDL_PAD_DI0_PIN15__IPU1_DI0_PIN15       0x38
+			MX6QDL_PAD_DI0_PIN2__IPU1_DI0_PIN02        0x38
+			MX6QDL_PAD_DI0_PIN3__IPU1_DI0_PIN03        0x38
+			MX6QDL_PAD_DISP0_DAT0__IPU1_DISP0_DATA00   0x38
+			MX6QDL_PAD_DISP0_DAT1__IPU1_DISP0_DATA01   0x38
+			MX6QDL_PAD_DISP0_DAT2__IPU1_DISP0_DATA02   0x38
+			MX6QDL_PAD_DISP0_DAT3__IPU1_DISP0_DATA03   0x38
+			MX6QDL_PAD_DISP0_DAT4__IPU1_DISP0_DATA04   0x38
+			MX6QDL_PAD_DISP0_DAT5__IPU1_DISP0_DATA05   0x38
+			MX6QDL_PAD_DISP0_DAT6__IPU1_DISP0_DATA06   0x38
+			MX6QDL_PAD_DISP0_DAT7__IPU1_DISP0_DATA07   0x38
+			MX6QDL_PAD_DISP0_DAT8__IPU1_DISP0_DATA08   0x38
+			MX6QDL_PAD_DISP0_DAT9__IPU1_DISP0_DATA09   0x38
+			MX6QDL_PAD_DISP0_DAT10__IPU1_DISP0_DATA10  0x38
+			MX6QDL_PAD_DISP0_DAT11__IPU1_DISP0_DATA11  0x38
+			MX6QDL_PAD_DISP0_DAT12__IPU1_DISP0_DATA12  0x38
+			MX6QDL_PAD_DISP0_DAT13__IPU1_DISP0_DATA13  0x38
+			MX6QDL_PAD_DISP0_DAT14__IPU1_DISP0_DATA14  0x38
+			MX6QDL_PAD_DISP0_DAT15__IPU1_DISP0_DATA15  0x38
+			MX6QDL_PAD_DISP0_DAT16__IPU1_DISP0_DATA16  0x38
+			MX6QDL_PAD_DISP0_DAT17__IPU1_DISP0_DATA17  0x38
+			MX6QDL_PAD_DISP0_DAT18__IPU1_DISP0_DATA18  0x38
+			MX6QDL_PAD_DISP0_DAT19__IPU1_DISP0_DATA19  0x38
+			MX6QDL_PAD_DISP0_DAT20__IPU1_DISP0_DATA20  0x38
+			MX6QDL_PAD_DISP0_DAT21__IPU1_DISP0_DATA21  0x38
+			MX6QDL_PAD_DISP0_DAT22__IPU1_DISP0_DATA22  0x38
+			MX6QDL_PAD_DISP0_DAT23__IPU1_DISP0_DATA23  0x38
+		>;
+	};
+
 	pinctrl_uart2: uart2grp {
 		fsl,pins = <
 			MX6QDL_PAD_GPIO_7__UART2_TX_DATA 0x1b0b1

@@ -194,6 +305,10 @@
 	};
 };
 
+&ipu1_di0_disp0 {
+	remote-endpoint = <&parallel_display_in>;
+};
+
 &pcie {
 	pcie at 0,0 {
 		reg = <0x000000 0 0 0 0>;
--

2.10.2

author
#4 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

In reply to a message not in the archive

Hi Philipp,

ping? It would be very nice if you comment on this patch before it's too late for v4.11 (which is soon, I think).

Cheers,
Christopher

Quoted 224 lines · Shawn Guo
Quoted 9 lines · christopher.spinrath at rwth-aachen.de

From: Christopher Spinrath <[email protected]>

Apart from the already enabled Designware HDMI port, the Utilite Pro has a second display pipeline which has the following shape:

IPU1 DI0 --> Parallel display --> tfp410 rgb24 to DVI encoder

                                --> HDMI connector.

Enable support for it.

Quoted 7 lines

In addition, since this pipeline is hardwired to IPU1, sever the link between IPU1 and the SoC-internal Designware HDMI encoder forcing the latter to be connected to IPU2 instead of IPU1. Otherwise, it is not possible to drive both displays at high resolution due to the bandwidth limitations of a single IPU.

Signed-off-by Christopher Spinrath <[email protected]>

@Philipp, can you help review the changes?

Quoted 22 lines

---

Hi all,

the removal of the link between IPU1 and the Designware HDMI encoder is the result of a discussion I had with Philipp Zabel:

https://lists.freedesktop.org/archives/dri-devel/2016-November/125399.html .

Altough it is not possible to connect anything else to IPU1 on the Utilite, this approach has at least one disadvantage: if the resolution is low enough such that a single IPU can handle both displays then muxing both displays to IPU1 would reduce the power consumption.

However, IMHO omitting the link IPU1 <--> DW HDMI is still the preferrable solution since I'm not aware of any OS/driver that is capable of switching IPUs or can handle the bandwidth limitation in a sane way. In particular, Linux is unusable when both displays are supposed to be driven at high resolution and both muxing options for the DW HDMI are available (this is not a userspace issue; the system becomes almost unresponsive as soon as the kernel sets the initial resolution).

Quoted 5 lines

Cheers,
Christopher

P.S.: this patch depends on the tfp410 bridge driver which has recently been merged into drm-next.

v4.10-rc1 has the driver, so the dependency is gone now, I guess.

Quoted 43 lines
 arch/arm/boot/dts/imx6q-utilite-pro.dts | 115 ++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +27 -0
index 2200994..69bdd82 100644
@@ -59,6 +59,33 @@
 		rtc1 = &snvs_rtc;
 	};
 
+	encoder {
+		compatible = "ti,tfp410";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+
+				tfp410_in: endpoint {
+					remote-endpoint = <&parallel_display_out>;
+				};
+			};
+
+			port at 1 {
+				reg = <1>;
+
+				tfp410_out: endpoint {
+					remote-endpoint = <&hdmi_connector_in>;
+				};
+			};
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";

arch/arm/boot/dts/imx6q-utilite-pro.dts line 104

Quoted 6 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +3 -0
@@ -72,6 +99,19 @@
 		};
 	};
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+

The newline is unnecessary.

Quoted 15 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +10 -0
+		type = "a";
+		ddc-i2c-bus = <&i2c_dvi_ddc>;
+
+		port {
+			hdmi_connector_in: endpoint {
+				remote-endpoint = <&tfp410_out>;
+			};
+		};
+	};
+
 	i2cmux {
 		compatible = "i2c-mux-gpio";
 		pinctrl-names = "default";

@@ -105,8 +145,46 @@
 			#size-cells = <0>;

arch/arm/boot/dts/imx6q-utilite-pro.dts line 155

Quoted 10 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +8 -0
 		};
 	};
+
+	parallel-display {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu1>;
+

Ditto

I can fix them up if I get a Reviewed-by tag from Philipp on this version.

Shawn

Quoted 89 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +67 -1
+		interface-pix-fmt = "rgb24";
+
+		port at 0 {
+			reg = <0>;
+
+			parallel_display_in: endpoint {
+				remote-endpoint = <&ipu1_di0_disp0>;
+			};
+		};
+
+		port at 1 {
+			reg = <1>;
+
+			parallel_display_out: endpoint {
+				remote-endpoint = <&tfp410_in>;
+			};
+		};
+	};
 };
 
+/*
+ * A single IPU is not able to drive both display interfaces available on the
+ * Utilite Pro at high resolution due to its bandwidth limitation. Since the
+ * tfp410 encoder is wired up to IPU1, sever the link between IPU1 and the
+ * SoC-internal Designware HDMI encoder forcing the latter to be connected to
+ * IPU2 instead of IPU1.
+ */
+/delete-node/&ipu1_di0_hdmi;
+/delete-node/&hdmi_mux_0;
+/delete-node/&ipu1_di1_hdmi;
+/delete-node/&hdmi_mux_1;
+
 &hdmi {
 	ddc-i2c-bus = <&i2c2>;
 	status = "okay";

@@ -151,6 +229,39 @@
 		>;
 	};
 
+	pinctrl_ipu1: ipu1grp {
+		fsl,pins = <
+			MX6QDL_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK 0x38
+			MX6QDL_PAD_DI0_PIN15__IPU1_DI0_PIN15       0x38
+			MX6QDL_PAD_DI0_PIN2__IPU1_DI0_PIN02        0x38
+			MX6QDL_PAD_DI0_PIN3__IPU1_DI0_PIN03        0x38
+			MX6QDL_PAD_DISP0_DAT0__IPU1_DISP0_DATA00   0x38
+			MX6QDL_PAD_DISP0_DAT1__IPU1_DISP0_DATA01   0x38
+			MX6QDL_PAD_DISP0_DAT2__IPU1_DISP0_DATA02   0x38
+			MX6QDL_PAD_DISP0_DAT3__IPU1_DISP0_DATA03   0x38
+			MX6QDL_PAD_DISP0_DAT4__IPU1_DISP0_DATA04   0x38
+			MX6QDL_PAD_DISP0_DAT5__IPU1_DISP0_DATA05   0x38
+			MX6QDL_PAD_DISP0_DAT6__IPU1_DISP0_DATA06   0x38
+			MX6QDL_PAD_DISP0_DAT7__IPU1_DISP0_DATA07   0x38
+			MX6QDL_PAD_DISP0_DAT8__IPU1_DISP0_DATA08   0x38
+			MX6QDL_PAD_DISP0_DAT9__IPU1_DISP0_DATA09   0x38
+			MX6QDL_PAD_DISP0_DAT10__IPU1_DISP0_DATA10  0x38
+			MX6QDL_PAD_DISP0_DAT11__IPU1_DISP0_DATA11  0x38
+			MX6QDL_PAD_DISP0_DAT12__IPU1_DISP0_DATA12  0x38
+			MX6QDL_PAD_DISP0_DAT13__IPU1_DISP0_DATA13  0x38
+			MX6QDL_PAD_DISP0_DAT14__IPU1_DISP0_DATA14  0x38
+			MX6QDL_PAD_DISP0_DAT15__IPU1_DISP0_DATA15  0x38
+			MX6QDL_PAD_DISP0_DAT16__IPU1_DISP0_DATA16  0x38
+			MX6QDL_PAD_DISP0_DAT17__IPU1_DISP0_DATA17  0x38
+			MX6QDL_PAD_DISP0_DAT18__IPU1_DISP0_DATA18  0x38
+			MX6QDL_PAD_DISP0_DAT19__IPU1_DISP0_DATA19  0x38
+			MX6QDL_PAD_DISP0_DAT20__IPU1_DISP0_DATA20  0x38
+			MX6QDL_PAD_DISP0_DAT21__IPU1_DISP0_DATA21  0x38
+			MX6QDL_PAD_DISP0_DAT22__IPU1_DISP0_DATA22  0x38
+			MX6QDL_PAD_DISP0_DAT23__IPU1_DISP0_DATA23  0x38
+		>;
+	};
+
 	pinctrl_uart2: uart2grp {
 		fsl,pins = <
 			MX6QDL_PAD_GPIO_7__UART2_TX_DATA 0x1b0b1

@@ -194,6 +305,10 @@
 	};
 };
 
+&ipu1_di0_disp0 {
+	remote-endpoint = <&parallel_display_in>;
+};
+
 &pcie {
 	pcie at 0,0 {
 		reg = <0x000000 0 0 0 0>;
--

2.10.2

#5 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

In reply to christopher.spinrath at rwth-aachen.de

Hi Christopher,

Quoted 109 lines · christopher.spinrath at rwth-aachen.de

From: Christopher Spinrath <[email protected]>

Apart from the already enabled Designware HDMI port, the Utilite Pro has a second display pipeline which has the following shape:

IPU1 DI0 --> Parallel display --> tfp410 rgb24 to DVI encoder

                                --> HDMI connector.

Enable support for it.

In addition, since this pipeline is hardwired to IPU1, sever the link between IPU1 and the SoC-internal Designware HDMI encoder forcing the latter to be connected to IPU2 instead of IPU1. Otherwise, it is not possible to drive both displays at high resolution due to the bandwidth limitations of a single IPU.

Signed-off-by Christopher Spinrath <[email protected]>

---

Hi all,

the removal of the link between IPU1 and the Designware HDMI encoder is the result of a discussion I had with Philipp Zabel:

https://lists.freedesktop.org/archives/dri-devel/2016-November/125399.html .

Altough it is not possible to connect anything else to IPU1 on the Utilite, this approach has at least one disadvantage: if the resolution is low enough such that a single IPU can handle both displays then muxing both displays to IPU1 would reduce the power consumption.

However, IMHO omitting the link IPU1 <--> DW HDMI is still the preferrable solution since I'm not aware of any OS/driver that is capable of switching IPUs or can handle the bandwidth limitation in a sane way. In particular, Linux is unusable when both displays are supposed to be driven at high resolution and both muxing options for the DW HDMI are available (this is not a userspace issue; the system becomes almost unresponsive as soon as the kernel sets the initial resolution).

Cheers,
Christopher

P.S.: this patch depends on the tfp410 bridge driver which has recently been merged into drm-next.

 arch/arm/boot/dts/imx6q-utilite-pro.dts | 115 ++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +40 -0
index 2200994..69bdd82 100644
@@ -59,6 +59,33 @@
 		rtc1 = &snvs_rtc;
 	};
 
+	encoder {
+		compatible = "ti,tfp410";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+
+				tfp410_in: endpoint {
+					remote-endpoint = <&parallel_display_out>;
+				};
+			};
+
+			port at 1 {
+				reg = <1>;
+
+				tfp410_out: endpoint {
+					remote-endpoint = <&hdmi_connector_in>;
+				};
+			};
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";

@@ -72,6 +99,19 @@
 		};
 	};
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+
+		type = "a";
+		ddc-i2c-bus = <&i2c_dvi_ddc>;
+
+		port {
+			hdmi_connector_in: endpoint {
+				remote-endpoint = <&tfp410_out>;
+			};
+		};
+	};
+
 	i2cmux {
 		compatible = "i2c-mux-gpio";
 		pinctrl-names = "default";

@@ -105,8 +145,46 @@
 			#size-cells = <0>;
 		};

arch/arm/boot/dts/imx6q-utilite-pro.dts line 156

Quoted 10 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +9 -0
 	};
+
+	parallel-display {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu1>;
+
+		interface-pix-fmt = "rgb24";

This is not necessary if the connector created by the tpf410 has the correct media bus format set in its display_info structure. This can be done in tfp410_attach, before calling drm_mode_connector_attach_encoder:

        u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;

	drm_display_info_set_bus_formats(&dvi->connector.display_info,                                                                         
					 &bus_format, 1);

After this is done, the above line should be removed in a follow-up patch.

Quoted 74 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +61 -0
+		port at 0 {
+			reg = <0>;
+
+			parallel_display_in: endpoint {
+				remote-endpoint = <&ipu1_di0_disp0>;
+			};
+		};
+
+		port at 1 {
+			reg = <1>;
+
+			parallel_display_out: endpoint {
+				remote-endpoint = <&tfp410_in>;
+			};
+		};
+	};
 };
 
+/*
+ * A single IPU is not able to drive both display interfaces available on the
+ * Utilite Pro at high resolution due to its bandwidth limitation. Since the
+ * tfp410 encoder is wired up to IPU1, sever the link between IPU1 and the
+ * SoC-internal Designware HDMI encoder forcing the latter to be connected to
+ * IPU2 instead of IPU1.
+ */
+/delete-node/&ipu1_di0_hdmi;
+/delete-node/&hdmi_mux_0;
+/delete-node/&ipu1_di1_hdmi;
+/delete-node/&hdmi_mux_1;
+
 &hdmi {
 	ddc-i2c-bus = <&i2c2>;
 	status = "okay";

@@ -151,6 +229,39 @@
 		>;
 	};
 
+	pinctrl_ipu1: ipu1grp {
+		fsl,pins = <
+			MX6QDL_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK 0x38
+			MX6QDL_PAD_DI0_PIN15__IPU1_DI0_PIN15       0x38
+			MX6QDL_PAD_DI0_PIN2__IPU1_DI0_PIN02        0x38
+			MX6QDL_PAD_DI0_PIN3__IPU1_DI0_PIN03        0x38
+			MX6QDL_PAD_DISP0_DAT0__IPU1_DISP0_DATA00   0x38
+			MX6QDL_PAD_DISP0_DAT1__IPU1_DISP0_DATA01   0x38
+			MX6QDL_PAD_DISP0_DAT2__IPU1_DISP0_DATA02   0x38
+			MX6QDL_PAD_DISP0_DAT3__IPU1_DISP0_DATA03   0x38
+			MX6QDL_PAD_DISP0_DAT4__IPU1_DISP0_DATA04   0x38
+			MX6QDL_PAD_DISP0_DAT5__IPU1_DISP0_DATA05   0x38
+			MX6QDL_PAD_DISP0_DAT6__IPU1_DISP0_DATA06   0x38
+			MX6QDL_PAD_DISP0_DAT7__IPU1_DISP0_DATA07   0x38
+			MX6QDL_PAD_DISP0_DAT8__IPU1_DISP0_DATA08   0x38
+			MX6QDL_PAD_DISP0_DAT9__IPU1_DISP0_DATA09   0x38
+			MX6QDL_PAD_DISP0_DAT10__IPU1_DISP0_DATA10  0x38
+			MX6QDL_PAD_DISP0_DAT11__IPU1_DISP0_DATA11  0x38
+			MX6QDL_PAD_DISP0_DAT12__IPU1_DISP0_DATA12  0x38
+			MX6QDL_PAD_DISP0_DAT13__IPU1_DISP0_DATA13  0x38
+			MX6QDL_PAD_DISP0_DAT14__IPU1_DISP0_DATA14  0x38
+			MX6QDL_PAD_DISP0_DAT15__IPU1_DISP0_DATA15  0x38
+			MX6QDL_PAD_DISP0_DAT16__IPU1_DISP0_DATA16  0x38
+			MX6QDL_PAD_DISP0_DAT17__IPU1_DISP0_DATA17  0x38
+			MX6QDL_PAD_DISP0_DAT18__IPU1_DISP0_DATA18  0x38
+			MX6QDL_PAD_DISP0_DAT19__IPU1_DISP0_DATA19  0x38
+			MX6QDL_PAD_DISP0_DAT20__IPU1_DISP0_DATA20  0x38
+			MX6QDL_PAD_DISP0_DAT21__IPU1_DISP0_DATA21  0x38
+			MX6QDL_PAD_DISP0_DAT22__IPU1_DISP0_DATA22  0x38
+			MX6QDL_PAD_DISP0_DAT23__IPU1_DISP0_DATA23  0x38
+		>;
+	};
+
 	pinctrl_uart2: uart2grp {
 		fsl,pins = <
 			MX6QDL_PAD_GPIO_7__UART2_TX_DATA 0x1b0b1

arch/arm/boot/dts/imx6q-utilite-pro.dts line 314

Quoted 10 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +4 -0
@@ -194,6 +305,10 @@
 	};
 };
 
+&ipu1_di0_disp0 {
+	remote-endpoint = <&parallel_display_in>;
+};
+
 &pcie {
 	pcie at 0,0 {
 		reg = <0x000000 0 0 0 0>;
Reviewed-by Philipp Zabel <[email protected]>

regards
Philipp

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#7 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

Hi Philipp,

thanks for the review!

Quoted 15 lines · Philipp Zabel

[...]

arch/arm/boot/dts/imx6q-utilite-pro.dts line 156

Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +9 -0
+
+	parallel-display {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu1>;
+
+		interface-pix-fmt = "rgb24";

This is not necessary if the connector created by the tpf410 has the correct media bus format set in its display_info structure. This can be done in tfp410_attach, before calling drm_mode_connector_attach_encoder:

Quoted 7 lines
        u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;

	drm_display_info_set_bus_formats(&dvi->connector.display_info,
					 &bus_format, 1);

After this is done, the above line should be removed in a follow-up patch.

Ok, I will send a mini follow-up series doing that with your Suggested-by (unless you object) in the next few days.

Cheers,
Christopher

Quoted 18 lines
Quoted 18 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +16 -0
+		port at 0 {
+			reg = <0>;
+
+			parallel_display_in: endpoint {
+				remote-endpoint = <&ipu1_di0_disp0>;
+			};
+		};
+
+		port at 1 {
+			reg = <1>;
+
+			parallel_display_out: endpoint {
+				remote-endpoint = <&tfp410_in>;
+			};
+		};
+	};
 };
[...]

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#9 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

Hi Philipp,

turns out I have a question on your comment after all:

Quoted 27 lines · Christopher Spinrath

Hi Philipp,

thanks for the review!

Quoted 22 lines · Philipp Zabel

[...]

arch/arm/boot/dts/imx6q-utilite-pro.dts line 156

Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +9 -0
+
+    parallel-display {
+        compatible = "fsl,imx-parallel-display";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_ipu1>;
+
+        interface-pix-fmt = "rgb24";

This is not necessary if the connector created by the tpf410 has the correct media bus format set in its display_info structure. This can be done in tfp410_attach, before calling drm_mode_connector_attach_encoder:

        u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;

    drm_display_info_set_bus_formats(&dvi->connector.display_info,
                     &bus_format, 1);

After this is done, the above line should be removed in a follow-up patch.

On closer inspection the tfp410 can handle rgb12, rgb24, and DVI formats. Considering this it feels wrong to hardcode the bus format to rgb24 (isn't it?).

So a solution might be to add a property specifying the bus format to the tfp410 binding. But then we would effectively just move this property from one node to another. I wonder if this is still desireable...?

Cheers,
Christopher

Quoted 24 lines

Ok, I will send a mini follow-up series doing that with your Suggested-by (unless you object) in the next few days.

Cheers,
Christopher

Quoted 18 lines
Quoted 18 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +16 -0
+        port at 0 {
+            reg = <0>;
+
+            parallel_display_in: endpoint {
+                remote-endpoint = <&ipu1_di0_disp0>;
+            };
+        };
+
+        port at 1 {
+            reg = <1>;
+
+            parallel_display_out: endpoint {
+                remote-endpoint = <&tfp410_in>;
+            };
+        };
+    };
 };
[...]
#10 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

Hi Christopher,

Quoted 30 lines · Christopher Spinrath

Hi Philipp,

turns out I have a question on your comment after all:

Quoted 25 lines · Christopher Spinrath

Hi Philipp,

thanks for the review!

Quoted 20 lines · Philipp Zabel

[...]

arch/arm/boot/dts/imx6q-utilite-pro.dts line 156

Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +9 -0
+
+    parallel-display {
+        compatible = "fsl,imx-parallel-display";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_ipu1>;
+
+        interface-pix-fmt = "rgb24";

This is not necessary if the connector created by the tpf410 has the correct media bus format set in its display_info structure. This can be done in tfp410_attach, before calling drm_mode_connector_attach_encoder:

        u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;

    drm_display_info_set_bus_formats(&dvi->connector.display_info,
                     &bus_format, 1);
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

After this is done, the above line should be removed in a follow-up patch.

On closer inspection the tfp410 can handle rgb12, rgb24, and DVI formats. Considering this it feels wrong to hardcode the bus format to rgb24 (isn't it?).

That is a good point, I agree. I have some thoughts on this:

Quoted 3 lines

So a solution might be to add a property specifying the bus format to the tfp410 binding. But then we would effectively just move this property from one node to another. I wonder if this is still desireable...?

If this is configurable on both ends, the necessary setting is neither a hardware property of the bridge, nor of the display interface, but rather one of the board specific wiring between the two (if at all).

Ideally all possible settings should be known to the drivers and the best format should be negotiated. Note that display_info.bus_formats already is an array of possible bus formats, even though the parallel display driver currently only looks at the first element.

If there is no limitation imposed by the wiring, presenting best format first in that array seems reasonable for now.

Otherwise I think the links should describe the parallel bus layout, which is specified for the (input) media video interfaces:

Documentation/devicetree/bindings/media/video-interfaces.txt

Using the

	bus-width = <24>;
or
	bus-width = <12>;

property the tfp510 driver could choose which bus_formats to add to the display_info.

regards
Philipp

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#12 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

Hi Philipp,

sorry for the late answer; I'm quite busy with other stuff these days.

Quoted 50 lines · Philipp Zabel

Hi Christopher,

Quoted 30 lines · Christopher Spinrath

Hi Philipp,

turns out I have a question on your comment after all:

Quoted 25 lines · Christopher Spinrath

Hi Philipp,

thanks for the review!

Quoted 20 lines · Philipp Zabel

[...]

arch/arm/boot/dts/imx6q-utilite-pro.dts line 156

Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +9 -0
+
+    parallel-display {
+        compatible = "fsl,imx-parallel-display";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_ipu1>;
+
+        interface-pix-fmt = "rgb24";

This is not necessary if the connector created by the tpf410 has the correct media bus format set in its display_info structure. This can be done in tfp410_attach, before calling drm_mode_connector_attach_encoder:

        u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;

    drm_display_info_set_bus_formats(&dvi->connector.display_info,
                     &bus_format, 1);
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

After this is done, the above line should be removed in a follow-up patch.

On closer inspection the tfp410 can handle rgb12, rgb24, and DVI formats. Considering this it feels wrong to hardcode the bus format to rgb24 (isn't it?).

That is a good point, I agree. I have some thoughts on this:

Quoted 3 lines

So a solution might be to add a property specifying the bus format to the tfp410 binding. But then we would effectively just move this property from one node to another. I wonder if this is still desireable...?

If this is configurable on both ends, the necessary setting is neither a hardware property of the bridge, nor of the display interface, but rather one of the board specific wiring between the two (if at all).

Quoted 7 lines

Ideally all possible settings should be known to the drivers and the best format should be negotiated. Note that display_info.bus_formats already is an array of possible bus formats, even though the parallel display driver currently only looks at the first element.

If there is no limitation imposed by the wiring, presenting best format first in that array seems reasonable for now.

The tfp410 can be operated either in i2c mode or in dump mode (i2c is not wired up). The Linux driver does only support dump mode so far. (In essence the driver does nothing but the neccesary boilerplate stuff.) While the format can be read and changed neither is possible in the dumb mode. The format is configured by connecting some pins of the chip to ground. So we have to deal with the "otherwise" case below.

Quoted 11 lines

Otherwise I think the links should describe the parallel bus layout, which is specified for the (input) media video interfaces:

Documentation/devicetree/bindings/media/video-interfaces.txt

Using the

	bus-width = <24>;
or
	bus-width = <12>;

property the tfp510 driver could choose which bus_formats to add to the display_info.

I see your point that the format is a property of the wiring and, thus, it is better to utilizie the bus-width property in the of graph remote nodes from a modelling point of view.

On the other hand, the parallel-display already has some data on the
wiring: the pinctrl settings. This weakens the above reasoning because the information is splitted up. Moreover, a quick grep session revealed that on all arm platforms the format is obtained from the "SoC-side" (read parallel-display) node, if it is dynamic. Is there some effort to change that? (essentially, I'm trying to understand here the justification for the change we are discussing).

Cheers,
Christopher

Quoted 3 lines

regards
Philipp

Applied
#13 lore raw

PATCH 1/1 ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline

In reply to christopher.spinrath at rwth-aachen.de
Quoted 9 lines · christopher.spinrath at rwth-aachen.de

From: Christopher Spinrath <[email protected]>

Apart from the already enabled Designware HDMI port, the Utilite Pro has a second display pipeline which has the following shape:

IPU1 DI0 --> Parallel display --> tfp410 rgb24 to DVI encoder

                                --> HDMI connector.

Enable support for it.

Quoted 7 lines

In addition, since this pipeline is hardwired to IPU1, sever the link between IPU1 and the SoC-internal Designware HDMI encoder forcing the latter to be connected to IPU2 instead of IPU1. Otherwise, it is not possible to drive both displays at high resolution due to the bandwidth limitations of a single IPU.

Signed-off-by Christopher Spinrath <[email protected]>

Applied, thanks.