[PATCH 0/4] KUnit tests for RGB565 conversion

8 views
Skip to first unread message

José Expósito

unread,
Jun 27, 2022, 12:13:06 PMJun 27
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
Hello everyone,

This series is a follow up of the XRGB8888 to RGB332 conversion KUnit tests.

The first 3 patches refactor the existing test to make them agnostic of the target format and add support for "swab".

The last patch adds the RGB565 conversion values, and shows how more formats will be easily added in the future.

Thank you very much in advance for your feedback,
José Expósito

José Expósito (4):
drm/format-helper: Rename test cases to make them more generic
drm/format-helper: Transform tests to be agnostic of target format
drm/format-helper: Add support for conversion functions with swab
drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()

.../gpu/drm/tests/drm_format_helper_test.c | 231 +++++++++++++++---
1 file changed, 196 insertions(+), 35 deletions(-)


base-commit: 6fde8eec71796f3534f0c274066862829813b21f
prerequisite-patch-id: 8a16f4c8004d6161035eaea275c8eafaa0ac927e
prerequisite-patch-id: 53fded2a49e6212b546db76ec52563a683752e65
prerequisite-patch-id: 294b0ca27a6ee57096c8f097c0572336b8a2d583
prerequisite-patch-id: 5e05bfc5287d16c207bfc616b2776ad72eb4ab29
prerequisite-patch-id: e94560be85dffb62a5b3cf58d1f0fc3d278ad806
prerequisite-patch-id: a471df39c7b32c69dd2b138a7d0af015ea42e00a
--
2.25.1

José Expósito

unread,
Jun 27, 2022, 12:13:08 PMJun 27
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
The tests available at the moment only check the conversion from
XRGB8888 to RGB332. However, more conversion will be tested in the
future.

In order to make the struct and functions present in the tests more
generic, rename xrgb8888_to_rgb332_* to convert_xrgb8888_*.

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
.../gpu/drm/tests/drm_format_helper_test.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 98583bf56044..de8cf525109e 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -16,7 +16,7 @@

#define TEST_BUF_SIZE 50

-struct xrgb8888_to_rgb332_case {
+struct convert_xrgb8888_case {
const char *name;
unsigned int pitch;
unsigned int dst_pitch;
@@ -25,7 +25,7 @@ struct xrgb8888_to_rgb332_case {
const u8 expected[4 * TEST_BUF_SIZE];
};

-static struct xrgb8888_to_rgb332_case xrgb8888_to_rgb332_cases[] = {
+static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
{
.name = "single_pixel_source_buffer",
.pitch = 1 * 4,
@@ -111,18 +111,18 @@ static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
return dst_pitch * drm_rect_height(clip);
}

-static void xrgb8888_to_rgb332_case_desc(struct xrgb8888_to_rgb332_case *t,
- char *desc)
+static void convert_xrgb8888_case_desc(struct convert_xrgb8888_case *t,
+ char *desc)
{
strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
}

-KUNIT_ARRAY_PARAM(xrgb8888_to_rgb332, xrgb8888_to_rgb332_cases,
- xrgb8888_to_rgb332_case_desc);
+KUNIT_ARRAY_PARAM(convert_xrgb8888, convert_xrgb8888_cases,
+ convert_xrgb8888_case_desc);

-static void xrgb8888_to_rgb332_test(struct kunit *test)
+static void convert_xrgb8888_test(struct kunit *test)
{
- const struct xrgb8888_to_rgb332_case *params = test->param_value;
+ const struct convert_xrgb8888_case *params = test->param_value;
size_t dst_size;
__u8 *dst = NULL;

@@ -144,8 +144,7 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)
}

static struct kunit_case drm_format_helper_test_cases[] = {
- KUNIT_CASE_PARAM(xrgb8888_to_rgb332_test,
- xrgb8888_to_rgb332_gen_params),
+ KUNIT_CASE_PARAM(convert_xrgb8888_test, convert_xrgb8888_gen_params),
{}
};

--
2.25.1

José Expósito

unread,
Jun 27, 2022, 12:13:09 PMJun 27
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
The RGB565 conversion functions take an extra parameter ("swab")
indicating whether the bytes should be swapped into the clip buffer or
not.

Create a union in the "convert_xrgb8888_result" structure holding the
value of the "swab" parameter as well as the conversion function
pointer.

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
.../gpu/drm/tests/drm_format_helper_test.c | 44 ++++++++++++++-----
1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 732d945e7f4e..52dc41cc7c60 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -16,12 +16,29 @@

#define TEST_BUF_SIZE 50

+struct convert_xrgb8888_func {
+ void (*func)(void *dst, unsigned int dst_pitch,
+ const void *src,
+ const struct drm_framebuffer *fb,
+ const struct drm_rect *clip);
+};
+
+struct convert_xrgb8888_func_swab {
+ void (*func)(void *dst, unsigned int dst_pitch,
+ const void *src,
+ const struct drm_framebuffer *fb,
+ const struct drm_rect *clip,
+ bool swab);
+ bool swab;
+};
+
struct convert_xrgb8888_result {
u32 dst_format;
- void (*conv_func)(void *dst, unsigned int dst_pitch,
- const void *src,
- const struct drm_framebuffer *fb,
- const struct drm_rect *clip);
+ bool has_swab;
+ union {
+ struct convert_xrgb8888_func conv;
+ struct convert_xrgb8888_func_swab conv_swab;
+ };
unsigned int dst_pitch;
const u8 expected[4 * TEST_BUF_SIZE];
};
@@ -43,7 +60,7 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.results = {
{
.dst_format = DRM_FORMAT_RGB332,
- .conv_func = drm_fb_xrgb8888_to_rgb332,
+ .conv = { .func = drm_fb_xrgb8888_to_rgb332 },
.dst_pitch = 0,
.expected = { 0xE0 },
},
@@ -60,7 +77,7 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.results = {
{
.dst_format = DRM_FORMAT_RGB332,
- .conv_func = drm_fb_xrgb8888_to_rgb332,
+ .conv = { .func = drm_fb_xrgb8888_to_rgb332 },
.dst_pitch = 0,
.expected = { 0xE0 },
},
@@ -84,7 +101,7 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.results = {
{
.dst_format = DRM_FORMAT_RGB332,
- .conv_func = drm_fb_xrgb8888_to_rgb332,
+ .conv = { .func = drm_fb_xrgb8888_to_rgb332 },
.dst_pitch = 0,
.expected = {
0xFF, 0x00,
@@ -108,7 +125,7 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.results = {
{
.dst_format = DRM_FORMAT_RGB332,
- .conv_func = drm_fb_xrgb8888_to_rgb332,
+ .conv = { .func = drm_fb_xrgb8888_to_rgb332 },
.dst_pitch = 5,
.expected = {
0x0A, 0x08, 0xA0, 0x00, 0x00,
@@ -177,8 +194,15 @@ static void convert_xrgb8888_test(struct kunit *test)
dst = kunit_kzalloc(test, dst_size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dst);

- result->conv_func(dst, result->dst_pitch, params->xrgb8888,
- &fb, &params->clip);
+ if (result->has_swab) {
+ result->conv_swab.func(dst, result->dst_pitch,
+ params->xrgb8888, &fb,
+ &params->clip,
+ result->conv_swab.swab);
+ } else {
+ result->conv.func(dst, result->dst_pitch,
+ params->xrgb8888, &fb, &params->clip);
+ }
KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected, dst_size), 0);
}
}
--
2.25.1

José Expósito

unread,
Jun 27, 2022, 12:13:09 PMJun 27
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
In order to support multiple destination format conversions, store the
target format, conversion function, parameters and expected result in
its own structure.

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
.../gpu/drm/tests/drm_format_helper_test.c | 88 ++++++++++++++-----
1 file changed, 64 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index de8cf525109e..732d945e7f4e 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -16,34 +16,55 @@

#define TEST_BUF_SIZE 50

+struct convert_xrgb8888_result {
+ u32 dst_format;
+ void (*conv_func)(void *dst, unsigned int dst_pitch,
+ const void *src,
+ const struct drm_framebuffer *fb,
+ const struct drm_rect *clip);
+ unsigned int dst_pitch;
+ const u8 expected[4 * TEST_BUF_SIZE];
+};
+
struct convert_xrgb8888_case {
const char *name;
unsigned int pitch;
- unsigned int dst_pitch;
struct drm_rect clip;
const u32 xrgb8888[TEST_BUF_SIZE];
- const u8 expected[4 * TEST_BUF_SIZE];
+ struct convert_xrgb8888_result results[1];
};

static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
{
.name = "single_pixel_source_buffer",
.pitch = 1 * 4,
- .dst_pitch = 0,
.clip = DRM_RECT_INIT(0, 0, 1, 1),
.xrgb8888 = { 0x01FF0000 },
- .expected = { 0xE0 },
+ .results = {
+ {
+ .dst_format = DRM_FORMAT_RGB332,
+ .conv_func = drm_fb_xrgb8888_to_rgb332,
+ .dst_pitch = 0,
+ .expected = { 0xE0 },
+ },
+ },
},
{
.name = "single_pixel_clip_rectangle",
.pitch = 2 * 4,
- .dst_pitch = 0,
.clip = DRM_RECT_INIT(1, 1, 1, 1),
.xrgb8888 = {
0x00000000, 0x00000000,
0x00000000, 0x10FF0000,
},
- .expected = { 0xE0 },
+ .results = {
+ {
+ .dst_format = DRM_FORMAT_RGB332,
+ .conv_func = drm_fb_xrgb8888_to_rgb332,
+ .dst_pitch = 0,
+ .expected = { 0xE0 },
+ },
+ },
},
{
/* Well known colors: White, black, red, green, blue, magenta,
@@ -52,7 +73,6 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
*/
.name = "well_known_colors",
.pitch = 4 * 4,
- .dst_pitch = 0,
.clip = DRM_RECT_INIT(1, 1, 2, 4),
.xrgb8888 = {
0x00000000, 0x00000000, 0x00000000, 0x00000000,
@@ -61,28 +81,41 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
0x00000000, 0x550000FF, 0x66FF00FF, 0x00000000,
0x00000000, 0x77FFFF00, 0x8800FFFF, 0x00000000,
},
- .expected = {
- 0xFF, 0x00,
- 0xE0, 0x1C,
- 0x03, 0xE3,
- 0xFC, 0x1F,
+ .results = {
+ {
+ .dst_format = DRM_FORMAT_RGB332,
+ .conv_func = drm_fb_xrgb8888_to_rgb332,
+ .dst_pitch = 0,
+ .expected = {
+ 0xFF, 0x00,
+ 0xE0, 0x1C,
+ 0x03, 0xE3,
+ 0xFC, 0x1F,
+ },
+ },
},
},
{
/* Randomly picked colors. Full buffer within the clip area. */
.name = "destination_pitch",
.pitch = 3 * 4,
- .dst_pitch = 5,
.clip = DRM_RECT_INIT(0, 0, 3, 3),
.xrgb8888 = {
0xA10E449C, 0xB1114D05, 0xC1A80303,
0xD16C7073, 0xA20E449C, 0xB2114D05,
0xC2A80303, 0xD26C7073, 0xA30E449C,
},
- .expected = {
- 0x0A, 0x08, 0xA0, 0x00, 0x00,
- 0x6D, 0x0A, 0x08, 0x00, 0x00,
- 0xA0, 0x6D, 0x0A, 0x00, 0x00,
+ .results = {
+ {
+ .dst_format = DRM_FORMAT_RGB332,
+ .conv_func = drm_fb_xrgb8888_to_rgb332,
+ .dst_pitch = 5,
+ .expected = {
+ 0x0A, 0x08, 0xA0, 0x00, 0x00,
+ 0x6D, 0x0A, 0x08, 0x00, 0x00,
+ 0xA0, 0x6D, 0x0A, 0x00, 0x00,
+ },
+ },
},
},
};
@@ -123,24 +156,31 @@ KUNIT_ARRAY_PARAM(convert_xrgb8888, convert_xrgb8888_cases,
static void convert_xrgb8888_test(struct kunit *test)
{
const struct convert_xrgb8888_case *params = test->param_value;
+ const struct convert_xrgb8888_result *result;
size_t dst_size;
__u8 *dst = NULL;
+ int n;

struct drm_framebuffer fb = {
.format = drm_format_info(DRM_FORMAT_XRGB8888),
.pitches = { params->pitch, 0, 0 },
};

- dst_size = conversion_buf_size(DRM_FORMAT_RGB332, params->dst_pitch,
- &params->clip);
- KUNIT_ASSERT_GT(test, dst_size, 0);
+ for (n = 0; n < ARRAY_SIZE(params->results); n++) {
+ result = &params->results[n];
+
+ dst_size = conversion_buf_size(result->dst_format,
+ result->dst_pitch,
+ &params->clip);
+ KUNIT_ASSERT_GT(test, dst_size, 0);

- dst = kunit_kzalloc(test, dst_size, GFP_KERNEL);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dst);
+ dst = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dst);

- drm_fb_xrgb8888_to_rgb332(dst, params->dst_pitch, params->xrgb8888,
+ result->conv_func(dst, result->dst_pitch, params->xrgb8888,
&fb, &params->clip);
- KUNIT_EXPECT_EQ(test, memcmp(dst, params->expected, dst_size), 0);
+ KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected, dst_size), 0);
+ }
}

static struct kunit_case drm_format_helper_test_cases[] = {
--
2.25.1

José Expósito

unread,
Jun 27, 2022, 12:13:11 PMJun 27
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
Extend the existing test cases to test the conversion from XRGB8888 to
RGB565.

The documentation and the color picker available on [1] are useful
resources to understand this patch and validate the values returned by
the conversion function.

[1] http://www.barth-dev.de/online/rgb565-color-picker/

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
.../gpu/drm/tests/drm_format_helper_test.c | 100 +++++++++++++++++-
1 file changed, 99 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 52dc41cc7c60..3fbe8026bccc 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -48,7 +48,7 @@ struct convert_xrgb8888_case {
unsigned int pitch;
struct drm_rect clip;
const u32 xrgb8888[TEST_BUF_SIZE];
- struct convert_xrgb8888_result results[1];
+ struct convert_xrgb8888_result results[3];
};

static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
@@ -64,6 +64,26 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.dst_pitch = 0,
.expected = { 0xE0 },
},
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb8888_to_rgb565,
+ .swab = false,
+ },
+ .dst_pitch = 0,
+ .expected = { 0x00, 0xF8 },
+ },
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb8888_to_rgb565,
+ .swab = true,
+ },
+ .dst_pitch = 0,
+ .expected = { 0xF8, 0x00 },
+ },
},
},
{
@@ -81,6 +101,26 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.dst_pitch = 0,
.expected = { 0xE0 },
},
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb8888_to_rgb565,
+ .swab = false,
+ },
+ .dst_pitch = 0,
+ .expected = { 0x00, 0xF8 },
+ },
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb8888_to_rgb565,
+ .swab = true,
+ },
+ .dst_pitch = 0,
+ .expected = { 0xF8, 0x00 },
+ },
},
},
{
@@ -110,6 +150,36 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
0xFC, 0x1F,
},
},
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb8888_to_rgb565,
+ .swab = false,
+ },
+ .dst_pitch = 0,
+ .expected = {
+ 0xFF, 0xFF, 0x00, 0x00,
+ 0x00, 0xF8, 0xE0, 0x07,
+ 0x1F, 0x00, 0x1F, 0xF8,
+ 0xE0, 0xFF, 0xFF, 0x07,
+ },
+ },
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb8888_to_rgb565,
+ .swab = true,
+ },
+ .dst_pitch = 0,
+ .expected = {
+ 0xFF, 0xFF, 0x00, 0x00,
+ 0xF8, 0x00, 0x07, 0xE0,
+ 0x00, 0x1F, 0xF8, 0x1F,
+ 0xFF, 0xE0, 0x07, 0xFF,
+ },
+ },
},
},
{
@@ -133,6 +203,34 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
0xA0, 0x6D, 0x0A, 0x00, 0x00,
},
},
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb8888_to_rgb565,
+ .swab = false,
+ },
+ .dst_pitch = 10,
+ .expected = {
+ 0x33, 0x0A, 0x60, 0x12, 0x00, 0xA8, 0x00, 0x00, 0x00, 0x00,
+ 0x8E, 0x6B, 0x33, 0x0A, 0x60, 0x12, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0xA8, 0x8E, 0x6B, 0x33, 0x0A, 0x00, 0x00, 0x00, 0x00,
+ },
+ },
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb8888_to_rgb565,
+ .swab = true,
+ },
+ .dst_pitch = 10,
+ .expected = {
+ 0x0A, 0x33, 0x12, 0x60, 0xA8, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x6B, 0x8E, 0x0A, 0x33, 0x12, 0x60, 0x00, 0x00, 0x00, 0x00,
+ 0xA8, 0x00, 0x6B, 0x8E, 0x0A, 0x33, 0x00, 0x00, 0x00, 0x00,
+ },
+ },
},
},
};
--
2.25.1

