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

[PATCH 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

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 | 85 +++++++++++++++++++++++++++++
 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, 128 insertions(+), 5 deletions(-)

-- 
2.32.0

[PATCH 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.

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

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

[Daniel Vetter]

On Tue, Aug 17, 2021 at 02:29:11PM +0200, Noralf Trønnes wrote:
> Add an entry in drm_format_info for the existing format DRM_FORMAT_R8.
> 
> 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 },

Uh the depth = 0 on these is all a big lie, same for the 332 formats
below. The only format which is officially depth = 8 is the C8 one. I'm
not sure it's a great idea to announce others as depth = 8 ...

Might be good to throw a patch on top to drop these. Same for the ARGB1555
ones and it's permutations.

Anyway it's consistent with what's there.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>  		{ .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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Daniel Vetter]

On Tue, Aug 17, 2021 at 03:59:56PM +0200, Daniel Vetter wrote:
> On Tue, Aug 17, 2021 at 02:29:11PM +0200, Noralf Trønnes wrote:
> > Add an entry in drm_format_info for the existing format DRM_FORMAT_R8.
> > 
> > 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 },
> 
> Uh the depth = 0 on these is all a big lie, same for the 332 formats
> below. The only format which is officially depth = 8 is the C8 one. I'm
> not sure it's a great idea to announce others as depth = 8 ...

btw if you really want to go fancy, full dithering for these might not be
a complete waste of effort to type it. Temporal dithering probably too
tricky, and also too many folks don't like that.
-Daniel

> 
> Might be good to throw a patch on top to drop these. Same for the ARGB1555
> ones and it's permutations.
> 
> Anyway it's consistent with what's there.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >  		{ .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
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 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.

Cc: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_format_helper.c | 47 +++++++++++++++++++++++++++++
 include/drm/drm_format_helper.h     |  2 ++
 2 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 5231104b1498..53b426da7467 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -135,6 +135,53 @@ 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, u32 *sbuf, unsigned int pixels)
