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