[PATCH 0/5] drm: lcdif: Use drm_bridge_connector

[PATCH 0/5] drm: lcdif: Use drm_bridge_connector

[Liu Ying]

Hi,

This patch series aims to use drm_bridge_connector in the i.MX8MP LCDIF
driver so that bridge drivers don't need to initialize DRM connectors.

Patch 1-3 add HDMI connectors to some i.MX8MP platforms's DT as preparation
work.  The Synopsys DW HDMI bridge core driver would try to find the bridge
of the HDMI connector after the LCDIF driver starts to use
drm_bridge_connector.

Patch 4 sets output_port to 1 in i.MX8MP HDMI TX driver, as a preparation
work too.  The Synopsys DW HDMI bridge core driver needs to know the output
port index so that the driver can use the port index to find and attach
the next bridge.  The next bridge attachment is needed after the LCDIF driver
starts to use drm_bridge_connector.

Patch 5 makes the LCDIF driver use drm_bridge_connector.

With this patch set, an in-flight ITE IT6263 bridge driver[1] doesn't need
to initialize a DRM connector.

[1] https://patchwork.freedesktop.org/patch/619465/?series=139266&rev=2

Liu Ying (5):
  arm64: dts: imx8mp-kontron-bl-osm-s: Add HDMI connector
  arm64: dts: imx8mp-kontron-smarc-eval-carrier: Add HDMI connector
  arm64: dts: imx8mp-msc-sm2s-ep1: Add HDMI connector
  drm/bridge: imx8mp-hdmi-tx: Set output_port to 1
  drm: lcdif: Use drm_bridge_connector

 .../dts/freescale/imx8mp-kontron-bl-osm-s.dts | 19 +++++++++++++++++++
 .../imx8mp-kontron-smarc-eval-carrier.dts     | 19 +++++++++++++++++++
 .../dts/freescale/imx8mp-msc-sm2s-ep1.dts     | 19 +++++++++++++++++++
 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c   |  1 +
 drivers/gpu/drm/mxsfb/Kconfig                 |  1 +
 drivers/gpu/drm/mxsfb/lcdif_drv.c             | 17 ++++++++++++++++-
 6 files changed, 75 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/5] arm64: dts: imx8mp-kontron-bl-osm-s: Add HDMI connector

[Liu Ying]