+{
+	unsigned int x;
+
+	for (x = 0; x < pixels; x++)
+		dbuf[x] = ((sbuf[x] & 0x00e00000) >> 16) |
+			  ((sbuf[x] & 0x0000e000) >> 11) |
+			  ((sbuf[x] & 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

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

[Daniel Vetter]

On Tue, Aug 17, 2021 at 02:29:12PM +0200, Noralf Trønnes wrote:
> Add XRGB8888 emulation support for devices that can only do RGB332.
> 
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/drm_format_helper.c | 47 +++++++++++++++++++++++++++++
>  include/drm/drm_format_helper.h     |  2 ++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 5231104b1498..53b426da7467 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -135,6 +135,53 @@ 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, u32 *sbuf, unsigned int pixels)
> +{
> +	unsigned int x;
> +
> +	for (x = 0; x < pixels; x++)
> +		dbuf[x] = ((sbuf[x] & 0x00e00000) >> 16) |

I think for 2/3 bits correct rounding would be useful, not just masking.
i.e. before you shift add 0x00100000 here, and similar below.

Also I just realized we've totally ignored endianess on these, which is
not great, because strictly speaking all the drm_fourcc codes should be
little endian. But I'm also not sure that's worth fixing ...

Either way, lgtm:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +			  ((sbuf[x] & 0x0000e000) >> 11) |
> +			  ((sbuf[x] & 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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Peter Stuge]

Daniel Vetter wrote:
> Also I just realized we've totally ignored endianess on these, which is
> not great, because strictly speaking all the drm_fourcc codes should be
> little endian. But I'm also not sure that's worth fixing ...

We discussed framebuffer endianess when introducing the driver,
in the thread linked near the FIXME comment in the code.

I proposed an untested fix but Noralf wanted to wait for testing,
which I find fair. I don't think anyone has tested on BE yet.

It's on my nice-to-have list, but not at the top, and has blockers,
so if anyone else can test on BE please do. I'd recommend testing
with an actual device to compare LE and BE behavior easily.


Kind regards

//Peter

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

[Noralf Trønnes]

Den 17.08.2021 15.56, skrev Daniel Vetter:
> On Tue, Aug 17, 2021 at 02:29:12PM +0200, Noralf Trønnes wrote:
>> Add XRGB8888 emulation support for devices that can only do RGB332.
>>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>  drivers/gpu/drm/drm_format_helper.c | 47 +++++++++++++++++++++++++++++
>>  include/drm/drm_format_helper.h     |  2 ++
>>  2 files changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
>> index 5231104b1498..53b426da7467 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
>> @@ -135,6 +135,53 @@ 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, u32 *sbuf, unsigned int pixels)
>> +{
>> +	unsigned int x;
>> +
>> +	for (x = 0; x < pixels; x++)
>> +		dbuf[x] = ((sbuf[x] & 0x00e00000) >> 16) |
> 
> I think for 2/3 bits correct rounding would be useful, not just masking.
> i.e. before you shift add 0x00100000 here, and similar below.
> 

Math isn't my strongest side and my brain failed to turn this into code.

> Also I just realized we've totally ignored endianess on these, which is
> not great, because strictly speaking all the drm_fourcc codes should be
> little endian. But I'm also not sure that's worth fixing ...
> 

Is it as simple as using le32_to_cpu()?

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);
	}
}

Noralf.

> Either way, lgtm:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
>> +			  ((sbuf[x] & 0x0000e000) >> 11) |
>> +			  ((sbuf[x] & 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
>>
> 

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

[Daniel Vetter]

On Mon, Aug 30, 2021 at 02:08:14PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 17.08.2021 15.56, skrev Daniel Vetter:
> > On Tue, Aug 17, 2021 at 02:29:12PM +0200, Noralf Trønnes wrote:
> >> Add XRGB8888 emulation support for devices that can only do RGB332.
> >>
> >> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >> ---
> >>  drivers/gpu/drm/drm_format_helper.c | 47 +++++++++++++++++++++++++++++
> >>  include/drm/drm_format_helper.h     |  2 ++
> >>  2 files changed, 49 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> >> index 5231104b1498..53b426da7467 100644
> >> --- a/drivers/gpu/drm/drm_format_helper.c
> >> +++ b/drivers/gpu/drm/drm_format_helper.c
> >> @@ -135,6 +135,53 @@ 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, u32 *sbuf, unsigned int pixels)
> >> +{
> >> +	unsigned int x;
> >> +
> >> +	for (x = 0; x < pixels; x++)
> >> +		dbuf[x] = ((sbuf[x] & 0x00e00000) >> 16) |
> > 
> > I think for 2/3 bits correct rounding would be useful, not just masking.
> > i.e. before you shift add 0x00100000 here, and similar below.
> > 
> 
> Math isn't my strongest side and my brain failed to turn this into code.

Essentially add half of the lowest bit before you mask, so

((sbuf[x] + 0x10) & 0xe0 )

I dropped the shift to make it clear what's going on. If you're mask is
0xc0, then you simply add 0x20 before masking.

> > Also I just realized we've totally ignored endianess on these, which is
> > not great, because strictly speaking all the drm_fourcc codes should be
> > little endian. But I'm also not sure that's worth fixing ...
> > 
> 
> Is it as simple as using le32_to_cpu()?

I think so.

Plus on any format that has u16 output we'd need a cpu_to_le16 wrapped
around the entire thing.
-Daniel

> 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);
> 	}
> }
> 
> Noralf.
> 
> > Either way, lgtm:
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> >> +			  ((sbuf[x] & 0x0000e000) >> 11) |
> >> +			  ((sbuf[x] & 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
> >>
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Noralf Trønnes]

Den 31.08.2021 14.23, skrev Daniel Vetter:
> On Mon, Aug 30, 2021 at 02:08:14PM +0200, Noralf Trønnes wrote:
>>
>>
>> Den 17.08.2021 15.56, skrev Daniel Vetter:
>>> On Tue, Aug 17, 2021 at 02:29:12PM +0200, Noralf Trønnes wrote:
>>>> Add XRGB8888 emulation support for devices that can only do RGB332.
>>>>
>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>> ---
>>>>  drivers/gpu/drm/drm_format_helper.c | 47 +++++++++++++++++++++++++++++
>>>>  include/drm/drm_format_helper.h     |  2 ++
>>>>  2 files changed, 49 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
>>>> index 5231104b1498..53b426da7467 100644
>>>> --- a/drivers/gpu/drm/drm_format_helper.c
>>>> +++ b/drivers/gpu/drm/drm_format_helper.c
>>>> @@ -135,6 +135,53 @@ 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, u32 *sbuf, unsigned int pixels)
>>>> +{
>>>> +	unsigned int x;
>>>> +
>>>> +	for (x = 0; x < pixels; x++)
>>>> +		dbuf[x] = ((sbuf[x] & 0x00e00000) >> 16) |
>>>
>>> I think for 2/3 bits correct rounding would be useful, not just masking.
>>> i.e. before you shift add 0x00100000 here, and similar below.
>>>
>>
>> Math isn't my strongest side and my brain failed to turn this into code.
> 
> Essentially add half of the lowest bit before you mask, so
> 
> ((sbuf[x] + 0x10) & 0xe0 )
> 

But what if the value is 0xff, it overflows:

((0xff + 0x10) & 0xe0 ) == 0x00

Should it be OR?

((0xff | 0x10) & 0xe0 ) == 0xe0

Noralf.

> I dropped the shift to make it clear what's going on. If you're mask is
> 0xc0, then you simply add 0x20 before masking.
> 
>>> Also I just realized we've totally ignored endianess on these, which is
>>> not great, because strictly speaking all the drm_fourcc codes should be
>>> little endian. But I'm also not sure that's worth fixing ...
>>>
>>
>> Is it as simple as using le32_to_cpu()?
> 
> I think so.
> 
> Plus on any format that has u16 output we'd need a cpu_to_le16 wrapped
> around the entire thing.
> -Daniel
> 
>> 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);
>> 	}
>> }
>>
>> Noralf.
>>
>>> Either way, lgtm:
>>>
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>
>>>> +			  ((sbuf[x] & 0x0000e000) >> 11) |
>>>> +			  ((sbuf[x] & 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
>>>>
>>>
> 

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

[Daniel Vetter]

On Thu, Sep 02, 2021 at 04:08:14PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 31.08.2021 14.23, skrev Daniel Vetter:
> > On Mon, Aug 30, 2021 at 02:08:14PM +0200, Noralf Trønnes wrote:
> >>
> >>
> >> Den 17.08.2021 15.56, skrev Daniel Vetter:
> >>> On Tue, Aug 17, 2021 at 02:29:12PM +0200, Noralf Trønnes wrote:
> >>>> Add XRGB8888 emulation support for devices that can only do RGB332.
> >>>>
> >>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >>>> ---
> >>>>  drivers/gpu/drm/drm_format_helper.c | 47 +++++++++++++++++++++++++++++
> >>>>  include/drm/drm_format_helper.h     |  2 ++
> >>>>  2 files changed, 49 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> >>>> index 5231104b1498..53b426da7467 100644
> >>>> --- a/drivers/gpu/drm/drm_format_helper.c
> >>>> +++ b/drivers/gpu/drm/drm_format_helper.c
> >>>> @@ -135,6 +135,53 @@ 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, u32 *sbuf, unsigned int pixels)
> >>>> +{
> >>>> +	unsigned int x;
> >>>> +
> >>>> +	for (x = 0; x < pixels; x++)
> >>>> +		dbuf[x] = ((sbuf[x] & 0x00e00000) >> 16) |
> >>>
> >>> I think for 2/3 bits correct rounding would be useful, not just masking.
> >>> i.e. before you shift add 0x00100000 here, and similar below.
> >>>
> >>
> >> Math isn't my strongest side and my brain failed to turn this into code.
> > 
> > Essentially add half of the lowest bit before you mask, so
> > 
> > ((sbuf[x] + 0x10) & 0xe0 )
> > 
> 
> But what if the value is 0xff, it overflows:

Your math is better than mine ...

> ((0xff + 0x10) & 0xe0 ) == 0x00
> 
> Should it be OR?
> 
> ((0xff | 0x10) & 0xe0 ) == 0xe0

Probably need a max on top to limit the overflow:

max(sbuf[x] + 0x10, 0xe)

But also maybe really not worth the head-scratching :-)
-Daniel

> 
> Noralf.
> 
> > I dropped the shift to make it clear what's going on. If you're mask is
> > 0xc0, then you simply add 0x20 before masking.
> > 
> >>> Also I just realized we've totally ignored endianess on these, which is
> >>> not great, because strictly speaking all the drm_fourcc codes should be
> >>> little endian. But I'm also not sure that's worth fixing ...
> >>>
> >>
> >> Is it as simple as using le32_to_cpu()?
> > 
> > I think so.
> > 
> > Plus on any format that has u16 output we'd need a cpu_to_le16 wrapped
> > around the entire thing.
> > -Daniel
> > 
> >> 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);
> >> 	}
> >> }
> >>
> >> Noralf.
> >>
> >>> Either way, lgtm:
> >>>
> >>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>>
> >>>> +			  ((sbuf[x] & 0x0000e000) >> 11) |
> >>>> +			  ((sbuf[x] & 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
> >>>>
> >>>
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 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>
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 53b426da7467..c42d50315123 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -297,6 +297,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

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

[Daniel Vetter]

On Tue, Aug 17, 2021 at 02:29:13PM +0200, Noralf Trønnes wrote:
> Add XRGB8888 emulation support for devices that can only do RGB888.
> 
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> 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 53b426da7467..c42d50315123 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -297,6 +297,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)

I do wonder whether we really need all the combinations here. E.g. if we
allow dst_pitch == 0 and just automatically pick this one here, and then
paper over the __iomem differences with dma_buf_map we wouldn't need two
different functions.

Just an idea.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> +{
> +	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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 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 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 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 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 7/7] drm/gud: Add module parameter to control emulation: xrgb8888

[Daniel Vetter]

On Tue, Aug 17, 2021 at 02:29:17PM +0200, Noralf Trønnes wrote:
> 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;

Shouldn't the emulation format be per drm_device instance?
-Daniel

>  			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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Noralf Trønnes]

Den 17.08.2021 15.49, skrev Daniel Vetter:
> On Tue, Aug 17, 2021 at 02:29:17PM +0200, Noralf Trønnes wrote:
>> 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;
> 
> Shouldn't the emulation format be per drm_device instance?

Ideally yes, this happens during probe so I can only use a module
parameter and I don't know how to differenciate the devices since the
DRM minor is unknown at this point and implementing filtering on the
various USB properties (VID:PID, serial number,..) will involve a lot of
code. So I've kept it simple. It can be expanded on later should someone
come up with a clever idea.

Noralf.

> -Daniel
> 
>>  			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
>>
>