[PATCH v2 0/7] drm/gud: Add some more pixel formats

[PATCH v2 0/7] drm/gud: Add some more pixel formats

[Noralf Trønnes]

Hi,

This adds some more pixel formats and gives the user the ability to
choose the xrgb8888 emulation format.

Pixel formats:
R8: For greyscale e-ink displays
RGB332: For e-ink displays and some niche displays
RGB888: Same color depth as XRGB8888 but the smaller buffer gives better
fps

Changes since first version:
- RGB332: Support Big Endian in the emulation (Daniel)

Noralf.


Noralf Trønnes (7):
  drm/fourcc: Add R8 to drm_format_info
  drm/format-helper: Add drm_fb_xrgb8888_to_rgb332()
  drm/format-helper: Add drm_fb_xrgb8888_to_rgb888()
  drm/gud: Add GUD_PIXEL_FORMAT_R8
  drm/gud: Add GUD_PIXEL_FORMAT_RGB332
  drm/gud: Add GUD_PIXEL_FORMAT_RGB888
  drm/gud: Add module parameter to control emulation: xrgb8888

 drivers/gpu/drm/drm_format_helper.c | 88 +++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_fourcc.c        |  1 +
 drivers/gpu/drm/gud/gud_drv.c       | 19 ++++++-
 drivers/gpu/drm/gud/gud_internal.h  | 12 ++++
 drivers/gpu/drm/gud/gud_pipe.c      |  6 ++
 include/drm/drm_format_helper.h     |  4 ++
 include/drm/gud.h                   |  6 +-
 7 files changed, 131 insertions(+), 5 deletions(-)

-- 
2.32.0

[PATCH v2 1/7] drm/fourcc: Add R8 to drm_format_info

[Noralf Trønnes]

Add an entry in drm_format_info for the existing format DRM_FORMAT_R8.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fourcc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index eda832f9200d..783844bfecc1 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -133,6 +133,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
 {
 	static const struct drm_format_info formats[] = {
 		{ .format = DRM_FORMAT_C8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_R8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_RGB332,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_BGR233,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_XRGB4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
-- 
2.32.0

[PATCH v2 2/7] drm/format-helper: Add drm_fb_xrgb8888_to_rgb332()

[Noralf Trønnes]

Add XRGB8888 emulation support for devices that can only do RGB332.

v2:
- Support Big Endian (Daniel)

Cc: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_format_helper.c | 50 +++++++++++++++++++++++++++++
 include/drm/drm_format_helper.h     |  2 ++
 2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 5231104b1498..dab4f382d469 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -135,6 +135,56 @@ void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL(drm_fb_swab);
 
+static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned int pixels)
+{
+	unsigned int x;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf[x]);
+		dbuf[x] = ((pix & 0x00e00000) >> 16) |
+			  ((pix & 0x0000e000) >> 11) |
+			  ((pix & 0x000000c0) >> 6);
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_rgb332 - Convert XRGB8888 to RGB332 clip buffer
+ * @dst: RGB332 destination buffer
+ * @src: XRGB8888 source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ *
+ * Drivers can use this function for RGB332 devices that don't natively support XRGB8888.
+ *
+ * This function does not apply clipping on dst, i.e. the destination is a small buffer
+ * containing the clip rect only.
+ */
+void drm_fb_xrgb8888_to_rgb332(void *dst, void *src, struct drm_framebuffer *fb,
+			       struct drm_rect *clip)
+{
+	size_t width = drm_rect_width(clip);
+	size_t src_len = width * sizeof(u32);
+	unsigned int y;
+	void *sbuf;
+
+	/* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */
+	sbuf = kmalloc(src_len, GFP_KERNEL);
+	if (!sbuf)
+		return;
+
+	src += clip_offset(clip, fb->pitches[0], sizeof(u32));
+	for (y = 0; y < drm_rect_height(clip); y++) {
+		memcpy(sbuf, src, src_len);
+		drm_fb_xrgb8888_to_rgb332_line(dst, sbuf, width);
+		src += fb->pitches[0];
+		dst += width;
+	}
+
+	kfree(sbuf);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
+
 static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf,
 					   unsigned int pixels,
 					   bool swab)
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 4e0258a61311..d0809aff5cf8 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -16,6 +16,8 @@ void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vadd
 			   struct drm_rect *clip);
 void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
 		 struct drm_rect *clip, bool cached);
