My penguin has blue feets (aka: RGB versus BGR troubles)

* My penguin has blue feets (aka: RGB versus BGR troubles)
@ 2019-07-26 20:36 Sam Ravnborg
  2019-07-26 20:44 ` [RFC PATCH] drm: add Atmel LCDC display controller support Sam Ravnborg
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sam Ravnborg @ 2019-07-26 20:36 UTC (permalink / raw)
  To: dri-devel

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@ravnborg.org>
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@ravnborg.org>
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread