[PATCH 0/4] drm/format-helper: Share common code among conversion helpers

[PATCH 0/4] drm/format-helper: Share common code among conversion helpers

[Thomas Zimmermann]

Move all format-specific handling in per-line conversion functions and
share the overall loop among conversion helpers. This is another step
towards composable format conversion. 

Thomas Zimmermann (4):
  drm/format-helper: Implement drm_fb_swab() with per-line helpers
  drm/format-helper: Remove optional byte-swap from line convertion
  drm/format-helper: Unify the parameters of all per-line conversion
    helpers
  drm/format-helper: Share implementation among conversion helpers

 drivers/gpu/drm/drm_format_helper.c | 479 ++++++++++++----------------
 1 file changed, 198 insertions(+), 281 deletions(-)

-- 
2.36.0

[PATCH 1/4] drm/format-helper: Implement drm_fb_swab() with per-line helpers

[Thomas Zimmermann]

Replace the inner loop of drm_fb_swab() with helper functions that
swap the bytes in each pixel. This will allow to share the outer
loop with other conversion helpers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 60 +++++++++++++++++------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 34b7ef443ad2..f70499344a04 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -100,6 +100,26 @@ void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *v
 }
 EXPORT_SYMBOL(drm_fb_memcpy_toio);
 
+static void drm_fb_swab16_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	u16 *dbuf16 = dbuf;
+	const u16 *sbuf16 = sbuf;
+	const u16 *send16 = sbuf16 + pixels;
+
+	while (sbuf16 < send16)
+		*dbuf16++ = swab16(*sbuf16++);
+}
+
+static void drm_fb_swab32_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	u32 *dbuf32 = dbuf;
+	const u32 *sbuf32 = sbuf;
+	const u32 *send32 = sbuf32 + pixels;
+
+	while (sbuf32 < send32)
+		*dbuf32++ = swab32(*sbuf32++);
+}
+
 /**
  * drm_fb_swab - Swap bytes into clip buffer
  * @dst: Destination buffer
@@ -120,12 +140,11 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
 		 bool cached)
 {
 	u8 cpp = fb->format->cpp[0];
-	size_t len = drm_rect_width(clip) * cpp;
-	const u16 *src16;
-	const u32 *src32;
-	u16 *dst16;
-	u32 *dst32;
-	unsigned int x, y;
+	unsigned long linepixels = drm_rect_width(clip);
+	size_t len = linepixels * cpp;
+	const void *sbuf;
+	void *dbuf;
+	unsigned int y;
 	void *buf = NULL;
 
 	if (WARN_ON_ONCE(cpp != 2 && cpp != 4))
@@ -133,31 +152,22 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
 
 	if (!dst_pitch)
 		dst_pitch = len;
+	src += clip_offset(clip, fb->pitches[0], cpp);
 
 	if (!cached)
 		buf = kmalloc(len, GFP_KERNEL);
 
-	src += clip_offset(clip, fb->pitches[0], cpp);
-
 	for (y = clip->y1; y < clip->y2; y++) {
-		if (buf) {
-			memcpy(buf, src, len);
-			src16 = buf;
-			src32 = buf;
-		} else {
-			src16 = src;
-			src32 = src;
-		}
-
-		dst16 = dst;
-		dst32 = dst;
+		if (buf)
+			sbuf = memcpy(buf, src, len);
+		else
+			sbuf = src;
+		dbuf = dst + clip->x1 * cpp;
 
-		for (x = clip->x1; x < clip->x2; x++) {
-			if (cpp == 4)
-				*dst32++ = swab32(*src32++);
-			else
-				*dst16++ = swab16(*src16++);
-		}
+		if (cpp == 4)
+			drm_fb_swab32_line(dbuf, sbuf, linepixels);
+		else
+			drm_fb_swab16_line(dbuf, sbuf, linepixels);
 
 		src += fb->pitches[0];
 		dst += dst_pitch;
-- 
2.36.0

[PATCH 2/4] drm/format-helper: Remove optional byte-swap from line convertion

[Thomas Zimmermann]

Implement per-pixel byte swapping in a separate conversion helper
for the single function that requires it. Select the correct helper
for each conversion.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 32 +++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index f70499344a04..b7daa40fc856 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -229,8 +229,7 @@ void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *sr
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
 
 static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, const u32 *sbuf,
-					   unsigned int pixels,
-					   bool swab)
+					   unsigned int pixels)
 {
 	unsigned int x;
 	u16 val16;
@@ -239,10 +238,21 @@ static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, const u32 *sbuf,
 		val16 = ((sbuf[x] & 0x00F80000) >> 8) |
 			((sbuf[x] & 0x0000FC00) >> 5) |
 			((sbuf[x] & 0x000000F8) >> 3);
-		if (swab)
-			dbuf[x] = swab16(val16);
-		else
-			dbuf[x] = val16;
+		dbuf[x] = val16;
+	}
+}
+
+static void drm_fb_xrgb8888_to_rgb565_swab_line(u16 *dbuf, const u32 *sbuf,
+						unsigned int pixels)
+{
+	unsigned int x;
+	u16 val16;
+
+	for (x = 0; x < pixels; x++) {
+		val16 = ((sbuf[x] & 0x00F80000) >> 8) |
+			((sbuf[x] & 0x0000FC00) >> 5) |
+			((sbuf[x] & 0x000000F8) >> 3);
+		dbuf[x] = swab16(val16);
 	}
 }
 
@@ -282,7 +292,10 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, unsigned int dst_pitch, const void *va
 	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
 	for (y = 0; y < lines; y++) {
 		memcpy(sbuf, vaddr, src_len);
-		drm_fb_xrgb8888_to_rgb565_line(dst, sbuf, linepixels, swab);
+		if (swab)
+			drm_fb_xrgb8888_to_rgb565_swab_line(dst, sbuf, linepixels);
+		else
+			drm_fb_xrgb8888_to_rgb565_line(dst, sbuf, linepixels);
 		vaddr += fb->pitches[0];
 		dst += dst_pitch;
 	}
@@ -321,7 +334,10 @@ void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch,
 
 	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
 	for (y = 0; y < lines; y++) {
-		drm_fb_xrgb8888_to_rgb565_line(dbuf, vaddr, linepixels, swab);
+		if (swab)
+			drm_fb_xrgb8888_to_rgb565_swab_line(dbuf, vaddr, linepixels);
+		else
+			drm_fb_xrgb8888_to_rgb565_line(dbuf, vaddr, linepixels);
 		memcpy_toio(dst, dbuf, dst_len);
 		vaddr += fb->pitches[0];
 		dst += dst_pitch;
-- 
2.36.0

[PATCH 3/4] drm/format-helper: Unify the parameters of all per-line conversion helpers

[Thomas Zimmermann]

Give each per-line conversion helper pointers of type void and the
number of pixels in the line. Remove the unused swab parameters.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 87 +++++++++++++++++------------
 1 file changed, 50 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index b7daa40fc856..21d0d282c6a1 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -177,16 +177,19 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
 }
 EXPORT_SYMBOL(drm_fb_swab);
 
-static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, const __le32 *sbuf, unsigned int pixels)
+static void drm_fb_xrgb8888_to_rgb332_line(void *dbuf, const void *sbuf, unsigned int pixels,
+					   bool swab)
 {
+	u8 *dbuf8 = dbuf;
+	const __le32 *sbuf32 = sbuf;
 	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);
+		pix = le32_to_cpu(sbuf32[x]);
+		dbuf8[x] = ((pix & 0x00e00000) >> 16) |
+			   ((pix & 0x0000e000) >> 11) |
+			   ((pix & 0x000000c0) >> 6);
 	}
 }
 
@@ -219,7 +222,7 @@ void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *sr
 	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);
+		drm_fb_xrgb8888_to_rgb332_line(dst, sbuf, width, false);
 		src += fb->pitches[0];
 		dst += dst_pitch;
 	}
@@ -228,31 +231,34 @@ void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *sr
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
 
-static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, const u32 *sbuf,
-					   unsigned int pixels)
+static void drm_fb_xrgb8888_to_rgb565_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
+	u16 *dbuf16 = dbuf;
+	const u32 *sbuf32 = sbuf;
 	unsigned int x;
 	u16 val16;
 
 	for (x = 0; x < pixels; x++) {
-		val16 = ((sbuf[x] & 0x00F80000) >> 8) |
-			((sbuf[x] & 0x0000FC00) >> 5) |
-			((sbuf[x] & 0x000000F8) >> 3);
-		dbuf[x] = val16;
+		val16 = ((sbuf32[x] & 0x00F80000) >> 8) |
+			((sbuf32[x] & 0x0000FC00) >> 5) |
+			((sbuf32[x] & 0x000000F8) >> 3);
+		dbuf16[x] = val16;
 	}
 }
 
-static void drm_fb_xrgb8888_to_rgb565_swab_line(u16 *dbuf, const u32 *sbuf,
+static void drm_fb_xrgb8888_to_rgb565_swab_line(void *dbuf, const void *sbuf,
 						unsigned int pixels)
 {
+	u16 *dbuf16 = dbuf;
+	const u32 *sbuf32 = sbuf;
 	unsigned int x;
 	u16 val16;
 
 	for (x = 0; x < pixels; x++) {
-		val16 = ((sbuf[x] & 0x00F80000) >> 8) |
-			((sbuf[x] & 0x0000FC00) >> 5) |
-			((sbuf[x] & 0x000000F8) >> 3);
-		dbuf[x] = swab16(val16);
+		val16 = ((sbuf32[x] & 0x00F80000) >> 8) |
+			((sbuf32[x] & 0x0000FC00) >> 5) |
+			((sbuf32[x] & 0x000000F8) >> 3);
+		dbuf16[x] = swab16(val16);
 	}
 }
 
@@ -347,15 +353,16 @@ void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch,
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_toio);
 
-static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, const u32 *sbuf,
-					   unsigned int pixels)
+static void drm_fb_xrgb8888_to_rgb888_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
+	u8 *dbuf8 = dbuf;
+	const u32 *sbuf32 = sbuf;
 	unsigned int x;
 
 	for (x = 0; x < pixels; x++) {
-		*dbuf++ = (sbuf[x] & 0x000000FF) >>  0;
-		*dbuf++ = (sbuf[x] & 0x0000FF00) >>  8;
-		*dbuf++ = (sbuf[x] & 0x00FF0000) >> 16;
+		*dbuf8++ = (sbuf32[x] & 0x000000FF) >>  0;
+		*dbuf8++ = (sbuf32[x] & 0x0000FF00) >>  8;
+		*dbuf8++ = (sbuf32[x] & 0x00FF0000) >> 16;
 	}
 }
 
@@ -521,17 +528,18 @@ static void drm_fb_rgb888_to_xrgb8888_toio(void __iomem *dst, unsigned int dst_p
 	kfree(dbuf);
 }
 
-static void drm_fb_xrgb8888_to_xrgb2101010_line(u32 *dbuf, const u32 *sbuf,
-						unsigned int pixels)
+static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
+	u32 *dbuf32 = dbuf;
+	const u32 *sbuf32 = sbuf;
 	unsigned int x;
 	u32 val32;
 
 	for (x = 0; x < pixels; x++) {
-		val32 = ((sbuf[x] & 0x000000FF) << 2) |
-			((sbuf[x] & 0x0000FF00) << 4) |
-			((sbuf[x] & 0x00FF0000) << 6);
-		*dbuf++ = val32 | ((val32 >> 8) & 0x00300C03);
+		val32 = ((sbuf32[x] & 0x000000FF) << 2) |
+			((sbuf32[x] & 0x0000FF00) << 4) |
+			((sbuf32[x] & 0x00FF0000) << 6);
+		*dbuf32++ = val32 | ((val32 >> 8) & 0x00300C03);
 	}
 }
 
@@ -576,18 +584,20 @@ void drm_fb_xrgb8888_to_xrgb2101010_toio(void __iomem *dst,
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb2101010_toio);
 
-static void drm_fb_xrgb8888_to_gray8_line(u8 *dst, const u32 *src, unsigned int pixels)
+static void drm_fb_xrgb8888_to_gray8_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
+	u8 *dbuf8 = dbuf;
+	const u32 *sbuf32 = sbuf;
 	unsigned int x;
 
 	for (x = 0; x < pixels; x++) {
-		u8 r = (*src & 0x00ff0000) >> 16;
-		u8 g = (*src & 0x0000ff00) >> 8;
-		u8 b =  *src & 0x000000ff;
+		u8 r = (*sbuf32 & 0x00ff0000) >> 16;
+		u8 g = (*sbuf32 & 0x0000ff00) >> 8;
+		u8 b =  *sbuf32 & 0x000000ff;
 
 		/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
-		*dst++ = (3 * r + 6 * g + b) / 10;
-		src++;
+		*dbuf8++ = (3 * r + 6 * g + b) / 10;
+		sbuf32++;
 	}
 }
 
@@ -716,17 +726,20 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for
 EXPORT_SYMBOL(drm_fb_blit_toio);
 
 
-static void drm_fb_gray8_to_mono_line(u8 *dst, const u8 *src, unsigned int pixels)
+static void drm_fb_gray8_to_mono_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
+	u8 *dbuf8 = dbuf;
+	const u8 *sbuf8 = sbuf;
+
 	while (pixels) {
 		unsigned int i, bits = min(pixels, 8U);
 		u8 byte = 0;
 
 		for (i = 0; i < bits; i++, pixels--) {
-			if (*src++ >= 128)
+			if (*sbuf8++ >= 128)
 				byte |= BIT(i);
 		}
-		*dst++ = byte;
+		*dbuf8++ = byte;
 	}
 }
 
-- 
2.36.0

[PATCH 4/4] drm/format-helper: Share implementation among conversion helpers

[Thomas Zimmermann]

Provide format-independent conversion helpers for system and I/O
memory. Implement most existing helpers on top of it. The source and
destination formats of each conversion is handled by a per-line
helper that is given to the generic implementation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 370 ++++++++++------------------
 1 file changed, 124 insertions(+), 246 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 21d0d282c6a1..6f8030ebb56d 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -40,6 +40,95 @@ unsigned int drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info
 }
 EXPORT_SYMBOL(drm_fb_clip_offset);
 
+/* TODO: Make this functon work with multi-plane formats. */
+static int drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
+		       const void *vaddr, const struct drm_framebuffer *fb,
+		       const struct drm_rect *clip, bool vaddr_cached_hint,
+		       void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
+{
+	unsigned long linepixels = drm_rect_width(clip);
+	unsigned long lines = drm_rect_height(clip);
+	size_t sbuf_len = linepixels * fb->format->cpp[0];
+	void *stmp = NULL;
+	unsigned long i;
+	const void *sbuf;
+
+	/*
+	 * Some source buffers, such as CMA memory, use write-combine
+	 * caching, so reads are uncached. Speed up access by fetching
+	 * one line at a time.
+	 */
+	if (!vaddr_cached_hint) {
+		stmp = kmalloc(sbuf_len, GFP_KERNEL);
+		if (!stmp)
+			return -ENOMEM;
+	}
+
+	if (!dst_pitch)
+		dst_pitch = drm_rect_width(clip) * dst_pixsize;
+	vaddr += clip_offset(clip, fb->pitches[0], fb->format->cpp[0]);
+
+	for (i = 0; i < lines; ++i) {
+		if (stmp)
+			sbuf = memcpy(stmp, vaddr, sbuf_len);
+		else
+			sbuf = vaddr;
+		xfrm_line(dst, sbuf, linepixels);
+		vaddr += fb->pitches[0];
+		dst += dst_pitch;
+	}
+
+	kfree(stmp);
+
+	return 0;
+}
+
+/* TODO: Make this functon work with multi-plane formats. */
+static int drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
+			    const void *vaddr, const struct drm_framebuffer *fb,
+			    const struct drm_rect *clip, bool vaddr_cached_hint,
+			    void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
+{
+	unsigned long linepixels = drm_rect_width(clip);
+	unsigned long lines = drm_rect_height(clip);
+	size_t dbuf_len = linepixels * dst_pixsize;
+	size_t stmp_off = round_up(dbuf_len, ARCH_KMALLOC_MINALIGN); /* for sbuf alignment */
+	size_t sbuf_len = linepixels * fb->format->cpp[0];
+	void *stmp = NULL;
+	unsigned long i;
+	const void *sbuf;
+	void *dbuf;
+
+	if (vaddr_cached_hint) {
+		dbuf = kmalloc(dbuf_len, GFP_KERNEL);
+	} else {
+		dbuf = kmalloc(stmp_off + sbuf_len, GFP_KERNEL);
+		stmp = dbuf + stmp_off;
+	}
+	if (!dbuf)
+		return -ENOMEM;
+
+	if (!dst_pitch)
+		dst_pitch = linepixels * dst_pixsize;
+	vaddr += clip_offset(clip, fb->pitches[0], fb->format->cpp[0]);
+
+	for (i = 0; i < lines; ++i) {
+		if (stmp)
+			sbuf = memcpy(stmp, vaddr, sbuf_len);
+		else
+			sbuf = vaddr;
+		xfrm_line(dbuf, sbuf, linepixels);
+		memcpy_toio(dst, dbuf, dbuf_len);
+		vaddr += fb->pitches[0];
+		dst += dst_pitch;
+	}
+
+	kfree(dbuf);
+
+	return 0;
+}
+
+
 /**
  * drm_fb_memcpy - Copy clip buffer
  * @dst: Destination buffer
@@ -140,45 +229,23 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
 		 bool cached)
 {
 	u8 cpp = fb->format->cpp[0];
-	unsigned long linepixels = drm_rect_width(clip);
-	size_t len = linepixels * cpp;
-	const void *sbuf;
-	void *dbuf;
-	unsigned int y;
-	void *buf = NULL;
-
-	if (WARN_ON_ONCE(cpp != 2 && cpp != 4))
-		return;
-
-	if (!dst_pitch)
-		dst_pitch = len;
-	src += clip_offset(clip, fb->pitches[0], cpp);
-
-	if (!cached)
-		buf = kmalloc(len, GFP_KERNEL);
 
-	for (y = clip->y1; y < clip->y2; y++) {
-		if (buf)
-			sbuf = memcpy(buf, src, len);
-		else
-			sbuf = src;
-		dbuf = dst + clip->x1 * cpp;
-
-		if (cpp == 4)
-			drm_fb_swab32_line(dbuf, sbuf, linepixels);
-		else
-			drm_fb_swab16_line(dbuf, sbuf, linepixels);
-
-		src += fb->pitches[0];
-		dst += dst_pitch;
+	switch (cpp) {
+	case 4:
+		drm_fb_xfrm(dst, dst_pitch, cpp, src, fb, clip, cached, drm_fb_swab32_line);
+		break;
+	case 2:
+		drm_fb_xfrm(dst, dst_pitch, cpp, src, fb, clip, cached, drm_fb_swab16_line);
+		break;
+	default:
+		drm_warn_once(fb->dev, "Format %p4cc has unsupported pixel size.\n",
+			      &fb->format->format);
+		break;
 	}
-
-	kfree(buf);
 }
 EXPORT_SYMBOL(drm_fb_swab);
 
-static void drm_fb_xrgb8888_to_rgb332_line(void *dbuf, const void *sbuf, unsigned int pixels,
-					   bool swab)
+static void drm_fb_xrgb8888_to_rgb332_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	u8 *dbuf8 = dbuf;
 	const __le32 *sbuf32 = sbuf;
@@ -206,28 +273,7 @@ static void drm_fb_xrgb8888_to_rgb332_line(void *dbuf, const void *sbuf, unsigne
 void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *src,
 			       const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
-	size_t width = drm_rect_width(clip);
-	size_t src_len = width * sizeof(u32);
-	unsigned int y;
-	void *sbuf;
-
-	if (!dst_pitch)
-		dst_pitch = width;
-
-	/* 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, false);
-		src += fb->pitches[0];
-		dst += dst_pitch;
-	}
-
-	kfree(sbuf);
+	drm_fb_xfrm(dst, dst_pitch, 1, src, fb, clip, false, drm_fb_xrgb8888_to_rgb332_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
 
@@ -278,35 +324,12 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, unsigned int dst_pitch, const void *va
 			       const struct drm_framebuffer *fb, const struct drm_rect *clip,
 			       bool swab)
 {
-	size_t linepixels = clip->x2 - clip->x1;
-	size_t src_len = linepixels * sizeof(u32);
-	size_t dst_len = linepixels * sizeof(u16);
-	unsigned y, lines = clip->y2 - clip->y1;
-	void *sbuf;
-
-	if (!dst_pitch)
-		dst_pitch = dst_len;
-
-	/*
-	 * The cma memory is write-combined so reads are uncached.
-	 * Speed up by fetching one line at a time.
-	 */
-	sbuf = kmalloc(src_len, GFP_KERNEL);
-	if (!sbuf)
-		return;
-
-	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	for (y = 0; y < lines; y++) {
-		memcpy(sbuf, vaddr, src_len);
-		if (swab)
-			drm_fb_xrgb8888_to_rgb565_swab_line(dst, sbuf, linepixels);
-		else
-			drm_fb_xrgb8888_to_rgb565_line(dst, sbuf, linepixels);
-		vaddr += fb->pitches[0];
-		dst += dst_pitch;
-	}
-
-	kfree(sbuf);
+	if (swab)
+		drm_fb_xfrm(dst, dst_pitch, 2, vaddr, fb, clip, false,
+			    drm_fb_xrgb8888_to_rgb565_swab_line);
+	else
+		drm_fb_xfrm(dst, dst_pitch, 2, vaddr, fb, clip, false,
+			    drm_fb_xrgb8888_to_rgb565_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
 
@@ -326,30 +349,12 @@ void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch,
 				    const void *vaddr, const struct drm_framebuffer *fb,
 				    const struct drm_rect *clip, bool swab)
 {
-	size_t linepixels = clip->x2 - clip->x1;
-	size_t dst_len = linepixels * sizeof(u16);
-	unsigned y, lines = clip->y2 - clip->y1;
-	void *dbuf;
-
-	if (!dst_pitch)
-		dst_pitch = dst_len;
-
-	dbuf = kmalloc(dst_len, GFP_KERNEL);
-	if (!dbuf)
-		return;
-
-	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	for (y = 0; y < lines; y++) {
-		if (swab)
-			drm_fb_xrgb8888_to_rgb565_swab_line(dbuf, vaddr, linepixels);
-		else
-			drm_fb_xrgb8888_to_rgb565_line(dbuf, vaddr, linepixels);
-		memcpy_toio(dst, dbuf, dst_len);
-		vaddr += fb->pitches[0];
-		dst += dst_pitch;
-	}
-
-	kfree(dbuf);
+	if (swab)
+		drm_fb_xfrm_toio(dst, dst_pitch, 2, vaddr, fb, clip, false,
+				 drm_fb_xrgb8888_to_rgb565_swab_line);
+	else
+		drm_fb_xfrm_toio(dst, dst_pitch, 2, vaddr, fb, clip, false,
+				 drm_fb_xrgb8888_to_rgb565_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_toio);
 
@@ -380,28 +385,7 @@ static void drm_fb_xrgb8888_to_rgb888_line(void *dbuf, const void *sbuf, unsigne
 void drm_fb_xrgb8888_to_rgb888(void *dst, unsigned int dst_pitch, const void *src,
 			       const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
-	size_t width = drm_rect_width(clip);
-	size_t src_len = width * sizeof(u32);
-	unsigned int y;
-	void *sbuf;
-
-	if (!dst_pitch)
-		dst_pitch = width * 3;
-
-	/* 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 += dst_pitch;
-	}
-
-	kfree(sbuf);
+	drm_fb_xfrm(dst, dst_pitch, 3, src, fb, clip, false, drm_fb_xrgb8888_to_rgb888_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
 
@@ -420,27 +404,8 @@ void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch,
 				    const void *vaddr, const struct drm_framebuffer *fb,
 				    const struct drm_rect *clip)
 {
-	size_t linepixels = clip->x2 - clip->x1;
-	size_t dst_len = linepixels * 3;
-	unsigned y, lines = clip->y2 - clip->y1;
-	void *dbuf;
-
-	if (!dst_pitch)
-		dst_pitch = dst_len;
-
-	dbuf = kmalloc(dst_len, GFP_KERNEL);
-	if (!dbuf)
-		return;
-
-	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	for (y = 0; y < lines; y++) {
-		drm_fb_xrgb8888_to_rgb888_line(dbuf, vaddr, linepixels);
-		memcpy_toio(dst, dbuf, dst_len);
-		vaddr += fb->pitches[0];
-		dst += dst_pitch;
-	}
-
-	kfree(dbuf);
+	drm_fb_xfrm_toio(dst, dst_pitch, 3, vaddr, fb, clip, false,
+			 drm_fb_xrgb8888_to_rgb888_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_toio);
 
@@ -464,27 +429,8 @@ static void drm_fb_rgb565_to_xrgb8888_toio(void __iomem *dst, unsigned int dst_p
 					   const void *vaddr, const struct drm_framebuffer *fb,
 					   const struct drm_rect *clip)
 {
-	size_t linepixels = drm_rect_width(clip);
-	size_t dst_len = linepixels * 4;
-	unsigned int y, lines = drm_rect_height(clip);
-	void *dbuf;
-
-	if (!dst_pitch)
-		dst_pitch = dst_len;
-
-	dbuf = kmalloc(dst_len, GFP_KERNEL);
-	if (!dbuf)
-		return;
-
-	vaddr += clip_offset(clip, fb->pitches[0], 2);
-	for (y = 0; y < lines; y++) {
-		drm_fb_rgb565_to_xrgb8888_line(dbuf, vaddr, linepixels);
-		memcpy_toio(dst, dbuf, dst_len);
-		vaddr += fb->pitches[0];
-		dst += dst_pitch;
-	}
-
-	kfree(dbuf);
+	drm_fb_xfrm_toio(dst, dst_pitch, 4, vaddr, fb, clip, false,
+			 drm_fb_rgb565_to_xrgb8888_line);
 }
 
 static void drm_fb_rgb888_to_xrgb8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
@@ -505,27 +451,8 @@ static void drm_fb_rgb888_to_xrgb8888_toio(void __iomem *dst, unsigned int dst_p
 					   const void *vaddr, const struct drm_framebuffer *fb,
 					   const struct drm_rect *clip)
 {
-	size_t linepixels = drm_rect_width(clip);
-	size_t dst_len = linepixels * 4;
-	unsigned int y, lines = drm_rect_height(clip);
-	void *dbuf;
-
-	if (!dst_pitch)
-		dst_pitch = dst_len;
-
-	dbuf = kmalloc(dst_len, GFP_KERNEL);
-	if (!dbuf)
-		return;
-
-	vaddr += clip_offset(clip, fb->pitches[0], 3);
-	for (y = 0; y < lines; y++) {
-		drm_fb_rgb888_to_xrgb8888_line(dbuf, vaddr, linepixels);
-		memcpy_toio(dst, dbuf, dst_len);
-		vaddr += fb->pitches[0];
-		dst += dst_pitch;
-	}
-
-	kfree(dbuf);
+	drm_fb_xfrm_toio(dst, dst_pitch, 4, vaddr, fb, clip, false,
+			 drm_fb_rgb888_to_xrgb8888_line);
 }
 
 static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, unsigned int pixels)
@@ -560,27 +487,8 @@ void drm_fb_xrgb8888_to_xrgb2101010_toio(void __iomem *dst,
 					 const struct drm_framebuffer *fb,
 					 const struct drm_rect *clip)
 {
-	size_t linepixels = clip->x2 - clip->x1;
-	size_t dst_len = linepixels * sizeof(u32);
-	unsigned int y, lines = clip->y2 - clip->y1;
-	void *dbuf;
-
-	if (!dst_pitch)
-		dst_pitch = dst_len;
-
-	dbuf = kmalloc(dst_len, GFP_KERNEL);
-	if (!dbuf)
-		return;
-
-	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	for (y = 0; y < lines; y++) {
-		drm_fb_xrgb8888_to_xrgb2101010_line(dbuf, vaddr, linepixels);
-		memcpy_toio(dst, dbuf, dst_len);
-		vaddr += fb->pitches[0];
-		dst += dst_pitch;
-	}
-
-	kfree(dbuf);
+	drm_fb_xfrm_toio(dst, dst_pitch, 4, vaddr, fb, clip, false,
+			 drm_fb_xrgb8888_to_xrgb2101010_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb2101010_toio);
 
@@ -621,37 +529,7 @@ static void drm_fb_xrgb8888_to_gray8_line(void *dbuf, const void *sbuf, unsigned
 void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
 			      const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
-	unsigned int linepixels = clip->x2 - clip->x1;
-	unsigned int len = linepixels * sizeof(u32);
-	unsigned int y;
-	void *buf;
-	u8 *dst8;
-	u32 *src32;
-
-	if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
-		return;
-
-	if (!dst_pitch)
-		dst_pitch = drm_rect_width(clip);
-
-	/*
-	 * The cma memory is write-combined so reads are uncached.
-	 * Speed up by fetching one line at a time.
-	 */
-	buf = kmalloc(len, GFP_KERNEL);
-	if (!buf)
-		return;
-
-	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	for (y = clip->y1; y < clip->y2; y++) {
-		dst8 = dst;
-		src32 = memcpy(buf, vaddr, len);
-		drm_fb_xrgb8888_to_gray8_line(dst8, src32, linepixels);
-		vaddr += fb->pitches[0];
-		dst += dst_pitch;
-	}
-
-	kfree(buf);
+	drm_fb_xfrm(dst, dst_pitch, 1, vaddr, fb, clip, false, drm_fb_xrgb8888_to_gray8_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
 
-- 
2.36.0

Re: [PATCH 1/4] drm/format-helper: Implement drm_fb_swab() with per-line helpers

[Javier Martinez Canillas]

Hello Thomas,

On 4/27/22 16:14, Thomas Zimmermann wrote:
> Replace the inner loop of drm_fb_swab() with helper functions that
> swap the bytes in each pixel. This will allow to share the outer
> loop with other conversion helpers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

I've a meta question though.

Something that I never fully understood in the conversion helpers is if there
is some convention about the name of the parameters. Since it seems that in
some places we use dbuf, sbuf but in others we use src and dst, and so forth.

If is just that the naming haven't been used consistently, maybe this rework
(or a follow-up) could be an opportunity to add consistency in that regard.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 2/4] drm/format-helper: Remove optional byte-swap from line convertion

[Javier Martinez Canillas]

Hello Thomas,

On 4/27/22 16:14, Thomas Zimmermann wrote:
> Implement per-pixel byte swapping in a separate conversion helper
> for the single function that requires it. Select the correct helper
> for each conversion.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

I couldn't spot any issues but I believe these conversion helpers are
not easy to read and one could miss subtle issues so it would be nice
to have unit tests for these using the KUnit infrastructure:

https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html

I'm not asking to do it as a part of this series but something that
we may do as a follow-up though, just mentioning.

I'll post a patch to add that task to Documentation/gpu/todo.rst,
so it is tracked somewhere.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 1/4] drm/format-helper: Implement drm_fb_swab() with per-line helpers

[Thomas Zimmermann]

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

Hi

Am 03.05.22 um 09:48 schrieb Javier Martinez Canillas:
> Hello Thomas,
> 
> On 4/27/22 16:14, Thomas Zimmermann wrote:
>> Replace the inner loop of drm_fb_swab() with helper functions that
>> swap the bytes in each pixel. This will allow to share the outer
>> loop with other conversion helpers.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> I've a meta question though.
> 
> Something that I never fully understood in the conversion helpers is if there
> is some convention about the name of the parameters. Since it seems that in
> some places we use dbuf, sbuf but in others we use src and dst, and so forth.
> 
> If is just that the naming haven't been used consistently, maybe this rework
> (or a follow-up) could be an opportunity to add consistency in that regard.

It's just inconsistent. I try to use dbuf and sbuf if possible.

Best regards
Thomas

> 

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

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

Re: [PATCH 3/4] drm/format-helper: Unify the parameters of all per-line conversion helpers

[Javier Martinez Canillas]

On 4/27/22 16:14, Thomas Zimmermann wrote:
> Give each per-line conversion helper pointers of type void and the
> number of pixels in the line. Remove the unused swab parameters.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 4/4] drm/format-helper: Share implementation among conversion helpers

[Javier Martinez Canillas]

On 4/27/22 16:14, Thomas Zimmermann wrote:
> Provide format-independent conversion helpers for system and I/O
> memory. Implement most existing helpers on top of it. The source and
> destination formats of each conversion is handled by a per-line
> helper that is given to the generic implementation.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat