Claude review: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

Claude review: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

[Claude Code Review Bot]

Overall Series Review

Subject: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
Patches: 10
Reviewed: 2026-03-22T03:48:36.879364

---

This is a well-structured and well-motivated series that modernizes the i.MX8MP LCDIF driver to use `DRM_BRIDGE_ATTACH_NO_CONNECTOR` with `drm_bridge_connector`, which is the current DRM best practice. The cover letter is excellent and clearly explains the dependency chain between patches. The backward compatibility approach using a DT overlay (patch 7) is a reasonable reuse of the pattern from `drm/tilcdc`.

There is one clear bug in patch 1 (wrong cleanup type), and a few minor issues in other patches. The core changes in patches 6 and 8 look correct and well-reasoned.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

[Claude Code Review Bot]

Patch Review

This is the goal patch. The change is clean and follows the standard pattern:

```c
ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
...
connector = drm_bridge_connector_init(lcdif->drm, encoder);
...
ret = drm_connector_attach_encoder(connector, encoder);
```

The Kconfig `select DRM_BRIDGE_CONNECTOR` is correct.

This looks good. All the preparatory work in patches 1-7 makes this final patch straightforward.

---
Generated by Claude Code Patch Reviewer

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

[Luca Ceresoli]

This series modernizes the i.mx8mp LCDIF driver to use the
bridge-connector, which is the current best practice in DRM.

== Call for testing on i.MX8MP boards (especially those using HDMI)!

For who tested v1 (thanks!): some patches have changed so I had to drop
your Tested-by on them. A new round of test would still be useful.

This series applies changes to how video output devices are probed on
i.MX8MP, especially those using HDMI. Even though I have put care in not
breaking anything, there could potentially be pitfalls I haven't realized,
causing regressions on existing boards.

I have thus added in Cc all developers which appeared active on dts files
for imx8mp boards involving video. I would appreciate testing on as many
boards as possible, along with a Tested-by tag, or a report about any
issues encountered.

Thanks in advance to all testers!

== Review recommendation

I recommend reviewing patches in this order to be understood more
effectively:

 * Cover letter
 * Patches 1-6: small preliminary cleanups (can be applied independently)
 * Patch 10: the goal of this series, but would not work alone
 * Patch 9: lets patch 10 work; but in turn it can't work alone
 * Patch 8: lets patch 9 work; but in turn it can't work alone
 * Patch 7: lets patch 8 work

== Series description

This series is not strictly related to DRM bridge hotplug, it is rather a
preparation step. Introducing hotplug would need two different approaches:
one for the new way, for drivers using bridge-connector and
DRM_BRIDGE_ATTACH_NO_CONNECTOR, another for drivers using the "old, legacy
way" where the last bridge is supposed to instantiate the
drm_connector. Hotplug is complicated enough in one case, so it makes sense
to only support the new way.

The hardware I'm working on is an i.MX8MP, whose LCDIF driver is still
using the old way. So this series converts to the new way as a preparation
step.

Patch 10 does the conversion, which is simple. However this would introduce
a regression on some boards. Here's why:

There are 3 instances of the LCDIF in i.MX8MP:

 * LCDIF1, driving the DSI output
 * LCDIF2, driving the LVDS output
 * LCDIF3, driving the HDMI output

The device drivers of peripherals connected to LCDIF1 and LCDIF2 already
support the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag. So far so good.

LCDIF3 is more tricky. The HDMI pipeline is:

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

The fsl,imx8mp-hdmi-tx (hdmi-tx) does not support
DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it is based on the dw-hdmi component
which supports it by simply changing a setting in the driver platform
data. Patch 9 does this switch.

However, for that switch to work, the device tree must describe the HDMI
connector (compatible = "hdmi-connector").

Unfortunately not all device trees in mainline have an hdmi-connector
node. Adding one is easy, but would break existing hardware upgrading to a
newer kernel without upgrading the device tree blob. This is addressed by
patch 8 reusing an existing approach to add such a node to the live device
tree at init time using a device tree overlay for boards which don't have
one.

Finally, patch 8 cannot work alone because of a bad interaction between
devlink and device tree overlays. Patch 7 solves that.

== Grand plan

This is part of the work to support hotplug of DRM bridges. The grand plan
was discussed in [0].

Here's the work breakdown (➜ marks the current series):

 1. … add refcounting to DRM bridges struct drm_bridge,
      based on devm_drm_bridge_alloc()
    A. ✔ add new alloc API and refcounting (v6.16)
    B. ✔ convert all bridge drivers to new API (v6.17)
    C. ✔ kunit tests (v6.17)
    D. ✔ add get/put to drm_bridge_add/remove() + attach/detach()
         and warn on old allocation pattern (v6.17)
    E. … add get/put on drm_bridge accessors
       1. ✔ drm_bridge_chain_get_first_bridge(), add cleanup action (v6.18)
       2. ✔ drm_bridge_get_prev_bridge() (v6.18)
       3. ✔ drm_bridge_get_next_bridge() (v6.19)
       4. ✔ drm_for_each_bridge_in_chain() (v6.19)
       5. ✔ drm_bridge_connector_init (v6.19)
       6. … protect encoder bridge chain with a mutex
       7. … of_drm_find_bridge
          a. ✔ add of_drm_get_bridge() (v7.0),
	       convert basic direct users (v7.0-v7.1)
	  b. ✔ convert direct of_drm_get_bridge() users, part 2 (v7.0)
	  c. ✔ convert direct of_drm_get_bridge() users, part 3 (v7.0)
	  d. ✔… convert direct of_drm_get_bridge() users, part 4
	        (some v7.1, some pending)
	  e.   convert bridge-only drm_of_find_panel_or_bridge() users
       8. drm_of_find_panel_or_bridge, *_of_get_bridge
       9. ✔ enforce drm_bridge_add before drm_bridge_attach (v6.19)
    F. ✔ debugfs improvements
       1. ✔ add top-level 'bridges' file (v6.16)
       2. ✔ show refcount and list lingering bridges (v6.19)
 2. … handle gracefully atomic updates during bridge removal
    A. ✔ Add drm_bridge_enter/exit() to protect device resources (v7.0)
    B. … protect private_obj removal from list
    C. ✔ Add drm_bridge_clear_and_put() (v7.1)
 3. … DSI host-device driver interaction
 4. ✔ removing the need for the "always-disconnected" connector
 5. ➜ Migrate i.MX LCDIF driver to bridge-connector
 6.   DRM bridge hotplug
    A.   Bridge hotplug management in the DRM core
    B.   Device tree description

[0] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/#t

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v2:
- Dropped patch removing the loop in lcdif_attach_bridge, adapted following
  patches as needed, added patch to use __free on the ep pointer
- Added new cleanup patch (patch 6)
- Build the fixup module unconditionally
- patch 7: fix returned error codes
- patch 1: fix cleanup action
- Various minor improvements based on reviews, see per-patch changelog
- Removed bouncing recipients from Cc
- Link to v1: https://lore.kernel.org/r/20260320-drm-lcdif-dbanc-v1-0-479a04133e70@bootlin.com

---
Luca Ceresoli (10):
      drm/mxsfb/lcdif: simplify remote pointer management using __free
      drm/mxsfb/lcdif: simplify ep pointer management using __free
      drm/mxsfb/lcdif: use dev_err_probe() consistently in lcdif_attach_bridge
      drm/mxsfb/lcdif: lcdif_attach_bridge: move iteration-specific variables declaration inside loop
      drm/bridge: dw-hdmi: document the output_port field
      drm/bridge: dw-hdmi: warn on unsupported attach combination
      drm/bridge: dw-hdmi: move next_bridge lookup to attach time
      drm/bridge: imx8mp-hdmi-tx: add an hdmi-connector when missing using a DT overlay at boot time
      drm/bridge: imx8mp-hdmi-tx: switch to DRM_BRIDGE_ATTACH_NO_CONNECTOR
      drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

 drivers/gpu/drm/bridge/imx/Kconfig                 | 18 ++++++
 drivers/gpu/drm/bridge/imx/Makefile                |  2 +
 .../bridge/imx/imx8mp-hdmi-tx-connector-fixup.c    | 53 +++++++++++++++++
 .../bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso | 56 ++++++++++++++++++
 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c        |  1 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c          | 49 +++++++--------
 drivers/gpu/drm/mxsfb/Kconfig                      |  2 +
 drivers/gpu/drm/mxsfb/lcdif_drv.c                  | 69 +++++++++++-----------
 include/drm/bridge/dw_hdmi.h                       |  6 ++
 9 files changed, 193 insertions(+), 63 deletions(-)
---
base-commit: cb6fd48ab016a4b2245d805a5b766eea590e32dd
change-id: 20260306-drm-lcdif-dbanc-83dd948327de

Best regards,
--  
Luca Ceresoli <luca.ceresoli@bootlin.com>

[PATCH v2 01/10] drm/mxsfb/lcdif: simplify remote pointer management using __free

[Luca Ceresoli]

Putting the remote device_node reference requires a of_node_put(remote) in
two places. Use a cleanup action to simplify the code.

Reviewed-by: Liu Ying <victor.liu@nxp.com>
Tested-by: Martyn Welch <martyn.welch@collabora.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8MPxL/MBa8MPxL
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- Use the correct cleanup action
- Fix commit message
- add missing cleanup.h include
---
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 47da1d9336b9..7719629487da 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -5,6 +5,7 @@
  * This code is based on drivers/gpu/drm/mxsfb/mxsfb*
  */
 
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
@@ -53,16 +54,13 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 	int ret;
 
 	for_each_endpoint_of_node(dev->of_node, ep) {
-		struct device_node *remote;
+		struct device_node *remote __free(device_node) =
+			of_graph_get_remote_port_parent(ep);
 		struct of_endpoint of_ep;
 		struct drm_encoder *encoder;
 
-		remote = of_graph_get_remote_port_parent(ep);
-		if (!of_device_is_available(remote)) {
-			of_node_put(remote);
+		if (!of_device_is_available(remote))
 			continue;
-		}
-		of_node_put(remote);
 
 		ret = of_graph_parse_endpoint(ep, &of_ep);
 		if (ret < 0) {

-- 
2.53.0

[PATCH v2 02/10] drm/mxsfb/lcdif: simplify ep pointer management using __free

[Luca Ceresoli]

Putting the ep device_node reference requires a of_node_put(ep) in many
return points. Use a cleanup action to simplify the code.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

This patch is new in v2
---
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 7719629487da..8da8a265c05c 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -49,7 +49,7 @@ static const struct drm_encoder_funcs lcdif_encoder_funcs = {
 static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 {
 	struct device *dev = lcdif->drm->dev;
-	struct device_node *ep;
+	struct device_node *ep __free(device_node) = NULL;
 	struct drm_bridge *bridge;
 	int ret;
 
@@ -65,23 +65,19 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 		ret = of_graph_parse_endpoint(ep, &of_ep);
 		if (ret < 0) {
 			dev_err(dev, "Failed to parse endpoint %pOF\n", ep);
-			of_node_put(ep);
 			return ret;
 		}
 
 		bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, of_ep.id);
-		if (IS_ERR(bridge)) {
-			of_node_put(ep);
+		if (IS_ERR(bridge))
 			return dev_err_probe(dev, PTR_ERR(bridge),
 					     "Failed to get bridge for endpoint%u\n",
 					     of_ep.id);
-		}
 
 		encoder = devm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL);
 		if (!encoder) {
 			dev_err(dev, "Failed to allocate encoder for endpoint%u\n",
 				of_ep.id);
-			of_node_put(ep);
 			return -ENOMEM;
 		}
 
@@ -91,17 +87,14 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 		if (ret) {
 			dev_err(dev, "Failed to initialize encoder for endpoint%u: %d\n",
 				of_ep.id, ret);
-			of_node_put(ep);
 			return ret;
 		}
 
 		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
-		if (ret) {
-			of_node_put(ep);
+		if (ret)
 			return dev_err_probe(dev, ret,
 					     "Failed to attach bridge for endpoint%u\n",
 					     of_ep.id);
-		}
 	}
 
 	return 0;

-- 
2.53.0

[PATCH v2 03/10] drm/mxsfb/lcdif: use dev_err_probe() consistently in lcdif_attach_bridge

[Luca Ceresoli]

lcdif_attach_bridge() uses dev_err_probe() in some error paths, dev_err() +
return in others. Use dev_err_probe() for all of them to make code
consistent, simpler and with bettere error reporting.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- rewrote after removal of previous patch removing loop
- not added review/test trailers as the code is a bit different
---
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 8da8a265c05c..1c76709c4d94 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -63,10 +63,8 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 			continue;
 
 		ret = of_graph_parse_endpoint(ep, &of_ep);
-		if (ret < 0) {
-			dev_err(dev, "Failed to parse endpoint %pOF\n", ep);
-			return ret;
-		}
+		if (ret < 0)
+			return dev_err_probe(dev, ret, "Failed to parse endpoint %pOF\n", ep);
 
 		bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, of_ep.id);
 		if (IS_ERR(bridge))
@@ -75,20 +73,18 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 					     of_ep.id);
 
 		encoder = devm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL);
-		if (!encoder) {
-			dev_err(dev, "Failed to allocate encoder for endpoint%u\n",
-				of_ep.id);
-			return -ENOMEM;
-		}
+		if (!encoder)
+			return dev_err_probe(dev, -ENOMEM,
+					     "Failed to allocate encoder for endpoint%u\n",
+					     of_ep.id);
 
 		encoder->possible_crtcs = drm_crtc_mask(&lcdif->crtc);
 		ret = drm_encoder_init(lcdif->drm, encoder, &lcdif_encoder_funcs,
 				       DRM_MODE_ENCODER_NONE, NULL);
-		if (ret) {
-			dev_err(dev, "Failed to initialize encoder for endpoint%u: %d\n",
-				of_ep.id, ret);
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to initialize encoder for endpoint%u\n",
+					     of_ep.id);
 
 		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
 		if (ret)

-- 
2.53.0

[PATCH v2 04/10] drm/mxsfb/lcdif: lcdif_attach_bridge: move iteration-specific variables declaration inside loop

[Luca Ceresoli]

The bridge and ret variables are per-iteration variables, whose values
don't have to be carried to the next iteration or be used after the loop
end. Move their declaration inside the loop scope as a cleanup and to make
code clearer.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

This patch is new in v2
---
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 1c76709c4d94..c8ba8f9b1da8 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -50,14 +50,14 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 {
 	struct device *dev = lcdif->drm->dev;
 	struct device_node *ep __free(device_node) = NULL;
-	struct drm_bridge *bridge;
-	int ret;
 
 	for_each_endpoint_of_node(dev->of_node, ep) {
 		struct device_node *remote __free(device_node) =
 			of_graph_get_remote_port_parent(ep);
 		struct of_endpoint of_ep;
+		struct drm_bridge *bridge;
 		struct drm_encoder *encoder;
+		int ret;
 
 		if (!of_device_is_available(remote))
 			continue;

-- 
2.53.0

[PATCH v2 05/10] drm/bridge: dw-hdmi: document the output_port field

[Luca Ceresoli]

The meaning of this flag may not be obvious at first sight.

Reviewed-by: Liu Ying <victor.liu@nxp.com>
Tested-by: Martyn Welch <martyn.welch@collabora.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8MPxL/MBa8MPxL
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- improved comment as suggested by Liu
---
 include/drm/bridge/dw_hdmi.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 336f062e1f9d..8500dd4f99d8 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -126,6 +126,12 @@ struct dw_hdmi_phy_ops {
 struct dw_hdmi_plat_data {
 	struct regmap *regm;
 
+	/*
+	 * The HDMI output port number must be 1 if the port is described
+	 * in the device tree. 0 if the device tree does not describe the
+	 * next component (legacy mode, i.e. without
+	 * DRM_BRIDGE_ATTACH_NO_CONNECTOR flag when attaching bridge).
+	 */
 	unsigned int output_port;
 
 	unsigned long input_bus_encoding;

-- 
2.53.0

[PATCH v2 06/10] drm/bridge: dw-hdmi: warn on unsupported attach combination

[Luca Ceresoli]

dw-hdmi 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

 B. hdmi->plat_data->output_port = 1:
    the HDMI output (port@1) is parsed to find the next bridge

Only case B is supported when the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is
passed to the attach callback. Emit a warning when this is violated. Also
return -EINVAL which would be returned by drm_bridge_attach() right after
anyway.

Reviewed-by: Liu Ying <victor.liu@nxp.com>
Tested-by: Martyn Welch <martyn.welch@collabora.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8MPxL/MBa8MPxL
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Note: Returning when the warning triggers does not change the functional
behaviour of this function. It is not strictly necessary in this patch but
it will have to be done anyway in the following patch.
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 0296e110ce65..ab1a6a8783cd 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2910,6 +2910,10 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge,
 {
 	struct dw_hdmi *hdmi = bridge->driver_private;
 
+	/* DRM_BRIDGE_ATTACH_NO_CONNECTOR requires a remote-endpoint to the next bridge */
+	if (WARN_ON((flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) && !hdmi->plat_data->output_port))
+		return -EINVAL;
+
 	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
 		return drm_bridge_attach(encoder, hdmi->bridge.next_bridge,
 					 bridge, flags);

-- 
2.53.0

[PATCH v2 07/10] drm/bridge: dw-hdmi: move next_bridge lookup to attach time

[Luca Ceresoli]

This driver looks up the next_bridge at probe time and stores it in
hdmi->bridge.next_bridge, but only uses the stored value when attaching,
and only in the DRM_BRIDGE_ATTACH_NO_CONNECTOR case.

This will be problematic with an upcoming change, adding an hdmi-connector
using a device tree overlay when not present. That change is in turn
necessary to migrate the i.MX LCDIF driver to the bridge-connector.

The problem is that, adding the hdmi-connector via an overlay, devlink
considers hdmi-connector a consumer of the dw-hdmi device, generating a
chicken-egg problem:

 * hdmi-connector probe won't be tried until dw-hdmi is probed (devlink)
 * dw-hdmi probe will defer until it finds the next_bridge (the
   hdmi-connector wrapper bridge)

In preparation for those changes, move the next_bridge lookup from probe to
attach, when it is actually used. This allows dw-hdmi to probe, so that the
hdmi-connector can probe as well.

Also avoid storing the pointer in hdmi->bridge.next_bridge: the value is
computed when needed, thus a local variable is enough.

Finally, this also allows to slightly improve the code by not doing any DT
lookup in the !DRM_BRIDGE_ATTACH_NO_CONNECTOR case.

Tested-by: Martyn Welch <martyn.welch@collabora.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8MPxL/MBa8MPxL
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- Fix returned error codes
- Added missing cleanup.h include
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++--------------------
 1 file changed, 16 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ab1a6a8783cd..f4a1ebb79716 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -6,6 +6,8 @@
  * Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
  * Copyright (C) 2010, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
  */
+
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
@@ -2914,9 +2916,20 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge,
 	if (WARN_ON((flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) && !hdmi->plat_data->output_port))
 		return -EINVAL;
 
-	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
-		return drm_bridge_attach(encoder, hdmi->bridge.next_bridge,
-					 bridge, flags);
+	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
+		struct device_node *remote __free(device_node) =
+			of_graph_get_remote_node(hdmi->dev->of_node,
+						 hdmi->plat_data->output_port, -1);
+		if (!remote)
+			return -ENODEV;
+
+		struct drm_bridge *next_bridge __free(drm_bridge_put) =
+			of_drm_find_and_get_bridge(remote);
+		if (!next_bridge)
+			return -EPROBE_DEFER;
+
+		return drm_bridge_attach(encoder, next_bridge, bridge, flags);
+	}
 
 	return dw_hdmi_connector_create(hdmi);
 }
@@ -3307,28 +3320,6 @@ static void dw_hdmi_init_hw(struct dw_hdmi *hdmi)
  * Probe/remove API, used from platforms based on the DRM bridge API.
  */
 
-static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
-{
-	struct device_node *remote;
-
-	if (!hdmi->plat_data->output_port)
-		return 0;
-
-
-	remote = of_graph_get_remote_node(hdmi->dev->of_node,
-					  hdmi->plat_data->output_port,
-					  -1);
-	if (!remote)
-		return -ENODEV;
-
-	hdmi->bridge.next_bridge = of_drm_find_and_get_bridge(remote);
-	of_node_put(remote);
-	if (!hdmi->bridge.next_bridge)
-		return -EPROBE_DEFER;
-
-	return 0;
-}
-
 bool dw_hdmi_bus_fmt_is_420(struct dw_hdmi *hdmi)
 {
 	return hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format);
@@ -3373,10 +3364,6 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 	mutex_init(&hdmi->cec_notifier_mutex);
 	spin_lock_init(&hdmi->audio_lock);
 
-	ret = dw_hdmi_parse_dt(hdmi);
-	if (ret < 0)
-		return ERR_PTR(ret);
-
 	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
 	if (ddc_node) {
 		hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node);

-- 
2.53.0

[PATCH v2 08/10] drm/bridge: imx8mp-hdmi-tx: add an hdmi-connector when missing using a DT overlay at boot time

[Luca Ceresoli]

The imx8mp-hdmi-tx is 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 an HDMI Type A connector programmatically at bridge
attach time.

Support for DRM_BRIDGE_ATTACH_NO_CONNECTOR is implemented by dw-hdmi.c in
case B. However switching to base B 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 the pipeline will be fully attached up
to a display-connector and a drm_connector added by the
bridge-connector. Sadly some of those dts files don't have the connector
described. Adding it would solve the problem easily, but this would break
existing devices which do not update the dtb when upgrading to a newer
kernel.

In preparation for switching to case B while preserving backward
compatibility for such devices, introduce a module adding the
hdmi-connector node to the live device tree at init time. This will allow
the dw-hdmi code to find the next bridge (the one wrapping the
hdmi-connector) and let the pipeline work as before.

The module is inserted only if there is no endpoint in port@1. So boards
whose device tree describe the connector will not have the device tre
modified, and will start isntantiating the correct HDMI connector type as
described in the device tree.

For boards lacking a connector description in DT the overlay will be added,
abd the HDMI connector will be Type A, which is a reasonable fallback and
is what the driver is currently doing.

[0] https://elixir.bootlin.com/linux/v7.0-rc1/source/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c#L3310
[1] https://elixir.bootlin.com/linux/v7.0-rc1/source/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c#L2907

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- don't apply the overlay if the SoC is not i.MX8MP
- build unconditionally, but depend on DRM_IMX_LCDIF
- remove useless error check
- add missing cleanup.h and init.h includes, remove unneeded includes
- avoid dtc warnings on overlay
- fix typo in Kconfig help text
- not added the Tested-bys because the code has changed
- split the 'plat_data->output_port = 1' line to a separate patch

This patch is inspired by commit 0ff223d99147 ("drm/tilcdc: Convert legacy
panel binding via DT overlay at boot time")
---
 drivers/gpu/drm/bridge/imx/Kconfig                 | 18 +++++++
 drivers/gpu/drm/bridge/imx/Makefile                |  2 +
 .../bridge/imx/imx8mp-hdmi-tx-connector-fixup.c    | 53 ++++++++++++++++++++
 .../bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso | 56 ++++++++++++++++++++++
 4 files changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
index b9028a5e5a06..49f074559b00 100644
--- a/drivers/gpu/drm/bridge/imx/Kconfig
+++ b/drivers/gpu/drm/bridge/imx/Kconfig
@@ -18,6 +18,8 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
 	depends on OF
 	depends on COMMON_CLK
 	select DRM_DW_HDMI
+	select OF_OVERLAY
+	select DRM_DISPLAY_CONNECTOR
 	imply DRM_IMX8MP_HDMI_PAI
 	imply DRM_IMX8MP_HDMI_PVI
 	imply PHY_FSL_SAMSUNG_HDMI_PHY
@@ -25,6 +27,22 @@ 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
+	default y
+	depends on DRM_IMX_LCDIF
+	depends on DRM_IMX8MP_DW_HDMI_BRIDGE
+	depends on OF
+	help
+	  Modifies at early boot the live device tree of boards using the
+	  i.MX8MP fsl,imx8mp-hdmi-tx adding a hdmi-connector node linked to
+	  the hdmi-tx. This is needed to support bridge-connector usage in
+	  the i.MX8MP LCDIF driver.
+
+	  You need this if you use the i.MX8MP HDMI output and your board
+	  device tree file does not have an hdmi-connector node connected
+	  to it.
+
 config DRM_IMX8MP_HDMI_PAI
 	tristate "Freescale i.MX8MP HDMI PAI bridge support"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
index 8d01fda25451..84499fe2e444 100644
--- a/drivers/gpu/drm/bridge/imx/Makefile
+++ b/drivers/gpu/drm/bridge/imx/Makefile
@@ -1,6 +1,8 @@
 obj-$(CONFIG_DRM_IMX_LDB_HELPER) += imx-ldb-helper.o
 obj-$(CONFIG_DRM_IMX_LEGACY_BRIDGE) += imx-legacy-bridge.o
 obj-$(CONFIG_DRM_IMX8MP_DW_HDMI_BRIDGE) += imx8mp-hdmi-tx.o
+obj-$(CONFIG_DRM_IMX8MP_DW_HDMI_BRIDGE_CONNECTOR_FIXUP) += imx8mp-hdmi-tx-connector-fixup.o \
+							   imx8mp-hdmi-tx-connector-fixup.dtbo.o
 obj-$(CONFIG_DRM_IMX8MP_HDMI_PAI) += imx8mp-hdmi-pai.o
 obj-$(CONFIG_DRM_IMX8MP_HDMI_PVI) += imx8mp-hdmi-pvi.o
 obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.c
new file mode 100644
index 000000000000..4c0a22bf416b
--- /dev/null
+++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Add an hdmi-connector node to boards using the imx8mp hdmi_tx which
+ * don't have one. This is needed for the i.MX LCDIF to work with
+ * DRM_BRIDGE_ATTACH_NO_CONNECTOR.
+ *
+ * Copyright (C) 2026 GE HealthCare
+ * Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
+ */
+
+#include <linux/cleanup.h>
+#include <linux/init.h>
+#include <linux/of.h>
+
+/* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.dtbs */
+extern char __dtbo_imx8mp_hdmi_tx_connector_fixup_begin[];
+extern char __dtbo_imx8mp_hdmi_tx_connector_fixup_end[];
+
+static int __init imx8mp_hdmi_tx_connector_fixup_init(void)
+{
+	struct device_node *soc       __free(device_node) = NULL;
+	struct device_node *hdmi_tx   __free(device_node) = NULL;
+	struct device_node *endpoint  __free(device_node) = NULL;
+	struct device_node *hdmi_conn __free(device_node) = NULL;
+	void *dtbo_start;
+	u32 dtbo_size;
+	int ovcs_id;
+
+	soc = of_find_node_by_path("/soc@0");
+	if (!soc)
+		return 0;
+
+	/* This applies to i.MX8MP only, do nothing on other systems */
+	if (!of_device_is_compatible(soc, "fsl,imx8mp-soc"))
+		return 0;
+
+	hdmi_tx = of_find_node_by_path("/soc@0/bus@32c00000/hdmi@32fd8000");
+	if (!of_device_is_available(hdmi_tx))
+		return 0;
+
+	/* If endpoint exists, assume an hdmi-connector exists already */
+	endpoint = of_find_node_by_path("/soc@0/bus@32c00000/hdmi@32fd8000/ports/port@1/endpoint");
+	if (endpoint)
+		return 0;
+
+	dtbo_start = __dtbo_imx8mp_hdmi_tx_connector_fixup_begin;
+	dtbo_size = __dtbo_imx8mp_hdmi_tx_connector_fixup_end -
+		    __dtbo_imx8mp_hdmi_tx_connector_fixup_begin;
+
+	return of_overlay_fdt_apply(dtbo_start, dtbo_size, &ovcs_id, NULL);
+}
+
+subsys_initcall(imx8mp_hdmi_tx_connector_fixup_init);
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..5dbe06d5ffe7
--- /dev/null
+++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx-connector-fixup.dtso
@@ -0,0 +1,56 @@
+// 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/;
+
+&{/} {
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	fixup-hdmi-connector {
+		compatible = "hdmi-connector";
+		label = "HDMI";
+		type = "a";
+
+		port {
+			fixup_hdmi_connector_in: endpoint {
+				remote-endpoint = <&hdmi_tx_out>;
+			};
+		};
+	};
+
+	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>;
+						};
+					};
+				};
+			};
+		};
+	};
+};