+void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
+			       struct drm_rect *clip);
 void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
 			       struct drm_framebuffer *fb,
 			       struct drm_rect *clip, bool swab);
-- 
2.32.0

[PATCH v2 3/7] drm/format-helper: Add drm_fb_xrgb8888_to_rgb888()

[Noralf Trønnes]

Add XRGB8888 emulation support for devices that can only do RGB888.

Cc: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_format_helper.c | 38 +++++++++++++++++++++++++++++
 include/drm/drm_format_helper.h     |  2 ++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index dab4f382d469..69fde60e36b3 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -300,6 +300,44 @@ static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, u32 *sbuf,
 	}
 }
 
+/**
+ * drm_fb_xrgb8888_to_rgb888 - Convert XRGB8888 to RGB888 clip buffer
+ * @dst: RGB888 destination buffer
+ * @src: XRGB8888 source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ *
+ * Drivers can use this function for RGB888 devices that don't natively
+ * support XRGB8888.
+ *
+ * This function does not apply clipping on dst, i.e. the destination
+ * is a small buffer containing the clip rect only.
+ */
+void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
+			       struct drm_rect *clip)
+{
+	size_t width = drm_rect_width(clip);
+	size_t src_len = width * sizeof(u32);
+	unsigned int y;
+	void *sbuf;
+
+	/* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */
+	sbuf = kmalloc(src_len, GFP_KERNEL);
+	if (!sbuf)
+		return;
+
+	src += clip_offset(clip, fb->pitches[0], sizeof(u32));
+	for (y = 0; y < drm_rect_height(clip); y++) {
+		memcpy(sbuf, src, src_len);
+		drm_fb_xrgb8888_to_rgb888_line(dst, sbuf, width);
+		src += fb->pitches[0];
+		dst += width * 3;
+	}
+
+	kfree(sbuf);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
+
 /**
  * drm_fb_xrgb8888_to_rgb888_dstclip - Convert XRGB8888 to RGB888 clip buffer
  * @dst: RGB565 destination buffer (iomem)
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index d0809aff5cf8..e86925cf07b9 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -24,6 +24,8 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
 void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst, unsigned int dst_pitch,
 				       void *vaddr, struct drm_framebuffer *fb,
 				       struct drm_rect *clip, bool swab);
+void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
+			       struct drm_rect *clip);
 void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst, unsigned int dst_pitch,
 				       void *vaddr, struct drm_framebuffer *fb,
 				       struct drm_rect *clip);
-- 
2.32.0

[PATCH v2 4/7] drm/gud: Add GUD_PIXEL_FORMAT_R8

[Noralf Trønnes]

Add support for 8-bit greyscale format.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/gud/gud_drv.c      | 2 ++
 drivers/gpu/drm/gud/gud_internal.h | 4 ++++
 drivers/gpu/drm/gud/gud_pipe.c     | 2 ++
 include/drm/gud.h                  | 4 ++--
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index eb4e08846da4..a8d76c76e868 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -523,6 +523,8 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
 		switch (format) {
 		case GUD_DRM_FORMAT_R1:
 			fallthrough;
+		case DRM_FORMAT_R8:
+			fallthrough;
 		case GUD_DRM_FORMAT_XRGB1111:
 			if (!xrgb8888_emulation_format)
 				xrgb8888_emulation_format = info;
diff --git a/drivers/gpu/drm/gud/gud_internal.h b/drivers/gpu/drm/gud/gud_internal.h
index 2a388e27d5d7..8499e713dbbc 100644
--- a/drivers/gpu/drm/gud/gud_internal.h
+++ b/drivers/gpu/drm/gud/gud_internal.h
@@ -80,6 +80,8 @@ static inline u8 gud_from_fourcc(u32 fourcc)
 	switch (fourcc) {
 	case GUD_DRM_FORMAT_R1:
 		return GUD_PIXEL_FORMAT_R1;
+	case DRM_FORMAT_R8:
+		return GUD_PIXEL_FORMAT_R8;
 	case GUD_DRM_FORMAT_XRGB1111:
 		return GUD_PIXEL_FORMAT_XRGB1111;
 	case DRM_FORMAT_RGB565:
@@ -98,6 +100,8 @@ static inline u32 gud_to_fourcc(u8 format)
 	switch (format) {
 	case GUD_PIXEL_FORMAT_R1:
 		return GUD_DRM_FORMAT_R1;
+	case GUD_PIXEL_FORMAT_R8:
+		return DRM_FORMAT_R8;
 	case GUD_PIXEL_FORMAT_XRGB1111:
 		return GUD_DRM_FORMAT_XRGB1111;
 	case GUD_PIXEL_FORMAT_RGB565:
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index b9b0e435ea0f..be4f95b2d59c 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -189,6 +189,8 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 				ret = -ENOMEM;
 				goto end_cpu_access;
 			}
+		} else if (format->format == DRM_FORMAT_R8) {
+			drm_fb_xrgb8888_to_gray8(buf, vaddr, fb, rect);
 		} else if (format->format == DRM_FORMAT_RGB565) {
 			drm_fb_xrgb8888_to_rgb565(buf, vaddr, fb, rect, gud_is_big_endian());
 		} else {
diff --git a/include/drm/gud.h b/include/drm/gud.h
index 0b46b54fe56e..1dc781009e62 100644
--- a/include/drm/gud.h
+++ b/include/drm/gud.h
@@ -246,8 +246,8 @@ struct gud_state_req {
 /* Get supported pixel formats as a byte array of GUD_PIXEL_FORMAT_* */
 #define GUD_REQ_GET_FORMATS				0x40
   #define GUD_FORMATS_MAX_NUM			32
-  /* R1 is a 1-bit monochrome transfer format presented to userspace as XRGB8888 */
-  #define GUD_PIXEL_FORMAT_R1			0x01
+  #define GUD_PIXEL_FORMAT_R1			0x01 /* 1-bit monochrome */
+  #define GUD_PIXEL_FORMAT_R8			0x08 /* 8-bit greyscale */
   #define GUD_PIXEL_FORMAT_XRGB1111		0x20
   #define GUD_PIXEL_FORMAT_RGB565		0x40
   #define GUD_PIXEL_FORMAT_XRGB8888		0x80
-- 
2.32.0

[PATCH v2 5/7] drm/gud: Add GUD_PIXEL_FORMAT_RGB332

[Noralf Trønnes]

Add support for the RGB332 pixel format.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/gud/gud_drv.c      | 2 ++
 drivers/gpu/drm/gud/gud_internal.h | 4 ++++
 drivers/gpu/drm/gud/gud_pipe.c     | 2 ++
 include/drm/gud.h                  | 1 +
 4 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index a8d76c76e868..e571ad10a12b 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -526,6 +526,8 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
 		case DRM_FORMAT_R8:
 			fallthrough;
 		case GUD_DRM_FORMAT_XRGB1111:
+			fallthrough;
+		case DRM_FORMAT_RGB332:
 			if (!xrgb8888_emulation_format)
 				xrgb8888_emulation_format = info;
 			break;
diff --git a/drivers/gpu/drm/gud/gud_internal.h b/drivers/gpu/drm/gud/gud_internal.h
index 8499e713dbbc..249e02d1f5ed 100644
--- a/drivers/gpu/drm/gud/gud_internal.h
+++ b/drivers/gpu/drm/gud/gud_internal.h
@@ -84,6 +84,8 @@ static inline u8 gud_from_fourcc(u32 fourcc)
 		return GUD_PIXEL_FORMAT_R8;
 	case GUD_DRM_FORMAT_XRGB1111:
 		return GUD_PIXEL_FORMAT_XRGB1111;
+	case DRM_FORMAT_RGB332:
+		return GUD_PIXEL_FORMAT_RGB332;
 	case DRM_FORMAT_RGB565:
 		return GUD_PIXEL_FORMAT_RGB565;
 	case DRM_FORMAT_XRGB8888:
@@ -104,6 +106,8 @@ static inline u32 gud_to_fourcc(u8 format)
 		return DRM_FORMAT_R8;
 	case GUD_PIXEL_FORMAT_XRGB1111:
 		return GUD_DRM_FORMAT_XRGB1111;
+	case GUD_PIXEL_FORMAT_RGB332:
+		return DRM_FORMAT_RGB332;
 	case GUD_PIXEL_FORMAT_RGB565:
 		return DRM_FORMAT_RGB565;
 	case GUD_PIXEL_FORMAT_XRGB8888:
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index be4f95b2d59c..868a0b8a1f3e 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -191,6 +191,8 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 			}
 		} else if (format->format == DRM_FORMAT_R8) {
 			drm_fb_xrgb8888_to_gray8(buf, vaddr, fb, rect);
+		} else if (format->format == DRM_FORMAT_RGB332) {
+			drm_fb_xrgb8888_to_rgb332(buf, vaddr, fb, rect);
 		} else if (format->format == DRM_FORMAT_RGB565) {
 			drm_fb_xrgb8888_to_rgb565(buf, vaddr, fb, rect, gud_is_big_endian());
 		} else {
diff --git a/include/drm/gud.h b/include/drm/gud.h
index 1dc781009e62..4118dce2fcec 100644
--- a/include/drm/gud.h
+++ b/include/drm/gud.h
@@ -249,6 +249,7 @@ struct gud_state_req {
   #define GUD_PIXEL_FORMAT_R1			0x01 /* 1-bit monochrome */
   #define GUD_PIXEL_FORMAT_R8			0x08 /* 8-bit greyscale */
   #define GUD_PIXEL_FORMAT_XRGB1111		0x20
+  #define GUD_PIXEL_FORMAT_RGB332		0x30
   #define GUD_PIXEL_FORMAT_RGB565		0x40
   #define GUD_PIXEL_FORMAT_XRGB8888		0x80
   #define GUD_PIXEL_FORMAT_ARGB8888		0x81
-- 
2.32.0

[PATCH v2 6/7] drm/gud: Add GUD_PIXEL_FORMAT_RGB888

[Noralf Trønnes]

Add support for the RGB888 pixel format.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/gud/gud_drv.c      | 2 ++
 drivers/gpu/drm/gud/gud_internal.h | 4 ++++
 drivers/gpu/drm/gud/gud_pipe.c     | 2 ++
 include/drm/gud.h                  | 1 +
 4 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index e571ad10a12b..3f9d4b9a1e3d 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -528,6 +528,8 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
 		case GUD_DRM_FORMAT_XRGB1111:
 			fallthrough;
 		case DRM_FORMAT_RGB332:
+			fallthrough;
+		case DRM_FORMAT_RGB888:
 			if (!xrgb8888_emulation_format)
 				xrgb8888_emulation_format = info;
 			break;
diff --git a/drivers/gpu/drm/gud/gud_internal.h b/drivers/gpu/drm/gud/gud_internal.h
index 249e02d1f5ed..e351a1f1420d 100644
--- a/drivers/gpu/drm/gud/gud_internal.h
+++ b/drivers/gpu/drm/gud/gud_internal.h
@@ -88,6 +88,8 @@ static inline u8 gud_from_fourcc(u32 fourcc)
 		return GUD_PIXEL_FORMAT_RGB332;
 	case DRM_FORMAT_RGB565:
 		return GUD_PIXEL_FORMAT_RGB565;
+	case DRM_FORMAT_RGB888:
+		return GUD_PIXEL_FORMAT_RGB888;
 	case DRM_FORMAT_XRGB8888:
 		return GUD_PIXEL_FORMAT_XRGB8888;
 	case DRM_FORMAT_ARGB8888:
@@ -110,6 +112,8 @@ static inline u32 gud_to_fourcc(u8 format)
 		return DRM_FORMAT_RGB332;
 	case GUD_PIXEL_FORMAT_RGB565:
 		return DRM_FORMAT_RGB565;
+	case GUD_PIXEL_FORMAT_RGB888:
+		return DRM_FORMAT_RGB888;
 	case GUD_PIXEL_FORMAT_XRGB8888:
 		return DRM_FORMAT_XRGB8888;
 	case GUD_PIXEL_FORMAT_ARGB8888:
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index 868a0b8a1f3e..daf75c178c2b 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -195,6 +195,8 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 			drm_fb_xrgb8888_to_rgb332(buf, vaddr, fb, rect);
 		} else if (format->format == DRM_FORMAT_RGB565) {
 			drm_fb_xrgb8888_to_rgb565(buf, vaddr, fb, rect, gud_is_big_endian());
+		} else if (format->format == DRM_FORMAT_RGB888) {
+			drm_fb_xrgb8888_to_rgb888(buf, vaddr, fb, rect);
 		} else {
 			len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect);
 		}
diff --git a/include/drm/gud.h b/include/drm/gud.h
index 4118dce2fcec..c52a8ba4ae4e 100644
--- a/include/drm/gud.h
+++ b/include/drm/gud.h
@@ -251,6 +251,7 @@ struct gud_state_req {
   #define GUD_PIXEL_FORMAT_XRGB1111		0x20
   #define GUD_PIXEL_FORMAT_RGB332		0x30
   #define GUD_PIXEL_FORMAT_RGB565		0x40
+  #define GUD_PIXEL_FORMAT_RGB888		0x50
   #define GUD_PIXEL_FORMAT_XRGB8888		0x80
   #define GUD_PIXEL_FORMAT_ARGB8888		0x81
 
-- 
2.32.0

[PATCH v2 7/7] drm/gud: Add module parameter to control emulation: xrgb8888

[Noralf Trønnes]

For devices that don't support XRGB8888 give the user the ability to
choose what's most important: Color depth or frames per second.

Add an 'xrgb8888' module parameter to override the emulation format.

Assume the user wants full control if xrgb8888 is set and don't set
DRM_CAP_DUMB_PREFERRED_DEPTH if RGB565 is supported (AFAIK only X.org
supports this).

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/gud/gud_drv.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index 3f9d4b9a1e3d..60d27ee5ddbd 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -30,6 +30,10 @@
 
 #include "gud_internal.h"
 
+static int gud_xrgb8888;
+module_param_named(xrgb8888, gud_xrgb8888, int, 0644);
+MODULE_PARM_DESC(xrgb8888, "XRGB8888 emulation format: GUD_PIXEL_FORMAT_* value, 0=auto, -1=disable [default=auto]");
+
 /* Only used internally */
 static const struct drm_format_info gud_drm_format_r1 = {
 	.format = GUD_DRM_FORMAT_R1,
@@ -530,12 +534,12 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
 		case DRM_FORMAT_RGB332:
 			fallthrough;
 		case DRM_FORMAT_RGB888:
-			if (!xrgb8888_emulation_format)
+			if (!gud_xrgb8888 && !xrgb8888_emulation_format)
 				xrgb8888_emulation_format = info;
 			break;
 		case DRM_FORMAT_RGB565:
 			rgb565_supported = true;
-			if (!xrgb8888_emulation_format)
+			if (!gud_xrgb8888 && !xrgb8888_emulation_format)
 				xrgb8888_emulation_format = info;
 			break;
 		case DRM_FORMAT_XRGB8888:
@@ -543,6 +547,9 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
 			break;
 		}
 
+		if (gud_xrgb8888 == formats_dev[i])
+			xrgb8888_emulation_format = info;
+
 		fmt_buf_size = drm_format_info_min_pitch(info, 0, drm->mode_config.max_width) *
 			       drm->mode_config.max_height;
 		max_buffer_size = max(max_buffer_size, fmt_buf_size);
@@ -559,7 +566,7 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	}
 
 	/* Prefer speed over color depth */
-	if (rgb565_supported)
+	if (!gud_xrgb8888 && rgb565_supported)
 		drm->mode_config.preferred_depth = 16;
 
 	if (!xrgb8888_supported && xrgb8888_emulation_format) {
-- 
2.32.0

Re: [PATCH v2 7/7] drm/gud: Add module parameter to control emulation: xrgb8888

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3170 bytes --]

Hi

Am 07.09.21 um 13:57 schrieb Noralf Trønnes:
> For devices that don't support XRGB8888 give the user the ability to
> choose what's most important: Color depth or frames per second.
> 
> Add an 'xrgb8888' module parameter to override the emulation format.
> 
> Assume the user wants full control if xrgb8888 is set and don't set
> DRM_CAP_DUMB_PREFERRED_DEPTH if RGB565 is supported (AFAIK only X.org
> supports this).

More of a general statement: wouldn't it make more sense to auto-detect 
this entirely? The GUD protocol could order the list of supported 
formats by preference (maybe it does already). Or you could take the 
type of USB connection into account.

Additionally, xrgb8888 is really a fall-back for lazy userspace 
programs, but userspace should do better IMHO.

Best regards
Thomas

> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>   drivers/gpu/drm/gud/gud_drv.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
> index 3f9d4b9a1e3d..60d27ee5ddbd 100644
> --- a/drivers/gpu/drm/gud/gud_drv.c
> +++ b/drivers/gpu/drm/gud/gud_drv.c
> @@ -30,6 +30,10 @@
>   
>   #include "gud_internal.h"
>   
> +static int gud_xrgb8888;
> +module_param_named(xrgb8888, gud_xrgb8888, int, 0644);
> +MODULE_PARM_DESC(xrgb8888, "XRGB8888 emulation format: GUD_PIXEL_FORMAT_* value, 0=auto, -1=disable [default=auto]");
> +
>   /* Only used internally */
>   static const struct drm_format_info gud_drm_format_r1 = {
>   	.format = GUD_DRM_FORMAT_R1,
> @@ -530,12 +534,12 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
>   		case DRM_FORMAT_RGB332:
>   			fallthrough;
>   		case DRM_FORMAT_RGB888:
> -			if (!xrgb8888_emulation_format)
> +			if (!gud_xrgb8888 && !xrgb8888_emulation_format)
>   				xrgb8888_emulation_format = info;
>   			break;
>   		case DRM_FORMAT_RGB565:
>   			rgb565_supported = true;
> -			if (!xrgb8888_emulation_format)
> +			if (!gud_xrgb8888 && !xrgb8888_emulation_format)
>   				xrgb8888_emulation_format = info;
>   			break;
>   		case DRM_FORMAT_XRGB8888:
> @@ -543,6 +547,9 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
>   			break;
>   		}
>   
> +		if (gud_xrgb8888 == formats_dev[i])
> +			xrgb8888_emulation_format = info;
> +
>   		fmt_buf_size = drm_format_info_min_pitch(info, 0, drm->mode_config.max_width) *
>   			       drm->mode_config.max_height;
>   		max_buffer_size = max(max_buffer_size, fmt_buf_size);
> @@ -559,7 +566,7 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
>   	}
>   
>   	/* Prefer speed over color depth */
> -	if (rgb565_supported)
> +	if (!gud_xrgb8888 && rgb565_supported)
>   		drm->mode_config.preferred_depth = 16;
>   
>   	if (!xrgb8888_supported && xrgb8888_emulation_format) {
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2 7/7] drm/gud: Add module parameter to control emulation: xrgb8888

