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

[Daniel Vetter <daniel@ffwll.ch>]

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