[PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Marek Vasut]

The dw-hdmi output_port is set to 1 in order to look for a connector
next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR working.
The output_port set to 1 makes the DW HDMI driver core look up the
next bridge in DT, where the next bridge is often the hdmi-connector .

Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR")

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Liu Ying <victor.liu@nxp.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
V2: No change
---
 drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
index 9a480c6abb856..d8e9fbf75edbb 100644
--- a/drivers/gpu/drm/bridge/imx/Kconfig
+++ b/drivers/gpu/drm/bridge/imx/Kconfig
@@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
 config DRM_IMX8MP_HDMI_PVI
 	tristate "Freescale i.MX8MP HDMI PVI bridge support"
 	depends on OF
+	select DRM_DISPLAY_CONNECTOR
 	help
 	  Choose this to enable support for the internal HDMI TX Parallel
 	  Video Interface found on the Freescale i.MX8MP SoC.
diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
index 1e7a789ec2890..4ebae5ad072ad 100644
--- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
+++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
@@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
 	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
 	plat_data->priv_data = hdmi;
 	plat_data->phy_force_vendor = true;
+	plat_data->output_port = 1;
 
 	hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data);
 	if (IS_ERR(hdmi->dw_hdmi))
-- 
2.45.2

[PATCH v2 2/3] drm/lcdif: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach

[Marek Vasut]

Commit a25b988ff83f ("drm/bridge: Extend bridge API to disable connector creation")
added DRM_BRIDGE_ATTACH_NO_CONNECTOR bridge flag and all bridges handle
this flag in some way since then.
Newly added bridge drivers must no longer contain the connector creation and
will fail probing if this flag isn't set.

In order to be able to connect to those newly added bridges as well,
make use of drm_bridge_connector API and have the connector initialized
by the display controller.

Based on 2e87bf389e13 ("drm/rockchip: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach")

This makes LT9611 work with i.MX8M Plus.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Liu Ying <victor.liu@nxp.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
V2: Add RB from Dmitry
---
 drivers/gpu/drm/mxsfb/Kconfig     |  1 +
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 23 ++++++++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
index 264e74f455547..07fb6901996ae 100644
--- a/drivers/gpu/drm/mxsfb/Kconfig
+++ b/drivers/gpu/drm/mxsfb/Kconfig
@@ -30,6 +30,7 @@ config DRM_IMX_LCDIF
 	select DRM_CLIENT_SELECTION
 	select DRM_MXS
 	select DRM_KMS_HELPER
+	select DRM_BRIDGE_CONNECTOR
 	select DRM_GEM_DMA_HELPER
 	select DRM_PANEL
 	select DRM_PANEL_BRIDGE
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 8ee00f59ca821..40dfbc3e6118e 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -17,6 +17,7 @@
 #include <drm/clients/drm_client_setup.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_fbdev_dma.h>
@@ -48,6 +49,8 @@ static const struct drm_encoder_funcs lcdif_encoder_funcs = {
 static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 {
 	struct device *dev = lcdif->drm->dev;
+	struct drm_device *drm = lcdif->drm;
+	struct drm_connector *connector;
 	struct device_node *ep;
 	struct drm_bridge *bridge;
 	int ret;
@@ -97,13 +100,31 @@ 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(drm, encoder);
+		if (IS_ERR(connector)) {
+			ret = PTR_ERR(connector);
+			dev_err_probe(drm->dev, ret,
+				      "Failed to initialize bridge connector: %pe\n",
+				      connector);
+			return ret;
+		}
+
+		ret = drm_connector_attach_encoder(connector, encoder);
+		if (ret < 0) {
+			dev_err_probe(drm->dev, ret,
+				      "Failed to attach encoder.\n");
+			drm_connector_cleanup(connector);
+			return ret;
+		}
 	}
 
 	return 0;
-- 
2.45.2

Re: [PATCH v2 2/3] drm/lcdif: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach

[Liu Ying]

On 12/24/2024, Marek Vasut wrote:
> Commit a25b988ff83f ("drm/bridge: Extend bridge API to disable connector creation")
> added DRM_BRIDGE_ATTACH_NO_CONNECTOR bridge flag and all bridges handle
> this flag in some way since then.
> Newly added bridge drivers must no longer contain the connector creation and
> will fail probing if this flag isn't set.
> 
> In order to be able to connect to those newly added bridges as well,
> make use of drm_bridge_connector API and have the connector initialized
> by the display controller.
> 
> Based on 2e87bf389e13 ("drm/rockchip: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach")
> 
> This makes LT9611 work with i.MX8M Plus.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Liu Ying <victor.liu@nxp.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> V2: Add RB from Dmitry
> ---
>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 23 ++++++++++++++++++++++-
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
> index 264e74f455547..07fb6901996ae 100644
> --- a/drivers/gpu/drm/mxsfb/Kconfig
> +++ b/drivers/gpu/drm/mxsfb/Kconfig
> @@ -30,6 +30,7 @@ config DRM_IMX_LCDIF
>  	select DRM_CLIENT_SELECTION
>  	select DRM_MXS
>  	select DRM_KMS_HELPER
> +	select DRM_BRIDGE_CONNECTOR

Select DRM_DISPLAY_HELPER.

>  	select DRM_GEM_DMA_HELPER
>  	select DRM_PANEL
>  	select DRM_PANEL_BRIDGE
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index 8ee00f59ca821..40dfbc3e6118e 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -17,6 +17,7 @@
>  #include <drm/clients/drm_client_setup.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_fbdev_dma.h>
> @@ -48,6 +49,8 @@ static const struct drm_encoder_funcs lcdif_encoder_funcs = {
>  static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  {
>  	struct device *dev = lcdif->drm->dev;
> +	struct drm_device *drm = lcdif->drm;
> +	struct drm_connector *connector;
>  	struct device_node *ep;
>  	struct drm_bridge *bridge;
>  	int ret;
> @@ -97,13 +100,31 @@ 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(drm, encoder);
> +		if (IS_ERR(connector)) {

of_node_put(ep);

> +			ret = PTR_ERR(connector);
> +			dev_err_probe(drm->dev, ret,
> +				      "Failed to initialize bridge connector: %pe\n",
> +				      connector);

return dev_err_probe(dev, PTR_ERR(connector),
"Failed to initialize bridge connector: %pe\n", connector);

> +			return ret;
> +		}
> +
> +		ret = drm_connector_attach_encoder(connector, encoder);
> +		if (ret < 0) {

of_node_put(ep);

> +			dev_err_probe(drm->dev, ret,
> +				      "Failed to attach encoder.\n");

It looks like no one else calls dev_err_probe() when
drm_connector_attach_encoder() fails, plus drm_connector_attach_encoder()
doesn't return -EPROBE_DEFER at all.

> +			drm_connector_cleanup(connector);
> +			return ret;
> +		}
>  	}
>  
>  	return 0;

-- 
Regards,
Liu Ying

Re: [PATCH v2 2/3] drm/lcdif: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach

[Marek Vasut]

On 12/30/24 8:18 AM, Liu Ying wrote:

[...]

>> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
>> index 264e74f455547..07fb6901996ae 100644
>> --- a/drivers/gpu/drm/mxsfb/Kconfig
>> +++ b/drivers/gpu/drm/mxsfb/Kconfig
>> @@ -30,6 +30,7 @@ config DRM_IMX_LCDIF
>>   	select DRM_CLIENT_SELECTION
>>   	select DRM_MXS
>>   	select DRM_KMS_HELPER
>> +	select DRM_BRIDGE_CONNECTOR
> 
> Select DRM_DISPLAY_HELPER.
Without select DRM_BRIDGE_CONNECTOR, the drm_bridge_connector_init() is 
not defined .

Re: [PATCH v2 2/3] drm/lcdif: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach

[Dmitry Baryshkov]

On Mon, Dec 30, 2024 at 11:11:35PM +0100, Marek Vasut wrote:
> On 12/30/24 8:18 AM, Liu Ying wrote:
> 
> [...]
> 
> > > diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
> > > index 264e74f455547..07fb6901996ae 100644
> > > --- a/drivers/gpu/drm/mxsfb/Kconfig
> > > +++ b/drivers/gpu/drm/mxsfb/Kconfig
> > > @@ -30,6 +30,7 @@ config DRM_IMX_LCDIF
> > >   	select DRM_CLIENT_SELECTION
> > >   	select DRM_MXS
> > >   	select DRM_KMS_HELPER
> > > +	select DRM_BRIDGE_CONNECTOR
> > 
> > Select DRM_DISPLAY_HELPER.
> Without select DRM_BRIDGE_CONNECTOR, the drm_bridge_connector_init() is not
> defined .

You need both.


-- 
With best wishes
Dmitry

[PATCH v2 3/3] drm/mxsfb: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach

[Marek Vasut]

Commit a25b988ff83f ("drm/bridge: Extend bridge API to disable connector creation")
added DRM_BRIDGE_ATTACH_NO_CONNECTOR bridge flag and all bridges handle
this flag in some way since then.
Newly added bridge drivers must no longer contain the connector creation and
will fail probing if this flag isn't set.

In order to be able to connect to those newly added bridges as well,
make use of drm_bridge_connector API and have the connector initialized
by the display controller.

Based on 2e87bf389e13 ("drm/rockchip: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach")

This makes LT9611 work with i.MX8M Mini.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Liu Ying <victor.liu@nxp.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
V2: Cache connector from drm_bridge_connector_init()
---
 drivers/gpu/drm/mxsfb/Kconfig     |  1 +
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 37 ++++++++++++++++++++++---------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
index 07fb6901996ae..e67de148955b2 100644
--- a/drivers/gpu/drm/mxsfb/Kconfig
+++ b/drivers/gpu/drm/mxsfb/Kconfig
@@ -12,6 +12,7 @@ config DRM_MXSFB
 	select DRM_CLIENT_SELECTION
 	select DRM_MXS
 	select DRM_KMS_HELPER
+	select DRM_BRIDGE_CONNECTOR
 	select DRM_GEM_DMA_HELPER
 	select DRM_PANEL
 	select DRM_PANEL_BRIDGE
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 59020862cf65e..2f205512f3105 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -20,6 +20,7 @@
 #include <drm/clients/drm_client_setup.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fbdev_dma.h>
@@ -119,9 +120,9 @@ static const struct drm_mode_config_helper_funcs mxsfb_mode_config_helpers = {
 static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb)
 {
 	struct drm_device *drm = mxsfb->drm;
-	struct drm_connector_list_iter iter;
-	struct drm_panel *panel;
+	struct drm_connector *connector;
 	struct drm_bridge *bridge;
+	struct drm_panel *panel;
 	int ret;
 
 	ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, &panel,
@@ -139,21 +140,35 @@ static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb)
 	if (!bridge)
 		return -ENODEV;
 
-	ret = drm_bridge_attach(&mxsfb->encoder, bridge, NULL, 0);
+	ret = drm_bridge_attach(&mxsfb->encoder, bridge, NULL,
+				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 	if (ret)
 		return dev_err_probe(drm->dev, ret, "Failed to attach bridge\n");
 
-	mxsfb->bridge = bridge;
+	connector = drm_bridge_connector_init(drm, &mxsfb->encoder);
+	if (IS_ERR(connector)) {
+		ret = PTR_ERR(connector);
+		dev_err_probe(drm->dev, ret,
+			      "Failed to initialize bridge connector: %pe\n",
+			      connector);
+		return ret;
+	}
 
-	/*
-	 * Get hold of the connector. This is a bit of a hack, until the bridge
-	 * API gives us bus flags and formats.
-	 */
-	drm_connector_list_iter_begin(drm, &iter);
-	mxsfb->connector = drm_connector_list_iter_next(&iter);
-	drm_connector_list_iter_end(&iter);
+	ret = drm_connector_attach_encoder(connector, &mxsfb->encoder);
+	if (ret < 0) {
+		dev_err_probe(drm->dev, ret,
+			      "Failed to attach encoder.\n");
+		goto err_cleanup_connector;
+	}
+
+	mxsfb->bridge = bridge;
+	mxsfb->connector = connector;
 
 	return 0;
+
+err_cleanup_connector:
+	drm_connector_cleanup(connector);
+	return ret;
 }
 
 static irqreturn_t mxsfb_irq_handler(int irq, void *data)
-- 
2.45.2

Re: [PATCH v2 3/3] drm/mxsfb: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach

[Dmitry Baryshkov]

On Tue, Dec 24, 2024 at 02:46:16AM +0100, Marek Vasut wrote:
> Commit a25b988ff83f ("drm/bridge: Extend bridge API to disable connector creation")
> added DRM_BRIDGE_ATTACH_NO_CONNECTOR bridge flag and all bridges handle
> this flag in some way since then.
> Newly added bridge drivers must no longer contain the connector creation and
> will fail probing if this flag isn't set.
> 
> In order to be able to connect to those newly added bridges as well,
> make use of drm_bridge_connector API and have the connector initialized
> by the display controller.
> 
> Based on 2e87bf389e13 ("drm/rockchip: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach")
> 
> This makes LT9611 work with i.MX8M Mini.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Liu Ying <victor.liu@nxp.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> V2: Cache connector from drm_bridge_connector_init()
> ---
>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 37 ++++++++++++++++++++++---------
>  2 files changed, 27 insertions(+), 11 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/3] drm/mxsfb: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach

[Liu Ying]

On 12/24/2024, Marek Vasut wrote:
> Commit a25b988ff83f ("drm/bridge: Extend bridge API to disable connector creation")
> added DRM_BRIDGE_ATTACH_NO_CONNECTOR bridge flag and all bridges handle
> this flag in some way since then.
> Newly added bridge drivers must no longer contain the connector creation and
> will fail probing if this flag isn't set.
> 
> In order to be able to connect to those newly added bridges as well,
> make use of drm_bridge_connector API and have the connector initialized
> by the display controller.
> 
> Based on 2e87bf389e13 ("drm/rockchip: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach")
> 
> This makes LT9611 work with i.MX8M Mini.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Liu Ying <victor.liu@nxp.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> V2: Cache connector from drm_bridge_connector_init()
> ---
>  drivers/gpu/drm/mxsfb/Kconfig     |  1 +
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 37 ++++++++++++++++++++++---------
>  2 files changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
> index 07fb6901996ae..e67de148955b2 100644
> --- a/drivers/gpu/drm/mxsfb/Kconfig
> +++ b/drivers/gpu/drm/mxsfb/Kconfig
> @@ -12,6 +12,7 @@ config DRM_MXSFB
>  	select DRM_CLIENT_SELECTION
>  	select DRM_MXS
>  	select DRM_KMS_HELPER
> +	select DRM_BRIDGE_CONNECTOR

Select DRM_DISPLAY_HELPER.

>  	select DRM_GEM_DMA_HELPER
>  	select DRM_PANEL
>  	select DRM_PANEL_BRIDGE
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 59020862cf65e..2f205512f3105 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -20,6 +20,7 @@
>  #include <drm/clients/drm_client_setup.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_connector.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_fbdev_dma.h>
> @@ -119,9 +120,9 @@ static const struct drm_mode_config_helper_funcs mxsfb_mode_config_helpers = {
>  static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb)
>  {
>  	struct drm_device *drm = mxsfb->drm;
> -	struct drm_connector_list_iter iter;
> -	struct drm_panel *panel;
> +	struct drm_connector *connector;
>  	struct drm_bridge *bridge;
> +	struct drm_panel *panel;
>  	int ret;
>  
>  	ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, &panel,
> @@ -139,21 +140,35 @@ static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb)
>  	if (!bridge)
>  		return -ENODEV;
>  
> -	ret = drm_bridge_attach(&mxsfb->encoder, bridge, NULL, 0);
> +	ret = drm_bridge_attach(&mxsfb->encoder, bridge, NULL,
> +				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>  	if (ret)
>  		return dev_err_probe(drm->dev, ret, "Failed to attach bridge\n");
>  
> -	mxsfb->bridge = bridge;
> +	connector = drm_bridge_connector_init(drm, &mxsfb->encoder);

Nit: Drop connector and use mxsfb->connector?

> +	if (IS_ERR(connector)) {
> +		ret = PTR_ERR(connector);
> +		dev_err_probe(drm->dev, ret,
> +			      "Failed to initialize bridge connector: %pe\n",
> +			      connector);
> +		return ret;

return dev_err_probe(drm->dev, PTR_ERR(connector),
"Failed to initialize bridge connector: %pe\n", mxsfb->connector);

> +	}
>  
> -	/*
> -	 * Get hold of the connector. This is a bit of a hack, until the bridge
> -	 * API gives us bus flags and formats.
> -	 */
> -	drm_connector_list_iter_begin(drm, &iter);
> -	mxsfb->connector = drm_connector_list_iter_next(&iter);
> -	drm_connector_list_iter_end(&iter);
> +	ret = drm_connector_attach_encoder(connector, &mxsfb->encoder);
> +	if (ret < 0) {
> +		dev_err_probe(drm->dev, ret,
> +			      "Failed to attach encoder.\n");

It looks like no one else calls dev_err_probe() when
drm_connector_attach_encoder() fails, plus drm_connector_attach_encoder()
doesn't return -EPROBE_DEFER at all.

> +		goto err_cleanup_connector;

Nit: Call drm_connector_cleanup() directly instead of using goto.

> +	}
> +
> +	mxsfb->bridge = bridge;
> +	mxsfb->connector = connector;
>  
>  	return 0;
> +
> +err_cleanup_connector:
> +	drm_connector_cleanup(connector);
> +	return ret;
>  }
>  
>  static irqreturn_t mxsfb_irq_handler(int irq, void *data)

-- 
Regards,
Liu Ying

Re: [PATCH v2 3/3] drm/mxsfb: add DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to drm_bridge_attach

[Marek Vasut]

On 12/30/24 8:29 AM, Liu Ying wrote:

[...]

>> @@ -139,21 +140,35 @@ static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb)
>>   	if (!bridge)
>>   		return -ENODEV;
>>   
>> -	ret = drm_bridge_attach(&mxsfb->encoder, bridge, NULL, 0);
>> +	ret = drm_bridge_attach(&mxsfb->encoder, bridge, NULL,
>> +				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>>   	if (ret)
>>   		return dev_err_probe(drm->dev, ret, "Failed to attach bridge\n");
>>   
>> -	mxsfb->bridge = bridge;
>> +	connector = drm_bridge_connector_init(drm, &mxsfb->encoder);
> 
> Nit: Drop connector and use mxsfb->connector?
The code becomes too wide. The rest is fixed in V3.

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Dmitry Baryshkov]

