[PATCH 2/2] drm/format: Split into more granular test cases

Maíra Canal mairacanal at riseup.net
Fri Sep 2 14:18:36 UTC 2022


On 9/2/22 11:06, Michał Winiarski wrote:
> On Wed, Aug 31, 2022 at 07:15:11PM -0300, Maíra Canal wrote:
>> Hi Michał
>>
>> Some very minor nits inline, but either way:
>>
>> Reviewed-by: Maíra Canal <mairacanal at riseup.net>
>>
>> On 8/31/22 18:56, Michał Winiarski wrote:
>>> While we have multiple test cases, most of them check multiple
>>> conditions, calling the function that is tested multiple times with
>>> different arguments (with comments that indicate test case boundary).
>>> This usually means that it can be easily converted into multiple test
>>> cases.
>>>
>>> Passing output:
>>> ============================================================
>>> ================= drm_format (18 subtests) =================
>>> [PASSED] drm_format_block_width_invalid
>>> [PASSED] drm_format_block_width_one_plane
>>> [PASSED] drm_format_block_width_two_plane
>>> [PASSED] drm_format_block_width_three_plane
>>> [PASSED] drm_format_block_width_tiled
>>> [PASSED] drm_format_block_height_invalid
>>> [PASSED] drm_format_block_height_one_plane
>>> [PASSED] drm_format_block_height_two_plane
>>> [PASSED] drm_format_block_height_three_plane
>>> [PASSED] drm_format_block_height_tiled
>>> [PASSED] drm_format_min_pitch_invalid
>>> [PASSED] drm_format_min_pitch_one_plane_8bpp
>>> [PASSED] drm_format_min_pitch_one_plane_16bpp
>>> [PASSED] drm_format_min_pitch_one_plane_24bpp
>>> [PASSED] drm_format_min_pitch_one_plane_32bpp
>>> [PASSED] drm_format_min_pitch_two_plane
>>> [PASSED] drm_format_min_pitch_three_plane_8bpp
>>> [PASSED] drm_format_min_pitch_tiled
>>
>> As Jani pointed out in [1], "drm_" prefix can be a bit confusing. I will
>> send a patch tomorrow using the prefix "test_drm_" on all tests to make the
>> naming more consistent. It would be nice if this patch already hold the new
>> naming, but anyway I can send a patch changing it later with the new prefix
>> gets approved.
>>
>> [1] https://lore.kernel.org/dri-devel/20220831104941.doc75juindcm5mcl@nostramo.hardline.pl/T/#m82b4e710063b47029a8bd4716d137e575640da9a
> 
> Sure - I can resend with different naming convention if needed.

The naming convention ended up being more complicated than I originally 
thought, so I believe it is great as it is. If the patch doesn't get 
other feedback and anyone pushes it, I can push in a couple of days.

Best Regards,
- Maíra Canal

