[PATCH v11 0/3] Add i.MX91/93 parallel display support

[PATCH v11 0/3] Add i.MX91/93 parallel display support

[Marco Felsch]

Hi,

this patchset adds the driver, dt-bindings and dt integration required
to drive a parallel display on the i.MX93.

Since the i.MX91 register layout equals the one from the i.MX93, I added
the support for both but tested only the i.MX93 case.

This patchset depends on:
 - https://lore.kernel.org/all/20251201-v6-18-topic-imx93-blkctrl-v1-0-b57a72e60105@pengutronix.de/

@Conor Dooley
I dropped your r-b tag since I added the 'bus-width' property.

Regards,
  Marco

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Changes in v11:
- Link to v10: https://lore.kernel.org/r/20260302-v6-18-topic-imx93-parallel-display-v10-0-634fe2778c7a@pengutronix.de
- Fix double space (Liu)
- Drop 'const' (Liu)
- Drop GFP_KERNEL from kmalloc_obj() (Liu)
- Add r-b (Luca)

Changes in v10:
- Link to v9: https://lore.kernel.org/r/20260115-v6-18-topic-imx93-parallel-display-v9-0-2c5051e4b144@pengutronix.de
- Add MEDIA_BUS_FMT_FIXED (Liu)
- Drop next_bridge from driver struct and use bridge.next_bridge (Liu)
- Drop linux/of_address.h include (Liu)
- imx93-pdfc: drop bridge.driver_private usage++ (Liu)
- Make use of kmalloc_obj() (Liu)

Changes in v9:
- Link to v8: https://lore.kernel.org/r/20260113-v6-18-topic-imx93-parallel-display-v8-0-4abccdc473a5@pengutronix.de
- dt-bindings: drop unncessary changes (Frank)
- imx93-pdfc: drop bridge.driver_private usage (Luca)
- Kconfig: Adapt Kconfig symbol and prompt (Luca)

Changes in v8:
- Link to v7: https://lore.kernel.org/r/20251202-v6-18-topic-imx93-parallel-display-v7-0-2cce31d64608@pengutronix.de
- dt-bindings: add nxp,imx91-pdfc compatible (Liu)
- dt-bindings: use video-interfaces.yaml# (Liu)
- dt-bindings: s/data lanes/data lines/ (Liu)
- dt-bindings: drop 'reg' poperty
- dt-bindings: drop #address-cells, #size-cells
- imx93-pdfc: drop drm/drm_print.h include (Liu)
- imx93-pdfc: s/exist/exists/ (Liu)
- imx93-pdfc: drop MEDIA_BUS_FMT_FIXED from imx93_pdfc_bus_output_fmts
- imx93-pdfc: imx93_pdfc_bus_output_fmt_supported: make fmt const
- imx93-pdfc: Rework input-fmt selection to always fallback to a sane
              default.
- imx93-pdfc: imx93_pdfc_bridge_atomic_check: make use of
	      imx93_pdfc_bus_output_fmt_supported() 
- imx93-pdfc: drop 'reg' dt-property usage
- imx93-pdfc: imx93_pdfc_bridge_probe: pass -1 for endpoint reg value (Liu)

Changes in v7:
- Link to v6: https://lore.kernel.org/r/20251201-v6-18-topic-imx93-parallel-display-v6-0-7b056e1e5b1e@pengutronix.de
- Add missing bits.h and bitfield.h headers (lkp)

Changes in v6:
- Link to v5: https://lore.kernel.org/all/20250304082434.834031-1-victor.liu@nxp.com/
- Add bus-width support
- rebase onto v6.18-rc1
- add review feedback (Alexander)
- driver license "GPL v2" -> "GPL" (checkpatch)
- make use of reg of-property
- fix to short Kconfig description (checkpath)
- add OF integration

---
Liu Ying (2):
      dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example
      drm/bridge: imx: Add i.MX93 parallel display format configuration support

Marco Felsch (1):
      arm64: dts: imx93: Add parallel display output nodes

 .../bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml |  78 +++++++
 arch/arm64/boot/dts/freescale/imx91_93_common.dtsi |  54 +++++
 arch/arm64/boot/dts/freescale/imx93.dtsi           |  12 ++
 drivers/gpu/drm/bridge/imx/Kconfig                 |  11 +
 drivers/gpu/drm/bridge/imx/Makefile                |   1 +
 drivers/gpu/drm/bridge/imx/imx93-pdfc.c            | 225 +++++++++++++++++++++
 6 files changed, 381 insertions(+)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20251201-v6-18-topic-imx93-parallel-display-95f9234bf6cc

Best regards,
-- 
Marco Felsch <m.felsch@pengutronix.de>

[PATCH v11 1/3] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example

[Marco Felsch]

From: Liu Ying <victor.liu@nxp.com>

i.MX93 SoC mediamix blk-ctrl contains one DISPLAY_MUX register which
configures parallel display format by using the "PARALLEL_DISP_FORMAT"
field. Document the Parallel Display Format Configuration(PDFC) subnode
and add the subnode to example.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
[m.felsch@pengutronix.de: port to v7.0-rc1]
[m.felsch@pengutronix.de: add bus-width]
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml | 78 ++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
index 34aea58094e55365a2f9c86092f637e533f954ff..d828c2e82965c7a4cd69a67136047d83c96b0a35 100644
--- a/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
@@ -40,6 +40,58 @@ properties:
     minItems: 8
     maxItems: 10
 
+  dpi-bridge:
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        enum:
+          - nxp,imx91-pdfc
+          - nxp,imx93-pdfc
+
+      ports:
+        $ref: /schemas/graph.yaml#/properties/ports
+
+        properties:
+          port@0:
+            $ref: /schemas/graph.yaml#/properties/port
+            description: Input port node to receive pixel data.
+
+          port@1:
+            $ref: /schemas/graph.yaml#/$defs/port-base
+            unevaluatedProperties: false
+            description: Output port node to downstream pixel data receivers.
+
+            properties:
+              endpoint:
+                $ref: /schemas/media/video-interfaces.yaml#
+                unevaluatedProperties: false
+
+                properties:
+                  bus-width:
+                    enum: [ 16, 18, 24 ]
+                    description:
+                      Specify the physical parallel bus width.
+
+                      This property is optional if the display bus-width
+                      matches the SoC bus-width, e.g. a 18-bit RGB666 (display)
+                      is connected and all 18-bit data lines are muxed to the
+                      parallel-output pads.
+
+                      This property must be set to 18 to cut only the LSBs
+                      instead of the MSBs in case a 24-bit RGB888 display is
+                      connected and only the lower 18-bit data lanes are muxed
+                      to the parallel-output pads.
+
+        required:
+          - port@0
+          - port@1
+
+    required:
+      - compatible
+      - ports
+
 allOf:
   - if:
       properties:
@@ -112,4 +164,30 @@ examples:
                clock-names = "apb", "axi", "nic", "disp", "cam",
                              "pxp", "lcdif", "isi", "csi", "dsi";
       #power-domain-cells = <1>;
+
+      dpi-bridge {
+        compatible = "nxp,imx93-pdfc";
+
+        ports {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          port@0 {
+            reg = <0>;
+
+            pdfc_from_lcdif: endpoint {
+              remote-endpoint = <&lcdif_to_pdfc>;
+            };
+          };
+
+          port@1 {
+            reg = <1>;
+
+            pdfc_to_panel: endpoint {
+              remote-endpoint = <&panel_from_pdfc>;
+              bus-width = <18>;
+            };
+          };
+        };
+      };
     };

-- 
2.47.3

[PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Marco Felsch]

From: Liu Ying <victor.liu@nxp.com>

NXP i.MX93 mediamix blk-ctrl contains one DISPLAY_MUX register which
configures parallel display format by using the "PARALLEL_DISP_FORMAT"
field. Add a DRM bridge driver to support the display format configuration.

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
[m.felsch@pengutronix.de: port to v7.0-rc1]
[m.felsch@pengutronix.de: add review feedback (Alexander)]
[m.felsch@pengutronix.de: fix to short Kconfig description (checkpath)]
[m.felsch@pengutronix.de: use "GPL" instead of "GPL v2" (checkpatch)]
[m.felsch@pengutronix.de: add bus-width support]
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/imx/Kconfig      |  11 ++
 drivers/gpu/drm/bridge/imx/Makefile     |   1 +
 drivers/gpu/drm/bridge/imx/imx93-pdfc.c | 225 ++++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+)

diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
index b9028a5e5a065c3237b404111d8df57e8e017f9d..47829300a4486a090514ebe914b7667a703039a9 100644
--- a/drivers/gpu/drm/bridge/imx/Kconfig
+++ b/drivers/gpu/drm/bridge/imx/Kconfig
@@ -99,4 +99,15 @@ config DRM_IMX93_MIPI_DSI
 	  Choose this to enable MIPI DSI controller found in Freescale i.MX93
 	  processor.
 
+config DRM_IMX93_PARALLEL_DISP_FMT_CONVERTER
+	tristate "NXP i.MX91/i.MX93 parallel display format converter"
+	depends on OF
+	select DRM_KMS_HELPER
+	help
+	  On i.MX93 and i.MX91 SoCs the parallel display format output is
+	  controlled via the MEDIAMIX BLK-CTRL DISPLAY_MUX.
+
+	  Say 'Y' or 'M' if you use the parallel display output path on a
+	  i.MX93 or i.MX91 SoC.
+
 endif # ARCH_MXC || COMPILE_TEST
diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
index 8d01fda25451aaa1bf51a068da18948094327116..da57fde2d813b88cdde89115ca801b4cfc69afbd 100644
--- a/drivers/gpu/drm/bridge/imx/Makefile
+++ b/drivers/gpu/drm/bridge/imx/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
 obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK) += imx8qxp-pixel-link.o
 obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK_TO_DPI) += imx8qxp-pxl2dpi.o
 obj-$(CONFIG_DRM_IMX93_MIPI_DSI) += imx93-mipi-dsi.o