On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
> The dw-hdmi output_port is set to 1 in order to look for a connector
> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR working.
> The output_port set to 1 makes the DW HDMI driver core look up the
> next bridge in DT, where the next bridge is often the hdmi-connector .
> 
> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR")
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Liu Ying <victor.liu@nxp.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> V2: No change
> ---
>  drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
>  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> index 9a480c6abb856..d8e9fbf75edbb 100644
> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>  config DRM_IMX8MP_HDMI_PVI
>  	tristate "Freescale i.MX8MP HDMI PVI bridge support"
>  	depends on OF
> +	select DRM_DISPLAY_CONNECTOR
>  	help
>  	  Choose this to enable support for the internal HDMI TX Parallel
>  	  Video Interface found on the Freescale i.MX8MP SoC.
> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> index 1e7a789ec2890..4ebae5ad072ad 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
>  	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
>  	plat_data->priv_data = hdmi;
>  	plat_data->phy_force_vendor = true;
> +	plat_data->output_port = 1;

Quoting my feedback to a similar Liu's patch:

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.

>  
>  	hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data);
>  	if (IS_ERR(hdmi->dw_hdmi))
> -- 
> 2.45.2
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Marek Vasut]

On 12/24/24 5:21 AM, Dmitry Baryshkov wrote:
> On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
>> The dw-hdmi output_port is set to 1 in order to look for a connector
>> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR working.
>> The output_port set to 1 makes the DW HDMI driver core look up the
>> next bridge in DT, where the next bridge is often the hdmi-connector .
>>
>> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR")
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Fabio Estevam <festevam@gmail.com>
>> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
>> Cc: Jonas Karlman <jonas@kwiboo.se>
>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
>> Cc: Liu Ying <victor.liu@nxp.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Neil Armstrong <neil.armstrong@linaro.org>
>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>> Cc: Robert Foss <rfoss@kernel.org>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Cc: Simona Vetter <simona@ffwll.ch>
>> Cc: Stefan Agner <stefan@agner.ch>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: imx@lists.linux.dev
>> Cc: linux-arm-kernel@lists.infradead.org
>> ---
>> V2: No change
>> ---
>>   drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
>>   drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>>   2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
>> index 9a480c6abb856..d8e9fbf75edbb 100644
>> --- a/drivers/gpu/drm/bridge/imx/Kconfig
>> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
>> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>>   config DRM_IMX8MP_HDMI_PVI
>>   	tristate "Freescale i.MX8MP HDMI PVI bridge support"
>>   	depends on OF
>> +	select DRM_DISPLAY_CONNECTOR
>>   	help
>>   	  Choose this to enable support for the internal HDMI TX Parallel
>>   	  Video Interface found on the Freescale i.MX8MP SoC.
>> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> index 1e7a789ec2890..4ebae5ad072ad 100644
>> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
>>   	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
>>   	plat_data->priv_data = hdmi;
>>   	plat_data->phy_force_vendor = true;
>> +	plat_data->output_port = 1;
> 
> Quoting my feedback to a similar Liu's patch:
> 
> 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.
Looking at the upstream imx8mp*dts , the oldest commit which adds HDMI 
support is commit:

3e67a1ddd56d ("arm64: dts: imx8mp: Enable HDMI on TQMa8MPxL/MBa8MPxL")

That already contains the HDMI connector node. Every follow up addition 
of HDMI to another device has been a copy of the same commit, with 
connector, so I think it is safe to say, no upstream DT is going to be 
broken by this change. Do we care about hypothetical downstream DTs 
which may be missing the connector ?

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Dmitry Baryshkov]

On Wed, 25 Dec 2024 at 22:50, Marek Vasut <marex@denx.de> wrote:
>
> On 12/24/24 5:21 AM, Dmitry Baryshkov wrote:
> > On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
> >> The dw-hdmi output_port is set to 1 in order to look for a connector
> >> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR working.
> >> The output_port set to 1 makes the DW HDMI driver core look up the
> >> next bridge in DT, where the next bridge is often the hdmi-connector .
> >>
> >> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR")
> >>
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> ---
> >> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> >> Cc: David Airlie <airlied@gmail.com>
> >> Cc: Fabio Estevam <festevam@gmail.com>
> >> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> >> Cc: Jonas Karlman <jonas@kwiboo.se>
> >> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> >> Cc: Liu Ying <victor.liu@nxp.com>
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Cc: Maxime Ripard <mripard@kernel.org>
> >> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> >> Cc: Robert Foss <rfoss@kernel.org>
> >> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> >> Cc: Shawn Guo <shawnguo@kernel.org>
> >> Cc: Simona Vetter <simona@ffwll.ch>
> >> Cc: Stefan Agner <stefan@agner.ch>
> >> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >> Cc: dri-devel@lists.freedesktop.org
> >> Cc: imx@lists.linux.dev
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> ---
> >> V2: No change
> >> ---
> >>   drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
> >>   drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
> >>   2 files changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> >> index 9a480c6abb856..d8e9fbf75edbb 100644
> >> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> >> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> >> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
> >>   config DRM_IMX8MP_HDMI_PVI
> >>      tristate "Freescale i.MX8MP HDMI PVI bridge support"
> >>      depends on OF
> >> +    select DRM_DISPLAY_CONNECTOR
> >>      help
> >>        Choose this to enable support for the internal HDMI TX Parallel
> >>        Video Interface found on the Freescale i.MX8MP SoC.
> >> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> >> index 1e7a789ec2890..4ebae5ad072ad 100644
> >> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> >> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> >> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
> >>      plat_data->phy_name = "SAMSUNG HDMI TX PHY";
> >>      plat_data->priv_data = hdmi;
> >>      plat_data->phy_force_vendor = true;
> >> +    plat_data->output_port = 1;
> >
> > Quoting my feedback to a similar Liu's patch:
> >
> > 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.
> Looking at the upstream imx8mp*dts , the oldest commit which adds HDMI
> support is commit:
>
> 3e67a1ddd56d ("arm64: dts: imx8mp: Enable HDMI on TQMa8MPxL/MBa8MPxL")
>
> That already contains the HDMI connector node. Every follow up addition
> of HDMI to another device has been a copy of the same commit, with
> connector, so I think it is safe to say, no upstream DT is going to be
> broken by this change. Do we care about hypothetical downstream DTs
> which may be missing the connector ?

