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

[Frank Li <Frank.li@oss.nxp.com>]

On Wed, Oct 14, 2026 at 10:57:40AM -0300, Leonardo Costa wrote:
> Hi Laurent,
>
> On Tue, Oct 13, 2026 at 01:11:58PM +0200, Laurent Pinchart wrote:
> > Hi Francesco,
> >
> > On Tue, Oct 13, 2026 at 11:06:31AM +0200, Francesco Dolcini wrote:
> > > On Mon, Oct 12, 2026 at 06:40:54PM +0200, Laurent Pinchart wrote:
> > > > On Mon, Oct 12, 2026 at 04:25:22AM -0300, Leonardo Costa wrote:
> > > > > 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 a new compatible for the bridge in the simple-bridge
> > > > >   binding. This is necessary because the existing dumb-vga-dac compatible
> > > > >   doesn't describe a data-enable signal polarity.
> > > > >
> > > > > - simple-bridge: Add the VGA bridge match-table entry and timings
> > > > >   struct, setting the DRM_BUS_FLAG_DE_LOW flag for the bridge.
> > > >
> > > > Is there schematics available anywhere ? That would help reviewing the
> > > > series ?
> > >
> > > This circuit is on the SoM, and the SoM schematics are
> > > confidential and not generally available.
> > >
> > > I tried to extract as ascii art the relevant circuit, hope it helps
> >
> > Thank you, it makes things clearer.
> >
> > The circuit seems fairly generic. Wouldn't it be better to add an
> > optional property ot the dumb-vga-dac instead of a new compatible string
> > specific to Apalis ?
>
> See the diff below, this was the initial idea we had when working on
> this. On one hand, at least to me it seems like something that other
> bridges could use, and a new property seems reasonable. On the other,
> since we don't know exactly how much configuration is expected from a
> bridge in the simple-bridge driver, we weren't sure if this would be
> accepted, and went with the simpler solution of creating a new binding
> for our bridge, which also seemed less intrusive.

Look like add data-enable-active make sense at this case. You can try it.

Frank

>
> What are your thoughts about a change like this?
>
> diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
> index 3fba7002322e..41b00a44983a 100644
> --- a/drivers/gpu/drm/bridge/simple-bridge.c
> +++ b/drivers/gpu/drm/bridge/simple-bridge.c
> @@ -171,6 +171,7 @@ static int simple_bridge_probe(struct platform_device *pdev)
>  {
>  	struct simple_bridge *sbridge;
>  	struct device_node *remote;
> +	u32 de;
>
>  	sbridge = devm_drm_bridge_alloc(&pdev->dev, struct simple_bridge,
>  					bridge, &simple_bridge_bridge_funcs);
> @@ -212,6 +213,19 @@ static int simple_bridge_probe(struct platform_device *pdev)
>  	sbridge->bridge.of_node = pdev->dev.of_node;
>  	sbridge->bridge.timings = sbridge->info->timings;
>
> +	if (!sbridge->bridge.timings &&
> +	    !of_property_read_u32(pdev->dev.of_node, "data-enable-active", &de)) {
> +		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;
> +	}
> +
>  	return devm_drm_bridge_add(&pdev->dev, &sbridge->bridge);
>  }
>