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 canrun 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_testThe problem is in the drm_fb_xrgb8888_to_*_line() functions, see the fixesinlined: 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@xxxxxxx> --- 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.cindex 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@xxxxxxxxx> 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 dif (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.cindex 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 = ¶ms->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,+ ¶ms->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, ¶ms->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 = ¶ms->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,+ ¶ms->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, ¶ms->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 = ¶ms->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,+ ¶ms->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, ¶ms->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.hindex 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:
OpenPGP_signature
Description: OpenPGP digital signature
