All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v12 00/10] Improve test coverage of TTM
@ 2024-05-15 11:24 Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 01/10] drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk Karolina Stolarek
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

Introduce tests for ttm_bo_validate()/ttm_bo_init_validate() that exercise
simple BO placement as well as eviction (including the case where the evict
domain also requires eviction to fit the incoming buffer). Prepare KUnit
helpers to handle such scenarios and add a mock VRAM manager. This series also
includes some updates to the helpers and more definitions used to define
"special" memory domains (e.g., one that can't allocate resources or is busy),
as well as drive-by fixes for the tests.

There are a couple of areas in which this test suite can be improved.
Suggestions for future work can be found in the TODO file.

Use kunit_tool script to manually run all the tests:

$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/ttm/tests

To build a kernel with TTM KUnit tests, use a UML configuration,
enable CONFIG_KUNIT, and then select CONFIG_DRM_TTM_KUNIT_TEST.

Many thanks,
Karolina

v12:
  - Rewrite "drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk" patch to
    extend ttm_bo_kunit_init() helper to accept a dma_resv object and update
    calls to that helper (Christian)
  - Update drm_buddy_free_list() calls with an extra argument

v11:
  - Delete CONFIG_DRM_KUNIT_TEST_HELPERS from .kunitconfig file, as it gets
    automatically selected when TTM KUnit tests are enabled
  - Call ttm_bo_put() in ttm_bo_validate_pinned() test case (Matt)
  - Fix a copy-paste mistake in ttm_mem_type_cases definition (Matt)
  - Update mock_move definition to do a hop on VRAM -> sysmem move and
    delete a dummy multihop domain. Fix the eviction tests accordingly (Matt)
  - Update ttm_bo_validate_swapout() to use TT domain instead of VRAM
  - Update eviction test cases to create TT domain, so they can perform multihop
  - Update TODO file, as it got outdated already

v10:
  Many things have happened over the course of three months, so the series
  had to be slightly reworked and expanded to accommodate these changes:
   - Set DMA coherent mapping mask in the KUnit device so ttm_pool_alloc
     tests can be executed
   - Update ttm_bo_validate_invalid_placement() test case to check against
     the right return error. It's no longer -EINVAL (which only is returned
     for pinned buffers), but -ENOMEM. The behaviour has changed in
     commit cc941c70df39 ("drm/ttm: improve idle/busy handling v5")
   - Rework ttm_placement_kunit_init() to accept only one array of places
     and update the tests that use that helper
   - Set fallback flags in eviction domains defined in TTM KUnit helpers
   - Fix a warning raised by ttm_bo_unreserve_bulk() test case
   - Scrap all r-bs and tested-by, as many things were updated and should
     be checked again

Karolina Stolarek (10):
  drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk
  drm/ttm/tests: Delete unnecessary config option
  drm/ttm/tests: Set DMA mask in KUnit device
  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
  drm/ttm/tests: Add eviction testing
  drm/ttm/tests: Add tests for ttm_tt_populate
  drm/ttm/tests: Add TODO file

 drivers/gpu/drm/Kconfig                       |    1 +
 drivers/gpu/drm/ttm/tests/.kunitconfig        |    2 +-
 drivers/gpu/drm/ttm/tests/Makefile            |    2 +
 drivers/gpu/drm/ttm/tests/TODO                |   25 +
 drivers/gpu/drm/ttm/tests/ttm_bo_test.c       |   40 +-
 .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  | 1225 +++++++++++++++++
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  178 ++-
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |   13 +-
 drivers/gpu/drm/ttm/tests/ttm_mock_manager.c  |  235 ++++
 drivers/gpu/drm/ttm/tests/ttm_mock_manager.h  |   33 +
 drivers/gpu/drm/ttm/tests/ttm_pool_test.c     |    4 +-
 drivers/gpu/drm/ttm/tests/ttm_resource_test.c |    2 +-
 drivers/gpu/drm/ttm/tests/ttm_tt_test.c       |  154 ++-
 drivers/gpu/drm/ttm/ttm_tt.c                  |    3 +
 14 files changed, 1862 insertions(+), 55 deletions(-)
 create mode 100644 drivers/gpu/drm/ttm/tests/TODO
 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

-- 
2.34.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v12 01/10] drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-24 15:16   ` Thomas Hellström
  2024-05-15 11:24 ` [PATCH v12 02/10] drm/ttm/tests: Delete unnecessary config option Karolina Stolarek
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

BOs in a bulk move have to share the same reservation object. That is
not the case in the ttm_bo_unreserve_bulk subtest. Update
ttm_bo_kunit_init() helper to accept dma_resv object so we can define
buffer objects that share the same resv. Update calls to that helper
accordingly.

Fixes: 995279d280d1 ("drm/ttm/tests: Add tests for ttm_bo functions")
Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
---
 drivers/gpu/drm/ttm/tests/ttm_bo_test.c       | 40 +++++++++++--------
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  7 +++-
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |  3 +-
 drivers/gpu/drm/ttm/tests/ttm_pool_test.c     |  4 +-
 drivers/gpu/drm/ttm/tests/ttm_resource_test.c |  2 +-
 drivers/gpu/drm/ttm/tests/ttm_tt_test.c       | 20 +++++-----
 6 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
index 1f8a4f8adc92..ffcfe5e6709a 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
@@ -56,7 +56,7 @@ static void ttm_bo_reserve_optimistic_no_ticket(struct kunit *test)
 	struct ttm_buffer_object *bo;
 	int err;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_bo_reserve(bo, params->interruptible, params->no_wait, NULL);
 	KUNIT_ASSERT_EQ(test, err, 0);
@@ -71,7 +71,7 @@ static void ttm_bo_reserve_locked_no_sleep(struct kunit *test)
 	bool no_wait = true;
 	int err;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	/* Let's lock it beforehand */
 	dma_resv_lock(bo->base.resv, NULL);
@@ -92,7 +92,7 @@ static void ttm_bo_reserve_no_wait_ticket(struct kunit *test)
 
 	ww_acquire_init(&ctx, &reservation_ww_class);
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_bo_reserve(bo, interruptible, no_wait, &ctx);
 	KUNIT_ASSERT_EQ(test, err, -EBUSY);
@@ -110,7 +110,7 @@ static void ttm_bo_reserve_double_resv(struct kunit *test)
 
 	ww_acquire_init(&ctx, &reservation_ww_class);
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_bo_reserve(bo, interruptible, no_wait, &ctx);
 	KUNIT_ASSERT_EQ(test, err, 0);
@@ -138,8 +138,8 @@ static void ttm_bo_reserve_deadlock(struct kunit *test)
 	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);
+	bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+	bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	ww_acquire_init(&ctx1, &reservation_ww_class);
 	mutex_lock(&bo2->base.resv->lock.base);
@@ -208,7 +208,7 @@ static void ttm_bo_reserve_interrupted(struct kunit *test)
 	struct task_struct *task;
 	int err;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	task = kthread_create(threaded_ttm_bo_reserve, bo, "ttm-bo-reserve");
 
@@ -249,7 +249,7 @@ static void ttm_bo_unreserve_basic(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, err, 0);
 	priv->ttm_dev = ttm_dev;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 	bo->priority = bo_prio;
 
 	err = ttm_resource_alloc(bo, place, &res1);
@@ -288,7 +288,7 @@ static void ttm_bo_unreserve_pinned(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, err, 0);
 	priv->ttm_dev = ttm_dev;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 	place = ttm_place_kunit_init(test, mem_type, 0);
 
 	dma_resv_lock(bo->base.resv, NULL);
@@ -321,6 +321,7 @@ static void ttm_bo_unreserve_bulk(struct kunit *test)
 	struct ttm_resource *res1, *res2;
 	struct ttm_device *ttm_dev;
 	struct ttm_place *place;
+	struct dma_resv *resv;
 	uint32_t mem_type = TTM_PL_SYSTEM;
 	unsigned int bo_priority = 0;
 	int err;
@@ -332,12 +333,17 @@ static void ttm_bo_unreserve_bulk(struct kunit *test)
 	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
 
+	resv = kunit_kzalloc(test, sizeof(*resv), 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_init(resv);
+
+	bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, resv);
+	bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, resv);
 
 	dma_resv_lock(bo1->base.resv, NULL);
 	ttm_bo_set_bulk_move(bo1, &lru_bulk_move);
@@ -363,6 +369,8 @@ static void ttm_bo_unreserve_bulk(struct kunit *test)
 
 	ttm_resource_free(bo1, &res1);
 	ttm_resource_free(bo2, &res2);
+
+	dma_resv_fini(resv);
 }
 
 static void ttm_bo_put_basic(struct kunit *test)
@@ -384,7 +392,7 @@ static void ttm_bo_put_basic(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, err, 0);
 	priv->ttm_dev = ttm_dev;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 	bo->type = ttm_bo_type_device;
 
 	err = ttm_resource_alloc(bo, place, &res);
@@ -445,7 +453,7 @@ static void ttm_bo_put_shared_resv(struct kunit *test)
 
 	dma_fence_signal(fence);
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 	bo->type = ttm_bo_type_device;
 	bo->base.resv = external_resv;
 
@@ -467,7 +475,7 @@ static void ttm_bo_pin_basic(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, err, 0);
 	priv->ttm_dev = ttm_dev;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	for (int i = 0; i < no_pins; i++) {
 		dma_resv_lock(bo->base.resv, NULL);
@@ -502,7 +510,7 @@ static void ttm_bo_pin_unpin_resource(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, err, 0);
 	priv->ttm_dev = ttm_dev;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_resource_alloc(bo, place, &res);
 	KUNIT_ASSERT_EQ(test, err, 0);
@@ -553,7 +561,7 @@ static void ttm_bo_multiple_pin_one_unpin(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, err, 0);
 	priv->ttm_dev = ttm_dev;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_resource_alloc(bo, place, &res);
 	KUNIT_ASSERT_EQ(test, err, 0);
diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
index 7b7c1fa805fc..5be317a0af56 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -51,7 +51,8 @@ EXPORT_SYMBOL_GPL(ttm_device_kunit_init);
 
 struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
 					    struct ttm_test_devices *devs,
-					    size_t size)
+					    size_t size,
+					    struct dma_resv *obj)
 {
 	struct drm_gem_object gem_obj = { };
 	struct ttm_buffer_object *bo;
@@ -61,6 +62,10 @@ struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
 	KUNIT_ASSERT_NOT_NULL(test, bo);
 
 	bo->base = gem_obj;
+
+	if (obj)
+		bo->base.resv = obj;
+
 	err = drm_gem_object_init(devs->drm, &bo->base, size);
 	KUNIT_ASSERT_EQ(test, err, 0);
 
diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
index 2f51c833a536..c83d31b23c9a 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
@@ -28,7 +28,8 @@ int ttm_device_kunit_init(struct ttm_test_devices *priv,
 			  bool use_dma32);
 struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
 					    struct ttm_test_devices *devs,
-					    size_t size);
+					    size_t size,
+					    struct dma_resv *obj);
 struct ttm_place *ttm_place_kunit_init(struct kunit *test,
 				       uint32_t mem_type, uint32_t flags);
 
diff --git a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
index 0a3fede84da9..4643f91c6bd5 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
@@ -57,7 +57,7 @@ static struct ttm_tt *ttm_tt_kunit_init(struct kunit *test,
 	struct ttm_tt *tt;
 	int err;
 
-	bo = ttm_bo_kunit_init(test, priv->devs, size);
+	bo = ttm_bo_kunit_init(test, priv->devs, size, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, bo);
 	priv->mock_bo = bo;
 
@@ -209,7 +209,7 @@ static void ttm_pool_alloc_basic_dma_addr(struct kunit *test)
 	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, tt);
 
-	bo = ttm_bo_kunit_init(test, devs, size);
+	bo = ttm_bo_kunit_init(test, devs, size, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, bo);
 
 	err = ttm_sg_tt_init(tt, bo, 0, caching);
diff --git a/drivers/gpu/drm/ttm/tests/ttm_resource_test.c b/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
index 029e1f094bb0..67584058dadb 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
@@ -54,7 +54,7 @@ static void ttm_init_test_mocks(struct kunit *test,
 	/* 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->bo = ttm_bo_kunit_init(test, priv->devs, size, NULL);
 	priv->place = ttm_place_kunit_init(test, mem_type, flags);
 }
 
diff --git a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
index fd4502c18de6..67bf51723c92 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
@@ -63,7 +63,7 @@ static void ttm_tt_init_basic(struct kunit *test)
 	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, tt);
 
-	bo = ttm_bo_kunit_init(test, test->priv, params->size);
+	bo = ttm_bo_kunit_init(test, test->priv, params->size, NULL);
 
 	err = ttm_tt_init(tt, bo, page_flags, caching, extra_pages);
 	KUNIT_ASSERT_EQ(test, err, 0);
@@ -89,7 +89,7 @@ static void ttm_tt_init_misaligned(struct kunit *test)
 	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, tt);
 
-	bo = ttm_bo_kunit_init(test, test->priv, size);
+	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
 
 	/* Make the object size misaligned */
 	bo->base.size += 1;
@@ -110,7 +110,7 @@ static void ttm_tt_fini_basic(struct kunit *test)
 	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, tt);
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_tt_init(tt, bo, 0, caching, 0);
 	KUNIT_ASSERT_EQ(test, err, 0);
@@ -130,7 +130,7 @@ static void ttm_tt_fini_sg(struct kunit *test)
 	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, tt);
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_sg_tt_init(tt, bo, 0, caching);
 	KUNIT_ASSERT_EQ(test, err, 0);
@@ -151,7 +151,7 @@ static void ttm_tt_fini_shmem(struct kunit *test)
 	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, tt);
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_tt_init(tt, bo, 0, caching, 0);
 	KUNIT_ASSERT_EQ(test, err, 0);
@@ -168,7 +168,7 @@ 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 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 	bo->type = ttm_bo_type_device;
 
 	dma_resv_lock(bo->base.resv, NULL);
@@ -187,7 +187,7 @@ 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 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 	bo->type = ttm_bo_type_sg + 1;
 
 	dma_resv_lock(bo->base.resv, NULL);
@@ -208,7 +208,7 @@ static void ttm_tt_create_ttm_exists(struct kunit *test)
 	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, tt);
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	err = ttm_tt_init(tt, bo, 0, caching, 0);
 	KUNIT_ASSERT_EQ(test, err, 0);
@@ -239,7 +239,7 @@ static void ttm_tt_create_failed(struct kunit *test)
 	struct ttm_buffer_object *bo;
 	int err;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	/* Update ttm_device_funcs so we don't alloc ttm_tt */
 	devs->ttm_dev->funcs = &ttm_dev_empty_funcs;
@@ -257,7 +257,7 @@ static void ttm_tt_destroy_basic(struct kunit *test)
 	struct ttm_buffer_object *bo;
 	int err;
 
-	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 
 	dma_resv_lock(bo->base.resv, NULL);
 	err = ttm_tt_create(bo, false);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v12 02/10] drm/ttm/tests: Delete unnecessary config option
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 01/10] drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 03/10] drm/ttm/tests: Set DMA mask in KUnit device Karolina Stolarek
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

DRM KUnit helpers are selected automatically when TTM tests are enabled,
so there's no need to do it directly in the .kunitconfig file.

Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/ttm/tests/.kunitconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig b/drivers/gpu/drm/ttm/tests/.kunitconfig
index 75fdce0cd98e..1ae1ffabd51e 100644
--- a/drivers/gpu/drm/ttm/tests/.kunitconfig
+++ b/drivers/gpu/drm/ttm/tests/.kunitconfig
@@ -1,4 +1,3 @@
 CONFIG_KUNIT=y
 CONFIG_DRM=y
-CONFIG_DRM_KUNIT_TEST_HELPERS=y
 CONFIG_DRM_TTM_KUNIT_TEST=y
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v12 03/10] drm/ttm/tests: Set DMA mask in KUnit device
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 01/10] drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 02/10] drm/ttm/tests: Delete unnecessary config option Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 04/10] drm/ttm/tests: Use an init function from the helpers lib Karolina Stolarek
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

In commit d393acce7b3f ("drm/tests: Switch to kunit devices"),
DRM test helpers migrated away from using a dummy platform driver
in favour of KUnit device. This means that DMA masks for the device
are not set but are required by ttm_pool_alloc tests.

Set the DMA mask for coherent mappings to unblock testing.

Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
index 5be317a0af56..c9ee7fe7c36d 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -103,6 +103,9 @@ struct ttm_test_devices *ttm_test_devices_basic(struct kunit *test)
 	devs->dev = drm_kunit_helper_alloc_device(test);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, devs->dev);
 
+	/* Set mask for alloc_coherent mappings to enable ttm_pool_alloc testing */
+	devs->dev->coherent_dma_mask = -1;
+
 	devs->drm = __drm_kunit_helper_alloc_drm_device(test, devs->dev,
 							sizeof(*devs->drm), 0,
 							DRIVER_GEM);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v12 04/10] drm/ttm/tests: Use an init function from the helpers lib
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
                   ` (2 preceding siblings ...)
  2024-05-15 11:24 ` [PATCH v12 03/10] drm/ttm/tests: Set DMA mask in KUnit device Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 05/10] drm/ttm/tests: Test simple BO creation and validation Karolina Stolarek
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

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>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Somalapuram, Amaranath <asomalap@amd.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 c9ee7fe7c36d..f25bd7951b74 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -158,6 +158,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 c83d31b23c9a..3dbf404e22a8 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
@@ -40,6 +40,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 67bf51723c92..17988fa99fa6 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] 20+ messages in thread

* [PATCH v12 05/10] drm/ttm/tests: Test simple BO creation and validation
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
                   ` (3 preceding siblings ...)
  2024-05-15 11:24 ` [PATCH v12 04/10] drm/ttm/tests: Use an init function from the helpers lib Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers Karolina Stolarek
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

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: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Somalapuram, Amaranath <asomalap@amd.com>
Tested-by: Somalapuram, Amaranath <asomalap@amd.com>
---
 drivers/gpu/drm/ttm/tests/Makefile            |   1 +
 .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  | 213 ++++++++++++++++++
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  14 +-
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |   1 +
 4 files changed, 228 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..a5520b0631a3
--- /dev/null
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -0,0 +1,213 @@
+// 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_placement *placement;
+
+	placement = kunit_kzalloc(test, sizeof(*placement), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, placement);
+
+	placement->num_placement = num_places;
+	placement->placement = 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;
+	enum ttm_bo_type bo_type = params->bo_type;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	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);
+
+	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)
+{
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	struct dma_resv resv;
+	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);
+
+	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)
+{
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	uint32_t unknown_mem_type = TTM_PL_PRIV + 1;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	int err;
+
+	place = ttm_place_kunit_init(test, unknown_mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
+	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, -ENOMEM);
+
+	ttm_bo_put(bo);
+}
+
+static void ttm_bo_validate_pinned(struct kunit *test)
+{
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	uint32_t mem_type = TTM_PL_SYSTEM;
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	int err;
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
+	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);
+
+	ttm_bo_reserve(bo, false, false, NULL);
+	ttm_bo_unpin(bo);
+	dma_resv_unlock(bo->base.resv);
+
+	ttm_bo_put(bo);
+}
+
+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 f25bd7951b74..2f590bae53f8 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);
 
@@ -93,6 +99,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 3dbf404e22a8..5ce1727b36fc 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
@@ -32,6 +32,7 @@ struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
 					    struct dma_resv *obj);
 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] 20+ messages in thread

* [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
                   ` (4 preceding siblings ...)
  2024-05-15 11:24 ` [PATCH v12 05/10] drm/ttm/tests: Test simple BO creation and validation Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-29 12:58   ` Thomas Hellström
  2024-05-15 11:24 ` [PATCH v12 07/10] drm/ttm/tests: Add test cases dependent on fence signaling Karolina Stolarek
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

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>
Tested-by: Somalapuram, Amaranath <asomalap@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  | 274 ++++++++++++++++++
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  38 ++-
 drivers/gpu/drm/ttm/tests/ttm_mock_manager.c  | 207 +++++++++++++
 drivers/gpu/drm/ttm/tests/ttm_mock_manager.h  |  31 ++
 7 files changed, 551 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 026444eeb5c6..4ba16501dbf7 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -234,6 +234,7 @@ config DRM_TTM_KUNIT_TEST
         default n
         depends on DRM && KUNIT && MMU && (UML || COMPILE_TEST)
         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 1ae1ffabd51e..772f0e1f4103 100644
--- a/drivers/gpu/drm/ttm/tests/.kunitconfig
+++ b/drivers/gpu/drm/ttm/tests/.kunitconfig
@@ -1,3 +1,4 @@
 CONFIG_KUNIT=y
 CONFIG_DRM=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 a5520b0631a3..8b62d95b8ab8 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;
 };
 
@@ -102,6 +105,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;
+	enum ttm_bo_type bo_type = params->bo_type;
+	struct ttm_test_devices *priv = test->priv;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_placement *placement;
+	uint32_t mem_type = TTM_PL_VRAM;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	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);
+
+	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)
 {
 	enum ttm_bo_type bo_type = ttm_bo_type_device;
@@ -136,6 +182,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;
+	uint32_t fst_mem = TTM_PL_SYSTEM, snd_mem = TTM_PL_VRAM;
+	struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
+	struct ttm_placement *fst_placement, *snd_placement;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_place *fst_place, *snd_place;
+	uint32_t size = ALIGN(SZ_8K, PAGE_SIZE);
+	struct ttm_buffer_object *bo;
+	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);
+
+	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);
+
+	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)
 {
 	enum ttm_bo_type bo_type = ttm_bo_type_device;
@@ -162,6 +253,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)
+{
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_placement *placement;
+	uint32_t mem_type = TTM_PL_VRAM;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	int err;
+
+	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
+	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);
+
+	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)
 {
 	enum ttm_bo_type bo_type = ttm_bo_type_device;
@@ -193,11 +314,164 @@ static void ttm_bo_validate_pinned(struct kunit *test)
 	ttm_bo_put(bo);
 }
 
+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_VRAM,
+	},
+};
+
+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_operation_ctx ctx_init = { }, ctx_val = { };
+	struct ttm_test_devices *priv = test->priv;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	int err;
+
+	place = ttm_place_kunit_init(test, params->mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	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);
+
+	ttm_bo_put(bo);
+
+	if (params->mem_type != TTM_PL_SYSTEM)
+		ttm_mock_manager_fini(priv->ttm_dev, params->mem_type);
+}
+
+static void ttm_bo_validate_busy_placement(struct kunit *test)
+{
+	uint32_t fst_mem = TTM_PL_VRAM, snd_mem = TTM_PL_VRAM + 1;
+	struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
+	struct ttm_placement *placement_init, *placement_val;
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_place *init_place, places[2];
+	struct ttm_resource_manager *man;
+	struct ttm_buffer_object *bo;
+	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);
+
+	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);
+
+	places[0] = (struct ttm_place){ .mem_type = fst_mem, .flags = TTM_PL_FLAG_DESIRED };
+	places[1] = (struct ttm_place){ .mem_type = snd_mem, .flags = TTM_PL_FLAG_FALLBACK };
+	placement_val = ttm_placement_kunit_init(test, places, 2);
+
+	err = ttm_bo_validate(bo, placement_val, &ctx_val);
+	dma_resv_unlock(bo->base.resv);
+
+	man = ttm_manager_type(priv->ttm_dev, snd_mem);
+
+	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_operation_ctx ctx_init = { }, ctx_val = { };
+	struct ttm_placement *placement_init, *placement_val;
+	uint32_t fst_mem = TTM_PL_VRAM, tmp_mem = TTM_PL_TT,
+		 final_mem = TTM_PL_SYSTEM;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_place *fst_place, *final_place;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_buffer_object *bo;
+	int err;
+
+	ttm_mock_manager_init(priv->ttm_dev, fst_mem, MANAGER_SIZE);
+	ttm_mock_manager_init(priv->ttm_dev, tmp_mem, MANAGER_SIZE);
+
+	fst_place = ttm_place_kunit_init(test, fst_mem, 0);
+	placement_init = ttm_placement_kunit_init(test, fst_place, 1);
+
+	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);
+
+	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);
+}
+
 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 2f590bae53f8..2a2585b37118 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -27,8 +27,42 @@ 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;
+	}
+
+	if (bo->resource->mem_type == TTM_PL_VRAM &&
+	    new_mem->mem_type == TTM_PL_SYSTEM) {
+		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_mock_manager.c b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
new file mode 100644
index 000000000000..eb9dca1de1a2
--- /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, 0);
+	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, 0);
+	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] 20+ messages in thread

* [PATCH v12 07/10] drm/ttm/tests: Add test cases dependent on fence signaling
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
                   ` (5 preceding siblings ...)
  2024-05-15 11:24 ` [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 08/10] drm/ttm/tests: Add eviction testing Karolina Stolarek
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

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>
Reviewed-by: Somalapuram, Amaranath <asomalap@amd.com>
Tested-by: Somalapuram, Amaranath <asomalap@amd.com>
---
 .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  | 305 ++++++++++++++++++
 1 file changed, 305 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 8b62d95b8ab8..6eec7b4fa776 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,
@@ -35,6 +40,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,262 @@ static void ttm_bo_validate_multihop(struct kunit *test)
 	ttm_mock_manager_fini(priv->ttm_dev, tmp_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;
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	uint32_t mem_type = TTM_PL_SYSTEM;
+	struct ttm_resource_manager *man;
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	struct ttm_tt *old_tt;
+	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, NULL);
+	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);
+	ttm_bo_unreserve(bo);
+
+	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;
+	enum dma_resv_usage usage = DMA_RESV_USAGE_BOOKKEEP;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	uint32_t mem_type = TTM_PL_SYSTEM;
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct task_struct *task;
+	struct ttm_place *place;
+	int err;
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+
+	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
+	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);
+	ttm_bo_unreserve(bo);
+
+	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)
+{
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_operation_ctx ctx = { };
+	uint32_t mem_type = TTM_PL_SYSTEM;
+	struct ttm_resource_manager *man;
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	int err;
+
+	man = ttm_manager_type(priv->ttm_dev, mem_type);
+	man->move = dma_fence_get_stub();
+
+	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
+	bo->type = bo_type;
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	ttm_bo_reserve(bo, false, false, NULL);
+	err = ttm_bo_validate(bo, placement, &ctx);
+	ttm_bo_unreserve(bo);
+
+	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;
+
+	msleep(20);
+
+	return dma_fence_signal(fence);
+}
+
+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_operation_ctx ctx_init = { },
+				 ctx_val  = { .no_wait_gpu = params->no_gpu_wait };
+	uint32_t fst_mem = TTM_PL_VRAM, snd_mem = TTM_PL_VRAM + 1;
+	struct ttm_placement *placement_init, *placement_val;
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
+	struct ttm_place *init_place, places[2];
+	struct ttm_resource_manager *man;
+	struct ttm_buffer_object *bo;
+	struct task_struct *task;
+	int err;
+
+	init_place = ttm_place_kunit_init(test, TTM_PL_SYSTEM, 0);
+	placement_init = ttm_placement_kunit_init(test, init_place, 1);
+
+	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);
+
+	places[0] = (struct ttm_place){ .mem_type = fst_mem, .flags = TTM_PL_FLAG_DESIRED };
+	places[1] = (struct ttm_place){ .mem_type = snd_mem, .flags = TTM_PL_FLAG_FALLBACK };
+	placement_val = ttm_placement_kunit_init(test, places, 2);
+
+	spin_lock_init(&fence_lock);
+	man = ttm_manager_type(priv->ttm_dev, fst_mem);
+	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 +770,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] 20+ messages in thread

* [PATCH v12 08/10] drm/ttm/tests: Add eviction testing
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
                   ` (6 preceding siblings ...)
  2024-05-15 11:24 ` [PATCH v12 07/10] drm/ttm/tests: Add test cases dependent on fence signaling Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 09/10] drm/ttm/tests: Add tests for ttm_tt_populate Karolina Stolarek
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

Add tests for ttm_bo_validate that focus on BO eviction and swapout.
Update device funcs definition with eviction-related callbacks. Add
alternative funcs where evict_flags() routes eviction to a domain
that can't allocate resources (dubbed "busy manager" in the tests).
Extract the common path of ttm_device init into a function.

Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Reviewed-by: Somalapuram, Amaranath <asomalap@amd.com>
Tested-by: Somalapuram, Amaranath <asomalap@amd.com>
---
 .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  | 433 ++++++++++++++++++
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 106 ++++-
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |   8 +
 drivers/gpu/drm/ttm/tests/ttm_mock_manager.c  |  28 ++
 drivers/gpu/drm/ttm/tests/ttm_mock_manager.h  |   2 +
 5 files changed, 569 insertions(+), 8 deletions(-)

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 6eec7b4fa776..749dadf4fce4 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -758,6 +758,431 @@ static void ttm_bo_validate_move_fence_not_signaled(struct kunit *test)
 	ttm_mock_manager_fini(priv->ttm_dev, snd_mem);
 }
 
+static void ttm_bo_validate_swapout(struct kunit *test)
+{
+	unsigned long size_big, size = ALIGN(BO_SIZE, PAGE_SIZE);
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_buffer_object *bo_small, *bo_big;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_placement *placement;
+	uint32_t mem_type = TTM_PL_TT;
+	struct ttm_place *place;
+	struct sysinfo si;
+	int err;
+
+	si_meminfo(&si);
+	size_big = ALIGN(((uint64_t)si.totalram * si.mem_unit / 2), PAGE_SIZE);
+
+	ttm_mock_manager_init(priv->ttm_dev, mem_type, size_big + size);
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	bo_small = kunit_kzalloc(test, sizeof(*bo_small), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_small);
+
+	drm_gem_private_object_init(priv->drm, &bo_small->base, size);
+
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_small, bo_type, placement,
+				   PAGE_SIZE, &ctx, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	dma_resv_unlock(bo_small->base.resv);
+
+	bo_big = ttm_bo_kunit_init(test, priv, size_big, NULL);
+
+	dma_resv_lock(bo_big->base.resv, NULL);
+	err = ttm_bo_validate(bo_big, placement, &ctx);
+	dma_resv_unlock(bo_big->base.resv);
+
+	KUNIT_EXPECT_EQ(test, err, 0);
+	KUNIT_EXPECT_NOT_NULL(test, bo_big->resource);
+	KUNIT_EXPECT_EQ(test, bo_big->resource->mem_type, mem_type);
+	KUNIT_EXPECT_EQ(test, bo_small->resource->mem_type, TTM_PL_SYSTEM);
+	KUNIT_EXPECT_TRUE(test, bo_small->ttm->page_flags & TTM_TT_FLAG_SWAPPED);
+
+	ttm_bo_put(bo_big);
+	ttm_bo_put(bo_small);
+
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+}
+
+static void ttm_bo_validate_happy_evict(struct kunit *test)
+{
+	uint32_t mem_type = TTM_PL_VRAM, mem_multihop = TTM_PL_TT,
+		 mem_type_evict = TTM_PL_SYSTEM;
+	struct ttm_operation_ctx ctx_init = { }, ctx_val  = { };
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	uint32_t small = SZ_8K, medium = SZ_512K,
+		 big = MANAGER_SIZE - (small + medium);
+	uint32_t bo_sizes[] = { small, medium, big };
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_buffer_object *bos, *bo_val;
+	struct ttm_placement *placement;
+	struct ttm_place *place;
+	uint32_t bo_no = 3;
+	int i, err;
+
+	ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+	ttm_mock_manager_init(priv->ttm_dev, mem_multihop, MANAGER_SIZE);
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	bos = kunit_kmalloc_array(test, bo_no, sizeof(*bos), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bos);
+
+	memset(bos, 0, sizeof(*bos) * bo_no);
+	for (i = 0; i < bo_no; i++) {
+		drm_gem_private_object_init(priv->drm, &bos[i].base, bo_sizes[i]);
+		err = ttm_bo_init_reserved(priv->ttm_dev, &bos[i], bo_type, placement,
+					   PAGE_SIZE, &ctx_init, NULL, NULL,
+					   &dummy_ttm_bo_destroy);
+		dma_resv_unlock(bos[i].base.resv);
+	}
+
+	bo_val = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+	bo_val->type = bo_type;
+
+	ttm_bo_reserve(bo_val, false, false, NULL);
+	err = ttm_bo_validate(bo_val, placement, &ctx_val);
+	ttm_bo_unreserve(bo_val);
+
+	KUNIT_EXPECT_EQ(test, err, 0);
+	KUNIT_EXPECT_EQ(test, bos[0].resource->mem_type, mem_type_evict);
+	KUNIT_EXPECT_TRUE(test, bos[0].ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC);
+	KUNIT_EXPECT_TRUE(test, bos[0].ttm->page_flags & TTM_TT_FLAG_PRIV_POPULATED);
+	KUNIT_EXPECT_EQ(test, ctx_val.bytes_moved, small * 2 + BO_SIZE);
+	KUNIT_EXPECT_EQ(test, bos[1].resource->mem_type, mem_type);
+
+	for (i = 0; i < bo_no; i++)
+		ttm_bo_put(&bos[i]);
+	ttm_bo_put(bo_val);
+
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+	ttm_mock_manager_fini(priv->ttm_dev, mem_multihop);
+}
+
+static void ttm_bo_validate_all_pinned_evict(struct kunit *test)
+{
+	struct ttm_operation_ctx ctx_init = { }, ctx_val  = { };
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_buffer_object *bo_big, *bo_small;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_placement *placement;
+	uint32_t mem_type = TTM_PL_VRAM, mem_multihop = TTM_PL_TT;
+	struct ttm_place *place;
+	int err;
+
+	ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+	ttm_mock_manager_init(priv->ttm_dev, mem_multihop, MANAGER_SIZE);
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	bo_big = kunit_kzalloc(test, sizeof(*bo_big), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_big);
+
+	drm_gem_private_object_init(priv->drm, &bo_big->base, MANAGER_SIZE);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_big, bo_type, placement,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+
+	ttm_bo_pin(bo_big);
+	dma_resv_unlock(bo_big->base.resv);
+
+	bo_small = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+	bo_small->type = bo_type;
+
+	ttm_bo_reserve(bo_small, false, false, NULL);
+	err = ttm_bo_validate(bo_small, placement, &ctx_val);
+	ttm_bo_unreserve(bo_small);
+
+	KUNIT_EXPECT_EQ(test, err, -ENOMEM);
+
+	ttm_bo_put(bo_small);
+
+	ttm_bo_reserve(bo_big, false, false, NULL);
+	ttm_bo_unpin(bo_big);
+	dma_resv_unlock(bo_big->base.resv);
+	ttm_bo_put(bo_big);
+
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+	ttm_mock_manager_fini(priv->ttm_dev, mem_multihop);
+}
+
+static void ttm_bo_validate_allowed_only_evict(struct kunit *test)
+{
+	uint32_t mem_type = TTM_PL_VRAM, mem_multihop = TTM_PL_TT,
+		 mem_type_evict = TTM_PL_SYSTEM;
+	struct ttm_buffer_object *bo, *bo_evictable, *bo_pinned;
+	struct ttm_operation_ctx ctx_init = { }, ctx_val  = { };
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_placement *placement;
+	struct ttm_place *place;
+	uint32_t size = SZ_512K;
+	int err;
+
+	ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+	ttm_mock_manager_init(priv->ttm_dev, mem_multihop, MANAGER_SIZE);
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	bo_pinned = kunit_kzalloc(test, sizeof(*bo_pinned), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_pinned);
+
+	drm_gem_private_object_init(priv->drm, &bo_pinned->base, size);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_pinned, bo_type, placement,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	ttm_bo_pin(bo_pinned);
+	dma_resv_unlock(bo_pinned->base.resv);
+
+	bo_evictable = kunit_kzalloc(test, sizeof(*bo_evictable), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_evictable);
+
+	drm_gem_private_object_init(priv->drm, &bo_evictable->base, size);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_evictable, bo_type, placement,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	dma_resv_unlock(bo_evictable->base.resv);
+
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+	bo->type = bo_type;
+
+	ttm_bo_reserve(bo, false, false, NULL);
+	err = ttm_bo_validate(bo, placement, &ctx_val);
+	ttm_bo_unreserve(bo);
+
+	KUNIT_EXPECT_EQ(test, err, 0);
+	KUNIT_EXPECT_EQ(test, bo->resource->mem_type, mem_type);
+	KUNIT_EXPECT_EQ(test, bo_pinned->resource->mem_type, mem_type);
+	KUNIT_EXPECT_EQ(test, bo_evictable->resource->mem_type, mem_type_evict);
+	KUNIT_EXPECT_EQ(test, ctx_val.bytes_moved, size * 2 + BO_SIZE);
+
+	ttm_bo_put(bo);
+	ttm_bo_put(bo_evictable);
+
+	ttm_bo_reserve(bo_pinned, false, false, NULL);
+	ttm_bo_unpin(bo_pinned);
+	dma_resv_unlock(bo_pinned->base.resv);
+	ttm_bo_put(bo_pinned);
+
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+	ttm_mock_manager_fini(priv->ttm_dev, mem_multihop);
+}
+
+static void ttm_bo_validate_deleted_evict(struct kunit *test)
+{
+	struct ttm_operation_ctx ctx_init = { }, ctx_val  = { };
+	uint32_t small = SZ_8K, big = MANAGER_SIZE - BO_SIZE;
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_buffer_object *bo_big, *bo_small;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_resource_manager *man;
+	uint32_t mem_type = TTM_PL_VRAM;
+	struct ttm_placement *placement;
+	struct ttm_place *place;
+	int err;
+
+	ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+	man = ttm_manager_type(priv->ttm_dev, mem_type);
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	bo_big = kunit_kzalloc(test, sizeof(*bo_big), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_big);
+
+	drm_gem_private_object_init(priv->drm, &bo_big->base, big);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_big, bo_type, placement,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	KUNIT_EXPECT_EQ(test, ttm_resource_manager_usage(man), big);
+
+	dma_resv_unlock(bo_big->base.resv);
+	bo_big->deleted = true;
+
+	bo_small = ttm_bo_kunit_init(test, test->priv, small, NULL);
+	bo_small->type = bo_type;
+
+	ttm_bo_reserve(bo_small, false, false, NULL);
+	err = ttm_bo_validate(bo_small, placement, &ctx_val);
+	ttm_bo_unreserve(bo_small);
+
+	KUNIT_EXPECT_EQ(test, err, 0);
+	KUNIT_EXPECT_EQ(test, bo_small->resource->mem_type, mem_type);
+	KUNIT_EXPECT_EQ(test, ttm_resource_manager_usage(man), small);
+	KUNIT_EXPECT_NULL(test, bo_big->ttm);
+	KUNIT_EXPECT_NULL(test, bo_big->resource);
+
+	ttm_bo_put(bo_small);
+	ttm_bo_put(bo_big);
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+}
+
+static void ttm_bo_validate_busy_domain_evict(struct kunit *test)
+{
+	uint32_t mem_type = TTM_PL_VRAM, mem_type_evict = TTM_PL_MOCK1;
+	struct ttm_operation_ctx ctx_init = { }, ctx_val  = { };
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_buffer_object *bo_init, *bo_val;
+	struct ttm_placement *placement;
+	struct ttm_place *place;
+	int err;
+
+	/*
+	 * Drop the default device and setup a new one that points to busy
+	 * thus unsuitable eviction domain
+	 */
+	ttm_device_fini(priv->ttm_dev);
+
+	err = ttm_device_kunit_init_bad_evict(test->priv, priv->ttm_dev, false, false);
+	KUNIT_ASSERT_EQ(test, err, 0);
+
+	ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+	ttm_busy_manager_init(priv->ttm_dev, mem_type_evict, MANAGER_SIZE);
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	bo_init = kunit_kzalloc(test, sizeof(*bo_init), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_init);
+
+	drm_gem_private_object_init(priv->drm, &bo_init->base, MANAGER_SIZE);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_init, bo_type, placement,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	dma_resv_unlock(bo_init->base.resv);
+
+	bo_val = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+	bo_val->type = bo_type;
+
+	ttm_bo_reserve(bo_val, false, false, NULL);
+	err = ttm_bo_validate(bo_val, placement, &ctx_val);
+	ttm_bo_unreserve(bo_val);
+
+	KUNIT_EXPECT_EQ(test, err, -ENOMEM);
+	KUNIT_EXPECT_EQ(test, bo_init->resource->mem_type, mem_type);
+	KUNIT_EXPECT_NULL(test, bo_val->resource);
+
+	ttm_bo_put(bo_init);
+	ttm_bo_put(bo_val);
+
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+	ttm_bad_manager_fini(priv->ttm_dev, mem_type_evict);
+}
+
+static void ttm_bo_validate_evict_gutting(struct kunit *test)
+{
+	struct ttm_operation_ctx ctx_init = { }, ctx_val  = { };
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_buffer_object *bo, *bo_evict;
+	uint32_t mem_type = TTM_PL_MOCK1;
+	struct ttm_placement *placement;
+	struct ttm_place *place;
+	int err;
+
+	ttm_mock_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);
+
+	bo_evict = kunit_kzalloc(test, sizeof(*bo_evict), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_evict);
+
+	drm_gem_private_object_init(priv->drm, &bo_evict->base, MANAGER_SIZE);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_evict, bo_type, placement,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	dma_resv_unlock(bo_evict->base.resv);
+
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+	bo->type = bo_type;
+
+	ttm_bo_reserve(bo, false, false, NULL);
+	err = ttm_bo_validate(bo, placement, &ctx_val);
+	ttm_bo_unreserve(bo);
+
+	KUNIT_EXPECT_EQ(test, err, 0);
+	KUNIT_EXPECT_EQ(test, bo->resource->mem_type, mem_type);
+	KUNIT_ASSERT_NULL(test, bo_evict->resource);
+	KUNIT_ASSERT_TRUE(test, bo_evict->ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC);
+
+	ttm_bo_put(bo_evict);
+	ttm_bo_put(bo);
+
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+}
+
+static void ttm_bo_validate_recrusive_evict(struct kunit *test)
+{
+	uint32_t mem_type = TTM_PL_TT, mem_type_evict = TTM_PL_MOCK2;
+	struct ttm_operation_ctx ctx_init = { }, ctx_val  = { };
+	struct ttm_placement *placement_tt, *placement_mock;
+	struct ttm_buffer_object *bo_tt, *bo_mock, *bo_val;
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_place *place_tt, *place_mock;
+	int err;
+
+	ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+	ttm_mock_manager_init(priv->ttm_dev, mem_type_evict, MANAGER_SIZE);
+
+	place_tt = ttm_place_kunit_init(test, mem_type, 0);
+	place_mock = ttm_place_kunit_init(test, mem_type_evict, 0);
+
+	placement_tt = ttm_placement_kunit_init(test, place_tt, 1);
+	placement_mock = ttm_placement_kunit_init(test, place_mock, 1);
+
+	bo_tt = kunit_kzalloc(test, sizeof(*bo_tt), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_tt);
+
+	bo_mock = kunit_kzalloc(test, sizeof(*bo_mock), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo_mock);
+
+	drm_gem_private_object_init(priv->drm, &bo_tt->base, MANAGER_SIZE);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_tt, bo_type, placement_tt,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	dma_resv_unlock(bo_tt->base.resv);
+
+	drm_gem_private_object_init(priv->drm, &bo_mock->base, MANAGER_SIZE);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo_mock, bo_type, placement_mock,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	dma_resv_unlock(bo_mock->base.resv);
+
+	bo_val = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+	bo_val->type = bo_type;
+
+	ttm_bo_reserve(bo_val, false, false, NULL);
+	err = ttm_bo_validate(bo_val, placement_tt, &ctx_val);
+	ttm_bo_unreserve(bo_val);
+
+	KUNIT_EXPECT_EQ(test, err, 0);
+
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type_evict);
+
+	ttm_bo_put(bo_val);
+	ttm_bo_put(bo_tt);
+	ttm_bo_put(bo_mock);
+}
+
 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),
@@ -777,6 +1202,14 @@ static struct kunit_case ttm_bo_validate_test_cases[] = {
 	KUNIT_CASE(ttm_bo_validate_move_fence_signaled),
 	KUNIT_CASE_PARAM(ttm_bo_validate_move_fence_not_signaled,
 			 ttm_bo_validate_wait_gen_params),
+	KUNIT_CASE(ttm_bo_validate_swapout),
+	KUNIT_CASE(ttm_bo_validate_happy_evict),
+	KUNIT_CASE(ttm_bo_validate_all_pinned_evict),
+	KUNIT_CASE(ttm_bo_validate_allowed_only_evict),
+	KUNIT_CASE(ttm_bo_validate_deleted_evict),
+	KUNIT_CASE(ttm_bo_validate_busy_domain_evict),
+	KUNIT_CASE(ttm_bo_validate_evict_gutting),
+	KUNIT_CASE(ttm_bo_validate_recrusive_evict),
 	{}
 };
 
diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
index 2a2585b37118..b062e64b91c7 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -6,6 +6,42 @@
 
 #include "ttm_kunit_helpers.h"
 
