[PATCH 0/9] drm: Fix color-format selection in fbdev emulation

[PATCH 0/9] drm: Fix color-format selection in fbdev emulation

[Thomas Zimmermann]

Fix the selection of the fbdev emulation's color format and make
XRGB8888 the only emulated color format. Resolves the blank screen
in cases where video= specifies an unsupported color format. Also
resolves the issues around current format-conversion helpers.

DRM drivers usually pick a default format for their fbdev emulation.
Via the kernel's video= parameter, users can specify a different
format. If the given format is unsupported by the driver, the fbdev
console screen remains dark. As the console is essential to many
systems, not displaying anything is to be avoided.

Patch 1 fixes the detection of the firmware's native color format.
The meaning of several color parameters is inconsistent among Linux
and various standards. Take this into account.

As drivers are supposed to provide XRGB8888 as a default fallback
format, provide XRGB8888 conversion helpers in patches 2 to 5. The
new helpers handle cases where the client uses a XRGB8888 frambuffer
and the display scanout buffer uses a different format. All scanout
formats of the simplefb infrastructure should now be covered. The
patchse also extend the Kunit tests for the new formats.

With format conversion in place, patches 6 and 7 fix the single-probe
function's format selection. The helper now goes over the given video=
parameters until it finds a compatible format. If none is found, the
uses driver's default format.

Patches 8 and 9 clean up DRM code in drivers and helpers.

Tested on x86-64 with EFI output and x86 with various VESA color
modes. Also tested on ppc64 with OF output.

Thomas Zimmermann (9):
  firmware/sysfb: Fix EFI/VESA format selection
  drm/format-helper: Flip src/dst-format branches in blit helper
  drm/format-helper: Add conversion from XRGB8888 to ARGB8888
  drm/format-helper: Add conversion from XRGB8888 to ARGB2101010
  drm/format-helper: Add conversion from XRGB8888 to 15-bit RGB555
    formats
  drm/fh-helper: Split fbdev single-probe helper
  drm/fb-helper: Fix single-probe color-format selection
  drm/format-helper: Simplify drm_fb_build_fourcc_list()
  drm/format-helper: Remove unnecessary conversion helpers

 drivers/firmware/sysfb_simplefb.c             |  43 +-
 drivers/gpu/drm/drm_fb_helper.c               | 252 ++++++----
 drivers/gpu/drm/drm_format_helper.c           | 454 +++++++++++++-----
 .../gpu/drm/tests/drm_format_helper_test.c    | 312 ++++++++++++
 drivers/gpu/drm/tiny/ofdrm.c                  |  20 -
 drivers/gpu/drm/tiny/simpledrm.c              |  21 -
 include/drm/drm_format_helper.h               |  16 +-
 7 files changed, 835 insertions(+), 283 deletions(-)


base-commit: d322881f7e33af24901ee8ccaec3beef82f21203
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: 3f204510fcbf9530d6540bd8e6128cce598988b6
-- 
2.38.1

[PATCH 1/9] firmware/sysfb: Fix EFI/VESA format selection

[Thomas Zimmermann]

Select color format for EFI/VESA firmware scanout buffer from the
number of bits per pixel and the position of the individual color
components. Fixes the selected format for the buffer in several odd
cases. For example, XRGB1555 has been reported as ARGB1555 because
of the different use of depth and transparency in VESA and Linux.

Bits-per-pixel is always the pixel's raw number of bits; including
alpha and filler bits. It is preferred over color depth, which has a
different meaning among various components and standards.

Also do not compare reserved bits and transparency bits to each other.
These values have different meanings, as reserved bits include filler
bits while transparency does not.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/sysfb_simplefb.c | 43 ++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c
index a353e27f83f5..ce9c007ed66f 100644
--- a/drivers/firmware/sysfb_simplefb.c
+++ b/drivers/firmware/sysfb_simplefb.c
@@ -27,25 +27,56 @@ static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
 __init bool sysfb_parse_mode(const struct screen_info *si,
 			     struct simplefb_platform_data *mode)
 {
-	const struct simplefb_format *f;
 	__u8 type;
+	u32 bits_per_pixel;
 	unsigned int i;
 
 	type = si->orig_video_isVGA;
 	if (type != VIDEO_TYPE_VLFB && type != VIDEO_TYPE_EFI)
 		return false;
 
+	/*
+	 * The meaning of depth and bpp for direct-color formats is
+	 * inconsistent:
+	 *
+	 *  - DRM format info specifies depth as the number of color
+	 *    bits; including alpha, but not including filler bits.
+	 *  - Linux' EFI platform code computes lfb_depth from the
+	 *    individual color channels, including the reserved bits.
+	 *  - VBE 1.1 defines lfb_depth for XRGB1555 as 16, but later
+	 *    versions use 15.
+	 *  - On the kernel command line, 'bpp' of 32 is usually
+	 *    XRGB8888 including the filler bits, but 15 is XRGB1555
+	 *    not including the filler bit.
+	 *
+	 * It's not easily possible to fix this in struct screen_info,
+	 * as this could break UAPI. The best solution is to compute
+	 * bits_per_pixel here and ignore lfb_depth. In the loop below,
+	 * ignore simplefb formats with alpha bits, as EFI and VESA
+	 * don't specify alpha channels.
+	 */
+	if (si->lfb_depth > 8) {
+		bits_per_pixel = max(max3(si->red_size + si->red_pos,
+					  si->green_size + si->green_pos,
+					  si->blue_size + si->blue_pos),
+				     si->rsvd_size + si->rsvd_pos);
+	} else {
+		bits_per_pixel = si->lfb_depth;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(formats); ++i) {
-		f = &formats[i];
-		if (si->lfb_depth == f->bits_per_pixel &&
+		const struct simplefb_format *f = &formats[i];
+
+		if (f->transp.length)
+			continue; /* transparent formats are unsupported by VESA/EFI */
+
+		if (bits_per_pixel == f->bits_per_pixel &&
 		    si->red_size == f->red.length &&
 		    si->red_pos == f->red.offset &&
 		    si->green_size == f->green.length &&
 		    si->green_pos == f->green.offset &&
 		    si->blue_size == f->blue.length &&
-		    si->blue_pos == f->blue.offset &&
-		    si->rsvd_size == f->transp.length &&
-		    si->rsvd_pos == f->transp.offset) {
+		    si->blue_pos == f->blue.offset) {
 			mode->format = f->name;
 			mode->width = si->lfb_width;
 			mode->height = si->lfb_height;
-- 
2.38.1

[PATCH 2/9] drm/format-helper: Flip src/dst-format branches in blit helper

[Thomas Zimmermann]

Upcoming changes to the format conversion will mostly blit from
XRGB8888 to some other format. So put the source format in blit's
outer branches to make the code more readable. For cases where
a format only changes its endianness, such as XRGB565, introduce
dedicated branches that handle this for all formats.

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

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 74ff33c2ddaa..1c7e12610042 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -651,41 +651,37 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 	if (dst_format == DRM_FORMAT_ARGB2101010)
 		dst_format = DRM_FORMAT_XRGB2101010;
 
-	if (dst_format == fb_format) {
+	if (fb_format == dst_format) {
 		drm_fb_memcpy(dst, dst_pitch, src, fb, clip);
 		return 0;
-
-	} else if (dst_format == DRM_FORMAT_RGB565) {
-		if (fb_format == DRM_FORMAT_XRGB8888) {
+	} else if (fb_format == (dst_format | DRM_FORMAT_BIG_ENDIAN)) {
+		drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
+		return 0;
+	} else if (fb_format == (dst_format & ~DRM_FORMAT_BIG_ENDIAN)) {
+		drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
+		return 0;
+	} else if (fb_format == DRM_FORMAT_XRGB8888) {
+		if (dst_format == DRM_FORMAT_RGB565) {
 			drm_fb_xrgb8888_to_rgb565(dst, dst_pitch, src, fb, clip, false);
 			return 0;
-		}
-	} else if (dst_format == (DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN)) {
-		if (fb_format == DRM_FORMAT_RGB565) {
-			drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
-			return 0;
-		}
-	} else if (dst_format == DRM_FORMAT_RGB888) {
-		if (fb_format == DRM_FORMAT_XRGB8888) {
+		} else if (dst_format == DRM_FORMAT_RGB888) {
 			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip);
 			return 0;
-		}
-	} else if (dst_format == DRM_FORMAT_XRGB8888) {
-		if (fb_format == DRM_FORMAT_RGB888) {
-			drm_fb_rgb888_to_xrgb8888(dst, dst_pitch, src, fb, clip);
+		} else if (dst_format == DRM_FORMAT_XRGB2101010) {
+			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip);
 			return 0;
-		} else if (fb_format == DRM_FORMAT_RGB565) {
-			drm_fb_rgb565_to_xrgb8888(dst, dst_pitch, src, fb, clip);
+		} else if (dst_format == DRM_FORMAT_BGRX8888) {
+			drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
 			return 0;
 		}
-	} else if (dst_format == DRM_FORMAT_XRGB2101010) {
-		if (fb_format == DRM_FORMAT_XRGB8888) {
-			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip);
+	} else if (fb_format == DRM_FORMAT_RGB888) {
+		if (dst_format == DRM_FORMAT_XRGB8888) {
+			drm_fb_rgb888_to_xrgb8888(dst, dst_pitch, src, fb, clip);
 			return 0;
 		}
-	} else if (dst_format == DRM_FORMAT_BGRX8888) {
-		if (fb_format == DRM_FORMAT_XRGB8888) {
-			drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
+	} else if (fb_format == DRM_FORMAT_RGB565) {
+		if (dst_format == DRM_FORMAT_XRGB8888) {
+			drm_fb_rgb565_to_xrgb8888(dst, dst_pitch, src, fb, clip);
 			return 0;
 		}
 	}
-- 
2.38.1

[PATCH 3/9] drm/format-helper: Add conversion from XRGB8888 to ARGB8888

[Thomas Zimmermann]

Add dedicated helper to convert from XRGB8888 to ARGB8888. Sets
all alpha bits to make pixels fully opaque.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c           | 53 +++++++++++++++-
 .../gpu/drm/tests/drm_format_helper_test.c    | 63 +++++++++++++++++++
 include/drm/drm_format_helper.h               |  3 +
 3 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 1c7e12610042..9bcb08cf56b5 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -444,6 +444,54 @@ void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pi
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
 
+static void drm_fb_xrgb8888_to_argb8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	u32 *dbuf32 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	unsigned int x;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf32[x]);
+		pix |= GENMASK(31, 24); /* fill alpha bits */
+		dbuf32[x] = cpu_to_le32(pix);
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_argb8888 - Convert XRGB8888 to ARGB8888 clip buffer
+ * @dst: Array of ARGB8888 destination buffers
+ * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
+ *             within @dst; can be NULL if scanlines are stored next to each other.
+ * @src: Array of XRGB8888 source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ *
+ * This function copies parts of a framebuffer to display memory and converts the
+ * color format during the process. The parameters @dst, @dst_pitch and @src refer
+ * to arrays. Each array must have at least as many entries as there are planes in
+ * @fb's format. Each entry stores the value for the format's respective color plane
+ * at the same index.
+ *
+ * This function does not apply clipping on @dst (i.e. the destination is at the
+ * top-left corner).
+ *
+ * Drivers can use this function for ARGB8888 devices that don't support XRGB8888
+ * natively. It sets an opaque alpha channel as part of the conversion.
+ */
+void drm_fb_xrgb8888_to_argb8888(struct iosys_map *dst, const unsigned int *dst_pitch,
+				 const struct iosys_map *src, const struct drm_framebuffer *fb,
+				 const struct drm_rect *clip)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		4,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+		    drm_fb_xrgb8888_to_argb8888_line);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb8888);
+
 static void drm_fb_rgb565_to_xrgb8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	__le32 *dbuf32 = dbuf;
@@ -644,8 +692,6 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 	/* treat alpha channel like filler bits */
 	if (fb_format == DRM_FORMAT_ARGB8888)
 		fb_format = DRM_FORMAT_XRGB8888;
-	if (dst_format == DRM_FORMAT_ARGB8888)
-		dst_format = DRM_FORMAT_XRGB8888;
 	if (fb_format == DRM_FORMAT_ARGB2101010)
 		fb_format = DRM_FORMAT_XRGB2101010;
 	if (dst_format == DRM_FORMAT_ARGB2101010)
@@ -667,6 +713,9 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		} else if (dst_format == DRM_FORMAT_RGB888) {
 			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip);
 			return 0;
+		} else if (dst_format == DRM_FORMAT_ARGB8888) {
+			drm_fb_xrgb8888_to_argb8888(dst, dst_pitch, src, fb, clip);
+			return 0;
 		} else if (dst_format == DRM_FORMAT_XRGB2101010) {
 			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip);
 			return 0;
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 2191e57f2297..189483ef98ea 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -37,6 +37,11 @@ struct convert_to_rgb888_result {
 	const u8 expected[TEST_BUF_SIZE];
 };
 
+struct convert_to_argb8888_result {
+	unsigned int dst_pitch;
+	const u32 expected[TEST_BUF_SIZE];
+};
+
 struct convert_to_xrgb2101010_result {
 	unsigned int dst_pitch;
 	const u32 expected[TEST_BUF_SIZE];
@@ -51,6 +56,7 @@ struct convert_xrgb8888_case {
 	struct convert_to_rgb332_result rgb332_result;
 	struct convert_to_rgb565_result rgb565_result;
 	struct convert_to_rgb888_result rgb888_result;
+	struct convert_to_argb8888_result argb8888_result;
 	struct convert_to_xrgb2101010_result xrgb2101010_result;
 };
 
@@ -77,6 +83,10 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 			.dst_pitch = 0,
 			.expected = { 0x00, 0x00, 0xFF },
 		},
+		.argb8888_result = {
+			.dst_pitch = 0,
+			.expected = { 0xFFFF0000 },
+		},
 		.xrgb2101010_result = {
 			.dst_pitch = 0,
 			.expected = { 0x3FF00000 },
@@ -107,6 +117,10 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 			.dst_pitch = 0,
 			.expected = { 0x00, 0x00, 0xFF },
 		},
+		.argb8888_result = {
+			.dst_pitch = 0,
+			.expected = { 0xFFFF0000 },
+		},
 		.xrgb2101010_result = {
 			.dst_pitch = 0,
 			.expected = { 0x3FF00000 },
@@ -169,6 +183,15 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 				0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0x00,
 			},
 		},
+		.argb8888_result = {
+			.dst_pitch = 0,
+			.expected = {
+				0xFFFFFFFF, 0xFF000000,
+				0xFFFF0000, 0xFF00FF00,
+				0xFF0000FF, 0xFFFF00FF,
+				0xFFFFFF00, 0xFF00FFFF,
+			},
+		},
 		.xrgb2101010_result = {
 			.dst_pitch = 0,
 			.expected = {
@@ -229,6 +252,14 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 				0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 			},
 		},
+		.argb8888_result = {
+			.dst_pitch = 20,
+			.expected = {
+				0xFF0E449C, 0xFF114D05, 0xFFA80303, 0x00000000, 0x00000000,
+				0xFF6C7073, 0xFF0E449C, 0xFF114D05, 0x00000000, 0x00000000,
+				0xFFA80303, 0xFF6C7073, 0xFF0E449C, 0x00000000, 0x00000000,
+			},
+		},
 		.xrgb2101010_result = {
 			.dst_pitch = 20,
 			.expected = {
@@ -411,6 +442,37 @@ static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
 }
 
+static void drm_test_fb_xrgb8888_to_argb8888(struct kunit *test)
+{
+	const struct convert_xrgb8888_case *params = test->param_value;
+	const struct convert_to_argb8888_result *result = &params->argb8888_result;
+	size_t dst_size;
+	__u32 *buf = NULL;
+	__u32 *xrgb8888 = NULL;
+	struct iosys_map dst, src;
+
+	struct drm_framebuffer fb = {
+		.format = drm_format_info(DRM_FORMAT_XRGB8888),
+		.pitches = { params->pitch, 0, 0 },
+	};
+
+	dst_size = conversion_buf_size(DRM_FORMAT_ARGB8888,
+				       result->dst_pitch, &params->clip);
+	KUNIT_ASSERT_GT(test, dst_size, 0);
+
+	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+	iosys_map_set_vaddr(&dst, buf);
+
+	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
+	iosys_map_set_vaddr(&src, xrgb8888);
+
+	drm_fb_xrgb8888_to_argb8888(&dst, &result->dst_pitch, &src, &fb, &params->clip);
+	buf = le32buf_to_cpu(test, buf, dst_size / sizeof(u32));
+	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+}
+
 static void drm_test_fb_xrgb8888_to_xrgb2101010(struct kunit *test)
 {
 	const struct convert_xrgb8888_case *params = test->param_value;
@@ -447,6 +509,7 @@ static struct kunit_case drm_format_helper_test_cases[] = {
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb332, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb565, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb888, convert_xrgb8888_gen_params),
+	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb8888, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb2101010, convert_xrgb8888_gen_params),
 	{}
 };
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index eb5c98cf82b8..3ce8129dfe43 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -33,6 +33,9 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
 void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
 			       const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_argb8888(struct iosys_map *dst, const unsigned int *dst_pitch,
+				 const struct iosys_map *src, const struct drm_framebuffer *fb,
+				 const struct drm_rect *clip);
 void drm_fb_xrgb8888_to_xrgb2101010(struct iosys_map *dst, const unsigned int *dst_pitch,
 				    const struct iosys_map *src, const struct drm_framebuffer *fb,
 				    const struct drm_rect *clip);
-- 
2.38.1

