On Fri, Jul 24, 2026 at 11:08:06AM +0300, Laurentiu Palcu wrote: > Hi Ying,
Hi Laurentiu, > > 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? Marco introduced the mask variable to the code. I'd say it makes the regmap_update_bits() call fit into one line, so it doesn't look too bad and I'm fine with it. > > Otherwise, > Reviewed-by: Laurentiu Palcu <[email protected]> If no objections, may I take this R-b tag as a valid one if keeping the mask variable as-is? > > -- > Thanks, > Laurentiu -- Regards, Liu Ying