-- 
2.53.0

[PATCH v2 09/10] drm/bridge: imx8mp-hdmi-tx: switch to DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Luca Ceresoli]

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. Switching to case B, which is
the current best practice, requires that the HDMI connector is always
described in the live device tree, and a previous commit ensured this is
always the case by adding an overlay in case the device tree does not
comply.

So now we can simply switch to the new mode and support
DRM_BRIDGE_ATTACH_NO_CONNECTOR.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- new patch, split out of the previous patch
---
 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
index 32fd3554e267..8e8cfd66f23b 100644
--- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
+++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
@@ -138,6 +138,7 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
 	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
 	plat_data->priv_data = hdmi;
 	plat_data->phy_force_vendor = true;
+	plat_data->output_port = 1;
 
 	platform_set_drvdata(pdev, hdmi);
 

-- 
2.53.0

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

[Luca Ceresoli]

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@collabora.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8MPxL/MBa8MPxL
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- Added missing select DRM_DISPLAY_HELPER in Kconfig
- Rebased on previous patch changes
---
 drivers/gpu/drm/mxsfb/Kconfig     |  2 ++
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 16 +++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
index 264e74f45554..31db7a824a93 100644
--- a/drivers/gpu/drm/mxsfb/Kconfig
+++ b/drivers/gpu/drm/mxsfb/Kconfig
@@ -33,6 +33,8 @@ config DRM_IMX_LCDIF
 	select DRM_GEM_DMA_HELPER
 	select DRM_PANEL
 	select DRM_PANEL_BRIDGE
+	select DRM_DISPLAY_HELPER
+	select DRM_BRIDGE_CONNECTOR
 	help
 	  Choose this option if you have an LCDIFv3 LCD controller.
 	  Those devices are found in various i.MX SoC (i.MX8MP,
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index c8ba8f9b1da8..7f07ae24e0dc 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -18,6 +18,7 @@
 #include <drm/clients/drm_client_setup.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_fbdev_dma.h>
@@ -57,6 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 		struct of_endpoint of_ep;
 		struct drm_bridge *bridge;
 		struct drm_encoder *encoder;
+		struct drm_connector *connector;
 		int ret;
 
 		if (!of_device_is_available(remote))
@@ -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);
 		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);
+		if (IS_ERR(connector))
+			return dev_err_probe(dev, PTR_ERR(connector),
+					     "Failed to init bridge_connector for endpoint%u\n",
+					     of_ep.id);
+
+		ret = drm_connector_attach_encoder(connector, encoder);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to attach connector for endpoint%u\n",
+					     of_ep.id);
 	}
 
 	return 0;

-- 
2.53.0

Claude review: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

[Claude Code Review Bot]

Overall Series Review

Subject: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
Patches: 11
Reviewed: 2026-03-31T16:54:42.874753

---

This is a well-structured 10-patch series that modernizes the i.MX8MP LCDIF driver to use `DRM_BRIDGE_ATTACH_NO_CONNECTOR` with the `drm_bridge_connector` framework, which is the current DRM best practice. The series is logically ordered: patches 1-4 are clean preparatory cleanups to lcdif_drv.c, patches 5-6 improve dw-hdmi documentation and validation, patch 7 resolves a devlink/probe-ordering problem, patch 8 provides DT backward compatibility via an overlay, patch 9 enables the new mode in imx8mp-hdmi-tx, and patch 10 makes the final LCDIF conversion.

