Re: [PATCH 7/8] drm/bridge: imx8mp-hdmi-tx: add an hdmi-connector when missing using a DT overlay at boot time

[Liu Ying <victor.liu@nxp.com>]

Hi Luca,

On Fri, Mar 27, 2026 at 03:46:43PM +0100, Luca Ceresoli wrote:
> Hello Liu,
> 
> On Thu Mar 26, 2026 at 9:15 AM CET, Liu Ying wrote:
>> Hi Luca,
>>
>> On Fri, Mar 20, 2026 at 11:46:18AM +0100, Luca Ceresoli wrote:
>>> The imx8mp-hdmi-tx one of many drivers based on dw-hdmi. dw-hdmi in turn
>>> 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 [0]
>>>
>>>  B. hdmi->plat_data->output_port = 1:
>>>     the HDMI output (port@1) is parsed to find the next bridge
>>>
>>> The imx8mp-hdmi-tx driver falls in case A. This implies next_bridge will
>>> always be NULL, and so dw_hdmi_bridge_attach() [1] will always fail if
>>> called with the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
>>>
>>> In fact case A assumes that DRM_BRIDGE_ATTACH_NO_CONNECTOR is not set and
>>> in that case it adds the connector programmatically at bridge attach time.
>>>
>>> Support for DRM_BRIDGE_ATTACH_NO_CONNECTOR is implemented by dw-hdmi.c in
>>> case B. So, in preparation to support DRM_BRIDGE_ATTACH_NO_CONNECTOR in
>>> imx8mp-hdmi-tx, move to case B by setting hdmi->plat_data->output_port = 1.
>>>
>>> However this change requires that port@1 is connected to a "next
>>> bridge" DT node, typically the HDMI connector, because dw-hdmi won't add
>>> the connector when using DRM_BRIDGE_ATTACH_NO_CONNECTOR.
>>>
>>> Many dts files for imx8mp-based boards in the kernel have such a connector
>>> described and linked to port@1, so a connector is added by the
>>> display-connector driver along with a bridge wrapping it. Sadly some of
>>
>> Hmm, display-connector driver is a bridge driver so it cannot add a connector.
>> I assume that you mean a connector will be added by the bridge connector
>> driver.
> 
> Indeed, rewording as:
> 
>   Many dts files for imx8mp-based boards in the kernel have such a
>   connector described and linked to port@1, so the pipeline will be fully
>   attached up to a display-connector and a drm_connector added by the
>   bridge-connector.

LGTM.

> 
>>> --- a/drivers/gpu/drm/bridge/imx/Kconfig
>>> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
>>> @@ -25,6 +25,23 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>>>  	  Choose this to enable support for the internal HDMI encoder found
>>>  	  on the i.MX8MP SoC.
>>>
>>> +config DRM_IMX8MP_DW_HDMI_BRIDGE_CONNECTOR_FIXUP
>>> +	bool "Support device tree blobs without an hdmi-connector node"
>>> +	default y
>>
>> depends on DRM_IMX_LCDIF ?
> 
> If the imx hdmi-tx is not enabled then HDMI won't work anyway, so users are
> not affected and the overlay is not needed. Am I missing something?

I meant I'm fine with "default y" and think that this could also depend on
DRM_IMX_LCDIF, because no display controller driver other than the LCDIF
driver needs the fixup.

[...]