[PATCH 4/9] drm/format-helper: Add conversion from XRGB8888 to ARGB2101010

[Thomas Zimmermann]

Add dedicated helper to convert from XRGB8888 to ARGB2101010. Sets
all alpha bits to make pixels fully opaque.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c           | 58 ++++++++++++++++-
 .../gpu/drm/tests/drm_format_helper_test.c    | 63 +++++++++++++++++++
 include/drm/drm_format_helper.h               |  3 +
 3 files changed, 122 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 9bcb08cf56b5..e562a3cefb89 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -603,6 +603,59 @@ void drm_fb_xrgb8888_to_xrgb2101010(struct iosys_map *dst, const unsigned int *d
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb2101010);
 
+static void drm_fb_xrgb8888_to_argb2101010_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	__le32 *dbuf32 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	unsigned int x;
+	u32 val32;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf32[x]);
+		val32 = ((pix & 0x000000ff) << 2) |
+			((pix & 0x0000ff00) << 4) |
+			((pix & 0x00ff0000) << 6);
+		pix = GENMASK(31, 30) | /* set alpha bits */
+		      val32 | ((val32 >> 8) & 0x00300c03);
+		*dbuf32++ = cpu_to_le32(pix);
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_argb2101010 - Convert XRGB8888 to ARGB2101010 clip buffer
+ * @dst: Array of ARGB2101010 destination buffers
+ * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
+ *             within @dst; can be NULL if scanlines are stored next to each other.
+ * @src: Array of XRGB8888 source buffers
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ *
+ * This function copies parts of a framebuffer to display memory and converts
+ * the color format during the process. The parameters @dst, @dst_pitch and
+ * @src refer to arrays. Each array must have at least as many entries as
+ * there are planes in @fb's format. Each entry stores the value for the
+ * format's respective color plane at the same index.
+ *
+ * This function does not apply clipping on @dst (i.e. the destination is at the
+ * top-left corner).
+ *
+ * Drivers can use this function for ARGB2101010 devices that don't support XRGB8888
+ * natively.
+ */
+void drm_fb_xrgb8888_to_argb2101010(struct iosys_map *dst, const unsigned int *dst_pitch,
+				    const struct iosys_map *src, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		4,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+		    drm_fb_xrgb8888_to_argb2101010_line);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb2101010);
+
 static void drm_fb_xrgb8888_to_gray8_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	u8 *dbuf8 = dbuf;
@@ -694,8 +747,6 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		fb_format = DRM_FORMAT_XRGB8888;
 	if (fb_format == DRM_FORMAT_ARGB2101010)
 		fb_format = DRM_FORMAT_XRGB2101010;
-	if (dst_format == DRM_FORMAT_ARGB2101010)
-		dst_format = DRM_FORMAT_XRGB2101010;
 
 	if (fb_format == dst_format) {
 		drm_fb_memcpy(dst, dst_pitch, src, fb, clip);
@@ -719,6 +770,9 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		} else if (dst_format == DRM_FORMAT_XRGB2101010) {
 			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip);
 			return 0;
+		} else if (dst_format == DRM_FORMAT_ARGB2101010) {
+			drm_fb_xrgb8888_to_argb2101010(dst, dst_pitch, src, fb, clip);
+			return 0;
 		} else if (dst_format == DRM_FORMAT_BGRX8888) {
 			drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
 			return 0;
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 189483ef98ea..b15dc7c1eb96 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -47,6 +47,11 @@ struct convert_to_xrgb2101010_result {
 	const u32 expected[TEST_BUF_SIZE];
 };
 
+struct convert_to_argb2101010_result {
+	unsigned int dst_pitch;
+	const u32 expected[TEST_BUF_SIZE];
+};
+
 struct convert_xrgb8888_case {
 	const char *name;
 	unsigned int pitch;
@@ -58,6 +63,7 @@ struct convert_xrgb8888_case {
 	struct convert_to_rgb888_result rgb888_result;
 	struct convert_to_argb8888_result argb8888_result;
 	struct convert_to_xrgb2101010_result xrgb2101010_result;
+	struct convert_to_argb2101010_result argb2101010_result;
 };
 
 static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
@@ -91,6 +97,10 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 			.dst_pitch = 0,
 			.expected = { 0x3FF00000 },
 		},
+		.argb2101010_result = {
+			.dst_pitch = 0,
+			.expected = { 0xFFF00000 },
+		},
 	},
 	{
 		.name = "single_pixel_clip_rectangle",
@@ -125,6 +135,10 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 			.dst_pitch = 0,
 			.expected = { 0x3FF00000 },
 		},
+		.argb2101010_result = {
+			.dst_pitch = 0,
+			.expected = { 0xFFF00000 },
+		},
 	},
 	{
 		/* Well known colors: White, black, red, green, blue, magenta,
@@ -201,6 +215,15 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 				0x3FFFFC00, 0x000FFFFF,
 			},
 		},
+		.argb2101010_result = {
+			.dst_pitch = 0,
+			.expected = {
+				0xFFFFFFFF, 0xC0000000,
+				0xFFF00000, 0xC00FFC00,
+				0xC00003FF, 0xFFF003FF,
+				0xFFFFFC00, 0xC00FFFFF,
+			},
+		},
 	},
 	{
 		/* Randomly picked colors. Full buffer within the clip area. */
@@ -268,6 +291,14 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 				0x2A20300C, 0x1B1705CD, 0x03844672, 0x00000000, 0x00000000,
 			},
 		},
+		.argb2101010_result = {
+			.dst_pitch = 20,
+			.expected = {
+				0xC3844672, 0xC444D414, 0xEA20300C, 0x00000000, 0x00000000,
+				0xDB1705CD, 0xC3844672, 0xC444D414, 0x00000000, 0x00000000,
+				0xEA20300C, 0xDB1705CD, 0xC3844672, 0x00000000, 0x00000000,
+			},
+		},
 	},
 };
 
@@ -504,6 +535,37 @@ static void drm_test_fb_xrgb8888_to_xrgb2101010(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
 }
 
+static void drm_test_fb_xrgb8888_to_argb2101010(struct kunit *test)
+{
+	const struct convert_xrgb8888_case *params = test->param_value;
+	const struct convert_to_argb2101010_result *result = &params->argb2101010_result;
+	size_t dst_size;
+	__u32 *buf = NULL;
+	__u32 *xrgb8888 = NULL;
+	struct iosys_map dst, src;
+
+	struct drm_framebuffer fb = {
+		.format = drm_format_info(DRM_FORMAT_ARGB8888),
+		.pitches = { params->pitch, 0, 0 },
+	};
+
+	dst_size = conversion_buf_size(DRM_FORMAT_ARGB2101010,
+				       result->dst_pitch, &params->clip);
+	KUNIT_ASSERT_GT(test, dst_size, 0);
+
+	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+	iosys_map_set_vaddr(&dst, buf);
+
+	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
+	iosys_map_set_vaddr(&src, xrgb8888);
+
+	drm_fb_xrgb8888_to_argb2101010(&dst, &result->dst_pitch, &src, &fb, &params->clip);
+	buf = le32buf_to_cpu(test, buf, dst_size / sizeof(u32));
+	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+}
+
 static struct kunit_case drm_format_helper_test_cases[] = {
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_gray8, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb332, convert_xrgb8888_gen_params),
@@ -511,6 +573,7 @@ static struct kunit_case drm_format_helper_test_cases[] = {
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb888, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb8888, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb2101010, convert_xrgb8888_gen_params),
+	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb2101010, convert_xrgb8888_gen_params),
 	{}
 };
 
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 3ce8129dfe43..10b2d19cdb66 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -39,6 +39,9 @@ void drm_fb_xrgb8888_to_argb8888(struct iosys_map *dst, const unsigned int *dst_
 void drm_fb_xrgb8888_to_xrgb2101010(struct iosys_map *dst, const unsigned int *dst_pitch,
 				    const struct iosys_map *src, const struct drm_framebuffer *fb,
 				    const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_argb2101010(struct iosys_map *dst, const unsigned int *dst_pitch,
+				    const struct iosys_map *src, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip);
 void drm_fb_xrgb8888_to_gray8(struct iosys_map *dst, const unsigned int *dst_pitch,
 			      const struct iosys_map *src, const struct drm_framebuffer *fb,
 			      const struct drm_rect *clip);
-- 
2.38.1

[PATCH 5/9] drm/format-helper: Add conversion from XRGB8888 to 15-bit RGB555 formats

[Thomas Zimmermann]

Add conversion from XRGB8888 to XRGB1555, ARGB1555 and RGBA5551, which
are the formats currently supported by the simplefb infrastructure. The
new helpers allow the output of XRGB8888 framebuffers to firmware
scanout buffers in one of the 15-bit formats.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c           | 164 +++++++++++++++
 .../gpu/drm/tests/drm_format_helper_test.c    | 186 ++++++++++++++++++
 include/drm/drm_format_helper.h               |   9 +
 3 files changed, 359 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index e562a3cefb89..aa6138756458 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -395,6 +395,161 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
 
+static void drm_fb_xrgb8888_to_xrgb1555_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	u16 *dbuf16 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	unsigned int x;
+	u16 val16;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf32[x]);
+		val16 = ((pix & 0x00f80000) >> 9) |
+			((pix & 0x0000f800) >> 6) |
+			((pix & 0x000000f8) >> 3);
+		dbuf16[x] = cpu_to_le16(val16);
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_xrgb1555 - Convert XRGB8888 to XRGB1555 clip buffer
+ * @dst: Array of XRGB1555 destination buffers
+ * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
+ *             within @dst; can be NULL if scanlines are stored next to each other.
+ * @src: Array of XRGB8888 source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ *
+ * This function copies parts of a framebuffer to display memory and converts
+ * the color format during the process. The parameters @dst, @dst_pitch and
+ * @src refer to arrays. Each array must have at least as many entries as
+ * there are planes in @fb's format. Each entry stores the value for the
+ * format's respective color plane at the same index.
+ *
+ * This function does not apply clipping on @dst (i.e. the destination is at the
+ * top-left corner).
+ *
+ * Drivers can use this function for XRGB1555 devices that don't support
+ * XRGB8888 natively.
+ */
+void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
+				 const struct iosys_map *src, const struct drm_framebuffer *fb,
+				 const struct drm_rect *clip)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		2,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+		    drm_fb_xrgb8888_to_xrgb1555_line);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb1555);
+
+static void drm_fb_xrgb8888_to_argb1555_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	u16 *dbuf16 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	unsigned int x;
+	u16 val16;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf32[x]);
+		val16 = BIT(15) | /* set alpha bit */
+			((pix & 0x00f80000) >> 9) |
+			((pix & 0x0000f800) >> 6) |
+			((pix & 0x000000f8) >> 3);
+		dbuf16[x] = cpu_to_le16(val16);
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_argb1555 - Convert XRGB8888 to ARGB1555 clip buffer
+ * @dst: Array of ARGB1555 destination buffers
+ * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
+ *             within @dst; can be NULL if scanlines are stored next to each other.
+ * @src: Array of XRGB8888 source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ *
+ * This function copies parts of a framebuffer to display memory and converts
+ * the color format during the process. The parameters @dst, @dst_pitch and
+ * @src refer to arrays. Each array must have at least as many entries as
+ * there are planes in @fb's format. Each entry stores the value for the
+ * format's respective color plane at the same index.
+ *
+ * This function does not apply clipping on @dst (i.e. the destination is at the
+ * top-left corner).
+ *
+ * Drivers can use this function for ARGB1555 devices that don't support
+ * XRGB8888 natively. It sets an opaque alpha channel as part of the conversion.
+ */
+void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
+				 const struct iosys_map *src, const struct drm_framebuffer *fb,
+				 const struct drm_rect *clip)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		2,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+		    drm_fb_xrgb8888_to_argb1555_line);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb1555);
+
+static void drm_fb_xrgb8888_to_rgba5551_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	u16 *dbuf16 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	unsigned int x;
+	u16 val16;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf32[x]);
+		val16 = ((pix & 0x00f80000) >> 8) |
+			((pix & 0x0000f800) >> 5) |
+			((pix & 0x000000f8) >> 2) |
+			BIT(0); /* set alpha bit */
+		dbuf16[x] = cpu_to_le16(val16);
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_rgba5551 - Convert XRGB8888 to RGBA5551 clip buffer
+ * @dst: Array of RGBA5551 destination buffers
+ * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
+ *             within @dst; can be NULL if scanlines are stored next to each other.
+ * @src: Array of XRGB8888 source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ *
+ * This function copies parts of a framebuffer to display memory and converts
+ * the color format during the process. The parameters @dst, @dst_pitch and
+ * @src refer to arrays. Each array must have at least as many entries as
+ * there are planes in @fb's format. Each entry stores the value for the
+ * format's respective color plane at the same index.
+ *
+ * This function does not apply clipping on @dst (i.e. the destination is at the
+ * top-left corner).
+ *
+ * Drivers can use this function for RGBA5551 devices that don't support
+ * XRGB8888 natively. It sets an opaque alpha channel as part of the conversion.
+ */
+void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_pitch,
+				 const struct iosys_map *src, const struct drm_framebuffer *fb,
+				 const struct drm_rect *clip)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		2,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+		    drm_fb_xrgb8888_to_rgba5551_line);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgba5551);
+
 static void drm_fb_xrgb8888_to_rgb888_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	u8 *dbuf8 = dbuf;
@@ -761,6 +916,15 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		if (dst_format == DRM_FORMAT_RGB565) {
 			drm_fb_xrgb8888_to_rgb565(dst, dst_pitch, src, fb, clip, false);
 			return 0;
+		} else if (dst_format == DRM_FORMAT_XRGB1555) {
+			drm_fb_xrgb8888_to_xrgb1555(dst, dst_pitch, src, fb, clip);
+			return 0;
+		} else if (dst_format == DRM_FORMAT_ARGB1555) {
+			drm_fb_xrgb8888_to_argb1555(dst, dst_pitch, src, fb, clip);
+			return 0;
+		} else if (dst_format == DRM_FORMAT_RGBA5551) {
+			drm_fb_xrgb8888_to_rgba5551(dst, dst_pitch, src, fb, clip);
+			return 0;
 		} else if (dst_format == DRM_FORMAT_RGB888) {
 			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip);
 			return 0;
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index b15dc7c1eb96..e01695c70bb6 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -32,6 +32,21 @@ struct convert_to_rgb565_result {
 	const u16 expected_swab[TEST_BUF_SIZE];
 };
 
+struct convert_to_xrgb1555_result {
+	unsigned int dst_pitch;
+	const u16 expected[TEST_BUF_SIZE];
+};
+
+struct convert_to_argb1555_result {
+	unsigned int dst_pitch;
+	const u16 expected[TEST_BUF_SIZE];
+};
+
+struct convert_to_rgba5551_result {
+	unsigned int dst_pitch;
+	const u16 expected[TEST_BUF_SIZE];
+};
+
 struct convert_to_rgb888_result {
 	unsigned int dst_pitch;
 	const u8 expected[TEST_BUF_SIZE];
@@ -60,6 +75,9 @@ struct convert_xrgb8888_case {
 	struct convert_to_gray8_result gray8_result;
 	struct convert_to_rgb332_result rgb332_result;
 	struct convert_to_rgb565_result rgb565_result;
+	struct convert_to_xrgb1555_result xrgb1555_result;
+	struct convert_to_argb1555_result argb1555_result;
+	struct convert_to_rgba5551_result rgba5551_result;
 	struct convert_to_rgb888_result rgb888_result;
 	struct convert_to_argb8888_result argb8888_result;
 	struct convert_to_xrgb2101010_result xrgb2101010_result;
@@ -85,6 +103,18 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 			.expected = { 0xF800 },
 			.expected_swab = { 0x00F8 },
 		},
+		.xrgb1555_result = {
+			.dst_pitch = 0,
+			.expected = { 0x7C00 },
+		},
+		.argb1555_result = {
+			.dst_pitch = 0,
+			.expected = { 0xFC00 },
+		},
+		.rgba5551_result = {
+			.dst_pitch = 0,
+			.expected = { 0xF801 },
+		},
 		.rgb888_result = {
 			.dst_pitch = 0,
 			.expected = { 0x00, 0x00, 0xFF },
@@ -123,6 +153,18 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 			.expected = { 0xF800 },
 			.expected_swab = { 0x00F8 },
 		},
+		.xrgb1555_result = {
+			.dst_pitch = 0,
+			.expected = { 0x7C00 },
+		},
+		.argb1555_result = {
+			.dst_pitch = 0,
+			.expected = { 0xFC00 },
+		},
+		.rgba5551_result = {
+			.dst_pitch = 0,
+			.expected = { 0xF801 },
+		},
 		.rgb888_result = {
 			.dst_pitch = 0,
 			.expected = { 0x00, 0x00, 0xFF },
@@ -188,6 +230,33 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 				0xE0FF, 0xFF07,
 			},
 		},
+		.xrgb1555_result = {
+			.dst_pitch = 0,
+			.expected = {
+				0x7FFF, 0x0000,
+				0x7C00, 0x03E0,
+				0x001F, 0x7C1F,
+				0x7FE0, 0x03FF,
+			},
+		},
+		.argb1555_result = {
+			.dst_pitch = 0,
+			.expected = {
+				0xFFFF, 0x8000,
+				0xFC00, 0x83E0,
+				0x801F, 0xFC1F,
+				0xFFE0, 0x83FF,
+			},
+		},
+		.rgba5551_result = {
+			.dst_pitch = 0,
+			.expected = {
+				0xFFFF, 0x0001,
+				0xF801, 0x07C1,
+				0x003F, 0xF83F,
+				0xFFC1, 0x07FF,
+			},
+		},
 		.rgb888_result = {
 			.dst_pitch = 0,
 			.expected = {
@@ -264,6 +333,30 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 				0x00A8, 0x8E6B, 0x330A, 0x0000, 0x0000,
 			},
 		},
