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

[Laurentiu Palcu]

Hi Ying,

On Thu, Jul 23, 2026 at 02:37:32PM +0800, Liu Ying wrote:
> 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.
> 
> [m.felsch@xxxxxxxxxxxxxx: port to v7.0-rc1]
> [m.felsch@xxxxxxxxxxxxxx: add review feedback (Alexander)]
> [m.felsch@xxxxxxxxxxxxxx: fix to short Kconfig description (checkpath)]
> [m.felsch@xxxxxxxxxxxxxx: use "GPL" instead of "GPL v2" (checkpatch)]
> [m.felsch@xxxxxxxxxxxxxx: add bus-width support]
> [ Liu Ying: Port to drm-misc/drm-misc-next(v7.2-rc2 based) ]
> [ Liu Ying: Add nxp,imx91-pdfc compatible string ]
> [ Liu Ying: Call drm_bridge_get() for next_bridge ]
> [ Liu Ying: Reject unsupported output bus fmt in .atomic_get_input_bus_fmts() ]
> 
> Reviewed-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
> Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> ---

[...]

> +static void imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
> +					    struct drm_atomic_commit *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);

Apparently, the 'mask' variable is only used in this call, nowhere else. Why
not use PARALLEL_DISP_FORMAT macro directly here instead of declaring a
variable just for that?

Otherwise,
Reviewed-by: Laurentiu Palcu <laurentiu.palcu@xxxxxxxxxxx>

-- 
Thanks,
Laurentiu