Re: [PATCH v2 10/10] drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

[Luca Ceresoli]

Hello Liu,

On Thu Apr 2, 2026 at 6:55 AM CEST, Liu Ying wrote:
> Hi Luca,
>
> On Mon, Mar 30, 2026 at 09:25:51PM +0200, Luca Ceresoli wrote:
>> Convert this driver to DRM_BRIDGE_ATTACH_NO_CONNECTOR and to the
>> drm_bridge_connector framework which is the current DRM bridge best
>> practice.
>>
>> Tested-by: Martyn Welch <martyn.welch@xxxxxxxxxxxxx>
>> Tested-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx> # TQMa8MPxL/MBa8MPxL
>> Signed-off-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>

>> @@ -86,11 +88,23 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>>  					     "Failed to initialize encoder for endpoint%u\n",
>>  					     of_ep.id);
>>
>> -		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
>> +		ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>
> It seems that only analogix-anx6345.c, analogix-anx78xx.c and analogix_dp_core.c
> don't allow DRM_BRIDGE_ATTACH_NO_CONNECTOR, since they error out when attaching
> the bridge with the flag:
>
> if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
>          DRM_ERROR("Fix bridge driver to make connector optional!");
>          return -EINVAL;
> }
>
> Looks like i.MX8MP platforms don't use these drivers.

Exactly. I have checked all the drivers involved with the i.MX8MP and all
of the support DRM_BRIDGE_ATTACH_NO_CONNECTOR.

While converting all drivers is surely a good goal, converting all of them
at once is not realistically doable. So the approach I took was to convert
one specifically (lcdif_drv.c) plus all those which would break because
they are used with the LCDIF.

> But, are we completely safe here by adding the flag?  You also mentioned
> "pitfalls" in commit mesg, which makes me a bit more worried.

I mentioned potential pitfalls in the cover letter mainly because of the DT
overlay insertion patch, which is somewhat tricky as it impacts many
boards.

Additionally it's not easy to spot all usages of this component by parsing
dozens of dts files, so I might have missed some.

So overall every patch sent has a potential for pitfalls, but for the
reasons above I think this series has a bit more.

Does this reassure you? :)

>>  		if (ret)
>>  			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);
>
> Also, kernel doc of drm_bridge_connector.c says:
>
>  * To make use of this helper, all bridges in the chain shall report bridge
>  * operation flags (&drm_bridge->ops) and bridge output type
>  * (&drm_bridge->type), as well as the DRM_BRIDGE_ATTACH_NO_CONNECTOR attach
>  * flag (none of the bridges shall create a DRM connector directly).
>
> Are you sure that we are safe to use this helper?

Yes. I have checked all in-tree dts[i] files for all the 3 LCDIFs.

For the LCDIF3, the pipeline is:

  LCDIF3 -> fsl,imx8mp-hdmi-pvi -> fsl,imx8mp-hdmi-tx -> HDMI connector

And the involved bridges are:

 * fsl,imx8mp-hdmi-pvi has ops = 0 (it doesn't set it) because it
   implements none the optional features mentioned by those flags, and it
   honors the DRM_BRIDGE_ATTACH_NO_CONNECTOR by propagating it

 * fsl,imx8mp-hdmi-tx is implemented based on dw-hdmi, which sets ops as
   appropriate and also propagates the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag

 * display-connector (enabled via the DT overlay if needed) sets ops and
   makes DRM_BRIDGE_ATTACH_NO_CONNECTOR mandatory

The LCDIF2 involves the panel-bridge, display-connector and lvds-decoder
which also set ops as needed and propagate DRM_BRIDGE_ATTACH_NO_CONNECTOR
or make it mandatory.

The same applies to the drivers used with the LCDIF1: adv7511, tc358767 and
the panel bridge.

I assume this answers your doubts. Let me know if it doesn't.

Luca

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