+		.xrgb1555_result = {
+			.dst_pitch = 10,
+			.expected = {
+				0x0513, 0x0920, 0x5400, 0x0000, 0x0000,
+				0x35CE, 0x0513, 0x0920, 0x0000, 0x0000,
+				0x5400, 0x35CE, 0x0513, 0x0000, 0x0000,
+			},
+		},
+		.argb1555_result = {
+			.dst_pitch = 10,
+			.expected = {
+				0x8513, 0x8920, 0xD400, 0x0000, 0x0000,
+				0xB5CE, 0x8513, 0x8920, 0x0000, 0x0000,
+				0xD400, 0xB5CE, 0x8513, 0x0000, 0x0000,
+			},
+		},
+		.rgba5551_result = {
+			.dst_pitch = 10,
+			.expected = {
+				0x0A27, 0x1241, 0xA801, 0x0000, 0x0000,
+				0x6B9D, 0x0A27, 0x1241, 0x0000, 0x0000,
+				0xA801, 0x6B9D, 0x0A27, 0x0000, 0x0000,
+			},
+		},
 		.rgb888_result = {
 			.dst_pitch = 15,
 			.expected = {
@@ -443,6 +536,96 @@ static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected_swab, dst_size), 0);
 }
 
+static void drm_test_fb_xrgb8888_to_xrgb1555(struct kunit *test)
+{
+	const struct convert_xrgb8888_case *params = test->param_value;
+	const struct convert_to_xrgb1555_result *result = &params->xrgb1555_result;
+	size_t dst_size;
+	__u16 *buf = NULL;
+	__u32 *xrgb8888 = NULL;
+	struct iosys_map dst, src;
+
+	struct drm_framebuffer fb = {
+		.format = drm_format_info(DRM_FORMAT_XRGB8888),
+		.pitches = { params->pitch, 0, 0 },
+	};
+
+	dst_size = conversion_buf_size(DRM_FORMAT_XRGB1555, result->dst_pitch,
+				       &params->clip);
+	KUNIT_ASSERT_GT(test, dst_size, 0);
+
+	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+	iosys_map_set_vaddr(&dst, buf);
+
+	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
+	iosys_map_set_vaddr(&src, xrgb8888);
+
+	drm_fb_xrgb8888_to_xrgb1555(&dst, &result->dst_pitch, &src, &fb, &params->clip);
+	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+}
+
+static void drm_test_fb_xrgb8888_to_argb1555(struct kunit *test)
+{
+	const struct convert_xrgb8888_case *params = test->param_value;
+	const struct convert_to_argb1555_result *result = &params->argb1555_result;
+	size_t dst_size;
+	__u16 *buf = NULL;
+	__u32 *xrgb8888 = NULL;
+	struct iosys_map dst, src;
+
+	struct drm_framebuffer fb = {
+		.format = drm_format_info(DRM_FORMAT_XRGB8888),
+		.pitches = { params->pitch, 0, 0 },
+	};
+
+	dst_size = conversion_buf_size(DRM_FORMAT_ARGB1555, result->dst_pitch,
+				       &params->clip);
+	KUNIT_ASSERT_GT(test, dst_size, 0);
+
+	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+	iosys_map_set_vaddr(&dst, buf);
+
+	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
+	iosys_map_set_vaddr(&src, xrgb8888);
+
+	drm_fb_xrgb8888_to_argb1555(&dst, &result->dst_pitch, &src, &fb, &params->clip);
+	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+}
+
+static void drm_test_fb_xrgb8888_to_rgba5551(struct kunit *test)
+{
+	const struct convert_xrgb8888_case *params = test->param_value;
+	const struct convert_to_rgba5551_result *result = &params->rgba5551_result;
+	size_t dst_size;
+	__u16 *buf = NULL;
+	__u32 *xrgb8888 = NULL;
+	struct iosys_map dst, src;
+
+	struct drm_framebuffer fb = {
+		.format = drm_format_info(DRM_FORMAT_XRGB8888),
+		.pitches = { params->pitch, 0, 0 },
+	};
+
+	dst_size = conversion_buf_size(DRM_FORMAT_RGBA5551, result->dst_pitch,
+				       &params->clip);
+	KUNIT_ASSERT_GT(test, dst_size, 0);
+
+	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+	iosys_map_set_vaddr(&dst, buf);
+
+	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
+	iosys_map_set_vaddr(&src, xrgb8888);
+
+	drm_fb_xrgb8888_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, &params->clip);
+	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+}
+
 static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
 {
 	const struct convert_xrgb8888_case *params = test->param_value;
@@ -570,6 +753,9 @@ static struct kunit_case drm_format_helper_test_cases[] = {
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_gray8, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb332, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb565, convert_xrgb8888_gen_params),
+	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb1555, convert_xrgb8888_gen_params),
+	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb1555, convert_xrgb8888_gen_params),
+	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgba5551, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb888, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb8888, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb2101010, convert_xrgb8888_gen_params),
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 10b2d19cdb66..2d04f5863722 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -30,6 +30,15 @@ void drm_fb_xrgb8888_to_rgb332(struct iosys_map *dst, const unsigned int *dst_pi
 void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
 			       const struct drm_rect *clip, bool swab);
+void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
+				 const struct iosys_map *src, const struct drm_framebuffer *fb,
+				 const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
+				 const struct iosys_map *src, const struct drm_framebuffer *fb,
+				 const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_pitch,
+				 const struct iosys_map *src, const struct drm_framebuffer *fb,
+				 const struct drm_rect *clip);
 void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
 			       const struct drm_rect *clip);
-- 
2.38.1

[PATCH 6/9] drm/fh-helper: Split fbdev single-probe helper

[Thomas Zimmermann]

Split the single-probe helper's implementation into multiple
functions and get locking and overallocation out of the way of
the surface setup. Simplifies later changes to the setup code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fb_helper.c | 116 ++++++++++++++++++++------------
 1 file changed, 73 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b3a731b9170a..af1495394d38 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1726,36 +1726,30 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 }
 EXPORT_SYMBOL(drm_fb_helper_pan_display);
 
-/*
- * Allocates the backing storage and sets up the fbdev info structure through
- * the ->fb_probe callback.
- */
-static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
-					 int preferred_bpp)
+
+static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferred_bpp,
+				      struct drm_fb_helper_surface_size *sizes)
 {
 	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_device *dev = fb_helper->dev;
-	struct drm_mode_config *config = &dev->mode_config;
-	int ret = 0;
 	int crtc_count = 0;
 	struct drm_connector_list_iter conn_iter;
-	struct drm_fb_helper_surface_size sizes;
 	struct drm_connector *connector;
 	struct drm_mode_set *mode_set;
 	int best_depth = 0;
 
-	memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
-	sizes.surface_depth = 24;
-	sizes.surface_bpp = 32;
-	sizes.fb_width = (u32)-1;
-	sizes.fb_height = (u32)-1;
+	memset(sizes, 0, sizeof(struct drm_fb_helper_surface_size));
+	sizes->surface_depth = 24;
+	sizes->surface_bpp = 32;
+	sizes->fb_width = (u32)-1;
+	sizes->fb_height = (u32)-1;
 
 	/*
 	 * If driver picks 8 or 16 by default use that for both depth/bpp
 	 * to begin with
 	 */
-	if (preferred_bpp != sizes.surface_bpp)
-		sizes.surface_depth = sizes.surface_bpp = preferred_bpp;
+	if (preferred_bpp != sizes->surface_bpp)
+		sizes->surface_depth = sizes->surface_bpp = preferred_bpp;
 
 	drm_connector_list_iter_begin(fb_helper->dev, &conn_iter);
 	drm_client_for_each_connector_iter(connector, &conn_iter) {
@@ -1766,21 +1760,21 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		if (cmdline_mode->bpp_specified) {
 			switch (cmdline_mode->bpp) {
 			case 8:
-				sizes.surface_depth = sizes.surface_bpp = 8;
+				sizes->surface_depth = sizes->surface_bpp = 8;
 				break;
 			case 15:
-				sizes.surface_depth = 15;
-				sizes.surface_bpp = 16;
+				sizes->surface_depth = 15;
+				sizes->surface_bpp = 16;
 				break;
 			case 16:
-				sizes.surface_depth = sizes.surface_bpp = 16;
+				sizes->surface_depth = sizes->surface_bpp = 16;
 				break;
 			case 24:
-				sizes.surface_depth = sizes.surface_bpp = 24;
+				sizes->surface_depth = sizes->surface_bpp = 24;
 				break;
 			case 32:
-				sizes.surface_depth = 24;
-				sizes.surface_bpp = 32;
+				sizes->surface_depth = 24;
+				sizes->surface_bpp = 32;
 				break;
 			}
 			break;
@@ -1793,7 +1787,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	 * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
 	 * 16) we need to scale down the depth of the sizes we request.
 	 */
-	mutex_lock(&client->modeset_mutex);
 	drm_client_for_each_modeset(mode_set, client) {
 		struct drm_crtc *crtc = mode_set->crtc;
 		struct drm_plane *plane = crtc->primary;
@@ -1817,13 +1810,13 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 				continue;
 
 			/* We found a perfect fit, great */
-			if (fmt->depth == sizes.surface_depth) {
+			if (fmt->depth == sizes->surface_depth) {
 				best_depth = fmt->depth;
 				break;
 			}
 
 			/* Skip depths above what we're looking for */
-			if (fmt->depth > sizes.surface_depth)
+			if (fmt->depth > sizes->surface_depth)
 				continue;
 
 			/* Best depth found so far */
@@ -1831,10 +1824,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 				best_depth = fmt->depth;
 		}
 	}
-	if (sizes.surface_depth != best_depth && best_depth) {
+	if (sizes->surface_depth != best_depth && best_depth) {
 		drm_info(dev, "requested bpp %d, scaled depth down to %d",
-			 sizes.surface_bpp, best_depth);
-		sizes.surface_depth = best_depth;
+			 sizes->surface_bpp, best_depth);
+		sizes->surface_depth = best_depth;
 	}
 
 	/* first up get a count of crtcs now in use and new min/maxes width/heights */
@@ -1858,8 +1851,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		x = mode_set->x;
 		y = mode_set->y;
 
-		sizes.surface_width  = max_t(u32, desired_mode->hdisplay + x, sizes.surface_width);
-		sizes.surface_height = max_t(u32, desired_mode->vdisplay + y, sizes.surface_height);
+		sizes->surface_width  =
+			max_t(u32, desired_mode->hdisplay + x, sizes->surface_width);
+		sizes->surface_height =
+			max_t(u32, desired_mode->vdisplay + y, sizes->surface_height);
 
 		for (j = 0; j < mode_set->num_connectors; j++) {
 			struct drm_connector *connector = mode_set->connectors[j];
@@ -1875,28 +1870,63 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		}
 
 		if (lasth)
-			sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
+			sizes->fb_width  = min_t(u32, desired_mode->hdisplay + x, sizes->fb_width);
 		if (lastv)
-			sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
+			sizes->fb_height = min_t(u32, desired_mode->vdisplay + y, sizes->fb_height);
 	}
-	mutex_unlock(&client->modeset_mutex);
 
-	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
+	if (crtc_count == 0 || sizes->fb_width == -1 || sizes->fb_height == -1) {
 		drm_info(dev, "Cannot find any crtc or sizes\n");
-
-		/* First time: disable all crtc's.. */
-		if (!fb_helper->deferred_setup)
-			drm_client_modeset_commit(client);
 		return -EAGAIN;
 	}
 
+	return 0;
+}
+
+static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferred_bpp,
+				    struct drm_fb_helper_surface_size *sizes)
+{
+	struct drm_client_dev *client = &fb_helper->client;
+	struct drm_device *dev = fb_helper->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+	int ret;
+
+	mutex_lock(&client->modeset_mutex);
+	ret = __drm_fb_helper_find_sizes(fb_helper, preferred_bpp, sizes);
+	mutex_unlock(&client->modeset_mutex);
+
+	if (ret)
+		return ret;
+
 	/* Handle our overallocation */
-	sizes.surface_height *= drm_fbdev_overalloc;
-	sizes.surface_height /= 100;
-	if (sizes.surface_height > config->max_height) {
+	sizes->surface_height *= drm_fbdev_overalloc;
+	sizes->surface_height /= 100;
+	if (sizes->surface_height > config->max_height) {
 		drm_dbg_kms(dev, "Fbdev over-allocation too large; clamping height to %d\n",
 			    config->max_height);
-		sizes.surface_height = config->max_height;
+		sizes->surface_height = config->max_height;
+	}
+
+	return 0;
+}
+
+/*
+ * Allocates the backing storage and sets up the fbdev info structure through
+ * the ->fb_probe callback.
+ */
+static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
+					 int preferred_bpp)
+{
+	struct drm_client_dev *client = &fb_helper->client;
+	struct drm_fb_helper_surface_size sizes;
+	int ret;
+
+	ret = drm_fb_helper_find_sizes(fb_helper, preferred_bpp, &sizes);
+	if (ret) {
+		/* First time: disable all crtc's.. */
+		if (!fb_helper->deferred_setup)
+			drm_client_modeset_commit(client);
+		return ret;
 	}
 
 #if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
-- 
2.38.1

[PATCH 7/9] drm/fb-helper: Fix single-probe color-format selection

[Thomas Zimmermann]

Fix the color-format selection of the single-probe helper. Go
through all user-specified values and test each for compatibility
with the driver. If none is supported, use the driver-provided
default. This guarantees that the console is always available in
any color format at least.

Until now, the format selection of the single-probe helper tried
to either use a user-specified format or a 32-bit default format.
If the user-specified format was not supported by the driver, the
selection failed and the display remained blank.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fb_helper.c | 172 +++++++++++++++++---------------
 1 file changed, 94 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index af1495394d38..1369ca4ae39b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1726,6 +1726,70 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 }
 EXPORT_SYMBOL(drm_fb_helper_pan_display);
 
+static uint32_t drm_fb_helper_find_format(struct drm_fb_helper *fb_helper, const uint32_t *formats,
+					  size_t format_count, uint32_t bpp, uint32_t depth)
+{
+	struct drm_device *dev = fb_helper->dev;
+	uint32_t format;
+	size_t i;
+
+	/*
+	 * Do not consider YUV or other complicated formats
+	 * for framebuffers. This means only legacy formats
+	 * are supported (fmt->depth is a legacy field), but
+	 * the framebuffer emulation can only deal with such
+	 * formats, specifically RGB/BGA formats.
+	 */
+	format = drm_mode_legacy_fb_format(bpp, depth);
+	if (!format)
+		goto err;
+
+	for (i = 0; i < format_count; ++i) {
+		if (formats[i] == format)
+			return format;
+	}
+
+err:
+	/* We found nothing. */
+	drm_warn(dev, "bpp/depth value of %u/%u not supported\n", bpp, depth);
+
+	return DRM_FORMAT_INVALID;
+}
+
+static uint32_t drm_fb_helper_find_cmdline_format(struct drm_fb_helper *fb_helper,
+						  const uint32_t *formats, size_t format_count,
+						  const struct drm_cmdline_mode *cmdline_mode)
+{
+	struct drm_device *dev = fb_helper->dev;
+	uint32_t bpp, depth;
+
+	if (!cmdline_mode->bpp_specified)
+		return DRM_FORMAT_INVALID;
+
+	switch (cmdline_mode->bpp) {
+	case 1:
+	case 2:
+	case 4:
+	case 8:
+	case 16:
+	case 24:
+		bpp = depth = cmdline_mode->bpp;
+		break;
+	case 15:
+		bpp = 16;
+		depth = 15;
+		break;
+	case 32:
+		bpp = 32;
+		depth = 24;
+		break;
+	default:
+		drm_info(dev, "unsupported bpp value of %d\n", cmdline_mode->bpp);
+		return DRM_FORMAT_INVALID;
+	}
+
+	return drm_fb_helper_find_format(fb_helper, formats, format_count, bpp, depth);
+}
 
 static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferred_bpp,
 				      struct drm_fb_helper_surface_size *sizes)
@@ -1736,100 +1800,52 @@ static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int prefe
 	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
 	struct drm_mode_set *mode_set;
-	int best_depth = 0;
+	uint32_t surface_format = DRM_FORMAT_INVALID;
+	const struct drm_format_info *info;
 
-	memset(sizes, 0, sizeof(struct drm_fb_helper_surface_size));
-	sizes->surface_depth = 24;
-	sizes->surface_bpp = 32;
+	memset(sizes, 0, sizeof(*sizes));
 	sizes->fb_width = (u32)-1;
 	sizes->fb_height = (u32)-1;
 
