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 <[email protected]> >> + */ >> + >> +/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