Tales

unread,
Jun 28, 2022, 2:48:21 PMJun 28
to José Expósito, jav...@redhat.com, davi...@google.com, dlat...@google.com, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, Maarten Lankhorst, jani....@linux.intel.com, Maíra Canal, isab...@riseup.net, magali...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org
Em seg., 27 de jun. de 2022 às 13:13, José Expósito
<jose.ex...@gmail.com> escreveu:
Tested with "./tools/testing/kunit/kunit.py run
--kunitconfig=drivers/gpu/drm/tests --arch=x86_64", "... --arch=i386"
and baremetal on x86_64 to be sure; everything looks fine, but I feel
like some patches could be squashed, though.

Tested-by: Tales L. Aparecida <tales.a...@gmail.com>

Inspiring work, José, keep it up!
Best regards, Tales

David Gow

unread,
Jun 29, 2022, 3:27:57 AMJun 29
to José Expósito, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
These look pretty good overall to me, but there is one big issue
(which is actually with the previous series -- oops!), and a few small
stylistic thoughts.

For the big issue: these tests don't work on big-endian systems. The
'swab' bit in this series reminded me to check, and sure enough, all
of the tests fail (including the rgb332 ones).

I tested it on PowerPC with:
./tools/testing/kunit/kunit.py run
--kunitconfig=drivers/gpu/drm/tests --arch=powerpc
--cross_compile=powerpc64-linux-gnu-

So that's something which needs to be fixed.

The smaller stylistic thoughts basically all revolve around the
complexity of convert_xrgb8888_cases: there are arrays of structs with
arrays of unions of structs (with function pointers in them). This
isn't a problem: it's actually a lot more readable than that
description implies, but there are other ways it could be tackled
(which have their own tradeoffs, of course).

One possibility would be to split up the test into a separate test per
destination format. They could reuse the convert_xrgb8888_cases array,
and just each access a different result. You could make things even
clearer (IMO) by replacing the results[] array with a separate, named,
member (since you don't need to iterate over them any more), and
remove the need to have the function pointer and swab union/members by
just hardcoding those in the separate test functions. It'd also make
the results a bit clearer, as each destination format would get its
own separate set of results.

Of course, that's just an idea: I don't actually have a problem with
the existing design either (other than the endianness issue, of
course).

Cheers,
-- David

Thomas Zimmermann

unread,
Jun 29, 2022, 3:34:29 AMJun 29
to José Expósito, jav...@redhat.com, dri-...@lists.freedesktop.org, magali...@gmail.com, air...@linux.ie, maira...@usp.br, dlat...@google.com, linux-...@vger.kernel.org, tales.a...@gmail.com, davi...@google.com, isab...@riseup.net, kuni...@googlegroups.com
Hi

Am 27.06.22 um 18:11 schrieb José Expósito:
> Extend the existing test cases to test the conversion from XRGB8888 to
> RGB565.
>
> The documentation and the color picker available on [1] are useful
> resources to understand this patch and validate the values returned by
> the conversion function.
>
> [1] http://www.barth-dev.de/online/rgb565-color-picker/

URLs in commit messages are usually given as Link tag like this:

Link: http://www.barth-dev.de/online/rgb565-color-picker/ # [1]

Put this somewhere below your signed-off-by tag.

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

Thomas Zimmermann

unread,
Jun 29, 2022, 3:37:15 AMJun 29
to José Expósito, jav...@redhat.com, davi...@google.com, dlat...@google.com, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org
Hi

Am 27.06.22 um 18:11 schrieb José Expósito:
> Hello everyone,
>
> This series is a follow up of the XRGB8888 to RGB332 conversion KUnit tests.
>
> The first 3 patches refactor the existing test to make them agnostic of the target format and add support for "swab".
>
> The last patch adds the RGB565 conversion values, and shows how more formats will be easily added in the future.

Thanks for your patches. I had one comment for the final one. With this
fixed, you can add

Acked-by: Thomas Zimmermann <tzimm...@suse.de>

to the series.

Best regards
Thomas

>
> Thank you very much in advance for your feedback,
> José Expósito
>
> José Expósito (4):
> drm/format-helper: Rename test cases to make them more generic
> drm/format-helper: Transform tests to be agnostic of target format
> drm/format-helper: Add support for conversion functions with swab
> drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
>
> .../gpu/drm/tests/drm_format_helper_test.c | 231 +++++++++++++++---
> 1 file changed, 196 insertions(+), 35 deletions(-)
>
>
> base-commit: 6fde8eec71796f3534f0c274066862829813b21f
> prerequisite-patch-id: 8a16f4c8004d6161035eaea275c8eafaa0ac927e
> prerequisite-patch-id: 53fded2a49e6212b546db76ec52563a683752e65
> prerequisite-patch-id: 294b0ca27a6ee57096c8f097c0572336b8a2d583
> prerequisite-patch-id: 5e05bfc5287d16c207bfc616b2776ad72eb4ab29
> prerequisite-patch-id: e94560be85dffb62a5b3cf58d1f0fc3d278ad806
> prerequisite-patch-id: a471df39c7b32c69dd2b138a7d0af015ea42e00a

--
OpenPGP_signature

José Expósito

unread,
Jul 3, 2022, 11:19:31 AMJul 3
to David Gow, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
Hi David,

Sorry for not getting back to you sooner, I've been swamped with work
this week.

On Wed, Jun 29, 2022 at 03:27:44PM +0800, David Gow wrote:
> These look pretty good overall to me, but there is one big issue
> (which is actually with the previous series -- oops!), and a few small
> stylistic thoughts.
>
> For the big issue: these tests don't work on big-endian systems. The
> 'swab' bit in this series reminded me to check, and sure enough, all
> of the tests fail (including the rgb332 ones).
>
> I tested it on PowerPC with:
> ./tools/testing/kunit/kunit.py run
> --kunitconfig=drivers/gpu/drm/tests --arch=powerpc
> --cross_compile=powerpc64-linux-gnu-
>
> So that's something which needs to be fixed.

Oops, yes, definitely something that I need to fix!
I'll include an extra patch at the beginning of v2 fixing this bug.

> The smaller stylistic thoughts basically all revolve around the
> complexity of convert_xrgb8888_cases: there are arrays of structs with
> arrays of unions of structs (with function pointers in them). This
> isn't a problem: it's actually a lot more readable than that
> description implies, but there are other ways it could be tackled
> (which have their own tradeoffs, of course).
>
> One possibility would be to split up the test into a separate test per
> destination format. They could reuse the convert_xrgb8888_cases array,
> and just each access a different result. You could make things even
> clearer (IMO) by replacing the results[] array with a separate, named,
> member (since you don't need to iterate over them any more), and
> remove the need to have the function pointer and swab union/members by
> just hardcoding those in the separate test functions. It'd also make
> the results a bit clearer, as each destination format would get its
> own separate set of results.
>
> Of course, that's just an idea: I don't actually have a problem with
> the existing design either (other than the endianness issue, of
> course).

I like from your approach that the output of the tests would be easier
to understand. At the moment, if a test fails, there is not enough
context to know which target format failed. I'll explore this approach
and see how it looks like.

Thanks,
Jose

> Cheers,
> -- David


José Expósito

unread,
Jul 9, 2022, 7:58:48 AMJul 9
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
The tests fail on big endian architectures, like PowerPC:

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

Transform the XRGB8888 buffer from little endian to the CPU endian
before calling the conversion function to avoid this error.

Fixes: 8f456104915f ("drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()")
Reported-by: David Gow <davi...@google.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
.../gpu/drm/tests/drm_format_helper_test.c | 23 +++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 98583bf56044..4d074c2e48bf 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -111,6 +111,21 @@ static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
return dst_pitch * drm_rect_height(clip);
}

+static u32 *le32buf_to_cpu(struct kunit *test, const u32 *buf, size_t buf_size)
+{
+ u32 *dst = NULL;
+ int n;
+
+ dst = kunit_kzalloc(test, buf_size, GFP_KERNEL);
+ if (!dst)
+ return NULL;
+
+ for (n = 0; n < buf_size; n++)
+ dst[n] = le32_to_cpu(buf[n]);
+
+ return dst;
+}
+
static void xrgb8888_to_rgb332_case_desc(struct xrgb8888_to_rgb332_case *t,
char *desc)
{
@@ -125,6 +140,7 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)
const struct xrgb8888_to_rgb332_case *params = test->param_value;
size_t dst_size;
__u8 *dst = NULL;
+ __u32 *src = NULL;

struct drm_framebuffer fb = {
.format = drm_format_info(DRM_FORMAT_XRGB8888),
@@ -138,8 +154,11 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)
dst = kunit_kzalloc(test, dst_size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dst);

- drm_fb_xrgb8888_to_rgb332(dst, params->dst_pitch, params->xrgb8888,
- &fb, &params->clip);
+ src = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, src);
+
+ drm_fb_xrgb8888_to_rgb332(dst, params->dst_pitch, src, &fb,
+ &params->clip);
KUNIT_EXPECT_EQ(test, memcmp(dst, params->expected, dst_size), 0);
}

--
2.25.1

José Expósito

unread,
Jul 9, 2022, 7:58:48 AMJul 9
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
Hello everyone,

This series is a follow up of the XRGB8888 to RGB332 conversion KUnit
tests.

The first patch fixes a bug reported by David Gow in the XRGB8888 to
RGB332 tests.
Note that the fix is required for this format because internally, the
drm_fb_xrgb8888_to_rgb332_line() function, calls le32_to_cpu().
The other *_line() functiones don't change the endian. That's why the
RGB565 tests don't need to make any endian conversions.

I'm not sure whether this inconsistency handling the endian could be
considered a bug or not, but at list it is confusing. It'd be
interesting to hear the opinion of the maintainers on this topic.

Patches 2 and 3 make the test generic and the last one tests
drm_fb_xrgb8888_to_rgb565().

Best wishes,
José Expósito

Changes since v1:

- Fix a bug reported by David Gow in the XRGB8888 to RGB332 tests
- Simplify the test structure as suggested by David Gow
- Add Tested-by Tales L. Aparecida and Acked-by Thomas Zimmermann
- Fix link in the last patch (Thomas Zimmermann)

José Expósito (4):
drm/format-helper: Fix test on big endian architectures
drm/format-helper: Rename test cases to make them more generic
drm/format-helper: Support multiple target formats results
drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()

.../gpu/drm/tests/drm_format_helper_test.c | 167 ++++++++++++++----
1 file changed, 136 insertions(+), 31 deletions(-)


base-commit: 6fde8eec71796f3534f0c274066862829813b21f
prerequisite-patch-id: 8a16f4c8004d6161035eaea275c8eafaa0ac927e
prerequisite-patch-id: 53fded2a49e6212b546db76ec52563a683752e65
prerequisite-patch-id: 294b0ca27a6ee57096c8f097c0572336b8a2d583
prerequisite-patch-id: 5e05bfc5287d16c207bfc616b2776ad72eb4ab29
prerequisite-patch-id: e94560be85dffb62a5b3cf58d1f0fc3d278ad806
prerequisite-patch-id: a471df39c7b32c69dd2b138a7d0af015ea42e00a
prerequisite-patch-id: 04e8b96dd1319088e45d4b4dc4beb29889b30941
prerequisite-patch-id: bda9f93309910ca6d9528c9c560a3ccb7c388bfa
prerequisite-patch-id: f45a301abf3874302b47751378d1cb4da01a1e2a
prerequisite-patch-id: 7e8d74a4769f5bc8f2a1ced1de64aa13ea7b9d6c
prerequisite-patch-id: cfa9be5a3b48cada74c681ebe967ef2a3ac572a0
prerequisite-patch-id: f4d0b5ba40437486248a6df6ab022a3428898273
prerequisite-patch-id: f6380c88b6640cd57b867a59ed42a2fc754fa9e4
prerequisite-patch-id: 112685a3c337b3bc8ff9507ab832c6322a77a2cc
prerequisite-patch-id: 3f69109f64a8b7ac85a0332d177c8b2216cc747c
prerequisite-patch-id: 7f0a3edb0b94bd54681d67c50a23008ccdb18c00
prerequisite-patch-id: bfb6024a0e542cdc89f1f2a98cc249a2c071c68f
prerequisite-patch-id: b569ec32d7a4a9c7be317e8a8f0177ad4f696a10
prerequisite-patch-id: f1f7adf96de1501469c223a50d6405dc4b378982
prerequisite-patch-id: ce194443e4d40ded9bcec371717566398333e1db
prerequisite-patch-id: 253ba111d18eea436c45fc43216b7d3ddba41388
prerequisite-patch-id: fac3f1b822c5154bc16eae9eb97b95fa19c178ad
prerequisite-patch-id: 7eeca32e531b0d97d67285c106c1e721158da931
prerequisite-patch-id: 2b1b9ce615e8856f7e236f5fbaf7dfcd625da2ea
prerequisite-patch-id: 6e45198758680c3ed23f58ee049d93f63d316d88
prerequisite-patch-id: 58c3a87ebf0a31d65a3a787121a066013e1da82e
prerequisite-patch-id: e62f9ef9e892d075778094ee5b4e4e9d9cc76c86
prerequisite-patch-id: 3a7abafa7b011cbeca304f8af85b88aefc55cf64
prerequisite-patch-id: 1146b840a8ff43a4fdc8382c6c4d133aae6ec2fa
prerequisite-patch-id: 1455f1117f208d983759d89b750440d349a880c2
prerequisite-patch-id: 92d0eb8a4e06da97b18a5cf28a81fc90158c444d
prerequisite-patch-id: 2ab38ae9cea6d979bff04345550a1a848b55a091
prerequisite-patch-id: 25b591d59400b972ed1b709d932feaba0d5f642e
prerequisite-patch-id: 003dca01353bba78d1a05baa5852acb1b313a154
prerequisite-patch-id: f7beda6d5b2aa56ecd5ce610f36db704d1fbd653
prerequisite-patch-id: f641abe09200a8b9507124949bab294a05529175
prerequisite-patch-id: 1fa4294e769b40dca3b89fa160c4ae1ff9eff8b7
prerequisite-patch-id: 9d04c01b752031e874286e121a64844891008a14
prerequisite-patch-id: f34a698723d7df6b6a4d17686395af8b694186e2
prerequisite-patch-id: 488ab7cec987c0a6c8c032fc74ad79a40bf546f9
prerequisite-patch-id: a5a4c847fb9b55f53eaef99b353749f61dd1f72f
prerequisite-patch-id: 443d79ee1834963cb1f2a4e0007d02e419a2f5ff
prerequisite-patch-id: 15add4faa533058e2ef962aeb7563835c6cbd82a
prerequisite-patch-id: 6afb078b02085c4b24136bff54941a11090ea738
prerequisite-patch-id: da36728d198481270fb9695c7d86d210111353c2
prerequisite-patch-id: eb412a9ebab86d80a96f3c32690e5a96ed5a9ebc
prerequisite-patch-id: e33a4314f9bb9caf819f5e350c50b5b571d68e37
prerequisite-patch-id: 10ec9def77f602a29e20ac1b4193bbc7e932a7f5
prerequisite-patch-id: 970edbb64a43f50ac7e80f2f0258767a08b8294c
prerequisite-patch-id: f089485485d6e374fca925da2b904867c8e60a74
prerequisite-patch-id: b60e812dbb263d1b9c27f46bf487f4f73568a9d0
prerequisite-patch-id: 6344323c6242f0fa4402003f63cac9bc63fc7752
prerequisite-patch-id: b4d1a179227563da227593d7588306c9e676c6af
prerequisite-patch-id: 48dbb90b0dbd77ad1e6ae8101804d23bb855645f
prerequisite-patch-id: f104eb773c8c0b0ead12b8576ce0052c646cf598
prerequisite-patch-id: 4e6659a16896cb1546dc272a7db2893873d3d7c0
prerequisite-patch-id: ea56ba8675748e2fb5a942244a77a17ca1650de5
prerequisite-patch-id: df4cd3120fe4bb1ae6322ae908739fb1f620e660
prerequisite-patch-id: 533aee0b212f50f0dcf705b7e04c626340f4bd36
prerequisite-patch-id: 599329c73d4c591a95c8031a41c080115a6d602b
prerequisite-patch-id: 0f27a6299d98e481e523a93f2c494079b4c2ad4a
prerequisite-patch-id: 7621b25a47ee43ae20862c285e0b2d236311788b
prerequisite-patch-id: 72b2b9c39cf28cf2c2282231838184caba0b250d
prerequisite-patch-id: 80bf2499edfee73e4e4672619ee4da47df53d4c5
prerequisite-patch-id: 97745925e55ae5c66e379a637f0c432b5cd4c5a9
prerequisite-patch-id: 76d682950bf55d9b6a6e4df9dcd52278f534d41c
prerequisite-patch-id: 4bf1847249b002130e4e59d8a83ad6af0b27508f
prerequisite-patch-id: 1760ca541aa2cf8e892d7e6b8620b566f5d9bd05
prerequisite-patch-id: f63faffa66e94f9f330fda71dfc9f4a55b4f3d07
prerequisite-patch-id: cb8e14eefb9813f539ba1a5b5f78be5df351888d
prerequisite-patch-id: d42116fc0ae5898cdcf0d47d99866a0e328d27c7
prerequisite-patch-id: bc81a5b598bdc0e91286cef08957f5ed627b6a41
prerequisite-patch-id: 693319f739fbffa8be024f7a6ef8265caa48ef1b
prerequisite-patch-id: e10dc3d38993222d15a095f33f5150c1998d2c62
prerequisite-patch-id: 6b2a39c25f3938b86cb71ed70b7330ded34e0a2e
prerequisite-patch-id: cde017ae3f0c1691dcb437095804ef5207520f9a
prerequisite-patch-id: 1c79ba2de78c4fbf91a4371c32f8e31bcffe5493
prerequisite-patch-id: 00079a82e7089c6aacb7610953b6fbe99167dba1
prerequisite-patch-id: 609e49dc054b83b9e4b5961e9a7c956a3d54ab51
prerequisite-patch-id: a8394a54421133ddfaeb486d13a135479beb12ad
prerequisite-patch-id: 25eaeb161a40979bfa8ca4ee6246a5338d61e972
prerequisite-patch-id: be89047878944cfb969e544179f42008a3b9dae4
prerequisite-patch-id: 45bc4815468fc4c240b9291e87195ba6a56c8ced
prerequisite-patch-id: df771a5eda31e2bf8a1d46f6d38049d76fd8fa3c
prerequisite-patch-id: 3031536f5d0e359d3ff190b08e8dd8b0a1cd9bff
prerequisite-patch-id: 4c241d97f6c49494f73978ec24691b91e5ef96af
prerequisite-patch-id: a0c4a7df919ffcfd2698d7a8aaf8ae08e5005153
prerequisite-patch-id: dd85b9081eb9bc7e15a54ac958a844afc88f6716
prerequisite-patch-id: fdee5e717d385329c229a42b7aea83d29ab6d2b6
prerequisite-patch-id: 7caeb6f71b1d618fe310bb05cc5f3e6b48bd16e9
prerequisite-patch-id: ed364b5c9a5014df39654a6ba626baa7f67cf99a
prerequisite-patch-id: 56d211f7e8c7601370370e07bedff01f9f3cd4df
prerequisite-patch-id: 1c43718b51bf2e0ae545367b48b2a353f0f7289f
--
2.25.1

José Expósito

unread,
Jul 9, 2022, 7:58:49 AMJul 9
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
The tests available at the moment only check the conversion from
XRGB8888 to RGB332. However, more conversions will be tested in the
future.

In order to make the struct and functions present in the tests more
generic, rename xrgb8888_to_rgb332_* to convert_xrgb8888_*.

Tested-by: Tales L. Aparecida <tales.a...@gmail.com>
Acked-by: Thomas Zimmermann <tzimm...@suse.de>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 4d074c2e48bf..f66aaa0e52c9 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -16,7 +16,7 @@

#define TEST_BUF_SIZE 50

-struct xrgb8888_to_rgb332_case {
+struct convert_xrgb8888_case {
const char *name;
unsigned int pitch;
unsigned int dst_pitch;
@@ -25,7 +25,7 @@ struct xrgb8888_to_rgb332_case {
const u8 expected[4 * TEST_BUF_SIZE];
};

-static struct xrgb8888_to_rgb332_case xrgb8888_to_rgb332_cases[] = {
+static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
{
.name = "single_pixel_source_buffer",
.pitch = 1 * 4,
@@ -126,18 +126,18 @@ static u32 *le32buf_to_cpu(struct kunit *test, const u32 *buf, size_t buf_size)
return dst;
}

-static void xrgb8888_to_rgb332_case_desc(struct xrgb8888_to_rgb332_case *t,
- char *desc)
+static void convert_xrgb8888_case_desc(struct convert_xrgb8888_case *t,
+ char *desc)
{
strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
}

-KUNIT_ARRAY_PARAM(xrgb8888_to_rgb332, xrgb8888_to_rgb332_cases,
- xrgb8888_to_rgb332_case_desc);
+KUNIT_ARRAY_PARAM(convert_xrgb8888, convert_xrgb8888_cases,
+ convert_xrgb8888_case_desc);

static void xrgb8888_to_rgb332_test(struct kunit *test)
{
- const struct xrgb8888_to_rgb332_case *params = test->param_value;
+ const struct convert_xrgb8888_case *params = test->param_value;
size_t dst_size;
__u8 *dst = NULL;
__u32 *src = NULL;
@@ -163,8 +163,7 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)
}

static struct kunit_case drm_format_helper_test_cases[] = {
- KUNIT_CASE_PARAM(xrgb8888_to_rgb332_test,
- xrgb8888_to_rgb332_gen_params),
+ KUNIT_CASE_PARAM(xrgb8888_to_rgb332_test, convert_xrgb8888_gen_params),
{}
};

--
2.25.1

José Expósito

unread,
Jul 9, 2022, 7:58:51 AMJul 9
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
In order to support multiple destination format conversions, store the
destination pitch and the expected result in its own structure.

Tested-by: Tales L. Aparecida <tales.a...@gmail.com>
Acked-by: Thomas Zimmermann <tzimm...@suse.de>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
.../gpu/drm/tests/drm_format_helper_test.c | 53 ++++++++++++-------
1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index f66aaa0e52c9..0a490ad4fd32 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -16,34 +16,42 @@

#define TEST_BUF_SIZE 50

+struct convert_to_rgb332_result {
+ unsigned int dst_pitch;
+ const u8 expected[TEST_BUF_SIZE];
+};
+
struct convert_xrgb8888_case {
const char *name;
unsigned int pitch;
- unsigned int dst_pitch;
struct drm_rect clip;
const u32 xrgb8888[TEST_BUF_SIZE];
- const u8 expected[4 * TEST_BUF_SIZE];
+ struct convert_to_rgb332_result rgb332_result;
};

static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
{
.name = "single_pixel_source_buffer",
.pitch = 1 * 4,
- .dst_pitch = 0,
.clip = DRM_RECT_INIT(0, 0, 1, 1),
.xrgb8888 = { 0x01FF0000 },
- .expected = { 0xE0 },
+ .rgb332_result = {
+ .dst_pitch = 0,
+ .expected = { 0xE0 },
+ },
},
{
.name = "single_pixel_clip_rectangle",
.pitch = 2 * 4,
- .dst_pitch = 0,
.clip = DRM_RECT_INIT(1, 1, 1, 1),
.xrgb8888 = {
0x00000000, 0x00000000,
0x00000000, 0x10FF0000,
},
- .expected = { 0xE0 },
+ .rgb332_result = {
+ .dst_pitch = 0,
+ .expected = { 0xE0 },
+ },
},
{
/* Well known colors: White, black, red, green, blue, magenta,
@@ -52,7 +60,6 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
*/
.name = "well_known_colors",
.pitch = 4 * 4,
- .dst_pitch = 0,
.clip = DRM_RECT_INIT(1, 1, 2, 4),
.xrgb8888 = {
0x00000000, 0x00000000, 0x00000000, 0x00000000,
@@ -61,28 +68,33 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
0x00000000, 0x550000FF, 0x66FF00FF, 0x00000000,
0x00000000, 0x77FFFF00, 0x8800FFFF, 0x00000000,
},
- .expected = {
- 0xFF, 0x00,
- 0xE0, 0x1C,
- 0x03, 0xE3,
- 0xFC, 0x1F,
+ .rgb332_result = {
+ .dst_pitch = 0,
+ .expected = {
+ 0xFF, 0x00,
+ 0xE0, 0x1C,
+ 0x03, 0xE3,
+ 0xFC, 0x1F,
+ },
},
},
{
/* Randomly picked colors. Full buffer within the clip area. */
.name = "destination_pitch",
.pitch = 3 * 4,
- .dst_pitch = 5,
.clip = DRM_RECT_INIT(0, 0, 3, 3),
.xrgb8888 = {
0xA10E449C, 0xB1114D05, 0xC1A80303,
0xD16C7073, 0xA20E449C, 0xB2114D05,
0xC2A80303, 0xD26C7073, 0xA30E449C,
},
- .expected = {
- 0x0A, 0x08, 0xA0, 0x00, 0x00,
- 0x6D, 0x0A, 0x08, 0x00, 0x00,
- 0xA0, 0x6D, 0x0A, 0x00, 0x00,
+ .rgb332_result = {
+ .dst_pitch = 5,
+ .expected = {
+ 0x0A, 0x08, 0xA0, 0x00, 0x00,
+ 0x6D, 0x0A, 0x08, 0x00, 0x00,
+ 0xA0, 0x6D, 0x0A, 0x00, 0x00,
+ },
},
},
};
@@ -138,6 +150,7 @@ KUNIT_ARRAY_PARAM(convert_xrgb8888, convert_xrgb8888_cases,
static void xrgb8888_to_rgb332_test(struct kunit *test)
{
const struct convert_xrgb8888_case *params = test->param_value;
+ const struct convert_to_rgb332_result *result = &params->rgb332_result;
size_t dst_size;
__u8 *dst = NULL;
__u32 *src = NULL;
@@ -147,7 +160,7 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)
.pitches = { params->pitch, 0, 0 },
};

- dst_size = conversion_buf_size(DRM_FORMAT_RGB332, params->dst_pitch,
+ dst_size = conversion_buf_size(DRM_FORMAT_RGB332, result->dst_pitch,
&params->clip);
KUNIT_ASSERT_GT(test, dst_size, 0);

@@ -157,9 +170,9 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)
src = le32buf_to_cpu(test, params->xrgb8888, TEST_BUF_SIZE);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, src);

- drm_fb_xrgb8888_to_rgb332(dst, params->dst_pitch, src, &fb,
+ drm_fb_xrgb8888_to_rgb332(dst, result->dst_pitch, src, &fb,
&params->clip);
- KUNIT_EXPECT_EQ(test, memcmp(dst, params->expected, dst_size), 0);
+ KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected, dst_size), 0);
}

static struct kunit_case drm_format_helper_test_cases[] = {
--
2.25.1

José Expósito

unread,
Jul 9, 2022, 7:58:51 AMJul 9
to jav...@redhat.com, davi...@google.com, dlat...@google.com, tzimm...@suse.de, mri...@kernel.org, dan...@ffwll.ch, air...@linux.ie, maarten....@linux.intel.com, jani....@linux.intel.com, maira...@usp.br, isab...@riseup.net, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, José Expósito
Extend the existing test cases to test the conversion from XRGB8888 to
RGB565.

The documentation and the color picker available on [1] are useful
resources to understand this patch and validate the values returned by
the conversion function.

Tested-by: Tales L. Aparecida <tales.a...@gmail.com>
Acked-by: Thomas Zimmermann <tzimm...@suse.de>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
.../gpu/drm/tests/drm_format_helper_test.c | 76 ++++++++++++++++++-
1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 0a490ad4fd32..c0592c1235cf 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -21,12 +21,19 @@ struct convert_to_rgb332_result {
const u8 expected[TEST_BUF_SIZE];
};

+struct convert_to_rgb565_result {
+ unsigned int dst_pitch;
+ const u16 expected[TEST_BUF_SIZE];
+ const u16 expected_swab[TEST_BUF_SIZE];
+};
+
struct convert_xrgb8888_case {
const char *name;
unsigned int pitch;
struct drm_rect clip;
const u32 xrgb8888[TEST_BUF_SIZE];
struct convert_to_rgb332_result rgb332_result;
+ struct convert_to_rgb565_result rgb565_result;
};

static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
@@ -39,6 +46,11 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.dst_pitch = 0,
.expected = { 0xE0 },
},
+ .rgb565_result = {
+ .dst_pitch = 0,
+ .expected = { 0xF800 },
+ .expected_swab = { 0x00F8 },
+ },
},
{
.name = "single_pixel_clip_rectangle",
@@ -52,6 +64,11 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.dst_pitch = 0,
.expected = { 0xE0 },
},
+ .rgb565_result = {
+ .dst_pitch = 0,
+ .expected = { 0xF800 },
+ .expected_swab = { 0x00F8 },
+ },
},
{
/* Well known colors: White, black, red, green, blue, magenta,
@@ -77,6 +94,21 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
0xFC, 0x1F,
},
},
+ .rgb565_result = {
+ .dst_pitch = 0,
+ .expected = {
+ 0xFFFF, 0x0000,
+ 0xF800, 0x07E0,
+ 0x001F, 0xF81F,
+ 0xFFE0, 0x07FF,
+ },
+ .expected_swab = {
+ 0xFFFF, 0x0000,
+ 0x00F8, 0xE007,
+ 0x1F00, 0x1FF8,
+ 0xE0FF, 0xFF07,
+ },
+ },
},
{
/* Randomly picked colors. Full buffer within the clip area. */
@@ -96,6 +128,19 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
0xA0, 0x6D, 0x0A, 0x00, 0x00,
},
},
+ .rgb565_result = {
+ .dst_pitch = 10,
+ .expected = {
+ 0x0A33, 0x1260, 0xA800, 0x0000, 0x0000,
+ 0x6B8E, 0x0A33, 0x1260, 0x0000, 0x0000,
+ 0xA800, 0x6B8E, 0x0A33, 0x0000, 0x0000,
+ },
+ .expected_swab = {
+ 0x330A, 0x6012, 0x00A8, 0x0000, 0x0000,
+ 0x8E6B, 0x330A, 0x6012, 0x0000, 0x0000,
+ 0x00A8, 0x8E6B, 0x330A, 0x0000, 0x0000,
+ },
+ },
},
};

@@ -120,7 +165,7 @@ static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
if (!dst_pitch)
dst_pitch = drm_rect_width(clip) * dst_fi->cpp[0];

- return dst_pitch * drm_rect_height(clip);
+ return (dst_pitch * drm_rect_height(clip)) / (dst_fi->depth / 8);
}

static u32 *le32buf_to_cpu(struct kunit *test, const u32 *buf, size_t buf_size)
@@ -175,8 +220,37 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)
KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected, dst_size), 0);
}

+static void xrgb8888_to_rgb565_test(struct kunit *test)
+{
+ const struct convert_xrgb8888_case *params = test->param_value;
+ const struct convert_to_rgb565_result *result = &params->rgb565_result;
+ size_t dst_size;
+ __u16 *dst = NULL;
+
+ struct drm_framebuffer fb = {
+ .format = drm_format_info(DRM_FORMAT_XRGB8888),
+ .pitches = { params->pitch, 0, 0 },
+ };
+
+ dst_size = conversion_buf_size(DRM_FORMAT_RGB565, result->dst_pitch,
+ &params->clip);
+ KUNIT_ASSERT_GT(test, dst_size, 0);
+
+ dst = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dst);
+
+ drm_fb_xrgb8888_to_rgb565(dst, result->dst_pitch, params->xrgb8888, &fb,
+ &params->clip, false);
+ KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected, dst_size), 0);
+
+ drm_fb_xrgb8888_to_rgb565(dst, result->dst_pitch, params->xrgb8888, &fb,
+ &params->clip, true);
+ KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected_swab, dst_size), 0);
+}
+
static struct kunit_case drm_format_helper_test_cases[] = {
KUNIT_CASE_PARAM(xrgb8888_to_rgb332_test, convert_xrgb8888_gen_params),
+ KUNIT_CASE_PARAM(xrgb8888_to_rgb565_test, convert_xrgb8888_gen_params),
{}
};

--
2.25.1

David Gow

unread,
Jul 16, 2022, 5:32:39 AMJul 16
to José Expósito, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.ex...@gmail.com> wrote:
>
> The tests fail on big endian architectures, like PowerPC:
>
> $ ./tools/testing/kunit/kunit.py run \
> --kunitconfig=drivers/gpu/drm/tests \
> --arch=powerpc --cross_compile=powerpc64-linux-gnu-
>
> Transform the XRGB8888 buffer from little endian to the CPU endian
> before calling the conversion function to avoid this error.
>
> Fixes: 8f456104915f ("drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()")
> Reported-by: David Gow <davi...@google.com>
> Signed-off-by: José Expósito <jose.ex...@gmail.com>
> ---

Thanks: I can confirm this now works on big-endian setups.

It might be worth using the __le32 type so that tools like 'sparse'
can verify the expected endianness. At the moment, sparse does
complain about this:

../drivers/gpu/drm/tests/drm_format_helper_test.c:181:26: warning:
cast to restricted __le32

Basically, this would involve replacing the u32 types with __le32 for
things you know to be little-endian. You can then run sparse over the
code with:

./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests \
--arch=powerpc --cross_compile=powerpc64-linux-gnu- \
--make_options C=2 --make_options CF=-D__CHECK_ENDIAN__

Otherwise, this looks good to me, thanks!

Reviewed-by: David Gow <davi...@google.com>

Cheers,
-- David

David Gow

unread,
Jul 16, 2022, 5:32:46 AMJul 16
to José Expósito, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.ex...@gmail.com> wrote:
>
> In order to support multiple destination format conversions, store the
> destination pitch and the expected result in its own structure.
>
> Tested-by: Tales L. Aparecida <tales.a...@gmail.com>
> Acked-by: Thomas Zimmermann <tzimm...@suse.de>
> Signed-off-by: José Expósito <jose.ex...@gmail.com>
> ---

Looks good to me. You could probably merge this with the previous
patch if you wanted to, IMO, but it's also fine like this.

Reviewed-by: David Gow <davi...@google.com>

Cheers,
-- David


David Gow

unread,
Jul 16, 2022, 5:32:55 AMJul 16
to José Expósito, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.ex...@gmail.com> wrote:
>
> The tests available at the moment only check the conversion from
> XRGB8888 to RGB332. However, more conversions will be tested in the
> future.
>
> In order to make the struct and functions present in the tests more
> generic, rename xrgb8888_to_rgb332_* to convert_xrgb8888_*.
>
> Tested-by: Tales L. Aparecida <tales.a...@gmail.com>
> Acked-by: Thomas Zimmermann <tzimm...@suse.de>
> Signed-off-by: José Expósito <jose.ex...@gmail.com>
> ---

Looks good to me from the KUnit point of view.

Reviewed-by: David Gow <davi...@google.com>

Cheers,
-- David


David Gow

unread,
Jul 16, 2022, 5:33:03 AMJul 16
to José Expósito, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.ex...@gmail.com> wrote:
>
> Extend the existing test cases to test the conversion from XRGB8888 to
> RGB565.
>
> The documentation and the color picker available on [1] are useful
> resources to understand this patch and validate the values returned by
> the conversion function.
>
> Tested-by: Tales L. Aparecida <tales.a...@gmail.com>
> Acked-by: Thomas Zimmermann <tzimm...@suse.de>
> Signed-off-by: José Expósito <jose.ex...@gmail.com>
> Link: http://www.barth-dev.de/online/rgb565-color-picker/ # [1]
> ---

Looks good and passes here.

Reviewed-by: David Gow <davi...@google.com>

Thanks,
-- David

José Expósito

unread,
Jul 17, 2022, 12:55:20 PMJul 17
to David Gow, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
Hi David,

On Sat, Jul 16, 2022 at 05:32:51PM +0800, David Gow wrote:
> On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.ex...@gmail.com> wrote:
> >
> > Extend the existing test cases to test the conversion from XRGB8888 to
> > RGB565.
> >
> > The documentation and the color picker available on [1] are useful
> > resources to understand this patch and validate the values returned by
> > the conversion function.
> >
> > Tested-by: Tales L. Aparecida <tales.a...@gmail.com>
> > Acked-by: Thomas Zimmermann <tzimm...@suse.de>
> > Signed-off-by: José Expósito <jose.ex...@gmail.com>
> > Link: http://www.barth-dev.de/online/rgb565-color-picker/ # [1]
> > ---
>
> Looks good and passes here.
>
> Reviewed-by: David Gow <davi...@google.com>
>
> Thanks,
> -- David

Thanks a lot for reviewing the series and for pointing out the Sparse
warning.

I already fixed the warning and added the reviewed by tags, however, I
noticed that rebasing the series on the latest drm-misc-next show this
error:

[18:49:32] ============================================================
[18:49:33] =========== drm_format_helper_test (2 subtests) ============
[18:49:33] ================= xrgb8888_to_rgb332_test ==================
[18:49:33] [ERROR] Test: xrgb8888_to_rgb332_test: missing subtest result line!
[18:49:33] [ERROR] Test: xrgb8888_to_rgb332_test: 0 tests run!
[18:49:33] ========== [NO TESTS RUN] xrgb8888_to_rgb332_test ==========
[18:49:33] [ERROR] Test: drm_format_helper_test: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: drm_format_helper_test: missing subtest result line!
[18:49:33] # Subtest: drm_format_helper_test
[18:49:33] 1..2
[18:49:33] ============= [CRASHED] drm_format_helper_test =============
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] [ERROR] Test: main: missing expected subtest!
[18:49:33] [CRASHED]
[18:49:33] ============================================================
[18:49:33] Testing complete. Ran 10 tests: crashed: 10, errors: 13

I bisected drm-misc-next to find out that the first bad commit is:
e23a5e14aa278858c2e3d81ec34e83aa9a4177c5

Not very usefull, because that commit merges v5.19-rc6 into misc.

I tested on the latest kselftest-master branch and the error is not
present.

Are you aware of any change that could cause this issue?

Jose

José Expósito

unread,
Jul 17, 2022, 1:01:01 PMJul 17
to David Gow, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
José Expósito <jose.ex...@gmail.com> wrote:
> I already fixed the warning and added the reviewed by tags, however, I
> noticed that rebasing the series on the latest drm-misc-next show this
> error:
> [...]

Sorry for the extra email. I forgot to mention that the error is only
present in UML. Running in other architectures works as expected.
Tested on x86_64 and PowerPC.

Jose

Daniel Vetter

unread,
Aug 10, 2022, 12:31:53 PMAug 10
to José Expósito, David Gow, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
Maybe a regression in the kunit infrastructure? Just guessing here ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Daniel Latypov

unread,
Aug 10, 2022, 12:41:44 PMAug 10
to José Expósito, David Gow, Javier Martinez Canillas, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
Can you take a look at the raw output?

It would be .kunit/test.log (or replace .kunit with w/e --build_dir
you're using).
You could also run the test with --raw_output to have kunit.py print
that out instead.
We might want to compare the output on UML vs x86 and see what looks suspicious.

These errors
[ERROR] Test: xrgb8888_to_rgb332_test: missing subtest result line!
means that kunit.py can't parse the KTAP output.

It could be that the output is mangled for some reason.
I recall running into a UML-specific issue with output mangling on an
old kernel fork. I doubt it's related to this one, but it shows that
it's possible there could be something going on.

-Daniel

Daniel Latypov

unread,
Aug 10, 2022, 12:45:32 PMAug 10
to José Expósito, David Gow, Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann, Maxime Ripard, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List, Daniel Vetter
On Wed, Aug 10, 2022 at 9:31 AM Daniel Vetter <dan...@ffwll.ch> wrote:
>
> On Sun, Jul 17, 2022 at 07:00:54PM +0200, José Expósito wrote:
> > José Expósito <jose.ex...@gmail.com> wrote:
> > > I already fixed the warning and added the reviewed by tags, however, I
> > > noticed that rebasing the series on the latest drm-misc-next show this
> > > error:
> > > [...]
> >
> > Sorry for the extra email. I forgot to mention that the error is only
> > present in UML. Running in other architectures works as expected.
> > Tested on x86_64 and PowerPC.
>
> Maybe a regression in the kunit infrastructure? Just guessing here ...
> -Daniel

As noted elsewhere on the thread, these errors means that kunit.py
can't parse the KTAP output coming from the kernel.
There shouldn't be any recent changes in either the python-side parser
or the kernel-side output logic.

So I can't think of a kunit-specific change that would trigger this.
There could be some other issue causing output mangling.

We'd want to look at what output the UML kernel produces (stored in
.kunit/test.log, or visible via kunit.py run --raw_output).

-A different Daniel

José Expósito

unread,
Aug 13, 2022, 6:36:32 AMAug 13
to Daniel Latypov, David Gow, Javier Martinez Canillas, Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie, maarten....@linux.intel.com, Jani Nikula, Maíra Canal, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, dri-...@lists.freedesktop.org, KUnit Development, Linux Kernel Mailing List
Hi!

Sorry for not clarifying the error in this thread.
I fixed it in v3 (already merged).

The issue was in my implementation. I was overwriting a few bytes of
memory due to an out of bounds bug. Thanks to the crash I mentioned,
I detected it before the code got merged.

Thanks a lot for the pointers Daniel, the next time I'll check
.kunit/test.log for usefull information.

Jose
Reply all
Reply to author
Forward
0 new messages