-	/*
-	 * If driver picks 8 or 16 by default use that for both depth/bpp
-	 * to begin with
-	 */
-	if (preferred_bpp != sizes->surface_bpp)
-		sizes->surface_depth = sizes->surface_bpp = preferred_bpp;
-
-	drm_connector_list_iter_begin(fb_helper->dev, &conn_iter);
-	drm_client_for_each_connector_iter(connector, &conn_iter) {
-		struct drm_cmdline_mode *cmdline_mode;
-
-		cmdline_mode = &connector->cmdline_mode;
-
-		if (cmdline_mode->bpp_specified) {
-			switch (cmdline_mode->bpp) {
-			case 8:
-				sizes->surface_depth = sizes->surface_bpp = 8;
-				break;
-			case 15:
-				sizes->surface_depth = 15;
-				sizes->surface_bpp = 16;
-				break;
-			case 16:
-				sizes->surface_depth = sizes->surface_bpp = 16;
-				break;
-			case 24:
-				sizes->surface_depth = sizes->surface_bpp = 24;
-				break;
-			case 32:
-				sizes->surface_depth = 24;
-				sizes->surface_bpp = 32;
-				break;
-			}
-			break;
-		}
-	}
-	drm_connector_list_iter_end(&conn_iter);
-
-	/*
-	 * If we run into a situation where, for example, the primary plane
-	 * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
-	 * 16) we need to scale down the depth of the sizes we request.
-	 */
 	drm_client_for_each_modeset(mode_set, client) {
 		struct drm_crtc *crtc = mode_set->crtc;
 		struct drm_plane *plane = crtc->primary;
-		int j;
 
 		drm_dbg_kms(dev, "test CRTC %u primary plane\n", drm_crtc_index(crtc));
 
-		for (j = 0; j < plane->format_count; j++) {
-			const struct drm_format_info *fmt;
-
-			fmt = drm_format_info(plane->format_types[j]);
+		drm_connector_list_iter_begin(fb_helper->dev, &conn_iter);
+		drm_client_for_each_connector_iter(connector, &conn_iter) {
+			struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode;
 
-			/*
-			 * Do not consider YUV or other complicated formats
-			 * for framebuffers. This means only legacy formats
-			 * are supported (fmt->depth is a legacy field) but
-			 * the framebuffer emulation can only deal with such
-			 * formats, specifically RGB/BGA formats.
-			 */
-			if (fmt->depth == 0)
-				continue;
-
-			/* We found a perfect fit, great */
-			if (fmt->depth == sizes->surface_depth) {
-				best_depth = fmt->depth;
-				break;
-			}
+			surface_format = drm_fb_helper_find_cmdline_format(fb_helper,
+									   plane->format_types,
+									   plane->format_count,
+									   cmdline_mode);
+			if (surface_format != DRM_FORMAT_INVALID)
+				break; /* found supported format */
+		}
+		drm_connector_list_iter_end(&conn_iter);
 
-			/* Skip depths above what we're looking for */
-			if (fmt->depth > sizes->surface_depth)
-				continue;
+		if (surface_format != DRM_FORMAT_INVALID)
+			break; /* found supported format */
 
-			/* Best depth found so far */
-			if (fmt->depth > best_depth)
-				best_depth = fmt->depth;
-		}
+		/* try preferred bpp/depth */
+		surface_format = drm_fb_helper_find_format(fb_helper, plane->format_types,
+							   plane->format_count, preferred_bpp,
+							   dev->mode_config.preferred_depth);
+		if (surface_format != DRM_FORMAT_INVALID)
+			break; /* found supported format */
 	}
-	if (sizes->surface_depth != best_depth && best_depth) {
-		drm_info(dev, "requested bpp %d, scaled depth down to %d",
-			 sizes->surface_bpp, best_depth);
-		sizes->surface_depth = best_depth;
+
+	if (surface_format == DRM_FORMAT_INVALID) {
+		drm_warn(dev, "No compatible format found\n");
+		return -EAGAIN;
 	}
 
+	info = drm_format_info(surface_format);
+	sizes->surface_bpp = drm_format_info_bpp(info, 0);
+	sizes->surface_depth = info->depth;
+
 	/* first up get a count of crtcs now in use and new min/maxes width/heights */
 	crtc_count = 0;
 	drm_client_for_each_modeset(mode_set, client) {
-- 
2.38.1

[PATCH 8/9] drm/format-helper: Simplify drm_fb_build_fourcc_list()

[Thomas Zimmermann]

The DRM helper drm_fb_build_fourcc_list() creates a list of color
formats for primary planes of the generic drivers. Simplify the helper:

 - It used to mix and filter native and emulated formats as provided
   by the driver. Now the only emulated format is XRGB8888, which is
   required as fallback by legacy software. Drop support for emulating
   any other formats.
 - Also convert alpha formats to their non-alpha counterparts. Generic
   drivers don't support primary planes with alpha formats and some
   DTs incorrectly advertise alpha channels for non-alpha hardware. So
   only export non-alpha formats for primary planes.

With the simplified helper, scrap format lists of the affected generic
drivers. All they need is the firmware buffer's native format, from which
the helper creates the list of color formats.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 112 ++++++++++++++--------------
 drivers/gpu/drm/tiny/ofdrm.c        |  20 -----
 drivers/gpu/drm/tiny/simpledrm.c    |  21 ------
 include/drm/drm_format_helper.h     |   1 -
 4 files changed, 57 insertions(+), 97 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index aa6138756458..25d572f7b116 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -1068,6 +1068,39 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono);
 
+static uint32_t drm_fb_nonalpha_fourcc(uint32_t fourcc)
+{
+	/* only handle formats with depth != 0 and alpha channel */
+	switch (fourcc) {
+	case DRM_FORMAT_ARGB1555:
+		return DRM_FORMAT_XRGB1555;
+	case DRM_FORMAT_ABGR1555:
+		return DRM_FORMAT_XBGR1555;
+	case DRM_FORMAT_RGBA5551:
+		return DRM_FORMAT_RGBX5551;
+	case DRM_FORMAT_BGRA5551:
+		return DRM_FORMAT_BGRX5551;
+	case DRM_FORMAT_ARGB8888:
+		return DRM_FORMAT_XRGB8888;
+	case DRM_FORMAT_ABGR8888:
+		return DRM_FORMAT_XBGR8888;
+	case DRM_FORMAT_RGBA8888:
+		return DRM_FORMAT_RGBX8888;
+	case DRM_FORMAT_BGRA8888:
+		return DRM_FORMAT_BGRX8888;
+	case DRM_FORMAT_ARGB2101010:
+		return DRM_FORMAT_XRGB2101010;
+	case DRM_FORMAT_ABGR2101010:
+		return DRM_FORMAT_XBGR2101010;
+	case DRM_FORMAT_RGBA1010102:
+		return DRM_FORMAT_RGBX1010102;
+	case DRM_FORMAT_BGRA1010102:
+		return DRM_FORMAT_BGRX1010102;
+	}
+
+	return fourcc;
+}
+
 static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, uint32_t fourcc)
 {
 	const uint32_t *fourccs_end = fourccs + nfourccs;
@@ -1080,73 +1113,48 @@ static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, uint32_t
 	return false;
 }
 