[Noralf Trønnes]

Den 08.09.2021 20.10, skrev Thomas Zimmermann:
> Hi
> 
> Am 07.09.21 um 13:57 schrieb Noralf Trønnes:
>> For devices that don't support XRGB8888 give the user the ability to
>> choose what's most important: Color depth or frames per second.
>>
>> Add an 'xrgb8888' module parameter to override the emulation format.
>>
>> Assume the user wants full control if xrgb8888 is set and don't set
>> DRM_CAP_DUMB_PREFERRED_DEPTH if RGB565 is supported (AFAIK only X.org
>> supports this).
> 
> More of a general statement: wouldn't it make more sense to auto-detect
> this entirely? The GUD protocol could order the list of supported
> formats by preference (maybe it does already). Or you could take the
> type of USB connection into account.
> 

There is preference in that the driver will use xrgb8888 emulation on
the first format returned by the device. Yeah USB speed could be used as
a metric for preferred depth, but I don't know of anyone else than X.org
using it so I'm unsure about the value.

Daniel mentioned that this parameter targets all devices, so it's not a
very good solution.

Maybe I should drop this patch and gather more user experience with the
depth/speed challenge before trying to come up with a solution.

> Additionally, xrgb8888 is really a fall-back for lazy userspace
> programs, but userspace should do better IMHO.
> 

