[PATCH 02/13] drm: add drm_exec selftests v2

[Christian König]

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 <christian.koenig at amd.com>
>> ---
>>   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 000000000000..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 <kunit/test.h>
>> +
>> +#include <linux/module.h>
>> +#include <linux/prime_numbers.h>
>> +
>> +#include <drm/drm_exec.h>
>> +#include <drm/drm_device.h>
>> +#include <drm/drm_gem.h>
>> +
>> +#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");