diff mbox series

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

Message ID 20210817122917.49929-3-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show
Series drm/gud: Add some more pixel formats | expand

Commit Message

Noralf Trønnes Aug. 17, 2021, 12:29 p.m. UTC
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(+)

Comments

Daniel Vetter Aug. 17, 2021, 1:56 p.m. UTC | #1
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
>
Peter Stuge Aug. 17, 2021, 4:03 p.m. UTC | #2
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
Noralf Trønnes Aug. 30, 2021, 12:08 p.m. UTC | #3
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
>>
>
Daniel Vetter Aug. 31, 2021, 12:23 p.m. UTC | #4
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
> >>
> >
Noralf Trønnes Sept. 2, 2021, 2:08 p.m. UTC | #5
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
>>>>
>>>
>
Daniel Vetter Sept. 2, 2021, 2:24 p.m. UTC | #6
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
> >>>>
> >>>
> >
diff mbox series

Patch

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