Re: [PATCH 02/13] drm: add drm_exec selftests v2
Hi Maira,
Am 04.05.23 um 14:07 schrieb Maíra Canal:
Hi Christian,
It would be nice if you use the KUnit macros, instead of pr_info.
yeah this was initially written before the DRM tests moved to KUnit and
I only quickly converted it over. Going to give this a cleanup.
Thanks,
Christian.
On 5/4/23 08:51, Christian König wrote:
Largely just the initial skeleton.
v2: add array test as well
Signed-off-by: Christian König
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/tests/Makefile | 3 +-
drivers/gpu/drm/tests/drm_exec_test.c | 96 +++
3 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/tests/drm_exec_test.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2dc81eb062eb..068e574e234e 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -80,6 +80,7 @@ config DRM_KUNIT_TEST
select DRM_BUDDY
select DRM_EXPORT_FOR_TESTS if m
select DRM_KUNIT_TEST_HELPERS
+ select DRM_EXEC
default KUNIT_ALL_TESTS
help
This builds unit tests for DRM. This option is not useful for
diff --git a/drivers/gpu/drm/tests/Makefile
b/drivers/gpu/drm/tests/Makefile
index bca726a8f483..ba7baa622675 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_modes_test.o \
drm_plane_helper_test.o \
drm_probe_helper_test.o \
- drm_rect_test.o
+ drm_rect_test.o \
+ drm_exec_test.o
CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN)
diff --git a/drivers/gpu/drm/tests/drm_exec_test.c
b/drivers/gpu/drm/tests/drm_exec_test.c
new file mode 100644
index ..26aa13e62d22
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_exec_test.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#define pr_fmt(fmt) "drm_exec: " fmt
+
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+
+#include "../lib/drm_random.h"
+
+static struct drm_device dev;
+
+static void drm_exec_sanitycheck(struct kunit *test)
+{
+ struct drm_exec exec;
+
+ drm_exec_init(&exec, true);
+ drm_exec_fini(&exec);
+ pr_info("%s - ok!\n", __func__);
Here you could use KUNIT_SUCCEED(test).
+}
+
+static void drm_exec_lock1(struct kunit *test)
Is there a reason to call the function drm_exec_lock1 instead of
just drm_exec_lock?
+{
+ struct drm_gem_object gobj = { };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+
+ drm_exec_init(&exec, true);
+ drm_exec_while_not_all_locked(&exec) {
+ ret = drm_exec_prepare_obj(&exec, &gobj, 1);
+ drm_exec_continue_on_contention(&exec);
+ if (ret) {
+ drm_exec_fini(&exec);
+ pr_err("%s - err %d!\n", __func__, ret);
Here you could use KUNIT_FAIL. Same for the other function.
Actually, it would be better if you created a function `exit`
associated with the test suite, where you would call drm_exec_fini,
and checked the ret variable with KUNIT_EXPECT_EQ(test, ret, 0) in
the test.
+ return;
+ }
+ }
+ drm_exec_fini(&exec);
+ pr_info("%s - ok!\n", __func__);
+}
+
+static void drm_exec_lock_array(struct kunit *test)
+{
+ struct drm_gem_object gobj1 = { };
+ struct drm_gem_object gobj2 = { };
+ struct drm_gem_object *array[] = { &gobj1, &gobj2 };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj1, PAGE_SIZE);
+ drm_gem_private_object_init(&dev, &gobj2, PAGE_SIZE);
+
+ drm_exec_init(&exec, true);
+ ret = drm_exec_prepare_array(&exec, array, ARRAY_SIZE(array), 0);
+ if (ret) {
+ drm_exec_fini(&exec);
+ pr_err("%s - err %d!\n", __func__, ret);
+ return;
+ }
+ drm_exec_fini(&exec)> + pr_info("%s - ok!\n", __func__);
+}
+
+static int drm_exec_suite_init(struct kunit_suite *suite)
+{
+ kunit_info(suite, "Testing DRM exec manager\n");
Isn't this already clear by the name of the test?
Best Regards,
- Maíra Canal
+ return 0;
+}
+
+static struct kunit_case drm_exec_tests[] = {
+ KUNIT_CASE(drm_exec_sanitycheck),
+ KUNIT_CASE(drm_exec_lock1),
+ KUNIT_CASE(drm_exec_lock_array),
+ {}
+};
+
+static struct kunit_suite drm_exec_test_suite = {
+ .name = "drm_exec",
+ .suite_init = drm_exec_suite_init,
+ .test_cases = drm_exec_tests,
+};
+
+kunit_test_suite(drm_exec_test_suite);
+
+MODULE_AUTHOR("AMD");
+MODULE_LICENSE("GPL and additional rights");
Re: [PATCH 02/13] drm: add drm_exec selftests v2
Hi Christian,
It would be nice if you use the KUnit macros, instead of pr_info.
On 5/4/23 08:51, Christian König wrote:
Largely just the initial skeleton.
v2: add array test as well
Signed-off-by: Christian König
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/tests/Makefile| 3 +-
drivers/gpu/drm/tests/drm_exec_test.c | 96 +++
3 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/tests/drm_exec_test.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2dc81eb062eb..068e574e234e 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -80,6 +80,7 @@ config DRM_KUNIT_TEST
select DRM_BUDDY
select DRM_EXPORT_FOR_TESTS if m
select DRM_KUNIT_TEST_HELPERS
+ select DRM_EXEC
default KUNIT_ALL_TESTS
help
This builds unit tests for DRM. This option is not useful for
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index bca726a8f483..ba7baa622675 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_modes_test.o \
drm_plane_helper_test.o \
drm_probe_helper_test.o \
- drm_rect_test.o
+ drm_rect_test.o \
+ drm_exec_test.o
CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN)
diff --git a/drivers/gpu/drm/tests/drm_exec_test.c
b/drivers/gpu/drm/tests/drm_exec_test.c
new file mode 100644
index ..26aa13e62d22
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_exec_test.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#define pr_fmt(fmt) "drm_exec: " fmt
+
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+
+#include "../lib/drm_random.h"
+
+static struct drm_device dev;
+
+static void drm_exec_sanitycheck(struct kunit *test)
+{
+ struct drm_exec exec;
+
+ drm_exec_init(&exec, true);
+ drm_exec_fini(&exec);
+ pr_info("%s - ok!\n", __func__);
Here you could use KUNIT_SUCCEED(test).
+}
+
+static void drm_exec_lock1(struct kunit *test)
Is there a reason to call the function drm_exec_lock1 instead of
just drm_exec_lock?
+{
+ struct drm_gem_object gobj = { };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+
+ drm_exec_init(&exec, true);
+ drm_exec_while_not_all_locked(&exec) {
+ ret = drm_exec_prepare_obj(&exec, &gobj, 1);
+ drm_exec_continue_on_contention(&exec);
+ if (ret) {
+ drm_exec_fini(&exec);
+ pr_err("%s - err %d!\n", __func__, ret);
Here you could use KUNIT_FAIL. Same for the other function.
Actually, it would be better if you created a function `exit`
associated with the test suite, where you would call drm_exec_fini,
and checked the ret variable with KUNIT_EXPECT_EQ(test, ret, 0) in
the test.
+ return;
+ }
+ }
+ drm_exec_fini(&exec);
+ pr_info("%s - ok!\n", __func__);
+}
+
+static void drm_exec_lock_array(struct kunit *test)
+{
+ struct drm_gem_object gobj1 = { };
+ struct drm_gem_object gobj2 = { };
+ struct drm_gem_object *array[] = { &gobj1, &gobj2 };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj1, PAGE_SIZE);
+ drm_gem_private_object_init(&dev, &gobj2, PAGE_SIZE);
+
+ drm_exec_init(&exec, true);
+ ret = drm_exec_prepare_array(&exec, array, ARRAY_SIZE(array), 0);
+ if (ret) {
+ drm_exec_fini(&exec);
+ pr_err("%s - err %d!\n", __func__, ret);
+ return;
+ }
+ drm_exec_fini(&exec)> + pr_info("%s - ok!\n", __func__);
+}
+
+static int drm_exec_suite_init(struct kunit_suite *suite)
+{
+ kunit_info(suite, "Testing DRM exec manager\n");
Isn't this already clear by the name of the test?
Best Regards,
- Maíra Canal
+ return 0;
+}
+
+static struct kunit_case drm_exec_tests[] = {
+ KUNIT_CASE(drm_exec_sanitycheck),
+ KUNIT_CASE(drm_exec_lock1),
+ KUNIT_CASE(drm_exec_lock_array),
+ {}
+};
+
+static struct kunit_suite drm_exec_test_suite = {
+ .name = "drm_exec",
+ .suite_init = drm_exec_suite_init,
+ .test_cases = drm_exec_tests,
+};
+
+kunit_test_suite(drm_exec_test_suite);
+
+MODULE_AUTHOR("AMD");
+MODULE_LICENSE("GPL and additional rights");
[PATCH 02/13] drm: add drm_exec selftests v2
Largely just the initial skeleton.
v2: add array test as well
Signed-off-by: Christian König
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/tests/Makefile| 3 +-
drivers/gpu/drm/tests/drm_exec_test.c | 96 +++
3 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/tests/drm_exec_test.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2dc81eb062eb..068e574e234e 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -80,6 +80,7 @@ config DRM_KUNIT_TEST
select DRM_BUDDY
select DRM_EXPORT_FOR_TESTS if m
select DRM_KUNIT_TEST_HELPERS
+ select DRM_EXEC
default KUNIT_ALL_TESTS
help
This builds unit tests for DRM. This option is not useful for
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index bca726a8f483..ba7baa622675 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_modes_test.o \
drm_plane_helper_test.o \
drm_probe_helper_test.o \
- drm_rect_test.o
+ drm_rect_test.o \
+ drm_exec_test.o
CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN)
diff --git a/drivers/gpu/drm/tests/drm_exec_test.c
b/drivers/gpu/drm/tests/drm_exec_test.c
new file mode 100644
index ..26aa13e62d22
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_exec_test.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#define pr_fmt(fmt) "drm_exec: " fmt
+
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+
+#include "../lib/drm_random.h"
+
+static struct drm_device dev;
+
+static void drm_exec_sanitycheck(struct kunit *test)
+{
+ struct drm_exec exec;
+
+ drm_exec_init(&exec, true);
+ drm_exec_fini(&exec);
+ pr_info("%s - ok!\n", __func__);
+}
+
+static void drm_exec_lock1(struct kunit *test)
+{
+ struct drm_gem_object gobj = { };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+
+ drm_exec_init(&exec, true);
+ drm_exec_while_not_all_locked(&exec) {
+ ret = drm_exec_prepare_obj(&exec, &gobj, 1);
+ drm_exec_continue_on_contention(&exec);
+ if (ret) {
+ drm_exec_fini(&exec);
+ pr_err("%s - err %d!\n", __func__, ret);
+ return;
+ }
+ }
+ drm_exec_fini(&exec);
+ pr_info("%s - ok!\n", __func__);
+}
+
+static void drm_exec_lock_array(struct kunit *test)
+{
+ struct drm_gem_object gobj1 = { };
+ struct drm_gem_object gobj2 = { };
+ struct drm_gem_object *array[] = { &gobj1, &gobj2 };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj1, PAGE_SIZE);
+ drm_gem_private_object_init(&dev, &gobj2, PAGE_SIZE);
+
+ drm_exec_init(&exec, true);
+ ret = drm_exec_prepare_array(&exec, array, ARRAY_SIZE(array), 0);
+ if (ret) {
+ drm_exec_fini(&exec);
+ pr_err("%s - err %d!\n", __func__, ret);
+ return;
+ }
+ drm_exec_fini(&exec);
+ pr_info("%s - ok!\n", __func__);
+}
+
+static int drm_exec_suite_init(struct kunit_suite *suite)
+{
+ kunit_info(suite, "Testing DRM exec manager\n");
+ return 0;
+}
+
+static struct kunit_case drm_exec_tests[] = {
+ KUNIT_CASE(drm_exec_sanitycheck),
+ KUNIT_CASE(drm_exec_lock1),
+ KUNIT_CASE(drm_exec_lock_array),
+ {}
+};
+
+static struct kunit_suite drm_exec_test_suite = {
+ .name = "drm_exec",
+ .suite_init = drm_exec_suite_init,
+ .test_cases = drm_exec_tests,
+};
+
+kunit_test_suite(drm_exec_test_suite);
+
+MODULE_AUTHOR("AMD");
+MODULE_LICENSE("GPL and additional rights");
--
2.34.1