-static const uint32_t conv_from_xrgb8888[] = {
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_ARGB8888,
-	DRM_FORMAT_XRGB2101010,
-	DRM_FORMAT_ARGB2101010,
-	DRM_FORMAT_RGB565,
-	DRM_FORMAT_RGB888,
-};
-
-static const uint32_t conv_from_rgb565_888[] = {
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_ARGB8888,
-};
-
-static bool is_conversion_supported(uint32_t from, uint32_t to)
-{
-	switch (from) {
-	case DRM_FORMAT_XRGB8888:
-	case DRM_FORMAT_ARGB8888:
-		return is_listed_fourcc(conv_from_xrgb8888, ARRAY_SIZE(conv_from_xrgb8888), to);
-	case DRM_FORMAT_RGB565:
-	case DRM_FORMAT_RGB888:
-		return is_listed_fourcc(conv_from_rgb565_888, ARRAY_SIZE(conv_from_rgb565_888), to);
-	case DRM_FORMAT_XRGB2101010:
-		return to == DRM_FORMAT_ARGB2101010;
-	case DRM_FORMAT_ARGB2101010:
-		return to == DRM_FORMAT_XRGB2101010;
-	default:
-		return false;
-	}
-}
-
 /**
  * drm_fb_build_fourcc_list - Filters a list of supported color formats against
  *                            the device's native formats
  * @dev: DRM device
  * @native_fourccs: 4CC codes of natively supported color formats
  * @native_nfourccs: The number of entries in @native_fourccs
- * @driver_fourccs: 4CC codes of all driver-supported color formats
- * @driver_nfourccs: The number of entries in @driver_fourccs
  * @fourccs_out: Returns 4CC codes of supported color formats
  * @nfourccs_out: The number of available entries in @fourccs_out
  *
  * This function create a list of supported color format from natively
- * supported formats and the emulated formats.
+ * supported formats and additional emulated formats.
  * At a minimum, most userspace programs expect at least support for
  * XRGB8888 on the primary plane. Devices that have to emulate the
  * format, and possibly others, can use drm_fb_build_fourcc_list() to
  * create a list of supported color formats. The returned list can
  * be handed over to drm_universal_plane_init() et al. Native formats
- * will go before emulated formats. Other heuristics might be applied
+ * will go before emulated formats. Native formats with alpha channel
+ * will be replaced by such without, as primary planes usually don't
+ * support alpha. Other heuristics might be applied
  * to optimize the order. Formats near the beginning of the list are
- * usually preferred over formats near the end of the list. Formats
- * without conversion helpers will be skipped. New drivers should only
- * pass in XRGB8888 and avoid exposing additional emulated formats.
+ * usually preferred over formats near the end of the list.
  *
  * Returns:
  * The number of color-formats 4CC codes returned in @fourccs_out.
  */
 size_t drm_fb_build_fourcc_list(struct drm_device *dev,
 				const u32 *native_fourccs, size_t native_nfourccs,
-				const u32 *driver_fourccs, size_t driver_nfourccs,
 				u32 *fourccs_out, size_t nfourccs_out)
 {
+	/*
+	 * XRGB8888 is the default fallback format for most of userspace
+	 * and it's currently the only format that should be emulated for
+	 * the primary plane. Only if there's ever another default fallback,
+	 * it should be added here.
+	 */
+	static const uint32_t extra_fourccs[] = {
+		DRM_FORMAT_XRGB8888,
+	};
+	static const size_t extra_nfourccs = ARRAY_SIZE(extra_fourccs);
+
 	u32 *fourccs = fourccs_out;
 	const u32 *fourccs_end = fourccs_out + nfourccs_out;
-	uint32_t native_format = 0;
 	size_t i;
 
 	/*
@@ -1154,7 +1162,12 @@ size_t drm_fb_build_fourcc_list(struct drm_device *dev,
 	 */
 
 	for (i = 0; i < native_nfourccs; ++i) {
-		u32 fourcc = native_fourccs[i];
+		/*
+		 * Several DTs, boot loaders and firmware report native
+		 * alpha formats that are non-alpha formats instead. So
+		 * replace alpha formats by non-alpha formats.
+		 */
+		u32 fourcc = drm_fb_nonalpha_fourcc(native_fourccs[i]);
 
 		if (is_listed_fourcc(fourccs_out, fourccs - fourccs_out, fourcc)) {
 			continue; /* skip duplicate entries */
@@ -1165,14 +1178,6 @@ size_t drm_fb_build_fourcc_list(struct drm_device *dev,
 
 		drm_dbg_kms(dev, "adding native format %p4cc\n", &fourcc);
 
-		/*
-		 * There should only be one native format with the current API.
-		 * This API needs to be refactored to correctly support arbitrary
-		 * sets of native formats, since it needs to report which native
-		 * format to use for each emulated format.
-		 */
-		if (!native_format)
-			native_format = fourcc;
 		*fourccs = fourcc;
 		++fourccs;
 	}
@@ -1181,17 +1186,14 @@ size_t drm_fb_build_fourcc_list(struct drm_device *dev,
 	 * The extra formats, emulated by the driver, go second.
 	 */
 
-	for (i = 0; (i < driver_nfourccs) && (fourccs < fourccs_end); ++i) {
-		u32 fourcc = driver_fourccs[i];
+	for (i = 0; (i < extra_nfourccs) && (fourccs < fourccs_end); ++i) {
+		u32 fourcc = extra_fourccs[i];
 
 		if (is_listed_fourcc(fourccs_out, fourccs - fourccs_out, fourcc)) {
 			continue; /* skip duplicate and native entries */
 		} else if (fourccs == fourccs_end) {
 			drm_warn(dev, "Ignoring emulated format %p4cc\n", &fourcc);
 			continue; /* end of available output buffer */
-		} else if (!is_conversion_supported(fourcc, native_format)) {
-			drm_dbg_kms(dev, "Unsupported emulated format %p4cc\n", &fourcc);
-			continue; /* format is not supported for conversion */
 		}
 
 		drm_dbg_kms(dev, "adding emulated format %p4cc\n", &fourcc);
diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c
index 33eefeba437c..39c5fd463fec 100644
--- a/drivers/gpu/drm/tiny/ofdrm.c
+++ b/drivers/gpu/drm/tiny/ofdrm.c
@@ -754,24 +754,6 @@ static void ofdrm_crtc_state_destroy(struct ofdrm_crtc_state *ofdrm_crtc_state)
 	kfree(ofdrm_crtc_state);
 }
 
-/*
- * Support all formats of OF display and maybe more; in order
- * of preference. The display's update function will do any
- * conversion necessary.
- *
- * TODO: Add blit helpers for remaining formats and uncomment
- *       constants.
- */
-static const uint32_t ofdrm_primary_plane_formats[] = {
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_RGB565,
-	//DRM_FORMAT_XRGB1555,
-	//DRM_FORMAT_C8,
-	/* Big-endian formats below */
-	DRM_FORMAT_BGRX8888,
-	DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN,
-};
-
 static const uint64_t ofdrm_primary_plane_format_modifiers[] = {
 	DRM_FORMAT_MOD_LINEAR,
 	DRM_FORMAT_MOD_INVALID
@@ -1290,8 +1272,6 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
 	/* Primary plane */
 
 	nformats = drm_fb_build_fourcc_list(dev, &format->format, 1,
-					    ofdrm_primary_plane_formats,
-					    ARRAY_SIZE(ofdrm_primary_plane_formats),
 					    odev->formats, ARRAY_SIZE(odev->formats));
 
 	primary_plane = &odev->primary_plane;
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 30e928d627e8..7355617f38d3 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -446,25 +446,6 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
  * Modesetting
  */
 
-/*
- * Support all formats of simplefb and maybe more; in order
- * of preference. The display's update function will do any
- * conversion necessary.
- *
- * TODO: Add blit helpers for remaining formats and uncomment
- *       constants.
- */
-static const uint32_t simpledrm_primary_plane_formats[] = {
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_ARGB8888,
-	DRM_FORMAT_RGB565,
-	//DRM_FORMAT_XRGB1555,
-	//DRM_FORMAT_ARGB1555,
-	DRM_FORMAT_RGB888,
-	DRM_FORMAT_XRGB2101010,
-	DRM_FORMAT_ARGB2101010,
-};
-
 static const uint64_t simpledrm_primary_plane_format_modifiers[] = {
 	DRM_FORMAT_MOD_LINEAR,
 	DRM_FORMAT_MOD_INVALID
@@ -745,8 +726,6 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	/* Primary plane */
 
 	nformats = drm_fb_build_fourcc_list(dev, &format->format, 1,
-					    simpledrm_primary_plane_formats,
-					    ARRAY_SIZE(simpledrm_primary_plane_formats),
 					    sdev->formats, ARRAY_SIZE(sdev->formats));
 
 	primary_plane = &sdev->primary_plane;
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 2d04f5863722..291deb09475b 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -65,7 +65,6 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 
 size_t drm_fb_build_fourcc_list(struct drm_device *dev,
 				const u32 *native_fourccs, size_t native_nfourccs,
-				const u32 *extra_fourccs, size_t extra_nfourccs,
 				u32 *fourccs_out, size_t nfourccs_out);
 
 #endif /* __LINUX_DRM_FORMAT_HELPER_H */
-- 
2.38.1

[PATCH 9/9] drm/format-helper: Remove unnecessary conversion helpers

[Thomas Zimmermann]

Drivers only emulate XRGB8888 framebuffers. Remove all conversion
helpers that do not use XRGB8888 as their source format. Also remove
some special cases for alpha formats in the blit helper.

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

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 25d572f7b116..46e02444d566 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -647,65 +647,6 @@ void drm_fb_xrgb8888_to_argb8888(struct iosys_map *dst, const unsigned int *dst_
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb8888);
 
-static void drm_fb_rgb565_to_xrgb8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
-{
-	__le32 *dbuf32 = dbuf;
-	const __le16 *sbuf16 = sbuf;
-	unsigned int x;
-
-	for (x = 0; x < pixels; x++) {
-		u16 val16 = le16_to_cpu(sbuf16[x]);
-		u32 val32 = ((val16 & 0xf800) << 8) |
-			    ((val16 & 0x07e0) << 5) |
-			    ((val16 & 0x001f) << 3);
-		val32 = 0xff000000 | val32 |
-			((val32 >> 3) & 0x00070007) |
-			((val32 >> 2) & 0x00000300);
-		dbuf32[x] = cpu_to_le32(val32);
-	}
-}
-
-static void drm_fb_rgb565_to_xrgb8888(struct iosys_map *dst, const unsigned int *dst_pitch,
-				      const struct iosys_map *src,
-				      const struct drm_framebuffer *fb,
-				      const struct drm_rect *clip)
-{
-	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
-		4,
-	};
-
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, 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)
-{
-	__le32 *dbuf32 = dbuf;
-	const u8 *sbuf8 = sbuf;
-	unsigned int x;
-
-	for (x = 0; x < pixels; x++) {
-		u8 r = *sbuf8++;
-		u8 g = *sbuf8++;
-		u8 b = *sbuf8++;
-		u32 pix = 0xff000000 | (r << 16) | (g << 8) | b;
-		dbuf32[x] = cpu_to_le32(pix);
-	}
-}
-
-static void drm_fb_rgb888_to_xrgb8888(struct iosys_map *dst, const unsigned int *dst_pitch,
-				      const struct iosys_map *src,
-				      const struct drm_framebuffer *fb,
-				      const struct drm_rect *clip)
-{
-	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
-		4,
-	};
-
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, 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)
 {
 	__le32 *dbuf32 = dbuf;
@@ -897,12 +838,6 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 {
 	uint32_t fb_format = fb->format->format;
 
-	/* treat alpha channel like filler bits */
-	if (fb_format == DRM_FORMAT_ARGB8888)
-		fb_format = DRM_FORMAT_XRGB8888;
-	if (fb_format == DRM_FORMAT_ARGB2101010)
-		fb_format = DRM_FORMAT_XRGB2101010;
-
 	if (fb_format == dst_format) {
 		drm_fb_memcpy(dst, dst_pitch, src, fb, clip);
 		return 0;
@@ -941,16 +876,6 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 			drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
 			return 0;
 		}
-	} else if (fb_format == DRM_FORMAT_RGB888) {
-		if (dst_format == DRM_FORMAT_XRGB8888) {
-			drm_fb_rgb888_to_xrgb8888(dst, dst_pitch, src, fb, clip);
-			return 0;
-		}
-	} else if (fb_format == DRM_FORMAT_RGB565) {
-		if (dst_format == DRM_FORMAT_XRGB8888) {
-			drm_fb_rgb565_to_xrgb8888(dst, dst_pitch, src, fb, clip);
-			return 0;
-		}
 	}
 
 	drm_warn_once(fb->dev, "No conversion helper from %p4cc to %p4cc found.\n",
-- 
2.38.1

Re: [PATCH 5/9] drm/format-helper: Add conversion from XRGB8888 to 15-bit RGB555 formats

[José Expósito]

Hi Thomas,

Thanks for CCing me.

There are some issues with this helpers on big endian architectures, you can
run the test on big endian with this command:

 $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests \
   --arch=powerpc --cross_compile=powerpc64-linux-gnu- drm_format_helper_test

The problem is in the drm_fb_xrgb8888_to_*_line() functions, see the fixes
inlined:

On Tue, Dec 13, 2022 at 09:12:29PM +0100, Thomas Zimmermann wrote:
> Add conversion from XRGB8888 to XRGB1555, ARGB1555 and RGBA5551, which
> are the formats currently supported by the simplefb infrastructure. The
> new helpers allow the output of XRGB8888 framebuffers to firmware
> scanout buffers in one of the 15-bit formats.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_format_helper.c           | 164 +++++++++++++++
>  .../gpu/drm/tests/drm_format_helper_test.c    | 186 ++++++++++++++++++
>  include/drm/drm_format_helper.h               |   9 +
>  3 files changed, 359 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index e562a3cefb89..aa6138756458 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -395,6 +395,161 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
>  }
>  EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
>  
> +static void drm_fb_xrgb8888_to_xrgb1555_line(void *dbuf, const void *sbuf, unsigned int pixels)
> +{
> +	u16 *dbuf16 = dbuf;
> +	const __le32 *sbuf32 = sbuf;
> +	unsigned int x;
> +	u16 val16;
> +	u32 pix;
> +
> +	for (x = 0; x < pixels; x++) {
> +		pix = le32_to_cpu(sbuf32[x]);
> +		val16 = ((pix & 0x00f80000) >> 9) |
> +			((pix & 0x0000f800) >> 6) |
> +			((pix & 0x000000f8) >> 3);
> +		dbuf16[x] = cpu_to_le16(val16);

You don't need the extra cpu_to_le16() here:

 - dbuf16[x] = cpu_to_le16(val16);
 + dbuf16[x] = val16;

> +	}
> +}
> +
> +/**
> + * drm_fb_xrgb8888_to_xrgb1555 - Convert XRGB8888 to XRGB1555 clip buffer
> + * @dst: Array of XRGB1555 destination buffers
> + * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
> + *             within @dst; can be NULL if scanlines are stored next to each other.
> + * @src: Array of XRGB8888 source buffer
> + * @fb: DRM framebuffer
> + * @clip: Clip rectangle area to copy
> + *
> + * This function copies parts of a framebuffer to display memory and converts
> + * the color format during the process. The parameters @dst, @dst_pitch and
> + * @src refer to arrays. Each array must have at least as many entries as
> + * there are planes in @fb's format. Each entry stores the value for the
> + * format's respective color plane at the same index.
> + *
> + * This function does not apply clipping on @dst (i.e. the destination is at the
> + * top-left corner).
> + *
> + * Drivers can use this function for XRGB1555 devices that don't support
> + * XRGB8888 natively.
> + */
> +void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
> +				 const struct drm_rect *clip)
> +{
> +	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> +		2,
> +	};
> +
> +	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
> +		    drm_fb_xrgb8888_to_xrgb1555_line);
> +}
> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb1555);
> +
> +static void drm_fb_xrgb8888_to_argb1555_line(void *dbuf, const void *sbuf, unsigned int pixels)
> +{
> +	u16 *dbuf16 = dbuf;
> +	const __le32 *sbuf32 = sbuf;
> +	unsigned int x;
> +	u16 val16;
> +	u32 pix;
> +
> +	for (x = 0; x < pixels; x++) {
> +		pix = le32_to_cpu(sbuf32[x]);
> +		val16 = BIT(15) | /* set alpha bit */
> +			((pix & 0x00f80000) >> 9) |
> +			((pix & 0x0000f800) >> 6) |
> +			((pix & 0x000000f8) >> 3);
> +		dbuf16[x] = cpu_to_le16(val16);

The same here:

 - dbuf16[x] = cpu_to_le16(val16);
 + dbuf16[x] = val16;

> +	}
> +}
> +
> +/**
> + * drm_fb_xrgb8888_to_argb1555 - Convert XRGB8888 to ARGB1555 clip buffer
> + * @dst: Array of ARGB1555 destination buffers
> + * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
> + *             within @dst; can be NULL if scanlines are stored next to each other.
> + * @src: Array of XRGB8888 source buffer
> + * @fb: DRM framebuffer
> + * @clip: Clip rectangle area to copy
> + *
> + * This function copies parts of a framebuffer to display memory and converts
> + * the color format during the process. The parameters @dst, @dst_pitch and
> + * @src refer to arrays. Each array must have at least as many entries as
> + * there are planes in @fb's format. Each entry stores the value for the
> + * format's respective color plane at the same index.
> + *
> + * This function does not apply clipping on @dst (i.e. the destination is at the
> + * top-left corner).
> + *
> + * Drivers can use this function for ARGB1555 devices that don't support
> + * XRGB8888 natively. It sets an opaque alpha channel as part of the conversion.
> + */
> +void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
> +				 const struct drm_rect *clip)
> +{
> +	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> +		2,
> +	};
> +
> +	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
> +		    drm_fb_xrgb8888_to_argb1555_line);
> +}
> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb1555);
> +
> +static void drm_fb_xrgb8888_to_rgba5551_line(void *dbuf, const void *sbuf, unsigned int pixels)
> +{
> +	u16 *dbuf16 = dbuf;
> +	const __le32 *sbuf32 = sbuf;
> +	unsigned int x;
> +	u16 val16;
> +	u32 pix;
> +
> +	for (x = 0; x < pixels; x++) {
> +		pix = le32_to_cpu(sbuf32[x]);
> +		val16 = ((pix & 0x00f80000) >> 8) |
> +			((pix & 0x0000f800) >> 5) |
> +			((pix & 0x000000f8) >> 2) |
> +			BIT(0); /* set alpha bit */
> +		dbuf16[x] = cpu_to_le16(val16);

And this is the last fix:

 - dbuf16[x] = cpu_to_le16(val16);
 + dbuf16[x] = val16;


Other than that, this patch looks good:

Reviewed-by: José Expósito <jose.exposito89@gmail.com>

Best wishes,
Jose

> +	}
> +}
> +
> +/**
> + * drm_fb_xrgb8888_to_rgba5551 - Convert XRGB8888 to RGBA5551 clip buffer
> + * @dst: Array of RGBA5551 destination buffers
> + * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
> + *             within @dst; can be NULL if scanlines are stored next to each other.
> + * @src: Array of XRGB8888 source buffer
> + * @fb: DRM framebuffer
> + * @clip: Clip rectangle area to copy
> + *
> + * This function copies parts of a framebuffer to display memory and converts
> + * the color format during the process. The parameters @dst, @dst_pitch and
> + * @src refer to arrays. Each array must have at least as many entries as
> + * there are planes in @fb's format. Each entry stores the value for the
> + * format's respective color plane at the same index.
> + *
> + * This function does not apply clipping on @dst (i.e. the destination is at the
> + * top-left corner).
> + *
> + * Drivers can use this function for RGBA5551 devices that don't support
> + * XRGB8888 natively. It sets an opaque alpha channel as part of the conversion.
> + */
> +void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_pitch,
> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
> +				 const struct drm_rect *clip)
> +{
> +	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> +		2,
> +	};
> +
> +	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
> +		    drm_fb_xrgb8888_to_rgba5551_line);
> +}
> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgba5551);
> +
>  static void drm_fb_xrgb8888_to_rgb888_line(void *dbuf, const void *sbuf, unsigned int pixels)
>  {
>  	u8 *dbuf8 = dbuf;
> @@ -761,6 +916,15 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
>  		if (dst_format == DRM_FORMAT_RGB565) {
>  			drm_fb_xrgb8888_to_rgb565(dst, dst_pitch, src, fb, clip, false);
>  			return 0;
> +		} else if (dst_format == DRM_FORMAT_XRGB1555) {
> +			drm_fb_xrgb8888_to_xrgb1555(dst, dst_pitch, src, fb, clip);
> +			return 0;
> +		} else if (dst_format == DRM_FORMAT_ARGB1555) {
> +			drm_fb_xrgb8888_to_argb1555(dst, dst_pitch, src, fb, clip);
> +			return 0;
> +		} else if (dst_format == DRM_FORMAT_RGBA5551) {
> +			drm_fb_xrgb8888_to_rgba5551(dst, dst_pitch, src, fb, clip);
> +			return 0;
>  		} else if (dst_format == DRM_FORMAT_RGB888) {
>  			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip);
>  			return 0;
> diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
> index b15dc7c1eb96..e01695c70bb6 100644
> --- a/drivers/gpu/drm/tests/drm_format_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
> @@ -32,6 +32,21 @@ struct convert_to_rgb565_result {
>  	const u16 expected_swab[TEST_BUF_SIZE];
>  };
>  
> +struct convert_to_xrgb1555_result {
> +	unsigned int dst_pitch;
> +	const u16 expected[TEST_BUF_SIZE];
> +};
> +
> +struct convert_to_argb1555_result {
> +	unsigned int dst_pitch;
> +	const u16 expected[TEST_BUF_SIZE];
> +};
> +
> +struct convert_to_rgba5551_result {
> +	unsigned int dst_pitch;
> +	const u16 expected[TEST_BUF_SIZE];
> +};
> +
>  struct convert_to_rgb888_result {
>  	unsigned int dst_pitch;
>  	const u8 expected[TEST_BUF_SIZE];
> @@ -60,6 +75,9 @@ struct convert_xrgb8888_case {
>  	struct convert_to_gray8_result gray8_result;
>  	struct convert_to_rgb332_result rgb332_result;
>  	struct convert_to_rgb565_result rgb565_result;
> +	struct convert_to_xrgb1555_result xrgb1555_result;
> +	struct convert_to_argb1555_result argb1555_result;
> +	struct convert_to_rgba5551_result rgba5551_result;
>  	struct convert_to_rgb888_result rgb888_result;
>  	struct convert_to_argb8888_result argb8888_result;
>  	struct convert_to_xrgb2101010_result xrgb2101010_result;
> @@ -85,6 +103,18 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
>  			.expected = { 0xF800 },
>  			.expected_swab = { 0x00F8 },
>  		},
> +		.xrgb1555_result = {
> +			.dst_pitch = 0,
> +			.expected = { 0x7C00 },
> +		},
> +		.argb1555_result = {
> +			.dst_pitch = 0,
> +			.expected = { 0xFC00 },
> +		},
> +		.rgba5551_result = {
> +			.dst_pitch = 0,
> +			.expected = { 0xF801 },
> +		},
>  		.rgb888_result = {
>  			.dst_pitch = 0,
>  			.expected = { 0x00, 0x00, 0xFF },
> @@ -123,6 +153,18 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
>  			.expected = { 0xF800 },
>  			.expected_swab = { 0x00F8 },
>  		},
> +		.xrgb1555_result = {
> +			.dst_pitch = 0,
> +			.expected = { 0x7C00 },
> +		},
> +		.argb1555_result = {
> +			.dst_pitch = 0,
> +			.expected = { 0xFC00 },
> +		},
> +		.rgba5551_result = {
> +			.dst_pitch = 0,
> +			.expected = { 0xF801 },
> +		},
>  		.rgb888_result = {
>  			.dst_pitch = 0,
>  			.expected = { 0x00, 0x00, 0xFF },
> @@ -188,6 +230,33 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
>  				0xE0FF, 0xFF07,
>  			},
>  		},
> +		.xrgb1555_result = {
> +			.dst_pitch = 0,
> +			.expected = {
> +				0x7FFF, 0x0000,
> +				0x7C00, 0x03E0,
> +				0x001F, 0x7C1F,
> +				0x7FE0, 0x03FF,
> +			},
> +		},
> +		.argb1555_result = {
> +			.dst_pitch = 0,
> +			.expected = {
> +				0xFFFF, 0x8000,
> +				0xFC00, 0x83E0,
> +				0x801F, 0xFC1F,
> +				0xFFE0, 0x83FF,
> +			},
> +		},
> +		.rgba5551_result = {
> +			.dst_pitch = 0,
> +			.expected = {
> +				0xFFFF, 0x0001,
> +				0xF801, 0x07C1,
> +				0x003F, 0xF83F,
> +				0xFFC1, 0x07FF,
> +			},
> +		},
>  		.rgb888_result = {
>  			.dst_pitch = 0,
>  			.expected = {
> @@ -264,6 +333,30 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
>  				0x00A8, 0x8E6B, 0x330A, 0x0000, 0x0000,
>  			},
>  		},
> +		.xrgb1555_result = {
> +			.dst_pitch = 10,
> +			.expected = {
> +				0x0513, 0x0920, 0x5400, 0x0000, 0x0000,
> +				0x35CE, 0x0513, 0x0920, 0x0000, 0x0000,
> +				0x5400, 0x35CE, 0x0513, 0x0000, 0x0000,
> +			},
> +		},
> +		.argb1555_result = {
> +			.dst_pitch = 10,
> +			.expected = {
> +				0x8513, 0x8920, 0xD400, 0x0000, 0x0000,
> +				0xB5CE, 0x8513, 0x8920, 0x0000, 0x0000,
> +				0xD400, 0xB5CE, 0x8513, 0x0000, 0x0000,
> +			},
> +		},
> +		.rgba5551_result = {
> +			.dst_pitch = 10,
> +			.expected = {
> +				0x0A27, 0x1241, 0xA801, 0x0000, 0x0000,
> +				0x6B9D, 0x0A27, 0x1241, 0x0000, 0x0000,
> +				0xA801, 0x6B9D, 0x0A27, 0x0000, 0x0000,
> +			},
> +		},
>  		.rgb888_result = {
>  			.dst_pitch = 15,
>  			.expected = {
> @@ -443,6 +536,96 @@ static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
>  	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected_swab, dst_size), 0);
>  }
>  
> +static void drm_test_fb_xrgb8888_to_xrgb1555(struct kunit *test)
> +{
> +	const struct convert_xrgb8888_case *params = test->param_value;
> +	const struct convert_to_xrgb1555_result *result = &params->xrgb1555_result;
> +	size_t dst_size;
> +	__u16 *buf = NULL;
> +	__u32 *xrgb8888 = NULL;
> +	struct iosys_map dst, src;
> +
> +	struct drm_framebuffer fb = {
> +		.format = drm_format_info(DRM_FORMAT_XRGB8888),
> +		.pitches = { params->pitch, 0, 0 },
> +	};
> +
> +	dst_size = conversion_buf_size(DRM_FORMAT_XRGB1555, result->dst_pitch,
> +				       &params->clip);
> +	KUNIT_ASSERT_GT(test, dst_size, 0);
> +
> +	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
> +	iosys_map_set_vaddr(&dst, buf);
> +
> +	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
> +	iosys_map_set_vaddr(&src, xrgb8888);
> +
> +	drm_fb_xrgb8888_to_xrgb1555(&dst, &result->dst_pitch, &src, &fb, &params->clip);
> +	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
> +}
> +
> +static void drm_test_fb_xrgb8888_to_argb1555(struct kunit *test)
> +{
> +	const struct convert_xrgb8888_case *params = test->param_value;
> +	const struct convert_to_argb1555_result *result = &params->argb1555_result;
> +	size_t dst_size;
> +	__u16 *buf = NULL;
> +	__u32 *xrgb8888 = NULL;
> +	struct iosys_map dst, src;
> +
> +	struct drm_framebuffer fb = {
> +		.format = drm_format_info(DRM_FORMAT_XRGB8888),
> +		.pitches = { params->pitch, 0, 0 },
> +	};
> +
> +	dst_size = conversion_buf_size(DRM_FORMAT_ARGB1555, result->dst_pitch,
> +				       &params->clip);
> +	KUNIT_ASSERT_GT(test, dst_size, 0);
> +
> +	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
> +	iosys_map_set_vaddr(&dst, buf);
> +
> +	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
> +	iosys_map_set_vaddr(&src, xrgb8888);
> +
> +	drm_fb_xrgb8888_to_argb1555(&dst, &result->dst_pitch, &src, &fb, &params->clip);
> +	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
> +}
> +
> +static void drm_test_fb_xrgb8888_to_rgba5551(struct kunit *test)
> +{
> +	const struct convert_xrgb8888_case *params = test->param_value;
> +	const struct convert_to_rgba5551_result *result = &params->rgba5551_result;
> +	size_t dst_size;
> +	__u16 *buf = NULL;
> +	__u32 *xrgb8888 = NULL;
> +	struct iosys_map dst, src;
> +
> +	struct drm_framebuffer fb = {
> +		.format = drm_format_info(DRM_FORMAT_XRGB8888),
> +		.pitches = { params->pitch, 0, 0 },
> +	};
> +
> +	dst_size = conversion_buf_size(DRM_FORMAT_RGBA5551, result->dst_pitch,
> +				       &params->clip);
> +	KUNIT_ASSERT_GT(test, dst_size, 0);
> +
> +	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
> +	iosys_map_set_vaddr(&dst, buf);
> +
> +	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
> +	iosys_map_set_vaddr(&src, xrgb8888);
> +
> +	drm_fb_xrgb8888_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, &params->clip);
> +	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
> +}
> +
>  static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
>  {
>  	const struct convert_xrgb8888_case *params = test->param_value;
> @@ -570,6 +753,9 @@ static struct kunit_case drm_format_helper_test_cases[] = {
>  	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_gray8, convert_xrgb8888_gen_params),
>  	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb332, convert_xrgb8888_gen_params),
>  	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb565, convert_xrgb8888_gen_params),
> +	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb1555, convert_xrgb8888_gen_params),
> +	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb1555, convert_xrgb8888_gen_params),
> +	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgba5551, convert_xrgb8888_gen_params),
>  	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb888, convert_xrgb8888_gen_params),
>  	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb8888, convert_xrgb8888_gen_params),
>  	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb2101010, convert_xrgb8888_gen_params),
> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
> index 10b2d19cdb66..2d04f5863722 100644
> --- a/include/drm/drm_format_helper.h
> +++ b/include/drm/drm_format_helper.h
> @@ -30,6 +30,15 @@ void drm_fb_xrgb8888_to_rgb332(struct iosys_map *dst, const unsigned int *dst_pi
>  void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pitch,
>  			       const struct iosys_map *src, const struct drm_framebuffer *fb,
>  			       const struct drm_rect *clip, bool swab);
> +void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
> +				 const struct drm_rect *clip);
> +void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
> +				 const struct drm_rect *clip);
> +void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_pitch,
> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
> +				 const struct drm_rect *clip);
>  void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pitch,
>  			       const struct iosys_map *src, const struct drm_framebuffer *fb,
>  			       const struct drm_rect *clip);
> -- 
> 2.38.1
> 

