dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests
@ 2022-09-01 12:42 Maíra Canal
  2022-09-01 12:42 ` [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_" Maíra Canal
  2022-09-02  7:52 ` [PATCH v2 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests David Gow
  0 siblings, 2 replies; 17+ messages in thread
From: Maíra Canal @ 2022-09-01 12:42 UTC (permalink / raw)
  To: Isabella Basso, magalilemes00, tales.aparecida, mwen,
	andrealmeid, siqueirajordao, Trevor Woerner, Daniel Vetter,
	David Airlie, Javier Martinez Canillas, David Gow,
	brendanhiggins, Arthur Grillo, michal.winiarski,
	José Expósito, Jani Nikula
  Cc: Maíra Canal, linux-kernel, dri-devel, kunit-dev

The igt_check_drm_framebuffer_create is based on a loop that executes
tests for all createbuffer_tests test cases. This could be better
represented by parameterized tests, provided by KUnit.

So, convert the igt_check_drm_framebuffer_create into parameterized tests.

Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
- Use .init for mock_drm_device instead of a global variable. (Michał Winiarski)
---
 drivers/gpu/drm/tests/drm_framebuffer_test.c | 45 ++++++++++++--------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index ec7a08ba4056..564794917784 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -25,7 +25,7 @@ struct drm_framebuffer_test {
 	const char *name;
 };
 
-static struct drm_framebuffer_test createbuffer_tests[] = {
+static const struct drm_framebuffer_test drm_framebuffer_create_cases[] = {
 { .buffer_created = 1, .name = "ABGR8888 normal sizes",
 	.cmd = { .width = 600, .height = 600, .pixel_format = DRM_FORMAT_ABGR8888,
 		 .handles = { 1, 0, 0 }, .pitches = { 4 * 600, 0, 0 },
@@ -330,43 +330,52 @@ static struct drm_mode_config_funcs mock_config_funcs = {
 	.fb_create = fb_create_mock,
 };
 
-static struct drm_device mock_drm_device = {
-	.mode_config = {
+static int drm_framebuffer_test_init(struct kunit *test)
+{
+	struct drm_device *mock;
+
+	mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mock);
+
+	mock->mode_config = (struct drm_mode_config) {
 		.min_width = MIN_WIDTH,
 		.max_width = MAX_WIDTH,
 		.min_height = MIN_HEIGHT,
 		.max_height = MAX_HEIGHT,
 		.funcs = &mock_config_funcs,
-	},
-};
+	};
 
-static int execute_drm_mode_fb_cmd2(struct drm_mode_fb_cmd2 *r)
+	test->priv = mock;
+	return 0;
+}
+
+static void test_drm_framebuffer_create(struct kunit *test)
 {
+	const struct drm_framebuffer_test *params = test->param_value;
+	struct drm_device *mock = test->priv;
 	int buffer_created = 0;
 
-	mock_drm_device.dev_private = &buffer_created;
-	drm_internal_framebuffer_create(&mock_drm_device, r, NULL);
-	return buffer_created;
+	mock->dev_private = &buffer_created;
+	drm_internal_framebuffer_create(mock, &params->cmd, NULL);
+	KUNIT_EXPECT_EQ(test, params->buffer_created, buffer_created);
 }
 
-static void igt_check_drm_framebuffer_create(struct kunit *test)
+static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, char *desc)
 {
-	int i = 0;
-
-	for (i = 0; i < ARRAY_SIZE(createbuffer_tests); i++) {
-		KUNIT_EXPECT_EQ_MSG(test, createbuffer_tests[i].buffer_created,
-				    execute_drm_mode_fb_cmd2(&createbuffer_tests[i].cmd),
-		     "Test %d: \"%s\" failed\n", i, createbuffer_tests[i].name);
-	}
+	strcpy(desc, t->name);
 }
 
+KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
+		  drm_framebuffer_test_to_desc);
+
 static struct kunit_case drm_framebuffer_tests[] = {
-	KUNIT_CASE(igt_check_drm_framebuffer_create),
+	KUNIT_CASE_PARAM(test_drm_framebuffer_create, drm_framebuffer_create_gen_params),
 	{ }
 };
 
 static struct kunit_suite drm_framebuffer_test_suite = {
 	.name = "drm_framebuffer",
+	.init = drm_framebuffer_test_init,
 	.test_cases = drm_framebuffer_tests,
 };
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-01 12:42 [PATCH v2 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests Maíra Canal
@ 2022-09-01 12:42 ` Maíra Canal
  2022-09-01 12:55   ` Maxime Ripard
  2022-09-02  7:52 ` [PATCH v2 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests David Gow
  1 sibling, 1 reply; 17+ messages in thread
From: Maíra Canal @ 2022-09-01 12:42 UTC (permalink / raw)
  To: Isabella Basso, magalilemes00, tales.aparecida, mwen,
	andrealmeid, siqueirajordao, Trevor Woerner, Daniel Vetter,
	David Airlie, Javier Martinez Canillas, David Gow,
	brendanhiggins, Arthur Grillo, michal.winiarski,
	José Expósito, Jani Nikula
  Cc: Maíra Canal, linux-kernel, dri-devel, kunit-dev

With the introduction of KUnit, IGT is no longer the only option to run
the DRM unit tests, as the tests can be run through kunit-tool or on
real hardware with CONFIG_KUNIT.

Therefore, remove the "igt_" prefix from the tests and replace it with
the "test_drm_" prefix, making the tests' names independent from the tool
used.

Signed-off-by: Maíra Canal <mairacanal@riseup.net>
---
v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
- Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
---
 drivers/gpu/drm/tests/drm_buddy_test.c        |  84 +++++-----
 .../gpu/drm/tests/drm_cmdline_parser_test.c   | 156 +++++++++---------
 .../gpu/drm/tests/drm_damage_helper_test.c    |  84 +++++-----
 .../gpu/drm/tests/drm_dp_mst_helper_test.c    |   8 +-
 .../gpu/drm/tests/drm_format_helper_test.c    |   8 +-
 drivers/gpu/drm/tests/drm_format_test.c       |  20 +--
 drivers/gpu/drm/tests/drm_mm_test.c           | 155 ++++++++---------
 drivers/gpu/drm/tests/drm_plane_helper_test.c |   4 +-
 drivers/gpu/drm/tests/drm_rect_test.c         |  16 +-
 9 files changed, 268 insertions(+), 267 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
index d76f83833e75..ac0d18c60d72 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -13,7 +13,7 @@
 
 #include "../lib/drm_random.h"
 
-#define IGT_TIMEOUT(name__)                                                    \
+#define TIMEOUT(name__)								\
 	unsigned long name__ = jiffies + MAX_SCHEDULE_TIMEOUT
 
 static unsigned int random_seed;
@@ -24,7 +24,7 @@ static inline u64 get_size(int order, u64 chunk_size)
 }
 
 __printf(2, 3)
-static bool __igt_timeout(unsigned long timeout, const char *fmt, ...)
+static bool __timeout(unsigned long timeout, const char *fmt, ...)
 {
 	va_list va;
 
@@ -43,8 +43,8 @@ static bool __igt_timeout(unsigned long timeout, const char *fmt, ...)
 	return true;
 }
 
-static void __igt_dump_block(struct kunit *test, struct drm_buddy *mm,
-			     struct drm_buddy_block *block, bool buddy)
+static void __dump_block(struct kunit *test, struct drm_buddy *mm,
+			 struct drm_buddy_block *block, bool buddy)
 {
 	kunit_err(test, "block info: header=%llx, state=%u, order=%d, offset=%llx size=%llx root=%d buddy=%d\n",
 		  block->header, drm_buddy_block_state(block),
@@ -52,20 +52,20 @@ static void __igt_dump_block(struct kunit *test, struct drm_buddy *mm,
 			  drm_buddy_block_size(mm, block), !block->parent, buddy);
 }
 
-static void igt_dump_block(struct kunit *test, struct drm_buddy *mm,
-			   struct drm_buddy_block *block)
+static void dump_block(struct kunit *test, struct drm_buddy *mm,
+		       struct drm_buddy_block *block)
 {
 	struct drm_buddy_block *buddy;
 
-	__igt_dump_block(test, mm, block, false);
+	__dump_block(test, mm, block, false);
 
 	buddy = drm_get_buddy(block);
 	if (buddy)
-		__igt_dump_block(test, mm, buddy, true);
+		__dump_block(test, mm, buddy, true);
 }
 
-static int igt_check_block(struct kunit *test, struct drm_buddy *mm,
-			   struct drm_buddy_block *block)
+static int check_block(struct kunit *test, struct drm_buddy *mm,
+		       struct drm_buddy_block *block)
 {
 	struct drm_buddy_block *buddy;
 	unsigned int block_state;
@@ -137,8 +137,8 @@ static int igt_check_block(struct kunit *test, struct drm_buddy *mm,
 	return err;
 }
 
-static int igt_check_blocks(struct kunit *test, struct drm_buddy *mm,
-			    struct list_head *blocks, u64 expected_size, bool is_contiguous)
+static int check_blocks(struct kunit *test, struct drm_buddy *mm,
+			struct list_head *blocks, u64 expected_size, bool is_contiguous)
 {
 	struct drm_buddy_block *block;
 	struct drm_buddy_block *prev;
@@ -150,7 +150,7 @@ static int igt_check_blocks(struct kunit *test, struct drm_buddy *mm,
 	total = 0;
 
 	list_for_each_entry(block, blocks, link) {
-		err = igt_check_block(test, mm, block);
+		err = check_block(test, mm, block);
 
 		if (!drm_buddy_block_is_allocated(block)) {
 			kunit_err(test, "block not allocated\n");
@@ -190,16 +190,16 @@ static int igt_check_blocks(struct kunit *test, struct drm_buddy *mm,
 
 	if (prev) {
 		kunit_err(test, "prev block, dump:\n");
-		igt_dump_block(test, mm, prev);
+		dump_block(test, mm, prev);
 	}
 
 	kunit_err(test, "bad block, dump:\n");
-	igt_dump_block(test, mm, block);
+	dump_block(test, mm, block);
 
 	return err;
 }
 
-static int igt_check_mm(struct kunit *test, struct drm_buddy *mm)
+static int check_mm(struct kunit *test, struct drm_buddy *mm)
 {
 	struct drm_buddy_block *root;
 	struct drm_buddy_block *prev;
@@ -233,7 +233,7 @@ static int igt_check_mm(struct kunit *test, struct drm_buddy *mm)
 			break;
 		}
 
-		err = igt_check_block(test, mm, root);
+		err = check_block(test, mm, root);
 
 		if (!drm_buddy_block_is_free(root)) {
 			kunit_err(test, "root not free\n");
@@ -289,18 +289,18 @@ static int igt_check_mm(struct kunit *test, struct drm_buddy *mm)
 
 	if (prev) {
 		kunit_err(test, "prev root(%u), dump:\n", i - 1);
-		igt_dump_block(test, mm, prev);
+		dump_block(test, mm, prev);
 	}
 
 	if (root) {
 		kunit_err(test, "bad root(%u), dump:\n", i);
-		igt_dump_block(test, mm, root);
+		dump_block(test, mm, root);
 	}
 
 	return err;
 }
 
-static void igt_mm_config(u64 *size, u64 *chunk_size)
+static void mm_config(u64 *size, u64 *chunk_size)
 {
 	DRM_RND_STATE(prng, random_seed);
 	u32 s, ms;
@@ -321,7 +321,7 @@ static void igt_mm_config(u64 *size, u64 *chunk_size)
 	*size = (u64)s << 12;
 }
 
-static void igt_buddy_alloc_pathological(struct kunit *test)
+static void test_drm_buddy_alloc_pathological(struct kunit *test)
 {
 	u64 mm_size, size, start = 0;
 	struct drm_buddy_block *block;
@@ -402,7 +402,7 @@ static void igt_buddy_alloc_pathological(struct kunit *test)
 	drm_buddy_fini(&mm);
 }
 
-static void igt_buddy_alloc_smoke(struct kunit *test)
+static void test_drm_buddy_alloc_smoke(struct kunit *test)
 {
 	u64 mm_size, chunk_size, start = 0;
 	unsigned long flags = 0;
@@ -411,9 +411,9 @@ static void igt_buddy_alloc_smoke(struct kunit *test)
 	int i;
 
 	DRM_RND_STATE(prng, random_seed);
-	IGT_TIMEOUT(end_time);
+	TIMEOUT(end_time);
 
-	igt_mm_config(&mm_size, &chunk_size);
+	mm_config(&mm_size, &chunk_size);
 
 	KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, chunk_size),
 			       "buddy_init failed\n");
@@ -430,7 +430,7 @@ static void igt_buddy_alloc_smoke(struct kunit *test)
 		LIST_HEAD(tmp);
 		int order, err;
 
-		KUNIT_ASSERT_FALSE_MSG(test, igt_check_mm(test, &mm),
+		KUNIT_ASSERT_FALSE_MSG(test, check_mm(test, &mm),
 				       "pre-mm check failed, abort\n");
 
 		order = max_order;
@@ -466,19 +466,19 @@ static void igt_buddy_alloc_smoke(struct kunit *test)
 
 			total += drm_buddy_block_size(&mm, block);
 
-			if (__igt_timeout(end_time, NULL)) {
+			if (__timeout(end_time, NULL)) {
 				timeout = true;
 				break;
 			}
 		} while (total < mm.size);
 
 		if (!err)
-			err = igt_check_blocks(test, &mm, &blocks, total, false);
+			err = check_blocks(test, &mm, &blocks, total, false);
 
 		drm_buddy_free_list(&mm, &blocks);
 
 		if (!err) {
-			KUNIT_EXPECT_FALSE_MSG(test, igt_check_mm(test, &mm),
+			KUNIT_EXPECT_FALSE_MSG(test, check_mm(test, &mm),
 					       "post-mm check failed\n");
 		}
 
@@ -492,7 +492,7 @@ static void igt_buddy_alloc_smoke(struct kunit *test)
 	drm_buddy_fini(&mm);
 }
 
-static void igt_buddy_alloc_pessimistic(struct kunit *test)
+static void test_drm_buddy_alloc_pessimistic(struct kunit *test)
 {
 	u64 mm_size, size, start = 0;
 	struct drm_buddy_block *block, *bn;
@@ -587,7 +587,7 @@ static void igt_buddy_alloc_pessimistic(struct kunit *test)
 	drm_buddy_fini(&mm);
 }
 
-static void igt_buddy_alloc_optimistic(struct kunit *test)
+static void test_drm_buddy_alloc_optimistic(struct kunit *test)
 {
 	u64 mm_size, size, start = 0;
 	struct drm_buddy_block *block;
@@ -633,7 +633,7 @@ static void igt_buddy_alloc_optimistic(struct kunit *test)
 	drm_buddy_fini(&mm);
 }
 
-static void igt_buddy_alloc_range(struct kunit *test)
+static void test_drm_buddy_alloc_range(struct kunit *test)
 {
 	unsigned long flags = DRM_BUDDY_RANGE_ALLOCATION;
 	u64 offset, size, rem, chunk_size, end;
@@ -641,12 +641,12 @@ static void igt_buddy_alloc_range(struct kunit *test)
 	struct drm_buddy mm;
 	LIST_HEAD(blocks);
 
-	igt_mm_config(&size, &chunk_size);
+	mm_config(&size, &chunk_size);
 
 	KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, size, chunk_size),
 			       "buddy_init failed");
 
-	KUNIT_ASSERT_FALSE_MSG(test, igt_check_mm(test, &mm),
+	KUNIT_ASSERT_FALSE_MSG(test, check_mm(test, &mm),
 			       "pre-mm check failed, abort!");
 
 	rem = mm.size;
@@ -671,7 +671,7 @@ static void igt_buddy_alloc_range(struct kunit *test)
 				    "alloc_range start offset mismatch, found=%llx, expected=%llx\n",
 							drm_buddy_block_offset(block), offset);
 
-		KUNIT_ASSERT_FALSE(test, igt_check_blocks(test, &mm, &tmp, size, true));
+		KUNIT_ASSERT_FALSE(test, check_blocks(test, &mm, &tmp, size, true));
 
 		list_splice_tail(&tmp, &blocks);
 
@@ -686,12 +686,12 @@ static void igt_buddy_alloc_range(struct kunit *test)
 
 	drm_buddy_free_list(&mm, &blocks);
 
-	KUNIT_EXPECT_FALSE_MSG(test, igt_check_mm(test, &mm), "post-mm check failed\n");
+	KUNIT_EXPECT_FALSE_MSG(test, check_mm(test, &mm), "post-mm check failed\n");
 
 	drm_buddy_fini(&mm);
 }
 
-static void igt_buddy_alloc_limit(struct kunit *test)
+static void test_drm_buddy_alloc_limit(struct kunit *test)
 {
 	u64 size = U64_MAX, start = 0;
 	struct drm_buddy_block *block;
@@ -735,12 +735,12 @@ static int drm_buddy_init_test(struct kunit *test)
 }
 
 static struct kunit_case drm_buddy_tests[] = {
-	KUNIT_CASE(igt_buddy_alloc_limit),
-	KUNIT_CASE(igt_buddy_alloc_range),
-	KUNIT_CASE(igt_buddy_alloc_optimistic),
-	KUNIT_CASE(igt_buddy_alloc_pessimistic),
-	KUNIT_CASE(igt_buddy_alloc_smoke),
-	KUNIT_CASE(igt_buddy_alloc_pathological),
+	KUNIT_CASE(test_drm_buddy_alloc_limit),
+	KUNIT_CASE(test_drm_buddy_alloc_range),
+	KUNIT_CASE(test_drm_buddy_alloc_optimistic),
+	KUNIT_CASE(test_drm_buddy_alloc_pessimistic),
+	KUNIT_CASE(test_drm_buddy_alloc_smoke),
+	KUNIT_CASE(test_drm_buddy_alloc_pathological),
 	{}
 };
 
diff --git a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
index 09b806e27506..6cc58f7518a1 100644
--- a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
+++ b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
@@ -11,7 +11,7 @@
 
 static const struct drm_connector no_connector = {};
 
-static void drm_cmdline_test_force_e_only(struct kunit *test)
+static void test_drm_cmdline_force_e_only(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "e";
@@ -29,7 +29,7 @@ static void drm_cmdline_test_force_e_only(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON);
 }
 
-static void drm_cmdline_test_force_D_only_not_digital(struct kunit *test)
+static void test_drm_cmdline_force_D_only_not_digital(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "D";
@@ -51,7 +51,7 @@ static const struct drm_connector connector_hdmi = {
 	.connector_type	= DRM_MODE_CONNECTOR_HDMIB,
 };
 
-static void drm_cmdline_test_force_D_only_hdmi(struct kunit *test)
+static void test_drm_cmdline_force_D_only_hdmi(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "D";
@@ -73,7 +73,7 @@ static const struct drm_connector connector_dvi = {
 	.connector_type	= DRM_MODE_CONNECTOR_DVII,
 };
 
-static void drm_cmdline_test_force_D_only_dvi(struct kunit *test)
+static void test_drm_cmdline_force_D_only_dvi(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "D";
@@ -91,7 +91,7 @@ static void drm_cmdline_test_force_D_only_dvi(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON_DIGITAL);
 }
 
-static void drm_cmdline_test_force_d_only(struct kunit *test)
+static void test_drm_cmdline_force_d_only(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "d";
@@ -109,7 +109,7 @@ static void drm_cmdline_test_force_d_only(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_OFF);
 }
 
-static void drm_cmdline_test_res(struct kunit *test)
+static void test_drm_cmdline_res(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480";
@@ -131,7 +131,7 @@ static void drm_cmdline_test_res(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_vesa(struct kunit *test)
+static void test_drm_cmdline_res_vesa(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480M";
@@ -153,7 +153,7 @@ static void drm_cmdline_test_res_vesa(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_vesa_rblank(struct kunit *test)
+static void test_drm_cmdline_res_vesa_rblank(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480MR";
@@ -175,7 +175,7 @@ static void drm_cmdline_test_res_vesa_rblank(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_rblank(struct kunit *test)
+static void test_drm_cmdline_res_rblank(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480R";
@@ -197,7 +197,7 @@ static void drm_cmdline_test_res_rblank(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_bpp(struct kunit *test)
+static void test_drm_cmdline_res_bpp(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24";
@@ -220,7 +220,7 @@ static void drm_cmdline_test_res_bpp(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_refresh(struct kunit *test)
+static void test_drm_cmdline_res_refresh(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480@60";
@@ -243,7 +243,7 @@ static void drm_cmdline_test_res_refresh(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_bpp_refresh(struct kunit *test)
+static void test_drm_cmdline_res_bpp_refresh(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24@60";
@@ -267,7 +267,7 @@ static void drm_cmdline_test_res_bpp_refresh(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_bpp_refresh_interlaced(struct kunit *test)
+static void test_drm_cmdline_res_bpp_refresh_interlaced(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24@60i";
@@ -291,7 +291,7 @@ static void drm_cmdline_test_res_bpp_refresh_interlaced(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_bpp_refresh_margins(struct kunit *test)
+static void test_drm_cmdline_res_bpp_refresh_margins(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24@60m";
@@ -315,7 +315,7 @@ static void drm_cmdline_test_res_bpp_refresh_margins(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_res_bpp_refresh_force_off(struct kunit *test)
+static void test_drm_cmdline_res_bpp_refresh_force_off(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24@60d";
@@ -339,7 +339,7 @@ static void drm_cmdline_test_res_bpp_refresh_force_off(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_OFF);
 }
 
-static void drm_cmdline_test_res_bpp_refresh_force_on(struct kunit *test)
+static void test_drm_cmdline_res_bpp_refresh_force_on(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24@60e";
@@ -363,7 +363,7 @@ static void drm_cmdline_test_res_bpp_refresh_force_on(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON);
 }
 
-static void drm_cmdline_test_res_bpp_refresh_force_on_analog(struct kunit *test)
+static void test_drm_cmdline_res_bpp_refresh_force_on_analog(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24@60D";
@@ -387,7 +387,7 @@ static void drm_cmdline_test_res_bpp_refresh_force_on_analog(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON);
 }
 
-static void drm_cmdline_test_res_bpp_refresh_force_on_digital(struct kunit *test)
+static void test_drm_cmdline_res_bpp_refresh_force_on_digital(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	static const struct drm_connector connector = {
@@ -414,7 +414,7 @@ static void drm_cmdline_test_res_bpp_refresh_force_on_digital(struct kunit *test
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON_DIGITAL);
 }
 
-static void drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on(struct kunit *test)
+static void test_drm_cmdline_res_bpp_refresh_interlaced_margins_force_on(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24@60ime";
@@ -438,7 +438,7 @@ static void drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on(struct
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON);
 }
 
-static void drm_cmdline_test_res_margins_force_on(struct kunit *test)
+static void test_drm_cmdline_res_margins_force_on(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480me";
@@ -460,7 +460,7 @@ static void drm_cmdline_test_res_margins_force_on(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON);
 }
 
-static void drm_cmdline_test_res_vesa_margins(struct kunit *test)
+static void test_drm_cmdline_res_vesa_margins(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480Mm";
@@ -482,7 +482,7 @@ static void drm_cmdline_test_res_vesa_margins(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_name(struct kunit *test)
+static void test_drm_cmdline_name(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "NTSC";
@@ -494,7 +494,7 @@ static void drm_cmdline_test_name(struct kunit *test)
 	KUNIT_EXPECT_FALSE(test, mode.bpp_specified);
 }
 
-static void drm_cmdline_test_name_bpp(struct kunit *test)
+static void test_drm_cmdline_name_bpp(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "NTSC-24";
@@ -509,7 +509,7 @@ static void drm_cmdline_test_name_bpp(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.bpp, 24);
 }
 
-static void drm_cmdline_test_name_option(struct kunit *test)
+static void test_drm_cmdline_name_option(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "NTSC,rotate=180";
@@ -521,7 +521,7 @@ static void drm_cmdline_test_name_option(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.rotation_reflection, DRM_MODE_ROTATE_180);
 }
 
-static void drm_cmdline_test_name_bpp_option(struct kunit *test)
+static void test_drm_cmdline_name_bpp_option(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "NTSC-24,rotate=180";
@@ -535,7 +535,7 @@ static void drm_cmdline_test_name_bpp_option(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.bpp, 24);
 }
 
-static void drm_cmdline_test_rotate_0(struct kunit *test)
+static void test_drm_cmdline_rotate_0(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480,rotate=0";
@@ -558,7 +558,7 @@ static void drm_cmdline_test_rotate_0(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_rotate_90(struct kunit *test)
+static void test_drm_cmdline_rotate_90(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480,rotate=90";
@@ -581,7 +581,7 @@ static void drm_cmdline_test_rotate_90(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_rotate_180(struct kunit *test)
+static void test_drm_cmdline_rotate_180(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480,rotate=180";
@@ -604,7 +604,7 @@ static void drm_cmdline_test_rotate_180(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_rotate_270(struct kunit *test)
+static void test_drm_cmdline_rotate_270(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480,rotate=270";
@@ -627,7 +627,7 @@ static void drm_cmdline_test_rotate_270(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_hmirror(struct kunit *test)
+static void test_drm_cmdline_hmirror(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480,reflect_x";
@@ -650,7 +650,7 @@ static void drm_cmdline_test_hmirror(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_vmirror(struct kunit *test)
+static void test_drm_cmdline_vmirror(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480,reflect_y";
@@ -673,7 +673,7 @@ static void drm_cmdline_test_vmirror(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_margin_options(struct kunit *test)
+static void test_drm_cmdline_margin_options(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline =
@@ -700,7 +700,7 @@ static void drm_cmdline_test_margin_options(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_multiple_options(struct kunit *test)
+static void test_drm_cmdline_multiple_options(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480,rotate=270,reflect_x";
@@ -723,7 +723,7 @@ static void drm_cmdline_test_multiple_options(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_bpp_extra_and_option(struct kunit *test)
+static void test_drm_cmdline_bpp_extra_and_option(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480-24e,rotate=180";
@@ -747,7 +747,7 @@ static void drm_cmdline_test_bpp_extra_and_option(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON);
 }
 
-static void drm_cmdline_test_extra_and_option(struct kunit *test)
+static void test_drm_cmdline_extra_and_option(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "720x480e,rotate=180";
@@ -769,7 +769,7 @@ static void drm_cmdline_test_extra_and_option(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON);
 }
 
-static void drm_cmdline_test_freestanding_options(struct kunit *test)
+static void test_drm_cmdline_freestanding_options(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "margin_right=14,margin_left=24,margin_bottom=36,margin_top=42";
@@ -792,7 +792,7 @@ static void drm_cmdline_test_freestanding_options(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
 }
 
-static void drm_cmdline_test_freestanding_force_e_and_options(struct kunit *test)
+static void test_drm_cmdline_freestanding_force_e_and_options(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "e,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42";
@@ -815,7 +815,7 @@ static void drm_cmdline_test_freestanding_force_e_and_options(struct kunit *test
 	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_ON);
 }
 
-static void drm_cmdline_test_panel_orientation(struct kunit *test)
+static void test_drm_cmdline_panel_orientation(struct kunit *test)
 {
 	struct drm_cmdline_mode mode = { };
 	const char *cmdline = "panel_orientation=upside_down";
@@ -840,7 +840,7 @@ struct drm_cmdline_invalid_test {
 	const char *cmdline;
 };
 
-static void drm_cmdline_test_invalid(struct kunit *test)
+static void test_drm_cmdline_invalid(struct kunit *test)
 {
 	const struct drm_cmdline_invalid_test *params = test->param_value;
 	struct drm_cmdline_mode mode = { };
@@ -938,45 +938,45 @@ static void drm_cmdline_invalid_desc(const struct drm_cmdline_invalid_test *t,
 KUNIT_ARRAY_PARAM(drm_cmdline_invalid, drm_cmdline_invalid_tests, drm_cmdline_invalid_desc);
 
 static struct kunit_case drm_cmdline_parser_tests[] = {
-	KUNIT_CASE(drm_cmdline_test_force_d_only),
-	KUNIT_CASE(drm_cmdline_test_force_D_only_dvi),
-	KUNIT_CASE(drm_cmdline_test_force_D_only_hdmi),
-	KUNIT_CASE(drm_cmdline_test_force_D_only_not_digital),
-	KUNIT_CASE(drm_cmdline_test_force_e_only),
-	KUNIT_CASE(drm_cmdline_test_res),
-	KUNIT_CASE(drm_cmdline_test_res_vesa),
-	KUNIT_CASE(drm_cmdline_test_res_vesa_rblank),
-	KUNIT_CASE(drm_cmdline_test_res_rblank),
-	KUNIT_CASE(drm_cmdline_test_res_bpp),
-	KUNIT_CASE(drm_cmdline_test_res_refresh),
-	KUNIT_CASE(drm_cmdline_test_res_bpp_refresh),
-	KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_interlaced),
-	KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_margins),
-	KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_off),
-	KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_on),
-	KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_on_analog),
-	KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_on_digital),
-	KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on),
-	KUNIT_CASE(drm_cmdline_test_res_margins_force_on),
-	KUNIT_CASE(drm_cmdline_test_res_vesa_margins),
-	KUNIT_CASE(drm_cmdline_test_name),
-	KUNIT_CASE(drm_cmdline_test_name_bpp),
-	KUNIT_CASE(drm_cmdline_test_name_option),
-	KUNIT_CASE(drm_cmdline_test_name_bpp_option),
-	KUNIT_CASE(drm_cmdline_test_rotate_0),
-	KUNIT_CASE(drm_cmdline_test_rotate_90),
-	KUNIT_CASE(drm_cmdline_test_rotate_180),
-	KUNIT_CASE(drm_cmdline_test_rotate_270),
-	KUNIT_CASE(drm_cmdline_test_hmirror),
-	KUNIT_CASE(drm_cmdline_test_vmirror),
-	KUNIT_CASE(drm_cmdline_test_margin_options),
-	KUNIT_CASE(drm_cmdline_test_multiple_options),
-	KUNIT_CASE(drm_cmdline_test_bpp_extra_and_option),
-	KUNIT_CASE(drm_cmdline_test_extra_and_option),
-	KUNIT_CASE(drm_cmdline_test_freestanding_options),
-	KUNIT_CASE(drm_cmdline_test_freestanding_force_e_and_options),
-	KUNIT_CASE(drm_cmdline_test_panel_orientation),
-	KUNIT_CASE_PARAM(drm_cmdline_test_invalid, drm_cmdline_invalid_gen_params),
+	KUNIT_CASE(test_drm_cmdline_force_d_only),
+	KUNIT_CASE(test_drm_cmdline_force_D_only_dvi),
+	KUNIT_CASE(test_drm_cmdline_force_D_only_hdmi),
+	KUNIT_CASE(test_drm_cmdline_force_D_only_not_digital),
+	KUNIT_CASE(test_drm_cmdline_force_e_only),
+	KUNIT_CASE(test_drm_cmdline_res),
+	KUNIT_CASE(test_drm_cmdline_res_vesa),
+	KUNIT_CASE(test_drm_cmdline_res_vesa_rblank),
+	KUNIT_CASE(test_drm_cmdline_res_rblank),
+	KUNIT_CASE(test_drm_cmdline_res_bpp),
+	KUNIT_CASE(test_drm_cmdline_res_refresh),
+	KUNIT_CASE(test_drm_cmdline_res_bpp_refresh),
+	KUNIT_CASE(test_drm_cmdline_res_bpp_refresh_interlaced),
+	KUNIT_CASE(test_drm_cmdline_res_bpp_refresh_margins),
+	KUNIT_CASE(test_drm_cmdline_res_bpp_refresh_force_off),
+	KUNIT_CASE(test_drm_cmdline_res_bpp_refresh_force_on),
+	KUNIT_CASE(test_drm_cmdline_res_bpp_refresh_force_on_analog),
+	KUNIT_CASE(test_drm_cmdline_res_bpp_refresh_force_on_digital),
+	KUNIT_CASE(test_drm_cmdline_res_bpp_refresh_interlaced_margins_force_on),
+	KUNIT_CASE(test_drm_cmdline_res_margins_force_on),
+	KUNIT_CASE(test_drm_cmdline_res_vesa_margins),
+	KUNIT_CASE(test_drm_cmdline_name),
+	KUNIT_CASE(test_drm_cmdline_name_bpp),
+	KUNIT_CASE(test_drm_cmdline_name_option),
+	KUNIT_CASE(test_drm_cmdline_name_bpp_option),
+	KUNIT_CASE(test_drm_cmdline_rotate_0),
+	KUNIT_CASE(test_drm_cmdline_rotate_90),
+	KUNIT_CASE(test_drm_cmdline_rotate_180),
+	KUNIT_CASE(test_drm_cmdline_rotate_270),
+	KUNIT_CASE(test_drm_cmdline_hmirror),
+	KUNIT_CASE(test_drm_cmdline_vmirror),
+	KUNIT_CASE(test_drm_cmdline_margin_options),
+	KUNIT_CASE(test_drm_cmdline_multiple_options),
+	KUNIT_CASE(test_drm_cmdline_bpp_extra_and_option),
+	KUNIT_CASE(test_drm_cmdline_extra_and_option),
+	KUNIT_CASE(test_drm_cmdline_freestanding_options),
+	KUNIT_CASE(test_drm_cmdline_freestanding_force_e_and_options),
+	KUNIT_CASE(test_drm_cmdline_panel_orientation),
+	KUNIT_CASE_PARAM(test_drm_cmdline_invalid, drm_cmdline_invalid_gen_params),
 	{}
 };
 
diff --git a/drivers/gpu/drm/tests/drm_damage_helper_test.c b/drivers/gpu/drm/tests/drm_damage_helper_test.c
index bf250bd08d7e..246c425094cf 100644
--- a/drivers/gpu/drm/tests/drm_damage_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_damage_helper_test.c
@@ -111,7 +111,7 @@ static void check_damage_clip(struct kunit *test, struct drm_rect *r,
 			   r->x1, r->y1, r->x2, r->y2, x1, y1, x2, y2);
 }
 
-static void igt_damage_iter_no_damage(struct kunit *test)
+static void test_drm_damage_iter_no_damage(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -129,7 +129,7 @@ static void igt_damage_iter_no_damage(struct kunit *test)
 	check_damage_clip(test, &clip, 0, 0, 2048, 2048);
 }
 
-static void igt_damage_iter_no_damage_fractional_src(struct kunit *test)
+static void test_drm_damage_iter_no_damage_fractional_src(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -150,7 +150,7 @@ static void igt_damage_iter_no_damage_fractional_src(struct kunit *test)
 	check_damage_clip(test, &clip, 3, 3, 1028, 772);
 }
 
-static void igt_damage_iter_no_damage_src_moved(struct kunit *test)
+static void test_drm_damage_iter_no_damage_src_moved(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -169,7 +169,7 @@ static void igt_damage_iter_no_damage_src_moved(struct kunit *test)
 	check_damage_clip(test, &clip, 10, 10, 1034, 778);
 }
 
-static void igt_damage_iter_no_damage_fractional_src_moved(struct kunit *test)
+static void test_drm_damage_iter_no_damage_fractional_src_moved(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -189,7 +189,7 @@ static void igt_damage_iter_no_damage_fractional_src_moved(struct kunit *test)
 	check_damage_clip(test, &clip, 4, 4, 1029, 773);
 }
 
-static void igt_damage_iter_no_damage_not_visible(struct kunit *test)
+static void test_drm_damage_iter_no_damage_not_visible(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -207,7 +207,7 @@ static void igt_damage_iter_no_damage_not_visible(struct kunit *test)
 	KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should have no damage.");
 }
 
-static void igt_damage_iter_no_damage_no_crtc(struct kunit *test)
+static void test_drm_damage_iter_no_damage_no_crtc(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -225,7 +225,7 @@ static void igt_damage_iter_no_damage_no_crtc(struct kunit *test)
 	KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should have no damage.");
 }
 
-static void igt_damage_iter_no_damage_no_fb(struct kunit *test)
+static void test_drm_damage_iter_no_damage_no_fb(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -243,7 +243,7 @@ static void igt_damage_iter_no_damage_no_fb(struct kunit *test)
 	KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should have no damage.");
 }
 
-static void igt_damage_iter_simple_damage(struct kunit *test)
+static void test_drm_damage_iter_simple_damage(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -266,7 +266,7 @@ static void igt_damage_iter_simple_damage(struct kunit *test)
 	check_damage_clip(test, &clip, 0, 0, 1024, 768);
 }
 
-static void igt_damage_iter_single_damage(struct kunit *test)
+static void test_drm_damage_iter_single_damage(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -288,7 +288,7 @@ static void igt_damage_iter_single_damage(struct kunit *test)
 	check_damage_clip(test, &clip, 256, 192, 768, 576);
 }
 
-static void igt_damage_iter_single_damage_intersect_src(struct kunit *test)
+static void test_drm_damage_iter_single_damage_intersect_src(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -311,7 +311,7 @@ static void igt_damage_iter_single_damage_intersect_src(struct kunit *test)
 	check_damage_clip(test, &clip, 256, 192, 1024, 768);
 }
 
-static void igt_damage_iter_single_damage_outside_src(struct kunit *test)
+static void test_drm_damage_iter_single_damage_outside_src(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -333,7 +333,7 @@ static void igt_damage_iter_single_damage_outside_src(struct kunit *test)
 	KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should have no damage.");
 }
 
-static void igt_damage_iter_single_damage_fractional_src(struct kunit *test)
+static void test_drm_damage_iter_single_damage_fractional_src(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -358,7 +358,7 @@ static void igt_damage_iter_single_damage_fractional_src(struct kunit *test)
 	check_damage_clip(test, &clip, 10, 10, 256, 330);
 }
 
-static void igt_damage_iter_single_damage_intersect_fractional_src(struct kunit *test)
+static void test_drm_damage_iter_single_damage_intersect_fractional_src(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -385,7 +385,7 @@ static void igt_damage_iter_single_damage_intersect_fractional_src(struct kunit
 	check_damage_clip(test, &clip, 10, 4, 1029, 330);
 }
 
-static void igt_damage_iter_single_damage_outside_fractional_src(struct kunit *test)
+static void test_drm_damage_iter_single_damage_outside_fractional_src(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -410,7 +410,7 @@ static void igt_damage_iter_single_damage_outside_fractional_src(struct kunit *t
 	KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should have no damage.");
 }
 
-static void igt_damage_iter_single_damage_src_moved(struct kunit *test)
+static void test_drm_damage_iter_single_damage_src_moved(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -435,7 +435,7 @@ static void igt_damage_iter_single_damage_src_moved(struct kunit *test)
 	check_damage_clip(test, &clip, 10, 10, 1034, 778);
 }
 
-static void igt_damage_iter_single_damage_fractional_src_moved(struct kunit *test)
+static void test_drm_damage_iter_single_damage_fractional_src_moved(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -462,7 +462,7 @@ static void igt_damage_iter_single_damage_fractional_src_moved(struct kunit *tes
 	check_damage_clip(test, &clip, 4, 4, 1029, 773);
 }
 
-static void igt_damage_iter_damage(struct kunit *test)
+static void test_drm_damage_iter_damage(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -490,7 +490,7 @@ static void igt_damage_iter_damage(struct kunit *test)
 	KUNIT_EXPECT_EQ_MSG(test, num_hits, 2, "Should return damage when set.");
 }
 
-static void igt_damage_iter_damage_one_intersect(struct kunit *test)
+static void test_drm_damage_iter_damage_one_intersect(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -520,7 +520,7 @@ static void igt_damage_iter_damage_one_intersect(struct kunit *test)
 	KUNIT_EXPECT_EQ_MSG(test, num_hits, 2, "Should return damage when set.");
 }
 
-static void igt_damage_iter_damage_one_outside(struct kunit *test)
+static void test_drm_damage_iter_damage_one_outside(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -544,7 +544,7 @@ static void igt_damage_iter_damage_one_outside(struct kunit *test)
 	check_damage_clip(test, &clip, 240, 200, 280, 250);
 }
 
-static void igt_damage_iter_damage_src_moved(struct kunit *test)
+static void test_drm_damage_iter_damage_src_moved(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -571,7 +571,7 @@ static void igt_damage_iter_damage_src_moved(struct kunit *test)
 	check_damage_clip(test, &clip, 3, 3, 1028, 772);
 }
 
-static void igt_damage_iter_damage_not_visible(struct kunit *test)
+static void test_drm_damage_iter_damage_not_visible(struct kunit *test)
 {
 	struct drm_damage_mock *mock = test->priv;
 	struct drm_atomic_helper_damage_iter iter;
@@ -599,27 +599,27 @@ static void igt_damage_iter_damage_not_visible(struct kunit *test)
 }
 
 static struct kunit_case drm_damage_helper_tests[] = {
-	KUNIT_CASE(igt_damage_iter_no_damage),
-	KUNIT_CASE(igt_damage_iter_no_damage_fractional_src),
-	KUNIT_CASE(igt_damage_iter_no_damage_src_moved),
-	KUNIT_CASE(igt_damage_iter_no_damage_fractional_src_moved),
-	KUNIT_CASE(igt_damage_iter_no_damage_not_visible),
-	KUNIT_CASE(igt_damage_iter_no_damage_no_crtc),
-	KUNIT_CASE(igt_damage_iter_no_damage_no_fb),
-	KUNIT_CASE(igt_damage_iter_simple_damage),
-	KUNIT_CASE(igt_damage_iter_single_damage),
-	KUNIT_CASE(igt_damage_iter_single_damage_intersect_src),
-	KUNIT_CASE(igt_damage_iter_single_damage_outside_src),
-	KUNIT_CASE(igt_damage_iter_single_damage_fractional_src),
-	KUNIT_CASE(igt_damage_iter_single_damage_intersect_fractional_src),
-	KUNIT_CASE(igt_damage_iter_single_damage_outside_fractional_src),
-	KUNIT_CASE(igt_damage_iter_single_damage_src_moved),
-	KUNIT_CASE(igt_damage_iter_single_damage_fractional_src_moved),
-	KUNIT_CASE(igt_damage_iter_damage),
-	KUNIT_CASE(igt_damage_iter_damage_one_intersect),
-	KUNIT_CASE(igt_damage_iter_damage_one_outside),
-	KUNIT_CASE(igt_damage_iter_damage_src_moved),
-	KUNIT_CASE(igt_damage_iter_damage_not_visible),
+	KUNIT_CASE(test_drm_damage_iter_no_damage),
+	KUNIT_CASE(test_drm_damage_iter_no_damage_fractional_src),
+	KUNIT_CASE(test_drm_damage_iter_no_damage_src_moved),
+	KUNIT_CASE(test_drm_damage_iter_no_damage_fractional_src_moved),
+	KUNIT_CASE(test_drm_damage_iter_no_damage_not_visible),
+	KUNIT_CASE(test_drm_damage_iter_no_damage_no_crtc),
+	KUNIT_CASE(test_drm_damage_iter_no_damage_no_fb),
+	KUNIT_CASE(test_drm_damage_iter_simple_damage),
+	KUNIT_CASE(test_drm_damage_iter_single_damage),
+	KUNIT_CASE(test_drm_damage_iter_single_damage_intersect_src),
+	KUNIT_CASE(test_drm_damage_iter_single_damage_outside_src),
+	KUNIT_CASE(test_drm_damage_iter_single_damage_fractional_src),
+	KUNIT_CASE(test_drm_damage_iter_single_damage_intersect_fractional_src),
+	KUNIT_CASE(test_drm_damage_iter_single_damage_outside_fractional_src),
+	KUNIT_CASE(test_drm_damage_iter_single_damage_src_moved),
+	KUNIT_CASE(test_drm_damage_iter_single_damage_fractional_src_moved),
+	KUNIT_CASE(test_drm_damage_iter_damage),
+	KUNIT_CASE(test_drm_damage_iter_damage_one_intersect),
+	KUNIT_CASE(test_drm_damage_iter_damage_one_outside),
+	KUNIT_CASE(test_drm_damage_iter_damage_src_moved),
+	KUNIT_CASE(test_drm_damage_iter_damage_not_visible),
 	{ }
 };
 
diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
index 1d2fade56227..784bb1fab3f3 100644
--- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
@@ -16,7 +16,7 @@
 
 #include "../display/drm_dp_mst_topology_internal.h"
 
-static void igt_dp_mst_calc_pbn_mode(struct kunit *test)
+static void test_drm_dp_mst_calc_pbn_mode(struct kunit *test)
 {
 	int pbn, i;
 	const struct {
@@ -177,7 +177,7 @@ sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in)
 	return result;
 }
 
-static void igt_dp_mst_sideband_msg_req_decode(struct kunit *test)
+static void test_drm_dp_mst_sideband_msg_req_decode(struct kunit *test)
 {
 	struct drm_dp_sideband_msg_req_body in = { 0 };
 	u8 data[] = { 0xff, 0x0, 0xdd };
@@ -271,8 +271,8 @@ static void igt_dp_mst_sideband_msg_req_decode(struct kunit *test)
 }
 
 static struct kunit_case drm_dp_mst_helper_tests[] = {
-	KUNIT_CASE(igt_dp_mst_calc_pbn_mode),
-	KUNIT_CASE(igt_dp_mst_sideband_msg_req_decode),
+	KUNIT_CASE(test_drm_dp_mst_calc_pbn_mode),
+	KUNIT_CASE(test_drm_dp_mst_sideband_msg_req_decode),
 	{ }
 };
 
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 828487071796..2bdae72ea9a1 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -192,7 +192,7 @@ static void convert_xrgb8888_case_desc(struct convert_xrgb8888_case *t,
 KUNIT_ARRAY_PARAM(convert_xrgb8888, convert_xrgb8888_cases,
 		  convert_xrgb8888_case_desc);
 
-static void xrgb8888_to_rgb332_test(struct kunit *test)
+static void test_drm_fb_xrgb8888_to_rgb332(struct kunit *test)
 {
 	const struct convert_xrgb8888_case *params = test->param_value;
 	const struct convert_to_rgb332_result *result = &params->rgb332_result;
@@ -222,7 +222,7 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
 }
 
-static void xrgb8888_to_rgb565_test(struct kunit *test)
+static void test_drm_fb_xrgb8888_to_rgb565(struct kunit *test)
 {
 	const struct convert_xrgb8888_case *params = test->param_value;
 	const struct convert_to_rgb565_result *result = &params->rgb565_result;
@@ -256,8 +256,8 @@ static void xrgb8888_to_rgb565_test(struct kunit *test)
 }
 
 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),
+	KUNIT_CASE_PARAM(test_drm_fb_xrgb8888_to_rgb332, convert_xrgb8888_gen_params),
+	KUNIT_CASE_PARAM(test_drm_fb_xrgb8888_to_rgb565, convert_xrgb8888_gen_params),
 	{}
 };
 
diff --git a/drivers/gpu/drm/tests/drm_format_test.c b/drivers/gpu/drm/tests/drm_format_test.c
index afb4bca72187..5cde2ba867df 100644
--- a/drivers/gpu/drm/tests/drm_format_test.c
+++ b/drivers/gpu/drm/tests/drm_format_test.c
@@ -9,7 +9,7 @@
 
 #include <drm/drm_fourcc.h>
 
-static void igt_check_drm_format_block_width(struct kunit *test)
+static void test_drm_format_block_width(struct kunit *test)
 {
 	const struct drm_format_info *info = NULL;
 
@@ -50,7 +50,7 @@ static void igt_check_drm_format_block_width(struct kunit *test)
 	KUNIT_EXPECT_FALSE(test, drm_format_info_block_width(info, -1));
 }
 
-static void igt_check_drm_format_block_height(struct kunit *test)
+static void test_drm_format_block_height(struct kunit *test)
 {
 	const struct drm_format_info *info = NULL;
 
@@ -91,7 +91,7 @@ static void igt_check_drm_format_block_height(struct kunit *test)
 	KUNIT_EXPECT_FALSE(test, drm_format_info_block_height(info, -1));
 }
 
-static void igt_check_drm_format_min_pitch_for_single_plane(struct kunit *test)
+static void test_drm_format_min_pitch_for_single_plane(struct kunit *test)
 {
 	const struct drm_format_info *info = NULL;
 
@@ -177,7 +177,7 @@ static void igt_check_drm_format_min_pitch_for_single_plane(struct kunit *test)
 			(uint64_t)(UINT_MAX - 1) * 4);
 }
 
-static void igt_check_drm_format_min_pitch_for_multi_planar(struct kunit *test)
+static void test_drm_format_min_pitch_for_multi_planar(struct kunit *test)
 {
 	const struct drm_format_info *info = NULL;
 
@@ -256,7 +256,7 @@ 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 test_drm_format_min_pitch_for_tiled_format(struct kunit *test)
 {
 	const struct drm_format_info *info = NULL;
 
@@ -281,11 +281,11 @@ 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(test_drm_format_block_width),
+	KUNIT_CASE(test_drm_format_block_height),
+	KUNIT_CASE(test_drm_format_min_pitch_for_single_plane),
+	KUNIT_CASE(test_drm_format_min_pitch_for_multi_planar),
+	KUNIT_CASE(test_drm_format_min_pitch_for_tiled_format),
 	{ }
 };
 
diff --git a/drivers/gpu/drm/tests/drm_mm_test.c b/drivers/gpu/drm/tests/drm_mm_test.c
index 1e2c1aa524bd..33740c658f62 100644
--- a/drivers/gpu/drm/tests/drm_mm_test.c
+++ b/drivers/gpu/drm/tests/drm_mm_test.c
@@ -191,7 +191,7 @@ static bool assert_node(struct kunit *test, struct drm_mm_node *node, struct drm
 	return ok;
 }
 
-static void igt_mm_init(struct kunit *test)
+static void test_drm_mm_init(struct kunit *test)
 {
 	const unsigned int size = 4096;
 	struct drm_mm mm;
@@ -245,7 +245,7 @@ static void igt_mm_init(struct kunit *test)
 	drm_mm_takedown(&mm);
 }
 
-static void igt_mm_debug(struct kunit *test)
+static void test_drm_mm_debug(struct kunit *test)
 {
 	struct drm_mm mm;
 	struct drm_mm_node nodes[2];
@@ -341,7 +341,7 @@ static bool check_reserve_boundaries(struct kunit *test, struct drm_mm *mm,
 	return true;
 }
 
-static int __igt_reserve(struct kunit *test, unsigned int count, u64 size)
+static int __test_drm_mm_reserve(struct kunit *test, unsigned int count, u64 size)
 {
 	DRM_RND_STATE(prng, random_seed);
 	struct drm_mm mm;
@@ -349,7 +349,7 @@ static int __igt_reserve(struct kunit *test, unsigned int count, u64 size)
 	unsigned int *order, n, m, o = 0;
 	int ret, err;
 
-	/* For exercising drm_mm_reserve_node(struct kunit *test, ), we want to check that
+	/* For exercising drm_mm_reserve_node(), we want to check that
 	 * reservations outside of the drm_mm range are rejected, and to
 	 * overlapping and otherwise already occupied ranges. Afterwards,
 	 * the tree and nodes should be intact.
@@ -463,7 +463,7 @@ static int __igt_reserve(struct kunit *test, unsigned int count, u64 size)
 	return ret;
 }
 
-static void igt_mm_reserve(struct kunit *test)
+static void test_drm_mm_reserve(struct kunit *test)
 {
 	const unsigned int count = min_t(unsigned int, BIT(10), max_iterations);
 	int n;
@@ -471,9 +471,9 @@ static void igt_mm_reserve(struct kunit *test)
 	for_each_prime_number_from(n, 1, 54) {
 		u64 size = BIT_ULL(n);
 
-		KUNIT_ASSERT_FALSE(test, __igt_reserve(test, count, size - 1));
-		KUNIT_ASSERT_FALSE(test, __igt_reserve(test, count, size));
-		KUNIT_ASSERT_FALSE(test, __igt_reserve(test, count, size + 1));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_reserve(test, count, size - 1));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_reserve(test, count, size));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_reserve(test, count, size + 1));
 
 		cond_resched();
 	}
@@ -524,7 +524,7 @@ static bool expect_insert_fail(struct kunit *test, struct drm_mm *mm, u64 size)
 	return false;
 }
 
-static int __igt_insert(struct kunit *test, unsigned int count, u64 size, bool replace)
+static int __test_drm_mm_insert(struct kunit *test, unsigned int count, u64 size, bool replace)
 {
 	DRM_RND_STATE(prng, random_seed);
 	const struct insert_mode *mode;
@@ -660,7 +660,7 @@ static int __igt_insert(struct kunit *test, unsigned int count, u64 size, bool r
 	return ret;
 }
 
-static void igt_mm_insert(struct kunit *test)
+static void test_drm_mm_insert(struct kunit *test)
 {
 	const unsigned int count = min_t(unsigned int, BIT(10), max_iterations);
 	unsigned int n;
@@ -668,20 +668,20 @@ static void igt_mm_insert(struct kunit *test)
 	for_each_prime_number_from(n, 1, 54) {
 		u64 size = BIT_ULL(n);
 
-		KUNIT_ASSERT_FALSE(test, __igt_insert(test, count, size - 1, false));
-		KUNIT_ASSERT_FALSE(test, __igt_insert(test, count, size, false));
-		KUNIT_ASSERT_FALSE(test, __igt_insert(test, count, size + 1, false));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert(test, count, size - 1, false));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert(test, count, size, false));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert(test, count, size + 1, false));
 
 		cond_resched();
 	}
 }
 
-static void igt_mm_replace(struct kunit *test)
+static void test_drm_mm_replace(struct kunit *test)
 {
 	const unsigned int count = min_t(unsigned int, BIT(10), max_iterations);
 	unsigned int n;
 
-	/* Reuse igt_insert to exercise replacement by inserting a dummy node,
+	/* Reuse __test_drm_mm_insert to exercise replacement by inserting a dummy node,
 	 * then replacing it with the intended node. We want to check that
 	 * the tree is intact and all the information we need is carried
 	 * across to the target node.
@@ -690,9 +690,9 @@ static void igt_mm_replace(struct kunit *test)
 	for_each_prime_number_from(n, 1, 54) {
 		u64 size = BIT_ULL(n);
 
-		KUNIT_ASSERT_FALSE(test, __igt_insert(test, count, size - 1, true));
-		KUNIT_ASSERT_FALSE(test, __igt_insert(test, count, size, true));
-		KUNIT_ASSERT_FALSE(test, __igt_insert(test, count, size + 1, true));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert(test, count, size - 1, true));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert(test, count, size, true));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert(test, count, size + 1, true));
 
 		cond_resched();
 	}
@@ -808,7 +808,8 @@ static bool assert_contiguous_in_range(struct kunit *test, struct drm_mm *mm,
 	return true;
 }
 
-static int __igt_insert_range(struct kunit *test, unsigned int count, u64 size, u64 start, u64 end)
+static int __test_drm_mm_insert_range(struct kunit *test, unsigned int count, u64 size,
+				      u64 start, u64 end)
 {
 	const struct insert_mode *mode;
 	struct drm_mm mm;
@@ -820,7 +821,7 @@ static int __igt_insert_range(struct kunit *test, unsigned int count, u64 size,
 	DRM_MM_BUG_ON(!size);
 	DRM_MM_BUG_ON(end <= start);
 
-	/* Very similar to __igt_insert(struct kunit *test, ), but now instead of populating the
+	/* Very similar to __test_drm_mm_insert(), but now instead of populating the
 	 * full range of the drm_mm, we try to fill a small portion of it.
 	 */
 
@@ -921,7 +922,7 @@ static int insert_outside_range(struct kunit *test)
 	return 0;
 }
 
-static void igt_mm_insert_range(struct kunit *test)
+static void test_drm_mm_insert_range(struct kunit *test)
 {
 	const unsigned int count = min_t(unsigned int, BIT(13), max_iterations);
 	unsigned int n;
@@ -933,21 +934,21 @@ static void igt_mm_insert_range(struct kunit *test)
 		const u64 size = BIT_ULL(n);
 		const u64 max = count * size;
 
-		KUNIT_ASSERT_FALSE(test, __igt_insert_range(test, count, size, 0, max));
-		KUNIT_ASSERT_FALSE(test, __igt_insert_range(test, count, size, 1, max));
-		KUNIT_ASSERT_FALSE(test, __igt_insert_range(test, count, size, 0, max - 1));
-		KUNIT_ASSERT_FALSE(test, __igt_insert_range(test, count, size, 0, max / 2));
-		KUNIT_ASSERT_FALSE(test, __igt_insert_range(test, count, size, max / 2, max / 2));
-		KUNIT_ASSERT_FALSE(test, __igt_insert_range(test, count, size,
-							    max / 4 + 1, 3 * max / 4 - 1));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert_range(test, count, size, 0, max));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert_range(test, count, size, 1, max));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert_range(test, count, size, 0, max - 1));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert_range(test, count, size, 0, max / 2));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert_range(test, count, size,
+								    max / 2, max / 2));
+		KUNIT_ASSERT_FALSE(test, __test_drm_mm_insert_range(test, count, size,
+								    max / 4 + 1, 3 * max / 4 - 1));
 
 		cond_resched();
 	}
 }
 
-static int prepare_igt_frag(struct kunit *test, struct drm_mm *mm,
-			    struct drm_mm_node *nodes, unsigned int num_insert,
-			    const struct insert_mode *mode)
+static int prepare_frag(struct kunit *test, struct drm_mm *mm, struct drm_mm_node *nodes,
+			unsigned int num_insert, const struct insert_mode *mode)
 {
 	unsigned int size = 4096;
 	unsigned int i;
@@ -987,7 +988,7 @@ static u64 get_insert_time(struct kunit *test, struct drm_mm *mm,
 	return ktime_to_ns(ktime_sub(ktime_get(), start));
 }
 
-static void igt_mm_frag(struct kunit *test)
+static void test_drm_mm_frag(struct kunit *test)
 {
 	struct drm_mm mm;
 	const struct insert_mode *mode;
@@ -997,15 +998,15 @@ static void igt_mm_frag(struct kunit *test)
 
 	/* We need 4 * insert_size nodes to hold intermediate allocated
 	 * drm_mm nodes.
-	 * 1 times for prepare_igt_frag(struct kunit *test, )
-	 * 1 times for get_insert_time(struct kunit *test, )
-	 * 2 times for get_insert_time(struct kunit *test, )
+	 * 1 times for prepare_frag()
+	 * 1 times for get_insert_time()
+	 * 2 times for get_insert_time()
 	 */
 	nodes = vzalloc(array_size(insert_size * 4, sizeof(*nodes)));
 	KUNIT_ASSERT_TRUE(test, nodes);
 
 	/* For BOTTOMUP and TOPDOWN, we first fragment the
-	 * address space using prepare_igt_frag(struct kunit *test, ) and then try to verify
+	 * address space using prepare_frag() and then try to verify
 	 * that insertions scale quadratically from 10k to 20k insertions
 	 */
 	drm_mm_init(&mm, 1, U64_MAX - 2);
@@ -1016,7 +1017,7 @@ static void igt_mm_frag(struct kunit *test)
 		    mode->mode != DRM_MM_INSERT_HIGH)
 			continue;
 
-		if (prepare_igt_frag(test, &mm, nodes, insert_size, mode))
+		if (prepare_frag(test, &mm, nodes, insert_size, mode))
 			goto err;
 
 		insert_time1 = get_insert_time(test, &mm, insert_size,
@@ -1049,7 +1050,7 @@ static void igt_mm_frag(struct kunit *test)
 	vfree(nodes);
 }
 
-static void igt_mm_align(struct kunit *test)
+static void test_drm_mm_align(struct kunit *test)
 {
 	const struct insert_mode *mode;
 	const unsigned int max_count = min(8192u, max_prime);
@@ -1096,7 +1097,7 @@ static void igt_mm_align(struct kunit *test)
 	vfree(nodes);
 }
 
-static void igt_align_pot(struct kunit *test, int max)
+static void test_drm_mm_align_pot(struct kunit *test, int max)
 {
 	struct drm_mm mm;
 	struct drm_mm_node *node, *next;
@@ -1133,14 +1134,14 @@ static void igt_align_pot(struct kunit *test, int max)
 	drm_mm_takedown(&mm);
 }
 
-static void igt_mm_align32(struct kunit *test)
+static void test_drm_mm_align32(struct kunit *test)
 {
-	igt_align_pot(test, 32);
+	test_drm_mm_align_pot(test, 32);
 }
 
-static void igt_mm_align64(struct kunit *test)
+static void test_drm_mm_align64(struct kunit *test)
 {
-	igt_align_pot(test, 64);
+	test_drm_mm_align_pot(test, 64);
 }
 
 static void show_scan(struct kunit *test, const struct drm_mm_scan *scan)
@@ -1386,7 +1387,7 @@ static int evict_something(struct kunit *test, struct drm_mm *mm,
 	return 0;
 }
 
-static void igt_mm_evict(struct kunit *test)
+static void test_drm_mm_evict(struct kunit *test)
 {
 	DRM_RND_STATE(prng, random_seed);
 	const unsigned int size = 8192;
@@ -1477,7 +1478,7 @@ static void igt_mm_evict(struct kunit *test)
 	vfree(nodes);
 }
 
-static void igt_mm_evict_range(struct kunit *test)
+static void test_drm_mm_evict_range(struct kunit *test)
 {
 	DRM_RND_STATE(prng, random_seed);
 	const unsigned int size = 8192;
@@ -1490,7 +1491,7 @@ static void igt_mm_evict_range(struct kunit *test)
 	struct drm_mm_node *node, *next;
 	unsigned int *order, n;
 
-	/* Like igt_evict() but now we are limiting the search to a
+	/* Like test_drm_mm_evict() but now we are limiting the search to a
 	 * small portion of the full drm_mm.
 	 */
 
@@ -1564,7 +1565,7 @@ static unsigned int node_index(const struct drm_mm_node *node)
 	return div64_u64(node->start, node->size);
 }
 
-static void igt_mm_topdown(struct kunit *test)
+static void test_drm_mm_topdown(struct kunit *test)
 {
 	const struct insert_mode *topdown = &insert_modes[TOPDOWN];
 
@@ -1671,7 +1672,7 @@ static void igt_mm_topdown(struct kunit *test)
 	vfree(nodes);
 }
 
-static void igt_mm_bottomup(struct kunit *test)
+static void test_drm_mm_bottomup(struct kunit *test)
 {
 	const struct insert_mode *bottomup = &insert_modes[BOTTOMUP];
 
@@ -1683,7 +1684,7 @@ static void igt_mm_bottomup(struct kunit *test)
 	struct drm_mm_node *nodes, *node, *next;
 	unsigned int *order, n, m, o = 0;
 
-	/* Like igt_topdown, but instead of searching for the last hole,
+	/* Like test_drm_mm_topdown, but instead of searching for the last hole,
 	 * we search for the first.
 	 */
 
@@ -1763,7 +1764,7 @@ static void igt_mm_bottomup(struct kunit *test)
 	vfree(nodes);
 }
 
-static void __igt_once(struct kunit *test, unsigned int mode)
+static void test_drm_mm_once(struct kunit *test, unsigned int mode)
 {
 	struct drm_mm mm;
 	struct drm_mm_node rsvd_lo, rsvd_hi, node;
@@ -1806,14 +1807,14 @@ static void __igt_once(struct kunit *test, unsigned int mode)
 	drm_mm_takedown(&mm);
 }
 
-static void igt_mm_lowest(struct kunit *test)
+static void test_drm_mm_lowest(struct kunit *test)
 {
-	__igt_once(test, DRM_MM_INSERT_LOW);
+	test_drm_mm_once(test, DRM_MM_INSERT_LOW);
 }
 
-static void igt_mm_highest(struct kunit *test)
+static void test_drm_mm_highest(struct kunit *test)
 {
-	__igt_once(test, DRM_MM_INSERT_HIGH);
+	test_drm_mm_once(test, DRM_MM_INSERT_HIGH);
 }
 
 static void separate_adjacent_colors(const struct drm_mm_node *node,
@@ -1842,7 +1843,7 @@ static bool colors_abutt(struct kunit *test, const struct drm_mm_node *node)
 	return false;
 }
 
-static void igt_mm_color(struct kunit *test)
+static void test_drm_mm_color(struct kunit *test)
 {
 	const unsigned int count = min(4096u, max_iterations);
 	const struct insert_mode *mode;
@@ -2041,7 +2042,7 @@ static int evict_color(struct kunit *test, struct drm_mm *mm, u64 range_start,
 	return 0;
 }
 
-static void igt_mm_color_evict(struct kunit *test)
+static void test_drm_mm_color_evict(struct kunit *test)
 {
 	DRM_RND_STATE(prng, random_seed);
 	const unsigned int total_size = min(8192u, max_iterations);
@@ -2122,7 +2123,7 @@ static void igt_mm_color_evict(struct kunit *test)
 	vfree(nodes);
 }
 
-static void igt_mm_color_evict_range(struct kunit *test)
+static void test_drm_mm_color_evict_range(struct kunit *test)
 {
 	DRM_RND_STATE(prng, random_seed);
 	const unsigned int total_size = 8192;
@@ -2136,7 +2137,7 @@ static void igt_mm_color_evict_range(struct kunit *test)
 	struct drm_mm_node *node, *next;
 	unsigned int *order, n;
 
-	/* Like igt_color_evict(), but limited to small portion of the full
+	/* Like test_drm_mm_color_evict(), but limited to small portion of the full
 	 * drm_mm range.
 	 */
 
@@ -2221,25 +2222,25 @@ module_param(max_iterations, uint, 0400);
 module_param(max_prime, uint, 0400);
 
 static struct kunit_case drm_mm_tests[] = {
-	KUNIT_CASE(igt_mm_init),
-	KUNIT_CASE(igt_mm_debug),
-	KUNIT_CASE(igt_mm_reserve),
-	KUNIT_CASE(igt_mm_insert),
-	KUNIT_CASE(igt_mm_replace),
-	KUNIT_CASE(igt_mm_insert_range),
-	KUNIT_CASE(igt_mm_frag),
-	KUNIT_CASE(igt_mm_align),
-	KUNIT_CASE(igt_mm_align32),
-	KUNIT_CASE(igt_mm_align64),
-	KUNIT_CASE(igt_mm_evict),
-	KUNIT_CASE(igt_mm_evict_range),
-	KUNIT_CASE(igt_mm_topdown),
-	KUNIT_CASE(igt_mm_bottomup),
-	KUNIT_CASE(igt_mm_lowest),
-	KUNIT_CASE(igt_mm_highest),
-	KUNIT_CASE(igt_mm_color),
-	KUNIT_CASE(igt_mm_color_evict),
-	KUNIT_CASE(igt_mm_color_evict_range),
+	KUNIT_CASE(test_drm_mm_init),
+	KUNIT_CASE(test_drm_mm_debug),
+	KUNIT_CASE(test_drm_mm_reserve),
+	KUNIT_CASE(test_drm_mm_insert),
+	KUNIT_CASE(test_drm_mm_replace),
+	KUNIT_CASE(test_drm_mm_insert_range),
+	KUNIT_CASE(test_drm_mm_frag),
+	KUNIT_CASE(test_drm_mm_align),
+	KUNIT_CASE(test_drm_mm_align32),
+	KUNIT_CASE(test_drm_mm_align64),
+	KUNIT_CASE(test_drm_mm_evict),
+	KUNIT_CASE(test_drm_mm_evict_range),
+	KUNIT_CASE(test_drm_mm_topdown),
+	KUNIT_CASE(test_drm_mm_bottomup),
+	KUNIT_CASE(test_drm_mm_lowest),
+	KUNIT_CASE(test_drm_mm_highest),
+	KUNIT_CASE(test_drm_mm_color),
+	KUNIT_CASE(test_drm_mm_color_evict),
+	KUNIT_CASE(test_drm_mm_color_evict_range),
 	{}
 };
 
diff --git a/drivers/gpu/drm/tests/drm_plane_helper_test.c b/drivers/gpu/drm/tests/drm_plane_helper_test.c
index be6cff0020ed..3a70af9a1426 100644
--- a/drivers/gpu/drm/tests/drm_plane_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_plane_helper_test.c
@@ -73,7 +73,7 @@ static bool check_crtc_eq(struct drm_plane_state *plane_state,
 	return true;
 }
 
-static void igt_check_plane_state(struct kunit *test)
+static void test_drm_check_plane_state(struct kunit *test)
 {
 	int ret;
 
@@ -223,7 +223,7 @@ static void igt_check_plane_state(struct kunit *test)
 }
 
 static struct kunit_case drm_plane_helper_test[] = {
-	KUNIT_CASE(igt_check_plane_state),
+	KUNIT_CASE(test_drm_check_plane_state),
 	{}
 };
 
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c b/drivers/gpu/drm/tests/drm_rect_test.c
index c1dbefd49a4c..d8970a92e5ec 100644
--- a/drivers/gpu/drm/tests/drm_rect_test.c
+++ b/drivers/gpu/drm/tests/drm_rect_test.c
@@ -9,7 +9,7 @@
 
 #include <drm/drm_rect.h>
 
-static void igt_drm_rect_clip_scaled_div_by_zero(struct kunit *test)
+static void test_drm_rect_clip_scaled_div_by_zero(struct kunit *test)
 {
 	struct drm_rect src, dst, clip;
 	bool visible;
@@ -35,7 +35,7 @@ static void igt_drm_rect_clip_scaled_div_by_zero(struct kunit *test)
 	KUNIT_EXPECT_FALSE_MSG(test, drm_rect_visible(&src), "Source should not be visible\n");
 }
 
-static void igt_drm_rect_clip_scaled_not_clipped(struct kunit *test)
+static void test_drm_rect_clip_scaled_not_clipped(struct kunit *test)
 {
 	struct drm_rect src, dst, clip;
 	bool visible;
@@ -83,7 +83,7 @@ static void igt_drm_rect_clip_scaled_not_clipped(struct kunit *test)
 	KUNIT_EXPECT_TRUE_MSG(test, drm_rect_visible(&src), "Source should be visible\n");
 }
 
-static void igt_drm_rect_clip_scaled_clipped(struct kunit *test)
+static void test_drm_rect_clip_scaled_clipped(struct kunit *test)
 {
 	struct drm_rect src, dst, clip;
 	bool visible;
@@ -173,7 +173,7 @@ static void igt_drm_rect_clip_scaled_clipped(struct kunit *test)
 	KUNIT_EXPECT_TRUE_MSG(test, drm_rect_visible(&src), "Source should be visible\n");
 }
 
-static void igt_drm_rect_clip_scaled_signed_vs_unsigned(struct kunit *test)
+static void test_drm_rect_clip_scaled_signed_vs_unsigned(struct kunit *test)
 {
 	struct drm_rect src, dst, clip;
 	bool visible;
@@ -197,10 +197,10 @@ static void igt_drm_rect_clip_scaled_signed_vs_unsigned(struct kunit *test)
 }
 
 static struct kunit_case drm_rect_tests[] = {
-	KUNIT_CASE(igt_drm_rect_clip_scaled_div_by_zero),
-	KUNIT_CASE(igt_drm_rect_clip_scaled_not_clipped),
-	KUNIT_CASE(igt_drm_rect_clip_scaled_clipped),
-	KUNIT_CASE(igt_drm_rect_clip_scaled_signed_vs_unsigned),
+	KUNIT_CASE(test_drm_rect_clip_scaled_div_by_zero),
+	KUNIT_CASE(test_drm_rect_clip_scaled_not_clipped),
+	KUNIT_CASE(test_drm_rect_clip_scaled_clipped),
+	KUNIT_CASE(test_drm_rect_clip_scaled_signed_vs_unsigned),
 	{ }
 };
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-01 12:42 ` [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_" Maíra Canal
@ 2022-09-01 12:55   ` Maxime Ripard
  2022-09-01 13:17     ` Javier Martinez Canillas
  2022-09-01 22:33     ` Maíra Canal
  0 siblings, 2 replies; 17+ messages in thread
From: Maxime Ripard @ 2022-09-01 12:55 UTC (permalink / raw)
  To: Maíra Canal
  Cc: michal.winiarski, David Gow, siqueirajordao, magalilemes00,
	David Airlie, tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, linux-kernel, mwen, dri-devel,
	kunit-dev, José Expósito, Isabella Basso, andrealmeid

Hi,

On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
> With the introduction of KUnit, IGT is no longer the only option to run
> the DRM unit tests, as the tests can be run through kunit-tool or on
> real hardware with CONFIG_KUNIT.
> 
> Therefore, remove the "igt_" prefix from the tests and replace it with
> the "test_drm_" prefix, making the tests' names independent from the tool
> used.
> 
> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
>
> ---
> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).

I appreciate it's a bit of a bikeshed but I disagree with this. The
majority of the kunit tests already out there start with the framework
name, including *all* the examples in the kunit doc. Plus, it's fairly
obvious that it's a test, kunit is only about running tests in the first
place.

Maxime

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-01 12:55   ` Maxime Ripard
@ 2022-09-01 13:17     ` Javier Martinez Canillas
  2022-09-01 22:33     ` Maíra Canal
  1 sibling, 0 replies; 17+ messages in thread
From: Javier Martinez Canillas @ 2022-09-01 13:17 UTC (permalink / raw)
  To: Maxime Ripard, Maíra Canal
  Cc: michal.winiarski, Arthur Grillo, siqueirajordao, magalilemes00,
	David Airlie, tales.aparecida, brendanhiggins, linux-kernel,
	dri-devel, mwen, David Gow, José Expósito,
	Isabella Basso, andrealmeid, kunit-dev

On 9/1/22 14:55, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
>> With the introduction of KUnit, IGT is no longer the only option to run
>> the DRM unit tests, as the tests can be run through kunit-tool or on
>> real hardware with CONFIG_KUNIT.
>>
>> Therefore, remove the "igt_" prefix from the tests and replace it with
>> the "test_drm_" prefix, making the tests' names independent from the tool
>> used.
>>
>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
>>
>> ---
>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
> 
> I appreciate it's a bit of a bikeshed but I disagree with this. The
> majority of the kunit tests already out there start with the framework
> name, including *all* the examples in the kunit doc. Plus, it's fairly
> obvious that it's a test, kunit is only about running tests in the first
> place.
>

Agree with Maxime on this.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-01 12:55   ` Maxime Ripard
  2022-09-01 13:17     ` Javier Martinez Canillas
@ 2022-09-01 22:33     ` Maíra Canal
  2022-09-02  8:04       ` Jani Nikula
  2022-09-02  8:08       ` Maxime Ripard
  1 sibling, 2 replies; 17+ messages in thread
From: Maíra Canal @ 2022-09-01 22:33 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: michal.winiarski, David Gow, siqueirajordao, magalilemes00,
	David Airlie, tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, linux-kernel, mwen, dri-devel,
	kunit-dev, José Expósito, Isabella Basso, andrealmeid

Hi Maxime,

On 9/1/22 09:55, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
>> With the introduction of KUnit, IGT is no longer the only option to run
>> the DRM unit tests, as the tests can be run through kunit-tool or on
>> real hardware with CONFIG_KUNIT.
>>
>> Therefore, remove the "igt_" prefix from the tests and replace it with
>> the "test_drm_" prefix, making the tests' names independent from the tool
>> used.
>>
>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
>>
>> ---
>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
> 
> I appreciate it's a bit of a bikeshed but I disagree with this. The
> majority of the kunit tests already out there start with the framework
> name, including *all* the examples in the kunit doc. Plus, it's fairly
> obvious that it's a test, kunit is only about running tests in the first
> place.

Would it be better to keep it as "drm_"?

Currently, I don't think it is appropriate to hold the "igt_" prefix, as
the tests are not IGT exclusive, but I don't have a strong opinion on
using the "drm_" or the "test_drm" prefixes.

Best Regards,
- Maíra Canal

> 
> Maxime
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests
  2022-09-01 12:42 [PATCH v2 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests Maíra Canal
  2022-09-01 12:42 ` [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_" Maíra Canal
@ 2022-09-02  7:52 ` David Gow
  1 sibling, 0 replies; 17+ messages in thread
From: David Gow @ 2022-09-02  7:52 UTC (permalink / raw)
  To: Maíra Canal
  Cc: michal.winiarski, Arthur Grillo, siqueirajordao, magalilemes00,
	David Airlie, Tales Aparecida, dri-devel, Brendan Higgins,
	Javier Martinez Canillas, Linux Kernel Mailing List, mwen,
	KUnit Development, José Expósito, Isabella Basso,
	andrealmeid

On Thu, Sep 1, 2022 at 8:44 PM Maíra Canal <mairacanal@riseup.net> wrote:
>
> The igt_check_drm_framebuffer_create is based on a loop that executes
> tests for all createbuffer_tests test cases. This could be better
> represented by parameterized tests, provided by KUnit.
>
> So, convert the igt_check_drm_framebuffer_create into parameterized tests.
>
> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
> - Use .init for mock_drm_device instead of a global variable. (Michał Winiarski)
> ---

This looks good to me, and worked well here.

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

Cheers,
-- David

>  drivers/gpu/drm/tests/drm_framebuffer_test.c | 45 ++++++++++++--------
>  1 file changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
> index ec7a08ba4056..564794917784 100644
> --- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
> +++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
> @@ -25,7 +25,7 @@ struct drm_framebuffer_test {
>         const char *name;
>  };
>
> -static struct drm_framebuffer_test createbuffer_tests[] = {
> +static const struct drm_framebuffer_test drm_framebuffer_create_cases[] = {

FYI: It shouldn't be _necessary_ to namespace this, though I quite
like it personally. I definitely approve of the const, though.

>  { .buffer_created = 1, .name = "ABGR8888 normal sizes",
>         .cmd = { .width = 600, .height = 600, .pixel_format = DRM_FORMAT_ABGR8888,
>                  .handles = { 1, 0, 0 }, .pitches = { 4 * 600, 0, 0 },
> @@ -330,43 +330,52 @@ static struct drm_mode_config_funcs mock_config_funcs = {
>         .fb_create = fb_create_mock,
>  };
>
> -static struct drm_device mock_drm_device = {
> -       .mode_config = {
> +static int drm_framebuffer_test_init(struct kunit *test)
> +{
> +       struct drm_device *mock;
> +
> +       mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mock);
> +
> +       mock->mode_config = (struct drm_mode_config) {
>                 .min_width = MIN_WIDTH,
>                 .max_width = MAX_WIDTH,
>                 .min_height = MIN_HEIGHT,
>                 .max_height = MAX_HEIGHT,
>                 .funcs = &mock_config_funcs,
> -       },
> -};
> +       };
>
> -static int execute_drm_mode_fb_cmd2(struct drm_mode_fb_cmd2 *r)
> +       test->priv = mock;
> +       return 0;
> +}
> +
> +static void test_drm_framebuffer_create(struct kunit *test)
>  {
> +       const struct drm_framebuffer_test *params = test->param_value;
> +       struct drm_device *mock = test->priv;
>         int buffer_created = 0;
>
> -       mock_drm_device.dev_private = &buffer_created;
> -       drm_internal_framebuffer_create(&mock_drm_device, r, NULL);
> -       return buffer_created;
> +       mock->dev_private = &buffer_created;
> +       drm_internal_framebuffer_create(mock, &params->cmd, NULL);
> +       KUNIT_EXPECT_EQ(test, params->buffer_created, buffer_created);
>  }
>
> -static void igt_check_drm_framebuffer_create(struct kunit *test)
> +static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, char *desc)
>  {
> -       int i = 0;
> -
> -       for (i = 0; i < ARRAY_SIZE(createbuffer_tests); i++) {
> -               KUNIT_EXPECT_EQ_MSG(test, createbuffer_tests[i].buffer_created,
> -                                   execute_drm_mode_fb_cmd2(&createbuffer_tests[i].cmd),
> -                    "Test %d: \"%s\" failed\n", i, createbuffer_tests[i].name);
> -       }
> +       strcpy(desc, t->name);
>  }
>
> +KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
> +                 drm_framebuffer_test_to_desc);
> +
>  static struct kunit_case drm_framebuffer_tests[] = {
> -       KUNIT_CASE(igt_check_drm_framebuffer_create),
> +       KUNIT_CASE_PARAM(test_drm_framebuffer_create, drm_framebuffer_create_gen_params),
>         { }
>  };
>
>  static struct kunit_suite drm_framebuffer_test_suite = {
>         .name = "drm_framebuffer",
> +       .init = drm_framebuffer_test_init,
>         .test_cases = drm_framebuffer_tests,
>  };
>
> --
> 2.37.2
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220901124210.591994-1-mairacanal%40riseup.net.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-01 22:33     ` Maíra Canal
@ 2022-09-02  8:04       ` Jani Nikula
  2022-09-02 12:34         ` Maxime Ripard
  2022-09-02  8:08       ` Maxime Ripard
  1 sibling, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2022-09-02  8:04 UTC (permalink / raw)
  To: Maíra Canal, Maxime Ripard
  Cc: michal.winiarski, David Gow, siqueirajordao, magalilemes00,
	David Airlie, tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, dri-devel, linux-kernel, mwen,
	kunit-dev, José Expósito, Isabella Basso, andrealmeid

On Thu, 01 Sep 2022, Maíra Canal <mairacanal@riseup.net> wrote:
> Hi Maxime,
>
> On 9/1/22 09:55, Maxime Ripard wrote:
>> Hi,
>> 
>> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
>>> With the introduction of KUnit, IGT is no longer the only option to run
>>> the DRM unit tests, as the tests can be run through kunit-tool or on
>>> real hardware with CONFIG_KUNIT.
>>>
>>> Therefore, remove the "igt_" prefix from the tests and replace it with
>>> the "test_drm_" prefix, making the tests' names independent from the tool
>>> used.
>>>
>>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
>>>
>>> ---
>>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
>>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
>> 
>> I appreciate it's a bit of a bikeshed but I disagree with this. The
>> majority of the kunit tests already out there start with the framework
>> name, including *all* the examples in the kunit doc. Plus, it's fairly
>> obvious that it's a test, kunit is only about running tests in the first
>> place.
>
> Would it be better to keep it as "drm_"?

That's not "keeping". That's renaming igt to drm.

> Currently, I don't think it is appropriate to hold the "igt_" prefix, as
> the tests are not IGT exclusive, but I don't have a strong opinion on
> using the "drm_" or the "test_drm" prefixes.

I repeat my stance that "drm_" alone is confusing. For the reason alone
that it pollutes the code tagging tools, mixing actual drm_ types and
functions with unit test functions.

BR,
Jani.



>
> Best Regards,
> - Maíra Canal
>
>> 
>> Maxime
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-01 22:33     ` Maíra Canal
  2022-09-02  8:04       ` Jani Nikula
@ 2022-09-02  8:08       ` Maxime Ripard
  2022-09-02  8:24         ` Jani Nikula
  1 sibling, 1 reply; 17+ messages in thread
From: Maxime Ripard @ 2022-09-02  8:08 UTC (permalink / raw)
  To: Maíra Canal
  Cc: michal.winiarski, David Gow, siqueirajordao, magalilemes00,
	David Airlie, tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, linux-kernel, mwen, dri-devel,
	kunit-dev, José Expósito, Isabella Basso, andrealmeid

[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

On Thu, Sep 01, 2022 at 07:33:18PM -0300, Maíra Canal wrote:
> Hi Maxime,
> 
> On 9/1/22 09:55, Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
> >> With the introduction of KUnit, IGT is no longer the only option to run
> >> the DRM unit tests, as the tests can be run through kunit-tool or on
> >> real hardware with CONFIG_KUNIT.
> >>
> >> Therefore, remove the "igt_" prefix from the tests and replace it with
> >> the "test_drm_" prefix, making the tests' names independent from the tool
> >> used.
> >>
> >> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
> >>
> >> ---
> >> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
> >> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
> > 
> > I appreciate it's a bit of a bikeshed but I disagree with this. The
> > majority of the kunit tests already out there start with the framework
> > name, including *all* the examples in the kunit doc. Plus, it's fairly
> > obvious that it's a test, kunit is only about running tests in the first
> > place.
> 
> Would it be better to keep it as "drm_"?
> 
> Currently, I don't think it is appropriate to hold the "igt_" prefix, as
> the tests are not IGT exclusive, but I don't have a strong opinion on
> using the "drm_" or the "test_drm" prefixes.

Yes, using drm as our prefix everywhere seems like a good idea :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-02  8:08       ` Maxime Ripard
@ 2022-09-02  8:24         ` Jani Nikula
  0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-09-02  8:24 UTC (permalink / raw)
  To: Maxime Ripard, Maíra Canal
  Cc: michal.winiarski, David Gow, siqueirajordao, magalilemes00,
	David Airlie, tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, dri-devel, linux-kernel, mwen,
	kunit-dev, José Expósito, Isabella Basso, andrealmeid

On Fri, 02 Sep 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> On Thu, Sep 01, 2022 at 07:33:18PM -0300, Maíra Canal wrote:
>> Hi Maxime,
>> 
>> On 9/1/22 09:55, Maxime Ripard wrote:
>> > Hi,
>> > 
>> > On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
>> >> With the introduction of KUnit, IGT is no longer the only option to run
>> >> the DRM unit tests, as the tests can be run through kunit-tool or on
>> >> real hardware with CONFIG_KUNIT.
>> >>
>> >> Therefore, remove the "igt_" prefix from the tests and replace it with
>> >> the "test_drm_" prefix, making the tests' names independent from the tool
>> >> used.
>> >>
>> >> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
>> >>
>> >> ---
>> >> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
>> >> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
>> > 
>> > I appreciate it's a bit of a bikeshed but I disagree with this. The
>> > majority of the kunit tests already out there start with the framework
>> > name, including *all* the examples in the kunit doc. Plus, it's fairly
>> > obvious that it's a test, kunit is only about running tests in the first
>> > place.
>> 
>> Would it be better to keep it as "drm_"?
>> 
>> Currently, I don't think it is appropriate to hold the "igt_" prefix, as
>> the tests are not IGT exclusive, but I don't have a strong opinion on
>> using the "drm_" or the "test_drm" prefixes.
>
> Yes, using drm as our prefix everywhere seems like a good idea :)

Disagreed for reasons explained in other mails.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-02  8:04       ` Jani Nikula
@ 2022-09-02 12:34         ` Maxime Ripard
  2022-09-02 13:03           ` Jani Nikula
  0 siblings, 1 reply; 17+ messages in thread
From: Maxime Ripard @ 2022-09-02 12:34 UTC (permalink / raw)
  To: Jani Nikula
  Cc: michal.winiarski, David Gow, siqueirajordao, magalilemes00,
	David Airlie, tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, dri-devel, linux-kernel, mwen,
	Maíra Canal, kunit-dev, José Expósito,
	Isabella Basso, andrealmeid

[-- Attachment #1: Type: text/plain, Size: 2199 bytes --]

On Fri, Sep 02, 2022 at 11:04:14AM +0300, Jani Nikula wrote:
> On Thu, 01 Sep 2022, Maíra Canal <mairacanal@riseup.net> wrote:
> > Hi Maxime,
> >
> > On 9/1/22 09:55, Maxime Ripard wrote:
> >> Hi,
> >> 
> >> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
> >>> With the introduction of KUnit, IGT is no longer the only option to run
> >>> the DRM unit tests, as the tests can be run through kunit-tool or on
> >>> real hardware with CONFIG_KUNIT.
> >>>
> >>> Therefore, remove the "igt_" prefix from the tests and replace it with
> >>> the "test_drm_" prefix, making the tests' names independent from the tool
> >>> used.
> >>>
> >>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
> >>>
> >>> ---
> >>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
> >>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
> >> 
> >> I appreciate it's a bit of a bikeshed but I disagree with this. The
> >> majority of the kunit tests already out there start with the framework
> >> name, including *all* the examples in the kunit doc. Plus, it's fairly
> >> obvious that it's a test, kunit is only about running tests in the first
> >> place.
> >
> > Would it be better to keep it as "drm_"?
> 
> That's not "keeping". That's renaming igt to drm.

Well, there's like half the tests that are prefixed with drm, the other
with igt, so it's both really

> > Currently, I don't think it is appropriate to hold the "igt_" prefix, as
> > the tests are not IGT exclusive, but I don't have a strong opinion on
> > using the "drm_" or the "test_drm" prefixes.
> 
> I repeat my stance that "drm_" alone is confusing.

What are you confusing it with?

> For the reason alone that it pollutes the code tagging tools, mixing
> actual drm_ types and functions with unit test functions.

I don't get it, I'm sorry. All these functions are static and not part
of any API, so I can't see how it would pollute a code tagging tool. Or
at least, not more than any driver does.

And we're part of a larger project here, it's about consistency with the
rest of the ecosystem.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-02 12:34         ` Maxime Ripard
@ 2022-09-02 13:03           ` Jani Nikula
  2022-09-02 13:38             ` Michał Winiarski
  0 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2022-09-02 13:03 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: michal.winiarski, David Gow, siqueirajordao, magalilemes00,
	David Airlie, tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, dri-devel, linux-kernel, mwen,
	Maíra Canal, kunit-dev, José Expósito,
	Isabella Basso, andrealmeid

On Fri, 02 Sep 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> On Fri, Sep 02, 2022 at 11:04:14AM +0300, Jani Nikula wrote:
>> On Thu, 01 Sep 2022, Maíra Canal <mairacanal@riseup.net> wrote:
>> > Hi Maxime,
>> >
>> > On 9/1/22 09:55, Maxime Ripard wrote:
>> >> Hi,
>> >> 
>> >> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
>> >>> With the introduction of KUnit, IGT is no longer the only option to run
>> >>> the DRM unit tests, as the tests can be run through kunit-tool or on
>> >>> real hardware with CONFIG_KUNIT.
>> >>>
>> >>> Therefore, remove the "igt_" prefix from the tests and replace it with
>> >>> the "test_drm_" prefix, making the tests' names independent from the tool
>> >>> used.
>> >>>
>> >>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
>> >>>
>> >>> ---
>> >>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
>> >>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
>> >> 
>> >> I appreciate it's a bit of a bikeshed but I disagree with this. The
>> >> majority of the kunit tests already out there start with the framework
>> >> name, including *all* the examples in the kunit doc. Plus, it's fairly
>> >> obvious that it's a test, kunit is only about running tests in the first
>> >> place.
>> >
>> > Would it be better to keep it as "drm_"?
>> 
>> That's not "keeping". That's renaming igt to drm.
>
> Well, there's like half the tests that are prefixed with drm, the other
> with igt, so it's both really
>
>> > Currently, I don't think it is appropriate to hold the "igt_" prefix, as
>> > the tests are not IGT exclusive, but I don't have a strong opinion on
>> > using the "drm_" or the "test_drm" prefixes.
>> 
>> I repeat my stance that "drm_" alone is confusing.
>
> What are you confusing it with?
>
>> For the reason alone that it pollutes the code tagging tools, mixing
>> actual drm_ types and functions with unit test functions.
>
> I don't get it, I'm sorry. All these functions are static and not part
> of any API, so I can't see how it would pollute a code tagging tool. Or
> at least, not more than any driver does.
>
> And we're part of a larger project here, it's about consistency with the
> rest of the ecosystem.

Okay, so I'm just going to explain what I mean, but say "whatever" right
after and move on.

For example, drm_buddy_test.c includes drm_buddy.h so with the igt_ ->
drm_ rename none of the test functions may clash with the drm_buddy_
prefixed existing functions. Ditto for all tests similarly.

For example drm_buddy_alloc_range() as a name sounds like something that
allocs a range, not something that tests range allocation. On the other
hand, you have drm_buddy_alloc_blocks() which is actually a real
drm_buddy function, not a test. What would you call a test that tests
that? Here, we end up with names that are all prefixed drm_buddy and you
won't know what's the actual function and what's the test unless you
look it up.

I use code tagging that I can use for finding and completing
e.g. functions. Currently I have the following completions, for
igt_buddy_ and drm_buddy_, respectively:

Possible completions are:
igt_buddy_alloc_limit 	igt_buddy_alloc_optimistic 	igt_buddy_alloc_pathological
igt_buddy_alloc_pessimistic 	igt_buddy_alloc_range 	igt_buddy_alloc_smoke

Possible completions are:
drm_buddy_alloc_blocks 	drm_buddy_block 	drm_buddy_block_is_allocated 	drm_buddy_block_is_free
drm_buddy_block_is_split 	drm_buddy_block_offset 	drm_buddy_block_order 	drm_buddy_block_print
drm_buddy_block_size 	drm_buddy_block_state 	drm_buddy_block_trim 	drm_buddy_fini
drm_buddy_free_block 	drm_buddy_free_list 	drm_buddy_init 	drm_buddy_init_test
drm_buddy_module_exit 	drm_buddy_module_init 	drm_buddy_print

With the patch at hand, they'll all be lumped under drm_buddy_
completions, and some of them will be actual drm buddy functions and
some not.

I just find it a very odd convention to name the tests in a way that's
indistinguishable from the real things. Even *within* drm_buddy_test.c
where you read the test code. Because currently you do have calls to
igt_buddy_ prefixed functions from other igt_buddy_ prefixed functions,
along with the drm_buddy_ prefixed calls. I think it's just going to be
a mess.

/rant

Whatever. Moving on.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-02 13:03           ` Jani Nikula
@ 2022-09-02 13:38             ` Michał Winiarski
  2022-09-02 13:56               ` Jani Nikula
                                 ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Michał Winiarski @ 2022-09-02 13:38 UTC (permalink / raw)
  To: Jani Nikula
  Cc: David Gow, siqueirajordao, magalilemes00, David Airlie,
	tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, dri-devel, linux-kernel, mwen,
	Maíra Canal, Maxime Ripard, kunit-dev,
	José Expósito, Isabella Basso, andrealmeid

On Fri, Sep 02, 2022 at 04:03:20PM +0300, Jani Nikula wrote:
> On Fri, 02 Sep 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> > On Fri, Sep 02, 2022 at 11:04:14AM +0300, Jani Nikula wrote:
> >> On Thu, 01 Sep 2022, Maíra Canal <mairacanal@riseup.net> wrote:
> >> > Hi Maxime,
> >> >
> >> > On 9/1/22 09:55, Maxime Ripard wrote:
> >> >> Hi,
> >> >> 
> >> >> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
> >> >>> With the introduction of KUnit, IGT is no longer the only option to run
> >> >>> the DRM unit tests, as the tests can be run through kunit-tool or on
> >> >>> real hardware with CONFIG_KUNIT.
> >> >>>
> >> >>> Therefore, remove the "igt_" prefix from the tests and replace it with
> >> >>> the "test_drm_" prefix, making the tests' names independent from the tool
> >> >>> used.
> >> >>>
> >> >>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
> >> >>>
> >> >>> ---
> >> >>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
> >> >>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
> >> >> 
> >> >> I appreciate it's a bit of a bikeshed but I disagree with this. The
> >> >> majority of the kunit tests already out there start with the framework
> >> >> name, including *all* the examples in the kunit doc. Plus, it's fairly
> >> >> obvious that it's a test, kunit is only about running tests in the first
> >> >> place.
> >> >
> >> > Would it be better to keep it as "drm_"?
> >> 
> >> That's not "keeping". That's renaming igt to drm.
> >
> > Well, there's like half the tests that are prefixed with drm, the other
> > with igt, so it's both really
> >
> >> > Currently, I don't think it is appropriate to hold the "igt_" prefix, as
> >> > the tests are not IGT exclusive, but I don't have a strong opinion on
> >> > using the "drm_" or the "test_drm" prefixes.
> >> 
> >> I repeat my stance that "drm_" alone is confusing.
> >
> > What are you confusing it with?
> >
> >> For the reason alone that it pollutes the code tagging tools, mixing
> >> actual drm_ types and functions with unit test functions.
> >
> > I don't get it, I'm sorry. All these functions are static and not part
> > of any API, so I can't see how it would pollute a code tagging tool. Or
> > at least, not more than any driver does.
> >
> > And we're part of a larger project here, it's about consistency with the
> > rest of the ecosystem.
> 
> Okay, so I'm just going to explain what I mean, but say "whatever" right
> after and move on.
> 
> For example, drm_buddy_test.c includes drm_buddy.h so with the igt_ ->
> drm_ rename none of the test functions may clash with the drm_buddy_
> prefixed existing functions. Ditto for all tests similarly.
> 
> For example drm_buddy_alloc_range() as a name sounds like something that
> allocs a range, not something that tests range allocation. On the other
> hand, you have drm_buddy_alloc_blocks() which is actually a real
> drm_buddy function, not a test. What would you call a test that tests
> that? Here, we end up with names that are all prefixed drm_buddy and you
> won't know what's the actual function and what's the test unless you
> look it up.
> 
> I use code tagging that I can use for finding and completing
> e.g. functions. Currently I have the following completions, for
> igt_buddy_ and drm_buddy_, respectively:
> 
> Possible completions are:
> igt_buddy_alloc_limit 	igt_buddy_alloc_optimistic 	igt_buddy_alloc_pathological
> igt_buddy_alloc_pessimistic 	igt_buddy_alloc_range 	igt_buddy_alloc_smoke
> 
> Possible completions are:
> drm_buddy_alloc_blocks 	drm_buddy_block 	drm_buddy_block_is_allocated 	drm_buddy_block_is_free
> drm_buddy_block_is_split 	drm_buddy_block_offset 	drm_buddy_block_order 	drm_buddy_block_print
> drm_buddy_block_size 	drm_buddy_block_state 	drm_buddy_block_trim 	drm_buddy_fini
> drm_buddy_free_block 	drm_buddy_free_list 	drm_buddy_init 	drm_buddy_init_test
> drm_buddy_module_exit 	drm_buddy_module_init 	drm_buddy_print
> 
> With the patch at hand, they'll all be lumped under drm_buddy_
> completions, and some of them will be actual drm buddy functions and
> some not.
> 
> I just find it a very odd convention to name the tests in a way that's
> indistinguishable from the real things. Even *within* drm_buddy_test.c
> where you read the test code. Because currently you do have calls to
> igt_buddy_ prefixed functions from other igt_buddy_ prefixed functions,
> along with the drm_buddy_ prefixed calls. I think it's just going to be
> a mess.
> 
> /rant
> 
> Whatever. Moving on.

KUnit docs [1] state:
https://docs.kernel.org/dev-tools/kunit/style.html#test-cases
"As tests are themselves functions, their names cannot conflict with other
C identifiers in the kernel. This may require some creative naming."
And give examples of names. But this should be local to individual test suite -
as long as the test is readable, and the name describes what it is testing, we
should be fine. We don't even need to pass drm_* prefix, as this convention is
expected for test suites, not test cases [2].

Having said that - I do believe that igt_* prefix don't belong here (which is
why I'm progressively trying to get rid of in the patches that refactor some of
the tests).
I agree with Jani - can we take it on a case-by-case basis?
If the test name is too similar to the function that it is testing, we could go
with one of the following (taking igt_buddy_alloc_limit as example):
drm_buddy_test_alloc_limit
test_drm_buddy_alloc_limit
buddy_test_alloc_limit
test_buddy_alloc_limit

And either of those is fine in my opinion (I'd personally go with
test_buddy_alloc_limit in this case).
We don't really need a DRM-wide (or worse, kernel wide) convention for test case
names (it's only really needed for test suites).

[1] https://docs.kernel.org/dev-tools/kunit/style.html#test-cases
[2] https://docs.kernel.org/dev-tools/kunit/style.html#suites

-Michał

> 
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-02 13:38             ` Michał Winiarski
@ 2022-09-02 13:56               ` Jani Nikula
  2022-09-02 22:53               ` Maíra Canal
  2022-09-05 12:10               ` Maxime Ripard
  2 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-09-02 13:56 UTC (permalink / raw)
  To: Michał Winiarski
  Cc: David Gow, siqueirajordao, magalilemes00, David Airlie,
	tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, dri-devel, linux-kernel, mwen,
	Maíra Canal, Maxime Ripard, kunit-dev,
	José Expósito, Isabella Basso, andrealmeid

On Fri, 02 Sep 2022, Michał Winiarski <michal.winiarski@intel.com> wrote:
> Having said that - I do believe that igt_* prefix don't belong here
> (which is why I'm progressively trying to get rid of in the patches
> that refactor some of the tests).

Just clarifying that I'm not trying to defend igt_ prefix at all, and I
agree we should get rid of it.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-02 13:38             ` Michał Winiarski
  2022-09-02 13:56               ` Jani Nikula
@ 2022-09-02 22:53               ` Maíra Canal
  2022-09-05 12:10               ` Maxime Ripard
  2 siblings, 0 replies; 17+ messages in thread
From: Maíra Canal @ 2022-09-02 22:53 UTC (permalink / raw)
  To: Michał Winiarski, Jani Nikula
  Cc: Arthur Grillo, siqueirajordao, magalilemes00, David Airlie,
	tales.aparecida, brendanhiggins, Javier Martinez Canillas,
	dri-devel, linux-kernel, mwen, Maxime Ripard, David Gow,
	José Expósito, Isabella Basso, andrealmeid, kunit-dev

On 9/2/22 10:38, Michał Winiarski wrote:
> On Fri, Sep 02, 2022 at 04:03:20PM +0300, Jani Nikula wrote:
>> On Fri, 02 Sep 2022, Maxime Ripard <maxime@cerno.tech> wrote:
>>> On Fri, Sep 02, 2022 at 11:04:14AM +0300, Jani Nikula wrote:
>>>> On Thu, 01 Sep 2022, Maíra Canal <mairacanal@riseup.net> wrote:
>>>>> Hi Maxime,
>>>>>
>>>>> On 9/1/22 09:55, Maxime Ripard wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
>>>>>>> With the introduction of KUnit, IGT is no longer the only option to run
>>>>>>> the DRM unit tests, as the tests can be run through kunit-tool or on
>>>>>>> real hardware with CONFIG_KUNIT.
>>>>>>>
>>>>>>> Therefore, remove the "igt_" prefix from the tests and replace it with
>>>>>>> the "test_drm_" prefix, making the tests' names independent from the tool
>>>>>>> used.
>>>>>>>
>>>>>>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
>>>>>>>
>>>>>>> ---
>>>>>>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
>>>>>>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
>>>>>>
>>>>>> I appreciate it's a bit of a bikeshed but I disagree with this. The
>>>>>> majority of the kunit tests already out there start with the framework
>>>>>> name, including *all* the examples in the kunit doc. Plus, it's fairly
>>>>>> obvious that it's a test, kunit is only about running tests in the first
>>>>>> place.
>>>>>
>>>>> Would it be better to keep it as "drm_"?
>>>>
>>>> That's not "keeping". That's renaming igt to drm.
>>>
>>> Well, there's like half the tests that are prefixed with drm, the other
>>> with igt, so it's both really
>>>
>>>>> Currently, I don't think it is appropriate to hold the "igt_" prefix, as
>>>>> the tests are not IGT exclusive, but I don't have a strong opinion on
>>>>> using the "drm_" or the "test_drm" prefixes.
>>>>
>>>> I repeat my stance that "drm_" alone is confusing.
>>>
>>> What are you confusing it with?
>>>
>>>> For the reason alone that it pollutes the code tagging tools, mixing
>>>> actual drm_ types and functions with unit test functions.
>>>
>>> I don't get it, I'm sorry. All these functions are static and not part
>>> of any API, so I can't see how it would pollute a code tagging tool. Or
>>> at least, not more than any driver does.
>>>
>>> And we're part of a larger project here, it's about consistency with the
>>> rest of the ecosystem.
>>
>> Okay, so I'm just going to explain what I mean, but say "whatever" right
>> after and move on.
>>
>> For example, drm_buddy_test.c includes drm_buddy.h so with the igt_ ->
>> drm_ rename none of the test functions may clash with the drm_buddy_
>> prefixed existing functions. Ditto for all tests similarly.
>>
>> For example drm_buddy_alloc_range() as a name sounds like something that
>> allocs a range, not something that tests range allocation. On the other
>> hand, you have drm_buddy_alloc_blocks() which is actually a real
>> drm_buddy function, not a test. What would you call a test that tests
>> that? Here, we end up with names that are all prefixed drm_buddy and you
>> won't know what's the actual function and what's the test unless you
>> look it up.
>>
>> I use code tagging that I can use for finding and completing
>> e.g. functions. Currently I have the following completions, for
>> igt_buddy_ and drm_buddy_, respectively:
>>
>> Possible completions are:
>> igt_buddy_alloc_limit 	igt_buddy_alloc_optimistic 	igt_buddy_alloc_pathological
>> igt_buddy_alloc_pessimistic 	igt_buddy_alloc_range 	igt_buddy_alloc_smoke
>>
>> Possible completions are:
>> drm_buddy_alloc_blocks 	drm_buddy_block 	drm_buddy_block_is_allocated 	drm_buddy_block_is_free
>> drm_buddy_block_is_split 	drm_buddy_block_offset 	drm_buddy_block_order 	drm_buddy_block_print
>> drm_buddy_block_size 	drm_buddy_block_state 	drm_buddy_block_trim 	drm_buddy_fini
>> drm_buddy_free_block 	drm_buddy_free_list 	drm_buddy_init 	drm_buddy_init_test
>> drm_buddy_module_exit 	drm_buddy_module_init 	drm_buddy_print
>>
>> With the patch at hand, they'll all be lumped under drm_buddy_
>> completions, and some of them will be actual drm buddy functions and
>> some not.
>>
>> I just find it a very odd convention to name the tests in a way that's
>> indistinguishable from the real things. Even *within* drm_buddy_test.c
>> where you read the test code. Because currently you do have calls to
>> igt_buddy_ prefixed functions from other igt_buddy_ prefixed functions,
>> along with the drm_buddy_ prefixed calls. I think it's just going to be
>> a mess.
>>
>> /rant
>>
>> Whatever. Moving on.
> 
> KUnit docs [1] state:
> https://docs.kernel.org/dev-tools/kunit/style.html#test-cases
> "As tests are themselves functions, their names cannot conflict with other
> C identifiers in the kernel. This may require some creative naming."
> And give examples of names. But this should be local to individual test suite -
> as long as the test is readable, and the name describes what it is testing, we
> should be fine. We don't even need to pass drm_* prefix, as this convention is
> expected for test suites, not test cases [2].
> 
> Having said that - I do believe that igt_* prefix don't belong here (which is
> why I'm progressively trying to get rid of in the patches that refactor some of
> the tests).
> I agree with Jani - can we take it on a case-by-case basis?
> If the test name is too similar to the function that it is testing, we could go
> with one of the following (taking igt_buddy_alloc_limit as example):
> drm_buddy_test_alloc_limit
> test_drm_buddy_alloc_limit
> buddy_test_alloc_limit
> test_buddy_alloc_limit
> 
> And either of those is fine in my opinion (I'd personally go with
> test_buddy_alloc_limit in this case).
> We don't really need a DRM-wide (or worse, kernel wide) convention for test case
> names (it's only really needed for test suites).

Although I do like the idea of having a DRM-wide name convention for
test cases as it would ease the identification of test cases, I can see
that consensus on this matter might be hard. So I guess we can take case
by case.

But as it would be nice to remove the "igt_" prefix from the tests, a
first name convention might be needed. Currently, I'm not sure in which
direction to go for removing the "igt_" from the tests.

For me, any of the options mentioned above would be okay, with a slight
preference for the test_drm_ one.

Mentioning "test" on the test case really helps distinguish the API and
the test cases. Most of the KUnit tests that I saw have the "test" in
some part of the function (sometimes in the beginning, sometimes in the
end, or after the subsystem prefix). So, I guess using "test" on the
functions is not a bad practice, or redundant.

Best Regards,
- Maíra Canal

> 
> [1] https://docs.kernel.org/dev-tools/kunit/style.html#test-cases
> [2] https://docs.kernel.org/dev-tools/kunit/style.html#suites
> 
> -Michał
> 
>>
>>
>> BR,
>> Jani.
>>
>>
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-02 13:38             ` Michał Winiarski
  2022-09-02 13:56               ` Jani Nikula
  2022-09-02 22:53               ` Maíra Canal
@ 2022-09-05 12:10               ` Maxime Ripard
  2022-09-05 13:11                 ` Michał Winiarski
  2022-09-05 13:37                 ` Maíra Canal
  2 siblings, 2 replies; 17+ messages in thread
From: Maxime Ripard @ 2022-09-05 12:10 UTC (permalink / raw)
  To: Michał Winiarski
  Cc: David Gow, siqueirajordao, magalilemes00, David Airlie,
	tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, linux-kernel, mwen, Maíra Canal,
	dri-devel, kunit-dev, José Expósito, Isabella Basso,
	andrealmeid

On Fri, Sep 02, 2022 at 03:38:28PM +0200, Michał Winiarski wrote:
> On Fri, Sep 02, 2022 at 04:03:20PM +0300, Jani Nikula wrote:
> > On Fri, 02 Sep 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> > > On Fri, Sep 02, 2022 at 11:04:14AM +0300, Jani Nikula wrote:
> > >> On Thu, 01 Sep 2022, Maíra Canal <mairacanal@riseup.net> wrote:
> > >> > Hi Maxime,
> > >> >
> > >> > On 9/1/22 09:55, Maxime Ripard wrote:
> > >> >> Hi,
> > >> >> 
> > >> >> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
> > >> >>> With the introduction of KUnit, IGT is no longer the only option to run
> > >> >>> the DRM unit tests, as the tests can be run through kunit-tool or on
> > >> >>> real hardware with CONFIG_KUNIT.
> > >> >>>
> > >> >>> Therefore, remove the "igt_" prefix from the tests and replace it with
> > >> >>> the "test_drm_" prefix, making the tests' names independent from the tool
> > >> >>> used.
> > >> >>>
> > >> >>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
> > >> >>>
> > >> >>> ---
> > >> >>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
> > >> >>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
> > >> >> 
> > >> >> I appreciate it's a bit of a bikeshed but I disagree with this. The
> > >> >> majority of the kunit tests already out there start with the framework
> > >> >> name, including *all* the examples in the kunit doc. Plus, it's fairly
> > >> >> obvious that it's a test, kunit is only about running tests in the first
> > >> >> place.
> > >> >
> > >> > Would it be better to keep it as "drm_"?
> > >> 
> > >> That's not "keeping". That's renaming igt to drm.
> > >
> > > Well, there's like half the tests that are prefixed with drm, the other
> > > with igt, so it's both really
> > >
> > >> > Currently, I don't think it is appropriate to hold the "igt_" prefix, as
> > >> > the tests are not IGT exclusive, but I don't have a strong opinion on
> > >> > using the "drm_" or the "test_drm" prefixes.
> > >> 
> > >> I repeat my stance that "drm_" alone is confusing.
> > >
> > > What are you confusing it with?
> > >
> > >> For the reason alone that it pollutes the code tagging tools, mixing
> > >> actual drm_ types and functions with unit test functions.
> > >
> > > I don't get it, I'm sorry. All these functions are static and not part
> > > of any API, so I can't see how it would pollute a code tagging tool. Or
> > > at least, not more than any driver does.
> > >
> > > And we're part of a larger project here, it's about consistency with the
> > > rest of the ecosystem.
> > 
> > Okay, so I'm just going to explain what I mean, but say "whatever" right
> > after and move on.
> > 
> > For example, drm_buddy_test.c includes drm_buddy.h so with the igt_ ->
> > drm_ rename none of the test functions may clash with the drm_buddy_
> > prefixed existing functions. Ditto for all tests similarly.
> > 
> > For example drm_buddy_alloc_range() as a name sounds like something that
> > allocs a range, not something that tests range allocation. On the other
> > hand, you have drm_buddy_alloc_blocks() which is actually a real
> > drm_buddy function, not a test. What would you call a test that tests
> > that? Here, we end up with names that are all prefixed drm_buddy and you
> > won't know what's the actual function and what's the test unless you
> > look it up.
> > 
> > I use code tagging that I can use for finding and completing
> > e.g. functions. Currently I have the following completions, for
> > igt_buddy_ and drm_buddy_, respectively:
> > 
> > Possible completions are:
> > igt_buddy_alloc_limit 	igt_buddy_alloc_optimistic 	igt_buddy_alloc_pathological
> > igt_buddy_alloc_pessimistic 	igt_buddy_alloc_range 	igt_buddy_alloc_smoke
> > 
> > Possible completions are:
> > drm_buddy_alloc_blocks 	drm_buddy_block 	drm_buddy_block_is_allocated 	drm_buddy_block_is_free
> > drm_buddy_block_is_split 	drm_buddy_block_offset 	drm_buddy_block_order 	drm_buddy_block_print
> > drm_buddy_block_size 	drm_buddy_block_state 	drm_buddy_block_trim 	drm_buddy_fini
> > drm_buddy_free_block 	drm_buddy_free_list 	drm_buddy_init 	drm_buddy_init_test
> > drm_buddy_module_exit 	drm_buddy_module_init 	drm_buddy_print
> > 
> > With the patch at hand, they'll all be lumped under drm_buddy_
> > completions, and some of them will be actual drm buddy functions and
> > some not.
> > 
> > I just find it a very odd convention to name the tests in a way that's
> > indistinguishable from the real things. Even *within* drm_buddy_test.c
> > where you read the test code. Because currently you do have calls to
> > igt_buddy_ prefixed functions from other igt_buddy_ prefixed functions,
> > along with the drm_buddy_ prefixed calls. I think it's just going to be
> > a mess.
> > 
> > /rant
> > 
> > Whatever. Moving on.
> 
> KUnit docs [1] state:
> https://docs.kernel.org/dev-tools/kunit/style.html#test-cases
> "As tests are themselves functions, their names cannot conflict with other
> C identifiers in the kernel. This may require some creative naming."
> And give examples of names. But this should be local to individual test suite -
> as long as the test is readable, and the name describes what it is testing, we
> should be fine. We don't even need to pass drm_* prefix, as this convention is
> expected for test suites, not test cases [2].
> 
> Having said that - I do believe that igt_* prefix don't belong here (which is
> why I'm progressively trying to get rid of in the patches that refactor some of
> the tests).
> I agree with Jani - can we take it on a case-by-case basis?
> If the test name is too similar to the function that it is testing, we could go
> with one of the following (taking igt_buddy_alloc_limit as example):
> drm_buddy_test_alloc_limit
> test_drm_buddy_alloc_limit
> buddy_test_alloc_limit
> test_buddy_alloc_limit

We also have drm_test_buddy_alloc_limit, or drm_buddy_test_alloc_limit

Both would be fine for me, with a small preference for the former, which
should also address Jani's concerns?

> And either of those is fine in my opinion (I'd personally go with
> test_buddy_alloc_limit in this case).
> We don't really need a DRM-wide (or worse, kernel wide) convention for test case
> names (it's only really needed for test suites).

Sure we do. kunit.py can take some filters too. Being able to only run
DRM tests with a single filter is super convenient, and if we fail to
provide a consistent naming we're pretty much sure people running the
tests are going to miss some.

Maxime

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-05 12:10               ` Maxime Ripard
@ 2022-09-05 13:11                 ` Michał Winiarski
  2022-09-05 13:37                 ` Maíra Canal
  1 sibling, 0 replies; 17+ messages in thread
From: Michał Winiarski @ 2022-09-05 13:11 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: David Gow, siqueirajordao, magalilemes00, David Airlie,
	tales.aparecida, Arthur Grillo, brendanhiggins,
	Javier Martinez Canillas, linux-kernel, mwen, Maíra Canal,
	dri-devel, kunit-dev, José Expósito, Isabella Basso,
	andrealmeid

On Mon, Sep 05, 2022 at 02:10:00PM +0200, Maxime Ripard wrote:
> On Fri, Sep 02, 2022 at 03:38:28PM +0200, Michał Winiarski wrote:
> > On Fri, Sep 02, 2022 at 04:03:20PM +0300, Jani Nikula wrote:
> > > On Fri, 02 Sep 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> > > > On Fri, Sep 02, 2022 at 11:04:14AM +0300, Jani Nikula wrote:
> > > >> On Thu, 01 Sep 2022, Maíra Canal <mairacanal@riseup.net> wrote:
> > > >> > Hi Maxime,
> > > >> >
> > > >> > On 9/1/22 09:55, Maxime Ripard wrote:
> > > >> >> Hi,
> > > >> >> 
> > > >> >> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
> > > >> >>> With the introduction of KUnit, IGT is no longer the only option to run
> > > >> >>> the DRM unit tests, as the tests can be run through kunit-tool or on
> > > >> >>> real hardware with CONFIG_KUNIT.
> > > >> >>>
> > > >> >>> Therefore, remove the "igt_" prefix from the tests and replace it with
> > > >> >>> the "test_drm_" prefix, making the tests' names independent from the tool
> > > >> >>> used.
> > > >> >>>
> > > >> >>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
> > > >> >>>
> > > >> >>> ---
> > > >> >>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
> > > >> >>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
> > > >> >> 
> > > >> >> I appreciate it's a bit of a bikeshed but I disagree with this. The
> > > >> >> majority of the kunit tests already out there start with the framework
> > > >> >> name, including *all* the examples in the kunit doc. Plus, it's fairly
> > > >> >> obvious that it's a test, kunit is only about running tests in the first
> > > >> >> place.
> > > >> >
> > > >> > Would it be better to keep it as "drm_"?
> > > >> 
> > > >> That's not "keeping". That's renaming igt to drm.
> > > >
> > > > Well, there's like half the tests that are prefixed with drm, the other
> > > > with igt, so it's both really
> > > >
> > > >> > Currently, I don't think it is appropriate to hold the "igt_" prefix, as
> > > >> > the tests are not IGT exclusive, but I don't have a strong opinion on
> > > >> > using the "drm_" or the "test_drm" prefixes.
> > > >> 
> > > >> I repeat my stance that "drm_" alone is confusing.
> > > >
> > > > What are you confusing it with?
> > > >
> > > >> For the reason alone that it pollutes the code tagging tools, mixing
> > > >> actual drm_ types and functions with unit test functions.
> > > >
> > > > I don't get it, I'm sorry. All these functions are static and not part
> > > > of any API, so I can't see how it would pollute a code tagging tool. Or
> > > > at least, not more than any driver does.
> > > >
> > > > And we're part of a larger project here, it's about consistency with the
> > > > rest of the ecosystem.
> > > 
> > > Okay, so I'm just going to explain what I mean, but say "whatever" right
> > > after and move on.
> > > 
> > > For example, drm_buddy_test.c includes drm_buddy.h so with the igt_ ->
> > > drm_ rename none of the test functions may clash with the drm_buddy_
> > > prefixed existing functions. Ditto for all tests similarly.
> > > 
> > > For example drm_buddy_alloc_range() as a name sounds like something that
> > > allocs a range, not something that tests range allocation. On the other
> > > hand, you have drm_buddy_alloc_blocks() which is actually a real
> > > drm_buddy function, not a test. What would you call a test that tests
> > > that? Here, we end up with names that are all prefixed drm_buddy and you
> > > won't know what's the actual function and what's the test unless you
> > > look it up.
> > > 
> > > I use code tagging that I can use for finding and completing
> > > e.g. functions. Currently I have the following completions, for
> > > igt_buddy_ and drm_buddy_, respectively:
> > > 
> > > Possible completions are:
> > > igt_buddy_alloc_limit 	igt_buddy_alloc_optimistic 	igt_buddy_alloc_pathological
> > > igt_buddy_alloc_pessimistic 	igt_buddy_alloc_range 	igt_buddy_alloc_smoke
> > > 
> > > Possible completions are:
> > > drm_buddy_alloc_blocks 	drm_buddy_block 	drm_buddy_block_is_allocated 	drm_buddy_block_is_free
> > > drm_buddy_block_is_split 	drm_buddy_block_offset 	drm_buddy_block_order 	drm_buddy_block_print
> > > drm_buddy_block_size 	drm_buddy_block_state 	drm_buddy_block_trim 	drm_buddy_fini
> > > drm_buddy_free_block 	drm_buddy_free_list 	drm_buddy_init 	drm_buddy_init_test
> > > drm_buddy_module_exit 	drm_buddy_module_init 	drm_buddy_print
> > > 
> > > With the patch at hand, they'll all be lumped under drm_buddy_
> > > completions, and some of them will be actual drm buddy functions and
> > > some not.
> > > 
> > > I just find it a very odd convention to name the tests in a way that's
> > > indistinguishable from the real things. Even *within* drm_buddy_test.c
> > > where you read the test code. Because currently you do have calls to
> > > igt_buddy_ prefixed functions from other igt_buddy_ prefixed functions,
> > > along with the drm_buddy_ prefixed calls. I think it's just going to be
> > > a mess.
> > > 
> > > /rant
> > > 
> > > Whatever. Moving on.
> > 
> > KUnit docs [1] state:
> > https://docs.kernel.org/dev-tools/kunit/style.html#test-cases
> > "As tests are themselves functions, their names cannot conflict with other
> > C identifiers in the kernel. This may require some creative naming."
> > And give examples of names. But this should be local to individual test suite -
> > as long as the test is readable, and the name describes what it is testing, we
> > should be fine. We don't even need to pass drm_* prefix, as this convention is
> > expected for test suites, not test cases [2].
> > 
> > Having said that - I do believe that igt_* prefix don't belong here (which is
> > why I'm progressively trying to get rid of in the patches that refactor some of
> > the tests).
> > I agree with Jani - can we take it on a case-by-case basis?
> > If the test name is too similar to the function that it is testing, we could go
> > with one of the following (taking igt_buddy_alloc_limit as example):
> > drm_buddy_test_alloc_limit
> > test_drm_buddy_alloc_limit
> > buddy_test_alloc_limit
> > test_buddy_alloc_limit
> 
> We also have drm_test_buddy_alloc_limit, or drm_buddy_test_alloc_limit
> 
> Both would be fine for me, with a small preference for the former, which
> should also address Jani's concerns?
> 
> > And either of those is fine in my opinion (I'd personally go with
> > test_buddy_alloc_limit in this case).
> > We don't really need a DRM-wide (or worse, kernel wide) convention for test case
> > names (it's only really needed for test suites).
> 
> Sure we do. kunit.py can take some filters too. Being able to only run
> DRM tests with a single filter is super convenient, and if we fail to
> provide a consistent naming we're pretty much sure people running the
> tests are going to miss some.

We have "test suite" - and we do have a naming convention for that.
Filter glob takes test_suite.test_name.

Let's use specific examples.

Starting with igt_buddy_alloc_limit, "full" test name is
"drm_buddy.igt_buddy_alloc_limit" - the way to run this single test would be:
./kunit.py run 'drm_buddy.igt_buddy_alloc_limit'

Say we want to run all drm_buddy tests, we can do it with:
./kunit.py run 'drm_buddy.*'

We can also run all DRM tests:
./kunit.py run 'drm*'

What you're arguing, is that we need to rename all test cases to have drm_test
prefix, so the full testname for igt_buddy_alloc_limit becomes
drm_buddy.drm_test_buddy_alloc_limit, because we want to be able to use this
specific glob for matching by test case names (not test suites):
./kunit.py run '*.drm*'

Why? We can already do that with 'drm*', that's what the test suites are for,
and the glob is simpler and more intuitive IMO.

-Michał

> 
> Maxime

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_"
  2022-09-05 12:10               ` Maxime Ripard
  2022-09-05 13:11                 ` Michał Winiarski
@ 2022-09-05 13:37                 ` Maíra Canal
  1 sibling, 0 replies; 17+ messages in thread
From: Maíra Canal @ 2022-09-05 13:37 UTC (permalink / raw)
  To: Maxime Ripard, Michał Winiarski
  Cc: Arthur Grillo, siqueirajordao, magalilemes00, David Airlie,
	tales.aparecida, brendanhiggins, Javier Martinez Canillas,
	dri-devel, linux-kernel, mwen, David Gow,
	José Expósito, Isabella Basso, andrealmeid, kunit-dev

On 9/5/22 09:10, Maxime Ripard wrote:
> On Fri, Sep 02, 2022 at 03:38:28PM +0200, Michał Winiarski wrote:
>> On Fri, Sep 02, 2022 at 04:03:20PM +0300, Jani Nikula wrote:
>>> On Fri, 02 Sep 2022, Maxime Ripard <maxime@cerno.tech> wrote:
>>>> On Fri, Sep 02, 2022 at 11:04:14AM +0300, Jani Nikula wrote:
>>>>> On Thu, 01 Sep 2022, Maíra Canal <mairacanal@riseup.net> wrote:
>>>>>> Hi Maxime,
>>>>>>
>>>>>> On 9/1/22 09:55, Maxime Ripard wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Thu, Sep 01, 2022 at 09:42:10AM -0300, Maíra Canal wrote:
>>>>>>>> With the introduction of KUnit, IGT is no longer the only option to run
>>>>>>>> the DRM unit tests, as the tests can be run through kunit-tool or on
>>>>>>>> real hardware with CONFIG_KUNIT.
>>>>>>>>
>>>>>>>> Therefore, remove the "igt_" prefix from the tests and replace it with
>>>>>>>> the "test_drm_" prefix, making the tests' names independent from the tool
>>>>>>>> used.
>>>>>>>>
>>>>>>>> Signed-off-by: Maíra Canal <mairacanal@riseup.net>
>>>>>>>>
>>>>>>>> ---
>>>>>>>> v1 -> v2: https://lore.kernel.org/dri-devel/20220830211603.191734-1-mairacanal@riseup.net/
>>>>>>>> - Change "drm_" prefix to "test_drm_", as "drm_" can be a bit confusing (Jani Nikula).
>>>>>>>
>>>>>>> I appreciate it's a bit of a bikeshed but I disagree with this. The
>>>>>>> majority of the kunit tests already out there start with the framework
>>>>>>> name, including *all* the examples in the kunit doc. Plus, it's fairly
>>>>>>> obvious that it's a test, kunit is only about running tests in the first
>>>>>>> place.
>>>>>>
>>>>>> Would it be better to keep it as "drm_"?
>>>>>
>>>>> That's not "keeping". That's renaming igt to drm.
>>>>
>>>> Well, there's like half the tests that are prefixed with drm, the other
>>>> with igt, so it's both really
>>>>
>>>>>> Currently, I don't think it is appropriate to hold the "igt_" prefix, as
>>>>>> the tests are not IGT exclusive, but I don't have a strong opinion on
>>>>>> using the "drm_" or the "test_drm" prefixes.
>>>>>
>>>>> I repeat my stance that "drm_" alone is confusing.
>>>>
>>>> What are you confusing it with?
>>>>
>>>>> For the reason alone that it pollutes the code tagging tools, mixing
>>>>> actual drm_ types and functions with unit test functions.
>>>>
>>>> I don't get it, I'm sorry. All these functions are static and not part
>>>> of any API, so I can't see how it would pollute a code tagging tool. Or
>>>> at least, not more than any driver does.
>>>>
>>>> And we're part of a larger project here, it's about consistency with the
>>>> rest of the ecosystem.
>>>
>>> Okay, so I'm just going to explain what I mean, but say "whatever" right
>>> after and move on.
>>>
>>> For example, drm_buddy_test.c includes drm_buddy.h so with the igt_ ->
>>> drm_ rename none of the test functions may clash with the drm_buddy_
>>> prefixed existing functions. Ditto for all tests similarly.
>>>
>>> For example drm_buddy_alloc_range() as a name sounds like something that
>>> allocs a range, not something that tests range allocation. On the other
>>> hand, you have drm_buddy_alloc_blocks() which is actually a real
>>> drm_buddy function, not a test. What would you call a test that tests
>>> that? Here, we end up with names that are all prefixed drm_buddy and you
>>> won't know what's the actual function and what's the test unless you
>>> look it up.
>>>
>>> I use code tagging that I can use for finding and completing
>>> e.g. functions. Currently I have the following completions, for
>>> igt_buddy_ and drm_buddy_, respectively:
>>>
>>> Possible completions are:
>>> igt_buddy_alloc_limit 	igt_buddy_alloc_optimistic 	igt_buddy_alloc_pathological
>>> igt_buddy_alloc_pessimistic 	igt_buddy_alloc_range 	igt_buddy_alloc_smoke
>>>
>>> Possible completions are:
>>> drm_buddy_alloc_blocks 	drm_buddy_block 	drm_buddy_block_is_allocated 	drm_buddy_block_is_free
>>> drm_buddy_block_is_split 	drm_buddy_block_offset 	drm_buddy_block_order 	drm_buddy_block_print
>>> drm_buddy_block_size 	drm_buddy_block_state 	drm_buddy_block_trim 	drm_buddy_fini
>>> drm_buddy_free_block 	drm_buddy_free_list 	drm_buddy_init 	drm_buddy_init_test
>>> drm_buddy_module_exit 	drm_buddy_module_init 	drm_buddy_print
>>>
>>> With the patch at hand, they'll all be lumped under drm_buddy_
>>> completions, and some of them will be actual drm buddy functions and
>>> some not.
>>>
>>> I just find it a very odd convention to name the tests in a way that's
>>> indistinguishable from the real things. Even *within* drm_buddy_test.c
>>> where you read the test code. Because currently you do have calls to
>>> igt_buddy_ prefixed functions from other igt_buddy_ prefixed functions,
>>> along with the drm_buddy_ prefixed calls. I think it's just going to be
>>> a mess.
>>>
>>> /rant
>>>
>>> Whatever. Moving on.
>>
>> KUnit docs [1] state:
>> https://docs.kernel.org/dev-tools/kunit/style.html#test-cases
>> "As tests are themselves functions, their names cannot conflict with other
>> C identifiers in the kernel. This may require some creative naming."
>> And give examples of names. But this should be local to individual test suite -
>> as long as the test is readable, and the name describes what it is testing, we
>> should be fine. We don't even need to pass drm_* prefix, as this convention is
>> expected for test suites, not test cases [2].
>>
>> Having said that - I do believe that igt_* prefix don't belong here (which is
>> why I'm progressively trying to get rid of in the patches that refactor some of
>> the tests).
>> I agree with Jani - can we take it on a case-by-case basis?
>> If the test name is too similar to the function that it is testing, we could go
>> with one of the following (taking igt_buddy_alloc_limit as example):
>> drm_buddy_test_alloc_limit
>> test_drm_buddy_alloc_limit
>> buddy_test_alloc_limit
>> test_buddy_alloc_limit
> 
> We also have drm_test_buddy_alloc_limit, or drm_buddy_test_alloc_limit

I will send a v3 with the "drm_test" prefix, as it seems we can get a
bit more consensus with this one.

Best Regards,
- Maíra Canal

> 
> Both would be fine for me, with a small preference for the former, which
> should also address Jani's concerns?
> 
>> And either of those is fine in my opinion (I'd personally go with
>> test_buddy_alloc_limit in this case).
>> We don't really need a DRM-wide (or worse, kernel wide) convention for test case
>> names (it's only really needed for test suites).
> 
> Sure we do. kunit.py can take some filters too. Being able to only run
> DRM tests with a single filter is super convenient, and if we fail to
> provide a consistent naming we're pretty much sure people running the
> tests are going to miss some.
> 
> Maxime

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-09-05 13:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-01 12:42 [PATCH v2 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests Maíra Canal
2022-09-01 12:42 ` [PATCH v2 2/2] drm/tests: Change "igt_" prefix to "test_drm_" Maíra Canal
2022-09-01 12:55   ` Maxime Ripard
2022-09-01 13:17     ` Javier Martinez Canillas
2022-09-01 22:33     ` Maíra Canal
2022-09-02  8:04       ` Jani Nikula
2022-09-02 12:34         ` Maxime Ripard
2022-09-02 13:03           ` Jani Nikula
2022-09-02 13:38             ` Michał Winiarski
2022-09-02 13:56               ` Jani Nikula
2022-09-02 22:53               ` Maíra Canal
2022-09-05 12:10               ` Maxime Ripard
2022-09-05 13:11                 ` Michał Winiarski
2022-09-05 13:37                 ` Maíra Canal
2022-09-02  8:08       ` Maxime Ripard
2022-09-02  8:24         ` Jani Nikula
2022-09-02  7:52 ` [PATCH v2 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests David Gow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).