I'd say, not really. If you have to resend the series for whatever
reason, please mention that in the commit message.
If you don't it should be fine.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

RE: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Ying Liu]

On 12/26/2024, Marek Vasut wrote:
> On 12/24/24 5:21 AM, Dmitry Baryshkov wrote:
> > On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
> >> The dw-hdmi output_port is set to 1 in order to look for a connector
> >> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR
> working.
> >> The output_port set to 1 makes the DW HDMI driver core look up the
> >> next bridge in DT, where the next bridge is often the hdmi-connector .
> >>
> >> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
> DRM_BRIDGE_ATTACH_NO_CONNECTOR")
> >>
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> ---
> >> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> >> Cc: David Airlie <airlied@gmail.com>
> >> Cc: Fabio Estevam <festevam@gmail.com>
> >> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> >> Cc: Jonas Karlman <jonas@kwiboo.se>
> >> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> >> Cc: Liu Ying <victor.liu@nxp.com>
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Cc: Maxime Ripard <mripard@kernel.org>
> >> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> >> Cc: Robert Foss <rfoss@kernel.org>
> >> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> >> Cc: Shawn Guo <shawnguo@kernel.org>
> >> Cc: Simona Vetter <simona@ffwll.ch>
> >> Cc: Stefan Agner <stefan@agner.ch>
> >> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >> Cc: dri-devel@lists.freedesktop.org
> >> Cc: imx@lists.linux.dev
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> ---
> >> V2: No change
> >> ---
> >>   drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
> >>   drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
> >>   2 files changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
> b/drivers/gpu/drm/bridge/imx/Kconfig
> >> index 9a480c6abb856..d8e9fbf75edbb 100644
> >> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> >> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> >> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
> >>   config DRM_IMX8MP_HDMI_PVI
> >>   	tristate "Freescale i.MX8MP HDMI PVI bridge support"
> >>   	depends on OF
> >> +	select DRM_DISPLAY_CONNECTOR
> >>   	help
> >>   	  Choose this to enable support for the internal HDMI TX Parallel
> >>   	  Video Interface found on the Freescale i.MX8MP SoC.
> >> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> >> index 1e7a789ec2890..4ebae5ad072ad 100644
> >> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> >> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> >> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct
> platform_device *pdev)
> >>   	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
> >>   	plat_data->priv_data = hdmi;
> >>   	plat_data->phy_force_vendor = true;
> >> +	plat_data->output_port = 1;
> >
> > Quoting my feedback to a similar Liu's patch:
> >
> > 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.
> Looking at the upstream imx8mp*dts , the oldest commit which adds HDMI
> support is commit:
> 
> 3e67a1ddd56d ("arm64: dts: imx8mp: Enable HDMI on
> TQMa8MPxL/MBa8MPxL")
> 
> That already contains the HDMI connector node. Every follow up addition
> of HDMI to another device has been a copy of the same commit, with
> connector, so I think it is safe to say, no upstream DT is going to be
> broken by this change. Do we care about hypothetical downstream DTs
> which may be missing the connector ?

These have no HDMI connector nodes:
arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi
arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts

Regards,
Liu Ying

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Dmitry Baryshkov]