Re: [PATCH 5/9] drm/format-helper: Add conversion from XRGB8888 to 15-bit RGB555 formats

[Thomas Zimmermann]

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

Hi

Am 19.12.22 um 10:23 schrieb José Expósito:
> Hi Thomas,
> 
> Thanks for CCing me.
> 
> There are some issues with this helpers on big endian architectures, you can
> run the test on big endian with this command:
> 
>   $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests \
>     --arch=powerpc --cross_compile=powerpc64-linux-gnu- drm_format_helper_test
> 
> The problem is in the drm_fb_xrgb8888_to_*_line() functions, see the fixes
> inlined:
> 
> On Tue, Dec 13, 2022 at 09:12:29PM +0100, Thomas Zimmermann wrote:
>> Add conversion from XRGB8888 to XRGB1555, ARGB1555 and RGBA5551, which
>> are the formats currently supported by the simplefb infrastructure. The
>> new helpers allow the output of XRGB8888 framebuffers to firmware
>> scanout buffers in one of the 15-bit formats.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/drm_format_helper.c           | 164 +++++++++++++++
>>   .../gpu/drm/tests/drm_format_helper_test.c    | 186 ++++++++++++++++++
>>   include/drm/drm_format_helper.h               |   9 +
>>   3 files changed, 359 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
>> index e562a3cefb89..aa6138756458 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
>> @@ -395,6 +395,161 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
>>   }
>>   EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
>>   
>> +static void drm_fb_xrgb8888_to_xrgb1555_line(void *dbuf, const void *sbuf, unsigned int pixels)
>> +{
>> +	u16 *dbuf16 = dbuf;
>> +	const __le32 *sbuf32 = sbuf;
>> +	unsigned int x;
>> +	u16 val16;
>> +	u32 pix;
>> +
>> +	for (x = 0; x < pixels; x++) {
>> +		pix = le32_to_cpu(sbuf32[x]);
>> +		val16 = ((pix & 0x00f80000) >> 9) |
>> +			((pix & 0x0000f800) >> 6) |
>> +			((pix & 0x000000f8) >> 3);
>> +		dbuf16[x] = cpu_to_le16(val16);
> 
> You don't need the extra cpu_to_le16() here:
> 
>   - dbuf16[x] = cpu_to_le16(val16);
>   + dbuf16[x] = val16;

Thanks for testing this. The DRM formats are defined to be in little 
endian (unless the DRM_FORMAT_BIG_ENDIAN flag has been set). So we have 
to convert the result from host to LE endianess.

I'm pretty sure that the hardcoded output in the testcases needs to be 
adapted instead. But did this never happen with the older tests?

Best regards
Thomas

> 
>> +	}
>> +}
>> +
>> +/**
>> + * drm_fb_xrgb8888_to_xrgb1555 - Convert XRGB8888 to XRGB1555 clip buffer
>> + * @dst: Array of XRGB1555 destination buffers
>> + * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
>> + *             within @dst; can be NULL if scanlines are stored next to each other.
>> + * @src: Array of XRGB8888 source buffer
>> + * @fb: DRM framebuffer
>> + * @clip: Clip rectangle area to copy
>> + *
>> + * This function copies parts of a framebuffer to display memory and converts
>> + * the color format during the process. The parameters @dst, @dst_pitch and
>> + * @src refer to arrays. Each array must have at least as many entries as
>> + * there are planes in @fb's format. Each entry stores the value for the
>> + * format's respective color plane at the same index.
>> + *
>> + * This function does not apply clipping on @dst (i.e. the destination is at the
>> + * top-left corner).
>> + *
>> + * Drivers can use this function for XRGB1555 devices that don't support
>> + * XRGB8888 natively.
>> + */
>> +void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
>> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
>> +				 const struct drm_rect *clip)
>> +{
>> +	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
>> +		2,
>> +	};
>> +
>> +	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
>> +		    drm_fb_xrgb8888_to_xrgb1555_line);
>> +}
>> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb1555);
>> +
>> +static void drm_fb_xrgb8888_to_argb1555_line(void *dbuf, const void *sbuf, unsigned int pixels)
>> +{
>> +	u16 *dbuf16 = dbuf;
>> +	const __le32 *sbuf32 = sbuf;
>> +	unsigned int x;
>> +	u16 val16;
>> +	u32 pix;
>> +
>> +	for (x = 0; x < pixels; x++) {
>> +		pix = le32_to_cpu(sbuf32[x]);
>> +		val16 = BIT(15) | /* set alpha bit */
>> +			((pix & 0x00f80000) >> 9) |
>> +			((pix & 0x0000f800) >> 6) |
>> +			((pix & 0x000000f8) >> 3);
>> +		dbuf16[x] = cpu_to_le16(val16);
> 
> The same here:
> 
>   - dbuf16[x] = cpu_to_le16(val16);
>   + dbuf16[x] = val16;
> 
>> +	}
>> +}
>> +
>> +/**
>> + * drm_fb_xrgb8888_to_argb1555 - Convert XRGB8888 to ARGB1555 clip buffer
>> + * @dst: Array of ARGB1555 destination buffers
>> + * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
>> + *             within @dst; can be NULL if scanlines are stored next to each other.
>> + * @src: Array of XRGB8888 source buffer
>> + * @fb: DRM framebuffer
>> + * @clip: Clip rectangle area to copy
>> + *
>> + * This function copies parts of a framebuffer to display memory and converts
>> + * the color format during the process. The parameters @dst, @dst_pitch and
>> + * @src refer to arrays. Each array must have at least as many entries as
>> + * there are planes in @fb's format. Each entry stores the value for the
>> + * format's respective color plane at the same index.
>> + *
>> + * This function does not apply clipping on @dst (i.e. the destination is at the
>> + * top-left corner).
>> + *
>> + * Drivers can use this function for ARGB1555 devices that don't support
>> + * XRGB8888 natively. It sets an opaque alpha channel as part of the conversion.
>> + */
>> +void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
>> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
>> +				 const struct drm_rect *clip)
>> +{
>> +	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
>> +		2,
>> +	};
>> +
>> +	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
>> +		    drm_fb_xrgb8888_to_argb1555_line);
>> +}
>> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb1555);
>> +
>> +static void drm_fb_xrgb8888_to_rgba5551_line(void *dbuf, const void *sbuf, unsigned int pixels)
>> +{
>> +	u16 *dbuf16 = dbuf;
>> +	const __le32 *sbuf32 = sbuf;
>> +	unsigned int x;
>> +	u16 val16;
>> +	u32 pix;
>> +
>> +	for (x = 0; x < pixels; x++) {
>> +		pix = le32_to_cpu(sbuf32[x]);
>> +		val16 = ((pix & 0x00f80000) >> 8) |
>> +			((pix & 0x0000f800) >> 5) |
>> +			((pix & 0x000000f8) >> 2) |
>> +			BIT(0); /* set alpha bit */
>> +		dbuf16[x] = cpu_to_le16(val16);
> 
> And this is the last fix:
> 
>   - dbuf16[x] = cpu_to_le16(val16);
>   + dbuf16[x] = val16;
> 
> 
> Other than that, this patch looks good:
> 
> Reviewed-by: José Expósito <jose.exposito89@gmail.com>
> 
> Best wishes,
> Jose
> 
>> +	}
>> +}
>> +
>> +/**
>> + * drm_fb_xrgb8888_to_rgba5551 - Convert XRGB8888 to RGBA5551 clip buffer
>> + * @dst: Array of RGBA5551 destination buffers
>> + * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
>> + *             within @dst; can be NULL if scanlines are stored next to each other.
>> + * @src: Array of XRGB8888 source buffer
>> + * @fb: DRM framebuffer
>> + * @clip: Clip rectangle area to copy
>> + *
>> + * This function copies parts of a framebuffer to display memory and converts
>> + * the color format during the process. The parameters @dst, @dst_pitch and
>> + * @src refer to arrays. Each array must have at least as many entries as
>> + * there are planes in @fb's format. Each entry stores the value for the
>> + * format's respective color plane at the same index.
>> + *
>> + * This function does not apply clipping on @dst (i.e. the destination is at the
>> + * top-left corner).
>> + *
>> + * Drivers can use this function for RGBA5551 devices that don't support
>> + * XRGB8888 natively. It sets an opaque alpha channel as part of the conversion.
>> + */
>> +void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_pitch,
>> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
>> +				 const struct drm_rect *clip)
>> +{
>> +	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
>> +		2,
>> +	};
>> +
>> +	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
>> +		    drm_fb_xrgb8888_to_rgba5551_line);
>> +}
>> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgba5551);
>> +
>>   static void drm_fb_xrgb8888_to_rgb888_line(void *dbuf, const void *sbuf, unsigned int pixels)
>>   {
>>   	u8 *dbuf8 = dbuf;
>> @@ -761,6 +916,15 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
>>   		if (dst_format == DRM_FORMAT_RGB565) {
>>   			drm_fb_xrgb8888_to_rgb565(dst, dst_pitch, src, fb, clip, false);
>>   			return 0;
>> +		} else if (dst_format == DRM_FORMAT_XRGB1555) {
>> +			drm_fb_xrgb8888_to_xrgb1555(dst, dst_pitch, src, fb, clip);
>> +			return 0;
>> +		} else if (dst_format == DRM_FORMAT_ARGB1555) {
>> +			drm_fb_xrgb8888_to_argb1555(dst, dst_pitch, src, fb, clip);
>> +			return 0;
>> +		} else if (dst_format == DRM_FORMAT_RGBA5551) {
>> +			drm_fb_xrgb8888_to_rgba5551(dst, dst_pitch, src, fb, clip);
>> +			return 0;
>>   		} else if (dst_format == DRM_FORMAT_RGB888) {
>>   			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip);
>>   			return 0;
>> diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
>> index b15dc7c1eb96..e01695c70bb6 100644
>> --- a/drivers/gpu/drm/tests/drm_format_helper_test.c
>> +++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
>> @@ -32,6 +32,21 @@ struct convert_to_rgb565_result {
>>   	const u16 expected_swab[TEST_BUF_SIZE];
>>   };
>>   
>> +struct convert_to_xrgb1555_result {
>> +	unsigned int dst_pitch;
>> +	const u16 expected[TEST_BUF_SIZE];
>> +};
>> +
>> +struct convert_to_argb1555_result {
>> +	unsigned int dst_pitch;
>> +	const u16 expected[TEST_BUF_SIZE];
>> +};
>> +
>> +struct convert_to_rgba5551_result {
>> +	unsigned int dst_pitch;
>> +	const u16 expected[TEST_BUF_SIZE];
>> +};
>> +
>>   struct convert_to_rgb888_result {
>>   	unsigned int dst_pitch;
>>   	const u8 expected[TEST_BUF_SIZE];
>> @@ -60,6 +75,9 @@ struct convert_xrgb8888_case {
>>   	struct convert_to_gray8_result gray8_result;
>>   	struct convert_to_rgb332_result rgb332_result;
>>   	struct convert_to_rgb565_result rgb565_result;
>> +	struct convert_to_xrgb1555_result xrgb1555_result;
>> +	struct convert_to_argb1555_result argb1555_result;
>> +	struct convert_to_rgba5551_result rgba5551_result;
>>   	struct convert_to_rgb888_result rgb888_result;
>>   	struct convert_to_argb8888_result argb8888_result;
>>   	struct convert_to_xrgb2101010_result xrgb2101010_result;
>> @@ -85,6 +103,18 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
>>   			.expected = { 0xF800 },
>>   			.expected_swab = { 0x00F8 },
>>   		},
>> +		.xrgb1555_result = {
>> +			.dst_pitch = 0,
>> +			.expected = { 0x7C00 },
>> +		},
>> +		.argb1555_result = {
>> +			.dst_pitch = 0,
>> +			.expected = { 0xFC00 },
>> +		},
>> +		.rgba5551_result = {
>> +			.dst_pitch = 0,
>> +			.expected = { 0xF801 },
>> +		},
>>   		.rgb888_result = {
>>   			.dst_pitch = 0,
>>   			.expected = { 0x00, 0x00, 0xFF },
>> @@ -123,6 +153,18 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
>>   			.expected = { 0xF800 },
>>   			.expected_swab = { 0x00F8 },
>>   		},
>> +		.xrgb1555_result = {
>> +			.dst_pitch = 0,
>> +			.expected = { 0x7C00 },
>> +		},
>> +		.argb1555_result = {
>> +			.dst_pitch = 0,
>> +			.expected = { 0xFC00 },
>> +		},
>> +		.rgba5551_result = {
>> +			.dst_pitch = 0,
>> +			.expected = { 0xF801 },
>> +		},
>>   		.rgb888_result = {
>>   			.dst_pitch = 0,
>>   			.expected = { 0x00, 0x00, 0xFF },
>> @@ -188,6 +230,33 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
>>   				0xE0FF, 0xFF07,
>>   			},
>>   		},
>> +		.xrgb1555_result = {
>> +			.dst_pitch = 0,
>> +			.expected = {
>> +				0x7FFF, 0x0000,
>> +				0x7C00, 0x03E0,
>> +				0x001F, 0x7C1F,
>> +				0x7FE0, 0x03FF,
>> +			},
>> +		},
>> +		.argb1555_result = {
>> +			.dst_pitch = 0,
>> +			.expected = {
>> +				0xFFFF, 0x8000,
>> +				0xFC00, 0x83E0,
>> +				0x801F, 0xFC1F,
>> +				0xFFE0, 0x83FF,
>> +			},
>> +		},
>> +		.rgba5551_result = {
>> +			.dst_pitch = 0,
>> +			.expected = {
>> +				0xFFFF, 0x0001,
>> +				0xF801, 0x07C1,
>> +				0x003F, 0xF83F,
>> +				0xFFC1, 0x07FF,
>> +			},
>> +		},
>>   		.rgb888_result = {
>>   			.dst_pitch = 0,
>>   			.expected = {
>> @@ -264,6 +333,30 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
>>   				0x00A8, 0x8E6B, 0x330A, 0x0000, 0x0000,
>>   			},
>>   		},
>> +		.xrgb1555_result = {
>> +			.dst_pitch = 10,
>> +			.expected = {
>> +				0x0513, 0x0920, 0x5400, 0x0000, 0x0000,
>> +				0x35CE, 0x0513, 0x0920, 0x0000, 0x0000,
>> +				0x5400, 0x35CE, 0x0513, 0x0000, 0x0000,
>> +			},
>> +		},
>> +		.argb1555_result = {
>> +			.dst_pitch = 10,
>> +			.expected = {
>> +				0x8513, 0x8920, 0xD400, 0x0000, 0x0000,
>> +				0xB5CE, 0x8513, 0x8920, 0x0000, 0x0000,
>> +				0xD400, 0xB5CE, 0x8513, 0x0000, 0x0000,
>> +			},
>> +		},
>> +		.rgba5551_result = {
>> +			.dst_pitch = 10,
>> +			.expected = {
>> +				0x0A27, 0x1241, 0xA801, 0x0000, 0x0000,
>> +				0x6B9D, 0x0A27, 0x1241, 0x0000, 0x0000,
>> +				0xA801, 0x6B9D, 0x0A27, 0x0000, 0x0000,
>> +			},
>> +		},
>>   		.rgb888_result = {
>>   			.dst_pitch = 15,
>>   			.expected = {
>> @@ -443,6 +536,96 @@ static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
>>   	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected_swab, dst_size), 0);
>>   }
>>   
>> +static void drm_test_fb_xrgb8888_to_xrgb1555(struct kunit *test)
>> +{
>> +	const struct convert_xrgb8888_case *params = test->param_value;
>> +	const struct convert_to_xrgb1555_result *result = &params->xrgb1555_result;
>> +	size_t dst_size;
>> +	__u16 *buf = NULL;
>> +	__u32 *xrgb8888 = NULL;
>> +	struct iosys_map dst, src;
>> +
>> +	struct drm_framebuffer fb = {
>> +		.format = drm_format_info(DRM_FORMAT_XRGB8888),
>> +		.pitches = { params->pitch, 0, 0 },
>> +	};
>> +
>> +	dst_size = conversion_buf_size(DRM_FORMAT_XRGB1555, result->dst_pitch,
>> +				       &params->clip);
>> +	KUNIT_ASSERT_GT(test, dst_size, 0);
>> +
>> +	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
>> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
>> +	iosys_map_set_vaddr(&dst, buf);
>> +
>> +	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
>> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
>> +	iosys_map_set_vaddr(&src, xrgb8888);
>> +
>> +	drm_fb_xrgb8888_to_xrgb1555(&dst, &result->dst_pitch, &src, &fb, &params->clip);
>> +	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
>> +}
>> +
>> +static void drm_test_fb_xrgb8888_to_argb1555(struct kunit *test)
>> +{
>> +	const struct convert_xrgb8888_case *params = test->param_value;
>> +	const struct convert_to_argb1555_result *result = &params->argb1555_result;
>> +	size_t dst_size;
>> +	__u16 *buf = NULL;
>> +	__u32 *xrgb8888 = NULL;
>> +	struct iosys_map dst, src;
>> +
>> +	struct drm_framebuffer fb = {
>> +		.format = drm_format_info(DRM_FORMAT_XRGB8888),
>> +		.pitches = { params->pitch, 0, 0 },
>> +	};
>> +
>> +	dst_size = conversion_buf_size(DRM_FORMAT_ARGB1555, result->dst_pitch,
>> +				       &params->clip);
>> +	KUNIT_ASSERT_GT(test, dst_size, 0);
>> +
>> +	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
>> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
>> +	iosys_map_set_vaddr(&dst, buf);
>> +
>> +	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
>> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
>> +	iosys_map_set_vaddr(&src, xrgb8888);
>> +
>> +	drm_fb_xrgb8888_to_argb1555(&dst, &result->dst_pitch, &src, &fb, &params->clip);
>> +	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
>> +}
>> +
>> +static void drm_test_fb_xrgb8888_to_rgba5551(struct kunit *test)
>> +{
>> +	const struct convert_xrgb8888_case *params = test->param_value;
>> +	const struct convert_to_rgba5551_result *result = &params->rgba5551_result;
>> +	size_t dst_size;
>> +	__u16 *buf = NULL;
>> +	__u32 *xrgb8888 = NULL;
>> +	struct iosys_map dst, src;
>> +
>> +	struct drm_framebuffer fb = {
>> +		.format = drm_format_info(DRM_FORMAT_XRGB8888),
>> +		.pitches = { params->pitch, 0, 0 },
>> +	};
>> +
>> +	dst_size = conversion_buf_size(DRM_FORMAT_RGBA5551, result->dst_pitch,
>> +				       &params->clip);
>> +	KUNIT_ASSERT_GT(test, dst_size, 0);
>> +
>> +	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
>> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
>> +	iosys_map_set_vaddr(&dst, buf);
>> +
>> +	xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
>> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
>> +	iosys_map_set_vaddr(&src, xrgb8888);
>> +
>> +	drm_fb_xrgb8888_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, &params->clip);
>> +	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
>> +}
>> +
>>   static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
>>   {
>>   	const struct convert_xrgb8888_case *params = test->param_value;
>> @@ -570,6 +753,9 @@ static struct kunit_case drm_format_helper_test_cases[] = {
>>   	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_gray8, convert_xrgb8888_gen_params),
>>   	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb332, convert_xrgb8888_gen_params),
>>   	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb565, convert_xrgb8888_gen_params),
>> +	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb1555, convert_xrgb8888_gen_params),
>> +	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb1555, convert_xrgb8888_gen_params),
>> +	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgba5551, convert_xrgb8888_gen_params),
>>   	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb888, convert_xrgb8888_gen_params),
>>   	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb8888, convert_xrgb8888_gen_params),
>>   	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb2101010, convert_xrgb8888_gen_params),
>> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
>> index 10b2d19cdb66..2d04f5863722 100644
>> --- a/include/drm/drm_format_helper.h
>> +++ b/include/drm/drm_format_helper.h
>> @@ -30,6 +30,15 @@ void drm_fb_xrgb8888_to_rgb332(struct iosys_map *dst, const unsigned int *dst_pi
>>   void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pitch,
>>   			       const struct iosys_map *src, const struct drm_framebuffer *fb,
>>   			       const struct drm_rect *clip, bool swab);
>> +void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
>> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
>> +				 const struct drm_rect *clip);
>> +void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
>> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
>> +				 const struct drm_rect *clip);
>> +void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_pitch,
>> +				 const struct iosys_map *src, const struct drm_framebuffer *fb,
>> +				 const struct drm_rect *clip);
>>   void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pitch,
>>   			       const struct iosys_map *src, const struct drm_framebuffer *fb,
>>   			       const struct drm_rect *clip);
>> -- 
>> 2.38.1
>>