+obj-$(CONFIG_DRM_IMX93_PARALLEL_DISP_FMT_CONVERTER) += imx93-pdfc.o
diff --git a/drivers/gpu/drm/bridge/imx/imx93-pdfc.c b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
new file mode 100644
index 0000000000000000000000000000000000000000..27a54424319838517b206ab7d7447538aa1489eb
--- /dev/null
+++ b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright 2022-2025 NXP
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/media-bus-format.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_bridge.h>
+
+#define IMX93_DISPLAY_MUX_REG		0x60
+#define PARALLEL_DISP_FORMAT		GENMASK(10, 8)
+#define FORMAT_RGB888_TO_RGB888		FIELD_PREP(PARALLEL_DISP_FORMAT, 0)
+#define FORMAT_RGB888_TO_RGB666		FIELD_PREP(PARALLEL_DISP_FORMAT, 1)
+#define FORMAT_RGB565_TO_RGB565		FIELD_PREP(PARALLEL_DISP_FORMAT, 2)
+
+struct imx93_pdfc {
+	struct drm_bridge bridge;
+	struct device *dev;
+	struct regmap *regmap;
+	u32 phy_bus_width;
+};
+
+static struct imx93_pdfc *bridge_to_imx93_pdfc(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct imx93_pdfc, bridge);
+}
+
+static int
+imx93_pdfc_bridge_attach(struct drm_bridge *bridge, struct drm_encoder *encoder,
+			 enum drm_bridge_attach_flags flags)
+{
+	return drm_bridge_attach(bridge->encoder, bridge->next_bridge, bridge, flags);
+}
+
+static void imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
+					    struct drm_atomic_state *state)
+{
+	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
+	const struct drm_bridge_state *bridge_state;
+	unsigned int mask = PARALLEL_DISP_FORMAT;
+	unsigned int val;
+
+	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
+
+	switch (bridge_state->output_bus_cfg.format) {
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_FIXED:
+		val = FORMAT_RGB888_TO_RGB888;
+		if (pdfc->phy_bus_width == 18) {
+			/*
+			 * Can be valid if physical bus limitation exists,
+			 * therefore use dev_dbg().
+			 */
+			dev_dbg(pdfc->dev, "Truncate two LSBs from each color\n");
+			val = FORMAT_RGB888_TO_RGB666;
+		}
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X18:
+		val = FORMAT_RGB888_TO_RGB666;
+		break;
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		val = FORMAT_RGB565_TO_RGB565;
+		break;
+	}
+
+	regmap_update_bits(pdfc->regmap, IMX93_DISPLAY_MUX_REG, mask, val);
+}
+
+/* TODO: Add YUV formats */
+static const u32 imx93_pdfc_bus_output_fmts[] = {
+	MEDIA_BUS_FMT_FIXED,
+	MEDIA_BUS_FMT_RGB888_1X24,
+	MEDIA_BUS_FMT_RGB666_1X18,
+	MEDIA_BUS_FMT_RGB565_1X16,
+};
+
+static bool imx93_pdfc_bus_output_fmt_supported(u32 fmt)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(imx93_pdfc_bus_output_fmts); i++) {
+		if (imx93_pdfc_bus_output_fmts[i] == fmt)
+			return true;
+	}
+
+	return false;
+}
+
+static u32 *
+imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+					    struct drm_bridge_state *bridge_state,
+					    struct drm_crtc_state *crtc_state,
+					    struct drm_connector_state *conn_state,
+					    u32 output_fmt,
+					    unsigned int *num_input_fmts)
+{
+	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
+	u32 *input_fmts;
+
+	*num_input_fmts = 0;
+
+	input_fmts = kmalloc_obj(*input_fmts);
+	if (!input_fmts)
+		return NULL;
+
+	*num_input_fmts = 1;
+
+	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
+		dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");
+		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+		return input_fmts;
+	}
+
+	switch (output_fmt) {
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		input_fmts[0] = output_fmt;
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X18:
+	case MEDIA_BUS_FMT_FIXED:
+		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	}
+
+	return input_fmts;
+}
+
+static int imx93_pdfc_bridge_atomic_check(struct drm_bridge *bridge,
+					  struct drm_bridge_state *bridge_state,
+					  struct drm_crtc_state *crtc_state,
+					  struct drm_connector_state *conn_state)
+{
+	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
+	u32 format = bridge_state->output_bus_cfg.format;
+
+	if (imx93_pdfc_bus_output_fmt_supported(format))
+		return 0;
+
+	dev_warn(pdfc->dev, "Unsupported output bus format: 0x%x\n", format);
+
+	return -EINVAL;
+}
+
+static const struct drm_bridge_funcs funcs = {
+	.attach			= imx93_pdfc_bridge_attach,
+	.atomic_enable		= imx93_pdfc_bridge_atomic_enable,
+	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
+	.atomic_get_input_bus_fmts	= imx93_pdfc_bridge_atomic_get_input_bus_fmts,
+	.atomic_check		= imx93_pdfc_bridge_atomic_check,
+	.atomic_reset		= drm_atomic_helper_bridge_reset,
+};
+
+static int imx93_pdfc_bridge_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct drm_bridge *next_bridge;
+	struct imx93_pdfc *pdfc;
+	struct device_node *ep;
+	int err;
+
+	pdfc = devm_drm_bridge_alloc(dev, struct imx93_pdfc, bridge, &funcs);
+	if (IS_ERR(pdfc))
+		return PTR_ERR(pdfc);
+
+	pdfc->regmap = syscon_node_to_regmap(dev->of_node->parent);
+	if (IS_ERR(pdfc->regmap))
+		return dev_err_probe(dev, PTR_ERR(pdfc->regmap),
+				     "failed to get regmap\n");
+
+	/* No limits per default */
+	pdfc->phy_bus_width = 24;
+
+	/* Get output ep (port1/endpoint) */
+	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
+	if (ep) {
+		err = of_property_read_u32(ep, "bus-width", &pdfc->phy_bus_width);
+		of_node_put(ep);
+
+		/* bus-width is optional but it must have valid data if present */
+		if (err && err != -EINVAL)
+			return dev_err_probe(dev, err,
+					     "failed to query bus-width\n");
+	}
+
+	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(next_bridge))
+		return dev_err_probe(dev, PTR_ERR(next_bridge),
+				     "failed to get next bridge\n");
+	pdfc->dev = dev;
+	pdfc->bridge.of_node = dev->of_node;
+	pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
+	pdfc->bridge.next_bridge = next_bridge;
+
+	return devm_drm_bridge_add(dev, &pdfc->bridge);
+}
+
+static const struct of_device_id imx93_pdfc_dt_ids[] = {
+	{ .compatible = "nxp,imx93-pdfc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx93_pdfc_dt_ids);
+
+static struct platform_driver imx93_pdfc_bridge_driver = {
+	.probe	= imx93_pdfc_bridge_probe,
+	.driver	= {
+		.of_match_table = imx93_pdfc_dt_ids,
+		.name = "imx93_pdfc",
+	},
+};
+module_platform_driver(imx93_pdfc_bridge_driver);
+
+MODULE_DESCRIPTION("NXP i.MX93 parallel display format configuration driver");
+MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
+MODULE_LICENSE("GPL");

-- 
2.47.3

[PATCH v11 3/3] arm64: dts: imx93: Add parallel display output nodes

[Marco Felsch]

Add required OF nodes to support the i.MX93 parallel output (DPI) path.

On the i.MX93 a single LCDIF is connected to three bridges: DPI, LVDS
LDB and the MIPI-DSI whereas the i.MX91 support only the DPI bridge.

Map endpoint@0 as DPI bridge output since the i.MX93 TRM (Figure 485.
MEDIAMIX block diagram) doesn't mention any port-number <-> bridge
combination.

Set the MEDIA-AXI and MEDIA-APB clocks to the overdrive (OD) values
since the i.MX93 and i.MX91 use the overdrive (OD) clk settings per
default.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx91_93_common.dtsi | 54 ++++++++++++++++++++++
 arch/arm64/boot/dts/freescale/imx93.dtsi           | 12 +++++
 2 files changed, 66 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi b/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
index 7958cef353766a430df5e626ff2403dc05a974b1..5a8813df6bc993d559fb0b20fc742a106bfe6315 100644
--- a/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
@@ -1122,8 +1122,62 @@ media_blk_ctrl: system-controller@4ac10000 {
 				 <&clk IMX93_CLK_MIPI_DSI_GATE>;
 			clock-names = "apb", "axi", "nic", "disp", "cam",
 				      "pxp", "lcdif", "isi", "csi", "dsi";
+			assigned-clocks = <&clk IMX93_CLK_MEDIA_AXI>,
+					  <&clk IMX93_CLK_MEDIA_APB>,
+					  <&clk IMX93_CLK_MEDIA_DISP_PIX>;
+			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1>,
+						 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
+						 <&clk IMX93_CLK_VIDEO_PLL>;
+			assigned-clock-rates = <400000000>, <133333333>;
 			#power-domain-cells = <1>;
 			status = "disabled";
+
+			dpi_bridge: dpi-bridge {
+				compatible = "nxp,imx93-pdfc";
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						dpi_from_lcdif: endpoint {
+							remote-endpoint = <&lcdif_to_dpi>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						dpi_to_panel: endpoint {
+						};
+					};
+				};
+			};
+		};
+
+		lcdif: display-controller@4ae30000 {
+			compatible = "fsl,imx93-lcdif";
+			reg = <0x4ae30000 0x23c>;
+			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX93_CLK_MEDIA_DISP_PIX>,
+				 <&clk IMX93_CLK_LCDIF_GATE>,
+				 <&clk IMX93_CLK_MEDIA_AXI>;
+			clock-names = "pix", "axi", "disp_axi";
+			power-domains = <&media_blk_ctrl IMX93_MEDIABLK_PD_LCDIF>;
+			status = "disabled";
+
+			port {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				lcdif_to_dpi: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&dpi_from_lcdif>;
+				};
+			};
 		};
 
 		usbotg1: usb@4c100000 {
diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
index 7b27012dfcb564650882dc8c40e836e797b2fda1..5436b48b30e89eb1f939b398ce1bf105abe7e34b 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -150,6 +150,18 @@ l3_cache: l3-cache {
 	};
 };
 
