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.
>
> [[email protected]: port to v7.0-rc1]
> [[email protected]: add review feedback (Alexander)]
> [[email protected]: fix to short Kconfig description (checkpath)]
> [[email protected]: use "GPL" instead of "GPL v2" (checkpatch)]
> [[email protected]: 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 <[email protected]>
> Signed-off-by: Marco Felsch <[email protected]>
> Signed-off-by: Liu Ying <[email protected]>
> ---
[...]
> +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 <[email protected]>
--
Thanks,
Laurentiu
