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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DFO5LHWDD7S2.19P595M4CWIPI@bootlin.com>
Date: Wed, 14 Jan 2026 09:02:35 +0100
From: "Luca Ceresoli" <luca.ceresoli@...tlin.com>
To: "Marco Felsch" <m.felsch@...gutronix.de>, "Rob Herring"
 <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor
 Dooley" <conor+dt@...nel.org>, "Shawn Guo" <shawnguo@...nel.org>, "Sascha
 Hauer" <s.hauer@...gutronix.de>, "Pengutronix Kernel Team"
 <kernel@...gutronix.de>, "Fabio Estevam" <festevam@...il.com>, "Peng Fan"
 <peng.fan@....com>, "Liu Ying" <victor.liu@....com>, "Andrzej Hajda"
 <andrzej.hajda@...el.com>, "Neil Armstrong" <neil.armstrong@...aro.org>,
 "Robert Foss" <rfoss@...nel.org>, "Laurent Pinchart"
 <Laurent.pinchart@...asonboard.com>, "Jonas Karlman" <jonas@...boo.se>,
 "Jernej Skrabec" <jernej.skrabec@...il.com>, "Maarten Lankhorst"
 <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
 "Thomas Zimmermann" <tzimmermann@...e.de>, "David Airlie"
 <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>
Cc: <devicetree@...r.kernel.org>, <imx@...ts.linux.dev>,
 <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
 <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v8 2/3] drm/bridge: imx: Add i.MX93 parallel display
 format configuration support

Hello Marco, Liu,

On Tue Jan 13, 2026 at 8:07 PM CET, Marco Felsch wrote:
> From: Liu Ying <victor.liu@....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.
>
> Signed-off-by: Liu Ying <victor.liu@....com>
> [m.felsch@...gutronix.de: port to v6.19-rc1]
> [m.felsch@...gutronix.de: add review feedback (Alexander)]
> [m.felsch@...gutronix.de: fix to short Kconfig description (checkpath)]
> [m.felsch@...gutronix.de: use "GPL" instead of "GPL v2" (checkpatch)]
> [m.felsch@...gutronix.de: add bus-width support]
> Signed-off-by: Marco Felsch <m.felsch@...gutronix.de>

I'm sorry to be reviewing at v8 only, I hadn't noticed this series before.

> ---
>  drivers/gpu/drm/bridge/imx/Kconfig      |  11 ++
>  drivers/gpu/drm/bridge/imx/Makefile     |   1 +
>  drivers/gpu/drm/bridge/imx/imx93-pdfc.c | 221 ++++++++++++++++++++++++++++++++
>  3 files changed, 233 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> index b9028a5e5a065c3237b404111d8df57e8e017f9d..181ee87bc0f9f65ee0b6e5edbb48ba808dfbb71f 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_CONFIG
> +	tristate "NXP i.MX91/i.MX93 parallel display format configuration"

Minor nit: this is a driver for a device, so calling it "configuration"
seems weird. From the code it looks like a device converting the color
format, so what about "NXP i.MX91/i.MX93 parallel display format
converter"?

[...]

> +static int imx93_pdfc_bridge_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	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");
> +	}
> +
> +	pdfc->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	if (IS_ERR(pdfc->next_bridge))
> +		return dev_err_probe(dev, PTR_ERR(pdfc->next_bridge),
> +				     "failed to get next bridge\n");
> +
> +	pdfc->dev = dev;
> +	pdfc->bridge.driver_private = pdfc;

pdfc embeds the struct drm_bridge, which is the mandatory design since
devm_drm_bridge_alloc() got added, so driver_private shouldn't be needed
anymore. Most drivers have a bridge_to_foo() inline function using
component_of() to get the private struct from the drm_bridge pointer,
e.g. [0] and [1].

[0] https://elixir.bootlin.com/linux/v6.18.5/source/drivers/gpu/drm/bridge/simple-bridge.c#L39-L43
[1] https://elixir.bootlin.com/linux/v6.18.5/source/drivers/gpu/drm/bridge/ti-sn65dsi83.c#L287-L290

A short discussion took place a few months ago about driver_private, kind
of suggesting it might be removed after all drivers have switched to
devm_drm_bridge_alloc(). I think we should at least not introduce new users
of driver_private at least.

Best regards,
Luca

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