+&lcdif {
+	port {
+		lcdif_to_ldb: endpoint@1 {
+			reg = <1>;
+		};
+
+		lcdif_to_dsi: endpoint@2 {
+			reg = <2>;
+		};
+	};
+};
+
 &src {
 	mlmix: power-domain@44461800 {
 		compatible = "fsl,imx93-src-slice";

-- 
2.47.3

Re: [PATCH v11 3/3] arm64: dts: imx93: Add parallel display output nodes

[Marco Felsch]

Hi Frank,

On 26-03-03, Marco Felsch wrote:
> Add required OF nodes to support the i.MX93 parallel output (DPI) path.
> 
> On the i.MX93 a single LCDIF is connected to three bridges: DPI, LVDS
> LDB and the MIPI-DSI whereas the i.MX91 support only the DPI bridge.
> 
> Map endpoint@0 as DPI bridge output since the i.MX93 TRM (Figure 485.
> MEDIAMIX block diagram) doesn't mention any port-number <-> bridge
> combination.
> 
> Set the MEDIA-AXI and MEDIA-APB clocks to the overdrive (OD) values
> since the i.MX93 and i.MX91 use the overdrive (OD) clk settings per
> default.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Please ignore this particular patch since you already applied this one.

Next time we should align the apply with the rest of the patch series if
dt-bindings and driver behaviors are involved. In such case the final
integration patch should be merged at the end and not at the beginning
:)

Thanks,
  Marco

> ---
>  arch/arm64/boot/dts/freescale/imx91_93_common.dtsi | 54 ++++++++++++++++++++++
>  arch/arm64/boot/dts/freescale/imx93.dtsi           | 12 +++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi b/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> index 7958cef353766a430df5e626ff2403dc05a974b1..5a8813df6bc993d559fb0b20fc742a106bfe6315 100644
> --- a/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> @@ -1122,8 +1122,62 @@ media_blk_ctrl: system-controller@4ac10000 {
>  				 <&clk IMX93_CLK_MIPI_DSI_GATE>;
>  			clock-names = "apb", "axi", "nic", "disp", "cam",
>  				      "pxp", "lcdif", "isi", "csi", "dsi";
> +			assigned-clocks = <&clk IMX93_CLK_MEDIA_AXI>,
> +					  <&clk IMX93_CLK_MEDIA_APB>,
> +					  <&clk IMX93_CLK_MEDIA_DISP_PIX>;
> +			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1>,
> +						 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
> +						 <&clk IMX93_CLK_VIDEO_PLL>;
> +			assigned-clock-rates = <400000000>, <133333333>;
>  			#power-domain-cells = <1>;
>  			status = "disabled";
> +
> +			dpi_bridge: dpi-bridge {
> +				compatible = "nxp,imx93-pdfc";
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +
> +						dpi_from_lcdif: endpoint {
> +							remote-endpoint = <&lcdif_to_dpi>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +
> +						dpi_to_panel: endpoint {
> +						};
> +					};
> +				};
> +			};
> +		};
> +
> +		lcdif: display-controller@4ae30000 {
> +			compatible = "fsl,imx93-lcdif";
> +			reg = <0x4ae30000 0x23c>;
> +			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk IMX93_CLK_MEDIA_DISP_PIX>,
> +				 <&clk IMX93_CLK_LCDIF_GATE>,
> +				 <&clk IMX93_CLK_MEDIA_AXI>;
> +			clock-names = "pix", "axi", "disp_axi";
> +			power-domains = <&media_blk_ctrl IMX93_MEDIABLK_PD_LCDIF>;
> +			status = "disabled";
> +
> +			port {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				lcdif_to_dpi: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&dpi_from_lcdif>;
> +				};
> +			};
>  		};
>  
>  		usbotg1: usb@4c100000 {
> diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
> index 7b27012dfcb564650882dc8c40e836e797b2fda1..5436b48b30e89eb1f939b398ce1bf105abe7e34b 100644
> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> @@ -150,6 +150,18 @@ l3_cache: l3-cache {
>  	};
>  };
>  
> +&lcdif {
> +	port {
> +		lcdif_to_ldb: endpoint@1 {
> +			reg = <1>;
> +		};
> +
> +		lcdif_to_dsi: endpoint@2 {
> +			reg = <2>;
> +		};
> +	};
> +};
> +
>  &src {
>  	mlmix: power-domain@44461800 {
>  		compatible = "fsl,imx93-src-slice";
> 
> -- 
> 2.47.3
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v11 3/3] arm64: dts: imx93: Add parallel display output nodes

[Frank Li]

On Tue, Mar 03, 2026 at 11:42:08AM +0100, Marco Felsch wrote:
> Hi Frank,
>
> On 26-03-03, Marco Felsch wrote:
> > Add required OF nodes to support the i.MX93 parallel output (DPI) path.
> >
> > On the i.MX93 a single LCDIF is connected to three bridges: DPI, LVDS
> > LDB and the MIPI-DSI whereas the i.MX91 support only the DPI bridge.
> >
> > Map endpoint@0 as DPI bridge output since the i.MX93 TRM (Figure 485.
> > MEDIAMIX block diagram) doesn't mention any port-number <-> bridge
> > combination.
> >
> > Set the MEDIA-AXI and MEDIA-APB clocks to the overdrive (OD) values
> > since the i.MX93 and i.MX91 use the overdrive (OD) clk settings per
> > default.
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>
> Please ignore this particular patch since you already applied this one.

You can skip these when you post new version. post delta if need update,
I can squash to previous patch as need.

>
> Next time we should align the apply with the rest of the patch series if
> dt-bindings and driver behaviors are involved. In such case the final
> integration patch should be merged at the end and not at the beginning
> :)

You request apply at :)
https://lore.kernel.org/all/fl2br7rtcjrjj2uqxva7ai3xbvjwrrbbl2ruaoqolrccr2rd5p@z33qfx7dpavf/

