[PATCH v2 0/3] Add VGA support for Apalis iMX6

[PATCH v2 0/3] Add VGA support for Apalis iMX6

[Leonardo Costa]

This series adds a device tree overlay that enables the VGA output on Apalis
iMX6, which is driven from a parallel RGB interface passing through a
non-programmable video DAC.

The video DAC is a simple resistor network gated by an active-low
data-enable signal. For the bridge to work properly, this series also
changes the following in the dt-bindings and simple-bridge driver:

- dt-bindings: Add the existing data-enable-active property from
  video-interfaces.yaml to the simple-bridge binding. The property is
  read from the bridge's input endpoint. This allows a dumb-vga-dac to
  specify a data-enable signal polarity.

- simple-bridge: Read the data-enable-active property and fill out the
  bridge's input bus flags accordingly. The property is ignored if the
  bridge already has its own timings struct defined, to not interfere
  with its existing settings.

This change depends on another patch, and should be applied after it:

- https://lore.kernel.org/all/20261002-v1-simple-bridge-atomic-check-v1-1-ef5d8332b078@toradex.com/T/#u

  Adds the atomic_check function to the simple-bridge driver, which
  allows the data-enable bus flag to be propagated through the DRM
  pipeline. This is necessary for the VGA to work correctly on the
  Apalis iMX6.

Changes in v2:
  - Introduce the data-enable-active property.
  - Describe the VGA bridge on the Apalis iMX6 as a dumb-vga-dac using
    the data-enable-active property to specify the DE signal polarity,
    instead of using a dedicated compatible.
  - Link to v1: https://lore.kernel.org/all/as-heYw7PKLPm3yH@SMW015318/T/#u

Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
---
Leonardo Costa (3):
      dt-bindings: display: bridge: simple: Add data-enable-active to dumb-vga-dac
      drm/bridge: simple-bridge: Support data-enable-active endpoint property
      ARM: dts: imx6q-apalis: Add VGA overlay

 .../bindings/display/bridge/simple-bridge.yaml     | 28 +++++++-
 arch/arm/boot/dts/nxp/imx/Makefile                 | 21 ++++++
 .../boot/dts/nxp/imx/imx6q-apalis-eval-vga.dtso    | 80 ++++++++++++++++++++++
 drivers/gpu/drm/bridge/simple-bridge.c             | 21 +++++-
 4 files changed, 148 insertions(+), 2 deletions(-)
---
base-commit: f33c2a109da4a527ed3103acb649a88e88cf368c
change-id: 20261014-v2-apalis-imx6-enable-vga-450f5777a476

Best regards,
-- 
Leonardo Costa <leonardo.costa@toradex.com>

[PATCH v2 1/3] dt-bindings: display: bridge: simple: Add data-enable-active to dumb-vga-dac

[Leonardo Costa]

From: Leonardo Costa <leonardo.costa@toradex.com>

Some simple VGA DACs may include a data-enable input that must be driven
at a specific polarity. Add the data-enable-active property from
video-interfaces to the input endpoint to describe that polarity.

Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
---
 .../bindings/display/bridge/simple-bridge.yaml     | 28 +++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
index 7636c24906ba..b1b23d41245d 100644
--- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
@@ -43,9 +43,18 @@ properties:
 
     properties:
       port@0:
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
         description: The bridge input
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml
+            unevaluatedProperties: false
+
+            properties:
+              data-enable-active: true
+
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description: The bridge output
@@ -65,6 +74,23 @@ required:
   - compatible
   - ports
 
+allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: dumb-vga-dac
+    then:
+      properties:
+        ports:
+          properties:
+            port@0:
+              properties:
+                endpoint:
+                  properties:
+                    data-enable-active: false
+
 additionalProperties: false
 
 examples:

-- 
2.43.0

[PATCH v2 2/3] drm/bridge: simple-bridge: Support data-enable-active endpoint property

[Leonardo Costa]

From: Leonardo Costa <leonardo.costa@toradex.com>

Some simple bridges, such as a resistor ladder DAC, may include a
data-enable input that must be driven at a specific polarity.