Add a HDMI connector to connect with i.MX8MP HDMI TX output.
This is a preparation for making the i.MX8MP LCDIF driver use
drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
flag.  With that flag, the DW HDMI bridge core driver would
try to attach the next bridge which is the HDMI connector.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 .../dts/freescale/imx8mp-kontron-bl-osm-s.dts | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts b/arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
index 0eb9e726a9b8..445bf5a46c6a 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
@@ -23,6 +23,17 @@ extcon_usbc: usbc {
 		id-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
 	};
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+		type = "a";
+
+		port {
+			hdmi_in: endpoint {
+				remote-endpoint = <&hdmi_tx_out>;
+			};
+		};
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
@@ -168,6 +179,14 @@ &hdmi_tx {
 	pinctrl-0 = <&pinctrl_hdmi>;
 	ddc-i2c-bus = <&i2c2>;
 	status = "okay";
+
+	ports {
+		port@1 {
+			hdmi_tx_out: endpoint {
+				remote-endpoint = <&hdmi_in>;
+			};
+		};
+	};
 };
 
 &hdmi_tx_phy {
-- 
2.34.1

[PATCH 2/5] arm64: dts: imx8mp-kontron-smarc-eval-carrier: Add HDMI connector

[Liu Ying]

Add a HDMI connector to connect with i.MX8MP HDMI TX output.
This is a preparation for making the i.MX8MP LCDIF driver use
drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
flag.  With that flag, the DW HDMI bridge core driver would
try to attach the next bridge which is the HDMI connector.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 .../imx8mp-kontron-smarc-eval-carrier.dts     | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts b/arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
index 2173a36ff691..815f313a2d33 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
@@ -29,6 +29,17 @@ extcon_usbc: usbc {
 		id-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
 	};
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+		type = "a";
+
+		port {
+			hdmi_in: endpoint {
+				remote-endpoint = <&hdmi_tx_out>;
+			};
+		};
+	};
+
 	sound {
 		compatible = "simple-audio-card";
 		simple-audio-card,bitclock-master = <&codec_dai>;
@@ -108,6 +119,14 @@ &hdmi_tx {
 	pinctrl-0 = <&pinctrl_hdmi>;
 	ddc-i2c-bus = <&i2c3>;
 	status = "okay";
+
+	ports {
+		port@1 {
+			hdmi_tx_out: endpoint {
+				remote-endpoint = <&hdmi_in>;
+			};
+		};
+	};
 };
 
 &hdmi_tx_phy {
-- 
2.34.1

[PATCH 3/5] arm64: dts: imx8mp-msc-sm2s-ep1: Add HDMI connector

[Liu Ying]

Add a HDMI connector to connect with i.MX8MP HDMI TX output.
This is a preparation for making the i.MX8MP LCDIF driver use
drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
flag.  With that flag, the DW HDMI bridge core driver would
try to attach the next bridge which is the HDMI connector.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 .../dts/freescale/imx8mp-msc-sm2s-ep1.dts     | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
index 83194ea7cb81..b776646a258a 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
@@ -15,6 +15,17 @@ / {
 		     "avnet,sm2s-imx8mp-14N0600E", "avnet,sm2s-imx8mp",
 		     "fsl,imx8mp";
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+		type = "a";
+
+		port {
+			hdmi_in: endpoint {
+				remote-endpoint = <&hdmi_tx_out>;
+			};
+		};
+	};
+
 	reg_vcc_3v3_audio: 3v3-audio-regulator {
 		compatible = "regulator-fixed";
 		regulator-name = "VCC_3V3_AUD";
@@ -54,6 +65,14 @@ &hdmi_tx {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_hdmi>;
 	status = "okay";
+
+	ports {
+		port@1 {
+			hdmi_tx_out: endpoint {
+				remote-endpoint = <&hdmi_in>;
+			};
+		};
+	};
 };
 
 &hdmi_tx_phy {
-- 
2.34.1

[PATCH 4/5] drm/bridge: imx8mp-hdmi-tx: Set output_port to 1

[Liu Ying]

Set DW HDMI platform data's output_port to 1 in imx8mp_dw_hdmi_probe()
so that dw_hdmi_probe() called by imx8mp_dw_hdmi_probe() can tell the
DW HDMI bridge core driver about the output port we are using, hence
the next bridge can be found in dw_hdmi_parse_dt() according to the port
index, and furthermore the next bridge can be attached to bridge chain in
dw_hdmi_bridge_attach() when the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is
set.  The output_port value aligns to the value used by devicetree.
This is a preparation for making the i.MX8MP LCDIF driver use
drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
flag.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
index 8fcc6d18f4ab..54a53f96929a 100644
--- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
+++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
@@ -96,6 +96,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(hdmi->pixclk),
 				     "Unable to get pixel clock\n");
 
+	plat_data->output_port = 1;
 	plat_data->mode_valid = imx8mp_hdmi_mode_valid;
 	plat_data->phy_ops = &imx8mp_hdmi_phy_ops;
 	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
-- 
2.34.1

[PATCH 5/5] drm: lcdif: Use drm_bridge_connector

[Liu Ying]

Initialize a connector by calling drm_bridge_connector_init() for
each encoder so that down stream bridge drivers don't need to create
connectors any more.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 drivers/gpu/drm/mxsfb/Kconfig     |  1 +
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
index 264e74f45554..06c95e556380 100644
--- a/drivers/gpu/drm/mxsfb/Kconfig
+++ b/drivers/gpu/drm/mxsfb/Kconfig
@@ -27,6 +27,7 @@ config DRM_IMX_LCDIF
 	depends on DRM && OF
 	depends on COMMON_CLK
 	depends on ARCH_MXC || COMPILE_TEST
+	select DRM_BRIDGE_CONNECTOR
 	select DRM_CLIENT_SELECTION
 	select DRM_MXS
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 58ccad9c425d..d4521da6675e 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -16,7 +16,9 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_client_setup.h>
+#include <drm/drm_connector.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_fbdev_dma.h>
@@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 		struct device_node *remote;
 		struct of_endpoint of_ep;
 		struct drm_encoder *encoder;
+		struct drm_connector *connector;
 
 		remote = of_graph_get_remote_port_parent(ep);
 		if (!of_device_is_available(remote)) {
@@ -97,13 +100,25 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 			return ret;
 		}
 
-		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
+		ret = drm_bridge_attach(encoder, bridge, NULL,
+					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 		if (ret) {
 			of_node_put(ep);
 			return dev_err_probe(dev, ret,
 					     "Failed to attach bridge for endpoint%u\n",
 					     of_ep.id);
 		}
+
+		connector = drm_bridge_connector_init(lcdif->drm, encoder);
+		if (IS_ERR(connector)) {
+			ret = PTR_ERR(connector);
+			dev_err(dev, "Failed to initialize bridge connector: %d\n",
+				ret);
+			of_node_put(ep);
+			return ret;
+		}
+
+		drm_connector_attach_encoder(connector, encoder);
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH 4/5] drm/bridge: imx8mp-hdmi-tx: Set output_port to 1

[Alexander Stein]

Hi,

Am Freitag, 18. Oktober 2024, 08:48:12 CEST schrieb Liu Ying:
> Set DW HDMI platform data's output_port to 1 in imx8mp_dw_hdmi_probe()
> so that dw_hdmi_probe() called by imx8mp_dw_hdmi_probe() can tell the
> DW HDMI bridge core driver about the output port we are using, hence
> the next bridge can be found in dw_hdmi_parse_dt() according to the port
> index, and furthermore the next bridge can be attached to bridge chain in
> dw_hdmi_bridge_attach() when the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is
> set.  The output_port value aligns to the value used by devicetree.
> This is a preparation for making the i.MX8MP LCDIF driver use
> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
> flag.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>

Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> ---
>  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> index 8fcc6d18f4ab..54a53f96929a 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> @@ -96,6 +96,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(hdmi->pixclk),
>  				     "Unable to get pixel clock\n");
>  
> +	plat_data->output_port = 1;
>  	plat_data->mode_valid = imx8mp_hdmi_mode_valid;
>  	plat_data->phy_ops = &imx8mp_hdmi_phy_ops;
>  	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector

[Biju Das]

Hi Liu Ying,

Thanks for the patch.

> -----Original Message-----
> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Liu Ying
> Sent: Friday, October 18, 2024 7:48 AM
> Subject: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector
> 
> Initialize a connector by calling drm_bridge_connector_init() for each encoder so that down stream
> bridge drivers don't need to create connectors any more.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig index
> 264e74f45554..06c95e556380 100644
> --- a/drivers/gpu/drm/mxsfb/Kconfig
> +++ b/drivers/gpu/drm/mxsfb/Kconfig
> @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF
>  	depends on DRM && OF
>  	depends on COMMON_CLK
>  	depends on ARCH_MXC || COMPILE_TEST
> +	select DRM_BRIDGE_CONNECTOR
>  	select DRM_CLIENT_SELECTION
>  	select DRM_MXS
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index 58ccad9c425d..d4521da6675e 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -16,7 +16,9 @@
> 
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_client_setup.h>
> +#include <drm/drm_connector.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_fbdev_dma.h>
> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  		struct device_node *remote;
>  		struct of_endpoint of_ep;
>  		struct drm_encoder *encoder;
> +		struct drm_connector *connector;
> 
>  		remote = of_graph_get_remote_port_parent(ep);
>  		if (!of_device_is_available(remote)) { @@ -97,13 +100,25 @@ static int
> lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  			return ret;
>  		}
> 
> -		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
> +		ret = drm_bridge_attach(encoder, bridge, NULL,
> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>  		if (ret) {
>  			of_node_put(ep);
>  			return dev_err_probe(dev, ret,
>  					     "Failed to attach bridge for endpoint%u\n",
>  					     of_ep.id);
>  		}
> +
> +		connector = drm_bridge_connector_init(lcdif->drm, encoder);
> +		if (IS_ERR(connector)) {
> +			ret = PTR_ERR(connector);
> +			dev_err(dev, "Failed to initialize bridge connector: %d\n",
> +				ret);
> +			of_node_put(ep);
> +			return ret;

Maybe same error path style like above??
		of_node_put(ep);
		return dev_err_probe(dev, PTR_ERR(connector),"Failed to initialize bridge connector");

Cheers,
Biju

> +		}
> +
> +		drm_connector_attach_encoder(connector, encoder);
>  	}
> 
>  	return 0;
> --
> 2.34.1
> 

Re: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector

[Alexander Stein]

Hi,

Am Freitag, 18. Oktober 2024, 08:48:13 CEST schrieb Liu Ying:
> Initialize a connector by calling drm_bridge_connector_init() for
> each encoder so that down stream bridge drivers don't need to create
> connectors any more.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>

I've tested this using HDMI output. HPD and CEC is still working.
Also output using a DSI->DP bridge (tc358767) also works.
My platform: imx8mp-tqma8mpql-mba8mpxl.dts

Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
> index 264e74f45554..06c95e556380 100644
> --- a/drivers/gpu/drm/mxsfb/Kconfig
> +++ b/drivers/gpu/drm/mxsfb/Kconfig
> @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF
>  	depends on DRM && OF
>  	depends on COMMON_CLK
>  	depends on ARCH_MXC || COMPILE_TEST
> +	select DRM_BRIDGE_CONNECTOR
>  	select DRM_CLIENT_SELECTION
>  	select DRM_MXS
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index 58ccad9c425d..d4521da6675e 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -16,7 +16,9 @@
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_client_setup.h>
> +#include <drm/drm_connector.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_fbdev_dma.h>
> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  		struct device_node *remote;
>  		struct of_endpoint of_ep;
>  		struct drm_encoder *encoder;
> +		struct drm_connector *connector;
>  
>  		remote = of_graph_get_remote_port_parent(ep);
>  		if (!of_device_is_available(remote)) {
> @@ -97,13 +100,25 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  			return ret;
>  		}
>  
> -		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
> +		ret = drm_bridge_attach(encoder, bridge, NULL,
> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>  		if (ret) {
>  			of_node_put(ep);
>  			return dev_err_probe(dev, ret,
>  					     "Failed to attach bridge for endpoint%u\n",
>  					     of_ep.id);
>  		}
> +
> +		connector = drm_bridge_connector_init(lcdif->drm, encoder);
> +		if (IS_ERR(connector)) {
> +			ret = PTR_ERR(connector);
> +			dev_err(dev, "Failed to initialize bridge connector: %d\n",
> +				ret);
> +			of_node_put(ep);
> +			return ret;
> +		}
> +
> +		drm_connector_attach_encoder(connector, encoder);
>  	}
>  
>  	return 0;
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector

[Liu Ying]

On 10/18/2024, Biju Das wrote:
> Hi Liu Ying,

Hi Biju,

> 
> Thanks for the patch.

Thanks for your review.

> 
>> -----Original Message-----
>> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Liu Ying
>> Sent: Friday, October 18, 2024 7:48 AM
>> Subject: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector
>>
>> Initialize a connector by calling drm_bridge_connector_init() for each encoder so that down stream
>> bridge drivers don't need to create connectors any more.
>>
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
>>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++-
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig index
>> 264e74f45554..06c95e556380 100644
>> --- a/drivers/gpu/drm/mxsfb/Kconfig
>> +++ b/drivers/gpu/drm/mxsfb/Kconfig
>> @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF
>>  	depends on DRM && OF
>>  	depends on COMMON_CLK
>>  	depends on ARCH_MXC || COMPILE_TEST
>> +	select DRM_BRIDGE_CONNECTOR
>>  	select DRM_CLIENT_SELECTION
>>  	select DRM_MXS
>>  	select DRM_KMS_HELPER
>> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> index 58ccad9c425d..d4521da6675e 100644
>> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> @@ -16,7 +16,9 @@
>>
>>  #include <drm/drm_atomic_helper.h>
>>  #include <drm/drm_bridge.h>
>> +#include <drm/drm_bridge_connector.h>
>>  #include <drm/drm_client_setup.h>
>> +#include <drm/drm_connector.h>
>>  #include <drm/drm_drv.h>
>>  #include <drm/drm_encoder.h>
>>  #include <drm/drm_fbdev_dma.h>
>> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>>  		struct device_node *remote;
>>  		struct of_endpoint of_ep;
>>  		struct drm_encoder *encoder;
>> +		struct drm_connector *connector;
>>
>>  		remote = of_graph_get_remote_port_parent(ep);
>>  		if (!of_device_is_available(remote)) { @@ -97,13 +100,25 @@ static int
>> lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>>  			return ret;
>>  		}
>>
>> -		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
>> +		ret = drm_bridge_attach(encoder, bridge, NULL,
>> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>>  		if (ret) {
>>  			of_node_put(ep);
>>  			return dev_err_probe(dev, ret,
>>  					     "Failed to attach bridge for endpoint%u\n",
>>  					     of_ep.id);
>>  		}
>> +
>> +		connector = drm_bridge_connector_init(lcdif->drm, encoder);
>> +		if (IS_ERR(connector)) {
>> +			ret = PTR_ERR(connector);
>> +			dev_err(dev, "Failed to initialize bridge connector: %d\n",
>> +				ret);
>> +			of_node_put(ep);
>> +			return ret;
> 
> Maybe same error path style like above??
> 		of_node_put(ep);
> 		return dev_err_probe(dev, PTR_ERR(connector),"Failed to initialize bridge connector");

I thought about that and decided not to use dev_err_probe()
because I don't think drm_bridge_connector_init() may return
-EPROBE_DEFER, no?

> 
> Cheers,
> Biju
> 
>> +		}
>> +
>> +		drm_connector_attach_encoder(connector, encoder);
>>  	}
>>
>>  	return 0;
>> --
>> 2.34.1
>>
> 

-- 
Regards,
Liu Ying

Re: [PATCH 1/5] arm64: dts: imx8mp-kontron-bl-osm-s: Add HDMI connector

[Alexander Stein]

Am Freitag, 18. Oktober 2024, 08:48:09 CEST schrieb Liu Ying:
> Add a HDMI connector to connect with i.MX8MP HDMI TX output.
> This is a preparation for making the i.MX8MP LCDIF driver use
> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
> flag.  With that flag, the DW HDMI bridge core driver would
> try to attach the next bridge which is the HDMI connector.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>

Looks similar to imx8mp-tqma8mpql-mba8mpxl.dts, so:
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> ---
>  .../dts/freescale/imx8mp-kontron-bl-osm-s.dts | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts b/arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
> index 0eb9e726a9b8..445bf5a46c6a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
> @@ -23,6 +23,17 @@ extcon_usbc: usbc {
>  		id-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
>  	};
>  
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_in: endpoint {
> +				remote-endpoint = <&hdmi_tx_out>;
> +			};
> +		};
> +	};
> +
>  	leds {
>  		compatible = "gpio-leds";
>  
> @@ -168,6 +179,14 @@ &hdmi_tx {
>  	pinctrl-0 = <&pinctrl_hdmi>;
>  	ddc-i2c-bus = <&i2c2>;
>  	status = "okay";
> +
> +	ports {
> +		port@1 {
> +			hdmi_tx_out: endpoint {
> +				remote-endpoint = <&hdmi_in>;
> +			};
> +		};
> +	};
>  };
>  
>  &hdmi_tx_phy {
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 2/5] arm64: dts: imx8mp-kontron-smarc-eval-carrier: Add HDMI connector

[Alexander Stein]

Am Freitag, 18. Oktober 2024, 08:48:10 CEST schrieb Liu Ying:
> Add a HDMI connector to connect with i.MX8MP HDMI TX output.
> This is a preparation for making the i.MX8MP LCDIF driver use
> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
> flag.  With that flag, the DW HDMI bridge core driver would
> try to attach the next bridge which is the HDMI connector.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>

Looks similar to imx8mp-tqma8mpql-mba8mpxl.dts, so:
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> ---
>  .../imx8mp-kontron-smarc-eval-carrier.dts     | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts b/arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
> index 2173a36ff691..815f313a2d33 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
> @@ -29,6 +29,17 @@ extcon_usbc: usbc {
>  		id-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
>  	};
>  
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_in: endpoint {
> +				remote-endpoint = <&hdmi_tx_out>;
> +			};
> +		};
> +	};
> +
>  	sound {
>  		compatible = "simple-audio-card";
>  		simple-audio-card,bitclock-master = <&codec_dai>;
> @@ -108,6 +119,14 @@ &hdmi_tx {
>  	pinctrl-0 = <&pinctrl_hdmi>;
>  	ddc-i2c-bus = <&i2c3>;
>  	status = "okay";
> +
> +	ports {
> +		port@1 {
> +			hdmi_tx_out: endpoint {
> +				remote-endpoint = <&hdmi_in>;
> +			};
> +		};
> +	};
>  };
>  
>  &hdmi_tx_phy {
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 3/5] arm64: dts: imx8mp-msc-sm2s-ep1: Add HDMI connector

[Alexander Stein]

Am Freitag, 18. Oktober 2024, 08:48:11 CEST schrieb Liu Ying:
> Add a HDMI connector to connect with i.MX8MP HDMI TX output.
> This is a preparation for making the i.MX8MP LCDIF driver use
> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
> flag.  With that flag, the DW HDMI bridge core driver would
> try to attach the next bridge which is the HDMI connector.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>

Looks similar to imx8mp-tqma8mpql-mba8mpxl.dts, so:
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> ---
>  .../dts/freescale/imx8mp-msc-sm2s-ep1.dts     | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> index 83194ea7cb81..b776646a258a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> @@ -15,6 +15,17 @@ / {
>  		     "avnet,sm2s-imx8mp-14N0600E", "avnet,sm2s-imx8mp",
>  		     "fsl,imx8mp";
>  
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_in: endpoint {
> +				remote-endpoint = <&hdmi_tx_out>;
> +			};
> +		};
> +	};
> +
>  	reg_vcc_3v3_audio: 3v3-audio-regulator {
>  		compatible = "regulator-fixed";
>  		regulator-name = "VCC_3V3_AUD";
> @@ -54,6 +65,14 @@ &hdmi_tx {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_hdmi>;
>  	status = "okay";
> +
> +	ports {
> +		port@1 {
> +			hdmi_tx_out: endpoint {
> +				remote-endpoint = <&hdmi_in>;
> +			};
> +		};
> +	};
>  };
>  
>  &hdmi_tx_phy {
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector

[Biju Das]

Hi Liu Ying,

> -----Original Message-----
> From: Liu Ying <victor.liu@nxp.com>
> Sent: Friday, October 18, 2024 8:38 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>; devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dri-devel@lists.freedesktop.org
> Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com; andrzej.hajda@intel.com;
> neil.armstrong@linaro.org; rfoss@kernel.org; laurent.pinchart <laurent.pinchart@ideasonboard.com>;
> jonas@kwiboo.se; jernej.skrabec@gmail.com; maarten.lankhorst@linux.intel.com; mripard@kernel.org;
> tzimmermann@suse.de; airlied@gmail.com; simona@ffwll.ch; marex@denx.de; stefan@agner.ch;
> dmitry.baryshkov@linaro.org
> Subject: Re: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector
> 
> On 10/18/2024, Biju Das wrote:
> > Hi Liu Ying,
> 
> Hi Biju,
> 
> >
> > Thanks for the patch.
> 
> Thanks for your review.
> 
> >
> >> -----Original Message-----
> >> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org>
> >> On Behalf Of Liu Ying
> >> Sent: Friday, October 18, 2024 7:48 AM
> >> Subject: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector
> >>
> >> Initialize a connector by calling drm_bridge_connector_init() for
> >> each encoder so that down stream bridge drivers don't need to create connectors any more.
> >>
> >> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> >> ---
> >>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
> >>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++-
> >>  2 files changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/mxsfb/Kconfig
> >> b/drivers/gpu/drm/mxsfb/Kconfig index
> >> 264e74f45554..06c95e556380 100644
> >> --- a/drivers/gpu/drm/mxsfb/Kconfig
> >> +++ b/drivers/gpu/drm/mxsfb/Kconfig
> >> @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF
> >>  	depends on DRM && OF
> >>  	depends on COMMON_CLK
> >>  	depends on ARCH_MXC || COMPILE_TEST
> >> +	select DRM_BRIDGE_CONNECTOR
> >>  	select DRM_CLIENT_SELECTION
> >>  	select DRM_MXS
> >>  	select DRM_KMS_HELPER
> >> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> >> b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> >> index 58ccad9c425d..d4521da6675e 100644
> >> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> >> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> >> @@ -16,7 +16,9 @@
> >>
> >>  #include <drm/drm_atomic_helper.h>
> >>  #include <drm/drm_bridge.h>
> >> +#include <drm/drm_bridge_connector.h>
> >>  #include <drm/drm_client_setup.h>
> >> +#include <drm/drm_connector.h>
> >>  #include <drm/drm_drv.h>
> >>  #include <drm/drm_encoder.h>
> >>  #include <drm/drm_fbdev_dma.h>
> >> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
> >>  		struct device_node *remote;
> >>  		struct of_endpoint of_ep;
> >>  		struct drm_encoder *encoder;
> >> +		struct drm_connector *connector;
> >>
> >>  		remote = of_graph_get_remote_port_parent(ep);
> >>  		if (!of_device_is_available(remote)) { @@ -97,13 +100,25 @@ static
> >> int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
> >>  			return ret;
> >>  		}
> >>
> >> -		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
> >> +		ret = drm_bridge_attach(encoder, bridge, NULL,
> >> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> >>  		if (ret) {
> >>  			of_node_put(ep);
> >>  			return dev_err_probe(dev, ret,
> >>  					     "Failed to attach bridge for endpoint%u\n",
> >>  					     of_ep.id);
> >>  		}
> >> +
> >> +		connector = drm_bridge_connector_init(lcdif->drm, encoder);
> >> +		if (IS_ERR(connector)) {
> >> +			ret = PTR_ERR(connector);
> >> +			dev_err(dev, "Failed to initialize bridge connector: %d\n",
> >> +				ret);
> >> +			of_node_put(ep);
> >> +			return ret;
> >
> > Maybe same error path style like above??
> > 		of_node_put(ep);
> > 		return dev_err_probe(dev, PTR_ERR(connector),"Failed to initialize
> > bridge connector");
> 
> I thought about that and decided not to use dev_err_probe() because I don't think
> drm_bridge_connector_init() may return -EPROBE_DEFER, no?

Yes, you are correct.

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Cheers,
Biju

> 
> >
> > Cheers,
> > Biju
> >
> >> +		}
> >> +
> >> +		drm_connector_attach_encoder(connector, encoder);
> >>  	}
> >>
> >>  	return 0;
> >> --
> >> 2.34.1
> >>
> >
> 
> --
> Regards,
> Liu Ying

Re: [PATCH 3/5] arm64: dts: imx8mp-msc-sm2s-ep1: Add HDMI connector

[Marek Vasut]

On 10/18/24 8:48 AM, Liu Ying wrote:
> Add a HDMI connector to connect with i.MX8MP HDMI TX output.
> This is a preparation for making the i.MX8MP LCDIF driver use
> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
> flag.  With that flag, the DW HDMI bridge core driver would
> try to attach the next bridge which is the HDMI connector.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>   .../dts/freescale/imx8mp-msc-sm2s-ep1.dts     | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> index 83194ea7cb81..b776646a258a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> @@ -15,6 +15,17 @@ / {
>   		     "avnet,sm2s-imx8mp-14N0600E", "avnet,sm2s-imx8mp",
>   		     "fsl,imx8mp";
>   
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
Shouldn't this also have a 'label' property ?

Re: [PATCH 3/5] arm64: dts: imx8mp-msc-sm2s-ep1: Add HDMI connector

[Liu Ying]

On 10/18/2024, Marek Vasut wrote:
> On 10/18/24 8:48 AM, Liu Ying wrote:
>> Add a HDMI connector to connect with i.MX8MP HDMI TX output.
>> This is a preparation for making the i.MX8MP LCDIF driver use
>> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
>> flag.  With that flag, the DW HDMI bridge core driver would
>> try to attach the next bridge which is the HDMI connector.
>>
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>>   .../dts/freescale/imx8mp-msc-sm2s-ep1.dts     | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>> index 83194ea7cb81..b776646a258a 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>> @@ -15,6 +15,17 @@ / {
>>                "avnet,sm2s-imx8mp-14N0600E", "avnet,sm2s-imx8mp",
>>                "fsl,imx8mp";
>>   +    hdmi-connector {
>> +        compatible = "hdmi-connector";
>> +        type = "a";
> Shouldn't this also have a 'label' property ?

'label' property is not required by hdmi-connector.yaml and there
are in-tree hdmi-connector nodes that haven't got it.
I tried to find schematics for the board online, but failed.
I don't have the board to see the label printed in silk layer, either.

If anyone can provide a valid label name, I may add it.

-- 
Regards,
Liu Ying

Re: [PATCH 3/5] arm64: dts: imx8mp-msc-sm2s-ep1: Add HDMI connector

[Marek Vasut]

On 10/18/24 11:00 AM, Liu Ying wrote:
> On 10/18/2024, Marek Vasut wrote:
>> On 10/18/24 8:48 AM, Liu Ying wrote:
>>> Add a HDMI connector to connect with i.MX8MP HDMI TX output.
>>> This is a preparation for making the i.MX8MP LCDIF driver use
>>> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
>>> flag.  With that flag, the DW HDMI bridge core driver would
>>> try to attach the next bridge which is the HDMI connector.
>>>
>>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>>> ---
>>>    .../dts/freescale/imx8mp-msc-sm2s-ep1.dts     | 19 +++++++++++++++++++
>>>    1 file changed, 19 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>>> index 83194ea7cb81..b776646a258a 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>>> @@ -15,6 +15,17 @@ / {
>>>                 "avnet,sm2s-imx8mp-14N0600E", "avnet,sm2s-imx8mp",
>>>                 "fsl,imx8mp";
>>>    +    hdmi-connector {
>>> +        compatible = "hdmi-connector";
>>> +        type = "a";
>> Shouldn't this also have a 'label' property ?
> 
> 'label' property is not required by hdmi-connector.yaml and there
> are in-tree hdmi-connector nodes that haven't got it.
> I tried to find schematics for the board online, but failed.
> I don't have the board to see the label printed in silk layer, either.
> 
> If anyone can provide a valid label name, I may add it.
For the Kontron board, Frieder might be able to look it up for you ?

For the MSC one, hmmm, I am not sure. Anyone ?

Re: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector

[Dmitry Baryshkov]

On Fri, Oct 18, 2024 at 02:48:13PM +0800, Liu Ying wrote:
> Initialize a connector by calling drm_bridge_connector_init() for
> each encoder so that down stream bridge drivers don't need to create
> connectors any more.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
> index 264e74f45554..06c95e556380 100644
> --- a/drivers/gpu/drm/mxsfb/Kconfig
> +++ b/drivers/gpu/drm/mxsfb/Kconfig
> @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF
>  	depends on DRM && OF
>  	depends on COMMON_CLK
>  	depends on ARCH_MXC || COMPILE_TEST
> +	select DRM_BRIDGE_CONNECTOR
>  	select DRM_CLIENT_SELECTION

Missing `select DRM_DISPLAY_HELPER`. LGTM otherwise.

>  	select DRM_MXS
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index 58ccad9c425d..d4521da6675e 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -16,7 +16,9 @@
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_client_setup.h>
> +#include <drm/drm_connector.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_fbdev_dma.h>
> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  		struct device_node *remote;
>  		struct of_endpoint of_ep;
>  		struct drm_encoder *encoder;
> +		struct drm_connector *connector;
>  
>  		remote = of_graph_get_remote_port_parent(ep);
>  		if (!of_device_is_available(remote)) {
> @@ -97,13 +100,25 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  			return ret;
>  		}
>  
> -		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
> +		ret = drm_bridge_attach(encoder, bridge, NULL,
> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>  		if (ret) {
>  			of_node_put(ep);
>  			return dev_err_probe(dev, ret,
>  					     "Failed to attach bridge for endpoint%u\n",
>  					     of_ep.id);
>  		}
> +
> +		connector = drm_bridge_connector_init(lcdif->drm, encoder);
> +		if (IS_ERR(connector)) {
> +			ret = PTR_ERR(connector);
> +			dev_err(dev, "Failed to initialize bridge connector: %d\n",
> +				ret);
> +			of_node_put(ep);
> +			return ret;
> +		}
> +
> +		drm_connector_attach_encoder(connector, encoder);
>  	}
>  
>  	return 0;
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 4/5] drm/bridge: imx8mp-hdmi-tx: Set output_port to 1

[Dmitry Baryshkov]

On Fri, Oct 18, 2024 at 02:48:12PM +0800, Liu Ying wrote:
> Set DW HDMI platform data's output_port to 1 in imx8mp_dw_hdmi_probe()
> so that dw_hdmi_probe() called by imx8mp_dw_hdmi_probe() can tell the
> DW HDMI bridge core driver about the output port we are using, hence
> the next bridge can be found in dw_hdmi_parse_dt() according to the port
> index, and furthermore the next bridge can be attached to bridge chain in
> dw_hdmi_bridge_attach() when the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is
> set.  The output_port value aligns to the value used by devicetree.
> This is a preparation for making the i.MX8MP LCDIF driver use
> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
> flag.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> index 8fcc6d18f4ab..54a53f96929a 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> @@ -96,6 +96,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(hdmi->pixclk),
>  				     "Unable to get pixel clock\n");
>  
> +	plat_data->output_port = 1;

This will break compatibility with older DT files, which don't have
output port. I think you need to add output_port_optional flag to
dw_hdmi_plat_data and still return 0 from dw_hdmi_parse_dt() if the flag
is set, but there is no remote node.

Last, but not least, this changes behaviour of the connector.
dw_hdmi_connector_create() implements CEC support, handles
ycbcr_420_allowed, HDR metadata, etc.

We are slowly moving towards the supporting all of this in bridge
connector via the HDMI Connector framework, but this is not
implemented for now.

>  	plat_data->mode_valid = imx8mp_hdmi_mode_valid;
>  	plat_data->phy_ops = &imx8mp_hdmi_phy_ops;
>  	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 4/5] drm/bridge: imx8mp-hdmi-tx: Set output_port to 1

[Alexander Stein]

Am Freitag, 18. Oktober 2024, 14:31:20 CEST schrieb Dmitry Baryshkov:
> On Fri, Oct 18, 2024 at 02:48:12PM +0800, Liu Ying wrote:
> > Set DW HDMI platform data's output_port to 1 in imx8mp_dw_hdmi_probe()
> > so that dw_hdmi_probe() called by imx8mp_dw_hdmi_probe() can tell the
> > DW HDMI bridge core driver about the output port we are using, hence
> > the next bridge can be found in dw_hdmi_parse_dt() according to the port
> > index, and furthermore the next bridge can be attached to bridge chain in
> > dw_hdmi_bridge_attach() when the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is
> > set.  The output_port value aligns to the value used by devicetree.
> > This is a preparation for making the i.MX8MP LCDIF driver use
> > drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
> > flag.
> > 
> > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > ---
> >  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > index 8fcc6d18f4ab..54a53f96929a 100644
> > --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > @@ -96,6 +96,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
> >  		return dev_err_probe(dev, PTR_ERR(hdmi->pixclk),
> >  				     "Unable to get pixel clock\n");
> >  
> > +	plat_data->output_port = 1;
> 
> This will break compatibility with older DT files, which don't have
> output port. I think you need to add output_port_optional flag to
> dw_hdmi_plat_data and still return 0 from dw_hdmi_parse_dt() if the flag
> is set, but there is no remote node.
> 
> Last, but not least, this changes behaviour of the connector.
> dw_hdmi_connector_create() implements CEC support, handles
> ycbcr_420_allowed, HDR metadata, etc.

Mh, I was suspecting the same, but I couldn't see any regression regarding CEC.
Maybe the change is not doing what it is supposed to do...
I'll check again on Monday.

Best regards,
Alexander

> We are slowly moving towards the supporting all of this in bridge
> connector via the HDMI Connector framework, but this is not
> implemented for now.
> 
> >  	plat_data->mode_valid = imx8mp_hdmi_mode_valid;
> >  	plat_data->phy_ops = &imx8mp_hdmi_phy_ops;
> >  	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 4/5] drm/bridge: imx8mp-hdmi-tx: Set output_port to 1

[Liu Ying]

On 10/18/2024, Dmitry Baryshkov wrote:
> On Fri, Oct 18, 2024 at 02:48:12PM +0800, Liu Ying wrote:
>> Set DW HDMI platform data's output_port to 1 in imx8mp_dw_hdmi_probe()
>> so that dw_hdmi_probe() called by imx8mp_dw_hdmi_probe() can tell the
>> DW HDMI bridge core driver about the output port we are using, hence
>> the next bridge can be found in dw_hdmi_parse_dt() according to the port
>> index, and furthermore the next bridge can be attached to bridge chain in
>> dw_hdmi_bridge_attach() when the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is
>> set.  The output_port value aligns to the value used by devicetree.
>> This is a preparation for making the i.MX8MP LCDIF driver use
>> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
>> flag.
>>
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>>  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> index 8fcc6d18f4ab..54a53f96929a 100644
>> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> @@ -96,6 +96,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
>>  		return dev_err_probe(dev, PTR_ERR(hdmi->pixclk),
>>  				     "Unable to get pixel clock\n");
>>  
>> +	plat_data->output_port = 1;
> 
> This will break compatibility with older DT files, which don't have
> output port. I think you need to add output_port_optional flag to
> dw_hdmi_plat_data and still return 0 from dw_hdmi_parse_dt() if the flag
> is set, but there is no remote node.
> 
> Last, but not least, this changes behaviour of the connector.
> dw_hdmi_connector_create() implements CEC support, handles
> ycbcr_420_allowed, HDR metadata, etc.
> 
> We are slowly moving towards the supporting all of this in bridge
> connector via the HDMI Connector framework, but this is not
> implemented for now.

Then, it looks like this patch series needs to wait until this is
implemented.

> 
>>  	plat_data->mode_valid = imx8mp_hdmi_mode_valid;
>>  	plat_data->phy_ops = &imx8mp_hdmi_phy_ops;
>>  	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
>> -- 
>> 2.34.1
>>
> 

-- 
Regards,
Liu Ying

Re: [PATCH 5/5] drm: lcdif: Use drm_bridge_connector

[Liu Ying]

On 10/18/2024, Dmitry Baryshkov wrote:
> On Fri, Oct 18, 2024 at 02:48:13PM +0800, Liu Ying wrote:
>> Initialize a connector by calling drm_bridge_connector_init() for
>> each encoder so that down stream bridge drivers don't need to create
>> connectors any more.
>>
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
>>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 17 ++++++++++++++++-
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
>> index 264e74f45554..06c95e556380 100644
>> --- a/drivers/gpu/drm/mxsfb/Kconfig
>> +++ b/drivers/gpu/drm/mxsfb/Kconfig
>> @@ -27,6 +27,7 @@ config DRM_IMX_LCDIF
>>  	depends on DRM && OF
>>  	depends on COMMON_CLK
>>  	depends on ARCH_MXC || COMPILE_TEST
>> +	select DRM_BRIDGE_CONNECTOR
>>  	select DRM_CLIENT_SELECTION
> 
> Missing `select DRM_DISPLAY_HELPER`. LGTM otherwise.

Ack.

> 
>>  	select DRM_MXS
>>  	select DRM_KMS_HELPER
>> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> index 58ccad9c425d..d4521da6675e 100644
>> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> @@ -16,7 +16,9 @@
>>  
>>  #include <drm/drm_atomic_helper.h>
>>  #include <drm/drm_bridge.h>
>> +#include <drm/drm_bridge_connector.h>
>>  #include <drm/drm_client_setup.h>
>> +#include <drm/drm_connector.h>
>>  #include <drm/drm_drv.h>
>>  #include <drm/drm_encoder.h>
>>  #include <drm/drm_fbdev_dma.h>
>> @@ -56,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>>  		struct device_node *remote;
>>  		struct of_endpoint of_ep;
>>  		struct drm_encoder *encoder;
>> +		struct drm_connector *connector;
>>  
>>  		remote = of_graph_get_remote_port_parent(ep);
>>  		if (!of_device_is_available(remote)) {
>> @@ -97,13 +100,25 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>>  			return ret;
>>  		}
>>  
>> -		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
>> +		ret = drm_bridge_attach(encoder, bridge, NULL,
>> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>>  		if (ret) {
>>  			of_node_put(ep);
>>  			return dev_err_probe(dev, ret,
>>  					     "Failed to attach bridge for endpoint%u\n",
>>  					     of_ep.id);
>>  		}
>> +
>> +		connector = drm_bridge_connector_init(lcdif->drm, encoder);
>> +		if (IS_ERR(connector)) {
>> +			ret = PTR_ERR(connector);
>> +			dev_err(dev, "Failed to initialize bridge connector: %d\n",
>> +				ret);
>> +			of_node_put(ep);
>> +			return ret;
>> +		}
>> +
>> +		drm_connector_attach_encoder(connector, encoder);
>>  	}
>>  
>>  	return 0;
>> -- 
>> 2.34.1
>>
> 

-- 
Regards,
Liu Ying

Re: [PATCH 3/5] arm64: dts: imx8mp-msc-sm2s-ep1: Add HDMI connector

[Frieder Schrempf]

On 18.10.24 11:35 AM, Marek Vasut wrote:
> On 10/18/24 11:00 AM, Liu Ying wrote:
>> On 10/18/2024, Marek Vasut wrote:
>>> On 10/18/24 8:48 AM, Liu Ying wrote:
>>>> Add a HDMI connector to connect with i.MX8MP HDMI TX output.
>>>> This is a preparation for making the i.MX8MP LCDIF driver use
>>>> drm_bridge_connector which requires the DRM_BRIDGE_ATTACH_NO_CONNECTOR
>>>> flag.  With that flag, the DW HDMI bridge core driver would
>>>> try to attach the next bridge which is the HDMI connector.
>>>>
>>>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>>>> ---
>>>>    .../dts/freescale/imx8mp-msc-sm2s-ep1.dts     | 19 ++++++++++++++
>>>> +++++
>>>>    1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>>>> b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>>>> index 83194ea7cb81..b776646a258a 100644
>>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>>>> @@ -15,6 +15,17 @@ / {
>>>>                 "avnet,sm2s-imx8mp-14N0600E", "avnet,sm2s-imx8mp",
>>>>                 "fsl,imx8mp";
>>>>    +    hdmi-connector {
>>>> +        compatible = "hdmi-connector";
>>>> +        type = "a";
>>> Shouldn't this also have a 'label' property ?
>>
>> 'label' property is not required by hdmi-connector.yaml and there
>> are in-tree hdmi-connector nodes that haven't got it.
>> I tried to find schematics for the board online, but failed.
>> I don't have the board to see the label printed in silk layer, either.
>>
>> If anyone can provide a valid label name, I may add it.
> For the Kontron board, Frieder might be able to look it up for you ?

The Kontron PCB doesn't have a silkscreen label for the connector. The
schematic has the label "X801" for the part and the signals on the
connector are prefixed with "HDMI_CON".