>>> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso
>>> new file mode 100644
>>> index 000000000000..ee718ca1b11b
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso
>>> @@ -0,0 +1,38 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * DTS overlay adding an hdmi-connector node to boards using the imx8mp hdmi_tx
>>> + *
>>> + * Copyright (C) 2026 GE HealthCare
>>> + * Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
>>> + */
>>> +
>>> +/dts-v1/;
>>> +/plugin/;
>>> +
>>> +&{/} {
>>
>> I see build warnings(W=1):
>> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso:25.8-37.4: Warning (unit_address_vs_reg): /fragment@0/__overlay__/soc@0: node has a unit name, but no reg or ranges property
>> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso:26.16-36.5: Warning (unit_address_vs_reg): /fragment@0/__overlay__/soc@0/bus@32c00000: node has a unit name, but no reg or ranges property
>> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso:27.18-35.6: Warning (unit_address_vs_reg): /fragment@0/__overlay__/soc@0/bus@32c00000/hdmi@32fd8000: node has a unit name, but no reg or ranges property
>> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso:29.13-33.8: Warning (unit_address_vs_reg): /fragment@0/__overlay__/soc@0/bus@32c00000/hdmi@32fd8000/ports/port@1: node has a unit name, but no reg or ranges property
> 
> AFAIK the device tree checkes just can't work on overlays. The tools just
> cannot know on which base tree the overlay can be applied, so they cannot
> know the existing properties. That might change in the future, but for now
> my understanding is that it is OK to have overlays which produce such
> harmless warnings, at least for driver-specific overlays like the tilcdc
> one [0] which is already in linux-next since a few weeks.

Hmm, not sure a few weeks in linux-next is long enough ;)
I'd say, I saw the warnings, so simply reported along with a fix to suppress
them.  TBH, build warnings make me nervous, especially this DT overlay is
under the "DRM DRIVERS FOR FREESCALE IMX BRIDGE" umbrella.

> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=0ff223d991477fa4677dcb0f1fb00065847e2212
> 
>> Here is a patch to suppress them:
>>
>> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso
>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso
>> @@ -10,6 +10,9 @@
>>  /plugin/;
>>
>>  &{/} {
>> +       #address-cells = <2>;
>> +       #size-cells = <2>;
>> +
>>         fixup-hdmi-connector {
>>                 compatible = "hdmi-connector";
>>                 label = "HDMI";
>> @@ -23,10 +26,25 @@ fixup_hdmi_connector_in: endpoint {
>>         };
>>
>>         soc@0 {
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               ranges = <0x0 0x0 0x0 0x3e000000>;
>> +
>>                 bus@32c00000 {
>> +                       reg = <0x32c00000 0x400000>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <1>;
>> +
>>                         hdmi@32fd8000 {
>> +                               reg = <0x32fd8000 0x7eff>;
>> +
>>                                 ports {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +
>>                                         port@1 {
>> +                                               reg = <1>;
>> +
>>                                                 hdmi_tx_out: endpoint {
>>                                                         remote-endpoint = <&fixup_hdmi_connector_in>;
>>                                                 };
> 
> Thanks for taking time to look into how to get rid of the warnings.
> 
> However the sheer amount of lines added, by just duplicating lines already
> in the base tree and no added value, reinforces my opinion that we should
> keep the overlay as simple as it is.
> 
> Also, what if one of the property values that your diff is duplicating from
> the base tree turns out being wrong in the base tree and gets fixed later
> there? The wrong value would be re-added by the overlay unless someone goes
> hunting all the duplicated lines around.
> 
> Based on this, do you think we really need to get rid of those warnings?

Well, I tend to get rid of those warning, because it seems that people
usually avoid this kind of warnings for regular DT overlays, but I might
be wrong.

> 
> Side note: this discussion made me think about what would happen if
> DRM_IMX8MP_DW_HDMI_BRIDGE is enabled on a non-imx8mp board (as for
> distribution kernels as mentioned by Laurent). I think it makes sense to
> add a check that /soc@0/compatible matches "fsl,imx8mp-soc" and not apply
> the overlay otherwise. I'll look into that for v2.

Makes sense to me.

> 
>>> +	fixup-hdmi-connector {
>>> +		compatible = "hdmi-connector";
>>> +		label = "HDMI";
>>> +		type = "a";
>>
>> What if a board uses another type?
> 
> For boards affected by this patch, currently the connector is created by
> dw_hdmi_connector_create() which hardcodes type A [0], so there would be no
> difference.

Yes, that's from driver's PoV.  However, userspace may get the type
from /sys/firmware/devicetree/base/fixup-hdmi-connector/type and use it
to do something.

Maybe, that's trivial.

> 
> OTOH how can a common module know the specific connector?

Hmm, maybe add a module parameter or let users set the type through Kconfig
or even define an unknown type to honestly tell users that we don't know it?

> 
> Boards with a different connector should describe the connector in the
> device tree, if they need to instantiate the exact type.
> 
> [0] https://elixir.bootlin.com/linux/v7.0-rc5/source/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c#L2601
> 
> Luca
> 
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com/

-- 
Regards,
Liu Ying