Read the data-enable-active property from the input endpoint and fill
the bridge's input bus flags accordingly, so the upstream encoder drives
DE with the required polarity. The property is applied only if the
bridge doesn't provide its own timings struct.

Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
---
 drivers/gpu/drm/bridge/simple-bridge.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
index 848c2df2a276..c5301295658a 100644
--- a/drivers/gpu/drm/bridge/simple-bridge.c
+++ b/drivers/gpu/drm/bridge/simple-bridge.c
@@ -183,6 +183,9 @@ static int simple_bridge_probe(struct platform_device *pdev)
 {
 	struct simple_bridge *sbridge;
 	struct device_node *remote;
+	struct device_node *ep;
+	bool de_present;
+	u32 de;
 
 	sbridge = devm_drm_bridge_alloc(&pdev->dev, struct simple_bridge,
 					bridge, &simple_bridge_bridge_funcs);
@@ -220,10 +223,26 @@ static int simple_bridge_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(sbridge->enable),
 				     "Unable to retrieve enable GPIO\n");
 
-	/* Register the bridge. */
 	sbridge->bridge.of_node = pdev->dev.of_node;
 	sbridge->bridge.timings = sbridge->info->timings;
 
+	ep = of_graph_get_endpoint_by_regs(pdev->dev.of_node, 0, -1);
+	de_present = !of_property_read_u32(ep, "data-enable-active", &de);
+	of_node_put(ep);
+
+	if (de_present && !sbridge->bridge.timings) {
+		struct drm_bridge_timings *timings;
+
+		timings = devm_kzalloc(&pdev->dev, sizeof(*timings), GFP_KERNEL);
+		if (!timings)
+			return -ENOMEM;
+
+		timings->input_bus_flags = de ? DRM_BUS_FLAG_DE_HIGH :
+						DRM_BUS_FLAG_DE_LOW;
+		sbridge->bridge.timings = timings;
+	}
+
+	/* Register the bridge. */
 	return devm_drm_bridge_add(&pdev->dev, &sbridge->bridge);
 }
 

-- 
2.43.0

[PATCH v2 3/3] ARM: dts: imx6q-apalis: Add VGA overlay

[Leonardo Costa]

From: Leonardo Costa <leonardo.costa@toradex.com>

Add a device tree overlay for the VGA output available on the Apalis
Evaluation Board, which is driven from a parallel RGB interface passing
through a non-programmable video DAC to a VGA connector, without VESA
DDC I2C signals.

The non-programmable video DAC consists of a simple resistor ladder that
converts the digital 16-bit RGB signal to VGA. The VGA output is gated by
an active-low data-enable signal.

The overlay is also combined with the various Apalis Evaluation Board
device trees to provide ready-to-use DTBs.

Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
---
 arch/arm/boot/dts/nxp/imx/Makefile                 | 21 ++++++
 .../boot/dts/nxp/imx/imx6q-apalis-eval-vga.dtso    | 80 ++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/arch/arm/boot/dts/nxp/imx/Makefile b/arch/arm/boot/dts/nxp/imx/Makefile
index b5fad2ba764c..9c218260fdcb 100644
--- a/arch/arm/boot/dts/nxp/imx/Makefile
+++ b/arch/arm/boot/dts/nxp/imx/Makefile
@@ -60,6 +60,22 @@ dtb-$(CONFIG_SOC_IMX53) += \
 imx53-qsb-hdmi-dtbs := imx53-qsb.dtb imx53-qsb-hdmi.dtbo
 imx53-qsrb-hdmi-dtbs := imx53-qsrb.dtb imx53-qsb-hdmi.dtbo
 
+imx6q-apalis-eval-v1.2-vga-dtbs := \
+	imx6q-apalis-eval-v1.2.dtb \
+	imx6q-apalis-eval-vga.dtbo
+
+imx6q-apalis-eval-vga-dtbs := \
+	imx6q-apalis-eval.dtb \
+	imx6q-apalis-eval-vga.dtbo
+
+imx6q-apalis-v1.2-eval-v1.2-vga-dtbs := \
+	imx6q-apalis-v1.2-eval-v1.2.dtb \
+	imx6q-apalis-eval-vga.dtbo
+
+imx6q-apalis-v1.2-eval-vga-dtbs := \
+	imx6q-apalis-v1.2-eval.dtb \
+	imx6q-apalis-eval-vga.dtbo
+
 imx6qdl-dhcom-pdk2-overlay-497-200-x12-dtbs := \
 	imx6q-dhcom-pdk2.dtb \
 	imx6qdl-dhcom-pdk2-overlay-497-200-x12.dtbo
