From: Frank Li (AI-BOT) <[email protected]>

AI bot review and may be useless.

---

> +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;
> +             

Reply via email to