On Mon, Dec 30, 2024 at 07:04:51AM +0000, Ying Liu wrote:
> On 12/26/2024, Marek Vasut wrote:
> > On 12/24/24 5:21 AM, Dmitry Baryshkov wrote:
> > > On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
> > >> The dw-hdmi output_port is set to 1 in order to look for a connector
> > >> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR
> > working.
> > >> The output_port set to 1 makes the DW HDMI driver core look up the
> > >> next bridge in DT, where the next bridge is often the hdmi-connector .
> > >>
> > >> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR")
> > >>
> > >> Signed-off-by: Marek Vasut <marex@denx.de>
> > >> ---
> > >> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> > >> Cc: David Airlie <airlied@gmail.com>
> > >> Cc: Fabio Estevam <festevam@gmail.com>
> > >> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> > >> Cc: Jonas Karlman <jonas@kwiboo.se>
> > >> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> > >> Cc: Liu Ying <victor.liu@nxp.com>
> > >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > >> Cc: Maxime Ripard <mripard@kernel.org>
> > >> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> > >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> > >> Cc: Robert Foss <rfoss@kernel.org>
> > >> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > >> Cc: Shawn Guo <shawnguo@kernel.org>
> > >> Cc: Simona Vetter <simona@ffwll.ch>
> > >> Cc: Stefan Agner <stefan@agner.ch>
> > >> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > >> Cc: dri-devel@lists.freedesktop.org
> > >> Cc: imx@lists.linux.dev
> > >> Cc: linux-arm-kernel@lists.infradead.org
> > >> ---
> > >> V2: No change
> > >> ---
> > >>   drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
> > >>   drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
> > >>   2 files changed, 2 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
> > b/drivers/gpu/drm/bridge/imx/Kconfig
> > >> index 9a480c6abb856..d8e9fbf75edbb 100644
> > >> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> > >> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> > >> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
> > >>   config DRM_IMX8MP_HDMI_PVI
> > >>   	tristate "Freescale i.MX8MP HDMI PVI bridge support"
> > >>   	depends on OF
> > >> +	select DRM_DISPLAY_CONNECTOR
> > >>   	help
> > >>   	  Choose this to enable support for the internal HDMI TX Parallel
> > >>   	  Video Interface found on the Freescale i.MX8MP SoC.
> > >> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > >> index 1e7a789ec2890..4ebae5ad072ad 100644
> > >> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > >> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > >> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct
> > platform_device *pdev)
> > >>   	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
> > >>   	plat_data->priv_data = hdmi;
> > >>   	plat_data->phy_force_vendor = true;
> > >> +	plat_data->output_port = 1;
> > >
> > > Quoting my feedback to a similar Liu's patch:
> > >
> > > 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.
> > Looking at the upstream imx8mp*dts , the oldest commit which adds HDMI
> > support is commit:
> > 
> > 3e67a1ddd56d ("arm64: dts: imx8mp: Enable HDMI on
> > TQMa8MPxL/MBa8MPxL")
> > 
> > That already contains the HDMI connector node. Every follow up addition
> > of HDMI to another device has been a copy of the same commit, with
> > connector, so I think it is safe to say, no upstream DT is going to be
> > broken by this change. Do we care about hypothetical downstream DTs
> > which may be missing the connector ?
> 
> These have no HDMI connector nodes:
> arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi
> arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
> arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
> arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts

Indeed. I should be less trustful.

Unreviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Marek Vasut]

On 12/30/24 8:04 AM, Ying Liu wrote:
> On 12/26/2024, Marek Vasut wrote:
>> On 12/24/24 5:21 AM, Dmitry Baryshkov wrote:
>>> On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
>>>> The dw-hdmi output_port is set to 1 in order to look for a connector
>>>> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR
>> working.
>>>> The output_port set to 1 makes the DW HDMI driver core look up the
>>>> next bridge in DT, where the next bridge is often the hdmi-connector .
>>>>
>>>> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
>> DRM_BRIDGE_ATTACH_NO_CONNECTOR")
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>>>> Cc: David Airlie <airlied@gmail.com>
>>>> Cc: Fabio Estevam <festevam@gmail.com>
>>>> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
>>>> Cc: Jonas Karlman <jonas@kwiboo.se>
>>>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
>>>> Cc: Liu Ying <victor.liu@nxp.com>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Cc: Maxime Ripard <mripard@kernel.org>
>>>> Cc: Neil Armstrong <neil.armstrong@linaro.org>
>>>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>>>> Cc: Robert Foss <rfoss@kernel.org>
>>>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>>> Cc: Simona Vetter <simona@ffwll.ch>
>>>> Cc: Stefan Agner <stefan@agner.ch>
>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Cc: dri-devel@lists.freedesktop.org
>>>> Cc: imx@lists.linux.dev
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> ---
>>>> V2: No change
>>>> ---
>>>>    drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
>>>>    drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>>>>    2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
>> b/drivers/gpu/drm/bridge/imx/Kconfig
>>>> index 9a480c6abb856..d8e9fbf75edbb 100644
>>>> --- a/drivers/gpu/drm/bridge/imx/Kconfig
>>>> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
>>>> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>>>>    config DRM_IMX8MP_HDMI_PVI
>>>>    	tristate "Freescale i.MX8MP HDMI PVI bridge support"
>>>>    	depends on OF
>>>> +	select DRM_DISPLAY_CONNECTOR
>>>>    	help
>>>>    	  Choose this to enable support for the internal HDMI TX Parallel
>>>>    	  Video Interface found on the Freescale i.MX8MP SoC.
>>>> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>> index 1e7a789ec2890..4ebae5ad072ad 100644
>>>> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct
>> platform_device *pdev)
>>>>    	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
>>>>    	plat_data->priv_data = hdmi;
>>>>    	plat_data->phy_force_vendor = true;
>>>> +	plat_data->output_port = 1;
>>>
>>> Quoting my feedback to a similar Liu's patch:
>>>
>>> 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.
>> Looking at the upstream imx8mp*dts , the oldest commit which adds HDMI
>> support is commit:
>>
>> 3e67a1ddd56d ("arm64: dts: imx8mp: Enable HDMI on
>> TQMa8MPxL/MBa8MPxL")
>>
>> That already contains the HDMI connector node. Every follow up addition
>> of HDMI to another device has been a copy of the same commit, with
>> connector, so I think it is safe to say, no upstream DT is going to be
>> broken by this change. Do we care about hypothetical downstream DTs
>> which may be missing the connector ?
> 
> These have no HDMI connector nodes:
> arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi
> arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
> arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
> arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
Huh, I missed those, thanks.

Would it be OK with you to fix those DTs up and add the missing 
connector, rather than introduce some optional port workaround for them ?

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Dmitry Baryshkov]

On Mon, Dec 30, 2024 at 10:44:25PM +0100, Marek Vasut wrote:
> On 12/30/24 8:04 AM, Ying Liu wrote:
> > On 12/26/2024, Marek Vasut wrote:
> > > On 12/24/24 5:21 AM, Dmitry Baryshkov wrote:
> > > > On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
> > > > > The dw-hdmi output_port is set to 1 in order to look for a connector
> > > > > next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR
> > > working.
> > > > > The output_port set to 1 makes the DW HDMI driver core look up the
> > > > > next bridge in DT, where the next bridge is often the hdmi-connector .
> > > > > 
> > > > > Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
> > > DRM_BRIDGE_ATTACH_NO_CONNECTOR")
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > ---
> > > > > Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> > > > > Cc: David Airlie <airlied@gmail.com>
> > > > > Cc: Fabio Estevam <festevam@gmail.com>
> > > > > Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> > > > > Cc: Jonas Karlman <jonas@kwiboo.se>
> > > > > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> > > > > Cc: Liu Ying <victor.liu@nxp.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Cc: Maxime Ripard <mripard@kernel.org>
> > > > > Cc: Neil Armstrong <neil.armstrong@linaro.org>
> > > > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> > > > > Cc: Robert Foss <rfoss@kernel.org>
> > > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > Cc: Shawn Guo <shawnguo@kernel.org>
> > > > > Cc: Simona Vetter <simona@ffwll.ch>
> > > > > Cc: Stefan Agner <stefan@agner.ch>
> > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Cc: imx@lists.linux.dev
> > > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > > ---
> > > > > V2: No change
> > > > > ---
> > > > >    drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
> > > > >    drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
> > > > >    2 files changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
> > > b/drivers/gpu/drm/bridge/imx/Kconfig
> > > > > index 9a480c6abb856..d8e9fbf75edbb 100644
> > > > > --- a/drivers/gpu/drm/bridge/imx/Kconfig
> > > > > +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> > > > > @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
> > > > >    config DRM_IMX8MP_HDMI_PVI
> > > > >    	tristate "Freescale i.MX8MP HDMI PVI bridge support"
> > > > >    	depends on OF
> > > > > +	select DRM_DISPLAY_CONNECTOR
> > > > >    	help
> > > > >    	  Choose this to enable support for the internal HDMI TX Parallel
> > > > >    	  Video Interface found on the Freescale i.MX8MP SoC.
> > > > > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > > b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > > > > index 1e7a789ec2890..4ebae5ad072ad 100644
> > > > > --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > > > > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > > > > @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct
> > > platform_device *pdev)
> > > > >    	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
> > > > >    	plat_data->priv_data = hdmi;
> > > > >    	plat_data->phy_force_vendor = true;
> > > > > +	plat_data->output_port = 1;
> > > > 
> > > > Quoting my feedback to a similar Liu's patch:
> > > > 
> > > > 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.
> > > Looking at the upstream imx8mp*dts , the oldest commit which adds HDMI
> > > support is commit:
> > > 
> > > 3e67a1ddd56d ("arm64: dts: imx8mp: Enable HDMI on
> > > TQMa8MPxL/MBa8MPxL")
> > > 
> > > That already contains the HDMI connector node. Every follow up addition
> > > of HDMI to another device has been a copy of the same commit, with
> > > connector, so I think it is safe to say, no upstream DT is going to be
> > > broken by this change. Do we care about hypothetical downstream DTs
> > > which may be missing the connector ?
> > 
> > These have no HDMI connector nodes:
> > arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi
> > arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
> > arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
> > arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> Huh, I missed those, thanks.
> 
> Would it be OK with you to fix those DTs up and add the missing connector,
> rather than introduce some optional port workaround for them ?

I can't say for iMX8 particularly, but usually we try to keep backwards
compatibility, as DT can be coming from device vendors. So, I'd say, we
need both, the fixed DTS and the workaround.

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Frieder Schrempf]

