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

On Tue, Mar 03, 2026 at 09:05:43AM +0100, Marco Felsch wrote:

On 26-03-03, Liu Ying wrote:

quoted

On Mon, Mar 02, 2026 at 05:10:41PM +0100, Marco Felsch wrote:

quoted

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.

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 <redacted>
---
 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(+)

[...]

quoted

+static bool imx93_pdfc_bus_output_fmt_supported(const  u32 fmt)

As I mentioned in v9, can you drop const?
I don't think const is needed.

It makes sense to spot failures early. Albeit this function is very
small, the fmt shouldn't be changed and therefore needs to be const.

If fmt is a pointer, then I think it makes sense to keep const.
Otherwise, it looks a bit over engineered to have const, since this
function is really small and no one would try to change fmt.

I forgot to add the double space fix though, thanks.

quoted

quoted

+{
+	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;
+}

[...]

quoted

quoted

+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);
+	const u32 format = bridge_state->output_bus_cfg.format;

Can you drop const?

No because this function is not supposed to change the format. The
function documentation says: this function can alter the bus_cfg.flags
bit _not_ the format.

The local variable format is not a pointer, so literally the value of
bridge_state->output_bus_cfg.format cannot be changed through format,
which follows the documentation.  So, I think const could be dropped.

Regards,
  Marco

quoted

-- 
Regards,
Liu Ying

-- 
Regards,
Liu Ying