+static const struct ttm_place sys_place = {
+	.fpfn = 0,
+	.lpfn = 0,
+	.mem_type = TTM_PL_SYSTEM,
+	.flags = TTM_PL_FLAG_FALLBACK,
+};
+
+static const struct ttm_place mock1_place = {
+	.fpfn = 0,
+	.lpfn = 0,
+	.mem_type = TTM_PL_MOCK1,
+	.flags = TTM_PL_FLAG_FALLBACK,
+};
+
+static const struct ttm_place mock2_place = {
+	.fpfn = 0,
+	.lpfn = 0,
+	.mem_type = TTM_PL_MOCK2,
+	.flags = TTM_PL_FLAG_FALLBACK,
+};
+
+static struct ttm_placement sys_placement = {
+	.num_placement = 1,
+	.placement = &sys_place,
+};
+
+static struct ttm_placement bad_placement = {
+	.num_placement = 1,
+	.placement = &mock1_place,
+};
+
+static struct ttm_placement mock_placement = {
+	.num_placement = 1,
+	.placement = &mock2_place,
+};
+
 static struct ttm_tt *ttm_tt_simple_create(struct ttm_buffer_object *bo,
 					   uint32_t page_flags)
 {
@@ -65,10 +101,52 @@ static int mock_move(struct ttm_buffer_object *bo, bool evict,
 	return ttm_bo_move_memcpy(bo, ctx, new_mem);
 }
 
+static void mock_evict_flags(struct ttm_buffer_object *bo,
+			     struct ttm_placement *placement)
+{
+	switch (bo->resource->mem_type) {
+	case TTM_PL_VRAM:
+	case TTM_PL_SYSTEM:
+		*placement = sys_placement;
+		break;
+	case TTM_PL_TT:
+		*placement = mock_placement;
+		break;
+	case TTM_PL_MOCK1:
+		/* Purge objects coming from this domain */
+		break;
+	}
+}
+
+static void bad_evict_flags(struct ttm_buffer_object *bo,
+			    struct ttm_placement *placement)
+{
+	*placement = bad_placement;
+}
+
+static int ttm_device_kunit_init_with_funcs(struct ttm_test_devices *priv,
+					    struct ttm_device *ttm,
+					    bool use_dma_alloc,
+					    bool use_dma32,
+					    struct ttm_device_funcs *funcs)
+{
+	struct drm_device *drm = priv->drm;
+	int err;
+
+	err = ttm_device_init(ttm, funcs, drm->dev,
+			      drm->anon_inode->i_mapping,
+			      drm->vma_offset_manager,
+			      use_dma_alloc, use_dma32);
+
+	return err;
+}
+
 struct ttm_device_funcs ttm_dev_funcs = {
 	.ttm_tt_create = ttm_tt_simple_create,
 	.ttm_tt_destroy = ttm_tt_simple_destroy,
 	.move = mock_move,
+	.eviction_valuable = ttm_bo_eviction_valuable,
+	.evict_flags = mock_evict_flags,
 };
 EXPORT_SYMBOL_GPL(ttm_dev_funcs);
 
@@ -77,17 +155,29 @@ int ttm_device_kunit_init(struct ttm_test_devices *priv,
 			  bool use_dma_alloc,
 			  bool use_dma32)
 {
-	struct drm_device *drm = priv->drm;
-	int err;
+	return ttm_device_kunit_init_with_funcs(priv, ttm, use_dma_alloc,
+						use_dma32, &ttm_dev_funcs);
+}
+EXPORT_SYMBOL_GPL(ttm_device_kunit_init);
 
-	err = ttm_device_init(ttm, &ttm_dev_funcs, drm->dev,
-			      drm->anon_inode->i_mapping,
-			      drm->vma_offset_manager,
-			      use_dma_alloc, use_dma32);
+struct ttm_device_funcs ttm_dev_funcs_bad_evict = {
+	.ttm_tt_create = ttm_tt_simple_create,
+	.ttm_tt_destroy = ttm_tt_simple_destroy,
+	.move = mock_move,
+	.eviction_valuable = ttm_bo_eviction_valuable,
+	.evict_flags = bad_evict_flags,
+};
+EXPORT_SYMBOL_GPL(ttm_dev_funcs_bad_evict);
 
-	return err;
+int ttm_device_kunit_init_bad_evict(struct ttm_test_devices *priv,
+				    struct ttm_device *ttm,
+				    bool use_dma_alloc,
+				    bool use_dma32)
+{
+	return ttm_device_kunit_init_with_funcs(priv, ttm, use_dma_alloc,
+						use_dma32, &ttm_dev_funcs_bad_evict);
 }
-EXPORT_SYMBOL_GPL(ttm_device_kunit_init);
+EXPORT_SYMBOL_GPL(ttm_device_kunit_init_bad_evict);
 
 struct ttm_buffer_object *ttm_bo_kunit_init(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 5ce1727b36fc..aa70b50e7640 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
@@ -13,7 +13,11 @@
 #include <drm/drm_kunit_helpers.h>
 #include <kunit/test.h>
 
+#define TTM_PL_MOCK1 (TTM_PL_PRIV + 1)
+#define TTM_PL_MOCK2 (TTM_PL_PRIV + 2)
+
 extern struct ttm_device_funcs ttm_dev_funcs;
+extern struct ttm_device_funcs ttm_dev_funcs_bad_evict;
 
 struct ttm_test_devices {
 	struct drm_device *drm;
@@ -26,6 +30,10 @@ int ttm_device_kunit_init(struct ttm_test_devices *priv,
 			  struct ttm_device *ttm,
 			  bool use_dma_alloc,
 			  bool use_dma32);
+int ttm_device_kunit_init_bad_evict(struct ttm_test_devices *priv,
+				    struct ttm_device *ttm,
+				    bool use_dma_alloc,
+				    bool use_dma32);
 struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
 					    struct ttm_test_devices *devs,
 					    size_t size,
diff --git a/drivers/gpu/drm/ttm/tests/ttm_mock_manager.c b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
index eb9dca1de1a2..6ce3252ee9a9 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.c
@@ -153,6 +153,14 @@ static int ttm_bad_manager_alloc(struct ttm_resource_manager *man,
 	return -ENOSPC;
 }
 
+static int ttm_busy_manager_alloc(struct ttm_resource_manager *man,
+				  struct ttm_buffer_object *bo,
+				  const struct ttm_place *place,
+				  struct ttm_resource **res)
+{
+	return -EBUSY;
+}
+
 static void ttm_bad_manager_free(struct ttm_resource_manager *man,
 				 struct ttm_resource *res)
 {
@@ -172,6 +180,12 @@ static const struct ttm_resource_manager_func ttm_bad_manager_funcs = {
 	.compatible = ttm_bad_manager_compatible
 };
 
+static const struct ttm_resource_manager_func ttm_bad_busy_manager_funcs = {
+	.alloc = ttm_busy_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)
 {
@@ -191,6 +205,20 @@ int ttm_bad_manager_init(struct ttm_device *bdev, uint32_t mem_type,
 }
 EXPORT_SYMBOL_GPL(ttm_bad_manager_init);
 
+int ttm_busy_manager_init(struct ttm_device *bdev, uint32_t mem_type,
+			  uint32_t size)
+{
+	struct ttm_resource_manager *man;
+
+	ttm_bad_manager_init(bdev, mem_type, size);
+	man = ttm_manager_type(bdev, mem_type);
+
+	man->func = &ttm_bad_busy_manager_funcs;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ttm_busy_manager_init);
+
 void ttm_bad_manager_fini(struct ttm_device *bdev, uint32_t mem_type)
 {
 	struct ttm_resource_manager *man;
diff --git a/drivers/gpu/drm/ttm/tests/ttm_mock_manager.h b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.h
index d2db9de9d876..7a12d73fdc07 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_mock_manager.h
+++ b/drivers/gpu/drm/ttm/tests/ttm_mock_manager.h
@@ -25,6 +25,8 @@ 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);
+int ttm_busy_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);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v12 09/10] drm/ttm/tests: Add tests for ttm_tt_populate
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
                   ` (7 preceding siblings ...)
  2024-05-15 11:24 ` [PATCH v12 08/10] drm/ttm/tests: Add eviction testing Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-15 11:24 ` [PATCH v12 10/10] drm/ttm/tests: Add TODO file Karolina Stolarek
  2024-05-17  3:37 ` [PATCH v12 00/10] Improve test coverage of TTM Somalapuram, Amaranath
  10 siblings, 0 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

Add tests for functions that add and release pages to TTs. Test the
swapin operation. Export ttm_tt_unpopulate, ttm_tt_swapin and
ttm_tt_swapout symbols for testing purposes.

Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
Reviewed-by: Somalapuram, Amaranath <asomalap@amd.com>
Tested-by: Somalapuram, Amaranath <asomalap@amd.com>
---
 drivers/gpu/drm/ttm/tests/ttm_tt_test.c | 119 ++++++++++++++++++++++++
 drivers/gpu/drm/ttm/ttm_tt.c            |   3 +
 2 files changed, 122 insertions(+)

diff --git a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
index 17988fa99fa6..a9d75a33acaf 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
@@ -256,6 +256,120 @@ static void ttm_tt_destroy_basic(struct kunit *test)
 	ttm_tt_destroy(devs->ttm_dev, bo->ttm);
 }
 
+static void ttm_tt_populate_null_ttm(struct kunit *test)
+{
+	const struct ttm_test_devices *devs = test->priv;
+	struct ttm_operation_ctx ctx = { };
+	int err;
+
+	err = ttm_tt_populate(devs->ttm_dev, NULL, &ctx);
+	KUNIT_ASSERT_EQ(test, err, -EINVAL);
+}
+
+static void ttm_tt_populate_populated_ttm(struct kunit *test)
+{
+	const struct ttm_test_devices *devs = test->priv;
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_buffer_object *bo;
+	struct ttm_tt *tt;
+	struct page *populated_page;
+	int err;
+
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+
+	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, tt);
+
+	err = ttm_tt_init(tt, bo, 0, ttm_cached, 0);
+	KUNIT_ASSERT_EQ(test, err, 0);
+
+	err = ttm_tt_populate(devs->ttm_dev, tt, &ctx);
+	KUNIT_ASSERT_EQ(test, err, 0);
+	populated_page = *tt->pages;
+
+	err = ttm_tt_populate(devs->ttm_dev, tt, &ctx);
+	KUNIT_ASSERT_PTR_EQ(test, populated_page, *tt->pages);
+}
+
+static void ttm_tt_unpopulate_basic(struct kunit *test)
+{
+	const struct ttm_test_devices *devs = test->priv;
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_buffer_object *bo;
+	struct ttm_tt *tt;
+	int err;
+
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+
+	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, tt);
+
+	err = ttm_tt_init(tt, bo, 0, ttm_cached, 0);
+	KUNIT_ASSERT_EQ(test, err, 0);
+
+	err = ttm_tt_populate(devs->ttm_dev, tt, &ctx);
+	KUNIT_ASSERT_EQ(test, err, 0);
+	KUNIT_ASSERT_TRUE(test, ttm_tt_is_populated(tt));
+
+	ttm_tt_unpopulate(devs->ttm_dev, tt);
+	KUNIT_ASSERT_FALSE(test, ttm_tt_is_populated(tt));
+}
+
+static void ttm_tt_unpopulate_empty_ttm(struct kunit *test)
+{
+	const struct ttm_test_devices *devs = test->priv;
+	struct ttm_buffer_object *bo;
+	struct ttm_tt *tt;
+	int err;
+
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+
+	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, tt);
+
+	err = ttm_tt_init(tt, bo, 0, ttm_cached, 0);
+	KUNIT_ASSERT_EQ(test, err, 0);
+
+	ttm_tt_unpopulate(devs->ttm_dev, tt);
+	/* Expect graceful handling of unpopulated TTs */
+}
+
+static void ttm_tt_swapin_basic(struct kunit *test)
+{
+	const struct ttm_test_devices *devs = test->priv;
+	int expected_num_pages = BO_SIZE >> PAGE_SHIFT;
+	struct ttm_operation_ctx ctx = { };
+	struct ttm_buffer_object *bo;
+	struct ttm_tt *tt;
+	int err, num_pages;
+
+	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
+
+	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, tt);
+
+	err = ttm_tt_init(tt, bo, 0, ttm_cached, 0);
+	KUNIT_ASSERT_EQ(test, err, 0);
+
+	err = ttm_tt_populate(devs->ttm_dev, tt, &ctx);
+	KUNIT_ASSERT_EQ(test, err, 0);
+	KUNIT_ASSERT_TRUE(test, ttm_tt_is_populated(tt));
+
+	num_pages = ttm_tt_swapout(devs->ttm_dev, tt, GFP_KERNEL);
+	KUNIT_ASSERT_EQ(test, num_pages, expected_num_pages);
+	KUNIT_ASSERT_NOT_NULL(test, tt->swap_storage);
+	KUNIT_ASSERT_TRUE(test, tt->page_flags & TTM_TT_FLAG_SWAPPED);
+
+	/* Swapout depopulates TT, allocate pages and then swap them in */
+	err = ttm_pool_alloc(&devs->ttm_dev->pool, tt, &ctx);
+	KUNIT_ASSERT_EQ(test, err, 0);
+
+	err = ttm_tt_swapin(tt);
+	KUNIT_ASSERT_EQ(test, err, 0);
+	KUNIT_ASSERT_NULL(test, tt->swap_storage);
+	KUNIT_ASSERT_FALSE(test, tt->page_flags & TTM_TT_FLAG_SWAPPED);
+}
+
 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),
@@ -267,6 +381,11 @@ static struct kunit_case ttm_tt_test_cases[] = {
 	KUNIT_CASE(ttm_tt_create_ttm_exists),
 	KUNIT_CASE(ttm_tt_create_failed),
 	KUNIT_CASE(ttm_tt_destroy_basic),
+	KUNIT_CASE(ttm_tt_populate_null_ttm),
+	KUNIT_CASE(ttm_tt_populate_populated_ttm),
+	KUNIT_CASE(ttm_tt_unpopulate_basic),
+	KUNIT_CASE(ttm_tt_unpopulate_empty_ttm),
+	KUNIT_CASE(ttm_tt_swapin_basic),
 	{}
 };
 
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 7b00ddf0ce49..4b51b9023126 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -251,6 +251,7 @@ int ttm_tt_swapin(struct ttm_tt *ttm)
 out_err:
 	return ret;
 }
+EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_tt_swapin);
 
 /**
  * ttm_tt_swapout - swap out tt object
@@ -308,6 +309,7 @@ int ttm_tt_swapout(struct ttm_device *bdev, struct ttm_tt *ttm,
 
 	return ret;
 }
+EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_tt_swapout);
 
 int ttm_tt_populate(struct ttm_device *bdev,
 		    struct ttm_tt *ttm, struct ttm_operation_ctx *ctx)
@@ -386,6 +388,7 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct ttm_tt *ttm)
 
 	ttm->page_flags &= ~TTM_TT_FLAG_PRIV_POPULATED;
 }
+EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_tt_unpopulate);
 
 #ifdef CONFIG_DEBUG_FS
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v12 10/10] drm/ttm/tests: Add TODO file
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
                   ` (8 preceding siblings ...)
  2024-05-15 11:24 ` [PATCH v12 09/10] drm/ttm/tests: Add tests for ttm_tt_populate Karolina Stolarek
@ 2024-05-15 11:24 ` Karolina Stolarek
  2024-05-24 15:29   ` Thomas Hellström
  2024-05-17  3:37 ` [PATCH v12 00/10] Improve test coverage of TTM Somalapuram, Amaranath
  10 siblings, 1 reply; 20+ messages in thread
From: Karolina Stolarek @ 2024-05-15 11:24 UTC (permalink / raw)
  To: dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram,
	Thomas Hellström, Karolina Stolarek

List improvements for the test suite with some notes.

Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
---
 drivers/gpu/drm/ttm/tests/TODO | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 drivers/gpu/drm/ttm/tests/TODO

diff --git a/drivers/gpu/drm/ttm/tests/TODO b/drivers/gpu/drm/ttm/tests/TODO
new file mode 100644
index 000000000000..b48d83b6166e
--- /dev/null
+++ b/drivers/gpu/drm/ttm/tests/TODO
@@ -0,0 +1,25 @@
+TODO
+=====
+
+- Add a test case where the only evictable BO is busy
+- Update eviction tests so they use parametrized "from" memory type
+- Improve mock manager's implementation, e.g. allocate a block of
+  dummy memory that can be used when testing page mapping functions
+- Suggestion: Add test cases with external BOs
+- Suggestion: randomize the number and size of tested buffers in
+  ttm_bo_validate()
+- Agree on the naming convention
+
+Notes and gotchas
+=================
+
+- These tests are built and run with a UML kernel, because
+  1) We are interested in hardware-independent testing
+  2) We don't want to have actual DRM devices interacting with TTM
+     at the same time as the test one. Getting these to work in
+     parallel would require some time (...and that's a "todo" in itself!)
+- Triggering ttm_bo_vm_ops callbacks from KUnit (i.e. kernel) might be
+  a challenge, but is worth trying. Look at selftests like
+  i915/gem/selftests/i915_gem_mman.c for inspiration
+- The test suite uses UML where ioremap() call returns NULL, meaning that
+  ttm_bo_ioremap() can't be tested, unless we find a way to stub it
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 00/10] Improve test coverage of TTM
  2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
                   ` (9 preceding siblings ...)
  2024-05-15 11:24 ` [PATCH v12 10/10] drm/ttm/tests: Add TODO file Karolina Stolarek
@ 2024-05-17  3:37 ` Somalapuram, Amaranath
  10 siblings, 0 replies; 20+ messages in thread
From: Somalapuram, Amaranath @ 2024-05-17  3:37 UTC (permalink / raw)
  To: Karolina Stolarek, dri-devel
  Cc: Christian König, Matthew Auld, Thomas Hellström,
	Arunpravin Paneer Selvam

Test looks good.

Regards,
S.Amarnath

amar@amar-Artic:~/amar/drm_misc/drm-misc1$ 
./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/ttm/tests
[08:20:02] Configuring KUnit Kernel ...
Regenerating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[08:20:03] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=16
[08:20:11] Starting KUnit Kernel (1/1)...
[08:20:11] ============================================================
Running tests with:
$ .kunit/linux kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
[08:20:11] ================= ttm_device (5 subtests) ==================
[08:20:11] [PASSED] ttm_device_init_basic
[08:20:11] [PASSED] ttm_device_init_multiple
[08:20:11] [PASSED] ttm_device_fini_basic
[08:20:11] [PASSED] ttm_device_init_no_vma_man
[08:20:11] ================== ttm_device_init_pools ==================
[08:20:11] [PASSED] No DMA allocations, no DMA32 required
[08:20:11] [PASSED] DMA allocations, DMA32 required
[08:20:11] [PASSED] No DMA allocations, DMA32 required
[08:20:11] [PASSED] DMA allocations, no DMA32 required
[08:20:11] ============== [PASSED] ttm_device_init_pools ==============
[08:20:11] =================== [PASSED] ttm_device ====================
[08:20:11] ================== ttm_pool (8 subtests) ===================
[08:20:11] ================== ttm_pool_alloc_basic ===================
[08:20:11] [PASSED] One page
[08:20:11] [PASSED] More than one page
[08:20:11] [PASSED] Above the allocation limit
[08:20:11] [PASSED] One page, with coherent DMA mappings enabled
[08:20:11] [PASSED] Above the allocation limit, with coherent DMA 
mappings enabled
[08:20:11] ============== [PASSED] ttm_pool_alloc_basic ===============
[08:20:11] ============== ttm_pool_alloc_basic_dma_addr ==============
[08:20:11] [PASSED] One page
[08:20:11] [PASSED] More than one page
[08:20:11] [PASSED] Above the allocation limit
[08:20:11] [PASSED] One page, with coherent DMA mappings enabled
[08:20:11] [PASSED] Above the allocation limit, with coherent DMA 
mappings enabled
[08:20:11] ========== [PASSED] ttm_pool_alloc_basic_dma_addr ==========
[08:20:11] [PASSED] ttm_pool_alloc_order_caching_match
[08:20:11] [PASSED] ttm_pool_alloc_caching_mismatch
[08:20:11] [PASSED] ttm_pool_alloc_order_mismatch
[08:20:11] [PASSED] ttm_pool_free_dma_alloc
[08:20:11] [PASSED] ttm_pool_free_no_dma_alloc
[08:20:11] [PASSED] ttm_pool_fini_basic
[08:20:11] ==================== [PASSED] ttm_pool =====================
[08:20:11] ================ ttm_resource (8 subtests) =================
[08:20:11] ================= ttm_resource_init_basic =================
[08:20:11] [PASSED] Init resource in TTM_PL_SYSTEM
[08:20:11] [PASSED] Init resource in TTM_PL_VRAM
[08:20:11] [PASSED] Init resource in a private placement
[08:20:11] [PASSED] Init resource in TTM_PL_SYSTEM, set placement flags
[08:20:11] ============= [PASSED] ttm_resource_init_basic =============
[08:20:11] [PASSED] ttm_resource_init_pinned
[08:20:11] [PASSED] ttm_resource_fini_basic
[08:20:11] [PASSED] ttm_resource_manager_init_basic
[08:20:11] [PASSED] ttm_resource_manager_usage_basic
[08:20:11] [PASSED] ttm_resource_manager_set_used_basic
[08:20:11] [PASSED] ttm_sys_man_alloc_basic
[08:20:11] [PASSED] ttm_sys_man_free_basic
[08:20:11] ================== [PASSED] ttm_resource ===================
[08:20:11] =================== ttm_tt (15 subtests) ===================
[08:20:11] ==================== ttm_tt_init_basic ====================
[08:20:11] [PASSED] Page-aligned size
[08:20:11] [PASSED] Extra pages requested
[08:20:11] ================ [PASSED] ttm_tt_init_basic ================
[08:20:11] [PASSED] ttm_tt_init_misaligned
[08:20:11] [PASSED] ttm_tt_fini_basic
[08:20:11] [PASSED] ttm_tt_fini_sg
[08:20:11] [PASSED] ttm_tt_fini_shmem
[08:20:11] [PASSED] ttm_tt_create_basic
[08:20:11] [PASSED] ttm_tt_create_invalid_bo_type
[08:20:11] [PASSED] ttm_tt_create_ttm_exists
[08:20:11] [PASSED] ttm_tt_create_failed
[08:20:11] [PASSED] ttm_tt_destroy_basic
[08:20:11] [PASSED] ttm_tt_populate_null_ttm
[08:20:11] [PASSED] ttm_tt_populate_populated_ttm
[08:20:11] [PASSED] ttm_tt_unpopulate_basic
[08:20:11] [PASSED] ttm_tt_unpopulate_empty_ttm
[08:20:11] [PASSED] ttm_tt_swapin_basic
[08:20:11] ===================== [PASSED] ttm_tt ======================
[08:20:11] =================== ttm_bo (14 subtests) ===================
[08:20:11] =========== ttm_bo_reserve_optimistic_no_ticket ===========
[08:20:11] [PASSED] Cannot be interrupted and sleeps
[08:20:11] [PASSED] Cannot be interrupted, locks straight away
[08:20:11] [PASSED] Can be interrupted, sleeps
[08:20:11] ======= [PASSED] ttm_bo_reserve_optimistic_no_ticket =======
[08:20:11] [PASSED] ttm_bo_reserve_locked_no_sleep
[08:20:11] [PASSED] ttm_bo_reserve_no_wait_ticket
[08:20:11] [PASSED] ttm_bo_reserve_double_resv
[08:20:11] [PASSED] ttm_bo_reserve_interrupted
[08:20:11] [PASSED] ttm_bo_reserve_deadlock
[08:20:11] [PASSED] ttm_bo_unreserve_basic
[08:20:11] [PASSED] ttm_bo_unreserve_pinned
[08:20:11] [PASSED] ttm_bo_unreserve_bulk
[08:20:11] [PASSED] ttm_bo_put_basic
[08:20:11] [PASSED] ttm_bo_put_shared_resv
[08:20:11] [PASSED] ttm_bo_pin_basic
[08:20:11] [PASSED] ttm_bo_pin_unpin_resource
[08:20:11] [PASSED] ttm_bo_multiple_pin_one_unpin
[08:20:11] ===================== [PASSED] ttm_bo ======================
[08:20:11] ============== ttm_bo_validate (22 subtests) ===============
[08:20:11] ============== ttm_bo_init_reserved_sys_man ===============
[08:20:11] [PASSED] Buffer object for userspace
[08:20:11] [PASSED] Kernel buffer object
[08:20:11] [PASSED] Shared buffer object
[08:20:11] ========== [PASSED] ttm_bo_init_reserved_sys_man ===========
[08:20:11] ============== ttm_bo_init_reserved_mock_man ==============
[08:20:11] [PASSED] Buffer object for userspace
[08:20:11] [PASSED] Kernel buffer object
[08:20:11] [PASSED] Shared buffer object
[08:20:11] ========== [PASSED] ttm_bo_init_reserved_mock_man ==========
[08:20:11] [PASSED] ttm_bo_init_reserved_resv
[08:20:11] ================== ttm_bo_validate_basic ==================
[08:20:11] [PASSED] Buffer object for userspace
[08:20:11] [PASSED] Kernel buffer object
[08:20:11] [PASSED] Shared buffer object
[08:20:11] ============== [PASSED] ttm_bo_validate_basic ==============
[08:20:11] [PASSED] ttm_bo_validate_invalid_placement
[08:20:11] ============= ttm_bo_validate_same_placement ==============
[08:20:11] [PASSED] System manager
[08:20:11] [PASSED] VRAM manager
[08:20:11] ========= [PASSED] ttm_bo_validate_same_placement ==========
[08:20:11] [PASSED] ttm_bo_validate_failed_alloc
[08:20:11] [PASSED] ttm_bo_validate_pinned
[08:20:11] [PASSED] ttm_bo_validate_busy_placement
[08:20:11] ================ ttm_bo_validate_multihop =================
[08:20:11] [PASSED] Buffer object for userspace
[08:20:11] [PASSED] Kernel buffer object
[08:20:11] [PASSED] Shared buffer object
[08:20:11] ============ [PASSED] ttm_bo_validate_multihop =============
[08:20:11] ========== ttm_bo_validate_no_placement_signaled ==========
[08:20:11] [PASSED] Buffer object in system domain, no page vector
[08:20:11] [PASSED] Buffer object in system domain with an existing page 
vector
[08:20:11] ====== [PASSED] ttm_bo_validate_no_placement_signaled ======
[08:20:11] ======== ttm_bo_validate_no_placement_not_signaled ========
[08:20:11] [PASSED] Buffer object for userspace
[08:20:11] [PASSED] Kernel buffer object
[08:20:11] [PASSED] Shared buffer object
[08:20:11] ==== [PASSED] ttm_bo_validate_no_placement_not_signaled ====
[08:20:11] [PASSED] ttm_bo_validate_move_fence_signaled
[08:20:11] ========= ttm_bo_validate_move_fence_not_signaled =========
[08:20:11] [PASSED] Waits for GPU
[08:20:11] [PASSED] Tries to lock straight away
[08:20:11] ===== [PASSED] ttm_bo_validate_move_fence_not_signaled =====
[08:20:11] [PASSED] ttm_bo_validate_swapout
[08:20:11] [PASSED] ttm_bo_validate_happy_evict
[08:20:11] [PASSED] ttm_bo_validate_all_pinned_evict
[08:20:11] [PASSED] ttm_bo_validate_allowed_only_evict
[08:20:11] [PASSED] ttm_bo_validate_deleted_evict
[08:20:11] [PASSED] ttm_bo_validate_busy_domain_evict
[08:20:11] [PASSED] ttm_bo_validate_evict_gutting
[08:20:11] [PASSED] ttm_bo_validate_recrusive_evict
[08:20:11] ================= [PASSED] ttm_bo_validate =================
[08:20:11] ============================================================
[08:20:11] Testing complete. Ran 102 tests: passed: 102
[08:20:11] Elapsed time: 9.732s total, 0.927s configuring, 8.085s 
building, 0.559s running

On 5/15/2024 4:54 PM, Karolina Stolarek wrote:
> Introduce tests for ttm_bo_validate()/ttm_bo_init_validate() that exercise
> simple BO placement as well as eviction (including the case where the evict
> domain also requires eviction to fit the incoming buffer). Prepare KUnit
> helpers to handle such scenarios and add a mock VRAM manager. This series also
> includes some updates to the helpers and more definitions used to define
> "special" memory domains (e.g., one that can't allocate resources or is busy),
> as well as drive-by fixes for the tests.
>
> There are a couple of areas in which this test suite can be improved.
> Suggestions for future work can be found in the TODO file.
>
> Use kunit_tool script to manually run all the tests:
>
> $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/ttm/tests
>
> To build a kernel with TTM KUnit tests, use a UML configuration,
> enable CONFIG_KUNIT, and then select CONFIG_DRM_TTM_KUNIT_TEST.
>
> Many thanks,
> Karolina
>
> v12:
>    - Rewrite "drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk" patch to
>      extend ttm_bo_kunit_init() helper to accept a dma_resv object and update
>      calls to that helper (Christian)
>    - Update drm_buddy_free_list() calls with an extra argument
>
> v11:
>    - Delete CONFIG_DRM_KUNIT_TEST_HELPERS from .kunitconfig file, as it gets
>      automatically selected when TTM KUnit tests are enabled
>    - Call ttm_bo_put() in ttm_bo_validate_pinned() test case (Matt)
>    - Fix a copy-paste mistake in ttm_mem_type_cases definition (Matt)
>    - Update mock_move definition to do a hop on VRAM -> sysmem move and
>      delete a dummy multihop domain. Fix the eviction tests accordingly (Matt)
>    - Update ttm_bo_validate_swapout() to use TT domain instead of VRAM
>    - Update eviction test cases to create TT domain, so they can perform multihop
>    - Update TODO file, as it got outdated already
>
> v10:
>    Many things have happened over the course of three months, so the series
>    had to be slightly reworked and expanded to accommodate these changes:
>     - Set DMA coherent mapping mask in the KUnit device so ttm_pool_alloc
>       tests can be executed
>     - Update ttm_bo_validate_invalid_placement() test case to check against
>       the right return error. It's no longer -EINVAL (which only is returned
>       for pinned buffers), but -ENOMEM. The behaviour has changed in
>       commit cc941c70df39 ("drm/ttm: improve idle/busy handling v5")
>     - Rework ttm_placement_kunit_init() to accept only one array of places
>       and update the tests that use that helper
>     - Set fallback flags in eviction domains defined in TTM KUnit helpers
>     - Fix a warning raised by ttm_bo_unreserve_bulk() test case
>     - Scrap all r-bs and tested-by, as many things were updated and should
>       be checked again
>
> Karolina Stolarek (10):
>    drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk
>    drm/ttm/tests: Delete unnecessary config option
>    drm/ttm/tests: Set DMA mask in KUnit device
>    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
>    drm/ttm/tests: Add eviction testing
>    drm/ttm/tests: Add tests for ttm_tt_populate
>    drm/ttm/tests: Add TODO file
>
>   drivers/gpu/drm/Kconfig                       |    1 +
>   drivers/gpu/drm/ttm/tests/.kunitconfig        |    2 +-
>   drivers/gpu/drm/ttm/tests/Makefile            |    2 +
>   drivers/gpu/drm/ttm/tests/TODO                |   25 +
>   drivers/gpu/drm/ttm/tests/ttm_bo_test.c       |   40 +-
>   .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  | 1225 +++++++++++++++++
>   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  178 ++-
>   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |   13 +-
>   drivers/gpu/drm/ttm/tests/ttm_mock_manager.c  |  235 ++++
>   drivers/gpu/drm/ttm/tests/ttm_mock_manager.h  |   33 +
>   drivers/gpu/drm/ttm/tests/ttm_pool_test.c     |    4 +-
>   drivers/gpu/drm/ttm/tests/ttm_resource_test.c |    2 +-
>   drivers/gpu/drm/ttm/tests/ttm_tt_test.c       |  154 ++-
>   drivers/gpu/drm/ttm/ttm_tt.c                  |    3 +
>   14 files changed, 1862 insertions(+), 55 deletions(-)
>   create mode 100644 drivers/gpu/drm/ttm/tests/TODO
>   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
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 01/10] drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk
  2024-05-15 11:24 ` [PATCH v12 01/10] drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk Karolina Stolarek
@ 2024-05-24 15:16   ` Thomas Hellström
  0 siblings, 0 replies; 20+ messages in thread
From: Thomas Hellström @ 2024-05-24 15:16 UTC (permalink / raw)
  To: Karolina Stolarek, dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram

On Wed, 2024-05-15 at 13:24 +0200, Karolina Stolarek wrote:
> BOs in a bulk move have to share the same reservation object. That is
> not the case in the ttm_bo_unreserve_bulk subtest. Update
> ttm_bo_kunit_init() helper to accept dma_resv object so we can define
> buffer objects that share the same resv. Update calls to that helper
> accordingly.
> 
> Fixes: 995279d280d1 ("drm/ttm/tests: Add tests for ttm_bo functions")
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> ---
>  drivers/gpu/drm/ttm/tests/ttm_bo_test.c       | 40 +++++++++++------
> --
>  drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  7 +++-
>  drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |  3 +-
>  drivers/gpu/drm/ttm/tests/ttm_pool_test.c     |  4 +-
>  drivers/gpu/drm/ttm/tests/ttm_resource_test.c |  2 +-
>  drivers/gpu/drm/ttm/tests/ttm_tt_test.c       | 20 +++++-----
>  6 files changed, 45 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> index 1f8a4f8adc92..ffcfe5e6709a 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> @@ -56,7 +56,7 @@ static void
> ttm_bo_reserve_optimistic_no_ticket(struct kunit *test)
>  	struct ttm_buffer_object *bo;
>  	int err;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_bo_reserve(bo, params->interruptible, params-
> >no_wait, NULL);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> @@ -71,7 +71,7 @@ static void ttm_bo_reserve_locked_no_sleep(struct
> kunit *test)
>  	bool no_wait = true;
>  	int err;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	/* Let's lock it beforehand */
>  	dma_resv_lock(bo->base.resv, NULL);
> @@ -92,7 +92,7 @@ static void ttm_bo_reserve_no_wait_ticket(struct
> kunit *test)
>  
>  	ww_acquire_init(&ctx, &reservation_ww_class);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_bo_reserve(bo, interruptible, no_wait, &ctx);
>  	KUNIT_ASSERT_EQ(test, err, -EBUSY);
> @@ -110,7 +110,7 @@ static void ttm_bo_reserve_double_resv(struct
> kunit *test)
>  
>  	ww_acquire_init(&ctx, &reservation_ww_class);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_bo_reserve(bo, interruptible, no_wait, &ctx);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> @@ -138,8 +138,8 @@ static void ttm_bo_reserve_deadlock(struct kunit
> *test)
>  	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);
> +	bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
> +	bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	ww_acquire_init(&ctx1, &reservation_ww_class);
>  	mutex_lock(&bo2->base.resv->lock.base);
> @@ -208,7 +208,7 @@ static void ttm_bo_reserve_interrupted(struct
> kunit *test)
>  	struct task_struct *task;
>  	int err;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	task = kthread_create(threaded_ttm_bo_reserve, bo, "ttm-bo-
> reserve");
>  
> @@ -249,7 +249,7 @@ static void ttm_bo_unreserve_basic(struct kunit
> *test)
>  	KUNIT_ASSERT_EQ(test, err, 0);
>  	priv->ttm_dev = ttm_dev;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  	bo->priority = bo_prio;
>  
>  	err = ttm_resource_alloc(bo, place, &res1);
> @@ -288,7 +288,7 @@ static void ttm_bo_unreserve_pinned(struct kunit
> *test)
>  	KUNIT_ASSERT_EQ(test, err, 0);
>  	priv->ttm_dev = ttm_dev;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  	place = ttm_place_kunit_init(test, mem_type, 0);
>  
>  	dma_resv_lock(bo->base.resv, NULL);
> @@ -321,6 +321,7 @@ static void ttm_bo_unreserve_bulk(struct kunit
> *test)
>  	struct ttm_resource *res1, *res2;
>  	struct ttm_device *ttm_dev;
>  	struct ttm_place *place;
> +	struct dma_resv *resv;
>  	uint32_t mem_type = TTM_PL_SYSTEM;
>  	unsigned int bo_priority = 0;
>  	int err;
> @@ -332,12 +333,17 @@ static void ttm_bo_unreserve_bulk(struct kunit
> *test)
>  	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
>  
> +	resv = kunit_kzalloc(test, sizeof(*resv), 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_init(resv);
> +
> +	bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, resv);
> +	bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, resv);
>  
>  	dma_resv_lock(bo1->base.resv, NULL);
>  	ttm_bo_set_bulk_move(bo1, &lru_bulk_move);
> @@ -363,6 +369,8 @@ static void ttm_bo_unreserve_bulk(struct kunit
> *test)
>  
>  	ttm_resource_free(bo1, &res1);
>  	ttm_resource_free(bo2, &res2);
> +
> +	dma_resv_fini(resv);
>  }
>  
>  static void ttm_bo_put_basic(struct kunit *test)
> @@ -384,7 +392,7 @@ static void ttm_bo_put_basic(struct kunit *test)
>  	KUNIT_ASSERT_EQ(test, err, 0);
>  	priv->ttm_dev = ttm_dev;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  	bo->type = ttm_bo_type_device;
>  
>  	err = ttm_resource_alloc(bo, place, &res);
> @@ -445,7 +453,7 @@ static void ttm_bo_put_shared_resv(struct kunit
> *test)
>  
>  	dma_fence_signal(fence);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  	bo->type = ttm_bo_type_device;
>  	bo->base.resv = external_resv;
>  
> @@ -467,7 +475,7 @@ static void ttm_bo_pin_basic(struct kunit *test)
>  	KUNIT_ASSERT_EQ(test, err, 0);
>  	priv->ttm_dev = ttm_dev;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	for (int i = 0; i < no_pins; i++) {
>  		dma_resv_lock(bo->base.resv, NULL);
> @@ -502,7 +510,7 @@ static void ttm_bo_pin_unpin_resource(struct
> kunit *test)
>  	KUNIT_ASSERT_EQ(test, err, 0);
>  	priv->ttm_dev = ttm_dev;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_resource_alloc(bo, place, &res);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> @@ -553,7 +561,7 @@ static void ttm_bo_multiple_pin_one_unpin(struct
> kunit *test)
>  	KUNIT_ASSERT_EQ(test, err, 0);
>  	priv->ttm_dev = ttm_dev;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_resource_alloc(bo, place, &res);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> index 7b7c1fa805fc..5be317a0af56 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> @@ -51,7 +51,8 @@ EXPORT_SYMBOL_GPL(ttm_device_kunit_init);
>  
>  struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
>  					    struct ttm_test_devices
> *devs,
> -					    size_t size)
> +					    size_t size,
> +					    struct dma_resv *obj)
>  {
>  	struct drm_gem_object gem_obj = { };
>  	struct ttm_buffer_object *bo;
> @@ -61,6 +62,10 @@ struct ttm_buffer_object *ttm_bo_kunit_init(struct
> kunit *test,
>  	KUNIT_ASSERT_NOT_NULL(test, bo);
>  
>  	bo->base = gem_obj;
> +
> +	if (obj)
> +		bo->base.resv = obj;
> +
>  	err = drm_gem_object_init(devs->drm, &bo->base, size);
>  	KUNIT_ASSERT_EQ(test, err, 0);
>  
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
> index 2f51c833a536..c83d31b23c9a 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
> @@ -28,7 +28,8 @@ int ttm_device_kunit_init(struct ttm_test_devices
> *priv,
>  			  bool use_dma32);
>  struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
>  					    struct ttm_test_devices
> *devs,
> -					    size_t size);
> +					    size_t size,
> +					    struct dma_resv *obj);
>  struct ttm_place *ttm_place_kunit_init(struct kunit *test,
>  				       uint32_t mem_type, uint32_t
> flags);
>  
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
> b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
> index 0a3fede84da9..4643f91c6bd5 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
> @@ -57,7 +57,7 @@ static struct ttm_tt *ttm_tt_kunit_init(struct
> kunit *test,
>  	struct ttm_tt *tt;
>  	int err;
>  
> -	bo = ttm_bo_kunit_init(test, priv->devs, size);
> +	bo = ttm_bo_kunit_init(test, priv->devs, size, NULL);
>  	KUNIT_ASSERT_NOT_NULL(test, bo);
>  	priv->mock_bo = bo;
>  
> @@ -209,7 +209,7 @@ static void ttm_pool_alloc_basic_dma_addr(struct
> kunit *test)
>  	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, tt);
>  
> -	bo = ttm_bo_kunit_init(test, devs, size);
> +	bo = ttm_bo_kunit_init(test, devs, size, NULL);
>  	KUNIT_ASSERT_NOT_NULL(test, bo);
>  
>  	err = ttm_sg_tt_init(tt, bo, 0, caching);
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
> b/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
> index 029e1f094bb0..67584058dadb 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
> @@ -54,7 +54,7 @@ static void ttm_init_test_mocks(struct kunit *test,
>  	/* 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->bo = ttm_bo_kunit_init(test, priv->devs, size, NULL);
>  	priv->place = ttm_place_kunit_init(test, mem_type, flags);
>  }
>  
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
> b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
> index fd4502c18de6..67bf51723c92 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_tt_test.c
> @@ -63,7 +63,7 @@ static void ttm_tt_init_basic(struct kunit *test)
>  	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, tt);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, params->size);
> +	bo = ttm_bo_kunit_init(test, test->priv, params->size,
> NULL);
>  
>  	err = ttm_tt_init(tt, bo, page_flags, caching, extra_pages);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> @@ -89,7 +89,7 @@ static void ttm_tt_init_misaligned(struct kunit
> *test)
>  	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, tt);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, size);
> +	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
>  
>  	/* Make the object size misaligned */
>  	bo->base.size += 1;
> @@ -110,7 +110,7 @@ static void ttm_tt_fini_basic(struct kunit *test)
>  	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, tt);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_tt_init(tt, bo, 0, caching, 0);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> @@ -130,7 +130,7 @@ static void ttm_tt_fini_sg(struct kunit *test)
>  	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, tt);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_sg_tt_init(tt, bo, 0, caching);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> @@ -151,7 +151,7 @@ static void ttm_tt_fini_shmem(struct kunit *test)
>  	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, tt);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_tt_init(tt, bo, 0, caching, 0);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> @@ -168,7 +168,7 @@ 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 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  	bo->type = ttm_bo_type_device;
>  
>  	dma_resv_lock(bo->base.resv, NULL);
> @@ -187,7 +187,7 @@ 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 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  	bo->type = ttm_bo_type_sg + 1;
>  
>  	dma_resv_lock(bo->base.resv, NULL);
> @@ -208,7 +208,7 @@ static void ttm_tt_create_ttm_exists(struct kunit
> *test)
>  	tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, tt);
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	err = ttm_tt_init(tt, bo, 0, caching, 0);
>  	KUNIT_ASSERT_EQ(test, err, 0);
> @@ -239,7 +239,7 @@ static void ttm_tt_create_failed(struct kunit
> *test)
>  	struct ttm_buffer_object *bo;
>  	int err;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	/* Update ttm_device_funcs so we don't alloc ttm_tt */
>  	devs->ttm_dev->funcs = &ttm_dev_empty_funcs;
> @@ -257,7 +257,7 @@ static void ttm_tt_destroy_basic(struct kunit
> *test)
>  	struct ttm_buffer_object *bo;
>  	int err;
>  
> -	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
> +	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
>  
>  	dma_resv_lock(bo->base.resv, NULL);
>  	err = ttm_tt_create(bo, false);


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 10/10] drm/ttm/tests: Add TODO file
  2024-05-15 11:24 ` [PATCH v12 10/10] drm/ttm/tests: Add TODO file Karolina Stolarek
@ 2024-05-24 15:29   ` Thomas Hellström
  0 siblings, 0 replies; 20+ messages in thread