On 31.12.24 12:48 AM, Dmitry Baryshkov wrote:
> On Mon, Dec 30, 2024 at 10:44:25PM +0100, Marek Vasut wrote:
>> On 12/30/24 8:04 AM, Ying Liu wrote:
>>> On 12/26/2024, Marek Vasut wrote:
>>>> On 12/24/24 5:21 AM, Dmitry Baryshkov wrote:
>>>>> On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
>>>>>> The dw-hdmi output_port is set to 1 in order to look for a connector
>>>>>> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR
>>>> working.
>>>>>> The output_port set to 1 makes the DW HDMI driver core look up the
>>>>>> next bridge in DT, where the next bridge is often the hdmi-connector .
>>>>>>
>>>>>> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
>>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR")
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>> ---
>>>>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>>>>>> Cc: David Airlie <airlied@gmail.com>
>>>>>> Cc: Fabio Estevam <festevam@gmail.com>
>>>>>> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
>>>>>> Cc: Jonas Karlman <jonas@kwiboo.se>
>>>>>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
>>>>>> Cc: Liu Ying <victor.liu@nxp.com>
>>>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>> Cc: Maxime Ripard <mripard@kernel.org>
>>>>>> Cc: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>>>>>> Cc: Robert Foss <rfoss@kernel.org>
>>>>>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>>>>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>>>>> Cc: Simona Vetter <simona@ffwll.ch>
>>>>>> Cc: Stefan Agner <stefan@agner.ch>
>>>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>>>> Cc: dri-devel@lists.freedesktop.org
>>>>>> Cc: imx@lists.linux.dev
>>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>>> ---
>>>>>> V2: No change
>>>>>> ---
>>>>>>    drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
>>>>>>    drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>>>>>>    2 files changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
>>>> b/drivers/gpu/drm/bridge/imx/Kconfig
>>>>>> index 9a480c6abb856..d8e9fbf75edbb 100644
>>>>>> --- a/drivers/gpu/drm/bridge/imx/Kconfig
>>>>>> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
>>>>>> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>>>>>>    config DRM_IMX8MP_HDMI_PVI
>>>>>>    	tristate "Freescale i.MX8MP HDMI PVI bridge support"
>>>>>>    	depends on OF
>>>>>> +	select DRM_DISPLAY_CONNECTOR
>>>>>>    	help
>>>>>>    	  Choose this to enable support for the internal HDMI TX Parallel
>>>>>>    	  Video Interface found on the Freescale i.MX8MP SoC.
>>>>>> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>> b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>>>> index 1e7a789ec2890..4ebae5ad072ad 100644
>>>>>> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>>>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>>>> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct
>>>> platform_device *pdev)
>>>>>>    	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
>>>>>>    	plat_data->priv_data = hdmi;
>>>>>>    	plat_data->phy_force_vendor = true;
>>>>>> +	plat_data->output_port = 1;
>>>>>
>>>>> Quoting my feedback to a similar Liu's patch:
>>>>>
>>>>> 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.
>>>> Looking at the upstream imx8mp*dts , the oldest commit which adds HDMI
>>>> support is commit:
>>>>
>>>> 3e67a1ddd56d ("arm64: dts: imx8mp: Enable HDMI on
>>>> TQMa8MPxL/MBa8MPxL")
>>>>
>>>> That already contains the HDMI connector node. Every follow up addition
>>>> of HDMI to another device has been a copy of the same commit, with
>>>> connector, so I think it is safe to say, no upstream DT is going to be
>>>> broken by this change. Do we care about hypothetical downstream DTs
>>>> which may be missing the connector ?
>>>
>>> These have no HDMI connector nodes:
>>> arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi
>>> arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
>>> arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
>>> arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>> Huh, I missed those, thanks.
>>
>> Would it be OK with you to fix those DTs up and add the missing connector,
>> rather than introduce some optional port workaround for them ?
> 
> I can't say for iMX8 particularly, but usually we try to keep backwards
> compatibility, as DT can be coming from device vendors. So, I'd say, we
> need both, the fixed DTS and the workaround.

FWIW, personally for the Kontron devicetrees mentioned above, I'm okay
with adding the connector node and break compatibility. Those
devicetrees have been added recently and I forgot to include the
connector node.

Of course I can only speak for myself and for Kontron, not for others.

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Liu Ying]

On 12/24/2024, Marek Vasut wrote:
> The dw-hdmi output_port is set to 1 in order to look for a connector
> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR working.
> The output_port set to 1 makes the DW HDMI driver core look up the
> next bridge in DT, where the next bridge is often the hdmi-connector .
> 
> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR")
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Liu Ying <victor.liu@nxp.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> V2: No change
> ---
>  drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
>  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> index 9a480c6abb856..d8e9fbf75edbb 100644
> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>  config DRM_IMX8MP_HDMI_PVI
>  	tristate "Freescale i.MX8MP HDMI PVI bridge support"
>  	depends on OF
> +	select DRM_DISPLAY_CONNECTOR

Select it for DRM_IMX8MP_DW_HDMI_BRIDGE instead?
Furthermore, if yes, should it be even selected for DRM_DW_HDMI instead?

I'm not sure if this is needed to be selected though, since only DRM_MESON is
selecting it while CONFIG_DRM_DISPLAY_CONNECTOR is enabled in multiple
defconfig files(like arch/arm/configs/multi_v7_defconfig).

>  	help
>  	  Choose this to enable support for the internal HDMI TX Parallel
>  	  Video Interface found on the Freescale i.MX8MP SoC.
> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> index 1e7a789ec2890..4ebae5ad072ad 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
>  	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
>  	plat_data->priv_data = hdmi;
>  	plat_data->phy_force_vendor = true;
> +	plat_data->output_port = 1;

Dmitry's comment applies here.
https://lore.kernel.org/all/vvsj6ri2ke25nzocbq736yv7rphzma6pn3yk2uh7iu43zfe2sa@2fwye4k4w6he/

>  
>  	hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data);
>  	if (IS_ERR(hdmi->dw_hdmi))

-- 
Regards,
Liu Ying

Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Marek Vasut]

On 12/30/24 7:57 AM, Liu Ying wrote:

[...]

>> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
>> index 9a480c6abb856..d8e9fbf75edbb 100644
>> --- a/drivers/gpu/drm/bridge/imx/Kconfig
>> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
>> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>>   config DRM_IMX8MP_HDMI_PVI
>>   	tristate "Freescale i.MX8MP HDMI PVI bridge support"
>>   	depends on OF
>> +	select DRM_DISPLAY_CONNECTOR
> 
> Select it for DRM_IMX8MP_DW_HDMI_BRIDGE instead?
> Furthermore, if yes, should it be even selected for DRM_DW_HDMI instead?
> 
> I'm not sure if this is needed to be selected though, since only DRM_MESON is
> selecting it while CONFIG_DRM_DISPLAY_CONNECTOR is enabled in multiple
> defconfig files(like arch/arm/configs/multi_v7_defconfig).

It is necessary to update existing .config .

[...]