[RFC v2 1/3] drm/ttm: Introduce KUnit tests

Karolina Stolarek karolina.stolarek at intel.com
Thu Jun 29 10:05:27 UTC 2023


Hi Christian,

Thanks a lot for taking a look at my patches.

On 29.06.2023 09:50, Christian König wrote:
> Am 27.06.23 um 10:32 schrieb Karolina Stolarek:
>> Add the initial version of unit tests for ttm_device struct, together
>> with helper functions.
>>
>> Signed-off-by: Karolina Stolarek <karolina.stolarek at intel.com>
>> ---
>>   drivers/gpu/drm/Kconfig                       | 15 +++++
>>   drivers/gpu/drm/ttm/Makefile                  |  1 +
>>   drivers/gpu/drm/ttm/tests/.kunitconfig        |  4 ++
>>   drivers/gpu/drm/ttm/tests/Makefile            |  5 ++
>>   drivers/gpu/drm/ttm/tests/ttm_device_test.c   | 54 +++++++++++++++++
>>   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 59 +++++++++++++++++++
>>   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 29 +++++++++
>>   7 files changed, 167 insertions(+)
>>   create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig
>>   create mode 100644 drivers/gpu/drm/ttm/tests/Makefile
>>   create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c
>>   create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>   create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index afb3b2f5f425..53024e44a2d5 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -194,6 +194,21 @@ config DRM_TTM
>>         GPU memory types. Will be enabled automatically if a device 
>> driver
>>         uses it.
>> +config DRM_TTM_KUNIT_TEST
>> +        tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS
>> +        default n
>> +        depends on DRM && KUNIT
>> +        select DRM_TTM
>> +        select DRM_EXPORT_FOR_TESTS if m
>> +        select DRM_KUNIT_TEST_HELPERS
>> +        default KUNIT_ALL_TESTS
>> +        help
>> +          Enables unit tests for TTM, a GPU memory manager subsystem 
>> used
>> +          to manage memory buffers. This option is mostly useful for 
>> kernel
>> +          developers.
>> +
>> +          If in doubt, say "N".
>> +
>>   config DRM_BUDDY
>>       tristate
>>       depends on DRM
>> diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
>> index f906b22959cf..dad298127226 100644
>> --- a/drivers/gpu/drm/ttm/Makefile
>> +++ b/drivers/gpu/drm/ttm/Makefile
>> @@ -8,3 +8,4 @@ ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o ttm_bo_vm.o 
>> ttm_module.o \
>>   ttm-$(CONFIG_AGP) += ttm_agp_backend.o
>>   obj-$(CONFIG_DRM_TTM) += ttm.o
>> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += tests/
>> diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig 
>> b/drivers/gpu/drm/ttm/tests/.kunitconfig
>> new file mode 100644
>> index 000000000000..75fdce0cd98e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig
>> @@ -0,0 +1,4 @@
>> +CONFIG_KUNIT=y
>> +CONFIG_DRM=y
>> +CONFIG_DRM_KUNIT_TEST_HELPERS=y
>> +CONFIG_DRM_TTM_KUNIT_TEST=y
>> diff --git a/drivers/gpu/drm/ttm/tests/Makefile 
>> b/drivers/gpu/drm/ttm/tests/Makefile
>> new file mode 100644
>> index 000000000000..7917805f37af
>> --- /dev/null
>> +++ b/drivers/gpu/drm/ttm/tests/Makefile
>> @@ -0,0 +1,5 @@
>> +# SPDX-License-Identifier: GPL-2.0 AND MIT
>> +
>> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \
>> +        ttm_device_test.o \
>> +        ttm_kunit_helpers.o
>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c 
>> b/drivers/gpu/drm/ttm/tests/ttm_device_test.c
>> new file mode 100644
>> index 000000000000..08d7314b1e35
>> --- /dev/null
>> +++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c
>> @@ -0,0 +1,54 @@
>> +// SPDX-License-Identifier: GPL-2.0 AND MIT
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +#include <drm/ttm/ttm_resource.h>
>> +#include <drm/ttm/ttm_device.h>
>> +#include <drm/ttm/ttm_placement.h>
>> +
>> +#include "ttm_kunit_helpers.h"
>> +
>> +static void ttm_device_init_basic(struct kunit *test)
>> +{
>> +    struct ttm_test_devices_priv *priv = test->priv;
>> +    struct ttm_device *ttm_dev;
>> +    struct ttm_resource_manager *ttm_sys_man;
>> +    int err;
>> +
>> +    ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
>> +    KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
>> +
>> +    err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false);
>> +    KUNIT_ASSERT_EQ(test, err, 0);
>> +
>> +    KUNIT_EXPECT_PTR_EQ(test, ttm_dev->funcs, &ttm_dev_funcs);
>> +    KUNIT_ASSERT_NOT_NULL(test, ttm_dev->wq);
>> +    KUNIT_ASSERT_NOT_NULL(test, ttm_dev->man_drv[TTM_PL_SYSTEM]);
>> +
>> +    ttm_sys_man = &ttm_dev->sysman;
>> +    KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man);
>> +    KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_tt);
>> +    KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_type);
>> +    KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man->func);
>> +
>> +    KUNIT_EXPECT_PTR_EQ(test, ttm_dev->dev_mapping,
>> +                priv->drm->anon_inode->i_mapping);
>> +
>> +    ttm_device_fini(ttm_dev);
>> +}
>> +
>> +static struct kunit_case ttm_device_test_cases[] = {
>> +    KUNIT_CASE(ttm_device_init_basic),
>> +    {}
>> +};
>> +
>> +static struct kunit_suite ttm_device_test_suite = {
>> +    .name = "ttm_device",
>> +    .init = ttm_test_devices_init,
>> +    .exit = ttm_test_devices_fini,
>> +    .test_cases = ttm_device_test_cases,
>> +};
>> +
>> +kunit_test_suites(&ttm_device_test_suite);
>> +
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c 
>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>> new file mode 100644
>> index 000000000000..d03db0405484
>> --- /dev/null
>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>> @@ -0,0 +1,59 @@
>> +// SPDX-License-Identifier: GPL-2.0 AND MIT
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +#include "ttm_kunit_helpers.h"
>> +
>> +struct ttm_device_funcs ttm_dev_funcs = {
>> +};
>> +EXPORT_SYMBOL_GPL(ttm_dev_funcs);
>> +
>> +int ttm_kunit_helper_alloc_device(struct kunit *test,
> 
> Since this function is only initializing the ttm device I think we 
> should name it ttm_kunit_helper_init_device().
> 
> On the other hand I don't see a good reason why it can't also allocate 
> the device.

I believe that's a leftover from times when I thought I'll reuse DRM 
device between the tests. No problem, I can put them into one function.

All the best,
Karolina

> 
> Apart from that looks like a good start,
> Christian.
> 
>> +                  struct ttm_device *ttm,
>> +                  bool use_dma_alloc,
>> +                  bool use_dma32)
>> +{
>> +    struct ttm_test_devices_priv *priv = test->priv;
>> +    struct drm_device *drm = priv->drm;
>> +    int err;
>> +
>> +    err = ttm_device_init(ttm, &ttm_dev_funcs, drm->dev,
>> +                  drm->anon_inode->i_mapping,
>> +                  drm->vma_offset_manager,
>> +                  use_dma_alloc, use_dma32);
>> +
>> +    return err;
>> +}
>> +EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device);
>> +
>> +int ttm_test_devices_init(struct kunit *test)
>> +{
>> +    struct ttm_test_devices_priv *priv;
>> +
>> +    priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
>> +    KUNIT_ASSERT_NOT_NULL(test, priv);
>> +
>> +    test->priv = priv;
>> +
>> +    priv->dev = drm_kunit_helper_alloc_device(test);
>> +    KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev);
>> +
>> +    priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev,
>> +                            sizeof(*priv->drm), 0,
>> +                            DRIVER_GEM);
>> +    KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm);
>> +
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(ttm_test_devices_init);
>> +
>> +void ttm_test_devices_fini(struct kunit *test)
>> +{
>> +    struct ttm_test_devices_priv *priv = test->priv;
>> +
>> +    drm_kunit_helper_free_device(test, priv->dev);
>> +    drm_dev_put(priv->drm);
>> +}
>> +EXPORT_SYMBOL_GPL(ttm_test_devices_fini);
>> +
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h 
>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
>> new file mode 100644
>> index 000000000000..69fb03b9c4d2
>> --- /dev/null
>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: GPL-2.0 AND MIT */
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +#ifndef TTM_KUNIT_HELPERS_H
>> +#define TTM_KUNIT_HELPERS_H
>> +
>> +#include <drm/drm_drv.h>
>> +#include <drm/ttm/ttm_device.h>
>> +
>> +#include <drm/drm_kunit_helpers.h>
>> +#include <kunit/test.h>
>> +
>> +extern struct ttm_device_funcs ttm_dev_funcs;
>> +
>> +struct ttm_test_devices_priv {
>> +    struct drm_device *drm;
>> +    struct device *dev;
>> +};
>> +
>> +int ttm_kunit_helper_alloc_device(struct kunit *test,
>> +                  struct ttm_device *ttm,
>> +                  bool use_dma_alloc,
>> +                  bool use_dma32);
>> +
>> +int ttm_test_devices_init(struct kunit *test);
>> +void ttm_test_devices_fini(struct kunit *test);
>> +
>> +#endif // TTM_KUNIT_HELPERS_H
> 


More information about the dri-devel mailing list