Hi Uwe.

Two things I noticed while browsing the driver.

I did not try to do a full review - maybe for the next round.

        Sam

> +static unsigned int imx_lcdc_get_format(unsigned int drm_format)
> +{
> +     unsigned int bpp;
> +
> +     switch (drm_format) {
> +     default:
> +             DRM_WARN("Format not supported - fallback to RGB565\n");
> +             fallthrough;
> +     case DRM_FORMAT_RGB565:
> +             bpp = BPP_RGB565;
> +             break;
> +     }
> +
> +     return bpp;
> +}

It would be great if the driver had fallback to the generic XRGB8888
variant. So is was either the native or a fallback generic.
The latter just because most userspace assumes we have the XRGB8888
variant.


> +static int imx_lcdc_probe(struct platform_device *pdev)
> +{
> +     struct imx_lcdc *lcdc;
> +     struct drm_device *drm;
> +     int irq;
> +     int ret;
> +     struct device *dev = &pdev->dev;
> +
> +     lcdc = devm_drm_dev_alloc(dev, &imx_lcdc_drm_driver,
> +                               struct imx_lcdc, drm);
> +     if (!lcdc)
> +             return -ENOMEM;
> +
> +     drm = &lcdc->drm;
> +
> +     lcdc->base = devm_platform_ioremap_resource(pdev, 0);
> +     if (IS_ERR(lcdc->base))
> +             return dev_err_probe(dev, PTR_ERR(lcdc->base), "Cannot get IO 
> memory\n");
> +
> +     /* Panel */
> +     ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &lcdc->panel, 
> &lcdc->bridge);
>From the documentation of drm_of_find_panel_or_bridge():

 * This function is deprecated and should not be used in new drivers. Use
 * devm_drm_of_get_bridge() instead.

        Sam

Reply via email to