MXSFB error: -ENODEV: Cannot connect bridge

  Hello all,

while doing some tests with kernel v6.8-rc3 and Colibri iMX7D, we
noticed the following error:

[    0.432547] mxsfb 30730000.lcdif: error -ENODEV: Cannot connect bridge

This was introduced by commit edbbae7fba495284f72f05768696572691231558
("ARM: dts: imx7: add MIPI-DSI support"). This patch is routing the
lcdif to the mipi_dsi_in_lcdif endpoint, however we do not have the DSI
pins available in our edge connector. Instead, we use the parallel RGB
LCD interface directly with, as example, an external LVDS transmitter:

&lcdif {
..
	status = "disabled";

	port {
		lcdif_out: endpoint {
			remote-endpoint = <&lcd_panel_in>;
		};
	};
};

By applying the following patch, the issue is gone and the LVDS works
again:

I would like to know your opinion about this patch before sending it,
does it makes sense for you? I understand that routing to endpoint
should be done in the SoM device tree, so we are free to rout other
endpoint without issues.

Regards,
Hiago.
  

On Thu, Feb 08, 2024 at 12:58:02PM -0300, Hiago De Franco wrote:
> Hello all,
> 
> while doing some tests with kernel v6.8-rc3 and Colibri iMX7D, we
> noticed the following error:
> 
> [    0.432547] mxsfb 30730000.lcdif: error -ENODEV: Cannot connect bridge
> 
> This was introduced by commit edbbae7fba495284f72f05768696572691231558
> ("ARM: dts: imx7: add MIPI-DSI support"). This patch is routing the
> lcdif to the mipi_dsi_in_lcdif endpoint, however we do not have the DSI
> pins available in our edge connector. Instead, we use the parallel RGB
> LCD interface directly with, as example, an external LVDS transmitter:
> 
> &lcdif {
> ...
> 	status = "disabled";
> 
> 	port {
> 		lcdif_out: endpoint {
> 			remote-endpoint = <&lcd_panel_in>;
> 		};
> 	};
> };
> 
> By applying the following patch, the issue is gone and the LVDS works
> again:
> 
> diff --git a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> index ebf7befcc11e..9c81c6baa2d3 100644
> --- a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> +++ b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> @@ -834,16 +834,6 @@ lcdif: lcdif@30730000 {
>  					<&clks IMX7D_LCDIF_PIXEL_ROOT_CLK>;
>  				clock-names = "pix", "axi";
>  				status = "disabled";
> -
> -				port {
> -					#address-cells = <1>;
> -					#size-cells = <0>;
> -
> -					lcdif_out_mipi_dsi: endpoint@0 {
> -						reg = <0>;
> -						remote-endpoint = <&mipi_dsi_in_lcdif>;
> -					};
> -				};
>  			};
>  
>  			mipi_csi: mipi-csi@30750000 {
> @@ -895,22 +885,6 @@ mipi_dsi: dsi@30760000 {
>  				samsung,esc-clock-frequency = <20000000>;
>  				samsung,pll-clock-frequency = <24000000>;
>  				status = "disabled";
> -
> -				ports {
> -					#address-cells = <1>;
> -					#size-cells = <0>;
> -
> -					port@0 {
> -						reg = <0>;
> -						#address-cells = <1>;
> -						#size-cells = <0>;
> -
> -						mipi_dsi_in_lcdif: endpoint@0 {
> -							reg = <0>;
> -							remote-endpoint = <&lcdif_out_mipi_dsi>;
> -						};
> -					};
> -				};
>  			};
>  		};
> 
> I would like to know your opinion about this patch before sending it,
> does it makes sense for you? I understand that routing to endpoint
> should be done in the SoM device tree, so we are free to rout other
> endpoint without issues.

As far as I understood, the LCDIF -> DSI connection is always present in
the SoC. Can you overwrite the routing in your dts like this:?

    &lcdif_out_mipi_dsi {
        remote-endpoint = <&lcd_panel_in>;
    };

I'm not sure what is the best default solution here for imx7s.dtsi. Also
the labels don't work out in that case, this could be improved.

Regards,
 
 - Roland
  

+ Linux regression.

This is a regression on v6.8-rc1.