@@ -186,11 +202,16 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
 	imx6dl-yapp4-ursa.dtb \
 	imx6q-apalis-eval.dtb \
 	imx6q-apalis-eval-v1.2.dtb \
+	imx6q-apalis-eval-v1.2-vga.dtb \
+	imx6q-apalis-eval-vga.dtb \
+	imx6q-apalis-eval-vga.dtbo \
 	imx6q-apalis-ixora.dtb \
 	imx6q-apalis-ixora-v1.1.dtb \
 	imx6q-apalis-ixora-v1.2.dtb \
 	imx6q-apalis-v1.2-eval.dtb \
 	imx6q-apalis-v1.2-eval-v1.2.dtb \
+	imx6q-apalis-v1.2-eval-v1.2-vga.dtb \
+	imx6q-apalis-v1.2-eval-vga.dtb \
 	imx6q-apalis-v1.2-ixora.dtb \
 	imx6q-apalis-v1.2-ixora-v1.1.dtb \
 	imx6q-apalis-v1.2-ixora-v1.2.dtb \
diff --git a/arch/arm/boot/dts/nxp/imx/imx6q-apalis-eval-vga.dtso b/arch/arm/boot/dts/nxp/imx/imx6q-apalis-eval-vga.dtso
new file mode 100644
index 000000000000..991c2b7c2777
--- /dev/null
+++ b/arch/arm/boot/dts/nxp/imx/imx6q-apalis-eval-vga.dtso
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Copyright (c) Toradex
+ *
+ * VGA output connected to a parallel RGB LCD interface through a
+ * non-programmable video DAC with a VGA connector.
+ *
+ * The VGA output is available on the Apalis Evaluation Board without VESA DDC
+ * I2C signals.
+ */
+
+/dts-v1/;
+/plugin/;
+
+&{/} {
+	disp1 {
+		compatible = "fsl,imx-parallel-display";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu2_vdac>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		interface-pix-fmt = "rgb565";
+
+		port@0 {
+			reg = <0>;
+
+			vga_display_in: endpoint {
+				remote-endpoint = <&ipu2_di0_disp0>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			vga_display_out: endpoint {
+				remote-endpoint = <&vga_enc_in>;
+			};
+		};
+	};
+
+	vga-encoder {
+		compatible = "dumb-vga-dac";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				vga_enc_in: endpoint {
+					data-enable-active = <0>;
+					remote-endpoint = <&vga_display_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				vga_enc_out: endpoint {
+					remote-endpoint = <&vga_connector_in>;
+				};
+			};
+		};
+	};
+
+	vga-connector {
+		compatible = "vga-connector";
+
+		port {
+			vga_connector_in: endpoint {
+				remote-endpoint = <&vga_enc_out>;
+			};
+		};
+	};
+};
+
+&ipu2_di0_disp0 {
+	remote-endpoint = <&vga_display_in>;
+};

-- 
2.43.0

Re: [PATCH v2 3/3] ARM: dts: imx6q-apalis: Add VGA overlay

[Frank Li]

On Mon, Oct 19, 2026 at 07:17:16AM -0300, Leonardo Costa wrote:
> From: Leonardo Costa <leonardo.costa@toradex.com>
>
> Add a device tree overlay for the VGA output available on the Apalis
> Evaluation Board, which is driven from a parallel RGB interface passing
> through a non-programmable video DAC to a VGA connector, without VESA
> DDC I2C signals.
>
> The non-programmable video DAC consists of a simple resistor ladder that
> converts the digital 16-bit RGB signal to VGA. The VGA output is gated by
> an active-low data-enable signal.
>
> The overlay is also combined with the various Apalis Evaluation Board
> device trees to provide ready-to-use DTBs.
>
> Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> ---
>  arch/arm/boot/dts/nxp/imx/Makefile                 | 21 ++++++
>  .../boot/dts/nxp/imx/imx6q-apalis-eval-vga.dtso    | 80 ++++++++++++++++++++++

what's difference with

https://lore.kernel.org/imx/20261019-colibri-imx6-overlays-v1-5-e4d028727382@toradex.com/T/#u

Is it possible share one dtso also?

Frank

Re: [PATCH v2 1/3] dt-bindings: display: bridge: simple: Add data-enable-active to dumb-vga-dac

[Frank Li]

On Mon, Oct 19, 2026 at 07:17:14AM -0300, Leonardo Costa wrote:
> From: Leonardo Costa <leonardo.costa@toradex.com>
>
> Some simple VGA DACs may include a data-enable input that must be driven
> at a specific polarity. Add the data-enable-active property from
> video-interfaces to the input endpoint to describe that polarity.
>
> Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> ---
>  .../bindings/display/bridge/simple-bridge.yaml     | 28 +++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
> index 7636c24906ba..b1b23d41245d 100644
> --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
> @@ -43,9 +43,18 @@ properties:
>
>      properties:
>        port@0:
> -        $ref: /schemas/graph.yaml#/properties/port
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
>          description: The bridge input
>
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-enable-active: true
> +

suppose needn't it because unevaluatedProperties: false, which allow all
property in video-interfaces.yaml.

If you only want to enable one property, should be
additionalProperties: false

but I think unevaluatedProperties: false make sense and needn't below if
branch.

Frank

>        port@1:
>          $ref: /schemas/graph.yaml#/properties/port
>          description: The bridge output
> @@ -65,6 +74,23 @@ required:
>    - compatible
>    - ports
>
> +allOf:
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              const: dumb-vga-dac
> +    then:
> +      properties:
> +        ports:
> +          properties:
> +            port@0:
> +              properties:
> +                endpoint:
> +                  properties:
> +                    data-enable-active: false
> +
>  additionalProperties: false
>
>  examples:
>
> --
> 2.43.0
>
>

Re: [PATCH v2 2/3] drm/bridge: simple-bridge: Support data-enable-active endpoint property

[Frank Li]

On Mon, Oct 19, 2026 at 07:17:15AM -0300, Leonardo Costa wrote:
> From: Leonardo Costa <leonardo.costa@toradex.com>
>
> Some simple bridges, such as a resistor ladder DAC, may include a
> data-enable input that must be driven at a specific polarity.
>
> Read the data-enable-active property from the input endpoint and fill
> the bridge's input bus flags accordingly, so the upstream encoder drives
> DE with the required polarity. The property is applied only if the
> bridge doesn't provide its own timings struct.
>
> Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/gpu/drm/bridge/simple-bridge.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
> index 848c2df2a276..c5301295658a 100644
> --- a/drivers/gpu/drm/bridge/simple-bridge.c
> +++ b/drivers/gpu/drm/bridge/simple-bridge.c
> @@ -183,6 +183,9 @@ static int simple_bridge_probe(struct platform_device *pdev)
>  {
>  	struct simple_bridge *sbridge;
>  	struct device_node *remote;
> +	struct device_node *ep;
> +	bool de_present;
> +	u32 de;
>
>  	sbridge = devm_drm_bridge_alloc(&pdev->dev, struct simple_bridge,
>  					bridge, &simple_bridge_bridge_funcs);
> @@ -220,10 +223,26 @@ static int simple_bridge_probe(struct platform_device *pdev)
>  		return dev_err_probe(&pdev->dev, PTR_ERR(sbridge->enable),
>  				     "Unable to retrieve enable GPIO\n");
>
> -	/* Register the bridge. */
>  	sbridge->bridge.of_node = pdev->dev.of_node;
>  	sbridge->bridge.timings = sbridge->info->timings;
>
> +	ep = of_graph_get_endpoint_by_regs(pdev->dev.of_node, 0, -1);
> +	de_present = !of_property_read_u32(ep, "data-enable-active", &de);
> +	of_node_put(ep);
> +
> +	if (de_present && !sbridge->bridge.timings) {
> +		struct drm_bridge_timings *timings;
> +
> +		timings = devm_kzalloc(&pdev->dev, sizeof(*timings), GFP_KERNEL);
> +		if (!timings)
> +			return -ENOMEM;
> +
> +		timings->input_bus_flags = de ? DRM_BUS_FLAG_DE_HIGH :
> +						DRM_BUS_FLAG_DE_LOW;
> +		sbridge->bridge.timings = timings;
> +	}
> +
> +	/* Register the bridge. */
>  	return devm_drm_bridge_add(&pdev->dev, &sbridge->bridge);
>  }
>
>
> --
> 2.43.0
>
>

