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

[Luca Ceresoli]

Hello Damon,

On Thu Apr 2, 2026 at 11:14 AM CEST, Damon Ding wrote:
> 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@xxxxxxx>
>> Tested-by: Martyn Welch <martyn.welch@xxxxxxxxxxxxx>
>> Tested-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx> # TQMa8MPxL/MBa8MPxL
>> Signed-off-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
>> ---
>> 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.

I saw this only after sending v3. Anyway, replying now.

I'm not sure I follow completely your reasoning, but one aspect is that we
can (and should) convert drivers in small groups independently as I'm doing
with this series for the i.MX8MP and involved drivers and you are doing for
the analogix_dp in another series. Converting all (or large sets) at once
would just be not doable realistically.

And so I suggest you send a separate series to convert the Rockchip drivers
based on dw-hdmi. Please Cc me, I'll be glad to review it.

Also, it's not clear to me why we need a case C. We already have two cases
(A and B, the old and new way) and we should convert to case B unless there
is a strong reason. If Rockchip dts are missing a hdmi-connector node you
can reuse the fixup module in patch 8.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com