-- 
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 5/9] drm/format-helper: Add conversion from XRGB8888 to 15-bit RGB555 formats

[Thomas Zimmermann]

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



Am 19.12.22 um 11:58 schrieb Thomas Zimmermann:
> Hi
> 
> Am 19.12.22 um 10:23 schrieb José Expósito:
>> Hi Thomas,
>>
>> Thanks for CCing me.
>>
>> There are some issues with this helpers on big endian architectures, 
>> you can
>> run the test on big endian with this command:
>>
>>   $ ./tools/testing/kunit/kunit.py run 
>> --kunitconfig=drivers/gpu/drm/tests \
>>     --arch=powerpc --cross_compile=powerpc64-linux-gnu- 
>> drm_format_helper_test
>>
>> The problem is in the drm_fb_xrgb8888_to_*_line() functions, see the 
>> fixes
>> inlined:
>>
>> On Tue, Dec 13, 2022 at 09:12:29PM +0100, Thomas Zimmermann wrote:
>>> Add conversion from XRGB8888 to XRGB1555, ARGB1555 and RGBA5551, which
>>> are the formats currently supported by the simplefb infrastructure. The
>>> new helpers allow the output of XRGB8888 framebuffers to firmware
>>> scanout buffers in one of the 15-bit formats.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>   drivers/gpu/drm/drm_format_helper.c           | 164 +++++++++++++++
>>>   .../gpu/drm/tests/drm_format_helper_test.c    | 186 ++++++++++++++++++
>>>   include/drm/drm_format_helper.h               |   9 +
>>>   3 files changed, 359 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_format_helper.c 
>>> b/drivers/gpu/drm/drm_format_helper.c
>>> index e562a3cefb89..aa6138756458 100644
>>> --- a/drivers/gpu/drm/drm_format_helper.c
>>> +++ b/drivers/gpu/drm/drm_format_helper.c
>>> @@ -395,6 +395,161 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map 
>>> *dst, const unsigned int *dst_pi
>>>   }
>>>   EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
>>> +static void drm_fb_xrgb8888_to_xrgb1555_line(void *dbuf, const void 
>>> *sbuf, unsigned int pixels)
>>> +{
>>> +    u16 *dbuf16 = dbuf;
>>> +    const __le32 *sbuf32 = sbuf;
>>> +    unsigned int x;
>>> +    u16 val16;
>>> +    u32 pix;
>>> +
>>> +    for (x = 0; x < pixels; x++) {
>>> +        pix = le32_to_cpu(sbuf32[x]);
>>> +        val16 = ((pix & 0x00f80000) >> 9) |
>>> +            ((pix & 0x0000f800) >> 6) |
>>> +            ((pix & 0x000000f8) >> 3);
>>> +        dbuf16[x] = cpu_to_le16(val16);
>>
>> You don't need the extra cpu_to_le16() here:
>>
>>   - dbuf16[x] = cpu_to_le16(val16);
>>   + dbuf16[x] = val16;
> 
> Thanks for testing this. The DRM formats are defined to be in little 
> endian (unless the DRM_FORMAT_BIG_ENDIAN flag has been set). So we have 
> to convert the result from host to LE endianess.
> 
> I'm pretty sure that the hardcoded output in the testcases needs to be 
> adapted instead. But did this never happen with the older tests?

I ran the tests on big-endian ppc64 and found that some of the other 
tests include a call to le32buf_to_cpu() to convert the result to local 
endianess. I'll send out an update.

I also noted that RGB565 conversion does not do the final to-LE 
conversion. That's something for another patchset.

Best regards
Thomas

> 
> Best regards
> Thomas
> 
>>
>>> +    }
>>> +}
>>> +
>>> +/**
>>> + * drm_fb_xrgb8888_to_xrgb1555 - Convert XRGB8888 to XRGB1555 clip 
>>> buffer
>>> + * @dst: Array of XRGB1555 destination buffers
>>> + * @dst_pitch: Array of numbers of bytes between the start of two 
>>> consecutive scanlines
>>> + *             within @dst; can be NULL if scanlines are stored next 
>>> to each other.
>>> + * @src: Array of XRGB8888 source buffer
>>> + * @fb: DRM framebuffer
>>> + * @clip: Clip rectangle area to copy
>>> + *
>>> + * This function copies parts of a framebuffer to display memory and 
>>> converts
>>> + * the color format during the process. The parameters @dst, 
>>> @dst_pitch and
>>> + * @src refer to arrays. Each array must have at least as many 
>>> entries as
>>> + * there are planes in @fb's format. Each entry stores the value for 
>>> the
>>> + * format's respective color plane at the same index.
>>> + *
>>> + * This function does not apply clipping on @dst (i.e. the 
>>> destination is at the
>>> + * top-left corner).
>>> + *
>>> + * Drivers can use this function for XRGB1555 devices that don't 
>>> support
>>> + * XRGB8888 natively.
>>> + */
>>> +void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch,
>>> +                 const struct iosys_map *src, const struct 
>>> drm_framebuffer *fb,
>>> +                 const struct drm_rect *clip)
>>> +{
>>> +    static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
>>> +        2,
>>> +    };
>>> +
>>> +    drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
>>> +            drm_fb_xrgb8888_to_xrgb1555_line);
>>> +}
>>> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb1555);
>>> +
>>> +static void drm_fb_xrgb8888_to_argb1555_line(void *dbuf, const void 
>>> *sbuf, unsigned int pixels)
>>> +{
>>> +    u16 *dbuf16 = dbuf;
>>> +    const __le32 *sbuf32 = sbuf;
>>> +    unsigned int x;
>>> +    u16 val16;
>>> +    u32 pix;
>>> +
>>> +    for (x = 0; x < pixels; x++) {
>>> +        pix = le32_to_cpu(sbuf32[x]);
>>> +        val16 = BIT(15) | /* set alpha bit */
>>> +            ((pix & 0x00f80000) >> 9) |
>>> +            ((pix & 0x0000f800) >> 6) |
>>> +            ((pix & 0x000000f8) >> 3);
>>> +        dbuf16[x] = cpu_to_le16(val16);
>>
>> The same here:
>>
>>   - dbuf16[x] = cpu_to_le16(val16);
>>   + dbuf16[x] = val16;
>>
>>> +    }
>>> +}
>>> +
>>> +/**
>>> + * drm_fb_xrgb8888_to_argb1555 - Convert XRGB8888 to ARGB1555 clip 
>>> buffer
>>> + * @dst: Array of ARGB1555 destination buffers
>>> + * @dst_pitch: Array of numbers of bytes between the start of two 
>>> consecutive scanlines
>>> + *             within @dst; can be NULL if scanlines are stored next 
>>> to each other.
>>> + * @src: Array of XRGB8888 source buffer
>>> + * @fb: DRM framebuffer
>>> + * @clip: Clip rectangle area to copy
>>> + *
>>> + * This function copies parts of a framebuffer to display memory and 
>>> converts
>>> + * the color format during the process. The parameters @dst, 
>>> @dst_pitch and
>>> + * @src refer to arrays. Each array must have at least as many 
>>> entries as
>>> + * there are planes in @fb's format. Each entry stores the value for 
>>> the
>>> + * format's respective color plane at the same index.
>>> + *
>>> + * This function does not apply clipping on @dst (i.e. the 
>>> destination is at the
>>> + * top-left corner).
>>> + *
>>> + * Drivers can use this function for ARGB1555 devices that don't 
>>> support
>>> + * XRGB8888 natively. It sets an opaque alpha channel as part of the 
>>> conversion.
>>> + */
>>> +void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch,
>>> +                 const struct iosys_map *src, const struct 
>>> drm_framebuffer *fb,
>>> +                 const struct drm_rect *clip)
>>> +{
>>> +    static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
>>> +        2,
>>> +    };
>>> +
>>> +    drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
>>> +            drm_fb_xrgb8888_to_argb1555_line);
>>> +}
>>> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb1555);
>>> +
>>> +static void drm_fb_xrgb8888_to_rgba5551_line(void *dbuf, const void 
>>> *sbuf, unsigned int pixels)
>>> +{
>>> +    u16 *dbuf16 = dbuf;
>>> +    const __le32 *sbuf32 = sbuf;
>>> +    unsigned int x;
>>> +    u16 val16;
>>> +    u32 pix;
>>> +
>>> +    for (x = 0; x < pixels; x++) {
>>> +        pix = le32_to_cpu(sbuf32[x]);
>>> +        val16 = ((pix & 0x00f80000) >> 8) |
>>> +            ((pix & 0x0000f800) >> 5) |
>>> +            ((pix & 0x000000f8) >> 2) |
>>> +            BIT(0); /* set alpha bit */
>>> +        dbuf16[x] = cpu_to_le16(val16);
>>
>> And this is the last fix:
>>
>>   - dbuf16[x] = cpu_to_le16(val16);
>>   + dbuf16[x] = val16;
>>
>>
>> Other than that, this patch looks good:
>>
>> Reviewed-by: José Expósito <jose.exposito89@gmail.com>
>>
>> Best wishes,
>> Jose
>>
>>> +    }
>>> +}
>>> +
>>> +/**
>>> + * drm_fb_xrgb8888_to_rgba5551 - Convert XRGB8888 to RGBA5551 clip 
>>> buffer
>>> + * @dst: Array of RGBA5551 destination buffers
>>> + * @dst_pitch: Array of numbers of bytes between the start of two 
>>> consecutive scanlines
>>> + *             within @dst; can be NULL if scanlines are stored next 
>>> to each other.
>>> + * @src: Array of XRGB8888 source buffer
>>> + * @fb: DRM framebuffer
>>> + * @clip: Clip rectangle area to copy
>>> + *
>>> + * This function copies parts of a framebuffer to display memory and 
>>> converts
>>> + * the color format during the process. The parameters @dst, 
>>> @dst_pitch and
>>> + * @src refer to arrays. Each array must have at least as many 
>>> entries as
>>> + * there are planes in @fb's format. Each entry stores the value for 
>>> the
>>> + * format's respective color plane at the same index.
>>> + *
>>> + * This function does not apply clipping on @dst (i.e. the 
>>> destination is at the
>>> + * top-left corner).
>>> + *
>>> + * Drivers can use this function for RGBA5551 devices that don't 
>>> support
>>> + * XRGB8888 natively. It sets an opaque alpha channel as part of the 
>>> conversion.
>>> + */
>>> +void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch,
>>> +                 const struct iosys_map *src, const struct 
>>> drm_framebuffer *fb,
>>> +                 const struct drm_rect *clip)
>>> +{
>>> +    static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
>>> +        2,
>>> +    };
>>> +
>>> +    drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
>>> +            drm_fb_xrgb8888_to_rgba5551_line);
>>> +}
>>> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgba5551);
>>> +
>>>   static void drm_fb_xrgb8888_to_rgb888_line(void *dbuf, const void 
>>> *sbuf, unsigned int pixels)
>>>   {
>>>       u8 *dbuf8 = dbuf;
>>> @@ -761,6 +916,15 @@ int drm_fb_blit(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch, uint32_t d
>>>           if (dst_format == DRM_FORMAT_RGB565) {
>>>               drm_fb_xrgb8888_to_rgb565(dst, dst_pitch, src, fb, 
>>> clip, false);
>>>               return 0;
>>> +        } else if (dst_format == DRM_FORMAT_XRGB1555) {
>>> +            drm_fb_xrgb8888_to_xrgb1555(dst, dst_pitch, src, fb, clip);
>>> +            return 0;
>>> +        } else if (dst_format == DRM_FORMAT_ARGB1555) {
>>> +            drm_fb_xrgb8888_to_argb1555(dst, dst_pitch, src, fb, clip);
>>> +            return 0;
>>> +        } else if (dst_format == DRM_FORMAT_RGBA5551) {
>>> +            drm_fb_xrgb8888_to_rgba5551(dst, dst_pitch, src, fb, clip);
>>> +            return 0;
>>>           } else if (dst_format == DRM_FORMAT_RGB888) {
>>>               drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip);
>>>               return 0;
>>> diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c 
>>> b/drivers/gpu/drm/tests/drm_format_helper_test.c
>>> index b15dc7c1eb96..e01695c70bb6 100644
>>> --- a/drivers/gpu/drm/tests/drm_format_helper_test.c
>>> +++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
>>> @@ -32,6 +32,21 @@ struct convert_to_rgb565_result {
>>>       const u16 expected_swab[TEST_BUF_SIZE];
>>>   };
>>> +struct convert_to_xrgb1555_result {
>>> +    unsigned int dst_pitch;
>>> +    const u16 expected[TEST_BUF_SIZE];
>>> +};
>>> +
>>> +struct convert_to_argb1555_result {
>>> +    unsigned int dst_pitch;
>>> +    const u16 expected[TEST_BUF_SIZE];
>>> +};
>>> +
>>> +struct convert_to_rgba5551_result {
>>> +    unsigned int dst_pitch;
>>> +    const u16 expected[TEST_BUF_SIZE];
>>> +};
>>> +
>>>   struct convert_to_rgb888_result {
>>>       unsigned int dst_pitch;
>>>       const u8 expected[TEST_BUF_SIZE];
>>> @@ -60,6 +75,9 @@ struct convert_xrgb8888_case {
>>>       struct convert_to_gray8_result gray8_result;
>>>       struct convert_to_rgb332_result rgb332_result;
>>>       struct convert_to_rgb565_result rgb565_result;
>>> +    struct convert_to_xrgb1555_result xrgb1555_result;
>>> +    struct convert_to_argb1555_result argb1555_result;
>>> +    struct convert_to_rgba5551_result rgba5551_result;
>>>       struct convert_to_rgb888_result rgb888_result;
>>>       struct convert_to_argb8888_result argb8888_result;
>>>       struct convert_to_xrgb2101010_result xrgb2101010_result;
>>> @@ -85,6 +103,18 @@ static struct convert_xrgb8888_case 
>>> convert_xrgb8888_cases[] = {
>>>               .expected = { 0xF800 },
>>>               .expected_swab = { 0x00F8 },
>>>           },
>>> +        .xrgb1555_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = { 0x7C00 },
>>> +        },
>>> +        .argb1555_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = { 0xFC00 },
>>> +        },
>>> +        .rgba5551_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = { 0xF801 },
>>> +        },
>>>           .rgb888_result = {
>>>               .dst_pitch = 0,
>>>               .expected = { 0x00, 0x00, 0xFF },
>>> @@ -123,6 +153,18 @@ static struct convert_xrgb8888_case 
>>> convert_xrgb8888_cases[] = {
>>>               .expected = { 0xF800 },
>>>               .expected_swab = { 0x00F8 },
>>>           },
>>> +        .xrgb1555_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = { 0x7C00 },
>>> +        },
>>> +        .argb1555_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = { 0xFC00 },
>>> +        },
>>> +        .rgba5551_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = { 0xF801 },
>>> +        },
>>>           .rgb888_result = {
>>>               .dst_pitch = 0,
>>>               .expected = { 0x00, 0x00, 0xFF },
>>> @@ -188,6 +230,33 @@ static struct convert_xrgb8888_case 
>>> convert_xrgb8888_cases[] = {
>>>                   0xE0FF, 0xFF07,
>>>               },
>>>           },
>>> +        .xrgb1555_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = {
>>> +                0x7FFF, 0x0000,
>>> +                0x7C00, 0x03E0,
>>> +                0x001F, 0x7C1F,
>>> +                0x7FE0, 0x03FF,
>>> +            },
>>> +        },
>>> +        .argb1555_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = {
>>> +                0xFFFF, 0x8000,
>>> +                0xFC00, 0x83E0,
>>> +                0x801F, 0xFC1F,
>>> +                0xFFE0, 0x83FF,
>>> +            },
>>> +        },
>>> +        .rgba5551_result = {
>>> +            .dst_pitch = 0,
>>> +            .expected = {
>>> +                0xFFFF, 0x0001,
>>> +                0xF801, 0x07C1,
>>> +                0x003F, 0xF83F,
>>> +                0xFFC1, 0x07FF,
>>> +            },
>>> +        },
>>>           .rgb888_result = {
>>>               .dst_pitch = 0,
>>>               .expected = {
>>> @@ -264,6 +333,30 @@ static struct convert_xrgb8888_case 
>>> convert_xrgb8888_cases[] = {
>>>                   0x00A8, 0x8E6B, 0x330A, 0x0000, 0x0000,
>>>               },
>>>           },
>>> +        .xrgb1555_result = {
>>> +            .dst_pitch = 10,
>>> +            .expected = {
>>> +                0x0513, 0x0920, 0x5400, 0x0000, 0x0000,
>>> +                0x35CE, 0x0513, 0x0920, 0x0000, 0x0000,
>>> +                0x5400, 0x35CE, 0x0513, 0x0000, 0x0000,
>>> +            },
>>> +        },
>>> +        .argb1555_result = {
>>> +            .dst_pitch = 10,
>>> +            .expected = {
>>> +                0x8513, 0x8920, 0xD400, 0x0000, 0x0000,
>>> +                0xB5CE, 0x8513, 0x8920, 0x0000, 0x0000,
>>> +                0xD400, 0xB5CE, 0x8513, 0x0000, 0x0000,
>>> +            },
>>> +        },
>>> +        .rgba5551_result = {
>>> +            .dst_pitch = 10,
>>> +            .expected = {
>>> +                0x0A27, 0x1241, 0xA801, 0x0000, 0x0000,
>>> +                0x6B9D, 0x0A27, 0x1241, 0x0000, 0x0000,
>>> +                0xA801, 0x6B9D, 0x0A27, 0x0000, 0x0000,
>>> +            },
>>> +        },
>>>           .rgb888_result = {
>>>               .dst_pitch = 15,
>>>               .expected = {
>>> @@ -443,6 +536,96 @@ static void 
>>> drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
>>>       KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected_swab, 
>>> dst_size), 0);
>>>   }
>>> +static void drm_test_fb_xrgb8888_to_xrgb1555(struct kunit *test)
>>> +{
>>> +    const struct convert_xrgb8888_case *params = test->param_value;
>>> +    const struct convert_to_xrgb1555_result *result = 
>>> &params->xrgb1555_result;
>>> +    size_t dst_size;
>>> +    __u16 *buf = NULL;
>>> +    __u32 *xrgb8888 = NULL;
>>> +    struct iosys_map dst, src;
>>> +
>>> +    struct drm_framebuffer fb = {
>>> +        .format = drm_format_info(DRM_FORMAT_XRGB8888),
>>> +        .pitches = { params->pitch, 0, 0 },
>>> +    };
>>> +
>>> +    dst_size = conversion_buf_size(DRM_FORMAT_XRGB1555, 
>>> result->dst_pitch,
>>> +                       &params->clip);
>>> +    KUNIT_ASSERT_GT(test, dst_size, 0);
>>> +
>>> +    buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
>>> +    KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
>>> +    iosys_map_set_vaddr(&dst, buf);
>>> +
>>> +    xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
>>> +    KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
>>> +    iosys_map_set_vaddr(&src, xrgb8888);
>>> +
>>> +    drm_fb_xrgb8888_to_xrgb1555(&dst, &result->dst_pitch, &src, &fb, 
>>> &params->clip);
>>> +    KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
>>> +}
>>> +
>>> +static void drm_test_fb_xrgb8888_to_argb1555(struct kunit *test)
>>> +{
>>> +    const struct convert_xrgb8888_case *params = test->param_value;
>>> +    const struct convert_to_argb1555_result *result = 
>>> &params->argb1555_result;
>>> +    size_t dst_size;
>>> +    __u16 *buf = NULL;
>>> +    __u32 *xrgb8888 = NULL;
>>> +    struct iosys_map dst, src;
>>> +
>>> +    struct drm_framebuffer fb = {
>>> +        .format = drm_format_info(DRM_FORMAT_XRGB8888),
>>> +        .pitches = { params->pitch, 0, 0 },
>>> +    };
>>> +
>>> +    dst_size = conversion_buf_size(DRM_FORMAT_ARGB1555, 
>>> result->dst_pitch,
>>> +                       &params->clip);
>>> +    KUNIT_ASSERT_GT(test, dst_size, 0);
>>> +
>>> +    buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
>>> +    KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
>>> +    iosys_map_set_vaddr(&dst, buf);
>>> +
>>> +    xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
>>> +    KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
>>> +    iosys_map_set_vaddr(&src, xrgb8888);
>>> +
>>> +    drm_fb_xrgb8888_to_argb1555(&dst, &result->dst_pitch, &src, &fb, 
>>> &params->clip);
>>> +    KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
>>> +}
>>> +
>>> +static void drm_test_fb_xrgb8888_to_rgba5551(struct kunit *test)
>>> +{
>>> +    const struct convert_xrgb8888_case *params = test->param_value;
>>> +    const struct convert_to_rgba5551_result *result = 
>>> &params->rgba5551_result;
>>> +    size_t dst_size;
>>> +    __u16 *buf = NULL;
>>> +    __u32 *xrgb8888 = NULL;
>>> +    struct iosys_map dst, src;
>>> +
>>> +    struct drm_framebuffer fb = {
>>> +        .format = drm_format_info(DRM_FORMAT_XRGB8888),
>>> +        .pitches = { params->pitch, 0, 0 },
>>> +    };
>>> +
>>> +    dst_size = conversion_buf_size(DRM_FORMAT_RGBA5551, 
>>> result->dst_pitch,
>>> +                       &params->clip);
>>> +    KUNIT_ASSERT_GT(test, dst_size, 0);
>>> +
>>> +    buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
>>> +    KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
>>> +    iosys_map_set_vaddr(&dst, buf);
>>> +
>>> +    xrgb8888 = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
>>> +    KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
>>> +    iosys_map_set_vaddr(&src, xrgb8888);
>>> +
>>> +    drm_fb_xrgb8888_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, 
>>> &params->clip);
>>> +    KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
>>> +}
>>> +
>>>   static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
>>>   {
>>>       const struct convert_xrgb8888_case *params = test->param_value;
>>> @@ -570,6 +753,9 @@ static struct kunit_case 
>>> drm_format_helper_test_cases[] = {
>>>       KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_gray8, 
>>> convert_xrgb8888_gen_params),
>>>       KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb332, 
>>> convert_xrgb8888_gen_params),
>>>       KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb565, 
>>> convert_xrgb8888_gen_params),
>>> +    KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb1555, 
>>> convert_xrgb8888_gen_params),
>>> +    KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb1555, 
>>> convert_xrgb8888_gen_params),
>>> +    KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgba5551, 
>>> convert_xrgb8888_gen_params),
>>>       KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb888, 
>>> convert_xrgb8888_gen_params),
>>>       KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb8888, 
>>> convert_xrgb8888_gen_params),
>>>       KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb2101010, 
>>> convert_xrgb8888_gen_params),
>>> diff --git a/include/drm/drm_format_helper.h 
>>> b/include/drm/drm_format_helper.h
>>> index 10b2d19cdb66..2d04f5863722 100644
>>> --- a/include/drm/drm_format_helper.h
>>> +++ b/include/drm/drm_format_helper.h
>>> @@ -30,6 +30,15 @@ void drm_fb_xrgb8888_to_rgb332(struct iosys_map 
>>> *dst, const unsigned int *dst_pi
>>>   void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch,
>>>                      const struct iosys_map *src, const struct 
>>> drm_framebuffer *fb,
>>>                      const struct drm_rect *clip, bool swab);
>>> +void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch,
>>> +                 const struct iosys_map *src, const struct 
>>> drm_framebuffer *fb,
>>> +                 const struct drm_rect *clip);
>>> +void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch,
>>> +                 const struct iosys_map *src, const struct 
>>> drm_framebuffer *fb,
>>> +                 const struct drm_rect *clip);
>>> +void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch,
>>> +                 const struct iosys_map *src, const struct 
>>> drm_framebuffer *fb,
>>> +                 const struct drm_rect *clip);
>>>   void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const 
>>> unsigned int *dst_pitch,
>>>                      const struct iosys_map *src, const struct 
>>> drm_framebuffer *fb,
>>>                      const struct drm_rect *clip);
>>> -- 
>>> 2.38.1
>>>
> 