The approach is sound and the backward-compatibility strategy using DT overlays (following the tilcdc precedent) is reasonable. The series deserves broad hardware testing given the regression potential on i.MX8MP HDMI boards.

A few issues noted below, mostly minor, with one moderate concern in patch 7 and one dead-variable issue in patch 8.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mxsfb/lcdif: simplify remote pointer management using __free

[Claude Code Review Bot]

Patch Review

Clean conversion. Using `__free(device_node)` on `remote` correctly eliminates both `of_node_put(remote)` calls. The `continue` path and the fall-through path are both handled by the cleanup attribute.

Reviewed-by and Tested-by tags present from v1. Looks good.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mxsfb/lcdif: simplify ep pointer management using __free

[Claude Code Review Bot]

Patch Review

This converts the `ep` variable from `for_each_endpoint_of_node()` to use `__free(device_node)`. The key question is whether `__free` interacts correctly with `for_each_endpoint_of_node` — the macro reassigns `ep` on each iteration. Since `__free` fires on scope exit (function return), and `for_each_endpoint_of_node` internally calls `of_node_put` on the previous value before assigning the next, the `__free` only needs to handle the early-return case where the loop iterator hasn't been put yet. This is correct.

No review/test tags since this is new in v2. Should get review.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mxsfb/lcdif: use dev_err_probe() consistently in lcdif_attach_bridge

[Claude Code Review Bot]

Patch Review

Straightforward conversion of `dev_err()` + `return` to `dev_err_probe()`. One minor nit:

Using `dev_err_probe()` for `-ENOMEM` from `devm_kzalloc()` is a bit unusual — `-ENOMEM` is never `-EPROBE_DEFER`, so the deferred-probe tracking aspect of `dev_err_probe()` doesn't add value here. It's not wrong, just slightly unconventional. The consistency argument in the commit message is fair enough.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mxsfb/lcdif: lcdif_attach_bridge: move iteration-specific variables declaration inside loop

[Claude Code Review Bot]

Patch Review

Trivial scope-narrowing cleanup. Correct.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: dw-hdmi: document the output_port field

[Claude Code Review Bot]

Patch Review

The comment added is helpful:

```c
/*
 * The HDMI output port number must be 1 if the port is described
 * in the device tree. 0 if the device tree does not describe the
 * next component (legacy mode, i.e. without
 * DRM_BRIDGE_ATTACH_NO_CONNECTOR flag when attaching bridge).
 */
```

Clear and useful.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: dw-hdmi: warn on unsupported attach combination

[Claude Code Review Bot]

Patch Review

```c
if (WARN_ON((flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) && !hdmi->plat_data->output_port))
    return -EINVAL;
```

The WARN_ON is appropriate — this is a programming error (driver misconfiguration), not a runtime condition. Good.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: dw-hdmi: move next_bridge lookup to attach time

[Claude Code Review Bot]

Patch Review

This is the most complex change. The commit message explains the devlink chicken-egg problem well.

**Concern about `__free(drm_bridge_put)` on `next_bridge`:** The code does:

```c
struct drm_bridge *next_bridge __free(drm_bridge_put) =
    of_drm_find_and_get_bridge(remote);
if (!next_bridge)
    return -EPROBE_DEFER;

return drm_bridge_attach(encoder, next_bridge, bridge, flags);
```

