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

[Marco Felsch]

On 26-03-03, Liu Ying wrote:
> On Tue, Mar 03, 2026 at 09:05:43AM +0100, Marco Felsch wrote:
> > On 26-03-03, Liu Ying wrote:
> >> On Mon, Mar 02, 2026 at 05:10:41PM +0100, Marco Felsch wrote:
> >>> From: Liu Ying <victor.liu at 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 at nxp.com>
> >>> [m.felsch at pengutronix.de: port to v7.0-rc1]
> >>> [m.felsch at pengutronix.de: add review feedback (Alexander)]
> >>> [m.felsch at pengutronix.de: fix to short Kconfig description (checkpath)]
> >>> [m.felsch at pengutronix.de: use "GPL" instead of "GPL v2" (checkpatch)]
> >>> [m.felsch at pengutronix.de: add bus-width support]
> >>> Signed-off-by: Marco Felsch <m.felsch at pengutronix.de>
> >>> ---
> >>>  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(+)
> >>>
> >>
> >> [...]
> >>
> >>> +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'm aware that this is a copy of the output_cfg.format value. Would it
be a non const value you could mess with it and return the wrong value
afterwards. Keeping it const avoids this. Not sure why a 'const' is
worth the discussion :/

> > I forgot to add the double space fix though, thanks.
> > 
> >>> +{
> >>> +	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;
> >>> +}
> 
> [...]
> 
> >>> +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.

Same here, I'm aware that this is not the pointer value but it holds a
copy of the value. Any adaptions made to the copy will fail, any
assignment of a const value to a non const value will prints at least a
warning to show that something is wrong within this function. Only a
direct bridge_state->output_bus_cfg.format assigned would cause no
compiler error/warning, fingers crossed that this will not happen.

I'm not sure why the 'const' is such a big problem in your oppinion.
Everything works just fine and we've done everything in our power to
avoid a wrong function behavior.

Regards,
  Marco


> 
> > 
> > Regards,
> >   Marco
> > 
> > 
> > 
> >>
> >> -- 
> >> Regards,
> >> Liu Ying
> >>
> > 
> 
> -- 
> Regards,
> Liu Ying
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |