Re: [PATCH v2 2/3] drm/tests: Test drm_fb_build_fourcc_list() in separate test suite

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 13.06.25 um 16:03 schrieb José Expósito:
> On Thu, Jun 12, 2025 at 03:52:23PM +0200, Thomas Zimmermann wrote:
>> Only sysfb drivers use drm_fb_build_fourcc_list(). The helper will
>> be moved from format helpers to sysfb helpers. Moving the related
>> tests to their own test suite.
>>
>> v2:
>> - rename filename to match tested code (Maxime)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Acked-by: Maxime Ripard <mripard@kernel.org>
>> ---
>>   drivers/gpu/drm/tests/Makefile                |   3 +-
>>   .../gpu/drm/tests/drm_format_helper_test.c    | 142 ---------------
>>   .../gpu/drm/tests/drm_sysfb_modeset_test.c    | 166 ++++++++++++++++++
>>   3 files changed, 168 insertions(+), 143 deletions(-)
>>   create mode 100644 drivers/gpu/drm/tests/drm_sysfb_modeset_test.c
>>
>> diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
>> index 3afd6587df08..c0e952293ad0 100644
>> --- a/drivers/gpu/drm/tests/Makefile
>> +++ b/drivers/gpu/drm/tests/Makefile
>> @@ -23,6 +23,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
>>   	drm_modes_test.o \
>>   	drm_plane_helper_test.o \
>>   	drm_probe_helper_test.o \
>> -	drm_rect_test.o
>> +	drm_rect_test.o \
>> +	drm_sysfb_modeset_test.o
>>   
>>   CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN)
>> diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
>> index 8aacc1ffa93a..ef1cc3b28f1b 100644
>> --- a/drivers/gpu/drm/tests/drm_format_helper_test.c
>> +++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
>> @@ -1335,147 +1335,6 @@ static void drm_test_fb_clip_offset(struct kunit *test)
>>   	KUNIT_EXPECT_EQ(test, offset, params->expected_offset);
>>   }
>>   
>> -struct fb_build_fourcc_list_case {
>> -	const char *name;
>> -	u32 native_fourccs[TEST_BUF_SIZE];
>> -	size_t native_fourccs_size;
>> -	u32 expected[TEST_BUF_SIZE];
>> -	size_t expected_fourccs_size;
>> -};
>> -
>> -static struct fb_build_fourcc_list_case fb_build_fourcc_list_cases[] = {
>> -	{
>> -		.name = "no native formats",
>> -		.native_fourccs = { },
>> -		.native_fourccs_size = 0,
>> -		.expected = { DRM_FORMAT_XRGB8888 },
>> -		.expected_fourccs_size = 1,
>> -	},
>> -	{
>> -		.name = "XRGB8888 as native format",
>> -		.native_fourccs = { DRM_FORMAT_XRGB8888 },
>> -		.native_fourccs_size = 1,
>> -		.expected = { DRM_FORMAT_XRGB8888 },
>> -		.expected_fourccs_size = 1,
>> -	},
>> -	{
>> -		.name = "remove duplicates",
>> -		.native_fourccs = {
>> -			DRM_FORMAT_XRGB8888,
>> -			DRM_FORMAT_XRGB8888,
>> -			DRM_FORMAT_RGB888,
>> -			DRM_FORMAT_RGB888,
>> -			DRM_FORMAT_RGB888,
>> -			DRM_FORMAT_XRGB8888,
>> -			DRM_FORMAT_RGB888,
>> -			DRM_FORMAT_RGB565,
>> -			DRM_FORMAT_RGB888,
>> -			DRM_FORMAT_XRGB8888,
>> -			DRM_FORMAT_RGB565,
>> -			DRM_FORMAT_RGB565,
>> -			DRM_FORMAT_XRGB8888,
>> -		},
>> -		.native_fourccs_size = 11,
>> -		.expected = {
>> -			DRM_FORMAT_XRGB8888,
>> -			DRM_FORMAT_RGB888,
>> -			DRM_FORMAT_RGB565,
>> -		},
>> -		.expected_fourccs_size = 3,
>> -	},
>> -	{
>> -		.name = "convert alpha formats",
>> -		.native_fourccs = {
>> -			DRM_FORMAT_ARGB1555,
>> -			DRM_FORMAT_ABGR1555,
>> -			DRM_FORMAT_RGBA5551,
>> -			DRM_FORMAT_BGRA5551,
>> -			DRM_FORMAT_ARGB8888,
>> -			DRM_FORMAT_ABGR8888,
>> -			DRM_FORMAT_RGBA8888,
>> -			DRM_FORMAT_BGRA8888,
>> -			DRM_FORMAT_ARGB2101010,
>> -			DRM_FORMAT_ABGR2101010,
>> -			DRM_FORMAT_RGBA1010102,
>> -			DRM_FORMAT_BGRA1010102,
>> -		},
>> -		.native_fourccs_size = 12,
>> -		.expected = {
>> -			DRM_FORMAT_XRGB1555,
>> -			DRM_FORMAT_XBGR1555,
>> -			DRM_FORMAT_RGBX5551,
>> -			DRM_FORMAT_BGRX5551,
>> -			DRM_FORMAT_XRGB8888,
>> -			DRM_FORMAT_XBGR8888,
>> -			DRM_FORMAT_RGBX8888,
>> -			DRM_FORMAT_BGRX8888,
>> -			DRM_FORMAT_XRGB2101010,
>> -			DRM_FORMAT_XBGR2101010,
>> -			DRM_FORMAT_RGBX1010102,
>> -			DRM_FORMAT_BGRX1010102,
>> -		},
>> -		.expected_fourccs_size = 12,
>> -	},
>> -	{
>> -		.name = "random formats",
>> -		.native_fourccs = {
>> -			DRM_FORMAT_Y212,
>> -			DRM_FORMAT_ARGB1555,
>> -			DRM_FORMAT_ABGR16161616F,
>> -			DRM_FORMAT_C8,
>> -			DRM_FORMAT_BGR888,
>> -			DRM_FORMAT_XRGB1555,
>> -			DRM_FORMAT_RGBA5551,
>> -			DRM_FORMAT_BGR565_A8,
>> -			DRM_FORMAT_R10,
>> -			DRM_FORMAT_XYUV8888,
>> -		},
>> -		.native_fourccs_size = 10,
>> -		.expected = {
>> -			DRM_FORMAT_Y212,
>> -			DRM_FORMAT_XRGB1555,
>> -			DRM_FORMAT_ABGR16161616F,
>> -			DRM_FORMAT_C8,
>> -			DRM_FORMAT_BGR888,
>> -			DRM_FORMAT_RGBX5551,
>> -			DRM_FORMAT_BGR565_A8,
>> -			DRM_FORMAT_R10,
>> -			DRM_FORMAT_XYUV8888,
>> -			DRM_FORMAT_XRGB8888,
>> -		},
>> -		.expected_fourccs_size = 10,
>> -	},
>> -};
>> -
>> -static void fb_build_fourcc_list_case_desc(struct fb_build_fourcc_list_case *t, char *desc)
>> -{
>> -	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
>> -}
>> -
>> -KUNIT_ARRAY_PARAM(fb_build_fourcc_list, fb_build_fourcc_list_cases, fb_build_fourcc_list_case_desc);
>> -
>> -static void drm_test_fb_build_fourcc_list(struct kunit *test)
>> -{
>> -	const struct fb_build_fourcc_list_case *params = test->param_value;
>> -	u32 fourccs_out[TEST_BUF_SIZE] = {0};
>> -	size_t nfourccs_out;
>> -	struct drm_device *drm;
>> -	struct device *dev;
>> -
>> -	dev = drm_kunit_helper_alloc_device(test);
>> -	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
>> -
>> -	drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_MODESET);
>> -	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
>> -
>> -	nfourccs_out = drm_fb_build_fourcc_list(drm, params->native_fourccs,
>> -						params->native_fourccs_size,
>> -						fourccs_out, TEST_BUF_SIZE);
>> -
>> -	KUNIT_EXPECT_EQ(test, nfourccs_out, params->expected_fourccs_size);
>> -	KUNIT_EXPECT_MEMEQ(test, fourccs_out, params->expected, TEST_BUF_SIZE);
>> -}
>> -
>>   struct fb_memcpy_case {
>>   	const char *name;
>>   	u32 format;
>> @@ -1864,7 +1723,6 @@ static struct kunit_case drm_format_helper_test_cases[] = {
>>   	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xbgr8888, convert_xrgb8888_gen_params),
>>   	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_abgr8888, convert_xrgb8888_gen_params),
>>   	KUNIT_CASE_PARAM(drm_test_fb_clip_offset, clip_offset_gen_params),
>> -	KUNIT_CASE_PARAM(drm_test_fb_build_fourcc_list, fb_build_fourcc_list_gen_params),
>>   	KUNIT_CASE_PARAM(drm_test_fb_memcpy, fb_memcpy_gen_params),
>>   	{}
>>   };
>> diff --git a/drivers/gpu/drm/tests/drm_sysfb_modeset_test.c b/drivers/gpu/drm/tests/drm_sysfb_modeset_test.c
>> new file mode 100644
>> index 000000000000..88a4bf28c745
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tests/drm_sysfb_modeset_test.c
>> @@ -0,0 +1,166 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +#include <kunit/test.h>
>> +
>> +#include <drm/drm_format_helper.h>
>> +#include <drm/drm_fourcc.h>
>> +#include <drm/drm_kunit_helpers.h>
>> +
>> +#define TEST_BUF_SIZE 50
>> +
>> +struct fb_build_fourcc_list_case {
>> +	const char *name;
>> +	u32 native_fourccs[TEST_BUF_SIZE];
>> +	size_t native_fourccs_size;
>> +	u32 expected[TEST_BUF_SIZE];
>> +	size_t expected_fourccs_size;
>> +};
>> +
>> +static struct fb_build_fourcc_list_case fb_build_fourcc_list_cases[] = {
>> +	{
>> +		.name = "no native formats",
>> +		.native_fourccs = { },
>> +		.native_fourccs_size = 0,
>> +		.expected = { DRM_FORMAT_XRGB8888 },
>> +		.expected_fourccs_size = 1,
>> +	},
>> +	{
>> +		.name = "XRGB8888 as native format",
>> +		.native_fourccs = { DRM_FORMAT_XRGB8888 },
>> +		.native_fourccs_size = 1,
>> +		.expected = { DRM_FORMAT_XRGB8888 },
>> +		.expected_fourccs_size = 1,
>> +	},
>> +	{
>> +		.name = "remove duplicates",
>> +		.native_fourccs = {
>> +			DRM_FORMAT_XRGB8888,
>> +			DRM_FORMAT_XRGB8888,
>> +			DRM_FORMAT_RGB888,
>> +			DRM_FORMAT_RGB888,
>> +			DRM_FORMAT_RGB888,
>> +			DRM_FORMAT_XRGB8888,
>> +			DRM_FORMAT_RGB888,
>> +			DRM_FORMAT_RGB565,
>> +			DRM_FORMAT_RGB888,
>> +			DRM_FORMAT_XRGB8888,
>> +			DRM_FORMAT_RGB565,
>> +			DRM_FORMAT_RGB565,
>> +			DRM_FORMAT_XRGB8888,
>> +		},
>> +		.native_fourccs_size = 11,
>> +		.expected = {
>> +			DRM_FORMAT_XRGB8888,
>> +			DRM_FORMAT_RGB888,
>> +			DRM_FORMAT_RGB565,
>> +		},
>> +		.expected_fourccs_size = 3,
>> +	},
>> +	{
>> +		.name = "convert alpha formats",
>> +		.native_fourccs = {
>> +			DRM_FORMAT_ARGB1555,
>> +			DRM_FORMAT_ABGR1555,
>> +			DRM_FORMAT_RGBA5551,
>> +			DRM_FORMAT_BGRA5551,
>> +			DRM_FORMAT_ARGB8888,
>> +			DRM_FORMAT_ABGR8888,
>> +			DRM_FORMAT_RGBA8888,
>> +			DRM_FORMAT_BGRA8888,
>> +			DRM_FORMAT_ARGB2101010,
>> +			DRM_FORMAT_ABGR2101010,
>> +			DRM_FORMAT_RGBA1010102,
>> +			DRM_FORMAT_BGRA1010102,
>> +		},
>> +		.native_fourccs_size = 12,
>> +		.expected = {
>> +			DRM_FORMAT_XRGB1555,
>> +			DRM_FORMAT_XBGR1555,
>> +			DRM_FORMAT_RGBX5551,
>> +			DRM_FORMAT_BGRX5551,
>> +			DRM_FORMAT_XRGB8888,
>> +			DRM_FORMAT_XBGR8888,
>> +			DRM_FORMAT_RGBX8888,
>> +			DRM_FORMAT_BGRX8888,
>> +			DRM_FORMAT_XRGB2101010,
>> +			DRM_FORMAT_XBGR2101010,
>> +			DRM_FORMAT_RGBX1010102,
>> +			DRM_FORMAT_BGRX1010102,
>> +		},
>> +		.expected_fourccs_size = 12,
>> +	},
>> +	{
>> +		.name = "random formats",
>> +		.native_fourccs = {
>> +			DRM_FORMAT_Y212,
>> +			DRM_FORMAT_ARGB1555,
>> +			DRM_FORMAT_ABGR16161616F,
>> +			DRM_FORMAT_C8,
>> +			DRM_FORMAT_BGR888,
>> +			DRM_FORMAT_XRGB1555,
>> +			DRM_FORMAT_RGBA5551,
>> +			DRM_FORMAT_BGR565_A8,
>> +			DRM_FORMAT_R10,
>> +			DRM_FORMAT_XYUV8888,
>> +		},
>> +		.native_fourccs_size = 10,
>> +		.expected = {
>> +			DRM_FORMAT_Y212,
>> +			DRM_FORMAT_XRGB1555,
>> +			DRM_FORMAT_ABGR16161616F,
>> +			DRM_FORMAT_C8,
>> +			DRM_FORMAT_BGR888,
>> +			DRM_FORMAT_RGBX5551,
>> +			DRM_FORMAT_BGR565_A8,
>> +			DRM_FORMAT_R10,
>> +			DRM_FORMAT_XYUV8888,
>> +			DRM_FORMAT_XRGB8888,
>> +		},
>> +		.expected_fourccs_size = 10,
>> +	},
>> +};
>> +
>> +static void fb_build_fourcc_list_case_desc(struct fb_build_fourcc_list_case *t, char *desc)
>> +{
>> +	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
>> +}
>> +
>> +KUNIT_ARRAY_PARAM(fb_build_fourcc_list, fb_build_fourcc_list_cases, fb_build_fourcc_list_case_desc);
>> +
>> +static void drm_test_fb_build_fourcc_list(struct kunit *test)
>> +{
>> +	const struct fb_build_fourcc_list_case *params = test->param_value;
>> +	u32 fourccs_out[TEST_BUF_SIZE] = {0};
>> +	size_t nfourccs_out;
>> +	struct drm_device *drm;
>> +	struct device *dev;
>> +
>> +	dev = drm_kunit_helper_alloc_device(test);
>> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
>> +
>> +	drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_MODESET);
>> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
>> +
>> +	nfourccs_out = drm_fb_build_fourcc_list(drm, params->native_fourccs,
>> +						params->native_fourccs_size,
>> +						fourccs_out, TEST_BUF_SIZE);
>> +
>> +	KUNIT_EXPECT_EQ(test, nfourccs_out, params->expected_fourccs_size);
>> +	KUNIT_EXPECT_MEMEQ(test, fourccs_out, params->expected, TEST_BUF_SIZE);
>> +}
>> +
>> +static struct kunit_case drm_sysfb_helper_test_cases[] = {
>> +	KUNIT_CASE_PARAM(drm_test_fb_build_fourcc_list, fb_build_fourcc_list_gen_params),
>> +	{}
>> +};
>> +
>> +static struct kunit_suite drm_sysfb_helper_test_suite = {
>> +	.name = "drm_sysfb_helper_test",
> Other tests use the same name as the file live in, dropping or not
> the "_test" suffix.
>
> Is this a missing change from the rename in v1?

I renamed the source file as suggested in an earlier rename, but forgot 
about those names. Sorry, I'm not really familiar with the conventions 
here. I'll fix this in the next iteration. Thanks for reviewing.

Best regards
Thomas

>
> Other than that:
>
> Reviewed-by: José Expósito <jose.exposito89@gmail.com>
>
>> +	.test_cases = drm_sysfb_helper_test_cases,
>> +};
>> +
>> +kunit_test_suite(drm_sysfb_helper_test_suite);
>> +
>> +MODULE_DESCRIPTION("KUnit tests for the drm_sysfb_helper APIs");
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("José Expósito <jose.exposito89@gmail.com>");
>> -- 
>> 2.49.0
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)