The `__free(drm_bridge_put)` will call `drm_bridge_put(next_bridge)` when the variable goes out of scope (i.e., after `drm_bridge_attach` returns). This is correct only if `drm_bridge_attach` takes its own reference on the bridge. Given that this codebase has been converted to refcounted bridges (per the cover letter's grand plan), this should be fine — `drm_bridge_attach` presumably does a `drm_bridge_get`. But this is worth double-checking by the maintainers, since getting this wrong would cause a use-after-free.

**Error return semantics:** `of_drm_find_and_get_bridge()` returns `NULL` on failure (not `ERR_PTR`), and the code correctly checks `!next_bridge`. Good.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: imx8mp-hdmi-tx: add an hdmi-connector when missing using a DT overlay at boot time

[Claude Code Review Bot]

Patch Review

**Dead variable:** `hdmi_conn` is declared with `__free(device_node)` but is never assigned or used anywhere in the function. This should be removed.

```c
struct device_node *hdmi_conn __free(device_node) = NULL;  // unused
```

**Minor typos in commit message:** "device tre" should be "device tree", "isntantiating" should be "instantiating", "abd" should be "and".

**Error handling for `of_overlay_fdt_apply`:** The function returns `of_overlay_fdt_apply(...)` directly. If the overlay application fails, this returns an error from a `subsys_initcall`, which will be logged but won't prevent boot. This seems acceptable — the HDMI output simply won't work, which is a degraded but not catastrophic failure.

**Hardcoded paths:** The code uses hardcoded DT paths like `/soc@0/bus@32c00000/hdmi@32fd8000`. These are specific to the i.MX8MP SoC and the compatible check (`fsl,imx8mp-soc`) guards against running on other hardware. This is reasonable for a fixup module.

**Kconfig:** The `DRM_IMX8MP_DW_HDMI_BRIDGE_CONNECTOR_FIXUP` is a `bool` with `default y` — it's always built when both LCDIF and HDMI bridge are enabled. The `select OF_OVERLAY` and `select DRM_DISPLAY_CONNECTOR` on the parent config are appropriate.

**The overlay itself** looks correct — it adds the `fixup-hdmi-connector` node at root level and adds a `port@1/endpoint` to the existing hdmi node with proper remote-endpoint phandle linkage.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: imx8mp-hdmi-tx: switch to DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Claude Code Review Bot]

Patch Review

A one-liner:

```c
plat_data->output_port = 1;
```

This is the key change that switches from legacy mode (case A) to the modern mode (case B). Depends entirely on patches 7 and 8 being correct. The change itself is trivial and correct.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

[Claude Code Review Bot]

Patch Review

The final conversion. The diff is clean:

```c
ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
// ... then:
connector = drm_bridge_connector_init(lcdif->drm, encoder);
// ... then:
ret = drm_connector_attach_encoder(connector, encoder);
```

This follows the standard `DRM_BRIDGE_ATTACH_NO_CONNECTOR` pattern correctly. The Kconfig additions (`DRM_DISPLAY_HELPER`, `DRM_BRIDGE_CONNECTOR`) are the right dependencies.

**Summary of actionable items:**
1. **Patch 8:** Remove the unused `hdmi_conn` variable
2. **Patch 8:** Fix typos in commit message ("device tre", "isntantiating", "abd")
3. **Patch 7:** Maintainers should verify that `drm_bridge_attach` takes a reference, so that `__free(drm_bridge_put)` on `next_bridge` is safe

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

[Claude Code Review Bot]

Overall Series Review

Subject: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
Patches: 14
Reviewed: 2026-04-12T13:37:33.436594

---

This is a well-structured 11-patch series that migrates the i.MX8MP LCDIF driver from the legacy connector model to the modern `DRM_BRIDGE_ATTACH_NO_CONNECTOR` + bridge-connector pattern. The series is well organized: patches 1-4 are cleanups, patch 5-6 document and add safety checks to dw-hdmi, patch 7 resolves a devlink chicken-egg problem, patches 8-9 handle backward compatibility via a DT overlay fixup, patch 10 enables the new mode for imx8mp-hdmi-tx, and patch 11 does the actual LCDIF conversion.

The series shows good engineering discipline -- the backward-compatibility concern for boards without an `hdmi-connector` DT node is well thought out, the DT overlay approach reuses an existing pattern (tilcdc), and the series has gathered extensive testing and review tags from prior revisions.

There is one notable bug in patch 8 (error handling logic is inverted) and a potential bridge refcount lifetime concern in patch 7. The cleanup patches (1-4) and documentation patches (5-6) are straightforward and correct.

**Recommendation**: Address the bug in patch 8 and the lifetime concern in patch 7 before merging. The rest looks good.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

[Claude Code Review Bot]

Patch Review

The core change of the series:

```c
-		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
+		ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
...
+		connector = drm_bridge_connector_init(lcdif->drm, encoder);
+		if (IS_ERR(connector))
+			return dev_err_probe(dev, PTR_ERR(connector),
+					     "Failed to init bridge_connector for endpoint%u\n",
+					     of_ep.id);
+
+		ret = drm_connector_attach_encoder(connector, encoder);
```

This follows the standard bridge-connector pattern correctly. The Kconfig additions of `DRM_DISPLAY_HELPER` and `DRM_BRIDGE_CONNECTOR` are necessary.

The commit message's analysis of the three LCDIF pipelines and their bridge compatibility is thorough and appreciated.

**Minor typos in commit message**: "surrently" should be "currently", "amd" should be "and".

No functional issues.

---
Generated by Claude Code Patch Reviewer