Re: [PATCH v2 06/10] drm/bridge: dw-hdmi: warn on unsupported attach combination

[Damon Ding <damon.ding@rock-chips.com>]

[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]

Hi Luca,

On 3/31/2026 3:25 AM, Luca Ceresoli wrote:
> dw-hdmi can operate in two different modes, depending on the platform data
> as set by the driver:
> 
>   A. hdmi->plat_data->output_port = 0:
>      the HDMI output (port@1) in device tree is not used
> 
>   B. hdmi->plat_data->output_port = 1:
>      the HDMI output (port@1) is parsed to find the next bridge
> 
> Only case B is supported when the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is
> passed to the attach callback. Emit a warning when this is violated. Also
> return -EINVAL which would be returned by drm_bridge_attach() right after
> anyway.
> 
> Reviewed-by: Liu Ying <victor.liu@nxp.com>
> Tested-by: Martyn Welch <martyn.welch@collabora.com>
> Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8MPxL/MBa8MPxL
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
> Note: Returning when the warning triggers does not change the functional
> behaviour of this function. It is not strictly necessary in this patch but
> it will have to be done anyway in the following patch.
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 0296e110ce65..ab1a6a8783cd 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2910,6 +2910,10 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge,
>   {
>   	struct dw_hdmi *hdmi = bridge->driver_private;
>   
> +	/* DRM_BRIDGE_ATTACH_NO_CONNECTOR requires a remote-endpoint to the next bridge */
> +	if (WARN_ON((flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) && !hdmi->plat_data->output_port))
> +		return -EINVAL;
> +
>   	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
>   		return drm_bridge_attach(encoder, hdmi->bridge.next_bridge,
>   					 bridge, flags);
> 

Since many older Rockchip platforms (RK3288, RK3399, etc.) lack a 
hdmi-connector node linked to the HDMI DT node, which corresponds to 
case A. Could we relax this restriction and treat cases where 
DRM_BRIDGE_ATTACH_NO_CONNECTOR is set but hdmi->plat_data->output_port = 
0 as a new case C?

For Rockchip platforms, the HDMI driver invokes dw_hdmi_bind() to attach 
the Synopsys bridge. This sequence differs from that on the XNP 
platform, but is similar to the Allwinner implementation.

If we treat the case where DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and 
hdmi->plat_data->output_port = 0 as -EINVAL, I will have to modify the 
HDMI DT configuration for all Rockchip platforms when adapting to the 
bridge-connector framework.

The patch that adapts to the bridge-connector framework and has been 
verified OK on RK3399 is attached.

Best regards,
Damon

[-- Attachment #2: dw_hdmi-rockchip_test_env_20260402.patch --]
[-- Type: text/plain, Size: 3041 bytes --]

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dts b/arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dts
index 70aee1ab904c..5a78e741f113 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dts
@@ -17,6 +17,7 @@ aliases {
 
 	chosen {
 		stdout-path = "serial2:1500000n8";
+		bootargs = "root=PARTUUID=614e0000-0000 rootwait";
 	};
 
 	vcc5v0_sys: regulator-vcc5v0-sys {
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ada45e8b3e2c..0e97e183a8b3 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2914,7 +2914,7 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge,
 
 	/* DRM_BRIDGE_ATTACH_NO_CONNECTOR requires a remote-endpoint to the next bridge */
 	if (WARN_ON((flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) && !hdmi->plat_data->output_port))
-		return -EINVAL;
+		return 0;
 
 	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
 		struct device_node *remote __free(device_node) =
@@ -3628,7 +3628,7 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
 	if (IS_ERR(hdmi))
 		return hdmi;
 
-	ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL, 0);
+	ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 	if (ret) {
 		dw_hdmi_remove(hdmi);
 		return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index e7f49fe845ea..69b832d0c8c4 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -76,6 +76,7 @@ config ROCKCHIP_DW_DP
 
 config ROCKCHIP_DW_HDMI
 	bool "Rockchip specific extensions for Synopsys DW HDMI"
+	select DRM_BRIDGE_CONNECTOR
 	help
 	  This selects support for Rockchip SoC specific extensions
 	  for the Synopsys DesignWare HDMI driver. If you want to
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 0dc1eb5d2ae3..174a10c1d7c2 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -13,6 +13,7 @@
 #include <linux/regulator/consumer.h>
 
 #include <drm/bridge/dw_hdmi.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_of.h>
 #include <drm/drm_probe_helper.h>
@@ -542,6 +543,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 	struct drm_device *drm = data;
 	struct drm_encoder *encoder;
 	struct rockchip_hdmi *hdmi;
+	struct drm_connector *connector;
 	int ret;
 
 	if (!pdev->dev.of_node)
@@ -619,7 +621,14 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 		goto err_bind;
 	}
 
-	return 0;
+	connector = drm_bridge_connector_init(drm, encoder);
+	if (IS_ERR(connector)) {
+		ret = PTR_ERR(connector);
+		dev_err(hdmi->dev, "Failed to initialize bridge_connector\n");
+		goto err_bind;
+	}
+
+	return drm_connector_attach_encoder(connector, encoder);
 
 err_bind:
 	drm_encoder_cleanup(encoder);