[PATCH v3 1/4] drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man
Karolina Stolarek
karolina.stolarek at intel.com
Mon Sep 18 13:04:34 UTC 2023
On 18.09.2023 13:48, Karolina Stolarek wrote:
> On 12.09.2023 14:54, Christian König wrote:
>> Am 12.09.23 um 13:49 schrieb Karolina Stolarek:
>>> Test initialization of ttm_resource using different memory domains.
>>> Add tests for a system memory manager and functions that can be
>>> tested without a fully-featured resource manager. Update
>>> ttm_bo_kunit_init() to initialize BO's kref and reservation object.
>>> Export ttm_resource_alloc symbol for test purposes only.
>>>
>>> Signed-off-by: Karolina Stolarek <karolina.stolarek at intel.com>
>>> ---
>>> drivers/gpu/drm/ttm/tests/Makefile | 1 +
>>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 23 ++
>>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 4 +
>>> drivers/gpu/drm/ttm/tests/ttm_resource_test.c | 335 ++++++++++++++++++
>>> drivers/gpu/drm/ttm/ttm_resource.c | 3 +
>>> 5 files changed, 366 insertions(+)
>>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_resource_test.c
>>>
>>> diff --git a/drivers/gpu/drm/ttm/tests/Makefile
>>> b/drivers/gpu/drm/ttm/tests/Makefile
>>> index ec87c4fc1ad5..c92fe2052ef6 100644
>>> --- a/drivers/gpu/drm/ttm/tests/Makefile
>>> +++ b/drivers/gpu/drm/ttm/tests/Makefile
>>> @@ -3,4 +3,5 @@
>>> obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \
>>> ttm_device_test.o \
>>> ttm_pool_test.o \
>>> + ttm_resource_test.o \
>>> ttm_kunit_helpers.o
>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>> index 81661d8827aa..eccc59b981f8 100644
>>> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>> @@ -38,10 +38,33 @@ struct ttm_buffer_object
>>> *ttm_bo_kunit_init(struct kunit *test,
>>> bo->base = gem_obj;
>>> bo->bdev = devs->ttm_dev;
>>> + kref_init(&bo->kref);
>>> +
>>> + dma_resv_init(&bo->base._resv);
>>> + bo->base.resv = &bo->base._resv;
>>> +
>>
>> I'm really wondering if we shouldn't now initialize the GEM object
>> properly?
>>
>> That would also initialize the reservation object if I remember
>> correctly.
>
> Do you mean by using drm_gem_object_init()?
I tried initializing bo.base with drm_gem_object_init() and it's looking
good, but one check in that function makes testing of "Misaligned size"
subtest of ttm_tt_init_basic impossible:
BUG_ON((size & (PAGE_SIZE - 1)) != 0);
I wanted to test if ttm_tt_init properly calculates the number of pages,
but it looks like in the normal circuimstances it already gets a GEM
object of a size that is page-aligned. Would you prefer to write this
subtest as a separate test with a dummy misaligned GEM object, or to
delete this subtest altogether?
Many thanks,
Karolina
>
>>
>> The solution with EXPORT_SYMBOL_FOR_TESTS_ONLY looks really nice I
>> think and apart from that I can't see anything obviously wrong either,
>> but I only skimmed over the code.
>
> I'm also glad with EXPORT_SYMBOL_FOR_TESTS_ONLY solution, it's much
> better now. Right, you can take a closer look at the next version. I'll
> try to get an actual GEM object here, if you think that's feasible.
>
> All the best,
> Karolina
>
>>
>> Regards,
>> Christian.
>>
>>> return bo;
>>> }
>>> EXPORT_SYMBOL_GPL(ttm_bo_kunit_init);
>>> +struct ttm_place *ttm_place_kunit_init(struct kunit *test,
>>> + uint32_t mem_type, uint32_t flags,
>>> + size_t size)
>>> +{
>>> + struct ttm_place *place;
>>> +
>>> + place = kunit_kzalloc(test, sizeof(*place), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, place);
>>> +
>>> + place->mem_type = mem_type;
>>> + place->flags = flags;
>>> + place->fpfn = size >> PAGE_SHIFT;
>>> + place->lpfn = place->fpfn + (size >> PAGE_SHIFT);
>>> +
>>> + return place;
>>> +}
>>> +EXPORT_SYMBOL_GPL(ttm_place_kunit_init);
>>> +
>>> struct ttm_test_devices *ttm_test_devices_basic(struct kunit *test)
>>> {
>>> struct ttm_test_devices *devs;
>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
>>> index e261e3660d0b..f38140f93c05 100644
>>> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
>>> @@ -8,6 +8,7 @@
>>> #include <drm/drm_drv.h>
>>> #include <drm/ttm/ttm_device.h>
>>> #include <drm/ttm/ttm_bo.h>
>>> +#include <drm/ttm/ttm_placement.h>
>>> #include <drm/drm_kunit_helpers.h>
>>> #include <kunit/test.h>
>>> @@ -28,6 +29,9 @@ int ttm_device_kunit_init(struct ttm_test_devices
>>> *priv,
>>> struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
>>> struct ttm_test_devices *devs,
>>> size_t size);
>>> +struct ttm_place *ttm_place_kunit_init(struct kunit *test,
>>> + uint32_t mem_type, uint32_t flags,
>>> + size_t size);
>>> struct ttm_test_devices *ttm_test_devices_basic(struct kunit *test);
>>> struct ttm_test_devices *ttm_test_devices_all(struct kunit *test);
>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
>>> b/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
>>> new file mode 100644
>>> index 000000000000..851cdc43dc37
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
>>> @@ -0,0 +1,335 @@
>>> +// SPDX-License-Identifier: GPL-2.0 AND MIT
>>> +/*
>>> + * Copyright © 2023 Intel Corporation
>>> + */
>>> +#include <drm/ttm/ttm_resource.h>
>>> +
>>> +#include "ttm_kunit_helpers.h"
>>> +
>>> +#define RES_SIZE SZ_4K
>>> +#define TTM_PRIV_DUMMY_REG (TTM_NUM_MEM_TYPES - 1)
>>> +
>>> +struct ttm_resource_test_case {
>>> + const char *description;
>>> + uint32_t mem_type;
>>> + uint32_t flags;
>>> +};
>>> +
>>> +struct ttm_resource_test_priv {
>>> + struct ttm_test_devices *devs;
>>> + struct ttm_buffer_object *bo;
>>> + struct ttm_place *place;
>>> +};
>>> +
>>> +static const struct ttm_resource_manager_func
>>> ttm_resource_manager_mock_funcs = { };
>>> +
>>> +static int ttm_resource_test_init(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv;
>>> +
>>> + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, priv);
>>> +
>>> + priv->devs = ttm_test_devices_all(test);
>>> + KUNIT_ASSERT_NOT_NULL(test, priv->devs);
>>> +
>>> + test->priv = priv;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void ttm_resource_test_fini(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> +
>>> + ttm_test_devices_put(test, priv->devs);
>>> +}
>>> +
>>> +static void ttm_init_test_mocks(struct kunit *test,
>>> + struct ttm_resource_test_priv *priv,
>>> + uint32_t mem_type, uint32_t flags)
>>> +{
>>> + size_t size = RES_SIZE;
>>> +
>>> + /* Make sure we have what we need for a good BO mock */
>>> + KUNIT_ASSERT_NOT_NULL(test, priv->devs->ttm_dev);
>>> +
>>> + priv->bo = ttm_bo_kunit_init(test, priv->devs, size);
>>> + priv->place = ttm_place_kunit_init(test, mem_type, flags, size);
>>> +}
>>> +
>>> +static void ttm_init_test_manager(struct kunit *test,
>>> + struct ttm_resource_test_priv *priv,
>>> + uint32_t mem_type)
>>> +{
>>> + struct ttm_device *ttm_dev = priv->devs->ttm_dev;
>>> + struct ttm_resource_manager *man;
>>> + size_t size = SZ_16K;
>>> +
>>> + man = kunit_kzalloc(test, sizeof(*man), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, man);
>>> +
>>> + man->use_tt = false;
>>> + man->func = &ttm_resource_manager_mock_funcs;
>>> +
>>> + ttm_resource_manager_init(man, ttm_dev, size);
>>> + ttm_set_driver_manager(ttm_dev, mem_type, man);
>>> + ttm_resource_manager_set_used(man, true);
>>> +}
>>> +
>>> +static const struct ttm_resource_test_case ttm_resource_cases[] = {
>>> + {
>>> + .description = "Init resource in TTM_PL_SYSTEM",
>>> + .mem_type = TTM_PL_SYSTEM,
>>> + },
>>> + {
>>> + .description = "Init resource in TTM_PL_VRAM",
>>> + .mem_type = TTM_PL_VRAM,
>>> + },
>>> + {
>>> + .description = "Init resource in a private placement",
>>> + .mem_type = TTM_PRIV_DUMMY_REG,
>>> + },
>>> + {
>>> + .description = "Init resource in TTM_PL_SYSTEM, set
>>> placement flags",
>>> + .mem_type = TTM_PL_SYSTEM,
>>> + .flags = TTM_PL_FLAG_TOPDOWN,
>>> + },
>>> +};
>>> +
>>> +static void ttm_resource_case_desc(const struct
>>> ttm_resource_test_case *t, char *desc)
>>> +{
>>> + strscpy(desc, t->description, KUNIT_PARAM_DESC_SIZE);
>>> +}
>>> +
>>> +KUNIT_ARRAY_PARAM(ttm_resource, ttm_resource_cases,
>>> ttm_resource_case_desc);
>>> +
>>> +static void ttm_resource_init_basic(struct kunit *test)
>>> +{
>>> + const struct ttm_resource_test_case *params = test->param_value;
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> + struct ttm_resource *res;
>>> + struct ttm_buffer_object *bo;
>>> + struct ttm_place *place;
>>> + struct ttm_resource_manager *man;
>>> + uint64_t expected_usage;
>>> +
>>> + ttm_init_test_mocks(test, priv, params->mem_type, params->flags);
>>> + bo = priv->bo;
>>> + place = priv->place;
>>> +
>>> + if (params->mem_type > TTM_PL_SYSTEM)
>>> + ttm_init_test_manager(test, priv, params->mem_type);
>>> +
>>> + res = kunit_kzalloc(test, sizeof(*res), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, res);
>>> +
>>> + man = ttm_manager_type(priv->devs->ttm_dev, place->mem_type);
>>> + expected_usage = man->usage + RES_SIZE;
>>> +
>>> + KUNIT_ASSERT_TRUE(test, list_empty(&man->lru[bo->priority]));
>>> +
>>> + ttm_resource_init(bo, place, res);
>>> +
>>> + KUNIT_ASSERT_EQ(test, res->start, 0);
>>> + KUNIT_ASSERT_EQ(test, res->size, RES_SIZE);
>>> + KUNIT_ASSERT_EQ(test, res->mem_type, place->mem_type);
>>> + KUNIT_ASSERT_EQ(test, res->placement, place->flags);
>>> + KUNIT_ASSERT_PTR_EQ(test, res->bo, bo);
>>> +
>>> + KUNIT_ASSERT_NULL(test, res->bus.addr);
>>> + KUNIT_ASSERT_EQ(test, res->bus.offset, 0);
>>> + KUNIT_ASSERT_FALSE(test, res->bus.is_iomem);
>>> + KUNIT_ASSERT_EQ(test, res->bus.caching, ttm_cached);
>>> + KUNIT_ASSERT_EQ(test, man->usage, expected_usage);
>>> +
>>> + KUNIT_ASSERT_TRUE(test, list_is_singular(&man->lru[bo->priority]));
>>> +
>>> + ttm_resource_fini(man, res);
>>> +}
>>> +
>>> +static void ttm_resource_init_pinned(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> + struct ttm_resource *res;
>>> + struct ttm_buffer_object *bo;
>>> + struct ttm_place *place;
>>> + struct ttm_resource_manager *man;
>>> +
>>> + ttm_init_test_mocks(test, priv, TTM_PL_SYSTEM, 0);
>>> + bo = priv->bo;
>>> + place = priv->place;
>>> +
>>> + man = ttm_manager_type(priv->devs->ttm_dev, place->mem_type);
>>> +
>>> + res = kunit_kzalloc(test, sizeof(*res), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, res);
>>> + KUNIT_ASSERT_TRUE(test, list_empty(&bo->bdev->pinned));
>>> +
>>> + dma_resv_lock(bo->base.resv, NULL);
>>> + ttm_bo_pin(bo);
>>> + ttm_resource_init(bo, place, res);
>>> + KUNIT_ASSERT_TRUE(test, list_is_singular(&bo->bdev->pinned));
>>> +
>>> + ttm_bo_unpin(bo);
>>> + ttm_resource_fini(man, res);
>>> + dma_resv_unlock(bo->base.resv);
>>> +
>>> + KUNIT_ASSERT_TRUE(test, list_empty(&bo->bdev->pinned));
>>> +}
>>> +
>>> +static void ttm_resource_fini_basic(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> + struct ttm_resource *res;
>>> + struct ttm_buffer_object *bo;
>>> + struct ttm_place *place;
>>> + struct ttm_resource_manager *man;
>>> +
>>> + ttm_init_test_mocks(test, priv, TTM_PL_SYSTEM, 0);
>>> + bo = priv->bo;
>>> + place = priv->place;
>>> +
>>> + man = ttm_manager_type(priv->devs->ttm_dev, place->mem_type);
>>> +
>>> + res = kunit_kzalloc(test, sizeof(*res), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, res);
>>> +
>>> + ttm_resource_init(bo, place, res);
>>> + ttm_resource_fini(man, res);
>>> +
>>> + KUNIT_ASSERT_TRUE(test, list_empty(&res->lru));
>>> + KUNIT_ASSERT_EQ(test, man->usage, 0);
>>> +}
>>> +
>>> +static void ttm_resource_manager_init_basic(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> + struct ttm_resource_manager *man;
>>> + size_t size = SZ_16K;
>>> +
>>> + man = kunit_kzalloc(test, sizeof(*man), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, man);
>>> +
>>> + ttm_resource_manager_init(man, priv->devs->ttm_dev, size);
>>> +
>>> + KUNIT_ASSERT_PTR_EQ(test, man->bdev, priv->devs->ttm_dev);
>>> + KUNIT_ASSERT_EQ(test, man->size, size);
>>> + KUNIT_ASSERT_EQ(test, man->usage, 0);
>>> + KUNIT_ASSERT_NULL(test, man->move);
>>> + KUNIT_ASSERT_NOT_NULL(test, &man->move_lock);
>>> +
>>> + for (int i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
>>> + KUNIT_ASSERT_TRUE(test, list_empty(&man->lru[i]));
>>> +}
>>> +
>>> +static void ttm_resource_manager_usage_basic(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> + struct ttm_resource *res;
>>> + struct ttm_buffer_object *bo;
>>> + struct ttm_place *place;
>>> + struct ttm_resource_manager *man;
>>> + uint64_t actual_usage;
>>> +
>>> + ttm_init_test_mocks(test, priv, TTM_PL_SYSTEM,
>>> TTM_PL_FLAG_TOPDOWN);
>>> + bo = priv->bo;
>>> + place = priv->place;
>>> +
>>> + res = kunit_kzalloc(test, sizeof(*res), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, res);
>>> +
>>> + man = ttm_manager_type(priv->devs->ttm_dev, place->mem_type);
>>> +
>>> + ttm_resource_init(bo, place, res);
>>> + actual_usage = ttm_resource_manager_usage(man);
>>> +
>>> + KUNIT_ASSERT_EQ(test, actual_usage, RES_SIZE);
>>> +
>>> + ttm_resource_fini(man, res);
>>> +}
>>> +
>>> +static void ttm_resource_manager_set_used_basic(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> + struct ttm_resource_manager *man;
>>> +
>>> + man = ttm_manager_type(priv->devs->ttm_dev, TTM_PL_SYSTEM);
>>> + KUNIT_ASSERT_TRUE(test, man->use_type);
>>> +
>>> + ttm_resource_manager_set_used(man, false);
>>> + KUNIT_ASSERT_FALSE(test, man->use_type);
>>> +}
>>> +
>>> +static void ttm_sys_man_alloc_basic(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> + struct ttm_resource_manager *man;
>>> + struct ttm_buffer_object *bo;
>>> + struct ttm_place *place;
>>> + struct ttm_resource *res;
>>> + uint32_t mem_type = TTM_PL_SYSTEM;
>>> + int ret;
>>> +
>>> + ttm_init_test_mocks(test, priv, mem_type, 0);
>>> + bo = priv->bo;
>>> + place = priv->place;
>>> +
>>> + man = ttm_manager_type(priv->devs->ttm_dev, mem_type);
>>> + ret = man->func->alloc(man, bo, place, &res);
>>> +
>>> + KUNIT_ASSERT_EQ(test, ret, 0);
>>> + KUNIT_ASSERT_EQ(test, res->size, RES_SIZE);
>>> + KUNIT_ASSERT_EQ(test, res->mem_type, mem_type);
>>> + KUNIT_ASSERT_PTR_EQ(test, res->bo, bo);
>>> +
>>> + ttm_resource_fini(man, res);
>>> +}
>>> +
>>> +static void ttm_sys_man_free_basic(struct kunit *test)
>>> +{
>>> + struct ttm_resource_test_priv *priv = test->priv;
>>> + struct ttm_resource_manager *man;
>>> + struct ttm_buffer_object *bo;
>>> + struct ttm_place *place;
>>> + struct ttm_resource *res;
>>> + uint32_t mem_type = TTM_PL_SYSTEM;
>>> +
>>> + ttm_init_test_mocks(test, priv, mem_type, 0);
>>> + bo = priv->bo;
>>> + place = priv->place;
>>> +
>>> + res = kunit_kzalloc(test, sizeof(*res), GFP_KERNEL);
>>> + KUNIT_ASSERT_NOT_NULL(test, res);
>>> +
>>> + ttm_resource_alloc(bo, place, &res);
>>> +
>>> + man = ttm_manager_type(priv->devs->ttm_dev, mem_type);
>>> + man->func->free(man, res);
>>> +
>>> + KUNIT_ASSERT_TRUE(test, list_empty(&man->lru[bo->priority]));
>>> + KUNIT_ASSERT_EQ(test, man->usage, 0);
>>> +}
>>> +
>>> +static struct kunit_case ttm_resource_test_cases[] = {
>>> + KUNIT_CASE_PARAM(ttm_resource_init_basic, ttm_resource_gen_params),
>>> + KUNIT_CASE(ttm_resource_init_pinned),
>>> + KUNIT_CASE(ttm_resource_fini_basic),
>>> + KUNIT_CASE(ttm_resource_manager_init_basic),
>>> + KUNIT_CASE(ttm_resource_manager_usage_basic),
>>> + KUNIT_CASE(ttm_resource_manager_set_used_basic),
>>> + KUNIT_CASE(ttm_sys_man_alloc_basic),
>>> + KUNIT_CASE(ttm_sys_man_free_basic),
>>> + {}
>>> +};
>>> +
>>> +static struct kunit_suite ttm_resource_test_suite = {
>>> + .name = "ttm_resource",
>>> + .init = ttm_resource_test_init,
>>> + .exit = ttm_resource_test_fini,
>>> + .test_cases = ttm_resource_test_cases,
>>> +};
>>> +
>>> +kunit_test_suites(&ttm_resource_test_suite);
>>> +
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
>>> b/drivers/gpu/drm/ttm/ttm_resource.c
>>> index 46ff9c75bb12..02b96d23fdb9 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>>> @@ -30,6 +30,8 @@
>>> #include <drm/ttm/ttm_placement.h>
>>> #include <drm/ttm/ttm_resource.h>
>>> +#include <drm/drm_util.h>
>>> +
>>> /**
>>> * ttm_lru_bulk_move_init - initialize a bulk move structure
>>> * @bulk: the structure to init
>>> @@ -240,6 +242,7 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
>>> spin_unlock(&bo->bdev->lru_lock);
>>> return 0;
>>> }
>>> +EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_resource_alloc);
>>> void ttm_resource_free(struct ttm_buffer_object *bo, struct
>>> ttm_resource **res)
>>> {
>>
More information about the dri-devel
mailing list