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

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

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.

>> --- 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?

>> +	err = of_overlay_fdt_apply(dtbo_start, dtbo_size, &ovcs_id, NULL);
>> +	if (err)
>> +		return err;
>> +
>> +	hdmi_conn = of_find_node_by_name(NULL, "fixup-hdmi-connector");
>
> Do you really need to find the node, since the overlay was just applied?

That was to ensure the node is present and error out if it isn't. But
thinking again about it after your question I don't see how it could be
missing if the overlay was successfully applied.

Removing this check in v2, which makes this function a lot simpler!

>> 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.

[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?

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.

>> +	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.

OTOH how can a common module know the specific connector?

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