[PATCH v2 20/22] drm: mxsfb: Merge mxsfb_set_pixel_fmt() and mxsfb_set_bus_fmt()

Emil Velikov emil.l.velikov at gmail.com
Fri Jun 5 14:58:53 UTC 2020


Hi Laurent,

With the previous disclaimer in mind, the series is:
Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>

There's a small suggestion inline, for future work.

On Sat, 30 May 2020 at 04:11, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:

>         switch (bus_format) {
>         case MEDIA_BUS_FMT_RGB565_1X16:
> -               reg |= CTRL_BUS_WIDTH_16;
> +               ctrl |= CTRL_BUS_WIDTH_16;
>                 break;
>         case MEDIA_BUS_FMT_RGB666_1X18:
> -               reg |= CTRL_BUS_WIDTH_18;
> +               ctrl |= CTRL_BUS_WIDTH_18;
>                 break;
>         case MEDIA_BUS_FMT_RGB888_1X24:
> -               reg |= CTRL_BUS_WIDTH_24;
> +               ctrl |= CTRL_BUS_WIDTH_24;
>                 break;
>         default:
>                 dev_err(drm->dev, "Unknown media bus format %d\n", bus_format);
>                 break;

Off the top of my head, the default case should be handled in the
atomic_check() hook.
That is, unless I'm missing something and one can change the bus
format in between atomic_check() and atomic_enable(). Which would
sound like a very odd thing to do.

-Emil


More information about the dri-devel mailing list