Re: [RFC v2 2/3] drm/ttm/tests: Add tests for ttm_device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29.06.2023 11:14, Christian König wrote:


Am 27.06.23 um 10:32 schrieb Karolina Stolarek:
Test initialization and cleanup of the ttm_device struct, including
some error paths. Verify the creation of page pools if use_dma_alloc
param is true.

Signed-off-by: Karolina Stolarek <karolina.stolarek@xxxxxxxxx>
---
  drivers/gpu/drm/ttm/tests/ttm_device_test.c | 159 ++++++++++++++++++++
  1 file changed, 159 insertions(+)

diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c b/drivers/gpu/drm/ttm/tests/ttm_device_test.c
index 08d7314b1e35..67f7ec347a61 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_device_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c
@@ -8,6 +8,13 @@
  #include "ttm_kunit_helpers.h"
+struct ttm_device_test_case {
+    const char *description;
+    bool use_dma_alloc;
+    bool use_dma32;
+    bool pools_init_expected;
+};
+
  static void ttm_device_init_basic(struct kunit *test)
  {
      struct ttm_test_devices_priv *priv = test->priv;
@@ -37,8 +44,160 @@ static void ttm_device_init_basic(struct kunit *test)
      ttm_device_fini(ttm_dev);
  }
+static void ttm_device_init_multiple(struct kunit *test)
+{
+    struct ttm_test_devices_priv *priv = test->priv;
+    struct ttm_device *ttm_devs;
+    unsigned int i, num_dev = 3;
+    int err;
+
+    ttm_devs = kunit_kcalloc(test, num_dev, sizeof(*ttm_devs), GFP_KERNEL);
+    KUNIT_ASSERT_NOT_NULL(test, ttm_devs);
+
+    for (i = 0; i < num_dev; i++) {
+        err = ttm_kunit_helper_alloc_device(test, &ttm_devs[i],
+                            false, false);
+        KUNIT_ASSERT_EQ(test, err, 0);
+
+        KUNIT_EXPECT_PTR_EQ(test, ttm_devs[i].dev_mapping,
+                    priv->drm->anon_inode->i_mapping);
+        KUNIT_ASSERT_NOT_NULL(test, ttm_devs[i].wq);
+        KUNIT_EXPECT_PTR_EQ(test, ttm_devs[i].funcs, &ttm_dev_funcs);
+        KUNIT_ASSERT_NOT_NULL(test, ttm_devs[i].man_drv[TTM_PL_SYSTEM]);
+    }
+
+    KUNIT_ASSERT_EQ(test, list_count_nodes(&ttm_devs[0].device_list), num_dev);
+
+    for (i = 0; i < num_dev; i++)
+        ttm_device_fini(&ttm_devs[i]);
+}
+
+static void ttm_device_fini_basic(struct kunit *test)
+{
+    struct ttm_device *ttm_dev;
+    struct ttm_resource_manager *man;
+    int err;
+
+    ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+    KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+    err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false);
+    KUNIT_ASSERT_EQ(test, err, 0);
+
+    man = ttm_manager_type(ttm_dev, TTM_PL_SYSTEM);
+    KUNIT_ASSERT_NOT_NULL(test, man);
+
+    ttm_device_fini(ttm_dev);
+
+    KUNIT_ASSERT_FALSE(test, man->use_type);
+    KUNIT_ASSERT_TRUE(test, list_empty(&man->lru[0]));
+    KUNIT_ASSERT_NULL(test, ttm_dev->man_drv[TTM_PL_SYSTEM]);
+}
+
+static void ttm_device_init_no_vma_man(struct kunit *test)
+{
+    struct ttm_test_devices_priv *priv = test->priv;
+    struct drm_device *drm = priv->drm;
+    struct ttm_device *ttm_dev;
+    struct drm_vma_offset_manager *vma_man;
+    int err;
+
+    ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+    KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+    /* Let's pretend there's no VMA manager allocated */
+    vma_man = drm->vma_offset_manager;
+    drm->vma_offset_manager = NULL;
+
+    err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false);
+    KUNIT_EXPECT_EQ(test, err, -EINVAL);
+
+    /* Bring the manager back for a graceful cleanup */
+    drm->vma_offset_manager = vma_man;
+}
+
+static const struct ttm_device_test_case ttm_device_cases[] = {
+    {
+        .description = "No DMA allocations, no DMA32 required",
+        .use_dma_alloc = false,
+        .use_dma32 = false,
+        .pools_init_expected = false,
+    },
+    {
+        .description = "DMA allocations, DMA32 required",
+        .use_dma_alloc = true,
+        .use_dma32 = true,
+        .pools_init_expected = true,
+    },
+    {
+        .description = "No DMA allocations, DMA32 required",
+        .use_dma_alloc = false,
+        .use_dma32 = true,
+        .pools_init_expected = false,
+    },
+    {
+        .description = "DMA allocations, no DMA32 required",
+        .use_dma_alloc = true,
+        .use_dma32 = false,
+        .pools_init_expected = true,
+    },
+};
+
+static void ttm_device_case_desc(const struct ttm_device_test_case *t, char *desc)
+{
+    strscpy(desc, t->description, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(ttm_device, ttm_device_cases, ttm_device_case_desc);
+
+static void ttm_device_init_pools(struct kunit *test)
+{
+    struct ttm_test_devices_priv *priv = test->priv;
+    const struct ttm_device_test_case *params = test->param_value;
+    struct ttm_device *ttm_dev;
+    struct ttm_pool *pool;
+    struct ttm_pool_type pt;
+    int err;
+
+    ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL);
+    KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
+
+    err = ttm_kunit_helper_alloc_device(test, ttm_dev,
+                        params->use_dma_alloc,
+                        params->use_dma32);
+    KUNIT_ASSERT_EQ(test, err, 0);
+
+    pool = &ttm_dev->pool;
+    KUNIT_ASSERT_NOT_NULL(test, pool);
+    KUNIT_EXPECT_PTR_EQ(test, pool->dev, priv->dev);
+    KUNIT_EXPECT_EQ(test, pool->use_dma_alloc, params->use_dma_alloc);
+    KUNIT_EXPECT_EQ(test, pool->use_dma32, params->use_dma32);
+
+    if (params->pools_init_expected) {
+        for (int i = 0; i < TTM_NUM_CACHING_TYPES; ++i) {
+            for (int j = 0; j <= MAX_ORDER; ++j) {
+                pt = pool->caching[i].orders[j];
+                KUNIT_EXPECT_PTR_EQ(test, pt.pool, pool);
+                KUNIT_EXPECT_EQ(test, pt.caching, i);
+                KUNIT_EXPECT_EQ(test, pt.order, j);
+
+                if (params->use_dma_alloc) {
+                    KUNIT_ASSERT_FALSE(test,
+                               list_empty(&pt.pages));
+                }

That belongs more into the pools check I think, but not a blocker to have it here.

My reasoning was that the pool initialization happens in a specific path of ttm_device_init() and it should be tested. I'd like to keep it here, if you don't mind, but I can move it if you wish so.

I'm not familiar with some of the KUNIT stuff, but the TTM side looks good. Feel free to add Acked-by: Christian König <christian.koenig@xxxxxxx>.

Thanks a lot.

All the best,
Karolina


Christian.

+            }
+        }
+    }
+
+    ttm_device_fini(ttm_dev);
+}
+
  static struct kunit_case ttm_device_test_cases[] = {
      KUNIT_CASE(ttm_device_init_basic),
+    KUNIT_CASE(ttm_device_init_multiple),
+    KUNIT_CASE(ttm_device_fini_basic),
+    KUNIT_CASE(ttm_device_init_no_vma_man),
+    KUNIT_CASE_PARAM(ttm_device_init_pools, ttm_device_gen_params),
      {}
  };




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux