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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxxx/T/#m82b4e710063b47029a8bd4716d137e575640da9a

Sure - I can resend with different naming convention if needed.

> 
> > =================== [PASSED] drm_format ====================
> > ============================================================
> > Testing complete. Ran 18 tests: passed: 18
> > 
> > Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> > ---
> >   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 = {



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux