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

Hi Laurent,

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

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

On Sat, 30 May 2020 at 04:11, Laurent Pinchart laurent.pinchart@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