On Mon, Feb 12, 2024 at 12:07:06PM +0100, Roland Hieber wrote:
> On Thu, Feb 08, 2024 at 12:58:02PM -0300, Hiago De Franco wrote:
> > Hello all,
> > 
> > while doing some tests with kernel v6.8-rc3 and Colibri iMX7D, we
> > noticed the following error:
> > 
> > [    0.432547] mxsfb 30730000.lcdif: error -ENODEV: Cannot connect bridge
> > 
> > This was introduced by commit edbbae7fba495284f72f05768696572691231558
> > ("ARM: dts: imx7: add MIPI-DSI support"). This patch is routing the
> > lcdif to the mipi_dsi_in_lcdif endpoint, however we do not have the DSI
> > pins available in our edge connector. Instead, we use the parallel RGB
> > LCD interface directly with, as example, an external LVDS transmitter:
> > 
> > &lcdif {
> > ...
> > 	status = "disabled";
> > 
> > 	port {
> > 		lcdif_out: endpoint {
> > 			remote-endpoint = <&lcd_panel_in>;
> > 		};
> > 	};
> > };
> > 
> > By applying the following patch, the issue is gone and the LVDS works
> > again:
> > 
> > diff --git a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> > index ebf7befcc11e..9c81c6baa2d3 100644
> > --- a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> > @@ -834,16 +834,6 @@ lcdif: lcdif@30730000 {
> >  					<&clks IMX7D_LCDIF_PIXEL_ROOT_CLK>;
> >  				clock-names = "pix", "axi";
> >  				status = "disabled";
> > -
> > -				port {
> > -					#address-cells = <1>;
> > -					#size-cells = <0>;
> > -
> > -					lcdif_out_mipi_dsi: endpoint@0 {
> > -						reg = <0>;
> > -						remote-endpoint = <&mipi_dsi_in_lcdif>;
> > -					};
> > -				};
> >  			};
> >  
> >  			mipi_csi: mipi-csi@30750000 {
> > @@ -895,22 +885,6 @@ mipi_dsi: dsi@30760000 {
> >  				samsung,esc-clock-frequency = <20000000>;
> >  				samsung,pll-clock-frequency = <24000000>;
> >  				status = "disabled";
> > -
> > -				ports {
> > -					#address-cells = <1>;
> > -					#size-cells = <0>;
> > -
> > -					port@0 {
> > -						reg = <0>;
> > -						#address-cells = <1>;
> > -						#size-cells = <0>;
> > -
> > -						mipi_dsi_in_lcdif: endpoint@0 {
> > -							reg = <0>;
> > -							remote-endpoint = <&lcdif_out_mipi_dsi>;
> > -						};
> > -					};
> > -				};
> >  			};
> >  		};
> > 
> > I would like to know your opinion about this patch before sending it,
> > does it makes sense for you? I understand that routing to endpoint
> > should be done in the SoM device tree, so we are free to rout other
> > endpoint without issues.
> 
> As far as I understood, the LCDIF -> DSI connection is always present in
> the SoC. Can you overwrite the routing in your dts like this:?
> 
>     &lcdif_out_mipi_dsi {
>         remote-endpoint = <&lcd_panel_in>;
>     };
> 
> I'm not sure what is the best default solution here for imx7s.dtsi. Also
> the labels don't work out in that case, this could be improved.

For sure for something to be defined a solution it should not introduce
regressions :-)

This commit makes other boards not work anymore, specifically this
reports is about colibri-imx7.

With that said, the following patch solves the issue

diff --git a/arch/arm/boot/dts/nxp/imx/imx7-colibri.dtsi b/arch/arm/boot/dts/nxp/imx/imx7-colibri.dtsi
index 9fe51884af79..966ad13e7c78 100644
--- a/arch/arm/boot/dts/nxp/imx/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx7-colibri.dtsi
@@ -536,12 +536,15 @@ &lcdif {
        status = "disabled";

        port {
-               lcdif_out: endpoint {
+               lcdif_out: endpoint@0 {
+                       reg = <0>;
                        remote-endpoint = <&lcd_panel_in>;
                };
        };
 };

+/delete-node/ &mipi_dsi_in_lcdif;
+
 /* Colibri PWM<A> */
 &pwm1 {
        pinctrl-names = "default";


However, ... I do not really like the delete node, but it's required to
prevent this error:

arch/arm/boot/dts/nxp/imx/imx7s.dtsi:908.37-911.9: Warning (graph_endpoint): /soc/bus@30400000/dsi@30760000/ports/port@0/endpoint@0: graph connection to node '/soc/bus@30400000/lcdif@30730000/port/endpoint@0' is not bidirectional

With that said, unless you have a better solution I would just send a
revert for your change.

Francesco