From: Thomas Hellström @ 2024-05-24 15:29 UTC (permalink / raw)
  To: Karolina Stolarek, dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram

On Wed, 2024-05-15 at 13:24 +0200, Karolina Stolarek wrote:
> List improvements for the test suite with some notes.
> 
> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
LGTM.
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

> ---
>  drivers/gpu/drm/ttm/tests/TODO | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 drivers/gpu/drm/ttm/tests/TODO
> 
> diff --git a/drivers/gpu/drm/ttm/tests/TODO
> b/drivers/gpu/drm/ttm/tests/TODO
> new file mode 100644
> index 000000000000..b48d83b6166e
> --- /dev/null
> +++ b/drivers/gpu/drm/ttm/tests/TODO
> @@ -0,0 +1,25 @@
> +TODO
> +=====
> +
> +- Add a test case where the only evictable BO is busy
> +- Update eviction tests so they use parametrized "from" memory type
> +- Improve mock manager's implementation, e.g. allocate a block of
> +  dummy memory that can be used when testing page mapping functions
> +- Suggestion: Add test cases with external BOs
> +- Suggestion: randomize the number and size of tested buffers in
> +  ttm_bo_validate()
> +- Agree on the naming convention
> +
> +Notes and gotchas
> +=================
> +
> +- These tests are built and run with a UML kernel, because
> +  1) We are interested in hardware-independent testing
> +  2) We don't want to have actual DRM devices interacting with TTM
> +     at the same time as the test one. Getting these to work in
> +     parallel would require some time (...and that's a "todo" in
> itself!)
> +- Triggering ttm_bo_vm_ops callbacks from KUnit (i.e. kernel) might
> be
> +  a challenge, but is worth trying. Look at selftests like
> +  i915/gem/selftests/i915_gem_mman.c for inspiration
> +- The test suite uses UML where ioremap() call returns NULL, meaning
> that
> +  ttm_bo_ioremap() can't be tested, unless we find a way to stub it


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers
  2024-05-15 11:24 ` [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers Karolina Stolarek
@ 2024-05-29 12:58   ` Thomas Hellström
  2024-06-03  6:55     ` Karolina Stolarek
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Hellström @ 2024-05-29 12:58 UTC (permalink / raw)
  To: Karolina Stolarek, dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram

On Wed, 2024-05-15 at 13:24 +0200, Karolina Stolarek wrote:
> 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>
> Tested-by: Somalapuram, Amaranath <asomalap@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  | 274
> ++++++++++++++++++
>  drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  38 ++-
>  drivers/gpu/drm/ttm/tests/ttm_mock_manager.c  | 207 +++++++++++++
>  drivers/gpu/drm/ttm/tests/ttm_mock_manager.h  |  31 ++
>  7 files changed, 551 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 026444eeb5c6..4ba16501dbf7 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -234,6 +234,7 @@ config DRM_TTM_KUNIT_TEST
>          default n
>          depends on DRM && KUNIT && MMU && (UML || COMPILE_TEST)
>          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 1ae1ffabd51e..772f0e1f4103 100644
> --- a/drivers/gpu/drm/ttm/tests/.kunitconfig
> +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig
> @@ -1,3 +1,4 @@
>  CONFIG_KUNIT=y
>  CONFIG_DRM=y
>  CONFIG_DRM_TTM_KUNIT_TEST=y
> +CONFIG_DRM_BUDDY=y

Is this strictly needed when CONFIG_DRM_TTM_KUNIT_TEST is selected?
Wouldn't that be enabled implicitly?

> 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 a5520b0631a3..8b62d95b8ab8 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;

Please use u32 instead of unit32_t in new code. The unit32_t usage in
TTM is a remnant from when much of the drm- and ttm code was shared
with *bsd. Same in a couple of places below.

>  	bool with_ttm;
>  };
>  
> @@ -102,6 +105,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;
> +	enum ttm_bo_type bo_type = params->bo_type;
> +	struct ttm_test_devices *priv = test->priv;
> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> +	struct ttm_operation_ctx ctx = { };
> +	struct ttm_placement *placement;
> +	uint32_t mem_type = TTM_PL_VRAM;
> +	struct ttm_buffer_object *bo;
> +	struct ttm_place *place;
> +	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);
> +
> +	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)
>  {
>  	enum ttm_bo_type bo_type = ttm_bo_type_device;
> @@ -136,6 +182,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;
> +	uint32_t fst_mem = TTM_PL_SYSTEM, snd_mem = TTM_PL_VRAM;
> +	struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
> +	struct ttm_placement *fst_placement, *snd_placement;
> +	struct ttm_test_devices *priv = test->priv;
> +	struct ttm_place *fst_place, *snd_place;
> +	uint32_t size = ALIGN(SZ_8K, PAGE_SIZE);
> +	struct ttm_buffer_object *bo;
> +	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);
> +
> +	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);
> +
> +	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)
>  {
>  	enum ttm_bo_type bo_type = ttm_bo_type_device;
> @@ -162,6 +253,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)
> +{
> +	enum ttm_bo_type bo_type = ttm_bo_type_device;
> +	struct ttm_test_devices *priv = test->priv;
> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> +	struct ttm_operation_ctx ctx = { };
> +	struct ttm_placement *placement;
> +	uint32_t mem_type = TTM_PL_VRAM;
> +	struct ttm_buffer_object *bo;
> +	struct ttm_place *place;
> +	int err;
> +
> +	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
> +	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);
> +
> +	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)
>  {
>  	enum ttm_bo_type bo_type = ttm_bo_type_device;
> @@ -193,11 +314,164 @@ static void ttm_bo_validate_pinned(struct
> kunit *test)
>  	ttm_bo_put(bo);
>  }
>  
> +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_VRAM,
> +	},
> +};
> +
> +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_operation_ctx ctx_init = { }, ctx_val = { };
> +	struct ttm_test_devices *priv = test->priv;
> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> +	struct ttm_placement *placement;
> +	struct ttm_buffer_object *bo;
> +	struct ttm_place *place;
> +	int err;
> +
> +	place = ttm_place_kunit_init(test, params->mem_type, 0);
> +	placement = ttm_placement_kunit_init(test, place, 1);
> +
> +	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);
> +
> +	ttm_bo_put(bo);
> +
> +	if (params->mem_type != TTM_PL_SYSTEM)
> +		ttm_mock_manager_fini(priv->ttm_dev, params-
> >mem_type);
> +}
> +
> +static void ttm_bo_validate_busy_placement(struct kunit *test)
> +{
> +	uint32_t fst_mem = TTM_PL_VRAM, snd_mem = TTM_PL_VRAM + 1;
> +	struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
> +	struct ttm_placement *placement_init, *placement_val;
> +	enum ttm_bo_type bo_type = ttm_bo_type_device;
> +	struct ttm_test_devices *priv = test->priv;
> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> +	struct ttm_place *init_place, places[2];
> +	struct ttm_resource_manager *man;
> +	struct ttm_buffer_object *bo;
> +	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);
> +
> +	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);
> +
> +	places[0] = (struct ttm_place){ .mem_type = fst_mem, .flags
> = TTM_PL_FLAG_DESIRED };
> +	places[1] = (struct ttm_place){ .mem_type = snd_mem, .flags
> = TTM_PL_FLAG_FALLBACK };
> +	placement_val = ttm_placement_kunit_init(test, places, 2);
> +
> +	err = ttm_bo_validate(bo, placement_val, &ctx_val);
> +	dma_resv_unlock(bo->base.resv);
> +
> +	man = ttm_manager_type(priv->ttm_dev, snd_mem);
> +
> +	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_operation_ctx ctx_init = { }, ctx_val = { };
> +	struct ttm_placement *placement_init, *placement_val;
> +	uint32_t fst_mem = TTM_PL_VRAM, tmp_mem = TTM_PL_TT,
> +		 final_mem = TTM_PL_SYSTEM;
> +	struct ttm_test_devices *priv = test->priv;
> +	struct ttm_place *fst_place, *final_place;
> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> +	struct ttm_buffer_object *bo;
> +	int err;
> +
> +	ttm_mock_manager_init(priv->ttm_dev, fst_mem, MANAGER_SIZE);
> +	ttm_mock_manager_init(priv->ttm_dev, tmp_mem, MANAGER_SIZE);
> +
> +	fst_place = ttm_place_kunit_init(test, fst_mem, 0);
> +	placement_init = ttm_placement_kunit_init(test, fst_place,
> 1);
> +
> +	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);
> +
> +	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);
> +}
> +
>  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 2f590bae53f8..2a2585b37118 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> @@ -27,8 +27,42 @@ 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;
> +	}
> +
> +	if (bo->resource->mem_type == TTM_PL_VRAM &&
> +	    new_mem->mem_type == TTM_PL_SYSTEM) {
> +		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);

We're not doing any accelerated move here, so ttm_bo_move_null() should
be sufficient also in this case?

> +
> +		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);

Do we hit this move_memcpy()? Since the mock manager doesn't actually
reserve any memory to manager, I'd expect this to run into problems?



>  }
>  
>  struct ttm_device_funcs ttm_dev_funcs = {
> 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..eb9dca1de1a2
> --- /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, 0);
> +	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, 0);
> +	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");

When the module is dual-licensed IIRC the correct option to use here is
"GPL and additional rights"

> 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

Thanks,
Thomas


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers
  2024-05-29 12:58   ` Thomas Hellström
@ 2024-06-03  6:55     ` Karolina Stolarek
  2024-06-03  7:24       ` Thomas Hellström
  0 siblings, 1 reply; 20+ messages in thread
From: Karolina Stolarek @ 2024-06-03  6:55 UTC (permalink / raw)
  To: Thomas Hellström, dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram

Hi Thomas,

On 29.05.2024 14:58, Thomas Hellström wrote:
> On Wed, 2024-05-15 at 13:24 +0200, Karolina Stolarek wrote:
>> 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>
>> Tested-by: Somalapuram, Amaranath <asomalap@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  | 274
>> ++++++++++++++++++
>>   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  38 ++-
>>   drivers/gpu/drm/ttm/tests/ttm_mock_manager.c  | 207 +++++++++++++
>>   drivers/gpu/drm/ttm/tests/ttm_mock_manager.h  |  31 ++
>>   7 files changed, 551 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 026444eeb5c6..4ba16501dbf7 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -234,6 +234,7 @@ config DRM_TTM_KUNIT_TEST
>>           default n
>>           depends on DRM && KUNIT && MMU && (UML || COMPILE_TEST)
>>           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 1ae1ffabd51e..772f0e1f4103 100644
>> --- a/drivers/gpu/drm/ttm/tests/.kunitconfig
>> +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig
>> @@ -1,3 +1,4 @@
>>   CONFIG_KUNIT=y
>>   CONFIG_DRM=y
>>   CONFIG_DRM_TTM_KUNIT_TEST=y
>> +CONFIG_DRM_BUDDY=y
> 
> Is this strictly needed when CONFIG_DRM_TTM_KUNIT_TEST is selected?
> Wouldn't that be enabled implicitly?

Ah, yes, that should get selected implicitly. I'll check and remove if 
that works, thanks.

> 
>> 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 a5520b0631a3..8b62d95b8ab8 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;
> 
> Please use u32 instead of unit32_t in new code. The unit32_t usage in
> TTM is a remnant from when much of the drm- and ttm code was shared
> with *bsd. Same in a couple of places below.

I see. So, the question is what should I about other test code that is 
already merged? Submit a separate patch to change uint32_t --> u32?

> 
>>   	bool with_ttm;
>>   };
>>   
>> @@ -102,6 +105,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;
>> +	enum ttm_bo_type bo_type = params->bo_type;
>> +	struct ttm_test_devices *priv = test->priv;
>> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
>> +	struct ttm_operation_ctx ctx = { };
>> +	struct ttm_placement *placement;
>> +	uint32_t mem_type = TTM_PL_VRAM;
>> +	struct ttm_buffer_object *bo;
>> +	struct ttm_place *place;
>> +	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);
>> +
>> +	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)
>>   {
>>   	enum ttm_bo_type bo_type = ttm_bo_type_device;
>> @@ -136,6 +182,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;
>> +	uint32_t fst_mem = TTM_PL_SYSTEM, snd_mem = TTM_PL_VRAM;
>> +	struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
>> +	struct ttm_placement *fst_placement, *snd_placement;
>> +	struct ttm_test_devices *priv = test->priv;
>> +	struct ttm_place *fst_place, *snd_place;
>> +	uint32_t size = ALIGN(SZ_8K, PAGE_SIZE);
>> +	struct ttm_buffer_object *bo;
>> +	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);
>> +
>> +	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);
>> +
>> +	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)
>>   {
>>   	enum ttm_bo_type bo_type = ttm_bo_type_device;
>> @@ -162,6 +253,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)
>> +{
>> +	enum ttm_bo_type bo_type = ttm_bo_type_device;
>> +	struct ttm_test_devices *priv = test->priv;
>> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
>> +	struct ttm_operation_ctx ctx = { };
>> +	struct ttm_placement *placement;
>> +	uint32_t mem_type = TTM_PL_VRAM;
>> +	struct ttm_buffer_object *bo;
>> +	struct ttm_place *place;
>> +	int err;
>> +
>> +	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
>> +	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);
>> +
>> +	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)
>>   {
>>   	enum ttm_bo_type bo_type = ttm_bo_type_device;
>> @@ -193,11 +314,164 @@ static void ttm_bo_validate_pinned(struct
>> kunit *test)
>>   	ttm_bo_put(bo);
>>   }
>>   
>> +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_VRAM,
>> +	},
>> +};
>> +
>> +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_operation_ctx ctx_init = { }, ctx_val = { };
>> +	struct ttm_test_devices *priv = test->priv;
>> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
>> +	struct ttm_placement *placement;
>> +	struct ttm_buffer_object *bo;
>> +	struct ttm_place *place;
>> +	int err;
>> +
>> +	place = ttm_place_kunit_init(test, params->mem_type, 0);
>> +	placement = ttm_placement_kunit_init(test, place, 1);
>> +
>> +	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);
>> +
>> +	ttm_bo_put(bo);
>> +
>> +	if (params->mem_type != TTM_PL_SYSTEM)
>> +		ttm_mock_manager_fini(priv->ttm_dev, params-
>>> mem_type);
>> +}
>> +
>> +static void ttm_bo_validate_busy_placement(struct kunit *test)
>> +{
>> +	uint32_t fst_mem = TTM_PL_VRAM, snd_mem = TTM_PL_VRAM + 1;
>> +	struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
>> +	struct ttm_placement *placement_init, *placement_val;
>> +	enum ttm_bo_type bo_type = ttm_bo_type_device;
>> +	struct ttm_test_devices *priv = test->priv;
>> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
>> +	struct ttm_place *init_place, places[2];
>> +	struct ttm_resource_manager *man;
>> +	struct ttm_buffer_object *bo;
>> +	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);
>> +
>> +	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);
>> +
>> +	places[0] = (struct ttm_place){ .mem_type = fst_mem, .flags
>> = TTM_PL_FLAG_DESIRED };
>> +	places[1] = (struct ttm_place){ .mem_type = snd_mem, .flags
>> = TTM_PL_FLAG_FALLBACK };
>> +	placement_val = ttm_placement_kunit_init(test, places, 2);
>> +
>> +	err = ttm_bo_validate(bo, placement_val, &ctx_val);
>> +	dma_resv_unlock(bo->base.resv);
>> +
>> +	man = ttm_manager_type(priv->ttm_dev, snd_mem);
>> +
>> +	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_operation_ctx ctx_init = { }, ctx_val = { };
>> +	struct ttm_placement *placement_init, *placement_val;
>> +	uint32_t fst_mem = TTM_PL_VRAM, tmp_mem = TTM_PL_TT,
>> +		 final_mem = TTM_PL_SYSTEM;
>> +	struct ttm_test_devices *priv = test->priv;
>> +	struct ttm_place *fst_place, *final_place;
>> +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
>> +	struct ttm_buffer_object *bo;
>> +	int err;
>> +
>> +	ttm_mock_manager_init(priv->ttm_dev, fst_mem, MANAGER_SIZE);
>> +	ttm_mock_manager_init(priv->ttm_dev, tmp_mem, MANAGER_SIZE);
>> +
>> +	fst_place = ttm_place_kunit_init(test, fst_mem, 0);
>> +	placement_init = ttm_placement_kunit_init(test, fst_place,
>> 1);
>> +
>> +	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);
>> +
>> +	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);
>> +}
>> +
>>   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 2f590bae53f8..2a2585b37118 100644
>> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>> @@ -27,8 +27,42 @@ 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;
>> +	}
>> +
>> +	if (bo->resource->mem_type == TTM_PL_VRAM &&
>> +	    new_mem->mem_type == TTM_PL_SYSTEM) {
>> +		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);
> 
> We're not doing any accelerated move here, so ttm_bo_move_null() should
> be sufficient also in this case?

I'll remove that, thanks.

> 
>> +
>> +		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);
> 
> Do we hit this move_memcpy()? Since the mock manager doesn't actually
> reserve any memory to manager, I'd expect this to run into problems?

We do. The mock manager has use_tt=true, so on move, we'd use 
ttm_kmap_iter_tt_init() for src and dest and copy the pages. I'm not 
sure if that's the right approach, but it enables me to test if 
ttm_operation_ctx's bytes_moved is correctly updated.

> 
> 
> 
>>   }
>>   
>>   struct ttm_device_funcs ttm_dev_funcs = {
>> 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..eb9dca1de1a2
>> --- /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, 0);
>> +	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, 0);
>> +	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");
> 
> When the module is dual-licensed IIRC the correct option to use here is
> "GPL and additional rights"

I took that module license from DRM KUnit tests, but I'll update it, thanks.

All the best,
Karolina

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers
  2024-06-03  6:55     ` Karolina Stolarek
@ 2024-06-03  7:24       ` Thomas Hellström
  2024-06-03  8:28         ` Karolina Stolarek
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Hellström @ 2024-06-03  7:24 UTC (permalink / raw)
  To: Karolina Stolarek, dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram

Hi

On Mon, 2024-06-03 at 08:55 +0200, Karolina Stolarek wrote:
> Hi Thomas,
> 
> On 29.05.2024 14:58, Thomas Hellström wrote:
> > On Wed, 2024-05-15 at 13:24 +0200, Karolina Stolarek wrote:
> > > 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>
> > > Tested-by: Somalapuram, Amaranath <asomalap@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  | 274
> > > ++++++++++++++++++
> > >   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  38 ++-
> > >   drivers/gpu/drm/ttm/tests/ttm_mock_manager.c  | 207
> > > +++++++++++++
> > >   drivers/gpu/drm/ttm/tests/ttm_mock_manager.h  |  31 ++
> > >   7 files changed, 551 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 026444eeb5c6..4ba16501dbf7 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -234,6 +234,7 @@ config DRM_TTM_KUNIT_TEST
> > >           default n
> > >           depends on DRM && KUNIT && MMU && (UML || COMPILE_TEST)
> > >           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 1ae1ffabd51e..772f0e1f4103 100644
> > > --- a/drivers/gpu/drm/ttm/tests/.kunitconfig
> > > +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig
> > > @@ -1,3 +1,4 @@
> > >   CONFIG_KUNIT=y
> > >   CONFIG_DRM=y
> > >   CONFIG_DRM_TTM_KUNIT_TEST=y
> > > +CONFIG_DRM_BUDDY=y
> > 
> > Is this strictly needed when CONFIG_DRM_TTM_KUNIT_TEST is selected?
> > Wouldn't that be enabled implicitly?
> 
> Ah, yes, that should get selected implicitly. I'll check and remove
> if 
> that works, thanks.
> 
> > 
> > > 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 a5520b0631a3..8b62d95b8ab8 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;
> > 
> > Please use u32 instead of unit32_t in new code. The unit32_t usage
> > in
> > TTM is a remnant from when much of the drm- and ttm code was shared
> > with *bsd. Same in a couple of places below.
> 
> I see. So, the question is what should I about other test code that
> is 
> already merged? Submit a separate patch to change uint32_t --> u32?

Yes, IMO that's a good idea. And at some point I think we would want to
move all of TTM over as well.

Christian, any preferences?

> 
> > 
> > >   	bool with_ttm;
> > >   };
> > >   
> > > @@ -102,6 +105,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;
> > > +	enum ttm_bo_type bo_type = params->bo_type;
> > > +	struct ttm_test_devices *priv = test->priv;
> > > +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> > > +	struct ttm_operation_ctx ctx = { };
> > > +	struct ttm_placement *placement;
> > > +	uint32_t mem_type = TTM_PL_VRAM;
> > > +	struct ttm_buffer_object *bo;
> > > +	struct ttm_place *place;
> > > +	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);
> > > +
> > > +	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)
> > >   {
> > >   	enum ttm_bo_type bo_type = ttm_bo_type_device;
> > > @@ -136,6 +182,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;
> > > +	uint32_t fst_mem = TTM_PL_SYSTEM, snd_mem = TTM_PL_VRAM;
> > > +	struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
> > > +	struct ttm_placement *fst_placement, *snd_placement;
> > > +	struct ttm_test_devices *priv = test->priv;
> > > +	struct ttm_place *fst_place, *snd_place;
> > > +	uint32_t size = ALIGN(SZ_8K, PAGE_SIZE);
> > > +	struct ttm_buffer_object *bo;
> > > +	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);
> > > +
> > > +	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);
> > > +
> > > +	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)
> > >   {
> > >   	enum ttm_bo_type bo_type = ttm_bo_type_device;
> > > @@ -162,6 +253,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)
> > > +{
> > > +	enum ttm_bo_type bo_type = ttm_bo_type_device;
> > > +	struct ttm_test_devices *priv = test->priv;
> > > +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> > > +	struct ttm_operation_ctx ctx = { };
> > > +	struct ttm_placement *placement;
> > > +	uint32_t mem_type = TTM_PL_VRAM;
> > > +	struct ttm_buffer_object *bo;
> > > +	struct ttm_place *place;
> > > +	int err;
> > > +
> > > +	bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
> > > +	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);
> > > +
> > > +	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)
> > >   {
> > >   	enum ttm_bo_type bo_type = ttm_bo_type_device;
> > > @@ -193,11 +314,164 @@ static void ttm_bo_validate_pinned(struct
> > > kunit *test)
> > >   	ttm_bo_put(bo);
> > >   }
> > >   
> > > +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_VRAM,
> > > +	},
> > > +};
> > > +
> > > +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_operation_ctx ctx_init = { }, ctx_val = { };
> > > +	struct ttm_test_devices *priv = test->priv;
> > > +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> > > +	struct ttm_placement *placement;
> > > +	struct ttm_buffer_object *bo;
> > > +	struct ttm_place *place;
> > > +	int err;
> > > +
> > > +	place = ttm_place_kunit_init(test, params->mem_type, 0);
> > > +	placement = ttm_placement_kunit_init(test, place, 1);
> > > +
> > > +	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);
> > > +
> > > +	ttm_bo_put(bo);
> > > +
> > > +	if (params->mem_type != TTM_PL_SYSTEM)
> > > +		ttm_mock_manager_fini(priv->ttm_dev, params-
> > > > mem_type);
> > > +}
> > > +
> > > +static void ttm_bo_validate_busy_placement(struct kunit *test)
> > > +{
> > > +	uint32_t fst_mem = TTM_PL_VRAM, snd_mem = TTM_PL_VRAM +
> > > 1;
> > > +	struct ttm_operation_ctx ctx_init = { }, ctx_val = { };
> > > +	struct ttm_placement *placement_init, *placement_val;
> > > +	enum ttm_bo_type bo_type = ttm_bo_type_device;
> > > +	struct ttm_test_devices *priv = test->priv;
> > > +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> > > +	struct ttm_place *init_place, places[2];
> > > +	struct ttm_resource_manager *man;
> > > +	struct ttm_buffer_object *bo;
> > > +	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);
> > > +
> > > +	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);
> > > +
> > > +	places[0] = (struct ttm_place){ .mem_type = fst_mem,
> > > .flags
> > > = TTM_PL_FLAG_DESIRED };
> > > +	places[1] = (struct ttm_place){ .mem_type = snd_mem,
> > > .flags
> > > = TTM_PL_FLAG_FALLBACK };
> > > +	placement_val = ttm_placement_kunit_init(test, places,
> > > 2);
> > > +
> > > +	err = ttm_bo_validate(bo, placement_val, &ctx_val);
> > > +	dma_resv_unlock(bo->base.resv);
> > > +
> > > +	man = ttm_manager_type(priv->ttm_dev, snd_mem);
> > > +
> > > +	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_operation_ctx ctx_init = { }, ctx_val = { };
> > > +	struct ttm_placement *placement_init, *placement_val;
> > > +	uint32_t fst_mem = TTM_PL_VRAM, tmp_mem = TTM_PL_TT,
> > > +		 final_mem = TTM_PL_SYSTEM;
> > > +	struct ttm_test_devices *priv = test->priv;
> > > +	struct ttm_place *fst_place, *final_place;
> > > +	uint32_t size = ALIGN(BO_SIZE, PAGE_SIZE);
> > > +	struct ttm_buffer_object *bo;
> > > +	int err;
> > > +
> > > +	ttm_mock_manager_init(priv->ttm_dev, fst_mem,
> > > MANAGER_SIZE);
> > > +	ttm_mock_manager_init(priv->ttm_dev, tmp_mem,
> > > MANAGER_SIZE);
> > > +
> > > +	fst_place = ttm_place_kunit_init(test, fst_mem, 0);
> > > +	placement_init = ttm_placement_kunit_init(test,
> > > fst_place,
> > > 1);
> > > +
> > > +	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);
> > > +
> > > +	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);
> > > +}
> > > +
> > >   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 2f590bae53f8..2a2585b37118 100644
> > > --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > @@ -27,8 +27,42 @@ 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;
> > > +	}
> > > +
> > > +	if (bo->resource->mem_type == TTM_PL_VRAM &&
> > > +	    new_mem->mem_type == TTM_PL_SYSTEM) {
> > > +		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);
> > 
> > We're not doing any accelerated move here, so ttm_bo_move_null()
> > should
> > be sufficient also in this case?
> 
> I'll remove that, thanks.
> 
> > 
> > > +
> > > +		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);
> > 
> > Do we hit this move_memcpy()? Since the mock manager doesn't
> > actually
> > reserve any memory to manager, I'd expect this to run into
> > problems?
> 
> We do. The mock manager has use_tt=true, so on move, we'd use 
> ttm_kmap_iter_tt_init() for src and dest and copy the pages. I'm not 
> sure if that's the right approach, but it enables me to test if 
> ttm_operation_ctx's bytes_moved is correctly updated.

Ah, ok. It's probably not a common use-case since with both src and dst
having use_tt, IIRC ttm should keep the pages and their content mostly
intact across a move. So you would memcpy the source on itself?

But it would give some coverage of the copy code though.

/Thomas


> 
> > 
> > 
> > 
> > >   }
> > >   
> > >   struct ttm_device_funcs ttm_dev_funcs = {
> > > 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..eb9dca1de1a2
> > > --- /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, 0);
> > > +	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, 0);
> > > +	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");
> > 
> > When the module is dual-licensed IIRC the correct option to use
> > here is
> > "GPL and additional rights"
> 
> I took that module license from DRM KUnit tests, but I'll update it,
> thanks.
> 
> All the best,
> Karolina


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers
  2024-06-03  7:24       ` Thomas Hellström
@ 2024-06-03  8:28         ` Karolina Stolarek
  2024-06-03  9:30           ` Thomas Hellström
  0 siblings, 1 reply; 20+ messages in thread
From: Karolina Stolarek @ 2024-06-03  8:28 UTC (permalink / raw)
  To: Thomas Hellström, dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram

>>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>> index 2f590bae53f8..2a2585b37118 100644
>>>> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>> @@ -27,8 +27,42 @@ static int mock_move(struct ttm_buffer_object
>>>> *bo,
>>>> bool evict,
>>>>    		     struct ttm_resource *new_mem,
>>>>    		     struct ttm_place *hop)
>>>>    {
(...)
>>>> +
>>>> +		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);
>>>
>>> Do we hit this move_memcpy()? Since the mock manager doesn't
>>> actually
>>> reserve any memory to manager, I'd expect this to run into
>>> problems?
>>
>> We do. The mock manager has use_tt=true, so on move, we'd use
>> ttm_kmap_iter_tt_init() for src and dest and copy the pages. I'm not
>> sure if that's the right approach, but it enables me to test if
>> ttm_operation_ctx's bytes_moved is correctly updated.
> 
> Ah, ok. It's probably not a common use-case since with both src and dst
> having use_tt, IIRC ttm should keep the pages and their content mostly
> intact across a move. So you would memcpy the source on itself?
> 
> But it would give some coverage of the copy code though.

I dug around and it looks like, in the current scenario, 
ttm_bo_move_memcpy() is just ttm_bo_move_sync_cleanup() 
(ttm_resource_free + ttm_bo_assign_mem). That means I should revisit the 
definitions of move and mock manager... I'll try to simplify them.

Do I understand correctly that we'd prefer to have a mock manager with 
user_tt=false?

All the best,
Karolina

> 
> /Thomas

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers
  2024-06-03  8:28         ` Karolina Stolarek
@ 2024-06-03  9:30           ` Thomas Hellström
  2024-06-03  9:46             ` Karolina Stolarek
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Hellström @ 2024-06-03  9:30 UTC (permalink / raw)
  To: Karolina Stolarek, dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram

On Mon, 2024-06-03 at 10:28 +0200, Karolina Stolarek wrote:
> > > > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > index 2f590bae53f8..2a2585b37118 100644
> > > > > --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > @@ -27,8 +27,42 @@ static int mock_move(struct
> > > > > ttm_buffer_object
> > > > > *bo,
> > > > > bool evict,
> > > > >    		     struct ttm_resource *new_mem,
> > > > >    		     struct ttm_place *hop)
> > > > >    {
> (...)
> > > > > +
> > > > > +		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);
> > > > 
> > > > Do we hit this move_memcpy()? Since the mock manager doesn't
> > > > actually
> > > > reserve any memory to manager, I'd expect this to run into
> > > > problems?
> > > 
> > > We do. The mock manager has use_tt=true, so on move, we'd use
> > > ttm_kmap_iter_tt_init() for src and dest and copy the pages. I'm
> > > not
> > > sure if that's the right approach, but it enables me to test if
> > > ttm_operation_ctx's bytes_moved is correctly updated.
> > 
> > Ah, ok. It's probably not a common use-case since with both src and
> > dst
> > having use_tt, IIRC ttm should keep the pages and their content
> > mostly
> > intact across a move. So you would memcpy the source on itself?
> > 
> > But it would give some coverage of the copy code though.
> 
> I dug around and it looks like, in the current scenario, 
> ttm_bo_move_memcpy() is just ttm_bo_move_sync_cleanup() 
> (ttm_resource_free + ttm_bo_assign_mem). That means I should revisit
> the 
> definitions of move and mock manager... I'll try to simplify them.
> 
> Do I understand correctly that we'd prefer to have a mock manager
> with 
> user_tt=false?

Yes, but then you need to allocate a chunk of contigous memory for the
mock manager to manage. And instead of using drm_buddy you'd have to
use drm_mm to manage it, since the ttm_kmap_iter default iterators can
only handle either
a) Contigous memory regions as returned from the drm_mm manager.
b) Fragmented memory regions as returned from the drm_buddy manager,
but in that case, they currently only handle pci io memory.

So I'd suggest to go with the current code and mark as a TODO: to
implement a) above.

/Thomas


> 
> All the best,
> Karolina
> 
> > 
> > /Thomas


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers
  2024-06-03  9:30           ` Thomas Hellström
@ 2024-06-03  9:46             ` Karolina Stolarek
  0 siblings, 0 replies; 20+ messages in thread
From: Karolina Stolarek @ 2024-06-03  9:46 UTC (permalink / raw)
  To: Thomas Hellström, dri-devel
  Cc: Christian König, Matthew Auld, Amaranath Somalapuram

On 3.06.2024 11:30, Thomas Hellström wrote:
> On Mon, 2024-06-03 at 10:28 +0200, Karolina Stolarek wrote:
>>>>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>>>> index 2f590bae53f8..2a2585b37118 100644
>>>>>> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>>>> @@ -27,8 +27,42 @@ static int mock_move(struct
>>>>>> ttm_buffer_object
>>>>>> *bo,
>>>>>> bool evict,
>>>>>>     		     struct ttm_resource *new_mem,
>>>>>>     		     struct ttm_place *hop)
>>>>>>     {
>> (...)
>>>>>> +
>>>>>> +		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);
>>>>>
>>>>> Do we hit this move_memcpy()? Since the mock manager doesn't
>>>>> actually
>>>>> reserve any memory to manager, I'd expect this to run into
>>>>> problems?
>>>>
>>>> We do. The mock manager has use_tt=true, so on move, we'd use
>>>> ttm_kmap_iter_tt_init() for src and dest and copy the pages. I'm
>>>> not
>>>> sure if that's the right approach, but it enables me to test if
>>>> ttm_operation_ctx's bytes_moved is correctly updated.
>>>
>>> Ah, ok. It's probably not a common use-case since with both src and
>>> dst
>>> having use_tt, IIRC ttm should keep the pages and their content
>>> mostly
>>> intact across a move. So you would memcpy the source on itself?
>>>
>>> But it would give some coverage of the copy code though.
>>
>> I dug around and it looks like, in the current scenario,
>> ttm_bo_move_memcpy() is just ttm_bo_move_sync_cleanup()
>> (ttm_resource_free + ttm_bo_assign_mem). That means I should revisit
>> the
>> definitions of move and mock manager... I'll try to simplify them.
>>
>> Do I understand correctly that we'd prefer to have a mock manager
>> with
>> user_tt=false?
> 
> Yes, but then you need to allocate a chunk of contigous memory for the
> mock manager to manage. And instead of using drm_buddy you'd have to
> use drm_mm to manage it, since the ttm_kmap_iter default iterators can
> only handle either
> a) Contigous memory regions as returned from the drm_mm manager.
> b) Fragmented memory regions as returned from the drm_buddy manager,
> but in that case, they currently only handle pci io memory.
> 
> So I'd suggest to go with the current code and mark as a TODO: to
> implement a) above.

I understand, thank you for your explanation, that would require some 
rewrite indeed. I'll follow your suggestion and include it in the TODO file.

All the best,
Karolina

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2024-06-03  9:46 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-15 11:24 [PATCH v12 00/10] Improve test coverage of TTM Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 01/10] drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk Karolina Stolarek
2024-05-24 15:16   ` Thomas Hellström
2024-05-15 11:24 ` [PATCH v12 02/10] drm/ttm/tests: Delete unnecessary config option Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 03/10] drm/ttm/tests: Set DMA mask in KUnit device Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 04/10] drm/ttm/tests: Use an init function from the helpers lib Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 05/10] drm/ttm/tests: Test simple BO creation and validation Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers Karolina Stolarek
2024-05-29 12:58   ` Thomas Hellström
2024-06-03  6:55     ` Karolina Stolarek
2024-06-03  7:24       ` Thomas Hellström
2024-06-03  8:28         ` Karolina Stolarek
2024-06-03  9:30           ` Thomas Hellström
2024-06-03  9:46             ` Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 07/10] drm/ttm/tests: Add test cases dependent on fence signaling Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 08/10] drm/ttm/tests: Add eviction testing Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 09/10] drm/ttm/tests: Add tests for ttm_tt_populate Karolina Stolarek
2024-05-15 11:24 ` [PATCH v12 10/10] drm/ttm/tests: Add TODO file Karolina Stolarek
2024-05-24 15:29   ` Thomas Hellström
2024-05-17  3:37 ` [PATCH v12 00/10] Improve test coverage of TTM Somalapuram, Amaranath

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.