I don't think even the most agile userspace supports monochrome,
greyscale or even RGB332 ;)

Noralf.

> Best regards
> Thomas
> 
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/gud/gud_drv.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/gud/gud_drv.c
>> b/drivers/gpu/drm/gud/gud_drv.c
>> index 3f9d4b9a1e3d..60d27ee5ddbd 100644
>> --- a/drivers/gpu/drm/gud/gud_drv.c
>> +++ b/drivers/gpu/drm/gud/gud_drv.c
>> @@ -30,6 +30,10 @@
>>     #include "gud_internal.h"
>>   +static int gud_xrgb8888;
>> +module_param_named(xrgb8888, gud_xrgb8888, int, 0644);
>> +MODULE_PARM_DESC(xrgb8888, "XRGB8888 emulation format:
>> GUD_PIXEL_FORMAT_* value, 0=auto, -1=disable [default=auto]");
>> +
>>   /* Only used internally */
>>   static const struct drm_format_info gud_drm_format_r1 = {
>>       .format = GUD_DRM_FORMAT_R1,
>> @@ -530,12 +534,12 @@ static int gud_probe(struct usb_interface *intf,
>> const struct usb_device_id *id)
>>           case DRM_FORMAT_RGB332:
>>               fallthrough;
>>           case DRM_FORMAT_RGB888:
>> -            if (!xrgb8888_emulation_format)
>> +            if (!gud_xrgb8888 && !xrgb8888_emulation_format)
>>                   xrgb8888_emulation_format = info;
>>               break;
>>           case DRM_FORMAT_RGB565:
>>               rgb565_supported = true;
>> -            if (!xrgb8888_emulation_format)
>> +            if (!gud_xrgb8888 && !xrgb8888_emulation_format)
>>                   xrgb8888_emulation_format = info;
>>               break;
>>           case DRM_FORMAT_XRGB8888:
>> @@ -543,6 +547,9 @@ static int gud_probe(struct usb_interface *intf,
>> const struct usb_device_id *id)
>>               break;
>>           }
>>   +        if (gud_xrgb8888 == formats_dev[i])
>> +            xrgb8888_emulation_format = info;
>> +
>>           fmt_buf_size = drm_format_info_min_pitch(info, 0,
>> drm->mode_config.max_width) *
>>                      drm->mode_config.max_height;
>>           max_buffer_size = max(max_buffer_size, fmt_buf_size);
>> @@ -559,7 +566,7 @@ static int gud_probe(struct usb_interface *intf,
>> const struct usb_device_id *id)
>>       }
>>         /* Prefer speed over color depth */
>> -    if (rgb565_supported)
>> +    if (!gud_xrgb8888 && rgb565_supported)
>>           drm->mode_config.preferred_depth = 16;
>>         if (!xrgb8888_supported && xrgb8888_emulation_format) {
>>
>