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

Frank Li
2026-03-03T22:07:41Z

On Tue, Mar 03, 2026 at 10:22:02PM +0100, Marco Felsch wrote:
> On 26-03-03, Frank Li wrote:
> > From: Frank Li (AI-BOT) <frank.li@nxp.com>
> >
> > AI bot review and may be useless.
>
> Hi Frank,
>
> albeit I'm very open to new technology, I would appreciate it if your
> AI-BOT is used internally first till you're convinced that it reports
> real issues instead of false-positives.

I tested 22 patches, which sent to imx mail list only. Need test script.
Impact should be limited.

Frank

>
> Regards,
> Marco
>
> > > +static u32 *
> > > +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> > > + struct drm_bridge_state *bridge_state,
> > > + struct drm_crtc_state *crtc_state,
> > > + struct drm_connector_state *conn_state,
> > > + u32 output_fmt,
> > > + unsigned int *num_input_fmts)
> > > +{
> > > + struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
> > > + u32 *input_fmts;
> > > +
> > > + *num_input_fmts = 0;
> > > +
> > > + input_fmts = kmalloc_obj(*input_fmts);
> > > + if (!input_fmts)
> > > + return NULL;
> >
> > Missing kfree(input_fmts) in error path if the switch statement
> > or subsequent logic fails. Consider allocating a fixed-size array
> > or using devm_kzalloc() instead.
> >
> > > + *num_input_fmts = 1;
> > > +
> > > + if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
> > > + dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback to MEDIA_BUS_FMT_RGB888_1X24\n");
> >
> > Line exceeds 80 characters (97 chars). Break into two lines.
> >
> > > + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > > + return input_fmts;
> > > + }
> > > +
> > > + switch (output_fmt) {
> > > + case MEDIA_BUS_FMT_RGB888_1X24:
> > > + case MEDIA_BUS_FMT_RGB565_1X16:
> > > + input_fmts[0] = output_fmt;
> > > + break;
> > > + case MEDIA_BUS_FMT_RGB666_1X18:
> > > + case MEDIA_BUS_FMT_FIXED:
> > > + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > > + break;
> > > + }
> >
> > Switch statement lacks default case. Add default case to handle
> > unexpected format values explicitly.
> >
> > > +static int imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
> > > + struct drm_atomic_state *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;
> > > +
> >
>
> --
> #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 |