> 
>>
>>> =================== [PASSED] drm_format ====================
>>> ============================================================
>>> Testing complete. Ran 18 tests: passed: 18
>>>
>>> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
>>> ---
>>>    drivers/gpu/drm/tests/drm_format_test.c | 156 ++++++++++++++++--------
>>>    1 file changed, 108 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/tests/drm_format_test.c b/drivers/gpu/drm/tests/drm_format_test.c
>>> index 0efa88bf56a9..1936d2d59908 100644
>>> --- a/drivers/gpu/drm/tests/drm_format_test.c
>>> +++ b/drivers/gpu/drm/tests/drm_format_test.c
>>> @@ -9,100 +9,133 @@
>>>    #include <drm/drm_fourcc.h>
>>> -static void igt_check_drm_format_block_width(struct kunit *test)
>>> +static void drm_format_block_width_invalid(struct kunit *test)
>>>    {
>>>    	const struct drm_format_info *info = NULL;
>>> -	/* Test invalid arguments */
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, -1), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 1), 0);
>>> +}
>>> +
>>> +static void drm_format_block_width_one_plane(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_XRGB4444);
>>> -	/* Test 1 plane format */
>>> -	info = drm_format_info(DRM_FORMAT_XRGB4444);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 0), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 1), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, -1), 0);
>>> +}
>>> +
>>> +static void drm_format_block_width_two_plane(struct kunit *test)
>>
>> s/plane/planes
> 
> NV12 format has two planes, therefore it's a two-plane format.
> 
> -Michał
> 
>>
>> Best Regards,
>> - Maíra Canal
>>
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_NV12);
>>> -	/* Test 2 planes format */
>>> -	info = drm_format_info(DRM_FORMAT_NV12);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 0), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 1), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 2), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, -1), 0);
>>> +}
>>> +
>>> +static void drm_format_block_width_three_plane(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_YUV422);
>>> -	/* Test 3 planes format */
>>> -	info = drm_format_info(DRM_FORMAT_YUV422);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 0), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 1), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 2), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 3), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, -1), 0);
>>> +}
>>> +
>>> +static void drm_format_block_width_tiled(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_X0L0);
>>> -	/* Test a tiled format */
>>> -	info = drm_format_info(DRM_FORMAT_X0L0);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 0), 2);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 1), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, -1), 0);
>>>    }
>>> -static void igt_check_drm_format_block_height(struct kunit *test)
>>> +static void drm_format_block_height_invalid(struct kunit *test)
>>>    {
>>>    	const struct drm_format_info *info = NULL;
>>> -	/* Test invalid arguments */
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, -1), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 1), 0);
>>> +}
>>> +
>>> +static void drm_format_block_height_one_plane(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_XRGB4444);
>>> -	/* Test 1 plane format */
>>> -	info = drm_format_info(DRM_FORMAT_XRGB4444);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 0), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, -1), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 1), 0);
>>> +}
>>> +
>>> +static void drm_format_block_height_two_plane(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_NV12);
>>> -	/* Test 2 planes format */
>>> -	info = drm_format_info(DRM_FORMAT_NV12);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 0), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 1), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 2), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, -1), 0);
>>> +}
>>> +
>>> +static void drm_format_block_height_three_plane(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_YUV422);
>>> -	/* Test 3 planes format */
>>> -	info = drm_format_info(DRM_FORMAT_YUV422);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 0), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 1), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 2), 1);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 3), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, -1), 0);
>>> +}
>>> +
>>> +static void drm_format_block_height_tiled(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_X0L0);
>>> -	/* Test a tiled format */
>>> -	info = drm_format_info(DRM_FORMAT_X0L0);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 0), 2);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, 1), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_block_height(info, -1), 0);
>>>    }
>>> -static void igt_check_drm_format_min_pitch_for_single_plane(struct kunit *test)
>>> +static void drm_format_min_pitch_invalid(struct kunit *test)
>>>    {
>>>    	const struct drm_format_info *info = NULL;
>>> -	/* Test invalid arguments */
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, -1, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, 0), 0);
>>> +}
>>> +
>>> +static void drm_format_min_pitch_one_plane_8bpp(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_RGB332);
>>> -	/* Test 1 plane 8 bits per pixel format */
>>> -	info = drm_format_info(DRM_FORMAT_RGB332);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, -1, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, 0), 0);
>>> @@ -118,10 +151,14 @@ static void igt_check_drm_format_min_pitch_for_single_plane(struct kunit *test)
>>>    			(uint64_t)UINT_MAX);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)),
>>>    			(uint64_t)(UINT_MAX - 1));
>>> +}
>>> +
>>> +static void drm_format_min_pitch_one_plane_16bpp(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_XRGB4444);
>>> -	/* Test 1 plane 16 bits per pixel format */
>>> -	info = drm_format_info(DRM_FORMAT_XRGB4444);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, -1, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, 0), 0);
>>> @@ -137,10 +174,14 @@ static void igt_check_drm_format_min_pitch_for_single_plane(struct kunit *test)
>>>    			(uint64_t)UINT_MAX * 2);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)),
>>>    			(uint64_t)(UINT_MAX - 1) * 2);
>>> +}
>>> +
>>> +static void drm_format_min_pitch_one_plane_24bpp(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_RGB888);
>>> -	/* Test 1 plane 24 bits per pixel format */
>>> -	info = drm_format_info(DRM_FORMAT_RGB888);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, -1, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, 0), 0);
>>> @@ -154,12 +195,16 @@ static void igt_check_drm_format_min_pitch_for_single_plane(struct kunit *test)
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 671), 2013);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, UINT_MAX),
>>>    			(uint64_t)UINT_MAX * 3);
>>> -	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)),
>>> +	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, UINT_MAX - 1),
>>>    			(uint64_t)(UINT_MAX - 1) * 3);
>>> +}
>>> +
>>> +static void drm_format_min_pitch_one_plane_32bpp(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_ABGR8888);
>>> -	/* Test 1 plane 32 bits per pixel format */
>>> -	info = drm_format_info(DRM_FORMAT_ABGR8888);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, -1, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, 0), 0);
>>> @@ -173,17 +218,16 @@ static void igt_check_drm_format_min_pitch_for_single_plane(struct kunit *test)
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 671), 2684);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, UINT_MAX),
>>>    			(uint64_t)UINT_MAX * 4);
>>> -	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)),
>>> +	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, UINT_MAX - 1),
>>>    			(uint64_t)(UINT_MAX - 1) * 4);
>>>    }
>>> -static void igt_check_drm_format_min_pitch_for_multi_planar(struct kunit *test)
>>> +static void drm_format_min_pitch_two_plane(struct kunit *test)
>>>    {
>>> -	const struct drm_format_info *info = NULL;
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_NV12);
>>> -	/* Test 2 planes format */
>>> -	info = drm_format_info(DRM_FORMAT_NV12);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, -1, 0), 0);
>>> @@ -211,10 +255,14 @@ static void igt_check_drm_format_min_pitch_for_multi_planar(struct kunit *test)
>>>    			(uint64_t)(UINT_MAX - 1));
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, (UINT_MAX - 1) /  2),
>>>    			(uint64_t)(UINT_MAX - 1));
>>> +}
>>> +
>>> +static void drm_format_min_pitch_three_plane_8bpp(struct kunit *test)
>>> +{
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_YUV422);
>>> -	/* Test 3 planes 8 bits per pixel format */
>>> -	info = drm_format_info(DRM_FORMAT_YUV422);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 2, 0), 0);
>>> @@ -256,13 +304,12 @@ static void igt_check_drm_format_min_pitch_for_multi_planar(struct kunit *test)
>>>    			(uint64_t)(UINT_MAX - 1) / 2);
>>>    }
>>> -static void igt_check_drm_format_min_pitch_for_tiled_format(struct kunit *test)
>>> +static void drm_format_min_pitch_tiled(struct kunit *test)
>>>    {
>>> -	const struct drm_format_info *info = NULL;
>>> +	const struct drm_format_info *info = drm_format_info(DRM_FORMAT_X0L2);
>>> -	/* Test tiled format */
>>> -	info = drm_format_info(DRM_FORMAT_X0L2);
>>>    	KUNIT_ASSERT_NOT_NULL(test, info);
>>> +
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, -1, 0), 0);
>>>    	KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 1, 0), 0);
>>> @@ -281,12 +328,25 @@ static void igt_check_drm_format_min_pitch_for_tiled_format(struct kunit *test)
>>>    }
>>>    static struct kunit_case drm_format_tests[] = {
>>> -	KUNIT_CASE(igt_check_drm_format_block_width),
>>> -	KUNIT_CASE(igt_check_drm_format_block_height),
>>> -	KUNIT_CASE(igt_check_drm_format_min_pitch_for_single_plane),
>>> -	KUNIT_CASE(igt_check_drm_format_min_pitch_for_multi_planar),
>>> -	KUNIT_CASE(igt_check_drm_format_min_pitch_for_tiled_format),
>>> -	{ }
>>> +	KUNIT_CASE(drm_format_block_width_invalid),
>>> +	KUNIT_CASE(drm_format_block_width_one_plane),
>>> +	KUNIT_CASE(drm_format_block_width_two_plane),
>>> +	KUNIT_CASE(drm_format_block_width_three_plane),
>>> +	KUNIT_CASE(drm_format_block_width_tiled),
>>> +	KUNIT_CASE(drm_format_block_height_invalid),
>>> +	KUNIT_CASE(drm_format_block_height_one_plane),
>>> +	KUNIT_CASE(drm_format_block_height_two_plane),
>>> +	KUNIT_CASE(drm_format_block_height_three_plane),
>>> +	KUNIT_CASE(drm_format_block_height_tiled),
>>> +	KUNIT_CASE(drm_format_min_pitch_invalid),
>>> +	KUNIT_CASE(drm_format_min_pitch_one_plane_8bpp),
>>> +	KUNIT_CASE(drm_format_min_pitch_one_plane_16bpp),
>>> +	KUNIT_CASE(drm_format_min_pitch_one_plane_24bpp),
>>> +	KUNIT_CASE(drm_format_min_pitch_one_plane_32bpp),
>>> +	KUNIT_CASE(drm_format_min_pitch_two_plane),
>>> +	KUNIT_CASE(drm_format_min_pitch_three_plane_8bpp),
>>> +	KUNIT_CASE(drm_format_min_pitch_tiled),
>>> +	{}
>>>    };
>>>    static struct kunit_suite drm_format_test_suite = {


More information about the dri-devel mailing list