Hi all. The Atmel at91sam9263 has a nice LCDC IP core that supports several formats: DRM_FORMAT_XBGR8888, DRM_FORMAT_BGR888, DRM_FORMAT_BGR565 (It also supports a palletized C8 format, but thats for later). The formats are all BGR formats - and some boards actually reverse Blue and Red when wiring up the display. I have added a DT property to identify boards with this difference. The penguin shown during boot of the kernel had blue feet which is a clear sign that RED and GREEN was reversed. So to fix this I (got help from imirkin on irc) I implmented a quirk. See patch below. With the quirk enabled I see: - penguin shown during kernel boot has yellow feets (OK) - modetest tell the driver reports: XB24 BG24 BG16 (as expected) - modetest -s fails with: # modetest -M atmel-lcdc -s 34:800x480-60Hz setting mode 800x480-60Hz@XR24 on connectors 34, crtc 32 failed to set mode: Function not implemented Snip from dmesg: drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_ADDFB2 [drm:drm_mode_addfb2] [FB:37] [drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_SETCRTC [drm:drm_mode_setcrtc] [CRTC:32:crtc-0] [drm:drm_mode_setcrtc] Invalid pixel format XR24 little-endian (0x34325258), modifier 0x0 ^^^^ [drm:drm_mode_object_put] OBJ ID: 37 (2) [drm:drm_ioctl] pid=208, ret = -22 [drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_DIRTYFB [drm:drm_mode_object_put] OBJ ID: 37 (2) [drm:drm_ioctl] pid=208, ret = -38 [drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_RMFB [drm:drm_mode_object_put] OBJ ID: 37 (2) [drm:drm_mode_object_put] OBJ ID: 37 (1) [drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_DESTROY_DUMB [drm:drm_release] open_count = 1 [drm:drm_file_free] pid = 208, device = 0xe201, open_count = 1 [drm:drm_lastclose] [drm:drm_lastclose] driver lastclose completed Notice that somehow we have a framebuffer in XR24 format, which is not supported by the driver. I have tried to tell that my driver supports DRM_FORMAT_XRGB8888, DRM_FORMAT_RGB888, DRM_FORMAT_RGB565 and then modetest works. But in the output of modetest -s the blue and red colors are reversed. *Any* hints why modetest fails when my driver reports the correct formats? For reference I will post source for the driver in a follow-up mail. Sam The quirk I implmented looks like this (preliminary patch): >From ab0f213e50fbe1b053d624745306252b28e6ecdc Mon Sep 17 00:00:00 2001 From: Sam Ravnborg <sam@xxxxxxxxxxxx> Date: Sun, 21 Jul 2019 08:59:49 +0200 Subject: [PATCH 21/22] drm: add quirk_addfb_rgb_to_bgr Used for drivers where the format supported is default BGR. Then use this quirk to force drm_fb_helper to convert the framebuffer from RGB => BGR. Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx> --- drivers/gpu/drm/drm_fb_helper.c | 46 +++++++++++++++++++++++---------- drivers/gpu/drm/drm_fourcc.c | 14 ++++++++++ include/drm/drm_mode_config.h | 7 +++++ 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index a7ba5b4902d6..adc1b1fcb153 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1236,9 +1236,9 @@ static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, } static void drm_fb_helper_fill_pixel_fmt(struct fb_var_screeninfo *var, - u8 depth) + struct drm_framebuffer *fb) { - switch (depth) { + switch (fb->format->depth) { case 8: var->red.offset = 0; var->green.offset = 0; @@ -1260,18 +1260,30 @@ static void drm_fb_helper_fill_pixel_fmt(struct fb_var_screeninfo *var, var->transp.length = 1; break; case 16: - var->red.offset = 11; - var->green.offset = 5; - var->blue.offset = 0; + if (!fb->dev->mode_config.quirk_addfb_rgb_to_bgr) { + var->red.offset = 11; + var->green.offset = 5; + var->blue.offset = 0; + } else { + var->red.offset = 0; + var->green.offset = 5; + var->blue.offset = 11; + } var->red.length = 5; var->green.length = 6; var->blue.length = 5; var->transp.offset = 0; break; case 24: - var->red.offset = 16; - var->green.offset = 8; - var->blue.offset = 0; + if (!fb->dev->mode_config.quirk_addfb_rgb_to_bgr) { + var->red.offset = 16; + var->green.offset = 8; + var->blue.offset = 0; + } else { + var->red.offset = 0; + var->green.offset = 8; + var->blue.offset = 16; + } var->red.length = 8; var->green.length = 8; var->blue.length = 8; @@ -1279,9 +1291,15 @@ static void drm_fb_helper_fill_pixel_fmt(struct fb_var_screeninfo *var, var->transp.length = 0; break; case 32: - var->red.offset = 16; - var->green.offset = 8; - var->blue.offset = 0; + if (!fb->dev->mode_config.quirk_addfb_rgb_to_bgr) { + var->red.offset = 16; + var->green.offset = 8; + var->blue.offset = 0; + } else { + var->red.offset = 0; + var->green.offset = 8; + var->blue.offset = 16; + } var->red.length = 8; var->green.length = 8; var->blue.length = 8; @@ -1342,7 +1360,7 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, !var->blue.length && !var->transp.length && !var->red.msb_right && !var->green.msb_right && !var->blue.msb_right && !var->transp.msb_right) { - drm_fb_helper_fill_pixel_fmt(var, fb->format->depth); + drm_fb_helper_fill_pixel_fmt(var, fb); } /* @@ -1684,7 +1702,7 @@ static void drm_fb_helper_fill_var(struct fb_info *info, info->var.yoffset = 0; info->var.activate = FB_ACTIVATE_NOW; - drm_fb_helper_fill_pixel_fmt(&info->var, fb->format->depth); + drm_fb_helper_fill_pixel_fmt(&info->var, fb); info->var.xres = fb_width; info->var.yres = fb_height; @@ -2205,7 +2223,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, sizes->surface_width, sizes->surface_height, sizes->surface_bpp); - format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth); + format = drm_driver_legacy_fb_format(fb_helper->dev, sizes->surface_bpp, sizes->surface_depth); buffer = drm_client_framebuffer_create(client, sizes->surface_width, sizes->surface_height, format); if (IS_ERR(buffer)) diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index c630064ccf41..972ea746c06d 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -126,6 +126,20 @@ uint32_t drm_driver_legacy_fb_format(struct drm_device *dev, fmt == DRM_FORMAT_XRGB2101010) fmt = DRM_FORMAT_XBGR2101010; + if (dev->mode_config.quirk_addfb_rgb_to_bgr) { + switch (fmt) { + case DRM_FORMAT_RGB565: + fmt = DRM_FORMAT_BGR565; + break; + case DRM_FORMAT_RGB888: + fmt = DRM_FORMAT_BGR888; + break; + case DRM_FORMAT_XRGB8888: + fmt = DRM_FORMAT_XBGR8888; + break; + } + } + return fmt; } EXPORT_SYMBOL(drm_driver_legacy_fb_format); diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index f57eea0481e0..58aa8c02232e 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -881,6 +881,13 @@ struct drm_mode_config { */ bool quirk_addfb_prefer_host_byte_order; + /** + * @quirk_addfb_rgb_to_bgr: + * + * TODO + */ + bool quirk_addfb_rgb_to_bgr; + /** * @async_page_flip: Does this device support async flips on the primary * plane? -- 2.20.1 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel