* [PATCH v8 1/8] drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
@ 2023-11-29 12:02 ` Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 2/8] drm/ttm/tests: Add tests for ttm_tt Karolina Stolarek
` (8 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-11-29 12:02 UTC (permalink / raw)
To: dri-devel
Cc: Amaranath Somalapuram, Karolina Stolarek, Christian König,
Andi Shyti
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 a genuine GEM drm
object. Export ttm_resource_alloc for test purposes only.
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Tested-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/ttm/tests/Makefile | 1 +
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 22 +-
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 3 +
drivers/gpu/drm/ttm/tests/ttm_resource_test.c | 335 ++++++++++++++++++
drivers/gpu/drm/ttm/ttm_resource.c | 3 +
5 files changed, 363 insertions(+), 1 deletion(-)
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..779fbc038f17 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -29,19 +29,39 @@ struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
struct ttm_test_devices *devs,
size_t size)
{
- struct drm_gem_object gem_obj = { .size = size };
+ struct drm_gem_object gem_obj = { };
struct ttm_buffer_object *bo;
+ int err;
bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
KUNIT_ASSERT_NOT_NULL(test, bo);
bo->base = gem_obj;
+ err = drm_gem_object_init(devs->drm, &bo->base, size);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
bo->bdev = devs->ttm_dev;
+ kref_init(&bo->kref);
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)
+{
+ 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;
+
+ 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..2f51c833a536 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,8 @@ 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);
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..029e1f094bb0
--- /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);
+}
+
+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)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v8 2/8] drm/ttm/tests: Add tests for ttm_tt
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 1/8] drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man Karolina Stolarek
@ 2023-11-29 12:02 ` Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 3/8] drm/ttm/tests: Add tests for ttm_bo functions Karolina Stolarek
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-11-29 12:02 UTC (permalink / raw)
To: dri-devel
Cc: Amaranath Somalapuram, Karolina Stolarek, Christian König,
Andi Shyti
Test initialization, creation and destruction of ttm_tt instances.
Export ttm_tt_destroy and ttm_tt_create symbols for testing purposes.
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Tested-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com>
---
drivers/gpu/drm/ttm/tests/Makefile | 1 +
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 20 ++
drivers/gpu/drm/ttm/tests/ttm_tt_test.c | 295 ++++++++++++++++++
drivers/gpu/drm/ttm/ttm_tt.c | 3 +
4 files changed, 319 insertions(+)
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_tt_test.c
diff --git a/drivers/gpu/drm/ttm/tests/Makefile b/drivers/gpu/drm/ttm/tests/Makefile
index c92fe2052ef6..f570530bbb60 100644
--- a/drivers/gpu/drm/ttm/tests/Makefile
+++ b/drivers/gpu/drm/ttm/tests/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \
ttm_device_test.o \
ttm_pool_test.o \
ttm_resource_test.o \
+ ttm_tt_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 779fbc038f17..ba4e5c689164 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -2,9 +2,29 @@
/*
* Copyright © 2023 Intel Corporation
*/
+#include <drm/ttm/ttm_tt.h>
+
#include "ttm_kunit_helpers.h"
+static struct ttm_tt *ttm_tt_simple_create(struct ttm_buffer_object *bo,
+ uint32_t page_flags)
+{
+ struct ttm_tt *tt;
+
+ tt = kzalloc(sizeof(*tt), GFP_KERNEL);
+ ttm_tt_init(tt, bo, page_flags, ttm_cached, 0);
+
+ return tt;
+}
+
+static void ttm_tt_simple_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
+{
+ kfree(ttm);
+}
+
struct ttm_device_funcs ttm_dev_funcs = {
+ .ttm_tt_create = ttm_tt_simple_create,
+ .ttm_tt_destroy = ttm_tt_simple_destroy,
};
EXPORT_SYMBOL_GPL(ttm_dev_funcs);
diff --git a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
new file mode 100644
index 000000000000..fd4502c18de6
--- /dev/null
+++ b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
@@ -0,0 +1,295 @@
+// SPDX-License-Identifier: GPL-2.0 AND MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+#include <linux/shmem_fs.h>
+#include <drm/ttm/ttm_tt.h>
+
+#include "ttm_kunit_helpers.h"
+
+#define BO_SIZE SZ_4K
+
+struct ttm_tt_test_case {
+ const char *description;
+ uint32_t size;
+ uint32_t extra_pages_num;
+};
+
+static int ttm_tt_test_init(struct kunit *test)
+{
+ struct ttm_test_devices *priv;
+
+ priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ priv = ttm_test_devices_all(test);
+ test->priv = priv;
+
+ return 0;
+}
+
+static const struct ttm_tt_test_case ttm_tt_init_basic_cases[] = {
+ {
+ .description = "Page-aligned size",
+ .size = SZ_4K,
+ },
+ {
+ .description = "Extra pages requested",
+ .size = SZ_4K,
+ .extra_pages_num = 1,
+ },
+};
+
+static void ttm_tt_init_case_desc(const struct ttm_tt_test_case *t,
+ char *desc)
+{
+ strscpy(desc, t->description, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(ttm_tt_init_basic, ttm_tt_init_basic_cases,
+ ttm_tt_init_case_desc);
+
+static void ttm_tt_init_basic(struct kunit *test)
+{
+ const struct ttm_tt_test_case *params = test->param_value;
+ struct ttm_buffer_object *bo;
+ struct ttm_tt *tt;
+ uint32_t page_flags = TTM_TT_FLAG_ZERO_ALLOC;
+ enum ttm_caching caching = ttm_cached;
+ uint32_t extra_pages = params->extra_pages_num;
+ int num_pages = params->size >> PAGE_SHIFT;
+ int err;
+
+ tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, tt);
+
+ bo = ttm_bo_kunit_init(test, test->priv, params->size);
+
+ err = ttm_tt_init(tt, bo, page_flags, caching, extra_pages);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ KUNIT_ASSERT_EQ(test, tt->num_pages, num_pages + extra_pages);
+
+ KUNIT_ASSERT_EQ(test, tt->page_flags, page_flags);
+ KUNIT_ASSERT_EQ(test, tt->caching, caching);
+
+ KUNIT_ASSERT_NULL(test, tt->dma_address);
+ KUNIT_ASSERT_NULL(test, tt->swap_storage);
+}
+
+static void ttm_tt_init_misaligned(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ttm_tt *tt;
+ enum ttm_caching caching = ttm_cached;
+ uint32_t size = SZ_8K;
+ int num_pages = (size + SZ_4K) >> PAGE_SHIFT;
+ int err;
+
+ tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, tt);
+
+ bo = ttm_bo_kunit_init(test, test->priv, size);
+
+ /* Make the object size misaligned */
+ bo->base.size += 1;
+
+ err = ttm_tt_init(tt, bo, 0, caching, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ KUNIT_ASSERT_EQ(test, tt->num_pages, num_pages);
+}
+
+static void ttm_tt_fini_basic(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ttm_tt *tt;
+ enum ttm_caching caching = ttm_cached;
+ int err;
+
+ tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, tt);
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_tt_init(tt, bo, 0, caching, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ KUNIT_ASSERT_NOT_NULL(test, tt->pages);
+
+ ttm_tt_fini(tt);
+ KUNIT_ASSERT_NULL(test, tt->pages);
+}
+
+static void ttm_tt_fini_sg(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ttm_tt *tt;
+ enum ttm_caching caching = ttm_cached;
+ int err;
+
+ tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, tt);
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_sg_tt_init(tt, bo, 0, caching);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ KUNIT_ASSERT_NOT_NULL(test, tt->dma_address);
+
+ ttm_tt_fini(tt);
+ KUNIT_ASSERT_NULL(test, tt->dma_address);
+}
+
+static void ttm_tt_fini_shmem(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ttm_tt *tt;
+ struct file *shmem;
+ enum ttm_caching caching = ttm_cached;
+ int err;
+
+ tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, tt);
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_tt_init(tt, bo, 0, caching, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ shmem = shmem_file_setup("ttm swap", BO_SIZE, 0);
+ tt->swap_storage = shmem;
+
+ ttm_tt_fini(tt);
+ KUNIT_ASSERT_NULL(test, tt->swap_storage);
+}
+
+static void ttm_tt_create_basic(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ int err;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+ bo->type = ttm_bo_type_device;
+
+ dma_resv_lock(bo->base.resv, NULL);
+ err = ttm_tt_create(bo, false);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_NOT_NULL(test, bo->ttm);
+
+ /* Free manually, as it was allocated outside of KUnit */
+ kfree(bo->ttm);
+}
+
+static void ttm_tt_create_invalid_bo_type(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ int err;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+ bo->type = ttm_bo_type_sg + 1;
+
+ dma_resv_lock(bo->base.resv, NULL);
+ err = ttm_tt_create(bo, false);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, -EINVAL);
+ KUNIT_EXPECT_NULL(test, bo->ttm);
+}
+
+static void ttm_tt_create_ttm_exists(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ttm_tt *tt;
+ enum ttm_caching caching = ttm_cached;
+ int err;
+
+ tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, tt);
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_tt_init(tt, bo, 0, caching, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ bo->ttm = tt;
+
+ dma_resv_lock(bo->base.resv, NULL);
+ err = ttm_tt_create(bo, false);
+ dma_resv_unlock(bo->base.resv);
+
+ /* Expect to keep the previous TTM */
+ KUNIT_ASSERT_EQ(test, err, 0);
+ KUNIT_ASSERT_PTR_EQ(test, tt, bo->ttm);
+}
+
+static struct ttm_tt *ttm_tt_null_create(struct ttm_buffer_object *bo,
+ uint32_t page_flags)
+{
+ return NULL;
+}
+
+static struct ttm_device_funcs ttm_dev_empty_funcs = {
+ .ttm_tt_create = ttm_tt_null_create,
+};
+
+static void ttm_tt_create_failed(struct kunit *test)
+{
+ const struct ttm_test_devices *devs = test->priv;
+ struct ttm_buffer_object *bo;
+ int err;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ /* Update ttm_device_funcs so we don't alloc ttm_tt */
+ devs->ttm_dev->funcs = &ttm_dev_empty_funcs;
+
+ dma_resv_lock(bo->base.resv, NULL);
+ err = ttm_tt_create(bo, false);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_ASSERT_EQ(test, err, -ENOMEM);
+}
+
+static void ttm_tt_destroy_basic(struct kunit *test)
+{
+ const struct ttm_test_devices *devs = test->priv;
+ struct ttm_buffer_object *bo;
+ int err;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ dma_resv_lock(bo->base.resv, NULL);
+ err = ttm_tt_create(bo, false);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_ASSERT_EQ(test, err, 0);
+ KUNIT_ASSERT_NOT_NULL(test, bo->ttm);
+
+ ttm_tt_destroy(devs->ttm_dev, bo->ttm);
+}
+
+static struct kunit_case ttm_tt_test_cases[] = {
+ KUNIT_CASE_PARAM(ttm_tt_init_basic, ttm_tt_init_basic_gen_params),
+ KUNIT_CASE(ttm_tt_init_misaligned),
+ KUNIT_CASE(ttm_tt_fini_basic),
+ KUNIT_CASE(ttm_tt_fini_sg),
+ KUNIT_CASE(ttm_tt_fini_shmem),
+ KUNIT_CASE(ttm_tt_create_basic),
+ KUNIT_CASE(ttm_tt_create_invalid_bo_type),
+ KUNIT_CASE(ttm_tt_create_ttm_exists),
+ KUNIT_CASE(ttm_tt_create_failed),
+ KUNIT_CASE(ttm_tt_destroy_basic),
+ {}
+};
+
+static struct kunit_suite ttm_tt_test_suite = {
+ .name = "ttm_tt",
+ .init = ttm_tt_test_init,
+ .exit = ttm_test_devices_fini,
+ .test_cases = ttm_tt_test_cases,
+};
+
+kunit_test_suites(&ttm_tt_test_suite);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index e0a77671edd6..d978dc539a9b 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -36,6 +36,7 @@
#include <linux/file.h>
#include <linux/module.h>
#include <drm/drm_cache.h>
+#include <drm/drm_util.h>
#include <drm/ttm/ttm_bo.h>
#include <drm/ttm/ttm_tt.h>
@@ -91,6 +92,7 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc)
return 0;
}
+EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_tt_create);
/*
* Allocates storage for pointers to the pages that back the ttm.
@@ -129,6 +131,7 @@ void ttm_tt_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
{
bdev->funcs->ttm_tt_destroy(bdev, ttm);
}
+EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_tt_destroy);
static void ttm_tt_init_fields(struct ttm_tt *ttm,
struct ttm_buffer_object *bo,
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v8 3/8] drm/ttm/tests: Add tests for ttm_bo functions
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 1/8] drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 2/8] drm/ttm/tests: Add tests for ttm_tt Karolina Stolarek
@ 2023-11-29 12:02 ` Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 4/8] drm/ttm/tests: Fix argument in ttm_tt_kunit_init() Karolina Stolarek
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-11-29 12:02 UTC (permalink / raw)
To: dri-devel
Cc: Amaranath Somalapuram, Karolina Stolarek, Christian König,
Andi Shyti
Test reservation and release of TTM buffer objects. Add tests to check
pin and unpin operations.
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Tested-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
drivers/gpu/drm/ttm/tests/Makefile | 1 +
drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 622 ++++++++++++++++++
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 6 +
3 files changed, 629 insertions(+)
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_bo_test.c
diff --git a/drivers/gpu/drm/ttm/tests/Makefile b/drivers/gpu/drm/ttm/tests/Makefile
index f570530bbb60..468535f7eed2 100644
--- a/drivers/gpu/drm/ttm/tests/Makefile
+++ b/drivers/gpu/drm/ttm/tests/Makefile
@@ -5,4 +5,5 @@ obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \
ttm_pool_test.o \
ttm_resource_test.o \
ttm_tt_test.o \
+ ttm_bo_test.o \
ttm_kunit_helpers.o
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
new file mode 100644
index 000000000000..1f8a4f8adc92
--- /dev/null
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
@@ -0,0 +1,622 @@
+// SPDX-License-Identifier: GPL-2.0 AND MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+#include <linux/dma-resv.h>
+#include <linux/kthread.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/mutex.h>
+#include <linux/ww_mutex.h>
+
+#include <drm/ttm/ttm_resource.h>
+#include <drm/ttm/ttm_placement.h>
+#include <drm/ttm/ttm_tt.h>
+
+#include "ttm_kunit_helpers.h"
+
+#define BO_SIZE SZ_8K
+
+struct ttm_bo_test_case {
+ const char *description;
+ bool interruptible;
+ bool no_wait;
+};
+
+static const struct ttm_bo_test_case ttm_bo_reserved_cases[] = {
+ {
+ .description = "Cannot be interrupted and sleeps",
+ .interruptible = false,
+ .no_wait = false,
+ },
+ {
+ .description = "Cannot be interrupted, locks straight away",
+ .interruptible = false,
+ .no_wait = true,
+ },
+ {
+ .description = "Can be interrupted, sleeps",
+ .interruptible = true,
+ .no_wait = false,
+ },
+};
+
+static void ttm_bo_init_case_desc(const struct ttm_bo_test_case *t,
+ char *desc)
+{
+ strscpy(desc, t->description, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(ttm_bo_reserve, ttm_bo_reserved_cases, ttm_bo_init_case_desc);
+
+static void ttm_bo_reserve_optimistic_no_ticket(struct kunit *test)
+{
+ const struct ttm_bo_test_case *params = test->param_value;
+ struct ttm_buffer_object *bo;
+ int err;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_bo_reserve(bo, params->interruptible, params->no_wait, NULL);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ dma_resv_unlock(bo->base.resv);
+}
+
+static void ttm_bo_reserve_locked_no_sleep(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ bool interruptible = false;
+ bool no_wait = true;
+ int err;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ /* Let's lock it beforehand */
+ dma_resv_lock(bo->base.resv, NULL);
+
+ err = ttm_bo_reserve(bo, interruptible, no_wait, NULL);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_ASSERT_EQ(test, err, -EBUSY);
+}
+
+static void ttm_bo_reserve_no_wait_ticket(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ww_acquire_ctx ctx;
+ bool interruptible = false;
+ bool no_wait = true;
+ int err;
+
+ ww_acquire_init(&ctx, &reservation_ww_class);
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_bo_reserve(bo, interruptible, no_wait, &ctx);
+ KUNIT_ASSERT_EQ(test, err, -EBUSY);
+
+ ww_acquire_fini(&ctx);
+}
+
+static void ttm_bo_reserve_double_resv(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ww_acquire_ctx ctx;
+ bool interruptible = false;
+ bool no_wait = false;
+ int err;
+
+ ww_acquire_init(&ctx, &reservation_ww_class);
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_bo_reserve(bo, interruptible, no_wait, &ctx);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ err = ttm_bo_reserve(bo, interruptible, no_wait, &ctx);
+
+ dma_resv_unlock(bo->base.resv);
+ ww_acquire_fini(&ctx);
+
+ KUNIT_ASSERT_EQ(test, err, -EALREADY);
+}
+
+/*
+ * A test case heavily inspired by ww_test_edeadlk_normal(). It injects
+ * a deadlock by manipulating the sequence number of the context that holds
+ * dma_resv lock of bo2 so the other context is "wounded" and has to back off
+ * (indicated by -EDEADLK). The subtest checks if ttm_bo_reserve() properly
+ * propagates that error.
+ */
+static void ttm_bo_reserve_deadlock(struct kunit *test)
+{
+ struct ttm_buffer_object *bo1, *bo2;
+ struct ww_acquire_ctx ctx1, ctx2;
+ bool interruptible = false;
+ bool no_wait = false;
+ int err;
+
+ bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+ bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ ww_acquire_init(&ctx1, &reservation_ww_class);
+ mutex_lock(&bo2->base.resv->lock.base);
+
+ /* The deadlock will be caught by WW mutex, don't warn about it */
+ lock_release(&bo2->base.resv->lock.base.dep_map, 1);
+
+ bo2->base.resv->lock.ctx = &ctx2;
+ ctx2 = ctx1;
+ ctx2.stamp--; /* Make the context holding the lock younger */
+
+ err = ttm_bo_reserve(bo1, interruptible, no_wait, &ctx1);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ err = ttm_bo_reserve(bo2, interruptible, no_wait, &ctx1);
+ KUNIT_ASSERT_EQ(test, err, -EDEADLK);
+
+ dma_resv_unlock(bo1->base.resv);
+ ww_acquire_fini(&ctx1);
+}
+
+#if IS_BUILTIN(CONFIG_DRM_TTM_KUNIT_TEST)
+struct signal_timer {
+ struct timer_list timer;
+ struct ww_acquire_ctx *ctx;
+};
+
+static void signal_for_ttm_bo_reserve(struct timer_list *t)
+{
+ struct signal_timer *s_timer = from_timer(s_timer, t, timer);
+ struct task_struct *task = s_timer->ctx->task;
+
+ do_send_sig_info(SIGTERM, SEND_SIG_PRIV, task, PIDTYPE_PID);
+}
+
+static int threaded_ttm_bo_reserve(void *arg)
+{
+ struct ttm_buffer_object *bo = arg;
+ struct signal_timer s_timer;
+ struct ww_acquire_ctx ctx;
+ bool interruptible = true;
+ bool no_wait = false;
+ int err;
+
+ ww_acquire_init(&ctx, &reservation_ww_class);
+
+ /* Prepare a signal that will interrupt the reservation attempt */
+ timer_setup_on_stack(&s_timer.timer, &signal_for_ttm_bo_reserve, 0);
+ s_timer.ctx = &ctx;
+
+ mod_timer(&s_timer.timer, msecs_to_jiffies(100));
+
+ err = ttm_bo_reserve(bo, interruptible, no_wait, &ctx);
+
+ timer_delete_sync(&s_timer.timer);
+ destroy_timer_on_stack(&s_timer.timer);
+
+ ww_acquire_fini(&ctx);
+
+ return err;
+}
+
+static void ttm_bo_reserve_interrupted(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct task_struct *task;
+ int err;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ task = kthread_create(threaded_ttm_bo_reserve, bo, "ttm-bo-reserve");
+
+ if (IS_ERR(task))
+ KUNIT_FAIL(test, "Couldn't create ttm bo reserve task\n");
+
+ /* Take a lock so the threaded reserve has to wait */
+ mutex_lock(&bo->base.resv->lock.base);
+
+ wake_up_process(task);
+ msleep(20);
+ err = kthread_stop(task);
+
+ mutex_unlock(&bo->base.resv->lock.base);
+
+ KUNIT_ASSERT_EQ(test, err, -ERESTARTSYS);
+}
+#endif /* IS_BUILTIN(CONFIG_DRM_TTM_KUNIT_TEST) */
+
+static void ttm_bo_unreserve_basic(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_device *ttm_dev;
+ struct ttm_resource *res1, *res2;
+ struct ttm_place *place;
+ struct ttm_resource_manager *man;
+ unsigned int bo_prio = TTM_MAX_BO_PRIORITY - 1;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ int err;
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+
+ ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+ err = ttm_device_kunit_init(priv, ttm_dev, false, false);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ priv->ttm_dev = ttm_dev;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+ bo->priority = bo_prio;
+
+ err = ttm_resource_alloc(bo, place, &res1);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ bo->resource = res1;
+
+ /* Add a dummy resource to populate LRU */
+ ttm_resource_alloc(bo, place, &res2);
+
+ dma_resv_lock(bo->base.resv, NULL);
+ ttm_bo_unreserve(bo);
+
+ man = ttm_manager_type(priv->ttm_dev, mem_type);
+ KUNIT_ASSERT_EQ(test,
+ list_is_last(&res1->lru, &man->lru[bo->priority]), 1);
+
+ ttm_resource_free(bo, &res2);
+ ttm_resource_free(bo, &res1);
+}
+
+static void ttm_bo_unreserve_pinned(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_device *ttm_dev;
+ struct ttm_resource *res1, *res2;
+ struct ttm_place *place;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ int err;
+
+ ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+ err = ttm_device_kunit_init(priv, ttm_dev, false, false);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ priv->ttm_dev = ttm_dev;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+ place = ttm_place_kunit_init(test, mem_type, 0);
+
+ dma_resv_lock(bo->base.resv, NULL);
+ ttm_bo_pin(bo);
+
+ err = ttm_resource_alloc(bo, place, &res1);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ bo->resource = res1;
+
+ /* Add a dummy resource to the pinned list */
+ err = ttm_resource_alloc(bo, place, &res2);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test,
+ list_is_last(&res2->lru, &priv->ttm_dev->pinned), 1);
+
+ ttm_bo_unreserve(bo);
+ KUNIT_ASSERT_EQ(test,
+ list_is_last(&res1->lru, &priv->ttm_dev->pinned), 1);
+
+ ttm_resource_free(bo, &res1);
+ ttm_resource_free(bo, &res2);
+}
+
+static void ttm_bo_unreserve_bulk(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_lru_bulk_move lru_bulk_move;
+ struct ttm_lru_bulk_move_pos *pos;
+ struct ttm_buffer_object *bo1, *bo2;
+ struct ttm_resource *res1, *res2;
+ struct ttm_device *ttm_dev;
+ struct ttm_place *place;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ unsigned int bo_priority = 0;
+ int err;
+
+ ttm_lru_bulk_move_init(&lru_bulk_move);
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+
+ ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+ err = ttm_device_kunit_init(priv, ttm_dev, false, false);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ priv->ttm_dev = ttm_dev;
+
+ bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+ bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ dma_resv_lock(bo1->base.resv, NULL);
+ ttm_bo_set_bulk_move(bo1, &lru_bulk_move);
+ dma_resv_unlock(bo1->base.resv);
+
+ err = ttm_resource_alloc(bo1, place, &res1);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ bo1->resource = res1;
+
+ dma_resv_lock(bo2->base.resv, NULL);
+ ttm_bo_set_bulk_move(bo2, &lru_bulk_move);
+ dma_resv_unlock(bo2->base.resv);
+
+ err = ttm_resource_alloc(bo2, place, &res2);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ bo2->resource = res2;
+
+ ttm_bo_reserve(bo1, false, false, NULL);
+ ttm_bo_unreserve(bo1);
+
+ pos = &lru_bulk_move.pos[mem_type][bo_priority];
+ KUNIT_ASSERT_PTR_EQ(test, res1, pos->last);
+
+ ttm_resource_free(bo1, &res1);
+ ttm_resource_free(bo2, &res2);
+}
+
+static void ttm_bo_put_basic(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_resource *res;
+ struct ttm_device *ttm_dev;
+ struct ttm_place *place;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ int err;
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+
+ ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+ err = ttm_device_kunit_init(priv, ttm_dev, false, false);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ priv->ttm_dev = ttm_dev;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+ bo->type = ttm_bo_type_device;
+
+ err = ttm_resource_alloc(bo, place, &res);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ bo->resource = res;
+
+ dma_resv_lock(bo->base.resv, NULL);
+ err = ttm_tt_create(bo, false);
+ dma_resv_unlock(bo->base.resv);
+ KUNIT_EXPECT_EQ(test, err, 0);
+
+ ttm_bo_put(bo);
+}
+
+static const char *mock_name(struct dma_fence *f)
+{
+ return "kunit-ttm-bo-put";
+}
+
+static const struct dma_fence_ops mock_fence_ops = {
+ .get_driver_name = mock_name,
+ .get_timeline_name = mock_name,
+};
+
+static void ttm_bo_put_shared_resv(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct dma_resv *external_resv;
+ struct dma_fence *fence;
+ /* A dummy DMA fence lock */
+ spinlock_t fence_lock;
+ struct ttm_device *ttm_dev;
+ int err;
+
+ ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+ err = ttm_device_kunit_init(priv, ttm_dev, false, false);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ priv->ttm_dev = ttm_dev;
+
+ external_resv = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, external_resv);
+
+ dma_resv_init(external_resv);
+
+ fence = kunit_kzalloc(test, sizeof(*fence), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, fence);
+
+ spin_lock_init(&fence_lock);
+ dma_fence_init(fence, &mock_fence_ops, &fence_lock, 0, 0);
+
+ dma_resv_lock(external_resv, NULL);
+ dma_resv_reserve_fences(external_resv, 1);
+ dma_resv_add_fence(external_resv, fence, DMA_RESV_USAGE_BOOKKEEP);
+ dma_resv_unlock(external_resv);
+
+ dma_fence_signal(fence);
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+ bo->type = ttm_bo_type_device;
+ bo->base.resv = external_resv;
+
+ ttm_bo_put(bo);
+}
+
+static void ttm_bo_pin_basic(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_device *ttm_dev;
+ unsigned int no_pins = 3;
+ int err;
+
+ ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+ err = ttm_device_kunit_init(priv, ttm_dev, false, false);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ priv->ttm_dev = ttm_dev;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ for (int i = 0; i < no_pins; i++) {
+ dma_resv_lock(bo->base.resv, NULL);
+ ttm_bo_pin(bo);
+ dma_resv_unlock(bo->base.resv);
+ }
+
+ KUNIT_ASSERT_EQ(test, bo->pin_count, no_pins);
+}
+
+static void ttm_bo_pin_unpin_resource(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_lru_bulk_move lru_bulk_move;
+ struct ttm_lru_bulk_move_pos *pos;
+ struct ttm_buffer_object *bo;
+ struct ttm_resource *res;
+ struct ttm_device *ttm_dev;
+ struct ttm_place *place;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ unsigned int bo_priority = 0;
+ int err;
+
+ ttm_lru_bulk_move_init(&lru_bulk_move);
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+
+ ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+ err = ttm_device_kunit_init(priv, ttm_dev, false, false);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ priv->ttm_dev = ttm_dev;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_resource_alloc(bo, place, &res);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ bo->resource = res;
+
+ dma_resv_lock(bo->base.resv, NULL);
+ ttm_bo_set_bulk_move(bo, &lru_bulk_move);
+ ttm_bo_pin(bo);
+ dma_resv_unlock(bo->base.resv);
+
+ pos = &lru_bulk_move.pos[mem_type][bo_priority];
+
+ KUNIT_ASSERT_EQ(test, bo->pin_count, 1);
+ KUNIT_ASSERT_NULL(test, pos->first);
+ KUNIT_ASSERT_NULL(test, pos->last);
+
+ dma_resv_lock(bo->base.resv, NULL);
+ ttm_bo_unpin(bo);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_ASSERT_PTR_EQ(test, res, pos->last);
+ KUNIT_ASSERT_EQ(test, bo->pin_count, 0);
+
+ ttm_resource_free(bo, &res);
+}
+
+static void ttm_bo_multiple_pin_one_unpin(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_lru_bulk_move lru_bulk_move;
+ struct ttm_lru_bulk_move_pos *pos;
+ struct ttm_buffer_object *bo;
+ struct ttm_resource *res;
+ struct ttm_device *ttm_dev;
+ struct ttm_place *place;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ unsigned int bo_priority = 0;
+ int err;
+
+ ttm_lru_bulk_move_init(&lru_bulk_move);
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+
+ ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+ err = ttm_device_kunit_init(priv, ttm_dev, false, false);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ priv->ttm_dev = ttm_dev;
+
+ bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+
+ err = ttm_resource_alloc(bo, place, &res);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ bo->resource = res;
+
+ dma_resv_lock(bo->base.resv, NULL);
+ ttm_bo_set_bulk_move(bo, &lru_bulk_move);
+
+ /* Multiple pins */
+ ttm_bo_pin(bo);
+ ttm_bo_pin(bo);
+
+ dma_resv_unlock(bo->base.resv);
+
+ pos = &lru_bulk_move.pos[mem_type][bo_priority];
+
+ KUNIT_ASSERT_EQ(test, bo->pin_count, 2);
+ KUNIT_ASSERT_NULL(test, pos->first);
+ KUNIT_ASSERT_NULL(test, pos->last);
+
+ dma_resv_lock(bo->base.resv, NULL);
+ ttm_bo_unpin(bo);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_ASSERT_EQ(test, bo->pin_count, 1);
+ KUNIT_ASSERT_NULL(test, pos->first);
+ KUNIT_ASSERT_NULL(test, pos->last);
+
+ dma_resv_lock(bo->base.resv, NULL);
+ ttm_bo_unpin(bo);
+ dma_resv_unlock(bo->base.resv);
+
+ ttm_resource_free(bo, &res);
+}
+
+static struct kunit_case ttm_bo_test_cases[] = {
+ KUNIT_CASE_PARAM(ttm_bo_reserve_optimistic_no_ticket,
+ ttm_bo_reserve_gen_params),
+ KUNIT_CASE(ttm_bo_reserve_locked_no_sleep),
+ KUNIT_CASE(ttm_bo_reserve_no_wait_ticket),
+ KUNIT_CASE(ttm_bo_reserve_double_resv),
+#if IS_BUILTIN(CONFIG_DRM_TTM_KUNIT_TEST)
+ KUNIT_CASE(ttm_bo_reserve_interrupted),
+#endif
+ KUNIT_CASE(ttm_bo_reserve_deadlock),
+ KUNIT_CASE(ttm_bo_unreserve_basic),
+ KUNIT_CASE(ttm_bo_unreserve_pinned),
+ KUNIT_CASE(ttm_bo_unreserve_bulk),
+ KUNIT_CASE(ttm_bo_put_basic),
+ KUNIT_CASE(ttm_bo_put_shared_resv),
+ KUNIT_CASE(ttm_bo_pin_basic),
+ KUNIT_CASE(ttm_bo_pin_unpin_resource),
+ KUNIT_CASE(ttm_bo_multiple_pin_one_unpin),
+ {}
+};
+
+static struct kunit_suite ttm_bo_test_suite = {
+ .name = "ttm_bo",
+ .init = ttm_test_devices_init,
+ .exit = ttm_test_devices_fini,
+ .test_cases = ttm_bo_test_cases,
+};
+
+kunit_test_suites(&ttm_bo_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
index ba4e5c689164..7b7c1fa805fc 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -22,6 +22,10 @@ static void ttm_tt_simple_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
kfree(ttm);
}
+static void dummy_ttm_bo_destroy(struct ttm_buffer_object *bo)
+{
+}
+
struct ttm_device_funcs ttm_dev_funcs = {
.ttm_tt_create = ttm_tt_simple_create,
.ttm_tt_destroy = ttm_tt_simple_destroy,
@@ -61,6 +65,8 @@ struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
KUNIT_ASSERT_EQ(test, err, 0);
bo->bdev = devs->ttm_dev;
+ bo->destroy = dummy_ttm_bo_destroy;
+
kref_init(&bo->kref);
return bo;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v8 4/8] drm/ttm/tests: Fix argument in ttm_tt_kunit_init()
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
` (2 preceding siblings ...)
2023-11-29 12:02 ` [PATCH v8 3/8] drm/ttm/tests: Add tests for ttm_bo functions Karolina Stolarek
@ 2023-11-29 12:02 ` Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 5/8] drm/ttm/tests: Use an init function from the helpers lib Karolina Stolarek
` (5 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-11-29 12:02 UTC (permalink / raw)
To: dri-devel
Cc: Amaranath Somalapuram, Karolina Stolarek, Christian König,
Andi Shyti, Dominik Karol Piątkowski
Remove a leftover definition of page order and pass an empty flag value
in ttm_pool_pre_populated().
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Tested-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com>
Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
index 2d9cae8cd984..b97f7b6daf5b 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
@@ -78,10 +78,9 @@ static struct ttm_pool *ttm_pool_pre_populated(struct kunit *test,
struct ttm_test_devices *devs = priv->devs;
struct ttm_pool *pool;
struct ttm_tt *tt;
- unsigned long order = __fls(size / PAGE_SIZE);
int err;
- tt = ttm_tt_kunit_init(test, order, caching, size);
+ tt = ttm_tt_kunit_init(test, 0, caching, size);
KUNIT_ASSERT_NOT_NULL(test, tt);
pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v8 5/8] drm/ttm/tests: Use an init function from the helpers lib
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
` (3 preceding siblings ...)
2023-11-29 12:02 ` [PATCH v8 4/8] drm/ttm/tests: Fix argument in ttm_tt_kunit_init() Karolina Stolarek
@ 2023-11-29 12:02 ` Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 6/8] drm/ttm/tests: Test simple BO creation and validation Karolina Stolarek
` (4 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-11-29 12:02 UTC (permalink / raw)
To: dri-devel; +Cc: Karolina Stolarek, Christian König, Andi Shyti
Add a new helper function that also initializes the device. Use it in
ttm_tt test suite and delete the local definition.
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
---
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 14 ++++++++++++++
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 1 +
drivers/gpu/drm/ttm/tests/ttm_tt_test.c | 15 +--------------
3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
index 7b7c1fa805fc..899a54dbe443 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -150,6 +150,20 @@ int ttm_test_devices_init(struct kunit *test)
}
EXPORT_SYMBOL_GPL(ttm_test_devices_init);
+int ttm_test_devices_all_init(struct kunit *test)
+{
+ struct ttm_test_devices *priv;
+
+ priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ priv = ttm_test_devices_all(test);
+ test->priv = priv;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ttm_test_devices_all_init);
+
void ttm_test_devices_fini(struct kunit *test)
{
ttm_test_devices_put(test, test->priv);
diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
index 2f51c833a536..53bb5834939f 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
@@ -39,6 +39,7 @@ void ttm_test_devices_put(struct kunit *test, struct ttm_test_devices *devs);
/* Generic init/fini for tests that only need DRM/TTM devices */
int ttm_test_devices_init(struct kunit *test);
+int ttm_test_devices_all_init(struct kunit *test);
void ttm_test_devices_fini(struct kunit *test);
#endif // TTM_KUNIT_HELPERS_H
diff --git a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
index fd4502c18de6..a33a426a814d 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
@@ -15,19 +15,6 @@ struct ttm_tt_test_case {
uint32_t extra_pages_num;
};
-static int ttm_tt_test_init(struct kunit *test)
-{
- struct ttm_test_devices *priv;
-
- priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
- KUNIT_ASSERT_NOT_NULL(test, priv);
-
- priv = ttm_test_devices_all(test);
- test->priv = priv;
-
- return 0;
-}
-
static const struct ttm_tt_test_case ttm_tt_init_basic_cases[] = {
{
.description = "Page-aligned size",
@@ -285,7 +272,7 @@ static struct kunit_case ttm_tt_test_cases[] = {
static struct kunit_suite ttm_tt_test_suite = {
.name = "ttm_tt",
- .init = ttm_tt_test_init,
+ .init = ttm_test_devices_all_init,
.exit = ttm_test_devices_fini,
.test_cases = ttm_tt_test_cases,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v8 6/8] drm/ttm/tests: Test simple BO creation and validation
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
` (4 preceding siblings ...)
2023-11-29 12:02 ` [PATCH v8 5/8] drm/ttm/tests: Use an init function from the helpers lib Karolina Stolarek
@ 2023-11-29 12:02 ` Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 7/8] drm/ttm/tests: Add tests with mock resource managers Karolina Stolarek
` (3 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-11-29 12:02 UTC (permalink / raw)
To: dri-devel
Cc: Amaranath Somalapuram, Karolina Stolarek, Christian König,
Andi Shyti
Add tests for ttm_bo_init_reserved() and ttm_bo_validate() that use
sys manager. Define a simple move function in ttm_device_funcs. Expose
destroy callback of the buffer object to make testing of
ttm_bo_init_reserved() behaviour easier.
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Tested-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com>
---
drivers/gpu/drm/ttm/tests/Makefile | 1 +
.../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 211 ++++++++++++++++++
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 14 +-
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 1 +
4 files changed, 226 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
diff --git a/drivers/gpu/drm/ttm/tests/Makefile b/drivers/gpu/drm/ttm/tests/Makefile
index 468535f7eed2..2e5ed63fb414 100644
--- a/drivers/gpu/drm/ttm/tests/Makefile
+++ b/drivers/gpu/drm/ttm/tests/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \
ttm_resource_test.o \
ttm_tt_test.o \
ttm_bo_test.o \
+ ttm_bo_validate_test.o \
ttm_kunit_helpers.o
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
new file mode 100644
index 000000000000..1d50e4ba9775
--- /dev/null
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0 AND MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include <drm/ttm/ttm_resource.h>
+#include <drm/ttm/ttm_placement.h>
+#include <drm/ttm/ttm_tt.h>
+
+#include "ttm_kunit_helpers.h"
+
+#define BO_SIZE SZ_4K
+
+struct ttm_bo_validate_test_case {
+ const char *description;
+ enum ttm_bo_type bo_type;
+ bool with_ttm;
+};
+
+static struct ttm_placement *ttm_placement_kunit_init(struct kunit *test,
+ struct ttm_place *places,
+ unsigned int num_places,
+ struct ttm_place *busy_places,
+ unsigned int num_busy_places)
+{
+ struct ttm_placement *placement;
+
+ placement = kunit_kzalloc(test, sizeof(*placement), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, placement);
+
+ placement->num_placement = num_places;
+ placement->placement = places;
+ placement->num_busy_placement = num_busy_places;
+ placement->busy_placement = busy_places;
+
+ return placement;
+}
+
+static void ttm_bo_validate_case_desc(const struct ttm_bo_validate_test_case *t,
+ char *desc)
+{
+ strscpy(desc, t->description, KUNIT_PARAM_DESC_SIZE);
+}
+
+static const struct ttm_bo_validate_test_case ttm_bo_type_cases[] = {
+ {
+ .description = "Buffer object for userspace",
+ .bo_type = ttm_bo_type_device,
+ },
+ {
+ .description = "Kernel buffer object",
+ .bo_type = ttm_bo_type_kernel,
+ },
+ {
+ .description = "Shared buffer object",
+ .bo_type = ttm_bo_type_sg,
+ },
+};
+
+KUNIT_ARRAY_PARAM(ttm_bo_types, ttm_bo_type_cases,
+ ttm_bo_validate_case_desc);
+
+static void ttm_bo_init_reserved_sys_man(struct kunit *test)
+{
+ const struct ttm_bo_validate_test_case *params = test->param_value;
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ enum ttm_bo_type bo_type = params->bo_type;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, bo);
+
+ place = ttm_place_kunit_init(test, TTM_PL_SYSTEM, 0);
+ placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
+
+ drm_gem_private_object_init(priv->drm, &bo->base, size);
+
+ err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement,
+ PAGE_SIZE, &ctx, NULL, NULL,
+ &dummy_ttm_bo_destroy);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_EQ(test, kref_read(&bo->kref), 1);
+ KUNIT_EXPECT_PTR_EQ(test, bo->bdev, priv->ttm_dev);
+ KUNIT_EXPECT_EQ(test, bo->type, bo_type);
+ KUNIT_EXPECT_EQ(test, bo->page_alignment, PAGE_SIZE);
+ KUNIT_EXPECT_PTR_EQ(test, bo->destroy, &dummy_ttm_bo_destroy);
+ KUNIT_EXPECT_EQ(test, bo->pin_count, 0);
+ KUNIT_EXPECT_NULL(test, bo->bulk_move);
+ KUNIT_EXPECT_NOT_NULL(test, bo->ttm);
+ KUNIT_EXPECT_FALSE(test, ttm_tt_is_populated(bo->ttm));
+ KUNIT_EXPECT_NOT_NULL(test, (void *)bo->base.resv->fences);
+ KUNIT_EXPECT_EQ(test, ctx.bytes_moved, size);
+
+ if (bo_type != ttm_bo_type_kernel)
+ KUNIT_EXPECT_TRUE(test,
+ drm_mm_node_allocated(&bo->base.vma_node.vm_node));
+
+ ttm_resource_free(bo, &bo->resource);
+ ttm_bo_put(bo);
+}
+
+static void ttm_bo_init_reserved_resv(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ struct dma_resv resv;
+ enum ttm_bo_type bo_type = ttm_bo_type_device;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, bo);
+
+ place = ttm_place_kunit_init(test, TTM_PL_SYSTEM, 0);
+ placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
+
+ drm_gem_private_object_init(priv->drm, &bo->base, size);
+ dma_resv_init(&resv);
+ dma_resv_lock(&resv, NULL);
+
+ err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement,
+ PAGE_SIZE, &ctx, NULL, &resv,
+ &dummy_ttm_bo_destroy);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_PTR_EQ(test, bo->base.resv, &resv);
+
+ ttm_resource_free(bo, &bo->resource);
+ ttm_bo_put(bo);
+}
+
+static void ttm_bo_validate_invalid_placement(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ enum ttm_bo_type bo_type = ttm_bo_type_device;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ uint32_t unknown_mem_type = TTM_PL_PRIV + 1;
+ int err;
+
+ place = ttm_place_kunit_init(test, unknown_mem_type, 0);
+ placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
+
+ bo = ttm_bo_kunit_init(test, test->priv, size);
+ bo->type = bo_type;
+
+ ttm_bo_reserve(bo, false, false, NULL);
+ err = ttm_bo_validate(bo, placement, &ctx);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, -EINVAL);
+
+ ttm_bo_put(bo);
+}
+
+static void ttm_bo_validate_pinned(struct kunit *test)
+{
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ enum ttm_bo_type bo_type = ttm_bo_type_device;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+ placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
+
+ bo = ttm_bo_kunit_init(test, test->priv, size);
+ bo->type = bo_type;
+
+ ttm_bo_reserve(bo, false, false, NULL);
+ ttm_bo_pin(bo);
+ err = ttm_bo_validate(bo, placement, &ctx);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, -EINVAL);
+}
+
+static struct kunit_case ttm_bo_validate_test_cases[] = {
+ KUNIT_CASE_PARAM(ttm_bo_init_reserved_sys_man, ttm_bo_types_gen_params),
+ KUNIT_CASE(ttm_bo_init_reserved_resv),
+ KUNIT_CASE(ttm_bo_validate_invalid_placement),
+ KUNIT_CASE(ttm_bo_validate_pinned),
+ {}
+};
+
+static struct kunit_suite ttm_bo_validate_test_suite = {
+ .name = "ttm_bo_validate",
+ .init = ttm_test_devices_all_init,
+ .exit = ttm_test_devices_fini,
+ .test_cases = ttm_bo_validate_test_cases,
+};
+
+kunit_test_suites(&ttm_bo_validate_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
index 899a54dbe443..b07ea760d819 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -22,13 +22,19 @@ static void ttm_tt_simple_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
kfree(ttm);
}
-static void dummy_ttm_bo_destroy(struct ttm_buffer_object *bo)
+static int mock_move(struct ttm_buffer_object *bo, bool evict,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_resource *new_mem,
+ struct ttm_place *hop)
{
+ bo->resource = new_mem;
+ return 0;
}
struct ttm_device_funcs ttm_dev_funcs = {
.ttm_tt_create = ttm_tt_simple_create,
.ttm_tt_destroy = ttm_tt_simple_destroy,
+ .move = mock_move,
};
EXPORT_SYMBOL_GPL(ttm_dev_funcs);
@@ -88,6 +94,12 @@ struct ttm_place *ttm_place_kunit_init(struct kunit *test,
}
EXPORT_SYMBOL_GPL(ttm_place_kunit_init);
+void dummy_ttm_bo_destroy(struct ttm_buffer_object *bo)
+{
+ drm_gem_object_release(&bo->base);
+}
+EXPORT_SYMBOL_GPL(dummy_ttm_bo_destroy);
+
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 53bb5834939f..3e5fe272c54c 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
@@ -31,6 +31,7 @@ struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
size_t size);
struct ttm_place *ttm_place_kunit_init(struct kunit *test,
uint32_t mem_type, uint32_t flags);
+void dummy_ttm_bo_destroy(struct ttm_buffer_object *bo);
struct ttm_test_devices *ttm_test_devices_basic(struct kunit *test);
struct ttm_test_devices *ttm_test_devices_all(struct kunit *test);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v8 7/8] drm/ttm/tests: Add tests with mock resource managers
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
` (5 preceding siblings ...)
2023-11-29 12:02 ` [PATCH v8 6/8] drm/ttm/tests: Test simple BO creation and validation Karolina Stolarek
@ 2023-11-29 12:02 ` Karolina Stolarek
2023-11-29 12:02 ` [PATCH v8 8/8] drm/ttm/tests: Add test cases dependent on fence signaling Karolina Stolarek
` (2 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-11-29 12:02 UTC (permalink / raw)
To: dri-devel
Cc: Amaranath Somalapuram, Karolina Stolarek, Christian König,
Andi Shyti
Add mock resource manager to test ttm_bo_validate() with non-system
placements. Update KConfig entry to enable DRM Buddy allocator, used
by the mock manager. Update move function to do more than just assign
a resource.
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Tested-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com>
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/ttm/tests/.kunitconfig | 1 +
drivers/gpu/drm/ttm/tests/Makefile | 1 +
.../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 276 ++++++++++++++++++
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 39 ++-
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 2 +
drivers/gpu/drm/ttm/tests/ttm_mock_manager.c | 207 +++++++++++++
drivers/gpu/drm/ttm/tests/ttm_mock_manager.h | 31 ++
8 files changed, 556 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_mock_manager.h
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 740c1c0bd068..3be7afa6df3e 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -200,6 +200,7 @@ config DRM_TTM_KUNIT_TEST
default n
depends on DRM && KUNIT && MMU
select DRM_TTM
+ select DRM_BUDDY
select DRM_EXPORT_FOR_TESTS if m
select DRM_KUNIT_TEST_HELPERS
default KUNIT_ALL_TESTS
diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig b/drivers/gpu/drm/ttm/tests/.kunitconfig
index 75fdce0cd98e..9228ce9b913c 100644
--- a/drivers/gpu/drm/ttm/tests/.kunitconfig
+++ b/drivers/gpu/drm/ttm/tests/.kunitconfig
@@ -2,3 +2,4 @@ CONFIG_KUNIT=y
CONFIG_DRM=y
CONFIG_DRM_KUNIT_TEST_HELPERS=y
CONFIG_DRM_TTM_KUNIT_TEST=y
+CONFIG_DRM_BUDDY=y
diff --git a/drivers/gpu/drm/ttm/tests/Makefile b/drivers/gpu/drm/ttm/tests/Makefile
index 2e5ed63fb414..f3149de77541 100644
--- a/drivers/gpu/drm/ttm/tests/Makefile
+++ b/drivers/gpu/drm/ttm/tests/Makefile
@@ -7,4 +7,5 @@ obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \
ttm_tt_test.o \
ttm_bo_test.o \
ttm_bo_validate_test.o \
+ ttm_mock_manager.o \
ttm_kunit_helpers.o
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
index 1d50e4ba9775..5f6c24979f83 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -8,12 +8,15 @@
#include <drm/ttm/ttm_tt.h>
#include "ttm_kunit_helpers.h"
+#include "ttm_mock_manager.h"
#define BO_SIZE SZ_4K
+#define MANAGER_SIZE SZ_1M
struct ttm_bo_validate_test_case {
const char *description;
enum ttm_bo_type bo_type;
+ uint32_t mem_type;
bool with_ttm;
};
@@ -106,6 +109,49 @@ static void ttm_bo_init_reserved_sys_man(struct kunit *test)
ttm_bo_put(bo);
}
+static void ttm_bo_init_reserved_mock_man(struct kunit *test)
+{
+ const struct ttm_bo_validate_test_case *params = test->param_value;
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ enum ttm_bo_type bo_type = params->bo_type;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t mem_type = TTM_PL_VRAM;
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+
+ bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, bo);
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+ placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
+
+ drm_gem_private_object_init(priv->drm, &bo->base, size);
+
+ err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement,
+ PAGE_SIZE, &ctx, NULL, NULL,
+ &dummy_ttm_bo_destroy);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_EQ(test, kref_read(&bo->kref), 1);
+ KUNIT_EXPECT_PTR_EQ(test, bo->bdev, priv->ttm_dev);
+ KUNIT_EXPECT_EQ(test, bo->type, bo_type);
+ KUNIT_EXPECT_EQ(test, ctx.bytes_moved, size);
+
+ if (bo_type != ttm_bo_type_kernel)
+ KUNIT_EXPECT_TRUE(test,
+ drm_mm_node_allocated(&bo->base.vma_node.vm_node));
+
+ ttm_resource_free(bo, &bo->resource);
+ ttm_bo_put(bo);
+ ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+}
+
static void ttm_bo_init_reserved_resv(struct kunit *test)
{
struct ttm_test_devices *priv = test->priv;
@@ -140,6 +186,51 @@ static void ttm_bo_init_reserved_resv(struct kunit *test)
ttm_bo_put(bo);
}
+static void ttm_bo_validate_basic(struct kunit *test)
+{
+ const struct ttm_bo_validate_test_case *params = test->param_value;
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *fst_place, *snd_place;
+ struct ttm_placement *fst_placement, *snd_placement;
+ struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
+ uint32_t size = ALIGN(SZ_8K, PAGE_SIZE);
+ uint32_t fst_mem = TTM_PL_SYSTEM, snd_mem = TTM_PL_VRAM;
+ int err;
+
+ ttm_mock_manager_init(priv->ttm_dev, snd_mem, MANAGER_SIZE);
+
+ fst_place = ttm_place_kunit_init(test, fst_mem, 0);
+ fst_placement = ttm_placement_kunit_init(test, fst_place, 1, NULL, 0);
+
+ bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, bo);
+
+ drm_gem_private_object_init(priv->drm, &bo->base, size);
+
+ err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type,
+ fst_placement, PAGE_SIZE, &ctx_init, NULL,
+ NULL, &dummy_ttm_bo_destroy);
+ KUNIT_EXPECT_EQ(test, err, 0);
+
+ snd_place = ttm_place_kunit_init(test, snd_mem, DRM_BUDDY_TOPDOWN_ALLOCATION);
+ snd_placement = ttm_placement_kunit_init(test, snd_place, 1, NULL, 0);
+
+ err = ttm_bo_validate(bo, snd_placement, &ctx_val);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_EQ(test, ctx_val.bytes_moved, bo->base.size);
+ KUNIT_EXPECT_NOT_NULL(test, bo->ttm);
+ KUNIT_EXPECT_TRUE(test, ttm_tt_is_populated(bo->ttm));
+ KUNIT_EXPECT_EQ(test, bo->resource->mem_type, snd_mem);
+ KUNIT_EXPECT_EQ(test, bo->resource->placement,
+ DRM_BUDDY_TOPDOWN_ALLOCATION);
+
+ ttm_bo_put(bo);
+ ttm_mock_manager_fini(priv->ttm_dev, snd_mem);
+}
+
static void ttm_bo_validate_invalid_placement(struct kunit *test)
{
struct ttm_buffer_object *bo;
@@ -166,6 +257,36 @@ static void ttm_bo_validate_invalid_placement(struct kunit *test)
ttm_bo_put(bo);
}
+static void ttm_bo_validate_failed_alloc(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ uint32_t mem_type = TTM_PL_VRAM;
+ enum ttm_bo_type bo_type = ttm_bo_type_device;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ bo = ttm_bo_kunit_init(test, test->priv, size);
+ bo->type = bo_type;
+
+ ttm_bad_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+ placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
+
+ ttm_bo_reserve(bo, false, false, NULL);
+ err = ttm_bo_validate(bo, placement, &ctx);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, -ENOMEM);
+
+ ttm_bo_put(bo);
+ ttm_bad_manager_fini(priv->ttm_dev, mem_type);
+}
+
static void ttm_bo_validate_pinned(struct kunit *test)
{
struct ttm_buffer_object *bo;
@@ -191,11 +312,166 @@ static void ttm_bo_validate_pinned(struct kunit *test)
KUNIT_EXPECT_EQ(test, err, -EINVAL);
}
+static const struct ttm_bo_validate_test_case ttm_mem_type_cases[] = {
+ {
+ .description = "System manager",
+ .mem_type = TTM_PL_SYSTEM,
+ },
+ {
+ .description = "VRAM manager",
+ .mem_type = TTM_PL_SYSTEM,
+ },
+};
+
+KUNIT_ARRAY_PARAM(ttm_bo_validate_mem, ttm_mem_type_cases,
+ ttm_bo_validate_case_desc);
+
+static void ttm_bo_validate_same_placement(struct kunit *test)
+{
+ const struct ttm_bo_validate_test_case *params = test->param_value;
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ place = ttm_place_kunit_init(test, params->mem_type, 0);
+ placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
+
+ if (params->mem_type != TTM_PL_SYSTEM)
+ ttm_mock_manager_init(priv->ttm_dev, params->mem_type, MANAGER_SIZE);
+
+ bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, bo);
+
+ drm_gem_private_object_init(priv->drm, &bo->base, size);
+
+ err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type,
+ placement, PAGE_SIZE, &ctx_init, NULL,
+ NULL, &dummy_ttm_bo_destroy);
+ KUNIT_EXPECT_EQ(test, err, 0);
+
+ err = ttm_bo_validate(bo, placement, &ctx_val);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_EQ(test, ctx_val.bytes_moved, 0);
+
+ if (params->mem_type != TTM_PL_SYSTEM)
+ ttm_mock_manager_fini(priv->ttm_dev, params->mem_type);
+
+ ttm_bo_put(bo);
+}
+
+static void ttm_bo_validate_busy_placement(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *init_place, *place_bad, *place_busy;
+ struct ttm_placement *placement_init, *placement_val;
+ struct ttm_resource_manager *man;
+ enum ttm_bo_type bo_type = ttm_bo_type_device;
+ struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
+ uint32_t fst_mem = TTM_PL_VRAM, snd_mem = TTM_PL_VRAM + 1;
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ ttm_bad_manager_init(priv->ttm_dev, fst_mem, MANAGER_SIZE);
+ ttm_mock_manager_init(priv->ttm_dev, snd_mem, MANAGER_SIZE);
+
+ init_place = ttm_place_kunit_init(test, TTM_PL_SYSTEM, 0);
+ placement_init = ttm_placement_kunit_init(test, init_place, 1, NULL, 0);
+
+ bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, bo);
+
+ drm_gem_private_object_init(priv->drm, &bo->base, size);
+
+ err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement_init,
+ PAGE_SIZE, &ctx_init, NULL, NULL,
+ &dummy_ttm_bo_destroy);
+ KUNIT_EXPECT_EQ(test, err, 0);
+
+ place_bad = ttm_place_kunit_init(test, fst_mem, 0);
+ place_busy = ttm_place_kunit_init(test, snd_mem, 0);
+ placement_val = ttm_placement_kunit_init(test, place_bad, 1, place_busy, 1);
+
+ err = ttm_bo_validate(bo, placement_val, &ctx_val);
+ dma_resv_unlock(bo->base.resv);
+
+ man = ttm_manager_type(priv->ttm_dev, place_busy->mem_type);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_EQ(test, ctx_val.bytes_moved, bo->base.size);
+ KUNIT_EXPECT_EQ(test, bo->resource->mem_type, snd_mem);
+ KUNIT_ASSERT_TRUE(test, list_is_singular(&man->lru[bo->priority]));
+
+ ttm_bo_put(bo);
+ ttm_bad_manager_fini(priv->ttm_dev, fst_mem);
+ ttm_mock_manager_fini(priv->ttm_dev, snd_mem);
+}
+
+static void ttm_bo_validate_multihop(struct kunit *test)
+{
+ const struct ttm_bo_validate_test_case *params = test->param_value;
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *fst_place, *final_place;
+ struct ttm_placement *placement_init, *placement_val;
+ struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ uint32_t fst_mem = TTM_PL_VRAM, tmp_mem = TTM_PL_TT,
+ final_mem = TTM_PL_MULTIHOP;
+ int err;
+
+ ttm_mock_manager_init(priv->ttm_dev, fst_mem, MANAGER_SIZE);
+ ttm_mock_manager_init(priv->ttm_dev, tmp_mem, MANAGER_SIZE);
+ ttm_mock_manager_init(priv->ttm_dev, final_mem, MANAGER_SIZE);
+
+ fst_place = ttm_place_kunit_init(test, fst_mem, 0);
+ placement_init = ttm_placement_kunit_init(test, fst_place, 1, NULL, 0);
+
+ bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, bo);
+
+ drm_gem_private_object_init(priv->drm, &bo->base, size);
+
+ err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type,
+ placement_init, PAGE_SIZE, &ctx_init, NULL,
+ NULL, &dummy_ttm_bo_destroy);
+ KUNIT_EXPECT_EQ(test, err, 0);
+
+ final_place = ttm_place_kunit_init(test, final_mem, 0);
+ placement_val = ttm_placement_kunit_init(test, final_place, 1, NULL, 0);
+
+ err = ttm_bo_validate(bo, placement_val, &ctx_val);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_EQ(test, ctx_val.bytes_moved, size * 2);
+ KUNIT_EXPECT_EQ(test, bo->resource->mem_type, final_mem);
+
+ ttm_bo_put(bo);
+
+ ttm_mock_manager_fini(priv->ttm_dev, fst_mem);
+ ttm_mock_manager_fini(priv->ttm_dev, tmp_mem);
+ ttm_mock_manager_fini(priv->ttm_dev, final_mem);
+}
+
static struct kunit_case ttm_bo_validate_test_cases[] = {
KUNIT_CASE_PARAM(ttm_bo_init_reserved_sys_man, ttm_bo_types_gen_params),
+ KUNIT_CASE_PARAM(ttm_bo_init_reserved_mock_man, ttm_bo_types_gen_params),
KUNIT_CASE(ttm_bo_init_reserved_resv),
+ KUNIT_CASE_PARAM(ttm_bo_validate_basic, ttm_bo_types_gen_params),
KUNIT_CASE(ttm_bo_validate_invalid_placement),
+ KUNIT_CASE_PARAM(ttm_bo_validate_same_placement,
+ ttm_bo_validate_mem_gen_params),
+ KUNIT_CASE(ttm_bo_validate_failed_alloc),
KUNIT_CASE(ttm_bo_validate_pinned),
+ KUNIT_CASE(ttm_bo_validate_busy_placement),
+ KUNIT_CASE_PARAM(ttm_bo_validate_multihop, ttm_bo_types_gen_params),
{}
};
diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
index b07ea760d819..719c1d56388e 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -27,8 +27,43 @@ static int mock_move(struct ttm_buffer_object *bo, bool evict,
struct ttm_resource *new_mem,
struct ttm_place *hop)
{
- bo->resource = new_mem;
- return 0;
+ struct ttm_resource *old_mem = bo->resource;
+ int ret;
+
+ if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM && !bo->ttm)) {
+ ttm_bo_move_null(bo, new_mem);
+ return 0;
+ }
+
+ /* Dummy multihop */
+ if (bo->resource->mem_type == TTM_PL_VRAM &&
+ new_mem->mem_type == TTM_PL_MULTIHOP) {
+ hop->mem_type = TTM_PL_TT;
+ hop->flags = TTM_PL_FLAG_TEMPORARY;
+ hop->fpfn = 0;
+ hop->lpfn = 0;
+ return -EMULTIHOP;
+ }
+
+ if (old_mem->mem_type == TTM_PL_SYSTEM &&
+ new_mem->mem_type == TTM_PL_TT) {
+ ttm_bo_move_null(bo, new_mem);
+ return 0;
+ }
+
+ if (old_mem->mem_type == TTM_PL_TT &&
+ new_mem->mem_type == TTM_PL_SYSTEM) {
+ ret = ttm_bo_wait_ctx(bo, ctx);
+
+ if (ret)
+ return ret;
+
+ ttm_resource_free(bo, &bo->resource);
+ ttm_bo_assign_mem(bo, new_mem);
+ return 0;
+ }
+
+ return ttm_bo_move_memcpy(bo, ctx, new_mem);
}
struct ttm_device_funcs ttm_dev_funcs = {
diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
index 3e5fe272c54c..a8977eb606e5 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
@@ -13,6 +13,8 @@
#include <drm/drm_kunit_helpers.h>
#include <kunit/test.h>
+#define TTM_PL_MULTIHOP (TTM_PL_PRIV + 1)
+
extern struct ttm_device_funcs ttm_dev_funcs;
struct ttm_test_devices {
diff --git a/drivers/gpu/drm/ttm/tests/ttm_mock_manager.c b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
new file mode 100644
index 000000000000..21636b96ac38
--- /dev/null
+++ b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
@@ -0,0 +1,207 @@
+// 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_mock_manager.h"
+
+static inline struct ttm_mock_manager *
+to_mock_mgr(struct ttm_resource_manager *man)
+{
+ return container_of(man, struct ttm_mock_manager, man);
+}
+
+static inline struct ttm_mock_resource *
+to_mock_mgr_resource(struct ttm_resource *res)
+{
+ return container_of(res, struct ttm_mock_resource, base);
+}
+
+static int ttm_mock_manager_alloc(struct ttm_resource_manager *man,
+ struct ttm_buffer_object *bo,
+ const struct ttm_place *place,
+ struct ttm_resource **res)
+{
+ struct ttm_mock_manager *manager = to_mock_mgr(man);
+ struct ttm_mock_resource *mock_res;
+ struct drm_buddy *mm = &manager->mm;
+ uint64_t lpfn, fpfn, alloc_size;
+ int err;
+
+ mock_res = kzalloc(sizeof(*mock_res), GFP_KERNEL);
+
+ if (!mock_res)
+ return -ENOMEM;
+
+ fpfn = 0;
+ lpfn = man->size;
+
+ ttm_resource_init(bo, place, &mock_res->base);
+ INIT_LIST_HEAD(&mock_res->blocks);
+
+ if (place->flags & TTM_PL_FLAG_TOPDOWN)
+ mock_res->flags |= DRM_BUDDY_TOPDOWN_ALLOCATION;
+
+ if (place->flags & TTM_PL_FLAG_CONTIGUOUS)
+ mock_res->flags |= DRM_BUDDY_CONTIGUOUS_ALLOCATION;
+
+ alloc_size = (uint64_t)mock_res->base.size;
+ mutex_lock(&manager->lock);
+ err = drm_buddy_alloc_blocks(mm, fpfn, lpfn, alloc_size,
+ manager->default_page_size,
+ &mock_res->blocks,
+ mock_res->flags);
+
+ if (err)
+ goto error_free_blocks;
+ mutex_unlock(&manager->lock);
+
+ *res = &mock_res->base;
+ return 0;
+
+error_free_blocks:
+ drm_buddy_free_list(mm, &mock_res->blocks);
+ ttm_resource_fini(man, &mock_res->base);
+ mutex_unlock(&manager->lock);
+
+ return err;
+}
+
+static void ttm_mock_manager_free(struct ttm_resource_manager *man,
+ struct ttm_resource *res)
+{
+ struct ttm_mock_manager *manager = to_mock_mgr(man);
+ struct ttm_mock_resource *mock_res = to_mock_mgr_resource(res);
+ struct drm_buddy *mm = &manager->mm;
+
+ mutex_lock(&manager->lock);
+ drm_buddy_free_list(mm, &mock_res->blocks);
+ mutex_unlock(&manager->lock);
+
+ ttm_resource_fini(man, res);
+ kfree(mock_res);
+}
+
+static const struct ttm_resource_manager_func ttm_mock_manager_funcs = {
+ .alloc = ttm_mock_manager_alloc,
+ .free = ttm_mock_manager_free,
+};
+
+int ttm_mock_manager_init(struct ttm_device *bdev, uint32_t mem_type, uint32_t size)
+{
+ struct ttm_mock_manager *manager;
+ struct ttm_resource_manager *base;
+ int err;
+
+ manager = kzalloc(sizeof(*manager), GFP_KERNEL);
+ if (!manager)
+ return -ENOMEM;
+
+ mutex_init(&manager->lock);
+
+ err = drm_buddy_init(&manager->mm, size, PAGE_SIZE);
+
+ if (err) {
+ kfree(manager);
+ return err;
+ }
+
+ manager->default_page_size = PAGE_SIZE;
+ base = &manager->man;
+ base->func = &ttm_mock_manager_funcs;
+ base->use_tt = true;
+
+ ttm_resource_manager_init(base, bdev, size);
+ ttm_set_driver_manager(bdev, mem_type, base);
+ ttm_resource_manager_set_used(base, true);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ttm_mock_manager_init);
+
+void ttm_mock_manager_fini(struct ttm_device *bdev, uint32_t mem_type)
+{
+ struct ttm_resource_manager *man;
+ struct ttm_mock_manager *mock_man;
+ int err;
+
+ man = ttm_manager_type(bdev, mem_type);
+ mock_man = to_mock_mgr(man);
+
+ err = ttm_resource_manager_evict_all(bdev, man);
+ if (err)
+ return;
+
+ ttm_resource_manager_set_used(man, false);
+
+ mutex_lock(&mock_man->lock);
+ drm_buddy_fini(&mock_man->mm);
+ mutex_unlock(&mock_man->lock);
+
+ ttm_set_driver_manager(bdev, mem_type, NULL);
+}
+EXPORT_SYMBOL_GPL(ttm_mock_manager_fini);
+
+static int ttm_bad_manager_alloc(struct ttm_resource_manager *man,
+ struct ttm_buffer_object *bo,
+ const struct ttm_place *place,
+ struct ttm_resource **res)
+{
+ return -ENOSPC;
+}
+
+static void ttm_bad_manager_free(struct ttm_resource_manager *man,
+ struct ttm_resource *res)
+{
+}
+
+static bool ttm_bad_manager_compatible(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ return true;
+}
+
+static const struct ttm_resource_manager_func ttm_bad_manager_funcs = {
+ .alloc = ttm_bad_manager_alloc,
+ .free = ttm_bad_manager_free,
+ .compatible = ttm_bad_manager_compatible
+};
+
+int ttm_bad_manager_init(struct ttm_device *bdev, uint32_t mem_type,
+ uint32_t size)
+{
+ struct ttm_resource_manager *man;
+
+ man = kzalloc(sizeof(*man), GFP_KERNEL);
+ if (!man)
+ return -ENOMEM;
+
+ man->func = &ttm_bad_manager_funcs;
+
+ ttm_resource_manager_init(man, bdev, size);
+ ttm_set_driver_manager(bdev, mem_type, man);
+ ttm_resource_manager_set_used(man, true);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ttm_bad_manager_init);
+
+void ttm_bad_manager_fini(struct ttm_device *bdev, uint32_t mem_type)
+{
+ struct ttm_resource_manager *man;
+
+ man = ttm_manager_type(bdev, mem_type);
+
+ ttm_resource_manager_set_used(man, false);
+ ttm_set_driver_manager(bdev, mem_type, NULL);
+
+ kfree(man);
+}
+EXPORT_SYMBOL_GPL(ttm_bad_manager_fini);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/ttm/tests/ttm_mock_manager.h b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.h
new file mode 100644
index 000000000000..d2db9de9d876
--- /dev/null
+++ b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 AND MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+#ifndef TTM_MOCK_MANAGER_H
+#define TTM_MOCK_MANAGER_H
+
+#include <drm/drm_buddy.h>
+
+struct ttm_mock_manager {
+ struct ttm_resource_manager man;
+ struct drm_buddy mm;
+ uint64_t default_page_size;
+ /* protects allocations of mock buffer objects */
+ struct mutex lock;
+};
+
+struct ttm_mock_resource {
+ struct ttm_resource base;
+ struct list_head blocks;
+ unsigned long flags;
+};
+
+int ttm_mock_manager_init(struct ttm_device *bdev, uint32_t mem_type,
+ uint32_t size);
+int ttm_bad_manager_init(struct ttm_device *bdev, uint32_t mem_type,
+ uint32_t size);
+void ttm_mock_manager_fini(struct ttm_device *bdev, uint32_t mem_type);
+void ttm_bad_manager_fini(struct ttm_device *bdev, uint32_t mem_type);
+
+#endif // TTM_MOCK_MANAGER_H
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v8 8/8] drm/ttm/tests: Add test cases dependent on fence signaling
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
` (6 preceding siblings ...)
2023-11-29 12:02 ` [PATCH v8 7/8] drm/ttm/tests: Add tests with mock resource managers Karolina Stolarek
@ 2023-11-29 12:02 ` Karolina Stolarek
2023-12-05 15:29 ` Andi Shyti
2023-12-05 15:39 ` [PATCH v8 0/8] Improve test coverage of TTM Andi Shyti
2023-12-14 8:20 ` Karolina Stolarek
9 siblings, 1 reply; 17+ messages in thread
From: Karolina Stolarek @ 2023-11-29 12:02 UTC (permalink / raw)
To: dri-devel
Cc: Amaranath Somalapuram, Karolina Stolarek, Christian König,
Andi Shyti
Add test cases that check how the state of dma fences in BO's
reservation object influence the ttm_bo_validation() flow. Do similar
tests for resource manager's move fence.
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Tested-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com>
---
.../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 308 ++++++++++++++++++
1 file changed, 308 insertions(+)
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
index 5f6c24979f83..a659fc837186 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -2,6 +2,8 @@
/*
* Copyright © 2023 Intel Corporation
*/
+#include <linux/delay.h>
+#include <linux/kthread.h>
#include <drm/ttm/ttm_resource.h>
#include <drm/ttm/ttm_placement.h>
@@ -13,11 +15,14 @@
#define BO_SIZE SZ_4K
#define MANAGER_SIZE SZ_1M
+static struct spinlock fence_lock;
+
struct ttm_bo_validate_test_case {
const char *description;
enum ttm_bo_type bo_type;
uint32_t mem_type;
bool with_ttm;
+ bool no_gpu_wait;
};
static struct ttm_placement *ttm_placement_kunit_init(struct kunit *test,
@@ -39,6 +44,43 @@ static struct ttm_placement *ttm_placement_kunit_init(struct kunit *test,
return placement;
}
+static const char *fence_name(struct dma_fence *f)
+{
+ return "ttm-bo-validate-fence";
+}
+
+static const struct dma_fence_ops fence_ops = {
+ .get_driver_name = fence_name,
+ .get_timeline_name = fence_name,
+};
+
+static struct dma_fence *alloc_mock_fence(struct kunit *test)
+{
+ struct dma_fence *fence;
+
+ fence = kunit_kzalloc(test, sizeof(*fence), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, fence);
+
+ dma_fence_init(fence, &fence_ops, &fence_lock, 0, 0);
+
+ return fence;
+}
+
+static void dma_resv_kunit_active_fence_init(struct kunit *test,
+ struct dma_resv *resv,
+ enum dma_resv_usage usage)
+{
+ struct dma_fence *fence;
+
+ fence = alloc_mock_fence(test);
+ dma_fence_enable_sw_signaling(fence);
+
+ dma_resv_lock(resv, NULL);
+ dma_resv_reserve_fences(resv, 1);
+ dma_resv_add_fence(resv, fence, usage);
+ dma_resv_unlock(resv);
+}
+
static void ttm_bo_validate_case_desc(const struct ttm_bo_validate_test_case *t,
char *desc)
{
@@ -460,6 +502,265 @@ static void ttm_bo_validate_multihop(struct kunit *test)
ttm_mock_manager_fini(priv->ttm_dev, final_mem);
}
+static const struct ttm_bo_validate_test_case ttm_bo_no_placement_cases[] = {
+ {
+ .description = "Buffer object in system domain, no page vector",
+ },
+ {
+ .description = "Buffer object in system domain with an existing page vector",
+ .with_ttm = true,
+ },
+};
+
+KUNIT_ARRAY_PARAM(ttm_bo_no_placement, ttm_bo_no_placement_cases,
+ ttm_bo_validate_case_desc);
+
+static void ttm_bo_validate_no_placement_signaled(struct kunit *test)
+{
+ const struct ttm_bo_validate_test_case *params = test->param_value;
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_tt *old_tt;
+ struct ttm_placement *placement;
+ struct ttm_resource_manager *man;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ enum ttm_bo_type bo_type = ttm_bo_type_device;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ uint32_t flags;
+ int err;
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+ man = ttm_manager_type(priv->ttm_dev, mem_type);
+
+ bo = ttm_bo_kunit_init(test, test->priv, size);
+ bo->type = bo_type;
+
+ if (params->with_ttm) {
+ old_tt = priv->ttm_dev->funcs->ttm_tt_create(bo, 0);
+ ttm_pool_alloc(&priv->ttm_dev->pool, old_tt, &ctx);
+ bo->ttm = old_tt;
+ }
+
+ err = ttm_resource_alloc(bo, place, &bo->resource);
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, man->usage, size);
+
+ placement = kunit_kzalloc(test, sizeof(*placement), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, placement);
+
+ ttm_bo_reserve(bo, false, false, NULL);
+ err = ttm_bo_validate(bo, placement, &ctx);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, man->usage, 0);
+ KUNIT_ASSERT_NOT_NULL(test, bo->ttm);
+ KUNIT_EXPECT_EQ(test, ctx.bytes_moved, 0);
+
+ if (params->with_ttm) {
+ flags = bo->ttm->page_flags;
+
+ KUNIT_ASSERT_PTR_EQ(test, bo->ttm, old_tt);
+ KUNIT_ASSERT_FALSE(test, flags & TTM_TT_FLAG_PRIV_POPULATED);
+ KUNIT_ASSERT_TRUE(test, flags & TTM_TT_FLAG_ZERO_ALLOC);
+ }
+
+ ttm_bo_put(bo);
+}
+
+static int threaded_dma_resv_signal(void *arg)
+{
+ struct ttm_buffer_object *bo = arg;
+ struct dma_resv *resv = bo->base.resv;
+ struct dma_resv_iter cursor;
+ struct dma_fence *fence;
+
+ dma_resv_iter_begin(&cursor, resv, DMA_RESV_USAGE_BOOKKEEP);
+ dma_resv_for_each_fence_unlocked(&cursor, fence) {
+ dma_fence_signal(fence);
+ }
+ dma_resv_iter_end(&cursor);
+
+ return 0;
+}
+
+static void ttm_bo_validate_no_placement_not_signaled(struct kunit *test)
+{
+ const struct ttm_bo_validate_test_case *params = test->param_value;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ struct task_struct *task;
+ enum dma_resv_usage usage = DMA_RESV_USAGE_BOOKKEEP;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+
+ bo = ttm_bo_kunit_init(test, test->priv, size);
+ bo->type = params->bo_type;
+
+ err = ttm_resource_alloc(bo, place, &bo->resource);
+ KUNIT_EXPECT_EQ(test, err, 0);
+
+ placement = kunit_kzalloc(test, sizeof(*placement), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, placement);
+
+ /* Create an active fence to simulate a non-idle resv object */
+ spin_lock_init(&fence_lock);
+ dma_resv_kunit_active_fence_init(test, bo->base.resv, usage);
+
+ task = kthread_create(threaded_dma_resv_signal, bo, "dma-resv-signal");
+ if (IS_ERR(task))
+ KUNIT_FAIL(test, "Couldn't create dma resv signal task\n");
+
+ wake_up_process(task);
+ ttm_bo_reserve(bo, false, false, NULL);
+ err = ttm_bo_validate(bo, placement, &ctx);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_NOT_NULL(test, bo->ttm);
+ KUNIT_ASSERT_NULL(test, bo->resource);
+ KUNIT_ASSERT_NULL(test, bo->bulk_move);
+ KUNIT_EXPECT_EQ(test, ctx.bytes_moved, 0);
+
+ if (bo->type != ttm_bo_type_sg)
+ KUNIT_ASSERT_PTR_EQ(test, bo->base.resv, &bo->base._resv);
+
+ /* Make sure we have an idle object at this point */
+ dma_resv_wait_timeout(bo->base.resv, usage, false, MAX_SCHEDULE_TIMEOUT);
+
+ ttm_bo_put(bo);
+}
+
+static void ttm_bo_validate_move_fence_signaled(struct kunit *test)
+{
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *place;
+ struct ttm_placement *placement;
+ struct ttm_resource_manager *man;
+ enum ttm_bo_type bo_type = ttm_bo_type_device;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ struct ttm_operation_ctx ctx = { };
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ int err;
+
+ man = ttm_manager_type(priv->ttm_dev, mem_type);
+ spin_lock_init(&fence_lock);
+ man->move = dma_fence_get_stub();
+
+ bo = ttm_bo_kunit_init(test, test->priv, size);
+ bo->type = bo_type;
+
+ place = ttm_place_kunit_init(test, mem_type, 0);
+ placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
+
+ ttm_bo_reserve(bo, false, false, NULL);
+ err = ttm_bo_validate(bo, placement, &ctx);
+ dma_resv_unlock(bo->base.resv);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_EQ(test, bo->resource->mem_type, mem_type);
+ KUNIT_EXPECT_EQ(test, ctx.bytes_moved, size);
+
+ ttm_bo_put(bo);
+ dma_fence_put(man->move);
+}
+
+static const struct ttm_bo_validate_test_case ttm_bo_validate_wait_cases[] = {
+ {
+ .description = "Waits for GPU",
+ .no_gpu_wait = false,
+ },
+ {
+ .description = "Tries to lock straight away",
+ .no_gpu_wait = true,
+ },
+};
+
+KUNIT_ARRAY_PARAM(ttm_bo_validate_wait, ttm_bo_validate_wait_cases,
+ ttm_bo_validate_case_desc);
+
+static int threaded_fence_signal(void *arg)
+{
+ struct dma_fence *fence = arg;
+ int err;
+
+ msleep(20);
+ err = dma_fence_signal(fence);
+
+ return err;
+}
+
+static void ttm_bo_validate_move_fence_not_signaled(struct kunit *test)
+{
+ const struct ttm_bo_validate_test_case *params = test->param_value;
+ struct ttm_test_devices *priv = test->priv;
+ struct ttm_buffer_object *bo;
+ struct ttm_place *init_place, *fst_place, *snd_place;
+ struct ttm_placement *placement_init, *placement_val;
+ struct task_struct *task;
+ struct ttm_resource_manager *man;
+ enum ttm_bo_type bo_type = ttm_bo_type_device;
+ uint32_t fst_mem = TTM_PL_VRAM, snd_mem = TTM_PL_VRAM + 1;
+ uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+ struct ttm_operation_ctx ctx_init = { },
+ ctx_val = { .no_wait_gpu = params->no_gpu_wait };
+ int err;
+
+ init_place = ttm_place_kunit_init(test, TTM_PL_SYSTEM, 0);
+ placement_init = ttm_placement_kunit_init(test, init_place, 1, NULL, 0);
+
+ bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, bo);
+
+ drm_gem_private_object_init(priv->drm, &bo->base, size);
+
+ err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement_init,
+ PAGE_SIZE, &ctx_init, NULL, NULL,
+ &dummy_ttm_bo_destroy);
+ KUNIT_EXPECT_EQ(test, err, 0);
+
+ ttm_mock_manager_init(priv->ttm_dev, fst_mem, MANAGER_SIZE);
+ ttm_mock_manager_init(priv->ttm_dev, snd_mem, MANAGER_SIZE);
+
+ fst_place = ttm_place_kunit_init(test, fst_mem, 0);
+ snd_place = ttm_place_kunit_init(test, snd_mem, 0);
+ placement_val = ttm_placement_kunit_init(test, fst_place, 1, snd_place, 1);
+
+ spin_lock_init(&fence_lock);
+ man = ttm_manager_type(priv->ttm_dev, fst_place->mem_type);
+ man->move = alloc_mock_fence(test);
+
+ task = kthread_create(threaded_fence_signal, man->move, "move-fence-signal");
+ if (IS_ERR(task))
+ KUNIT_FAIL(test, "Couldn't create move fence signal task\n");
+
+ wake_up_process(task);
+ err = ttm_bo_validate(bo, placement_val, &ctx_val);
+ dma_resv_unlock(bo->base.resv);
+
+ dma_fence_wait_timeout(man->move, false, MAX_SCHEDULE_TIMEOUT);
+
+ KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_EXPECT_EQ(test, ctx_val.bytes_moved, size);
+
+ if (params->no_gpu_wait)
+ KUNIT_EXPECT_EQ(test, bo->resource->mem_type, snd_mem);
+ else
+ KUNIT_EXPECT_EQ(test, bo->resource->mem_type, fst_mem);
+
+ ttm_bo_put(bo);
+ ttm_mock_manager_fini(priv->ttm_dev, fst_mem);
+ ttm_mock_manager_fini(priv->ttm_dev, snd_mem);
+}
+
static struct kunit_case ttm_bo_validate_test_cases[] = {
KUNIT_CASE_PARAM(ttm_bo_init_reserved_sys_man, ttm_bo_types_gen_params),
KUNIT_CASE_PARAM(ttm_bo_init_reserved_mock_man, ttm_bo_types_gen_params),
@@ -472,6 +773,13 @@ static struct kunit_case ttm_bo_validate_test_cases[] = {
KUNIT_CASE(ttm_bo_validate_pinned),
KUNIT_CASE(ttm_bo_validate_busy_placement),
KUNIT_CASE_PARAM(ttm_bo_validate_multihop, ttm_bo_types_gen_params),
+ KUNIT_CASE_PARAM(ttm_bo_validate_no_placement_signaled,
+ ttm_bo_no_placement_gen_params),
+ KUNIT_CASE_PARAM(ttm_bo_validate_no_placement_not_signaled,
+ ttm_bo_types_gen_params),
+ KUNIT_CASE(ttm_bo_validate_move_fence_signaled),
+ KUNIT_CASE_PARAM(ttm_bo_validate_move_fence_not_signaled,
+ ttm_bo_validate_wait_gen_params),
{}
};
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v8 8/8] drm/ttm/tests: Add test cases dependent on fence signaling
2023-11-29 12:02 ` [PATCH v8 8/8] drm/ttm/tests: Add test cases dependent on fence signaling Karolina Stolarek
@ 2023-12-05 15:29 ` Andi Shyti
2023-12-06 11:49 ` Karolina Stolarek
0 siblings, 1 reply; 17+ messages in thread
From: Andi Shyti @ 2023-12-05 15:29 UTC (permalink / raw)
To: Karolina Stolarek
Cc: Amaranath Somalapuram, Christian König, dri-devel,
Andi Shyti
Hi Karolina,
...
> +static void ttm_bo_validate_no_placement_signaled(struct kunit *test)
> +{
> + const struct ttm_bo_validate_test_case *params = test->param_value;
> + struct ttm_test_devices *priv = test->priv;
> + struct ttm_buffer_object *bo;
> + struct ttm_place *place;
> + struct ttm_tt *old_tt;
> + struct ttm_placement *placement;
> + struct ttm_resource_manager *man;
> + uint32_t mem_type = TTM_PL_SYSTEM;
> + enum ttm_bo_type bo_type = ttm_bo_type_device;
> + struct ttm_operation_ctx ctx = { };
> + uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> + uint32_t flags;
> + int err;
> +
> + place = ttm_place_kunit_init(test, mem_type, 0);
> + man = ttm_manager_type(priv->ttm_dev, mem_type);
For next time, I find it difficult to follow all these variables,
it's easier to read
man = ttm_manager_type(priv->ttm_dev, TTM_PL_SYSTEM);
than
mem_type = TTM_PL_SYSTEM;
...
...
...
man = ttm_manager_type(priv->ttm_dev, mem_type);
> + bo = ttm_bo_kunit_init(test, test->priv, size);
> + bo->type = bo_type;
same here... the bo_type variable is not giving any value.
bo->type = ttm_bo_type_device;
is way more readable. You keep doing this all the way and I need
to check everytime what's the value in the declaration :-)
I'm not going to comment on this anymore.
> + if (params->with_ttm) {
> + old_tt = priv->ttm_dev->funcs->ttm_tt_create(bo, 0);
> + ttm_pool_alloc(&priv->ttm_dev->pool, old_tt, &ctx);
> + bo->ttm = old_tt;
> + }
> +
> + err = ttm_resource_alloc(bo, place, &bo->resource);
> + KUNIT_EXPECT_EQ(test, err, 0);
> + KUNIT_ASSERT_EQ(test, man->usage, size);
> +
> + placement = kunit_kzalloc(test, sizeof(*placement), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_NULL(test, placement);
> +
> + ttm_bo_reserve(bo, false, false, NULL);
> + err = ttm_bo_validate(bo, placement, &ctx);
> + dma_resv_unlock(bo->base.resv);
why don't you use here ttm_bo_unreserve()?
> + KUNIT_EXPECT_EQ(test, err, 0);
> + KUNIT_ASSERT_EQ(test, man->usage, 0);
> + KUNIT_ASSERT_NOT_NULL(test, bo->ttm);
> + KUNIT_EXPECT_EQ(test, ctx.bytes_moved, 0);
> +
> + if (params->with_ttm) {
> + flags = bo->ttm->page_flags;
> +
> + KUNIT_ASSERT_PTR_EQ(test, bo->ttm, old_tt);
> + KUNIT_ASSERT_FALSE(test, flags & TTM_TT_FLAG_PRIV_POPULATED);
> + KUNIT_ASSERT_TRUE(test, flags & TTM_TT_FLAG_ZERO_ALLOC);
> + }
> +
> + ttm_bo_put(bo);
> +}
...
> +static void ttm_bo_validate_move_fence_signaled(struct kunit *test)
> +{
> + struct ttm_test_devices *priv = test->priv;
> + struct ttm_buffer_object *bo;
> + struct ttm_place *place;
> + struct ttm_placement *placement;
> + struct ttm_resource_manager *man;
> + enum ttm_bo_type bo_type = ttm_bo_type_device;
> + uint32_t mem_type = TTM_PL_SYSTEM;
> + struct ttm_operation_ctx ctx = { };
> + uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> + int err;
> +
> + man = ttm_manager_type(priv->ttm_dev, mem_type);
> + spin_lock_init(&fence_lock);
where are we using the fence_lock here?
> + man->move = dma_fence_get_stub();
> +
> + bo = ttm_bo_kunit_init(test, test->priv, size);
> + bo->type = bo_type;
> + place = ttm_place_kunit_init(test, mem_type, 0);
> + placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
> +
> + ttm_bo_reserve(bo, false, false, NULL);
> + err = ttm_bo_validate(bo, placement, &ctx);
> + dma_resv_unlock(bo->base.resv);
> +
> + KUNIT_EXPECT_EQ(test, err, 0);
> + KUNIT_EXPECT_EQ(test, bo->resource->mem_type, mem_type);
> + KUNIT_EXPECT_EQ(test, ctx.bytes_moved, size);
Do we want to check also bo->ttm here?
> + ttm_bo_put(bo);
> + dma_fence_put(man->move);
> +}
> +
> +static const struct ttm_bo_validate_test_case ttm_bo_validate_wait_cases[] = {
> + {
> + .description = "Waits for GPU",
> + .no_gpu_wait = false,
> + },
> + {
> + .description = "Tries to lock straight away",
> + .no_gpu_wait = true,
> + },
> +};
> +
> +KUNIT_ARRAY_PARAM(ttm_bo_validate_wait, ttm_bo_validate_wait_cases,
> + ttm_bo_validate_case_desc);
> +
> +static int threaded_fence_signal(void *arg)
> +{
> + struct dma_fence *fence = arg;
> + int err;
> +
> + msleep(20);
> + err = dma_fence_signal(fence);
> +
> + return err;
if you do here "return dma_fence_signal(fence);" you don't need
for err.
Not a binding review, though, your choice.
Andi
...
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v8 8/8] drm/ttm/tests: Add test cases dependent on fence signaling
2023-12-05 15:29 ` Andi Shyti
@ 2023-12-06 11:49 ` Karolina Stolarek
0 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-12-06 11:49 UTC (permalink / raw)
To: Andi Shyti; +Cc: Amaranath Somalapuram, Christian König, dri-devel
Hi Andi,
> For next time, I find it difficult to follow all these variables,
> it's easier to read
>
> man = ttm_manager_type(priv->ttm_dev, TTM_PL_SYSTEM);
>
> than
>
> mem_type = TTM_PL_SYSTEM;
> ...
> ...
> ...
> man = ttm_manager_type(priv->ttm_dev, mem_type);
>
>
>> + bo = ttm_bo_kunit_init(test, test->priv, size);
>> + bo->type = bo_type;
>
> same here... the bo_type variable is not giving any value.
>
> bo->type = ttm_bo_type_device;
>
> is way more readable. You keep doing this all the way and I need
> to check everytime what's the value in the declaration :-)
The idea was that I'd provide these as parameters, but I limited the
scope of my tests and stick to set values. Also, in some cases, I
defined them because I keep checking for them in assertions, and didn't
want to repeat myself.
>
> I'm not going to comment on this anymore.
>
>> + if (params->with_ttm) {
>> + old_tt = priv->ttm_dev->funcs->ttm_tt_create(bo, 0);
>> + ttm_pool_alloc(&priv->ttm_dev->pool, old_tt, &ctx);
>> + bo->ttm = old_tt;
>> + }
>> +
>> + err = ttm_resource_alloc(bo, place, &bo->resource);
>> + KUNIT_EXPECT_EQ(test, err, 0);
>> + KUNIT_ASSERT_EQ(test, man->usage, size);
>> +
>> + placement = kunit_kzalloc(test, sizeof(*placement), GFP_KERNEL);
>> + KUNIT_ASSERT_NOT_NULL(test, placement);
>> +
>> + ttm_bo_reserve(bo, false, false, NULL);
>> + err = ttm_bo_validate(bo, placement, &ctx);
>> + dma_resv_unlock(bo->base.resv);
>
> why don't you use here ttm_bo_unreserve()?
That's a good question! I think that unreserve would work here as well.
(...)
>> +static void ttm_bo_validate_move_fence_signaled(struct kunit *test)
>> +{
>> + struct ttm_test_devices *priv = test->priv;
>> + struct ttm_buffer_object *bo;
>> + struct ttm_place *place;
>> + struct ttm_placement *placement;
>> + struct ttm_resource_manager *man;
>> + enum ttm_bo_type bo_type = ttm_bo_type_device;
>> + uint32_t mem_type = TTM_PL_SYSTEM;
>> + struct ttm_operation_ctx ctx = { };
>> + uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
>> + int err;
>> +
>> + man = ttm_manager_type(priv->ttm_dev, mem_type);
>> + spin_lock_init(&fence_lock);
>
> where are we using the fence_lock here?
Argh, it's a copy-paste mistake, sorry. We don't need it, as we use a
stub fence in man->move. I will delete that.
>
>> + man->move = dma_fence_get_stub();
>> +
>> + bo = ttm_bo_kunit_init(test, test->priv, size);
>> + bo->type = bo_type;
>> + place = ttm_place_kunit_init(test, mem_type, 0);
>> + placement = ttm_placement_kunit_init(test, place, 1, NULL, 0);
>> +
>> + ttm_bo_reserve(bo, false, false, NULL);
>> + err = ttm_bo_validate(bo, placement, &ctx);
>> + dma_resv_unlock(bo->base.resv);
>> +
>> + KUNIT_EXPECT_EQ(test, err, 0);
>> + KUNIT_EXPECT_EQ(test, bo->resource->mem_type, mem_type);
>> + KUNIT_EXPECT_EQ(test, ctx.bytes_moved, size);
>
> Do we want to check also bo->ttm here?
Hmm, I think that I covered that case with "normal" testing of
ttm_bo_validate, and kept this one minimal. Here, I'm just making sure
that a signaled move fence doesn't get into a way of BO validation.
>
>> + ttm_bo_put(bo);
>> + dma_fence_put(man->move);
>> +}
>> +
>> +static const struct ttm_bo_validate_test_case ttm_bo_validate_wait_cases[] = {
>> + {
>> + .description = "Waits for GPU",
>> + .no_gpu_wait = false,
>> + },
>> + {
>> + .description = "Tries to lock straight away",
>> + .no_gpu_wait = true,
>> + },
>> +};
>> +
>> +KUNIT_ARRAY_PARAM(ttm_bo_validate_wait, ttm_bo_validate_wait_cases,
>> + ttm_bo_validate_case_desc);
>> +
>> +static int threaded_fence_signal(void *arg)
>> +{
>> + struct dma_fence *fence = arg;
>> + int err;
>> +
>> + msleep(20);
>> + err = dma_fence_signal(fence);
>> +
>> + return err;
>
> if you do here "return dma_fence_signal(fence);" you don't need
> for err.
That is true!
> Not a binding review, though, your choice.
You spotted a couple of things I can improve, so I plan to include them
in the next version, after getting more review comments.
All the best,
Karolina
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v8 0/8] Improve test coverage of TTM
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
` (7 preceding siblings ...)
2023-11-29 12:02 ` [PATCH v8 8/8] drm/ttm/tests: Add test cases dependent on fence signaling Karolina Stolarek
@ 2023-12-05 15:39 ` Andi Shyti
2023-12-06 11:28 ` Karolina Stolarek
2023-12-14 8:20 ` Karolina Stolarek
9 siblings, 1 reply; 17+ messages in thread
From: Andi Shyti @ 2023-12-05 15:39 UTC (permalink / raw)
To: Karolina Stolarek; +Cc: Christian König, dri-devel, Andi Shyti
Hi Karolina and Christian,
> Karolina Stolarek (8):
> drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man
> drm/ttm/tests: Add tests for ttm_tt
> drm/ttm/tests: Add tests for ttm_bo functions
> drm/ttm/tests: Fix argument in ttm_tt_kunit_init()
> drm/ttm/tests: Use an init function from the helpers lib
> drm/ttm/tests: Test simple BO creation and validation
> drm/ttm/tests: Add tests with mock resource managers
> drm/ttm/tests: Add test cases dependent on fence signaling
I see that all these files (and previous patches, as well) don't
have a consistent prefix system, like kunit_ttm_*. But they all
start with ttm_*, thread_*, drm_*, etc, which makes it a bit
difficult to follow and grep through.
Can we please maintain a consistent prefix system?
I know it looks bad to come out with it after this series (and
previous work too) has been out for several months already. I
need to say my "mea culpa" as well as I've been in the review
loop, as well.
Thanks,
Andi
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v8 0/8] Improve test coverage of TTM
2023-12-05 15:39 ` [PATCH v8 0/8] Improve test coverage of TTM Andi Shyti
@ 2023-12-06 11:28 ` Karolina Stolarek
2023-12-06 11:46 ` Andi Shyti
0 siblings, 1 reply; 17+ messages in thread
From: Karolina Stolarek @ 2023-12-06 11:28 UTC (permalink / raw)
To: Andi Shyti; +Cc: Christian König, dri-devel
Hi Andi,
On 5.12.2023 16:39, Andi Shyti wrote:
> Hi Karolina and Christian,
>
>> Karolina Stolarek (8): drm/ttm/tests: Add tests for ttm_resource
>> and ttm_sys_man drm/ttm/tests: Add tests for ttm_tt drm/ttm/tests:
>> Add tests for ttm_bo functions drm/ttm/tests: Fix argument in
>> ttm_tt_kunit_init() drm/ttm/tests: Use an init function from the
>> helpers lib drm/ttm/tests: Test simple BO creation and validation
>> drm/ttm/tests: Add tests with mock resource managers drm/ttm/tests:
>> Add test cases dependent on fence signaling
>
> I see that all these files (and previous patches, as well) don't have
> a consistent prefix system, like kunit_ttm_*. But they all start with
> ttm_*, thread_*, drm_*, etc, which makes it a bit difficult to follow
> and grep through.
I see what you mean. As for ttm_* part, I decided to keep it as it is
based on what was in DRM KUnit tests and helpers lib. Almost all
functions and subtests start with drm_*, and as I'm working on TTM
tests, ttm_* prefix seemed like a natural choice. With thread_*, I could
add ttm_kunit_* in front of it, but not sure what would be the benefit
of doing this.
To be honest, I haven't considered using kunit_* prefix here. In the
code context, it's obvious these are kunit tests, and repeating that
information would make already long function/struct names even longer.
I had a quick glance at other KUnit tests in the kernel and this seems
to be the case, no kunit_* prefixes are used.
> Can we please maintain a consistent prefix system?
>
> I know it looks bad to come out with it after this series (and
> previous work too) has been out for several months already. I need to
> say my "mea culpa" as well as I've been in the review loop, as well.
After this series, I plan to send a small one to wrap up my work in this
field. How about adding a TODO to clean these up? I mean, that's
just how I see it, I'd like to hear Christian's thoughts on this.
All the best,
Karolina
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v8 0/8] Improve test coverage of TTM
2023-12-06 11:28 ` Karolina Stolarek
@ 2023-12-06 11:46 ` Andi Shyti
0 siblings, 0 replies; 17+ messages in thread
From: Andi Shyti @ 2023-12-06 11:46 UTC (permalink / raw)
To: Karolina Stolarek; +Cc: dri-devel, Andi Shyti, Christian König
Hi Karolina,
On Wed, Dec 06, 2023 at 12:28:14PM +0100, Karolina Stolarek wrote:
> On 5.12.2023 16:39, Andi Shyti wrote:
...
> > Hi Karolina and Christian,
...
> > > Karolina Stolarek (8): drm/ttm/tests: Add tests for ttm_resource and
> > > ttm_sys_man drm/ttm/tests: Add tests for ttm_tt drm/ttm/tests:
> > > Add tests for ttm_bo functions drm/ttm/tests: Fix argument in
> > > ttm_tt_kunit_init() drm/ttm/tests: Use an init function from the
> > > helpers lib drm/ttm/tests: Test simple BO creation and validation
> > > drm/ttm/tests: Add tests with mock resource managers drm/ttm/tests:
> > > Add test cases dependent on fence signaling
> >
> > I see that all these files (and previous patches, as well) don't have
> > a consistent prefix system, like kunit_ttm_*. But they all start with
> > ttm_*, thread_*, drm_*, etc, which makes it a bit difficult to follow
> > and grep through.
>
> I see what you mean. As for ttm_* part, I decided to keep it as it is
> based on what was in DRM KUnit tests and helpers lib. Almost all
> functions and subtests start with drm_*, and as I'm working on TTM
> tests, ttm_* prefix seemed like a natural choice. With thread_*, I could
> add ttm_kunit_* in front of it, but not sure what would be the benefit
> of doing this.
I do not agree here, ttm_* are related to ttm and not "testing
ttm" and we need to be consistent.
I'd be more inclined to a kunit_ttm_* or ttm_kunit_*.
Anyway, let's hear also what Christian thinks.
> To be honest, I haven't considered using kunit_* prefix here. In the
> code context, it's obvious these are kunit tests, and repeating that
> information would make already long function/struct names even longer.
>
> I had a quick glance at other KUnit tests in the kernel and this seems
> to be the case, no kunit_* prefixes are used.
>
> > Can we please maintain a consistent prefix system?
> >
> > I know it looks bad to come out with it after this series (and previous
> > work too) has been out for several months already. I need to
> > say my "mea culpa" as well as I've been in the review loop, as well.
>
> After this series, I plan to send a small one to wrap up my work in this
> field. How about adding a TODO to clean these up? I mean, that's
> just how I see it, I'd like to hear Christian's thoughts on this.
nah... a TODO note would be too much... your promise is enough
for me.
Andi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v8 0/8] Improve test coverage of TTM
2023-11-29 12:02 [PATCH v8 0/8] Improve test coverage of TTM Karolina Stolarek
` (8 preceding siblings ...)
2023-12-05 15:39 ` [PATCH v8 0/8] Improve test coverage of TTM Andi Shyti
@ 2023-12-14 8:20 ` Karolina Stolarek
2023-12-14 10:22 ` Christian König
9 siblings, 1 reply; 17+ messages in thread
From: Karolina Stolarek @ 2023-12-14 8:20 UTC (permalink / raw)
To: Christian König; +Cc: Andi Shyti, dri-devel
Hi Christian,
On 29.11.2023 13:02, Karolina Stolarek wrote:
> Karolina Stolarek (8):
> drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man
> drm/ttm/tests: Add tests for ttm_tt
> drm/ttm/tests: Add tests for ttm_bo functions
> drm/ttm/tests: Fix argument in ttm_tt_kunit_init()
Would it be possible to get these patches merged? They already have
r-b's. There are a couple more, but these don't depend on any other
patch that is pending review, and should be easy to get in.
I'm asking for this because I have three final patches ready for
submission (eviction, ttm_tt_populate, TODO file) and wondered if I
should submit them as v9, or if I could create a new series. I'd prefer
creating a new one, this patchset is already big and intimidating :)
Having said that, thank you for taking your time to review the tests, I
really appreciate it.
All the best,
Karolina
> drm/ttm/tests: Use an init function from the helpers lib
> drm/ttm/tests: Test simple BO creation and validation
> drm/ttm/tests: Add tests with mock resource managers
> drm/ttm/tests: Add test cases dependent on fence signaling
>
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/ttm/tests/.kunitconfig | 1 +
> drivers/gpu/drm/ttm/tests/Makefile | 5 +
> drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 622 ++++++++++++++
> .../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 795 ++++++++++++++++++
> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 109 ++-
> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 7 +
> drivers/gpu/drm/ttm/tests/ttm_mock_manager.c | 207 +++++
> drivers/gpu/drm/ttm/tests/ttm_mock_manager.h | 31 +
> drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 3 +-
> drivers/gpu/drm/ttm/tests/ttm_resource_test.c | 335 ++++++++
> drivers/gpu/drm/ttm/tests/ttm_tt_test.c | 282 +++++++
> drivers/gpu/drm/ttm/ttm_resource.c | 3 +
> drivers/gpu/drm/ttm/ttm_tt.c | 3 +
> 14 files changed, 2401 insertions(+), 3 deletions(-)
> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_mock_manager.h
> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_resource_test.c
> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_tt_test.c
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v8 0/8] Improve test coverage of TTM
2023-12-14 8:20 ` Karolina Stolarek
@ 2023-12-14 10:22 ` Christian König
2023-12-14 14:13 ` Karolina Stolarek
0 siblings, 1 reply; 17+ messages in thread
From: Christian König @ 2023-12-14 10:22 UTC (permalink / raw)
To: Karolina Stolarek; +Cc: Andi Shyti, dri-devel
Am 14.12.23 um 09:20 schrieb Karolina Stolarek:
>
> Hi Christian,
>
> On 29.11.2023 13:02, Karolina Stolarek wrote:
>> Karolina Stolarek (8):
>> drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man
>> drm/ttm/tests: Add tests for ttm_tt
>> drm/ttm/tests: Add tests for ttm_bo functions
> > drm/ttm/tests: Fix argument in ttm_tt_kunit_init()
>
> Would it be possible to get these patches merged? They already have
> r-b's. There are a couple more, but these don't depend on any other
> patch that is pending review, and should be easy to get in.
Sure, just pushed those three to drm-misc-next.
>
> I'm asking for this because I have three final patches ready for
> submission (eviction, ttm_tt_populate, TODO file) and wondered if I
> should submit them as v9, or if I could create a new series. I'd prefer
> creating a new one, this patchset is already big and intimidating :)
> Having said that, thank you for taking your time to review the tests, I
> really appreciate it.
No problem, you are actually helping getting stuff from my way to long
TODO list.
Regards,
Christian.
>
> All the best,
> Karolina
>
>> drm/ttm/tests: Use an init function from the helpers lib
>> drm/ttm/tests: Test simple BO creation and validation
>> drm/ttm/tests: Add tests with mock resource managers
>> drm/ttm/tests: Add test cases dependent on fence signaling
>>
>> drivers/gpu/drm/Kconfig | 1 +
>> drivers/gpu/drm/ttm/tests/.kunitconfig | 1 +
>> drivers/gpu/drm/ttm/tests/Makefile | 5 +
>> drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 622 ++++++++++++++
>> .../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 795 ++++++++++++++++++
>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 109 ++-
>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 7 +
>> drivers/gpu/drm/ttm/tests/ttm_mock_manager.c | 207 +++++
>> drivers/gpu/drm/ttm/tests/ttm_mock_manager.h | 31 +
>> drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 3 +-
>> drivers/gpu/drm/ttm/tests/ttm_resource_test.c | 335 ++++++++
>> drivers/gpu/drm/ttm/tests/ttm_tt_test.c | 282 +++++++
>> drivers/gpu/drm/ttm/ttm_resource.c | 3 +
>> drivers/gpu/drm/ttm/ttm_tt.c | 3 +
>> 14 files changed, 2401 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_bo_test.c
>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_mock_manager.h
>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_resource_test.c
>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_tt_test.c
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v8 0/8] Improve test coverage of TTM
2023-12-14 10:22 ` Christian König
@ 2023-12-14 14:13 ` Karolina Stolarek
0 siblings, 0 replies; 17+ messages in thread
From: Karolina Stolarek @ 2023-12-14 14:13 UTC (permalink / raw)
To: Christian König; +Cc: Andi Shyti, dri-devel
On 14.12.2023 11:22, Christian König wrote:
>
>
> Am 14.12.23 um 09:20 schrieb Karolina Stolarek:
>>
>> Hi Christian,
>>
>> On 29.11.2023 13:02, Karolina Stolarek wrote:
>>> Karolina Stolarek (8):
>>> drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man
>>> drm/ttm/tests: Add tests for ttm_tt
>>> drm/ttm/tests: Add tests for ttm_bo functions
>> > drm/ttm/tests: Fix argument in ttm_tt_kunit_init()
>>
>> Would it be possible to get these patches merged? They already have
>> r-b's. There are a couple more, but these don't depend on any other
>> patch that is pending review, and should be easy to get in.
>
> Sure, just pushed those three to drm-misc-next.
Awesome, thanks a lot. I see they went into drm-tip already, so I
rebased my series on the top of that.
>> I'm asking for this because I have three final patches ready for
>> submission (eviction, ttm_tt_populate, TODO file) and wondered if I
>> should submit them as v9, or if I could create a new series. I'd prefer
>> creating a new one, this patchset is already big and intimidating :)
>> Having said that, thank you for taking your time to review the tests, I
>> really appreciate it.
>
> No problem, you are actually helping getting stuff from my way to long
> TODO list.
By producing 4k+ lines of code for you to look at? ;-)
All the best,
Karolina
>
> Regards,
> Christian.
^ permalink raw reply [flat|nested] 17+ messages in thread