Re: [PATCH v2 3/3] ARM: dts: imx6q-apalis: Add VGA overlay

[Leonardo Costa]

On Mon, Oct 19, 2026 at 02:37:41PM -0500, Frank Li wrote:
> On Mon, Oct 19, 2026 at 07:17:16AM -0300, Leonardo Costa wrote:
> > From: Leonardo Costa <leonardo.costa@toradex.com>
> >
> > Add a device tree overlay for the VGA output available on the Apalis
> > Evaluation Board, which is driven from a parallel RGB interface passing
> > through a non-programmable video DAC to a VGA connector, without VESA
> > DDC I2C signals.
> >
> > The non-programmable video DAC consists of a simple resistor ladder that
> > converts the digital 16-bit RGB signal to VGA. The VGA output is gated by
> > an active-low data-enable signal.
> >
> > The overlay is also combined with the various Apalis Evaluation Board
> > device trees to provide ready-to-use DTBs.
> >
> > Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> > ---
> >  arch/arm/boot/dts/nxp/imx/Makefile                 | 21 ++++++
> >  .../boot/dts/nxp/imx/imx6q-apalis-eval-vga.dtso    | 80 ++++++++++++++++++++++
> 
> what's difference with
> 
> https://lore.kernel.org/imx/20261019-colibri-imx6-overlays-v1-5-e4d028727382@toradex.com/T/#u
> 
> Is it possible share one dtso also?
> 
> Frank

Hi Frank,

The Apalis iMX6 one needs to set up the connection between the IPU the
LCD interface (endpoints ipu2_di0_disp0 and vga_display_in), and the
Colibri iMX6 one uses a DVI connector instead of a VGA one. Most of all,
the bridge on the Colibri iMX6 doesn't have any control signals, while
this one on the Apalis iMX6 has an active-low data-enable signal
specified on the input endpoint. Because of all of these, the overlay
needs to be different between these modules.

Best regards,
Leonardo

Re: [PATCH v2 2/3] drm/bridge: simple-bridge: Support data-enable-active endpoint property

[Leonardo Costa]

On Mon, Oct 19, 2026 at 02:50:36PM -0500, Frank Li wrote:
> On Mon, Oct 19, 2026 at 07:17:15AM -0300, Leonardo Costa wrote:
> > From: Leonardo Costa <leonardo.costa@toradex.com>
> >
> > Some simple bridges, such as a resistor ladder DAC, may include a
> > data-enable input that must be driven at a specific polarity.
> >
> > Read the data-enable-active property from the input endpoint and fill
> > the bridge's input bus flags accordingly, so the upstream encoder drives
> > DE with the required polarity. The property is applied only if the
> > bridge doesn't provide its own timings struct.
> >
> > Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> > ---
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>

Hi Frank,

Sashiko's comment regarding atomic_check was a false positive, but I
wasn't too sure about the one regarding devm_kzalloc. It seemed to make
sense to me, but I'm not too familiar with this subsystem yet. Do you
think this was also a false positive?

Best regards,
Leonardo

Re: [PATCH v2 2/3] drm/bridge: simple-bridge: Support data-enable-active endpoint property

[Leonardo Costa]