-- 
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 1/9] firmware/sysfb: Fix EFI/VESA format selection

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Select color format for EFI/VESA firmware scanout buffer from the
> number of bits per pixel and the position of the individual color
> components. Fixes the selected format for the buffer in several odd
> cases. For example, XRGB1555 has been reported as ARGB1555 because
> of the different use of depth and transparency in VESA and Linux.
> 
> Bits-per-pixel is always the pixel's raw number of bits; including
> alpha and filler bits. It is preferred over color depth, which has a
> different meaning among various components and standards.
> 
> Also do not compare reserved bits and transparency bits to each other.
> These values have different meanings, as reserved bits include filler
> bits while transparency does not.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/firmware/sysfb_simplefb.c | 43 ++++++++++++++++++++++++++-----
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 

[...]

> +	/*
> +	 * The meaning of depth and bpp for direct-color formats is
> +	 * inconsistent:
> +	 *
> +	 *  - DRM format info specifies depth as the number of color
> +	 *    bits; including alpha, but not including filler bits.
> +	 *  - Linux' EFI platform code computes lfb_depth from the
> +	 *    individual color channels, including the reserved bits.
> +	 *  - VBE 1.1 defines lfb_depth for XRGB1555 as 16, but later
> +	 *    versions use 15.
> +	 *  - On the kernel command line, 'bpp' of 32 is usually
> +	 *    XRGB8888 including the filler bits, but 15 is XRGB1555
> +	 *    not including the filler bit.
> +	 *
> +	 * It's not easily possible to fix this in struct screen_info,
> +	 * as this could break UAPI. The best solution is to compute
> +	 * bits_per_pixel here and ignore lfb_depth. In the loop below,
> +	 * ignore simplefb formats with alpha bits, as EFI and VESA
> +	 * don't specify alpha channels.
> +	 */

Thanks a lot for adding this comment. It's very insightful.

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/9] drm/format-helper: Flip src/dst-format branches in blit helper

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Upcoming changes to the format conversion will mostly blit from
> XRGB8888 to some other format. So put the source format in blit's
> outer branches to make the code more readable. For cases where
> a format only changes its endianness, such as XRGB565, introduce
> dedicated branches that handle this for all formats.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 3/9] drm/format-helper: Add conversion from XRGB8888 to ARGB8888

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Add dedicated helper to convert from XRGB8888 to ARGB8888. Sets
> all alpha bits to make pixels fully opaque.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 4/9] drm/format-helper: Add conversion from XRGB8888 to ARGB2101010

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Add dedicated helper to convert from XRGB8888 to ARGB2101010. Sets
> all alpha bits to make pixels fully opaque.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

[...]

> +static void drm_test_fb_xrgb8888_to_argb2101010(struct kunit *test)
> +{
> +	const struct convert_xrgb8888_case *params = test->param_value;
> +	const struct convert_to_argb2101010_result *result = &params->argb2101010_result;
> +	size_t dst_size;
> +	__u32 *buf = NULL;
> +	__u32 *xrgb8888 = NULL;
> +	struct iosys_map dst, src;
> +
> +	struct drm_framebuffer fb = {
> +		.format = drm_format_info(DRM_FORMAT_ARGB8888),

Shouldn't this be DRM_FORMAT_XRGB8888 instead?

Other than that, the patch looks good to me.

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 5/9] drm/format-helper: Add conversion from XRGB8888 to 15-bit RGB555 formats

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Add conversion from XRGB8888 to XRGB1555, ARGB1555 and RGBA5551, which
> are the formats currently supported by the simplefb infrastructure. The
> new helpers allow the output of XRGB8888 framebuffers to firmware
> scanout buffers in one of the 15-bit formats.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

After addressing what was discussed with José,

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 6/9] drm/fh-helper: Split fbdev single-probe helper

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Split the single-probe helper's implementation into multiple
> functions and get locking and overallocation out of the way of
> the surface setup. Simplifies later changes to the setup code.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 7/9] drm/fb-helper: Fix single-probe color-format selection

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Fix the color-format selection of the single-probe helper. Go
> through all user-specified values and test each for compatibility
> with the driver. If none is supported, use the driver-provided
> default. This guarantees that the console is always available in
> any color format at least.
> 
> Until now, the format selection of the single-probe helper tried
> to either use a user-specified format or a 32-bit default format.
> If the user-specified format was not supported by the driver, the
> selection failed and the display remained blank.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Thanks a lot for fixing this.

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 8/9] drm/format-helper: Simplify drm_fb_build_fourcc_list()

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> The DRM helper drm_fb_build_fourcc_list() creates a list of color
> formats for primary planes of the generic drivers. Simplify the helper:
> 
>  - It used to mix and filter native and emulated formats as provided
>    by the driver. Now the only emulated format is XRGB8888, which is
>    required as fallback by legacy software. Drop support for emulating
>    any other formats.
>  - Also convert alpha formats to their non-alpha counterparts. Generic
>    drivers don't support primary planes with alpha formats and some
>    DTs incorrectly advertise alpha channels for non-alpha hardware. So
>    only export non-alpha formats for primary planes.
> 
> With the simplified helper, scrap format lists of the affected generic
> drivers. All they need is the firmware buffer's native format, from which
> the helper creates the list of color formats.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 9/9] drm/format-helper: Remove unnecessary conversion helpers

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Drivers only emulate XRGB8888 framebuffers. Remove all conversion
> helpers that do not use XRGB8888 as their source format. Also remove
> some special cases for alpha formats in the blit helper.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat