* [PATCH v2 1/4] drm/format-helper: Fix test on big endian architectures
2022-07-09 11:58 [PATCH v2 0/4] KUnit tests for RGB565 conversion José Expósito
@ 2022-07-09 11:58 ` José Expósito
2022-07-16 9:32 ` David Gow
2022-07-09 11:58 ` [PATCH v2 2/4] drm/format-helper: Rename test cases to make them more generic José Expósito
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-07-09 11:58 UTC (permalink / raw)
To: javierm
Cc: davidgow, dlatypov, tzimmermann, mripard, daniel, airlied,
maarten.lankhorst, jani.nikula, maira.canal, isabbasso,
magalilemes00, tales.aparecida, dri-devel, kunit-dev,
linux-kernel, 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 <davidgow@google.com>
Signed-off-by: José Expósito <jose.exposito89@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, ¶ms->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,
+ ¶ms->clip);
KUNIT_EXPECT_EQ(test, memcmp(dst, params->expected, dst_size), 0);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/4] drm/format-helper: Fix test on big endian architectures
2022-07-09 11:58 ` [PATCH v2 1/4] drm/format-helper: Fix test on big endian architectures José Expósito
@ 2022-07-16 9:32 ` David Gow
0 siblings, 0 replies; 15+ messages in thread
From: David Gow @ 2022-07-16 9:32 UTC (permalink / raw)
To: José Expósito
Cc: Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann,
Maxime Ripard, Daniel Vetter, David Airlie, maarten.lankhorst,
Jani Nikula, Maíra Canal, Isabella Basso, magalilemes00,
tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 3558 bytes --]
On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.exposito89@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 <davidgow@google.com>
> Signed-off-by: José Expósito <jose.exposito89@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 <davidgow@google.com>
Cheers,
-- David
> .../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, ¶ms->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,
> + ¶ms->clip);
> KUNIT_EXPECT_EQ(test, memcmp(dst, params->expected, dst_size), 0);
> }
>
> --
> 2.25.1
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/4] drm/format-helper: Rename test cases to make them more generic
2022-07-09 11:58 [PATCH v2 0/4] KUnit tests for RGB565 conversion José Expósito
2022-07-09 11:58 ` [PATCH v2 1/4] drm/format-helper: Fix test on big endian architectures José Expósito
@ 2022-07-09 11:58 ` José Expósito
2022-07-16 9:32 ` David Gow
2022-07-09 11:58 ` [PATCH v2 3/4] drm/format-helper: Support multiple target formats results José Expósito
2022-07-09 11:58 ` [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565() José Expósito
3 siblings, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-07-09 11:58 UTC (permalink / raw)
To: javierm
Cc: davidgow, dlatypov, tzimmermann, mripard, daniel, airlied,
maarten.lankhorst, jani.nikula, maira.canal, isabbasso,
magalilemes00, tales.aparecida, dri-devel, kunit-dev,
linux-kernel, 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.aparecida@gmail.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: José Expósito <jose.exposito89@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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/4] drm/format-helper: Rename test cases to make them more generic
2022-07-09 11:58 ` [PATCH v2 2/4] drm/format-helper: Rename test cases to make them more generic José Expósito
@ 2022-07-16 9:32 ` David Gow
0 siblings, 0 replies; 15+ messages in thread
From: David Gow @ 2022-07-16 9:32 UTC (permalink / raw)
To: José Expósito
Cc: Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann,
Maxime Ripard, Daniel Vetter, David Airlie, maarten.lankhorst,
Jani Nikula, Maíra Canal, Isabella Basso, magalilemes00,
tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 3070 bytes --]
On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.exposito89@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.aparecida@gmail.com>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> ---
Looks good to me from the KUnit point of view.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> 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
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] drm/format-helper: Support multiple target formats results
2022-07-09 11:58 [PATCH v2 0/4] KUnit tests for RGB565 conversion José Expósito
2022-07-09 11:58 ` [PATCH v2 1/4] drm/format-helper: Fix test on big endian architectures José Expósito
2022-07-09 11:58 ` [PATCH v2 2/4] drm/format-helper: Rename test cases to make them more generic José Expósito
@ 2022-07-09 11:58 ` José Expósito
2022-07-16 9:32 ` David Gow
2022-07-09 11:58 ` [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565() José Expósito
3 siblings, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-07-09 11:58 UTC (permalink / raw)
To: javierm
Cc: davidgow, dlatypov, tzimmermann, mripard, daniel, airlied,
maarten.lankhorst, jani.nikula, maira.canal, isabbasso,
magalilemes00, tales.aparecida, dri-devel, kunit-dev,
linux-kernel, 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.aparecida@gmail.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: José Expósito <jose.exposito89@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 = ¶ms->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,
¶ms->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,
¶ms->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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/4] drm/format-helper: Support multiple target formats results
2022-07-09 11:58 ` [PATCH v2 3/4] drm/format-helper: Support multiple target formats results José Expósito
@ 2022-07-16 9:32 ` David Gow
0 siblings, 0 replies; 15+ messages in thread
From: David Gow @ 2022-07-16 9:32 UTC (permalink / raw)
To: José Expósito
Cc: Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann,
Maxime Ripard, Daniel Vetter, David Airlie, maarten.lankhorst,
Jani Nikula, Maíra Canal, Isabella Basso, magalilemes00,
tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 6481 bytes --]
On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.exposito89@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.aparecida@gmail.com>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: José Expósito <jose.exposito89@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 <davidgow@google.com>
Cheers,
-- David
> .../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 = ¶ms->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,
> ¶ms->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,
> ¶ms->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
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
2022-07-09 11:58 [PATCH v2 0/4] KUnit tests for RGB565 conversion José Expósito
` (2 preceding siblings ...)
2022-07-09 11:58 ` [PATCH v2 3/4] drm/format-helper: Support multiple target formats results José Expósito
@ 2022-07-09 11:58 ` José Expósito
2022-07-16 9:32 ` David Gow
3 siblings, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-07-09 11:58 UTC (permalink / raw)
To: javierm
Cc: davidgow, dlatypov, tzimmermann, mripard, daniel, airlied,
maarten.lankhorst, jani.nikula, maira.canal, isabbasso,
magalilemes00, tales.aparecida, dri-devel, kunit-dev,
linux-kernel, 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.aparecida@gmail.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
Link: http://www.barth-dev.de/online/rgb565-color-picker/ # [1]
---
.../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 = ¶ms->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,
+ ¶ms->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,
+ ¶ms->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,
+ ¶ms->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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
2022-07-09 11:58 ` [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565() José Expósito
@ 2022-07-16 9:32 ` David Gow
2022-07-17 16:55 ` José Expósito
2022-07-17 17:00 ` José Expósito
0 siblings, 2 replies; 15+ messages in thread
From: David Gow @ 2022-07-16 9:32 UTC (permalink / raw)
To: José Expósito
Cc: Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann,
Maxime Ripard, Daniel Vetter, David Airlie, maarten.lankhorst,
Jani Nikula, Maíra Canal, Isabella Basso, magalilemes00,
tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 6763 bytes --]
On Sat, Jul 9, 2022 at 7:58 PM José Expósito <jose.exposito89@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.aparecida@gmail.com>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> Link: http://www.barth-dev.de/online/rgb565-color-picker/ # [1]
> ---
Looks good and passes here.
Reviewed-by: David Gow <davidgow@google.com>
Thanks,
-- David
> .../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 = ¶ms->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,
> + ¶ms->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,
> + ¶ms->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,
> + ¶ms->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
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
2022-07-16 9:32 ` David Gow
@ 2022-07-17 16:55 ` José Expósito
2022-07-17 17:00 ` José Expósito
1 sibling, 0 replies; 15+ messages in thread
From: José Expósito @ 2022-07-17 16:55 UTC (permalink / raw)
To: David Gow
Cc: Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann,
Maxime Ripard, Daniel Vetter, David Airlie, maarten.lankhorst,
Jani Nikula, Maíra Canal, Isabella Basso, magalilemes00,
tales.aparecida, dri-devel, 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.exposito89@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.aparecida@gmail.com>
> > Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> > Link: http://www.barth-dev.de/online/rgb565-color-picker/ # [1]
> > ---
>
> Looks good and passes here.
>
> Reviewed-by: David Gow <davidgow@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
> > .../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 = ¶ms->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,
> > + ¶ms->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,
> > + ¶ms->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,
> > + ¶ms->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
> >
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
2022-07-16 9:32 ` David Gow
2022-07-17 16:55 ` José Expósito
@ 2022-07-17 17:00 ` José Expósito
2022-08-10 16:31 ` Daniel Vetter
2022-08-10 16:41 ` Daniel Latypov
1 sibling, 2 replies; 15+ messages in thread
From: José Expósito @ 2022-07-17 17:00 UTC (permalink / raw)
To: David Gow
Cc: Javier Martinez Canillas, Daniel Latypov, Thomas Zimmermann,
Maxime Ripard, Daniel Vetter, David Airlie, maarten.lankhorst,
Jani Nikula, Maíra Canal, Isabella Basso, magalilemes00,
tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
José Expósito <jose.exposito89@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
2022-07-17 17:00 ` José Expósito
@ 2022-08-10 16:31 ` Daniel Vetter
2022-08-10 16:45 ` Daniel Latypov
2022-08-10 16:41 ` Daniel Latypov
1 sibling, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2022-08-10 16:31 UTC (permalink / raw)
To: José Expósito
Cc: David Gow, Javier Martinez Canillas, Daniel Latypov,
Thomas Zimmermann, Maxime Ripard, Daniel Vetter, David Airlie,
maarten.lankhorst, Jani Nikula, Maíra Canal, Isabella Basso,
magalilemes00, tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
On Sun, Jul 17, 2022 at 07:00:54PM +0200, José Expósito wrote:
> José Expósito <jose.exposito89@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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
2022-08-10 16:31 ` Daniel Vetter
@ 2022-08-10 16:45 ` Daniel Latypov
0 siblings, 0 replies; 15+ messages in thread
From: Daniel Latypov @ 2022-08-10 16:45 UTC (permalink / raw)
To: José Expósito, David Gow, Javier Martinez Canillas,
Daniel Latypov, Thomas Zimmermann, Maxime Ripard, David Airlie,
maarten.lankhorst, Jani Nikula, Maíra Canal, Isabella Basso,
magalilemes00, tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
Cc: Daniel Vetter
On Wed, Aug 10, 2022 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Sun, Jul 17, 2022 at 07:00:54PM +0200, José Expósito wrote:
> > José Expósito <jose.exposito89@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
2022-07-17 17:00 ` José Expósito
2022-08-10 16:31 ` Daniel Vetter
@ 2022-08-10 16:41 ` Daniel Latypov
2022-08-13 10:36 ` José Expósito
1 sibling, 1 reply; 15+ messages in thread
From: Daniel Latypov @ 2022-08-10 16:41 UTC (permalink / raw)
To: José Expósito
Cc: David Gow, Javier Martinez Canillas, Thomas Zimmermann,
Maxime Ripard, Daniel Vetter, David Airlie, maarten.lankhorst,
Jani Nikula, Maíra Canal, Isabella Basso, magalilemes00,
tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
On Sun, Jul 17, 2022 at 10:01 AM José Expósito
<jose.exposito89@gmail.com> wrote:
>
> José Expósito <jose.exposito89@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.
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
2022-08-10 16:41 ` Daniel Latypov
@ 2022-08-13 10:36 ` José Expósito
0 siblings, 0 replies; 15+ messages in thread
From: José Expósito @ 2022-08-13 10:36 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Javier Martinez Canillas, Thomas Zimmermann,
Maxime Ripard, Daniel Vetter, David Airlie, maarten.lankhorst,
Jani Nikula, Maíra Canal, Isabella Basso, magalilemes00,
tales.aparecida, dri-devel, KUnit Development,
Linux Kernel Mailing List
On Wed, Aug 10, 2022 at 09:41:18AM -0700, Daniel Latypov wrote:
> On Sun, Jul 17, 2022 at 10:01 AM José Expósito
> <jose.exposito89@gmail.com> wrote:
> >
> > José Expósito <jose.exposito89@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.
>
> 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
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
^ permalink raw reply [flat|nested] 15+ messages in thread