On Mon, Oct 19, 2026 at 10:34:06AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] drm/bridge: simple-bridge: missing atomic_check() to propagate DE bus flags
> - [Medium] drm/bridge: simple-bridge: timings devm_kzalloc() lifetime mismatch
> --
> 
> commit 17d03fa8dcd99e5bb3af7da622344c856ca2c887
> Author: Leonardo Costa <leonardo.costa@toradex.com>
> 
> drm/bridge: simple-bridge: Support data-enable-active endpoint property
> 
> This commit reads the data-enable-active property from the input endpoint in
> simple_bridge_probe() and allocates a drm_bridge_timings structure to set
> DRM_BUS_FLAG_DE_HIGH or DRM_BUS_FLAG_DE_LOW in input_bus_flags when the bridge
> does not already define its own timings.
> 
> > diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
> > index 3fba7002322e9..393228bed6e28 100644
> > --- a/drivers/gpu/drm/bridge/simple-bridge.c
> > +++ b/drivers/gpu/drm/bridge/simple-bridge.c
> > @@ -171,6 +171,9 @@ static int simple_bridge_probe(struct platform_device *pdev)
> >  {
> >  	struct simple_bridge *sbridge;
> >  	struct device_node *remote;
> > +	struct device_node *ep;
> > +	bool de_present;
> > +	u32 de;
> >  
> >  	sbridge = devm_drm_bridge_alloc(&pdev->dev, struct simple_bridge,
> >  					bridge, &simple_bridge_bridge_funcs);
> > @@ -208,10 +211,26 @@ static int simple_bridge_probe(struct platform_device *pdev)
> >  		return dev_err_probe(&pdev->dev, PTR_ERR(sbridge->enable),
> >  				     "Unable to retrieve enable GPIO\n");
> >  
> > -	/* Register the bridge. */
> >  	sbridge->bridge.of_node = pdev->dev.of_node;
> >  	sbridge->bridge.timings = sbridge->info->timings;
> >  
> > +	ep = of_graph_get_endpoint_by_regs(pdev->dev.of_node, 0, -1);
> > +	de_present = !of_property_read_u32(ep, "data-enable-active", &de);
> > +	of_node_put(ep);
> > +
> > +	if (de_present && !sbridge->bridge.timings) {
> > +		struct drm_bridge_timings *timings;
> > +
> > +		timings = devm_kzalloc(&pdev->dev, sizeof(*timings), GFP_KERNEL);
> > +		if (!timings)
> > +			return -ENOMEM;
> 
> [Severity: Medium]
> Can allocating timings with devm_kzalloc() in simple_bridge_probe() cause a
> lifetime mismatch with sbridge->bridge?
> 
> sbridge is allocated via devm_drm_bridge_alloc(), which manages the container's
> lifetime through sbridge->bridge.refcount and only frees it in
> __drm_bridge_free() after all drm_bridge references are dropped:
> 
> drivers/gpu/drm/drm_bridge.c:__drm_bridge_free()
> 	if (bridge->funcs->destroy)
> 		bridge->funcs->destroy(bridge);
> 
> 	drm_bridge_put(bridge->next_bridge);
> 
> 	kfree(bridge->container);
> 
> If pdev is unbound while an upstream bridge or encoder still holds a reference
> to sbridge->bridge, devres_release_all() will free timings immediately, leaving
> sbridge->bridge.timings as a dangling pointer that can be dereferenced by
> upstream drivers (for example, in imx8mp_hdmi_pvi_bridge_enable()).
> 
> Would embedding struct drm_bridge_timings inside struct simple_bridge (as done
> in lvds-codec.c and ti-tfp410.c), or pointing to static const
> drm_bridge_timings instances for DRM_BUS_FLAG_DE_HIGH and DRM_BUS_FLAG_DE_LOW,
> avoid this lifetime mismatch?

This seemed to make sense to me, but I'm having trouble implementing either of
Sashiko's suggestions in a proper way, as both seemed problematic to me when
actually trying to make the changes. 

Adding static const drm_bridge_timings instances for each of the flags goes
against the current pattern of using these to list configurations of actual
pieces of hardware.

On the other hand, embedding struct drm_bridge_timings inside
struct simple_bridge leads to duplicating information in the driver. It would
look something like in the diff below. In this case, when the bridge provides
a timings struct, the embedded drm_bridge_timings needs to receive a copy of it,
despite serving no purpose in this situation, it seems to me to add too much
complexity.

Laurent, do you think this is still better than adding a compatible for the
Apalis iMX6 VGA bridge, as was done in V1? Let me know if you think any of
these concerns are not actual issues, or if you think there is a pattern that
better fits this driver.

Best regards,
Leonardo

diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
index 848c2df2a276..9787eb7f7b82 100644
--- a/drivers/gpu/drm/bridge/simple-bridge.c
+++ b/drivers/gpu/drm/bridge/simple-bridge.c
@@ -28,6 +28,7 @@ struct simple_bridge_info {
 struct simple_bridge {
 	struct drm_bridge	bridge;
 	struct drm_connector	connector;
+	struct drm_bridge_timings timings;
 
 	const struct simple_bridge_info *info;
 
@@ -183,6 +184,8 @@ static int simple_bridge_probe(struct platform_device *pdev)
 {
 	struct simple_bridge *sbridge;
 	struct device_node *remote;
+	struct device_node *ep;
+	u32 de;
 
 	sbridge = devm_drm_bridge_alloc(&pdev->dev, struct simple_bridge,
 					bridge, &simple_bridge_bridge_funcs);
@@ -220,10 +223,24 @@ static int simple_bridge_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(sbridge->enable),
 				     "Unable to retrieve enable GPIO\n");
 
-	/* Register the bridge. */
 	sbridge->bridge.of_node = pdev->dev.of_node;
 	sbridge->bridge.timings = sbridge->info->timings;
 
+	if (sbridge->info->timings)
+		sbridge->timings = *sbridge->info->timings;
+
+	ep = of_graph_get_endpoint_by_regs(pdev->dev.of_node, 0, -1);
+
+	if (!of_property_read_u32(ep, "data-enable-active", &de) &&
+	    !sbridge->bridge.timings) {
+		sbridge->timings.input_bus_flags |= de ? DRM_BUS_FLAG_DE_HIGH :
+							 DRM_BUS_FLAG_DE_LOW;
+		sbridge->bridge.timings = &sbridge->timings;
+	}
+
+	of_node_put(ep);
+
+	/* Register the bridge. */
 	return devm_drm_bridge_add(&pdev->dev, &sbridge->bridge);
 }

> > +
> > +		timings->input_bus_flags = de ? DRM_BUS_FLAG_DE_HIGH :
> > +						DRM_BUS_FLAG_DE_LOW;
> > +		sbridge->bridge.timings = timings;
> > +	}
> 
> [Severity: High]
> Will setting only sbridge->bridge.timings->input_bus_flags actually propagate
> the DE polarity to upstream atomic encoders and bridges?
> 
> simple_bridge_bridge_funcs registers atomic state callbacks without an
> .atomic_check callback:
> 
> drivers/gpu/drm/bridge/simple-bridge.c:
> static const struct drm_bridge_funcs simple_bridge_bridge_funcs = {
> 	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> 	.attach		= simple_bridge_attach,
> 	.atomic_enable = simple_bridge_enable,
> 	.atomic_disable = simple_bridge_disable,
> };
> 
> During an atomic modeset, drm_atomic_bridge_chain_check() calls
> drm_atomic_bridge_propagate_bus_flags(), which overwrites
> bridge_state->input_bus_cfg.flags with output_flags from the downstream bridge
> or connector (0 for a VGA connector) and never checks
> bridge->timings->input_bus_flags:
> 
> drivers/gpu/drm/drm_bridge.c:drm_atomic_bridge_propagate_bus_flags()
> 	bridge_state->output_bus_cfg.flags = output_flags;
> 	...
> 	bridge_state->input_bus_cfg.flags = output_flags;
> 
> Because simple-bridge has a valid drm_bridge_state, upstream bridges and atomic
> encoders such as imx_pd_bridge_atomic_check() in
> drivers/gpu/drm/imx/ipuv3/parallel-display.c read
> next_bridge_state->input_bus_cfg.flags instead of
> next_bridge->timings->input_bus_flags:
> 
> drivers/gpu/drm/imx/ipuv3/parallel-display.c:imx_pd_bridge_atomic_check()
> 	if (next_bridge_state)
> 		bus_flags = next_bridge_state->input_bus_cfg.flags;
> 	else
> 		bus_flags = di->bus_flags;
> 
> As a result, bus_flags remains 0 instead of DRM_BUS_FLAG_DE_LOW or
> DRM_BUS_FLAG_DE_HIGH.
> 
> Should simple_bridge_bridge_funcs implement an .atomic_check callback that
> copies sbridge->bridge.timings->input_bus_flags into
> bridge_state->input_bus_cfg.flags (similar to tfp410_atomic_check() in
> drivers/gpu/drm/bridge/ti-tfp410.c)?
> 
> > +
> > +	/* Register the bridge. */
> >  	return devm_drm_bridge_add(&pdev->dev, &sbridge->bridge);
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261019-v2-apalis-imx6-enable-vga-v2-0-ef6a3a780472@toradex.com?part=2

This second finding was a false positive, as per my response in the 3/3
patch.

Re: [PATCH v2 2/3] drm/bridge: simple-bridge: Support data-enable-active endpoint property

[Frank Li]

On Tue, Oct 20, 2026 at 08:15:57AM -0300, Leonardo Costa wrote:
> On Mon, Oct 19, 2026 at 02:50:36PM -0500, Frank Li wrote:
> > On Mon, Oct 19, 2026 at 07:17:15AM -0300, Leonardo Costa wrote:
> > > From: Leonardo Costa <leonardo.costa@toradex.com>
> > >
> > > Some simple bridges, such as a resistor ladder DAC, may include a
> > > data-enable input that must be driven at a specific polarity.
> > >
> > > Read the data-enable-active property from the input endpoint and fill
> > > the bridge's input bus flags accordingly, so the upstream encoder drives
> > > DE with the required polarity. The property is applied only if the
> > > bridge doesn't provide its own timings struct.
> > >
> > > Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> > > ---
> >
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Hi Frank,
>
> Sashiko's comment regarding atomic_check was a false positive, but I
> wasn't too sure about the one regarding devm_kzalloc. It seemed to make
> sense to me, but I'm not too familiar with this subsystem yet. Do you
> think this was also a false positive?

I am also not familiar it, it looks make sense to me also.

Frank

>
> Best regards,
> Leonardo

Re: [PATCH v2 3/3] ARM: dts: imx6q-apalis: Add VGA overlay

[Frank Li]

On Tue, Oct 20, 2026 at 07:33:10AM -0300, Leonardo Costa wrote:
> On Mon, Oct 19, 2026 at 02:37:41PM -0500, Frank Li wrote:
> > On Mon, Oct 19, 2026 at 07:17:16AM -0300, Leonardo Costa wrote:
> > > From: Leonardo Costa <leonardo.costa@toradex.com>
> > >
> > > Add a device tree overlay for the VGA output available on the Apalis
> > > Evaluation Board, which is driven from a parallel RGB interface passing
> > > through a non-programmable video DAC to a VGA connector, without VESA
> > > DDC I2C signals.
> > >
> > > The non-programmable video DAC consists of a simple resistor ladder that
> > > converts the digital 16-bit RGB signal to VGA. The VGA output is gated by
> > > an active-low data-enable signal.
> > >
> > > The overlay is also combined with the various Apalis Evaluation Board
> > > device trees to provide ready-to-use DTBs.
> > >
> > > Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> > > ---
> > >  arch/arm/boot/dts/nxp/imx/Makefile                 | 21 ++++++
> > >  .../boot/dts/nxp/imx/imx6q-apalis-eval-vga.dtso    | 80 ++++++++++++++++++++++
> >
> > what's difference with
> >
> > https://lore.kernel.org/imx/20261019-colibri-imx6-overlays-v1-5-e4d028727382@toradex.com/T/#u
> >
> > Is it possible share one dtso also?
> >
> > Frank
>
> Hi Frank,
>
> The Apalis iMX6 one needs to set up the connection between the IPU the
> LCD interface (endpoints ipu2_di0_disp0 and vga_display_in), and the
> Colibri iMX6 one uses a DVI connector instead of a VGA one. Most of all,
> the bridge on the Colibri iMX6 doesn't have any control signals, while
> this one on the Apalis iMX6 has an active-low data-enable signal

This make sense to use difference dtso to cover it. Is it use name
imx6q-ipu-di0-vga.dtso, because this dtso have not boards specific part,
which can reuse for other similar hardware? Or we can rename it when reuse
happen.

Frank

> specified on the input endpoint. Because of all of these, the overlay
> needs to be different between these modules.
>
> Best regards,
> Leonardo