Frank
>
> Thanks,
>   Marco
>
> > ---
> >  arch/arm64/boot/dts/freescale/imx91_93_common.dtsi | 54 ++++++++++++++++++++++
> >  arch/arm64/boot/dts/freescale/imx93.dtsi           | 12 +++++
> >  2 files changed, 66 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi b/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> > index 7958cef353766a430df5e626ff2403dc05a974b1..5a8813df6bc993d559fb0b20fc742a106bfe6315 100644
> > --- a/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> > @@ -1122,8 +1122,62 @@ media_blk_ctrl: system-controller@4ac10000 {
> >  				 <&clk IMX93_CLK_MIPI_DSI_GATE>;
> >  			clock-names = "apb", "axi", "nic", "disp", "cam",
> >  				      "pxp", "lcdif", "isi", "csi", "dsi";
> > +			assigned-clocks = <&clk IMX93_CLK_MEDIA_AXI>,
> > +					  <&clk IMX93_CLK_MEDIA_APB>,
> > +					  <&clk IMX93_CLK_MEDIA_DISP_PIX>;
> > +			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1>,
> > +						 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
> > +						 <&clk IMX93_CLK_VIDEO_PLL>;
> > +			assigned-clock-rates = <400000000>, <133333333>;
> >  			#power-domain-cells = <1>;
> >  			status = "disabled";
> > +
> > +			dpi_bridge: dpi-bridge {
> > +				compatible = "nxp,imx93-pdfc";
> > +				status = "disabled";
> > +
> > +				ports {
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +
> > +					port@0 {
> > +						reg = <0>;
> > +
> > +						dpi_from_lcdif: endpoint {
> > +							remote-endpoint = <&lcdif_to_dpi>;
> > +						};
> > +					};
> > +
> > +					port@1 {
> > +						reg = <1>;
> > +
> > +						dpi_to_panel: endpoint {
> > +						};
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> > +		lcdif: display-controller@4ae30000 {
> > +			compatible = "fsl,imx93-lcdif";
> > +			reg = <0x4ae30000 0x23c>;
> > +			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&clk IMX93_CLK_MEDIA_DISP_PIX>,
> > +				 <&clk IMX93_CLK_LCDIF_GATE>,
> > +				 <&clk IMX93_CLK_MEDIA_AXI>;
> > +			clock-names = "pix", "axi", "disp_axi";
> > +			power-domains = <&media_blk_ctrl IMX93_MEDIABLK_PD_LCDIF>;
> > +			status = "disabled";
> > +
> > +			port {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				lcdif_to_dpi: endpoint@0 {
> > +					reg = <0>;
> > +					remote-endpoint = <&dpi_from_lcdif>;
> > +				};
> > +			};
> >  		};
> >
> >  		usbotg1: usb@4c100000 {
> > diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
> > index 7b27012dfcb564650882dc8c40e836e797b2fda1..5436b48b30e89eb1f939b398ce1bf105abe7e34b 100644
> > --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> > @@ -150,6 +150,18 @@ l3_cache: l3-cache {
> >  	};
> >  };
> >
> > +&lcdif {
> > +	port {
> > +		lcdif_to_ldb: endpoint@1 {
> > +			reg = <1>;
> > +		};
> > +
> > +		lcdif_to_dsi: endpoint@2 {
> > +			reg = <2>;
> > +		};
> > +	};
> > +};
> > +
> >  &src {
> >  	mlmix: power-domain@44461800 {
> >  		compatible = "fsl,imx93-src-slice";
> >
> > --
> > 2.47.3
> >
>
> --
> #gernperDu
> #CallMeByMyFirstName
>
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v11 3/3] arm64: dts: imx93: Add parallel display output nodes

[Marco Felsch]

On 26-03-03, Frank Li wrote:
> On Tue, Mar 03, 2026 at 11:42:08AM +0100, Marco Felsch wrote:
> > Hi Frank,
> >
> > On 26-03-03, Marco Felsch wrote:
> > > Add required OF nodes to support the i.MX93 parallel output (DPI) path.
> > >
> > > On the i.MX93 a single LCDIF is connected to three bridges: DPI, LVDS
> > > LDB and the MIPI-DSI whereas the i.MX91 support only the DPI bridge.
> > >
> > > Map endpoint@0 as DPI bridge output since the i.MX93 TRM (Figure 485.
> > > MEDIAMIX block diagram) doesn't mention any port-number <-> bridge
> > > combination.
> > >
> > > Set the MEDIA-AXI and MEDIA-APB clocks to the overdrive (OD) values
> > > since the i.MX93 and i.MX91 use the overdrive (OD) clk settings per
> > > default.
> > >
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >
> > Please ignore this particular patch since you already applied this one.
> 
> You can skip these when you post new version. post delta if need update,
> I can squash to previous patch as need.
> 
> >
> > Next time we should align the apply with the rest of the patch series if
> > dt-bindings and driver behaviors are involved. In such case the final
> > integration patch should be merged at the end and not at the beginning
> > :)
> 
> You request apply at :)
> https://lore.kernel.org/all/fl2br7rtcjrjj2uqxva7ai3xbvjwrrbbl2ruaoqolrccr2rd5p@z33qfx7dpavf/

I meant the whole series :) Although I'm unaware which maintainer is
taken which part. Sorry for causing troubles on your site. However if
you would try to apply this on your tree, git would have told you that
nothing needs to be applied because this patch is untouched.

Regards,
  Marco

> 
> Frank
> >
> > Thanks,
> >   Marco
> >
> > > ---
> > >  arch/arm64/boot/dts/freescale/imx91_93_common.dtsi | 54 ++++++++++++++++++++++
> > >  arch/arm64/boot/dts/freescale/imx93.dtsi           | 12 +++++
> > >  2 files changed, 66 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi b/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> > > index 7958cef353766a430df5e626ff2403dc05a974b1..5a8813df6bc993d559fb0b20fc742a106bfe6315 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx91_93_common.dtsi
> > > @@ -1122,8 +1122,62 @@ media_blk_ctrl: system-controller@4ac10000 {
> > >  				 <&clk IMX93_CLK_MIPI_DSI_GATE>;
> > >  			clock-names = "apb", "axi", "nic", "disp", "cam",
> > >  				      "pxp", "lcdif", "isi", "csi", "dsi";
> > > +			assigned-clocks = <&clk IMX93_CLK_MEDIA_AXI>,
> > > +					  <&clk IMX93_CLK_MEDIA_APB>,
> > > +					  <&clk IMX93_CLK_MEDIA_DISP_PIX>;
> > > +			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1>,
> > > +						 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
> > > +						 <&clk IMX93_CLK_VIDEO_PLL>;
> > > +			assigned-clock-rates = <400000000>, <133333333>;
> > >  			#power-domain-cells = <1>;
> > >  			status = "disabled";
> > > +
> > > +			dpi_bridge: dpi-bridge {
> > > +				compatible = "nxp,imx93-pdfc";
> > > +				status = "disabled";
> > > +
> > > +				ports {
> > > +					#address-cells = <1>;
> > > +					#size-cells = <0>;
> > > +
> > > +					port@0 {
> > > +						reg = <0>;
> > > +
> > > +						dpi_from_lcdif: endpoint {
> > > +							remote-endpoint = <&lcdif_to_dpi>;
> > > +						};
> > > +					};
> > > +
> > > +					port@1 {
> > > +						reg = <1>;
> > > +
> > > +						dpi_to_panel: endpoint {
> > > +						};
> > > +					};
> > > +				};
> > > +			};
> > > +		};
> > > +
> > > +		lcdif: display-controller@4ae30000 {
> > > +			compatible = "fsl,imx93-lcdif";
> > > +			reg = <0x4ae30000 0x23c>;
> > > +			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&clk IMX93_CLK_MEDIA_DISP_PIX>,
> > > +				 <&clk IMX93_CLK_LCDIF_GATE>,
> > > +				 <&clk IMX93_CLK_MEDIA_AXI>;
> > > +			clock-names = "pix", "axi", "disp_axi";
> > > +			power-domains = <&media_blk_ctrl IMX93_MEDIABLK_PD_LCDIF>;
> > > +			status = "disabled";
> > > +
> > > +			port {
> > > +				#address-cells = <1>;
> > > +				#size-cells = <0>;
> > > +
> > > +				lcdif_to_dpi: endpoint@0 {
> > > +					reg = <0>;
> > > +					remote-endpoint = <&dpi_from_lcdif>;
> > > +				};
> > > +			};
> > >  		};
> > >
> > >  		usbotg1: usb@4c100000 {
> > > diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
> > > index 7b27012dfcb564650882dc8c40e836e797b2fda1..5436b48b30e89eb1f939b398ce1bf105abe7e34b 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> > > @@ -150,6 +150,18 @@ l3_cache: l3-cache {
> > >  	};
> > >  };
> > >
> > > +&lcdif {
> > > +	port {
> > > +		lcdif_to_ldb: endpoint@1 {
> > > +			reg = <1>;
> > > +		};
> > > +
> > > +		lcdif_to_dsi: endpoint@2 {
> > > +			reg = <2>;
> > > +		};
> > > +	};
> > > +};
> > > +
> > >  &src {
> > >  	mlmix: power-domain@44461800 {
> > >  		compatible = "fsl,imx93-src-slice";
> > >
> > > --
> > > 2.47.3
> > >
> >
> > --
> > #gernperDu
> > #CallMeByMyFirstName
> >
> > Pengutronix e.K.                           |                             |
> > Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> > 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v11 1/3] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example

[Frank Li]

From: Frank Li (AI-BOT) <frank.li@nxp.com>

AI bot review and may be useless.

This is a device tree bindings patch, not kernel C code. The YAML
schema additions look structurally sound. A few observations:

> +  dpi-bridge:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        enum:
> +          - nxp,imx91-pdfc
> +          - nxp,imx93-pdfc

The compatible strings should be documented in a separate vendor
prefix file (nxp.yaml) if not already present. Verify they exist
in Documentation/devicetree/bindings/vendor-prefixes.yaml.

> +      ports:
> +        $ref: /schemas/graph.yaml#/properties/ports
> +
> +        properties:
> +          port@0:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description: Input port node to receive pixel data.
> +
> +          port@1:
> +            $ref: /schemas/graph.yaml#/$defs/port-base
> +            unevaluatedProperties: false

The port@0 and port@1 both reference graph.yaml but use slightly
different schemas ($defs/port-base vs #/properties/port). Ensure
this inconsistency is intentional; typically both would use the
same reference unless port@1 has special constraints.

> +                properties:
> +                  bus-width:
> +                    enum: [ 16, 18, 24 ]

Line length is acceptable. The bus-width enum values are clear.

> +                      This property must be set to 18 to cut only the LSBs
> +                      instead of the MSBs in case a 24-bit RGB888 display is
> +                      connected and only the lower 18-bit data lanes are muxed
> +                      to the parallel-output pads.

Documentation is thorough and explains the non-obvious behavior
(LSB vs MSB truncation). Good.

Overall: schema is well-structured, example is complete, and
documentation is clear. No blocking issues detected.

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Frank Li]

From: Frank Li (AI-BOT) <frank.li@nxp.com>

AI bot review and may be useless.

---

> +static u32 *
> +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> +					    struct drm_bridge_state *bridge_state,
> +					    struct drm_crtc_state *crtc_state,
> +					    struct drm_connector_state *conn_state,
> +					    u32 output_fmt,
> +					    unsigned int *num_input_fmts)
> +{
> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> +	u32 *input_fmts;
> +
> +	*num_input_fmts = 0;
> +
> +	input_fmts = kmalloc_obj(*input_fmts);
> +	if (!input_fmts)
> +		return NULL;

Missing kfree(input_fmts) in error path if the switch statement
or subsequent logic fails. Consider allocating a fixed-size array
or using devm_kzalloc() instead.

> +	*num_input_fmts = 1;
> +
> +	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
> +		dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");

Line exceeds 80 characters (97 chars). Break into two lines.

> +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> +		return input_fmts;
> +	}
> +
> +	switch (output_fmt) {
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +		input_fmts[0] = output_fmt;
> +		break;
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +	case MEDIA_BUS_FMT_FIXED:
> +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> +		break;
> +	}

Switch statement lacks default case. Add default case to handle
unexpected format values explicitly.

> +static int imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
> +					    struct drm_atomic_state *state)
> +{
> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> +	const struct drm_bridge_state *bridge_state;
> +	unsigned int mask = PARALLEL_DISP_FORMAT;
> +	unsigned int val;
> +
> +	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> +
> +	switch (bridge_state->output_bus_cfg.format) {
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_FIXED:
> +		val = FORMAT_RGB888_TO_RGB888;
> +		if (pdfc->phy_bus_width == 18) {
> +			/*
> +			 * Can be valid if physical bus limitation exists,
> +			 * therefore use dev_dbg().
> +			 */
> +			dev_dbg(pdfc->dev, "Truncate two LSBs from each color\n");
> +			val = FORMAT_RGB888_TO_RGB666;
> +		}
> +		break;
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +		val = FORMAT_RGB888_TO_RGB666;
> +		

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Marco Felsch]

On 26-03-03, Frank Li wrote:
> From: Frank Li (AI-BOT) <frank.li@nxp.com>
> 
> AI bot review and may be useless.

Hi Frank,

albeit I'm very open to new technology, I would appreciate it if your
AI-BOT is used internally first till you're convinced that it reports
real issues instead of false-positives.

Regards,
  Marco

> > +static u32 *
> > +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> > +					    struct drm_bridge_state *bridge_state,
> > +					    struct drm_crtc_state *crtc_state,
> > +					    struct drm_connector_state *conn_state,
> > +					    u32 output_fmt,
> > +					    unsigned int *num_input_fmts)
> > +{
> > +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> > +	u32 *input_fmts;
> > +
> > +	*num_input_fmts = 0;
> > +
> > +	input_fmts = kmalloc_obj(*input_fmts);
> > +	if (!input_fmts)
> > +		return NULL;
> 
> Missing kfree(input_fmts) in error path if the switch statement
> or subsequent logic fails. Consider allocating a fixed-size array
> or using devm_kzalloc() instead.
> 
> > +	*num_input_fmts = 1;
> > +
> > +	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
> > +		dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");
> 
> Line exceeds 80 characters (97 chars). Break into two lines.
> 
> > +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > +		return input_fmts;
> > +	}
> > +
> > +	switch (output_fmt) {
> > +	case MEDIA_BUS_FMT_RGB888_1X24:
> > +	case MEDIA_BUS_FMT_RGB565_1X16:
> > +		input_fmts[0] = output_fmt;
> > +		break;
> > +	case MEDIA_BUS_FMT_RGB666_1X18:
> > +	case MEDIA_BUS_FMT_FIXED:
> > +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > +		break;
> > +	}
> 
> Switch statement lacks default case. Add default case to handle
> unexpected format values explicitly.
> 
> > +static int imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
> > +					    struct drm_atomic_state *state)
> > +{
> > +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> > +	const struct drm_bridge_state *bridge_state;
> > +	unsigned int mask = PARALLEL_DISP_FORMAT;
> > +	unsigned int val;
> > +
> > +	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> > +
> > +	switch (bridge_state->output_bus_cfg.format) {
> > +	case MEDIA_BUS_FMT_RGB888_1X24:
> > +	case MEDIA_BUS_FMT_FIXED:
> > +		val = FORMAT_RGB888_TO_RGB888;
> > +		if (pdfc->phy_bus_width == 18) {
> > +			/*
> > +			 * Can be valid if physical bus limitation exists,
> > +			 * therefore use dev_dbg().
> > +			 */
> > +			dev_dbg(pdfc->dev, "Truncate two LSBs from each color\n");
> > +			val = FORMAT_RGB888_TO_RGB666;
> > +		}
> > +		break;
> > +	case MEDIA_BUS_FMT_RGB666_1X18:
> > +		val = FORMAT_RGB888_TO_RGB666;
> > +		
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Frank Li]

On Tue, Mar 03, 2026 at 10:22:02PM +0100, Marco Felsch wrote:
> On 26-03-03, Frank Li wrote:
> > From: Frank Li (AI-BOT) <frank.li@nxp.com>
> >
> > AI bot review and may be useless.
>
> Hi Frank,
>
> albeit I'm very open to new technology, I would appreciate it if your
> AI-BOT is used internally first till you're convinced that it reports
> real issues instead of false-positives.

I tested 22 patches, which sent to imx mail list only. Need test script.
Impact should be limited.

Frank

>
> Regards,
>   Marco
>
> > > +static u32 *
> > > +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> > > +					    struct drm_bridge_state *bridge_state,
> > > +					    struct drm_crtc_state *crtc_state,
> > > +					    struct drm_connector_state *conn_state,
> > > +					    u32 output_fmt,
> > > +					    unsigned int *num_input_fmts)
> > > +{
> > > +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> > > +	u32 *input_fmts;
> > > +
> > > +	*num_input_fmts = 0;
> > > +
> > > +	input_fmts = kmalloc_obj(*input_fmts);
> > > +	if (!input_fmts)
> > > +		return NULL;
> >
> > Missing kfree(input_fmts) in error path if the switch statement
> > or subsequent logic fails. Consider allocating a fixed-size array
> > or using devm_kzalloc() instead.
> >
> > > +	*num_input_fmts = 1;
> > > +
> > > +	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
> > > +		dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");
> >
> > Line exceeds 80 characters (97 chars). Break into two lines.
> >
> > > +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > > +		return input_fmts;
> > > +	}
> > > +
> > > +	switch (output_fmt) {
> > > +	case MEDIA_BUS_FMT_RGB888_1X24:
> > > +	case MEDIA_BUS_FMT_RGB565_1X16:
> > > +		input_fmts[0] = output_fmt;
> > > +		break;
> > > +	case MEDIA_BUS_FMT_RGB666_1X18:
> > > +	case MEDIA_BUS_FMT_FIXED:
> > > +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > > +		break;
> > > +	}
> >
> > Switch statement lacks default case. Add default case to handle
> > unexpected format values explicitly.
> >
> > > +static int imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
> > > +					    struct drm_atomic_state *state)
> > > +{
> > > +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> > > +	const struct drm_bridge_state *bridge_state;
> > > +	unsigned int mask = PARALLEL_DISP_FORMAT;
> > > +	unsigned int val;
> > > +
> > > +	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> > > +
> > > +	switch (bridge_state->output_bus_cfg.format) {
> > > +	case MEDIA_BUS_FMT_RGB888_1X24:
> > > +	case MEDIA_BUS_FMT_FIXED:
> > > +		val = FORMAT_RGB888_TO_RGB888;
> > > +		if (pdfc->phy_bus_width == 18) {
> > > +			/*
> > > +			 * Can be valid if physical bus limitation exists,
> > > +			 * therefore use dev_dbg().
> > > +			 */
> > > +			dev_dbg(pdfc->dev, "Truncate two LSBs from each color\n");
> > > +			val = FORMAT_RGB888_TO_RGB666;
> > > +		}
> > > +		break;
> > > +	case MEDIA_BUS_FMT_RGB666_1X18:
> > > +		val = FORMAT_RGB888_TO_RGB666;
> > > +
> >
>
> --
> #gernperDu
> #CallMeByMyFirstName
>
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Frank Li]

On Tue, Mar 03, 2026 at 10:22:02PM +0100, Marco Felsch wrote:
> On 26-03-03, Frank Li wrote:
> > From: Frank Li (AI-BOT) <frank.li@nxp.com>
> >
> > AI bot review and may be useless.
>
> Hi Frank,
>
> albeit I'm very open to new technology, I would appreciate it if your
> AI-BOT is used internally first till you're convinced that it reports
> real issues instead of false-positives.
>
> Regards,
>   Marco
>
> > > +static u32 *
> > > +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> > > +					    struct drm_bridge_state *bridge_state,
> > > +					    struct drm_crtc_state *crtc_state,
> > > +					    struct drm_connector_state *conn_state,
> > > +					    u32 output_fmt,
> > > +					    unsigned int *num_input_fmts)
> > > +{
> > > +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> > > +	u32 *input_fmts;
> > > +
> > > +	*num_input_fmts = 0;
> > > +
> > > +	input_fmts = kmalloc_obj(*input_fmts);
> > > +	if (!input_fmts)
> > > +		return NULL;
> >

> +     input_fmts = kmalloc_obj(*input_fmts);

AI:

Actually, this looks incorrect. kmalloc_obj() allocates based on the type
of the pointer argument. Here you're passing *input_fmts (a u32), not
input_fmts (a u32 *). Should be:

+       input_fmts = kmalloc_array(*num_input_fmts, sizeof(*input_fmts),
+                                  GFP_KERNEL);

suppose you use kmalloc_objs()

Frank

> > Missing kfree(input_fmts) in error path if the switch statement
> > or subsequent logic fails. Consider allocating a fixed-size array
> > or using devm_kzalloc() instead.
> >
> > > +	*num_input_fmts = 1;
> > > +
> > > +	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
> > > +		dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");
> >
> > Line exceeds 80 characters (97 chars). Break into two lines.
> >
> > > +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > > +		return input_fmts;
> > > +	}
> > > +
> > > +	switch (output_fmt) {
> > > +	case MEDIA_BUS_FMT_RGB888_1X24:
> > > +	case MEDIA_BUS_FMT_RGB565_1X16:
> > > +		input_fmts[0] = output_fmt;
> > > +		break;
> > > +	case MEDIA_BUS_FMT_RGB666_1X18:
> > > +	case MEDIA_BUS_FMT_FIXED:
> > > +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > > +		break;
> > > +	}
> >
> > Switch statement lacks default case. Add default case to handle
> > unexpected format values explicitly.
> >
> > > +static int imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
> > > +					    struct drm_atomic_state *state)
> > > +{
> > > +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> > > +	const struct drm_bridge_state *bridge_state;
> > > +	unsigned int mask = PARALLEL_DISP_FORMAT;
> > > +	unsigned int val;
> > > +
> > > +	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> > > +
> > > +	switch (bridge_state->output_bus_cfg.format) {
> > > +	case MEDIA_BUS_FMT_RGB888_1X24:
> > > +	case MEDIA_BUS_FMT_FIXED:
> > > +		val = FORMAT_RGB888_TO_RGB888;
> > > +		if (pdfc->phy_bus_width == 18) {
> > > +			/*
> > > +			 * Can be valid if physical bus limitation exists,
> > > +			 * therefore use dev_dbg().
> > > +			 */
> > > +			dev_dbg(pdfc->dev, "Truncate two LSBs from each color\n");
> > > +			val = FORMAT_RGB888_TO_RGB666;
> > > +		}
> > > +		break;
> > > +	case MEDIA_BUS_FMT_RGB666_1X18:
> > > +		val = FORMAT_RGB888_TO_RGB666;
> > > +
> >
>
> --
> #gernperDu
> #CallMeByMyFirstName
>
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Liu Ying]

On Tue, Mar 03, 2026 at 05:20:09PM -0500, Frank Li wrote:
> On Tue, Mar 03, 2026 at 10:22:02PM +0100, Marco Felsch wrote:
>> On 26-03-03, Frank Li wrote:
>>> From: Frank Li (AI-BOT) <frank.li@nxp.com>
>>>
>>> AI bot review and may be useless.
>>
>> Hi Frank,
>>
>> albeit I'm very open to new technology, I would appreciate it if your
>> AI-BOT is used internally first till you're convinced that it reports
>> real issues instead of false-positives.
>>
>> Regards,
>>   Marco
>>
>>>> +static u32 *
>>>> +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>>>> +					    struct drm_bridge_state *bridge_state,
>>>> +					    struct drm_crtc_state *crtc_state,
>>>> +					    struct drm_connector_state *conn_state,
>>>> +					    u32 output_fmt,
>>>> +					    unsigned int *num_input_fmts)
>>>> +{
>>>> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
>>>> +	u32 *input_fmts;
>>>> +
>>>> +	*num_input_fmts = 0;
>>>> +
>>>> +	input_fmts = kmalloc_obj(*input_fmts);
>>>> +	if (!input_fmts)
>>>> +		return NULL;
>>>
> 
>> +     input_fmts = kmalloc_obj(*input_fmts);
> 
> AI:
> 
> Actually, this looks incorrect. kmalloc_obj() allocates based on the type
> of the pointer argument. Here you're passing *input_fmts (a u32), not
> input_fmts (a u32 *).

This comment is false.  Per kmalloc_obj()'s kerneldoc, it's first argument
is "Variable or type to allocate".  In this particular case, the argument
should be "*input_fmts (a u32)".

/**
 * kmalloc_obj - Allocate a single instance of the given type
 * @VAR_OR_TYPE: Variable or type to allocate.
 * @GFP: GFP flags for the allocation.
 *
 * Returns: newly allocated pointer to a @VAR_OR_TYPE on success, or NULL
 * on failure.
 */
#define kmalloc_obj(VAR_OR_TYPE, ...) \
        __alloc_objs(kmalloc, default_gfp(__VA_ARGS__), typeof(VAR_OR_TYPE), 1)

> Should be:
> 
> +       input_fmts = kmalloc_array(*num_input_fmts, sizeof(*input_fmts),
> +                                  GFP_KERNEL);

No.  "input_fmts = kmalloc_obj(*input_fmts);" is just fine.

> 
> suppose you use kmalloc_objs()

No. We should use kmalloc_obj(), not kmalloc_objs().

> 
> Frank
> 
>>> Missing kfree(input_fmts) in error path if the switch statement
>>> or subsequent logic fails. Consider allocating a fixed-size array
>>> or using devm_kzalloc() instead.
>>>
>>>> +	*num_input_fmts = 1;
>>>> +
>>>> +	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
>>>> +		dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");
>>>
>>> Line exceeds 80 characters (97 chars). Break into two lines.
>>>
>>>> +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>>> +		return input_fmts;
>>>> +	}
>>>> +
>>>> +	switch (output_fmt) {
>>>> +	case MEDIA_BUS_FMT_RGB888_1X24:
>>>> +	case MEDIA_BUS_FMT_RGB565_1X16:
>>>> +		input_fmts[0] = output_fmt;
>>>> +		break;
>>>> +	case MEDIA_BUS_FMT_RGB666_1X18:
>>>> +	case MEDIA_BUS_FMT_FIXED:
>>>> +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>>> +		break;
>>>> +	}
>>>
>>> Switch statement lacks default case. Add default case to handle
>>> unexpected format values explicitly.
>>>
>>>> +static int imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
>>>> +					    struct drm_atomic_state *state)
>>>> +{
>>>> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
>>>> +	const struct drm_bridge_state *bridge_state;
>>>> +	unsigned int mask = PARALLEL_DISP_FORMAT;
>>>> +	unsigned int val;
>>>> +
>>>> +	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
>>>> +
>>>> +	switch (bridge_state->output_bus_cfg.format) {
>>>> +	case MEDIA_BUS_FMT_RGB888_1X24:
>>>> +	case MEDIA_BUS_FMT_FIXED:
>>>> +		val = FORMAT_RGB888_TO_RGB888;
>>>> +		if (pdfc->phy_bus_width == 18) {
>>>> +			/*
>>>> +			 * Can be valid if physical bus limitation exists,
>>>> +			 * therefore use dev_dbg().
>>>> +			 */
>>>> +			dev_dbg(pdfc->dev, "Truncate two LSBs from each color\n");
>>>> +			val = FORMAT_RGB888_TO_RGB666;
>>>> +		}
>>>> +		break;
>>>> +	case MEDIA_BUS_FMT_RGB666_1X18:
>>>> +		val = FORMAT_RGB888_TO_RGB666;
>>>> +
>>>
>>
>> --
>> #gernperDu
>> #CallMeByMyFirstName
>>
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

-- 
Regards,
Liu Ying

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Liu Ying]

On Tue, Mar 03, 2026 at 11:34:27AM +0100, Marco Felsch wrote:
> From: Liu Ying <victor.liu@nxp.com>
> 
> NXP i.MX93 mediamix blk-ctrl contains one DISPLAY_MUX register which
> configures parallel display format by using the "PARALLEL_DISP_FORMAT"
> field. Add a DRM bridge driver to support the display format configuration.
> 
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> [m.felsch@pengutronix.de: port to v7.0-rc1]
> [m.felsch@pengutronix.de: add review feedback (Alexander)]
> [m.felsch@pengutronix.de: fix to short Kconfig description (checkpath)]
> [m.felsch@pengutronix.de: use "GPL" instead of "GPL v2" (checkpatch)]
> [m.felsch@pengutronix.de: add bus-width support]
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/gpu/drm/bridge/imx/Kconfig      |  11 ++
>  drivers/gpu/drm/bridge/imx/Makefile     |   1 +
>  drivers/gpu/drm/bridge/imx/imx93-pdfc.c | 225 ++++++++++++++++++++++++++++++++
>  3 files changed, 237 insertions(+)

Reviewed-by: Liu Ying <victor.liu@nxp.com>

Thanks!

I'll wait for a week or so to apply this if no objections or someone else
applies it sooner.

-- 
Regards,
Liu Ying

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Frank Li]

On Wed, Mar 04, 2026 at 11:29:03AM +0800, Liu Ying wrote:
> On Tue, Mar 03, 2026 at 05:20:09PM -0500, Frank Li wrote:
> > On Tue, Mar 03, 2026 at 10:22:02PM +0100, Marco Felsch wrote:
> >> On 26-03-03, Frank Li wrote:
> >>> From: Frank Li (AI-BOT) <frank.li@nxp.com>
> >>>
> >>> AI bot review and may be useless.
> >>
> >> Hi Frank,
> >>
> >> albeit I'm very open to new technology, I would appreciate it if your
> >> AI-BOT is used internally first till you're convinced that it reports
> >> real issues instead of false-positives.
> >>
> >> Regards,
> >>   Marco
> >>
> >>>> +static u32 *
> >>>> +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> >>>> +					    struct drm_bridge_state *bridge_state,
> >>>> +					    struct drm_crtc_state *crtc_state,
> >>>> +					    struct drm_connector_state *conn_state,
> >>>> +					    u32 output_fmt,
> >>>> +					    unsigned int *num_input_fmts)
> >>>> +{
> >>>> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> >>>> +	u32 *input_fmts;
> >>>> +
> >>>> +	*num_input_fmts = 0;
> >>>> +
> >>>> +	input_fmts = kmalloc_obj(*input_fmts);
> >>>> +	if (!input_fmts)
> >>>> +		return NULL;
> >>>
> >
> >> +     input_fmts = kmalloc_obj(*input_fmts);
> >
> > AI:
> >
> > Actually, this looks incorrect. kmalloc_obj() allocates based on the type
> > of the pointer argument. Here you're passing *input_fmts (a u32), not
> > input_fmts (a u32 *).
>
> This comment is false.  Per kmalloc_obj()'s kerneldoc, it's first argument
> is "Variable or type to allocate".  In this particular case, the argument
> should be "*input_fmts (a u32)".

Yes, I wrongly think it is num_input_fmts and return array point for multi
formats, actually it is just 1 items for format.

imx93_pdfc_bridge_atomic_get_input_bus_fmts() is quite likely return many
format array especially has "s" after fmt.

Frank

>
> /**
>  * kmalloc_obj - Allocate a single instance of the given type
>  * @VAR_OR_TYPE: Variable or type to allocate.
>  * @GFP: GFP flags for the allocation.
>  *
>  * Returns: newly allocated pointer to a @VAR_OR_TYPE on success, or NULL
>  * on failure.
>  */
> #define kmalloc_obj(VAR_OR_TYPE, ...) \
>         __alloc_objs(kmalloc, default_gfp(__VA_ARGS__), typeof(VAR_OR_TYPE), 1)
>
> > Should be:
> >
> > +       input_fmts = kmalloc_array(*num_input_fmts, sizeof(*input_fmts),
> > +                                  GFP_KERNEL);
>
> No.  "input_fmts = kmalloc_obj(*input_fmts);" is just fine.
>
> >
> > suppose you use kmalloc_objs()
>
> No. We should use kmalloc_obj(), not kmalloc_objs().
>
> >
> > Frank
> >
> >>> Missing kfree(input_fmts) in error path if the switch statement
> >>> or subsequent logic fails. Consider allocating a fixed-size array
> >>> or using devm_kzalloc() instead.
> >>>
> >>>> +	*num_input_fmts = 1;
> >>>> +
> >>>> +	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
> >>>> +		dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");
> >>>
> >>> Line exceeds 80 characters (97 chars). Break into two lines.
> >>>
> >>>> +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> >>>> +		return input_fmts;
> >>>> +	}
> >>>> +
> >>>> +	switch (output_fmt) {
> >>>> +	case MEDIA_BUS_FMT_RGB888_1X24:
> >>>> +	case MEDIA_BUS_FMT_RGB565_1X16:
> >>>> +		input_fmts[0] = output_fmt;
> >>>> +		break;
> >>>> +	case MEDIA_BUS_FMT_RGB666_1X18:
> >>>> +	case MEDIA_BUS_FMT_FIXED:
> >>>> +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> >>>> +		break;
> >>>> +	}
> >>>
> >>> Switch statement lacks default case. Add default case to handle
> >>> unexpected format values explicitly.
> >>>
> >>>> +static int imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
> >>>> +					    struct drm_atomic_state *state)
> >>>> +{
> >>>> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> >>>> +	const struct drm_bridge_state *bridge_state;
> >>>> +	unsigned int mask = PARALLEL_DISP_FORMAT;
> >>>> +	unsigned int val;
> >>>> +
> >>>> +	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> >>>> +
> >>>> +	switch (bridge_state->output_bus_cfg.format) {
> >>>> +	case MEDIA_BUS_FMT_RGB888_1X24:
> >>>> +	case MEDIA_BUS_FMT_FIXED:
> >>>> +		val = FORMAT_RGB888_TO_RGB888;
> >>>> +		if (pdfc->phy_bus_width == 18) {
> >>>> +			/*
> >>>> +			 * Can be valid if physical bus limitation exists,
> >>>> +			 * therefore use dev_dbg().
> >>>> +			 */
> >>>> +			dev_dbg(pdfc->dev, "Truncate two LSBs from each color\n");
> >>>> +			val = FORMAT_RGB888_TO_RGB666;
> >>>> +		}
> >>>> +		break;
> >>>> +	case MEDIA_BUS_FMT_RGB666_1X18:
> >>>> +		val = FORMAT_RGB888_TO_RGB666;
> >>>> +
> >>>
> >>
> >> --
> >> #gernperDu
> >> #CallMeByMyFirstName
> >>
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |
>
> --
> Regards,
> Liu Ying

Re: [PATCH v11 1/3] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example

[Krzysztof Kozlowski]

On 03/03/2026 22:00, Frank Li wrote:
> From: Frank Li (AI-BOT) <frank.li@nxp.com>
> 
> AI bot review and may be useless.

Then don't post it. Please post useful reviews.

...

> 
> Overall: schema is well-structured, example is complete, and
> documentation is clear. No blocking issues detected.

What is the point of telling this? Just unnecessary traffic and mailbox
pollution.


Best regards,
Krzysztof

Re: [PATCH v11 1/3] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example

[Krzysztof Kozlowski]

On 03/03/2026 22:00, Frank Li wrote:
> From: Frank Li (AI-BOT) <frank.li@nxp.com>
> 
> AI bot review and may be useless.
> 
> This is a device tree bindings patch, not kernel C code. The YAML
> schema additions look structurally sound. A few observations:
> 
>> +  dpi-bridge:
>> +    type: object
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - nxp,imx91-pdfc
>> +          - nxp,imx93-pdfc
> 
> The compatible strings should be documented in a separate vendor
> prefix file (nxp.yaml) if not already present. Verify they exist
> in Documentation/devicetree/bindings/vendor-prefixes.yaml.

What?!?!?

This is nonsense.

Also, you are responding to an already reviewed patch.

Best regards,
Krzysztof

Re: [PATCH v11 1/3] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example

[Frank Li]

On Wed, Mar 04, 2026 at 04:49:27PM +0100, Krzysztof Kozlowski wrote:
> On 03/03/2026 22:00, Frank Li wrote:
> > From: Frank Li (AI-BOT) <frank.li@nxp.com>
> >
> > AI bot review and may be useless.
> >
> > This is a device tree bindings patch, not kernel C code. The YAML
> > schema additions look structurally sound. A few observations:
> >
> >> +  dpi-bridge:
> >> +    type: object
> >> +    additionalProperties: false
> >> +
> >> +    properties:
> >> +      compatible:
> >> +        enum:
> >> +          - nxp,imx91-pdfc
> >> +          - nxp,imx93-pdfc
> >
> > The compatible strings should be documented in a separate vendor
> > prefix file (nxp.yaml) if not already present. Verify they exist
> > in Documentation/devicetree/bindings/vendor-prefixes.yaml.
>
> What?!?!?
>
> This is nonsense.
>
> Also, you are responding to an already reviewed patch.

Sorry, I tested 22 patches of from imx and sent out intermediate result.

Frank
>
> Best regards,
> Krzysztof

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Alexander Stein]

[-- Attachment #1: Type: text/plain, Size: 10304 bytes --]

Am Dienstag, 3. März 2026, 11:34:27 CET schrieb Marco Felsch:
> From: Liu Ying <victor.liu@nxp.com>
> 
> NXP i.MX93 mediamix blk-ctrl contains one DISPLAY_MUX register which
> configures parallel display format by using the "PARALLEL_DISP_FORMAT"
> field. Add a DRM bridge driver to support the display format configuration.
> 
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> [m.felsch@pengutronix.de: port to v7.0-rc1]
> [m.felsch@pengutronix.de: add review feedback (Alexander)]
> [m.felsch@pengutronix.de: fix to short Kconfig description (checkpath)]
> [m.felsch@pengutronix.de: use "GPL" instead of "GPL v2" (checkpatch)]
> [m.felsch@pengutronix.de: add bus-width support]
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>

on TQMa93xxCA on MBa91xxCA

> ---
>  drivers/gpu/drm/bridge/imx/Kconfig      |  11 ++
>  drivers/gpu/drm/bridge/imx/Makefile     |   1 +
>  drivers/gpu/drm/bridge/imx/imx93-pdfc.c | 225 ++++++++++++++++++++++++++++++++
>  3 files changed, 237 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> index b9028a5e5a065c3237b404111d8df57e8e017f9d..47829300a4486a090514ebe914b7667a703039a9 100644
> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> @@ -99,4 +99,15 @@ config DRM_IMX93_MIPI_DSI
>  	  Choose this to enable MIPI DSI controller found in Freescale i.MX93
>  	  processor.
>  
> +config DRM_IMX93_PARALLEL_DISP_FMT_CONVERTER
> +	tristate "NXP i.MX91/i.MX93 parallel display format converter"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	help
> +	  On i.MX93 and i.MX91 SoCs the parallel display format output is
> +	  controlled via the MEDIAMIX BLK-CTRL DISPLAY_MUX.
> +
> +	  Say 'Y' or 'M' if you use the parallel display output path on a
> +	  i.MX93 or i.MX91 SoC.
> +
>  endif # ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
> index 8d01fda25451aaa1bf51a068da18948094327116..da57fde2d813b88cdde89115ca801b4cfc69afbd 100644
> --- a/drivers/gpu/drm/bridge/imx/Makefile
> +++ b/drivers/gpu/drm/bridge/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
>  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK) += imx8qxp-pixel-link.o
>  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK_TO_DPI) += imx8qxp-pxl2dpi.o
>  obj-$(CONFIG_DRM_IMX93_MIPI_DSI) += imx93-mipi-dsi.o
> +obj-$(CONFIG_DRM_IMX93_PARALLEL_DISP_FMT_CONVERTER) += imx93-pdfc.o
> diff --git a/drivers/gpu/drm/bridge/imx/imx93-pdfc.c b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..27a54424319838517b206ab7d7447538aa1489eb
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
> @@ -0,0 +1,225 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2022-2025 NXP
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <drm/drm_atomic_state_helper.h>
> +#include <drm/drm_bridge.h>
> +
> +#define IMX93_DISPLAY_MUX_REG		0x60
> +#define PARALLEL_DISP_FORMAT		GENMASK(10, 8)
> +#define FORMAT_RGB888_TO_RGB888		FIELD_PREP(PARALLEL_DISP_FORMAT, 0)
> +#define FORMAT_RGB888_TO_RGB666		FIELD_PREP(PARALLEL_DISP_FORMAT, 1)
> +#define FORMAT_RGB565_TO_RGB565		FIELD_PREP(PARALLEL_DISP_FORMAT, 2)
> +
> +struct imx93_pdfc {
> +	struct drm_bridge bridge;
> +	struct device *dev;
> +	struct regmap *regmap;
> +	u32 phy_bus_width;
> +};
> +
> +static struct imx93_pdfc *bridge_to_imx93_pdfc(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct imx93_pdfc, bridge);
> +}
> +
> +static int
> +imx93_pdfc_bridge_attach(struct drm_bridge *bridge, struct drm_encoder *encoder,
> +			 enum drm_bridge_attach_flags flags)
> +{
> +	return drm_bridge_attach(bridge->encoder, bridge->next_bridge, bridge, flags);
> +}
> +
> +static void imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
> +					    struct drm_atomic_state *state)
> +{
> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> +	const struct drm_bridge_state *bridge_state;
> +	unsigned int mask = PARALLEL_DISP_FORMAT;
> +	unsigned int val;
> +
> +	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> +
> +	switch (bridge_state->output_bus_cfg.format) {
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_FIXED:
> +		val = FORMAT_RGB888_TO_RGB888;
> +		if (pdfc->phy_bus_width == 18) {
> +			/*
> +			 * Can be valid if physical bus limitation exists,
> +			 * therefore use dev_dbg().
> +			 */
> +			dev_dbg(pdfc->dev, "Truncate two LSBs from each color\n");
> +			val = FORMAT_RGB888_TO_RGB666;
> +		}
> +		break;
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +		val = FORMAT_RGB888_TO_RGB666;
> +		break;
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +		val = FORMAT_RGB565_TO_RGB565;
> +		break;
> +	}
> +
> +	regmap_update_bits(pdfc->regmap, IMX93_DISPLAY_MUX_REG, mask, val);
> +}
> +
> +/* TODO: Add YUV formats */
> +static const u32 imx93_pdfc_bus_output_fmts[] = {
> +	MEDIA_BUS_FMT_FIXED,
> +	MEDIA_BUS_FMT_RGB888_1X24,
> +	MEDIA_BUS_FMT_RGB666_1X18,
> +	MEDIA_BUS_FMT_RGB565_1X16,
> +};
> +
> +static bool imx93_pdfc_bus_output_fmt_supported(u32 fmt)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(imx93_pdfc_bus_output_fmts); i++) {
> +		if (imx93_pdfc_bus_output_fmts[i] == fmt)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static u32 *
> +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> +					    struct drm_bridge_state *bridge_state,
> +					    struct drm_crtc_state *crtc_state,
> +					    struct drm_connector_state *conn_state,
> +					    u32 output_fmt,
> +					    unsigned int *num_input_fmts)
> +{
> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> +	u32 *input_fmts;
> +
> +	*num_input_fmts = 0;
> +
> +	input_fmts = kmalloc_obj(*input_fmts);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	*num_input_fmts = 1;
> +
> +	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
> +		dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");
> +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> +		return input_fmts;
> +	}
> +
> +	switch (output_fmt) {
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +		input_fmts[0] = output_fmt;
> +		break;
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +	case MEDIA_BUS_FMT_FIXED:
> +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> +		break;
> +	}
> +
> +	return input_fmts;
> +}
> +
> +static int imx93_pdfc_bridge_atomic_check(struct drm_bridge *bridge,
> +					  struct drm_bridge_state *bridge_state,
> +					  struct drm_crtc_state *crtc_state,
> +					  struct drm_connector_state *conn_state)
> +{
> +	struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> +	u32 format = bridge_state->output_bus_cfg.format;
> +
> +	if (imx93_pdfc_bus_output_fmt_supported(format))
> +		return 0;
> +
> +	dev_warn(pdfc->dev, "Unsupported output bus format: 0x%x\n", format);
> +
> +	return -EINVAL;
> +}
> +
> +static const struct drm_bridge_funcs funcs = {
> +	.attach			= imx93_pdfc_bridge_attach,
> +	.atomic_enable		= imx93_pdfc_bridge_atomic_enable,
> +	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
> +	.atomic_get_input_bus_fmts	= imx93_pdfc_bridge_atomic_get_input_bus_fmts,
> +	.atomic_check		= imx93_pdfc_bridge_atomic_check,
> +	.atomic_reset		= drm_atomic_helper_bridge_reset,
> +};
> +
> +static int imx93_pdfc_bridge_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct drm_bridge *next_bridge;
> +	struct imx93_pdfc *pdfc;
> +	struct device_node *ep;
> +	int err;
> +
> +	pdfc = devm_drm_bridge_alloc(dev, struct imx93_pdfc, bridge, &funcs);
> +	if (IS_ERR(pdfc))
> +		return PTR_ERR(pdfc);
> +
> +	pdfc->regmap = syscon_node_to_regmap(dev->of_node->parent);
> +	if (IS_ERR(pdfc->regmap))
> +		return dev_err_probe(dev, PTR_ERR(pdfc->regmap),
> +				     "failed to get regmap\n");
> +
> +	/* No limits per default */
> +	pdfc->phy_bus_width = 24;
> +
> +	/* Get output ep (port1/endpoint) */
> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (ep) {
> +		err = of_property_read_u32(ep, "bus-width", &pdfc->phy_bus_width);
> +		of_node_put(ep);
> +
> +		/* bus-width is optional but it must have valid data if present */
> +		if (err && err != -EINVAL)
> +			return dev_err_probe(dev, err,
> +					     "failed to query bus-width\n");
> +	}
> +
> +	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	if (IS_ERR(next_bridge))
> +		return dev_err_probe(dev, PTR_ERR(next_bridge),
> +				     "failed to get next bridge\n");
> +	pdfc->dev = dev;
> +	pdfc->bridge.of_node = dev->of_node;
> +	pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
> +	pdfc->bridge.next_bridge = next_bridge;
> +
> +	return devm_drm_bridge_add(dev, &pdfc->bridge);
> +}
> +
> +static const struct of_device_id imx93_pdfc_dt_ids[] = {
> +	{ .compatible = "nxp,imx93-pdfc", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx93_pdfc_dt_ids);
> +
> +static struct platform_driver imx93_pdfc_bridge_driver = {
> +	.probe	= imx93_pdfc_bridge_probe,
> +	.driver	= {
> +		.of_match_table = imx93_pdfc_dt_ids,
> +		.name = "imx93_pdfc",
> +	},
> +};
> +module_platform_driver(imx93_pdfc_bridge_driver);
> +
> +MODULE_DESCRIPTION("NXP i.MX93 parallel display format configuration driver");
> +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
> +MODULE_LICENSE("GPL");
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Liu Ying]

Hi Marco, Luca,

On Tue, Mar 03, 2026 at 11:34:27AM +0100, Marco Felsch wrote:

[...]

> +	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	if (IS_ERR(next_bridge))
> +		return dev_err_probe(dev, PTR_ERR(next_bridge),
> +				     "failed to get next bridge\n");
> +	pdfc->dev = dev;
> +	pdfc->bridge.of_node = dev->of_node;
> +	pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
> +	pdfc->bridge.next_bridge = next_bridge;

When I was reviewing another patch[1], I was aware of the necessity of
calling drm_bridge_get() for next_bridge to balance the next bridge's
refcount put from __drm_bridge_free() for this bridge.  I'd be good if
Luca may confirm this is correct.  Sorry for bringing this up late.

[1] https://lore.kernel.org/all/5edd40f3-f83e-4a1b-99d4-1e595fc0e4cb@nxp.com/

-- 
Regards,
Liu Ying

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Luca Ceresoli]

Hi Liu, Marco,

On Tue Mar 10, 2026 at 3:57 AM CET, Liu Ying wrote:
> Hi Marco, Luca,
>
> On Tue, Mar 03, 2026 at 11:34:27AM +0100, Marco Felsch wrote:
>
> [...]
>
>> +	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
>> +	if (IS_ERR(next_bridge))
>> +		return dev_err_probe(dev, PTR_ERR(next_bridge),
>> +				     "failed to get next bridge\n");
>> +	pdfc->dev = dev;
>> +	pdfc->bridge.of_node = dev->of_node;
>> +	pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
>> +	pdfc->bridge.next_bridge = next_bridge;
>
> When I was reviewing another patch[1], I was aware of the necessity of
> calling drm_bridge_get() for next_bridge to balance the next bridge's
> refcount put from __drm_bridge_free() for this bridge.  I'd be good if
> Luca may confirm this is correct.  Sorry for bringing this up late.

Indeed you have a good point.

After re-checking devm_drm_of_get_bridge(), as I wrote on the other thread
you pointed to, you should call drm_bridge_get():

-	pdfc->bridge.next_bridge = next_bridge;
+	pdfc->bridge.next_bridge = drm_bridge_get(next_bridge);

Marco, you can keep my R-by if you resend with just this change.

Sorry about the confusion here.

As mention on the other thread, devm_drm_of_get_bridge() is unable to
support bridge hotplug. So it should be deprecated, but as of now there is
no alternative.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Liu Ying]

On Tue, Mar 10, 2026 at 12:53:07PM +0100, Luca Ceresoli wrote:
> Hi Liu, Marco,
> 
> On Tue Mar 10, 2026 at 3:57 AM CET, Liu Ying wrote:
>> Hi Marco, Luca,
>>
>> On Tue, Mar 03, 2026 at 11:34:27AM +0100, Marco Felsch wrote:
>>
>> [...]
>>
>>> +	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
>>> +	if (IS_ERR(next_bridge))
>>> +		return dev_err_probe(dev, PTR_ERR(next_bridge),
>>> +				     "failed to get next bridge\n");
>>> +	pdfc->dev = dev;
>>> +	pdfc->bridge.of_node = dev->of_node;
>>> +	pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
>>> +	pdfc->bridge.next_bridge = next_bridge;
>>
>> When I was reviewing another patch[1], I was aware of the necessity of
>> calling drm_bridge_get() for next_bridge to balance the next bridge's
>> refcount put from __drm_bridge_free() for this bridge.  I'd be good if
>> Luca may confirm this is correct.  Sorry for bringing this up late.
> 
> Indeed you have a good point.
> 
> After re-checking devm_drm_of_get_bridge(), as I wrote on the other thread
> you pointed to, you should call drm_bridge_get():

Thanks for the confirmation.

> 
> -	pdfc->bridge.next_bridge = next_bridge;
> +	pdfc->bridge.next_bridge = drm_bridge_get(next_bridge);
> 
> Marco, you can keep my R-by if you resend with just this change.

Marco, care to resend with just this change?

> 
> Sorry about the confusion here.
> 
> As mention on the other thread, devm_drm_of_get_bridge() is unable to
> support bridge hotplug. So it should be deprecated, but as of now there is
> no alternative.
> 
> Luca
> 
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com/

-- 
Regards,
Liu Ying

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Marco Felsch]

Hi Liu, Luca,

sorry for the delayed response, I was at the EW26.

On 26-03-10, Luca Ceresoli wrote:
> Hi Liu, Marco,
> 
> On Tue Mar 10, 2026 at 3:57 AM CET, Liu Ying wrote:
> > Hi Marco, Luca,
> >
> > On Tue, Mar 03, 2026 at 11:34:27AM +0100, Marco Felsch wrote:
> >
> > [...]
> >
> >> +	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> >> +	if (IS_ERR(next_bridge))
> >> +		return dev_err_probe(dev, PTR_ERR(next_bridge),
> >> +				     "failed to get next bridge\n");
> >> +	pdfc->dev = dev;
> >> +	pdfc->bridge.of_node = dev->of_node;
> >> +	pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
> >> +	pdfc->bridge.next_bridge = next_bridge;
> >
> > When I was reviewing another patch[1], I was aware of the necessity of
> > calling drm_bridge_get() for next_bridge to balance the next bridge's
> > refcount put from __drm_bridge_free() for this bridge.  I'd be good if
> > Luca may confirm this is correct.  Sorry for bringing this up late.
> 
> Indeed you have a good point.

At which stage did you faced this issue? During driver probe, because of
EPROBE_DEFER?

That's the reason for having the local next_bridge variable since I
faced with the same issue. In other words this driver is correct and
it's on purpose to not assign it directly. Albeit I could/should have
added a comment.

> After re-checking devm_drm_of_get_bridge(), as I wrote on the other thread
> you pointed to, you should call drm_bridge_get():
> 
> -	pdfc->bridge.next_bridge = next_bridge;
> +	pdfc->bridge.next_bridge = drm_bridge_get(next_bridge);
>
> Marco, you can keep my R-by if you resend with just this change.
> 
> Sorry about the confusion here.
> 
> As mention on the other thread, devm_drm_of_get_bridge() is unable to
> support bridge hotplug. So it should be deprecated, but as of now there is
> no alternative.

Sorry I need a bit more context. What's the issue? How can I trigger the
issue? Why is bridge hotplug required at this stage? Why is only this
bridge affecte by the hotplug issue?

Regards,
  Marco



> 
> Luca
> 
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Liu Ying]

On Wed, Mar 11, 2026 at 08:45:25PM +0100, Marco Felsch wrote:
> Hi Liu, Luca,

Hi,

> 
> sorry for the delayed response, I was at the EW26.

Np at all, it's good to go out to meet people sometimes :)

> 
> On 26-03-10, Luca Ceresoli wrote:
>> Hi Liu, Marco,
>>
>> On Tue Mar 10, 2026 at 3:57 AM CET, Liu Ying wrote:
>>> Hi Marco, Luca,
>>>
>>> On Tue, Mar 03, 2026 at 11:34:27AM +0100, Marco Felsch wrote:
>>>
>>> [...]
>>>
>>>> +	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
>>>> +	if (IS_ERR(next_bridge))
>>>> +		return dev_err_probe(dev, PTR_ERR(next_bridge),
>>>> +				     "failed to get next bridge\n");
>>>> +	pdfc->dev = dev;
>>>> +	pdfc->bridge.of_node = dev->of_node;
>>>> +	pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
>>>> +	pdfc->bridge.next_bridge = next_bridge;
>>>
>>> When I was reviewing another patch[1], I was aware of the necessity of
>>> calling drm_bridge_get() for next_bridge to balance the next bridge's
>>> refcount put from __drm_bridge_free() for this bridge.  I'd be good if
>>> Luca may confirm this is correct.  Sorry for bringing this up late.
>>
>> Indeed you have a good point.
> 
> At which stage did you faced this issue? During driver probe, because of
> EPROBE_DEFER?

I just want to balance the get/put for the next bridge's refcount by
calling drm_bridge_get() for next_bridge, as I said above.  There could
be some way to trigger some particular Use-After-Free issues if you don't
do that.  I did take some time today to try to trigger UAF, but no luck,
though I found a potential bug in encoder_bridges_show() and generated a
fix[1](Cc'ed Marco) when I played with debugfs to read the refcounts.

My idea to trigger UAF is to remove imx_lcdif/imx93_pdfc/simple_panel
modules when doing pageflips.  Dunno if EPROBE_DEFER may trigger UAF.

[1] https://lore.kernel.org/all/20260312-drm-misc-next-2026-03-05-fix-encoder-bridges-refcount-v1-1-b9ba3d844732@nxp.com/

> 
> That's the reason for having the local next_bridge variable since I
> faced with the same issue. In other words this driver is correct and
> it's on purpose to not assign it directly. Albeit I could/should have
> added a comment.

What's the issue your faced?  How would using next_bridge variable impact?
Without knowing these info, I presume that you still need to call
drm_bridge_get() for next_bridge, since it's kind of obvious if you take
a look at __drm_bridge_free().

> 
>> After re-checking devm_drm_of_get_bridge(), as I wrote on the other thread
>> you pointed to, you should call drm_bridge_get():
>>
>> -	pdfc->bridge.next_bridge = next_bridge;
>> +	pdfc->bridge.next_bridge = drm_bridge_get(next_bridge);
>>
>> Marco, you can keep my R-by if you resend with just this change.
>>
>> Sorry about the confusion here.
>>
>> As mention on the other thread, devm_drm_of_get_bridge() is unable to
>> support bridge hotplug. So it should be deprecated, but as of now there is
>> no alternative.
> 
> Sorry I need a bit more context. What's the issue? How can I trigger the
> issue? Why is bridge hotplug required at this stage? Why is only this
> bridge affecte by the hotplug issue?

Maybe, Luca can comment here.

> 
> Regards,
>   Marco
> 
> 
> 
>>
>> Luca
>>
>> --
>> Luca Ceresoli, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com/
>>
> 

-- 
Regards,
Liu Ying

Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Luca Ceresoli]

Hello Marco, Liu,

On Thu Mar 12, 2026 at 10:15 AM CET, Liu Ying wrote:
> On Wed, Mar 11, 2026 at 08:45:25PM +0100, Marco Felsch wrote:
>> Hi Liu, Luca,
>
> Hi,
>
>>
>> sorry for the delayed response, I was at the EW26.
>
> Np at all, it's good to go out to meet people sometimes :)
>
>>
>> On 26-03-10, Luca Ceresoli wrote:
>>> Hi Liu, Marco,
>>>
>>> On Tue Mar 10, 2026 at 3:57 AM CET, Liu Ying wrote:
>>>> Hi Marco, Luca,
>>>>
>>>> On Tue, Mar 03, 2026 at 11:34:27AM +0100, Marco Felsch wrote:
>>>>
>>>> [...]
>>>>
>>>>> +	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
>>>>> +	if (IS_ERR(next_bridge))
>>>>> +		return dev_err_probe(dev, PTR_ERR(next_bridge),
>>>>> +				     "failed to get next bridge\n");
>>>>> +	pdfc->dev = dev;
>>>>> +	pdfc->bridge.of_node = dev->of_node;
>>>>> +	pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
>>>>> +	pdfc->bridge.next_bridge = next_bridge;
>>>>
>>>> When I was reviewing another patch[1], I was aware of the necessity of
>>>> calling drm_bridge_get() for next_bridge to balance the next bridge's
>>>> refcount put from __drm_bridge_free() for this bridge.  I'd be good if
>>>> Luca may confirm this is correct.  Sorry for bringing this up late.
>>>
>>> Indeed you have a good point.
>>
>> At which stage did you faced this issue? During driver probe, because of
>> EPROBE_DEFER?
>
> I just want to balance the get/put for the next bridge's refcount by
> calling drm_bridge_get() for next_bridge, as I said above.  There could
> be some way to trigger some particular Use-After-Free issues if you don't
> do that.  I did take some time today to try to trigger UAF, but no luck,
> though I found a potential bug in encoder_bridges_show() and generated a
> fix[1](Cc'ed Marco) when I played with debugfs to read the refcounts.

The whole need for refcounting DRM bridges exists if such bridges can be
removed when other parts of the kernel still have a pointer (= a reference)
to them. This is not the case with current DRM because you cannot remove
some bridges without removing the whole pipeline (card). However I am
working since a long time to suppor hardware where the last part of the
display pipeline is hot-pluggable, causing DRM bridges to be added and
removed at runtime without removal of the initial part of the pipeline
(CRCT, encoder and 0 or more bridges).

See these links for more background info:

 * ELCE 2025:
   - https://www.youtube.com/watch?v=C8dEQ4OzMnc
   - https://bootlin.com/pub/conferences/2025/elce/ceresoli-hotplug-status.pdf
 * Lots of links in the slides about previous activity
 * LPC 2024 has a general description fo the use case:
   - https://lpc.events/event/18/contributions/1696/

> My idea to trigger UAF is to remove imx_lcdif/imx93_pdfc/simple_panel
> modules when doing pageflips.  Dunno if EPROBE_DEFER may trigger UAF.

Perhaps there is some possible UAF on probe errors or deferrals, but the
typical case is when removing the final part of a pipeline, including 1+
bridges. This was discussed in detail in [0], but here's an excerpt with
the typical UAF scenario with hotplug:

  1. pipeline: encoder --> bridge A --> bridge B --> bridge C
  2. encoder takes a reference to bridge B
     using devm_drm_of_find_bridge() or other means
  3. bridge B takes a next_bridge reference to bridge C
     using devm_drm_of_find_bridge()
  4. encoder calls (bridge B)->foo(), which in turns references
     next_bridge, e.g.:

       b_foo() {
           bar(b->next_bridge);
       }

If bridges B and C are removed, bridge C can be freed but B is still
allocated because the encoder holds a ref. So when step 4 happens, 'b->c'
would be a use-after-free (or NULL deref if b.remove cleared it, which is
just as bad).

[0] https://lore.kernel.org/all/DEH2CVQV21Z2.25PJBAQAKFJSG@bootlin.com/

>> That's the reason for having the local next_bridge variable since I
>> faced with the same issue. In other words this driver is correct and
>> it's on purpose to not assign it directly. Albeit I could/should have
>> added a comment.
>
> What's the issue your faced?  How would using next_bridge variable impact?
> Without knowing these info, I presume that you still need to call
> drm_bridge_get() for next_bridge, since it's kind of obvious if you take
> a look at __drm_bridge_free().

Exactly. The idea in a nutshell is: every pointer to a drm_bridge stored
somewhere is a reference to a bridge. So:

 * When you take a reference (= you set a pointer to the address of a
   bridge) you need to call drm_bridge_get() to increment the
   refcount.
 * When you clear a reference (set it to NULL) or the pointer becomes
   unreachable (the device goes away, the struct holding it is not
   reachable anymore, etc) you need to call drm_bridge_put() to decrement
   the refcount.

This way the struct drm_bridge (or, typically, the private driver struct
embedding it) will be freed only when the refcount goes to 0, avoiding UAF.

I have been converting most of the accessors returning a drm_bridge pointer
in order to drm_bridge_get() the bridge before returning it. A few random
examples:

 - https://lore.kernel.org/lkml/20250620-drm-bridge-alloc-getput-drm-bridge-c-v9-0-ca53372c9a84@bootlin.com/
 - https://lore.kernel.org/lkml/20250709-drm-bridge-alloc-getput-drm_bridge_get_prev_bridge-v1-0-34ba6f395aaa@bootlin.com/
 - https://lore.kernel.org/lkml/20260109-drm-bridge-alloc-getput-drm_of_find_bridge-2-v2-4-8bad3ef90b9f@bootlin.com/

I hope this clarifies the general refcount topic. Now to *_of_get_bridge().

>>> After re-checking devm_drm_of_get_bridge(), as I wrote on the other thread
>>> you pointed to, you should call drm_bridge_get():
>>>
>>> -	pdfc->bridge.next_bridge = next_bridge;
>>> +	pdfc->bridge.next_bridge = drm_bridge_get(next_bridge);
>>>
>>> Marco, you can keep my R-by if you resend with just this change.
>>>
>>> Sorry about the confusion here.
>>>
>>> As mention on the other thread, devm_drm_of_get_bridge() is unable to
>>> support bridge hotplug. So it should be deprecated, but as of now there is
>>> no alternative.
>>
>> Sorry I need a bit more context. What's the issue?

The issue with devm_drm_of_get_bridge(MYDEV, ...) (similarly for other
*_of_get_bridge() functions) is it can return one of:

 A) an existing bridge, returned by drm_of_find_panel_or_bridge() in the
    &bridge parameter
 B) a panel_bridge it creates on the fly calling
    devm_drm_panel_bridge_add(MYDEV) based on the panel returned by
    drm_of_find_panel_or_bridge() in the &panel parameter

The caller of devm_drm_of_get_bridge() gets a drm_bridge pointer but
doesn't know whether case A or B happened.

This is not a problem if the entire card is torn down at once because
devm/drmm will take care or the removal. Even easier if the card is not
torn down at all.

Now imagine adding hotplug so you can keep MYDEV present but remove any
bridges after MYDEV and the panel. The devm action added by
devm_drm_of_get_bridge() won't trigger (because MYDEV is not going
away). The caller of devm_drm_of_get_bridge(), i.e. the MYDEV driver,
should remove the panel_bridge in case B but it has no way to know whether
case A or B happened.

So, this is tricky. The good news for you is you're not supposed to fix
it. The *_of_get_bridge() API is just unable to handle hotplug, that will
be fixed at some point. For now just ensure you get a reference for every
bridge pointer you store, and to put it when that reference goes away.

>> How can I trigger the
>> issue?

See the example above.

>> Why is bridge hotplug required at this stage?

Because there is hot-pluggable hardware we want to support, see the ELCE
and even better the LPC links above.

>> Why is only this
>> bridge affecte by the hotplug issue?

It's not only this driver. Hotplug can potentially be used on any hardware,
so I'm working to implement it in a general way in the DRM common code, but
all drviers need to at least do the refcounting on their side. I'm trying
to catch all patches involving bridge pointers and check whether they do
refcounting right. This is just one of many patches I have commented about
in the past months.

I hope this clarified a bit.

Best regards,
Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com