* [RFC v2 0/3] Introduce KUnit tests for TTM subsystem
@ 2023-06-27 8:32 Karolina Stolarek
2023-06-27 8:32 ` [RFC v2 1/3] drm/ttm: Introduce KUnit tests Karolina Stolarek
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Karolina Stolarek @ 2023-06-27 8:32 UTC (permalink / raw)
To: dri-devel
Cc: Thomas Hellström, Mauro Carvalho Chehab, Karolina Stolarek,
Andi Shyti, Shuah Khan, Christian König, Nirmoy Das
This series introduces KUnit[1] tests for TTM (Translation Table Manager)
subsystem, a memory manager used by graphics drivers to create and manage
memory buffers across different memory domains, such as system memory
or VRAM.
Unit tests implemented here cover two data structures:
- ttm_device -- referred as a buffer object device, which stores
resource managers and page pools
- ttm_pool -- a struct of pools (ttm_pool_type) of different page
orders and caching attributes, with pages that can be reused on
the next buffer allocation
Use kunit_tool script to manually run the tests:
$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/ttm/tests
The kunit tool might not work with older python versions. To fix that,
apply [2] patch.
To build a kernel with TTM KUnit tests, enable CONFIG_DRM_TTM_KUNIT_TEST
symbol.
As for now, tests are architecture-agnostic (i.e. KUnit runner uses UML
kernel), which means that we have limited coverage in some places. For
example, we can't fully test the initialization of global page pools,
such as global_write_combined. It is to be decided if we want to stick
to UML or use CONFIG_X86 (at least to some extent).
These patches are just a beginning of the work to improve the test
coverage of TTM. Feel free to suggest changes, test cases or priorities.
Many thanks,
Karolina
v2:
- Add missing symbol exports in ttm_kunit_helpers.c
- Update helpers include to fix compilation issues (didn't catch it as
KUnit tests weren't enabled in the kernel I tested, an oversight
on my part)
- Add checks for ttm_pool fields in ttm_pool_alloc_basic(), including the
one for NUMA node id
- Rebase the changes on the top of drm-tip
--------------------------------
[1] - https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html
[2] - https://lore.kernel.org/lkml/20230610175618.82271-1-sj@kernel.org/T/
Karolina Stolarek (3):
drm/ttm: Introduce KUnit tests
drm/ttm/tests: Add tests for ttm_device
drm/ttm/tests: Add tests for ttm_pool
drivers/gpu/drm/Kconfig | 15 +
drivers/gpu/drm/ttm/Makefile | 1 +
drivers/gpu/drm/ttm/tests/.kunitconfig | 4 +
drivers/gpu/drm/ttm/tests/Makefile | 6 +
drivers/gpu/drm/ttm/tests/ttm_device_test.c | 213 +++++++++
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 88 ++++
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 34 ++
drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 406 ++++++++++++++++++
8 files changed, 767 insertions(+)
create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig
create mode 100644 drivers/gpu/drm/ttm/tests/Makefile
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
create mode 100644 drivers/gpu/drm/ttm/tests/ttm_pool_test.c
--
2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC v2 1/3] drm/ttm: Introduce KUnit tests 2023-06-27 8:32 [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Karolina Stolarek @ 2023-06-27 8:32 ` Karolina Stolarek 2023-06-29 7:50 ` Christian König 2023-06-27 8:32 ` [RFC v2 2/3] drm/ttm/tests: Add tests for ttm_device Karolina Stolarek ` (2 subsequent siblings) 3 siblings, 1 reply; 15+ messages in thread From: Karolina Stolarek @ 2023-06-27 8:32 UTC (permalink / raw) To: dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Karolina Stolarek, Andi Shyti, Shuah Khan, Christian König, Nirmoy Das Add the initial version of unit tests for ttm_device struct, together with helper functions. Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> --- drivers/gpu/drm/Kconfig | 15 +++++ drivers/gpu/drm/ttm/Makefile | 1 + drivers/gpu/drm/ttm/tests/.kunitconfig | 4 ++ drivers/gpu/drm/ttm/tests/Makefile | 5 ++ drivers/gpu/drm/ttm/tests/ttm_device_test.c | 54 +++++++++++++++++ drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 59 +++++++++++++++++++ drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 29 +++++++++ 7 files changed, 167 insertions(+) create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig create mode 100644 drivers/gpu/drm/ttm/tests/Makefile create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index afb3b2f5f425..53024e44a2d5 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -194,6 +194,21 @@ config DRM_TTM GPU memory types. Will be enabled automatically if a device driver uses it. +config DRM_TTM_KUNIT_TEST + tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS + default n + depends on DRM && KUNIT + select DRM_TTM + select DRM_EXPORT_FOR_TESTS if m + select DRM_KUNIT_TEST_HELPERS + default KUNIT_ALL_TESTS + help + Enables unit tests for TTM, a GPU memory manager subsystem used + to manage memory buffers. This option is mostly useful for kernel + developers. + + If in doubt, say "N". + config DRM_BUDDY tristate depends on DRM diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile index f906b22959cf..dad298127226 100644 --- a/drivers/gpu/drm/ttm/Makefile +++ b/drivers/gpu/drm/ttm/Makefile @@ -8,3 +8,4 @@ ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o ttm_bo_vm.o ttm_module.o \ ttm-$(CONFIG_AGP) += ttm_agp_backend.o obj-$(CONFIG_DRM_TTM) += ttm.o +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += tests/ diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig b/drivers/gpu/drm/ttm/tests/.kunitconfig new file mode 100644 index 000000000000..75fdce0cd98e --- /dev/null +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig @@ -0,0 +1,4 @@ +CONFIG_KUNIT=y +CONFIG_DRM=y +CONFIG_DRM_KUNIT_TEST_HELPERS=y +CONFIG_DRM_TTM_KUNIT_TEST=y diff --git a/drivers/gpu/drm/ttm/tests/Makefile b/drivers/gpu/drm/ttm/tests/Makefile new file mode 100644 index 000000000000..7917805f37af --- /dev/null +++ b/drivers/gpu/drm/ttm/tests/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 AND MIT + +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ + ttm_device_test.o \ + ttm_kunit_helpers.o diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c b/drivers/gpu/drm/ttm/tests/ttm_device_test.c new file mode 100644 index 000000000000..08d7314b1e35 --- /dev/null +++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: GPL-2.0 AND MIT +/* + * Copyright © 2023 Intel Corporation + */ +#include <drm/ttm/ttm_resource.h> +#include <drm/ttm/ttm_device.h> +#include <drm/ttm/ttm_placement.h> + +#include "ttm_kunit_helpers.h" + +static void ttm_device_init_basic(struct kunit *test) +{ + struct ttm_test_devices_priv *priv = test->priv; + struct ttm_device *ttm_dev; + struct ttm_resource_manager *ttm_sys_man; + int err; + + ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, ttm_dev); + + err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false); + KUNIT_ASSERT_EQ(test, err, 0); + + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->funcs, &ttm_dev_funcs); + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->wq); + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->man_drv[TTM_PL_SYSTEM]); + + ttm_sys_man = &ttm_dev->sysman; + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man); + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_tt); + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_type); + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man->func); + + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->dev_mapping, + priv->drm->anon_inode->i_mapping); + + ttm_device_fini(ttm_dev); +} + +static struct kunit_case ttm_device_test_cases[] = { + KUNIT_CASE(ttm_device_init_basic), + {} +}; + +static struct kunit_suite ttm_device_test_suite = { + .name = "ttm_device", + .init = ttm_test_devices_init, + .exit = ttm_test_devices_fini, + .test_cases = ttm_device_test_cases, +}; + +kunit_test_suites(&ttm_device_test_suite); + +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c new file mode 100644 index 000000000000..d03db0405484 --- /dev/null +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0 AND MIT +/* + * Copyright © 2023 Intel Corporation + */ +#include "ttm_kunit_helpers.h" + +struct ttm_device_funcs ttm_dev_funcs = { +}; +EXPORT_SYMBOL_GPL(ttm_dev_funcs); + +int ttm_kunit_helper_alloc_device(struct kunit *test, + struct ttm_device *ttm, + bool use_dma_alloc, + bool use_dma32) +{ + struct ttm_test_devices_priv *priv = test->priv; + struct drm_device *drm = priv->drm; + int err; + + err = ttm_device_init(ttm, &ttm_dev_funcs, drm->dev, + drm->anon_inode->i_mapping, + drm->vma_offset_manager, + use_dma_alloc, use_dma32); + + return err; +} +EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); + +int ttm_test_devices_init(struct kunit *test) +{ + struct ttm_test_devices_priv *priv; + + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, priv); + + test->priv = priv; + + priv->dev = drm_kunit_helper_alloc_device(test); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev); + + priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev, + sizeof(*priv->drm), 0, + DRIVER_GEM); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm); + + return 0; +} +EXPORT_SYMBOL_GPL(ttm_test_devices_init); + +void ttm_test_devices_fini(struct kunit *test) +{ + struct ttm_test_devices_priv *priv = test->priv; + + drm_kunit_helper_free_device(test, priv->dev); + drm_dev_put(priv->drm); +} +EXPORT_SYMBOL_GPL(ttm_test_devices_fini); + +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h new file mode 100644 index 000000000000..69fb03b9c4d2 --- /dev/null +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: GPL-2.0 AND MIT */ +/* + * Copyright © 2023 Intel Corporation + */ +#ifndef TTM_KUNIT_HELPERS_H +#define TTM_KUNIT_HELPERS_H + +#include <drm/drm_drv.h> +#include <drm/ttm/ttm_device.h> + +#include <drm/drm_kunit_helpers.h> +#include <kunit/test.h> + +extern struct ttm_device_funcs ttm_dev_funcs; + +struct ttm_test_devices_priv { + struct drm_device *drm; + struct device *dev; +}; + +int ttm_kunit_helper_alloc_device(struct kunit *test, + struct ttm_device *ttm, + bool use_dma_alloc, + bool use_dma32); + +int ttm_test_devices_init(struct kunit *test); +void ttm_test_devices_fini(struct kunit *test); + +#endif // TTM_KUNIT_HELPERS_H -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC v2 1/3] drm/ttm: Introduce KUnit tests 2023-06-27 8:32 ` [RFC v2 1/3] drm/ttm: Introduce KUnit tests Karolina Stolarek @ 2023-06-29 7:50 ` Christian König 2023-06-29 10:05 ` Karolina Stolarek 0 siblings, 1 reply; 15+ messages in thread From: Christian König @ 2023-06-29 7:50 UTC (permalink / raw) To: Karolina Stolarek, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das Am 27.06.23 um 10:32 schrieb Karolina Stolarek: > Add the initial version of unit tests for ttm_device struct, together > with helper functions. > > Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> > --- > drivers/gpu/drm/Kconfig | 15 +++++ > drivers/gpu/drm/ttm/Makefile | 1 + > drivers/gpu/drm/ttm/tests/.kunitconfig | 4 ++ > drivers/gpu/drm/ttm/tests/Makefile | 5 ++ > drivers/gpu/drm/ttm/tests/ttm_device_test.c | 54 +++++++++++++++++ > drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 59 +++++++++++++++++++ > drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 29 +++++++++ > 7 files changed, 167 insertions(+) > create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig > create mode 100644 drivers/gpu/drm/ttm/tests/Makefile > create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c > create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c > create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index afb3b2f5f425..53024e44a2d5 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -194,6 +194,21 @@ config DRM_TTM > GPU memory types. Will be enabled automatically if a device driver > uses it. > > +config DRM_TTM_KUNIT_TEST > + tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS > + default n > + depends on DRM && KUNIT > + select DRM_TTM > + select DRM_EXPORT_FOR_TESTS if m > + select DRM_KUNIT_TEST_HELPERS > + default KUNIT_ALL_TESTS > + help > + Enables unit tests for TTM, a GPU memory manager subsystem used > + to manage memory buffers. This option is mostly useful for kernel > + developers. > + > + If in doubt, say "N". > + > config DRM_BUDDY > tristate > depends on DRM > diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile > index f906b22959cf..dad298127226 100644 > --- a/drivers/gpu/drm/ttm/Makefile > +++ b/drivers/gpu/drm/ttm/Makefile > @@ -8,3 +8,4 @@ ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o ttm_bo_vm.o ttm_module.o \ > ttm-$(CONFIG_AGP) += ttm_agp_backend.o > > obj-$(CONFIG_DRM_TTM) += ttm.o > +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += tests/ > diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig b/drivers/gpu/drm/ttm/tests/.kunitconfig > new file mode 100644 > index 000000000000..75fdce0cd98e > --- /dev/null > +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig > @@ -0,0 +1,4 @@ > +CONFIG_KUNIT=y > +CONFIG_DRM=y > +CONFIG_DRM_KUNIT_TEST_HELPERS=y > +CONFIG_DRM_TTM_KUNIT_TEST=y > diff --git a/drivers/gpu/drm/ttm/tests/Makefile b/drivers/gpu/drm/ttm/tests/Makefile > new file mode 100644 > index 000000000000..7917805f37af > --- /dev/null > +++ b/drivers/gpu/drm/ttm/tests/Makefile > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0 AND MIT > + > +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ > + ttm_device_test.o \ > + ttm_kunit_helpers.o > diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c b/drivers/gpu/drm/ttm/tests/ttm_device_test.c > new file mode 100644 > index 000000000000..08d7314b1e35 > --- /dev/null > +++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c > @@ -0,0 +1,54 @@ > +// SPDX-License-Identifier: GPL-2.0 AND MIT > +/* > + * Copyright © 2023 Intel Corporation > + */ > +#include <drm/ttm/ttm_resource.h> > +#include <drm/ttm/ttm_device.h> > +#include <drm/ttm/ttm_placement.h> > + > +#include "ttm_kunit_helpers.h" > + > +static void ttm_device_init_basic(struct kunit *test) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + struct ttm_device *ttm_dev; > + struct ttm_resource_manager *ttm_sys_man; > + int err; > + > + ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, ttm_dev); > + > + err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false); > + KUNIT_ASSERT_EQ(test, err, 0); > + > + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->funcs, &ttm_dev_funcs); > + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->wq); > + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->man_drv[TTM_PL_SYSTEM]); > + > + ttm_sys_man = &ttm_dev->sysman; > + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man); > + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_tt); > + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_type); > + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man->func); > + > + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->dev_mapping, > + priv->drm->anon_inode->i_mapping); > + > + ttm_device_fini(ttm_dev); > +} > + > +static struct kunit_case ttm_device_test_cases[] = { > + KUNIT_CASE(ttm_device_init_basic), > + {} > +}; > + > +static struct kunit_suite ttm_device_test_suite = { > + .name = "ttm_device", > + .init = ttm_test_devices_init, > + .exit = ttm_test_devices_fini, > + .test_cases = ttm_device_test_cases, > +}; > + > +kunit_test_suites(&ttm_device_test_suite); > + > +MODULE_LICENSE("GPL"); > diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c > new file mode 100644 > index 000000000000..d03db0405484 > --- /dev/null > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c > @@ -0,0 +1,59 @@ > +// SPDX-License-Identifier: GPL-2.0 AND MIT > +/* > + * Copyright © 2023 Intel Corporation > + */ > +#include "ttm_kunit_helpers.h" > + > +struct ttm_device_funcs ttm_dev_funcs = { > +}; > +EXPORT_SYMBOL_GPL(ttm_dev_funcs); > + > +int ttm_kunit_helper_alloc_device(struct kunit *test, Since this function is only initializing the ttm device I think we should name it ttm_kunit_helper_init_device(). On the other hand I don't see a good reason why it can't also allocate the device. Apart from that looks like a good start, Christian. > + struct ttm_device *ttm, > + bool use_dma_alloc, > + bool use_dma32) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + struct drm_device *drm = priv->drm; > + int err; > + > + err = ttm_device_init(ttm, &ttm_dev_funcs, drm->dev, > + drm->anon_inode->i_mapping, > + drm->vma_offset_manager, > + use_dma_alloc, use_dma32); > + > + return err; > +} > +EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); > + > +int ttm_test_devices_init(struct kunit *test) > +{ > + struct ttm_test_devices_priv *priv; > + > + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, priv); > + > + test->priv = priv; > + > + priv->dev = drm_kunit_helper_alloc_device(test); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev); > + > + priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev, > + sizeof(*priv->drm), 0, > + DRIVER_GEM); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(ttm_test_devices_init); > + > +void ttm_test_devices_fini(struct kunit *test) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + > + drm_kunit_helper_free_device(test, priv->dev); > + drm_dev_put(priv->drm); > +} > +EXPORT_SYMBOL_GPL(ttm_test_devices_fini); > + > +MODULE_LICENSE("GPL"); > diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h > new file mode 100644 > index 000000000000..69fb03b9c4d2 > --- /dev/null > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h > @@ -0,0 +1,29 @@ > +/* SPDX-License-Identifier: GPL-2.0 AND MIT */ > +/* > + * Copyright © 2023 Intel Corporation > + */ > +#ifndef TTM_KUNIT_HELPERS_H > +#define TTM_KUNIT_HELPERS_H > + > +#include <drm/drm_drv.h> > +#include <drm/ttm/ttm_device.h> > + > +#include <drm/drm_kunit_helpers.h> > +#include <kunit/test.h> > + > +extern struct ttm_device_funcs ttm_dev_funcs; > + > +struct ttm_test_devices_priv { > + struct drm_device *drm; > + struct device *dev; > +}; > + > +int ttm_kunit_helper_alloc_device(struct kunit *test, > + struct ttm_device *ttm, > + bool use_dma_alloc, > + bool use_dma32); > + > +int ttm_test_devices_init(struct kunit *test); > +void ttm_test_devices_fini(struct kunit *test); > + > +#endif // TTM_KUNIT_HELPERS_H ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC v2 1/3] drm/ttm: Introduce KUnit tests 2023-06-29 7:50 ` Christian König @ 2023-06-29 10:05 ` Karolina Stolarek 2023-06-30 11:09 ` Karolina Stolarek 0 siblings, 1 reply; 15+ messages in thread From: Karolina Stolarek @ 2023-06-29 10:05 UTC (permalink / raw) To: Christian König, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das Hi Christian, Thanks a lot for taking a look at my patches. On 29.06.2023 09:50, Christian König wrote: > Am 27.06.23 um 10:32 schrieb Karolina Stolarek: >> Add the initial version of unit tests for ttm_device struct, together >> with helper functions. >> >> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> >> --- >> drivers/gpu/drm/Kconfig | 15 +++++ >> drivers/gpu/drm/ttm/Makefile | 1 + >> drivers/gpu/drm/ttm/tests/.kunitconfig | 4 ++ >> drivers/gpu/drm/ttm/tests/Makefile | 5 ++ >> drivers/gpu/drm/ttm/tests/ttm_device_test.c | 54 +++++++++++++++++ >> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 59 +++++++++++++++++++ >> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 29 +++++++++ >> 7 files changed, 167 insertions(+) >> create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig >> create mode 100644 drivers/gpu/drm/ttm/tests/Makefile >> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c >> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index afb3b2f5f425..53024e44a2d5 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -194,6 +194,21 @@ config DRM_TTM >> GPU memory types. Will be enabled automatically if a device >> driver >> uses it. >> +config DRM_TTM_KUNIT_TEST >> + tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS >> + default n >> + depends on DRM && KUNIT >> + select DRM_TTM >> + select DRM_EXPORT_FOR_TESTS if m >> + select DRM_KUNIT_TEST_HELPERS >> + default KUNIT_ALL_TESTS >> + help >> + Enables unit tests for TTM, a GPU memory manager subsystem >> used >> + to manage memory buffers. This option is mostly useful for >> kernel >> + developers. >> + >> + If in doubt, say "N". >> + >> config DRM_BUDDY >> tristate >> depends on DRM >> diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile >> index f906b22959cf..dad298127226 100644 >> --- a/drivers/gpu/drm/ttm/Makefile >> +++ b/drivers/gpu/drm/ttm/Makefile >> @@ -8,3 +8,4 @@ ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o ttm_bo_vm.o >> ttm_module.o \ >> ttm-$(CONFIG_AGP) += ttm_agp_backend.o >> obj-$(CONFIG_DRM_TTM) += ttm.o >> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += tests/ >> diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig >> b/drivers/gpu/drm/ttm/tests/.kunitconfig >> new file mode 100644 >> index 000000000000..75fdce0cd98e >> --- /dev/null >> +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig >> @@ -0,0 +1,4 @@ >> +CONFIG_KUNIT=y >> +CONFIG_DRM=y >> +CONFIG_DRM_KUNIT_TEST_HELPERS=y >> +CONFIG_DRM_TTM_KUNIT_TEST=y >> diff --git a/drivers/gpu/drm/ttm/tests/Makefile >> b/drivers/gpu/drm/ttm/tests/Makefile >> new file mode 100644 >> index 000000000000..7917805f37af >> --- /dev/null >> +++ b/drivers/gpu/drm/ttm/tests/Makefile >> @@ -0,0 +1,5 @@ >> +# SPDX-License-Identifier: GPL-2.0 AND MIT >> + >> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ >> + ttm_device_test.o \ >> + ttm_kunit_helpers.o >> diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c >> b/drivers/gpu/drm/ttm/tests/ttm_device_test.c >> new file mode 100644 >> index 000000000000..08d7314b1e35 >> --- /dev/null >> +++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c >> @@ -0,0 +1,54 @@ >> +// SPDX-License-Identifier: GPL-2.0 AND MIT >> +/* >> + * Copyright © 2023 Intel Corporation >> + */ >> +#include <drm/ttm/ttm_resource.h> >> +#include <drm/ttm/ttm_device.h> >> +#include <drm/ttm/ttm_placement.h> >> + >> +#include "ttm_kunit_helpers.h" >> + >> +static void ttm_device_init_basic(struct kunit *test) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + struct ttm_device *ttm_dev; >> + struct ttm_resource_manager *ttm_sys_man; >> + int err; >> + >> + ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev); >> + >> + err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + >> + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->funcs, &ttm_dev_funcs); >> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->wq); >> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->man_drv[TTM_PL_SYSTEM]); >> + >> + ttm_sys_man = &ttm_dev->sysman; >> + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man); >> + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_tt); >> + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_type); >> + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man->func); >> + >> + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->dev_mapping, >> + priv->drm->anon_inode->i_mapping); >> + >> + ttm_device_fini(ttm_dev); >> +} >> + >> +static struct kunit_case ttm_device_test_cases[] = { >> + KUNIT_CASE(ttm_device_init_basic), >> + {} >> +}; >> + >> +static struct kunit_suite ttm_device_test_suite = { >> + .name = "ttm_device", >> + .init = ttm_test_devices_init, >> + .exit = ttm_test_devices_fini, >> + .test_cases = ttm_device_test_cases, >> +}; >> + >> +kunit_test_suites(&ttm_device_test_suite); >> + >> +MODULE_LICENSE("GPL"); >> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> new file mode 100644 >> index 000000000000..d03db0405484 >> --- /dev/null >> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> @@ -0,0 +1,59 @@ >> +// SPDX-License-Identifier: GPL-2.0 AND MIT >> +/* >> + * Copyright © 2023 Intel Corporation >> + */ >> +#include "ttm_kunit_helpers.h" >> + >> +struct ttm_device_funcs ttm_dev_funcs = { >> +}; >> +EXPORT_SYMBOL_GPL(ttm_dev_funcs); >> + >> +int ttm_kunit_helper_alloc_device(struct kunit *test, > > Since this function is only initializing the ttm device I think we > should name it ttm_kunit_helper_init_device(). > > On the other hand I don't see a good reason why it can't also allocate > the device. I believe that's a leftover from times when I thought I'll reuse DRM device between the tests. No problem, I can put them into one function. All the best, Karolina > > Apart from that looks like a good start, > Christian. > >> + struct ttm_device *ttm, >> + bool use_dma_alloc, >> + bool use_dma32) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + struct drm_device *drm = priv->drm; >> + int err; >> + >> + err = ttm_device_init(ttm, &ttm_dev_funcs, drm->dev, >> + drm->anon_inode->i_mapping, >> + drm->vma_offset_manager, >> + use_dma_alloc, use_dma32); >> + >> + return err; >> +} >> +EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); >> + >> +int ttm_test_devices_init(struct kunit *test) >> +{ >> + struct ttm_test_devices_priv *priv; >> + >> + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, priv); >> + >> + test->priv = priv; >> + >> + priv->dev = drm_kunit_helper_alloc_device(test); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev); >> + >> + priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev, >> + sizeof(*priv->drm), 0, >> + DRIVER_GEM); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(ttm_test_devices_init); >> + >> +void ttm_test_devices_fini(struct kunit *test) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + >> + drm_kunit_helper_free_device(test, priv->dev); >> + drm_dev_put(priv->drm); >> +} >> +EXPORT_SYMBOL_GPL(ttm_test_devices_fini); >> + >> +MODULE_LICENSE("GPL"); >> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> new file mode 100644 >> index 000000000000..69fb03b9c4d2 >> --- /dev/null >> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> @@ -0,0 +1,29 @@ >> +/* SPDX-License-Identifier: GPL-2.0 AND MIT */ >> +/* >> + * Copyright © 2023 Intel Corporation >> + */ >> +#ifndef TTM_KUNIT_HELPERS_H >> +#define TTM_KUNIT_HELPERS_H >> + >> +#include <drm/drm_drv.h> >> +#include <drm/ttm/ttm_device.h> >> + >> +#include <drm/drm_kunit_helpers.h> >> +#include <kunit/test.h> >> + >> +extern struct ttm_device_funcs ttm_dev_funcs; >> + >> +struct ttm_test_devices_priv { >> + struct drm_device *drm; >> + struct device *dev; >> +}; >> + >> +int ttm_kunit_helper_alloc_device(struct kunit *test, >> + struct ttm_device *ttm, >> + bool use_dma_alloc, >> + bool use_dma32); >> + >> +int ttm_test_devices_init(struct kunit *test); >> +void ttm_test_devices_fini(struct kunit *test); >> + >> +#endif // TTM_KUNIT_HELPERS_H > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC v2 1/3] drm/ttm: Introduce KUnit tests 2023-06-29 10:05 ` Karolina Stolarek @ 2023-06-30 11:09 ` Karolina Stolarek 2023-06-30 11:18 ` Christian König 0 siblings, 1 reply; 15+ messages in thread From: Karolina Stolarek @ 2023-06-30 11:09 UTC (permalink / raw) To: Christian König, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das Hi Christian, I'm taking a second look at this, and I wonder what would be the benefit of combining the initialization of device and ttm_device. (drm_)device can be initialized indepedently from the test params, so we can utilize .init and .exit callbacks offered by KUnit[1] to prepare and release the device indepedently of our setup. If we were to change it so ttm_device is also initialized there, we'd have to manually call init/fini in every single test. What's more, ttm_pool tests don't depend on ttm_device, and I can imagine that some structs can be tested in a similar way. Would it be fine with you to rename ttm_kunit_helper_alloc_device(), but leave its definition as it is? All the best, Karolina ------------------------------------------------------------------- [1] - https://kunit.dev/third_party/kernel/docs/api/test.html#c.kunit_suite On 29.06.2023 12:05, Karolina Stolarek wrote: > Hi Christian, > > Thanks a lot for taking a look at my patches. > > On 29.06.2023 09:50, Christian König wrote: >> Am 27.06.23 um 10:32 schrieb Karolina Stolarek: >>> Add the initial version of unit tests for ttm_device struct, together >>> with helper functions. >>> >>> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> >>> --- >>> drivers/gpu/drm/Kconfig | 15 +++++ >>> drivers/gpu/drm/ttm/Makefile | 1 + >>> drivers/gpu/drm/ttm/tests/.kunitconfig | 4 ++ >>> drivers/gpu/drm/ttm/tests/Makefile | 5 ++ >>> drivers/gpu/drm/ttm/tests/ttm_device_test.c | 54 +++++++++++++++++ >>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 59 +++++++++++++++++++ >>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 29 +++++++++ >>> 7 files changed, 167 insertions(+) >>> create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig >>> create mode 100644 drivers/gpu/drm/ttm/tests/Makefile >>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c >>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>> >>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>> index afb3b2f5f425..53024e44a2d5 100644 >>> --- a/drivers/gpu/drm/Kconfig >>> +++ b/drivers/gpu/drm/Kconfig >>> @@ -194,6 +194,21 @@ config DRM_TTM >>> GPU memory types. Will be enabled automatically if a device >>> driver >>> uses it. >>> +config DRM_TTM_KUNIT_TEST >>> + tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS >>> + default n >>> + depends on DRM && KUNIT >>> + select DRM_TTM >>> + select DRM_EXPORT_FOR_TESTS if m >>> + select DRM_KUNIT_TEST_HELPERS >>> + default KUNIT_ALL_TESTS >>> + help >>> + Enables unit tests for TTM, a GPU memory manager subsystem >>> used >>> + to manage memory buffers. This option is mostly useful for >>> kernel >>> + developers. >>> + >>> + If in doubt, say "N". >>> + >>> config DRM_BUDDY >>> tristate >>> depends on DRM >>> diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile >>> index f906b22959cf..dad298127226 100644 >>> --- a/drivers/gpu/drm/ttm/Makefile >>> +++ b/drivers/gpu/drm/ttm/Makefile >>> @@ -8,3 +8,4 @@ ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o ttm_bo_vm.o >>> ttm_module.o \ >>> ttm-$(CONFIG_AGP) += ttm_agp_backend.o >>> obj-$(CONFIG_DRM_TTM) += ttm.o >>> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += tests/ >>> diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig >>> b/drivers/gpu/drm/ttm/tests/.kunitconfig >>> new file mode 100644 >>> index 000000000000..75fdce0cd98e >>> --- /dev/null >>> +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig >>> @@ -0,0 +1,4 @@ >>> +CONFIG_KUNIT=y >>> +CONFIG_DRM=y >>> +CONFIG_DRM_KUNIT_TEST_HELPERS=y >>> +CONFIG_DRM_TTM_KUNIT_TEST=y >>> diff --git a/drivers/gpu/drm/ttm/tests/Makefile >>> b/drivers/gpu/drm/ttm/tests/Makefile >>> new file mode 100644 >>> index 000000000000..7917805f37af >>> --- /dev/null >>> +++ b/drivers/gpu/drm/ttm/tests/Makefile >>> @@ -0,0 +1,5 @@ >>> +# SPDX-License-Identifier: GPL-2.0 AND MIT >>> + >>> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ >>> + ttm_device_test.o \ >>> + ttm_kunit_helpers.o >>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>> b/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>> new file mode 100644 >>> index 000000000000..08d7314b1e35 >>> --- /dev/null >>> +++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>> @@ -0,0 +1,54 @@ >>> +// SPDX-License-Identifier: GPL-2.0 AND MIT >>> +/* >>> + * Copyright © 2023 Intel Corporation >>> + */ >>> +#include <drm/ttm/ttm_resource.h> >>> +#include <drm/ttm/ttm_device.h> >>> +#include <drm/ttm/ttm_placement.h> >>> + >>> +#include "ttm_kunit_helpers.h" >>> + >>> +static void ttm_device_init_basic(struct kunit *test) >>> +{ >>> + struct ttm_test_devices_priv *priv = test->priv; >>> + struct ttm_device *ttm_dev; >>> + struct ttm_resource_manager *ttm_sys_man; >>> + int err; >>> + >>> + ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); >>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev); >>> + >>> + err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false); >>> + KUNIT_ASSERT_EQ(test, err, 0); >>> + >>> + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->funcs, &ttm_dev_funcs); >>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->wq); >>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->man_drv[TTM_PL_SYSTEM]); >>> + >>> + ttm_sys_man = &ttm_dev->sysman; >>> + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man); >>> + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_tt); >>> + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_type); >>> + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man->func); >>> + >>> + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->dev_mapping, >>> + priv->drm->anon_inode->i_mapping); >>> + >>> + ttm_device_fini(ttm_dev); >>> +} >>> + >>> +static struct kunit_case ttm_device_test_cases[] = { >>> + KUNIT_CASE(ttm_device_init_basic), >>> + {} >>> +}; >>> + >>> +static struct kunit_suite ttm_device_test_suite = { >>> + .name = "ttm_device", >>> + .init = ttm_test_devices_init, >>> + .exit = ttm_test_devices_fini, >>> + .test_cases = ttm_device_test_cases, >>> +}; >>> + >>> +kunit_test_suites(&ttm_device_test_suite); >>> + >>> +MODULE_LICENSE("GPL"); >>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>> new file mode 100644 >>> index 000000000000..d03db0405484 >>> --- /dev/null >>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>> @@ -0,0 +1,59 @@ >>> +// SPDX-License-Identifier: GPL-2.0 AND MIT >>> +/* >>> + * Copyright © 2023 Intel Corporation >>> + */ >>> +#include "ttm_kunit_helpers.h" >>> + >>> +struct ttm_device_funcs ttm_dev_funcs = { >>> +}; >>> +EXPORT_SYMBOL_GPL(ttm_dev_funcs); >>> + >>> +int ttm_kunit_helper_alloc_device(struct kunit *test, >> >> Since this function is only initializing the ttm device I think we >> should name it ttm_kunit_helper_init_device(). >> >> On the other hand I don't see a good reason why it can't also allocate >> the device. > > I believe that's a leftover from times when I thought I'll reuse DRM > device between the tests. No problem, I can put them into one function. > > All the best, > Karolina > >> >> Apart from that looks like a good start, >> Christian. >> >>> + struct ttm_device *ttm, >>> + bool use_dma_alloc, >>> + bool use_dma32) >>> +{ >>> + struct ttm_test_devices_priv *priv = test->priv; >>> + struct drm_device *drm = priv->drm; >>> + int err; >>> + >>> + err = ttm_device_init(ttm, &ttm_dev_funcs, drm->dev, >>> + drm->anon_inode->i_mapping, >>> + drm->vma_offset_manager, >>> + use_dma_alloc, use_dma32); >>> + >>> + return err; >>> +} >>> +EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); >>> + >>> +int ttm_test_devices_init(struct kunit *test) >>> +{ >>> + struct ttm_test_devices_priv *priv; >>> + >>> + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); >>> + KUNIT_ASSERT_NOT_NULL(test, priv); >>> + >>> + test->priv = priv; >>> + >>> + priv->dev = drm_kunit_helper_alloc_device(test); >>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev); >>> + >>> + priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev, >>> + sizeof(*priv->drm), 0, >>> + DRIVER_GEM); >>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm); >>> + >>> + return 0; >>> +} >>> +EXPORT_SYMBOL_GPL(ttm_test_devices_init); >>> + >>> +void ttm_test_devices_fini(struct kunit *test) >>> +{ >>> + struct ttm_test_devices_priv *priv = test->priv; >>> + >>> + drm_kunit_helper_free_device(test, priv->dev); >>> + drm_dev_put(priv->drm); >>> +} >>> +EXPORT_SYMBOL_GPL(ttm_test_devices_fini); >>> + >>> +MODULE_LICENSE("GPL"); >>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>> new file mode 100644 >>> index 000000000000..69fb03b9c4d2 >>> --- /dev/null >>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>> @@ -0,0 +1,29 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 AND MIT */ >>> +/* >>> + * Copyright © 2023 Intel Corporation >>> + */ >>> +#ifndef TTM_KUNIT_HELPERS_H >>> +#define TTM_KUNIT_HELPERS_H >>> + >>> +#include <drm/drm_drv.h> >>> +#include <drm/ttm/ttm_device.h> >>> + >>> +#include <drm/drm_kunit_helpers.h> >>> +#include <kunit/test.h> >>> + >>> +extern struct ttm_device_funcs ttm_dev_funcs; >>> + >>> +struct ttm_test_devices_priv { >>> + struct drm_device *drm; >>> + struct device *dev; >>> +}; >>> + >>> +int ttm_kunit_helper_alloc_device(struct kunit *test, >>> + struct ttm_device *ttm, >>> + bool use_dma_alloc, >>> + bool use_dma32); >>> + >>> +int ttm_test_devices_init(struct kunit *test); >>> +void ttm_test_devices_fini(struct kunit *test); >>> + >>> +#endif // TTM_KUNIT_HELPERS_H >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC v2 1/3] drm/ttm: Introduce KUnit tests 2023-06-30 11:09 ` Karolina Stolarek @ 2023-06-30 11:18 ` Christian König 2023-06-30 12:35 ` Karolina Stolarek 0 siblings, 1 reply; 15+ messages in thread From: Christian König @ 2023-06-30 11:18 UTC (permalink / raw) To: Karolina Stolarek, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das Am 30.06.23 um 13:09 schrieb Karolina Stolarek: > Hi Christian, > > I'm taking a second look at this, and I wonder what would be the > benefit of combining the initialization of device and ttm_device. > (drm_)device can be initialized indepedently from the test params, so > we can utilize .init and .exit callbacks offered by KUnit[1] to > prepare and release the device indepedently of our setup. If we were > to change it so ttm_device is also initialized there, we'd have to > manually call init/fini in every single test. What's more, ttm_pool > tests don't depend on ttm_device, and I can imagine that some structs > can be tested in a similar way. > > Would it be fine with you to rename ttm_kunit_helper_alloc_device(), > but leave its definition as it is? Yeah, sure. It's perfectly up to you how to structurize that, I wasn't even aware that there are .init and .exit callbacks. I just found that the function name didn't match what the function was doing. Regards, Christian. > > All the best, > Karolina > ------------------------------------------------------------------- > [1] - > https://kunit.dev/third_party/kernel/docs/api/test.html#c.kunit_suite > > On 29.06.2023 12:05, Karolina Stolarek wrote: >> Hi Christian, >> >> Thanks a lot for taking a look at my patches. >> >> On 29.06.2023 09:50, Christian König wrote: >>> Am 27.06.23 um 10:32 schrieb Karolina Stolarek: >>>> Add the initial version of unit tests for ttm_device struct, together >>>> with helper functions. >>>> >>>> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> >>>> --- >>>> drivers/gpu/drm/Kconfig | 15 +++++ >>>> drivers/gpu/drm/ttm/Makefile | 1 + >>>> drivers/gpu/drm/ttm/tests/.kunitconfig | 4 ++ >>>> drivers/gpu/drm/ttm/tests/Makefile | 5 ++ >>>> drivers/gpu/drm/ttm/tests/ttm_device_test.c | 54 +++++++++++++++++ >>>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 59 >>>> +++++++++++++++++++ >>>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 29 +++++++++ >>>> 7 files changed, 167 insertions(+) >>>> create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig >>>> create mode 100644 drivers/gpu/drm/ttm/tests/Makefile >>>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c >>>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>>> >>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>>> index afb3b2f5f425..53024e44a2d5 100644 >>>> --- a/drivers/gpu/drm/Kconfig >>>> +++ b/drivers/gpu/drm/Kconfig >>>> @@ -194,6 +194,21 @@ config DRM_TTM >>>> GPU memory types. Will be enabled automatically if a device >>>> driver >>>> uses it. >>>> +config DRM_TTM_KUNIT_TEST >>>> + tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS >>>> + default n >>>> + depends on DRM && KUNIT >>>> + select DRM_TTM >>>> + select DRM_EXPORT_FOR_TESTS if m >>>> + select DRM_KUNIT_TEST_HELPERS >>>> + default KUNIT_ALL_TESTS >>>> + help >>>> + Enables unit tests for TTM, a GPU memory manager >>>> subsystem used >>>> + to manage memory buffers. This option is mostly useful >>>> for kernel >>>> + developers. >>>> + >>>> + If in doubt, say "N". >>>> + >>>> config DRM_BUDDY >>>> tristate >>>> depends on DRM >>>> diff --git a/drivers/gpu/drm/ttm/Makefile >>>> b/drivers/gpu/drm/ttm/Makefile >>>> index f906b22959cf..dad298127226 100644 >>>> --- a/drivers/gpu/drm/ttm/Makefile >>>> +++ b/drivers/gpu/drm/ttm/Makefile >>>> @@ -8,3 +8,4 @@ ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o >>>> ttm_bo_vm.o ttm_module.o \ >>>> ttm-$(CONFIG_AGP) += ttm_agp_backend.o >>>> obj-$(CONFIG_DRM_TTM) += ttm.o >>>> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += tests/ >>>> diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig >>>> b/drivers/gpu/drm/ttm/tests/.kunitconfig >>>> new file mode 100644 >>>> index 000000000000..75fdce0cd98e >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig >>>> @@ -0,0 +1,4 @@ >>>> +CONFIG_KUNIT=y >>>> +CONFIG_DRM=y >>>> +CONFIG_DRM_KUNIT_TEST_HELPERS=y >>>> +CONFIG_DRM_TTM_KUNIT_TEST=y >>>> diff --git a/drivers/gpu/drm/ttm/tests/Makefile >>>> b/drivers/gpu/drm/ttm/tests/Makefile >>>> new file mode 100644 >>>> index 000000000000..7917805f37af >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/ttm/tests/Makefile >>>> @@ -0,0 +1,5 @@ >>>> +# SPDX-License-Identifier: GPL-2.0 AND MIT >>>> + >>>> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ >>>> + ttm_device_test.o \ >>>> + ttm_kunit_helpers.o >>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>>> b/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>>> new file mode 100644 >>>> index 000000000000..08d7314b1e35 >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>>> @@ -0,0 +1,54 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 AND MIT >>>> +/* >>>> + * Copyright © 2023 Intel Corporation >>>> + */ >>>> +#include <drm/ttm/ttm_resource.h> >>>> +#include <drm/ttm/ttm_device.h> >>>> +#include <drm/ttm/ttm_placement.h> >>>> + >>>> +#include "ttm_kunit_helpers.h" >>>> + >>>> +static void ttm_device_init_basic(struct kunit *test) >>>> +{ >>>> + struct ttm_test_devices_priv *priv = test->priv; >>>> + struct ttm_device *ttm_dev; >>>> + struct ttm_resource_manager *ttm_sys_man; >>>> + int err; >>>> + >>>> + ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); >>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev); >>>> + >>>> + err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false); >>>> + KUNIT_ASSERT_EQ(test, err, 0); >>>> + >>>> + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->funcs, &ttm_dev_funcs); >>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->wq); >>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->man_drv[TTM_PL_SYSTEM]); >>>> + >>>> + ttm_sys_man = &ttm_dev->sysman; >>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man); >>>> + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_tt); >>>> + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_type); >>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man->func); >>>> + >>>> + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->dev_mapping, >>>> + priv->drm->anon_inode->i_mapping); >>>> + >>>> + ttm_device_fini(ttm_dev); >>>> +} >>>> + >>>> +static struct kunit_case ttm_device_test_cases[] = { >>>> + KUNIT_CASE(ttm_device_init_basic), >>>> + {} >>>> +}; >>>> + >>>> +static struct kunit_suite ttm_device_test_suite = { >>>> + .name = "ttm_device", >>>> + .init = ttm_test_devices_init, >>>> + .exit = ttm_test_devices_fini, >>>> + .test_cases = ttm_device_test_cases, >>>> +}; >>>> + >>>> +kunit_test_suites(&ttm_device_test_suite); >>>> + >>>> +MODULE_LICENSE("GPL"); >>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>>> new file mode 100644 >>>> index 000000000000..d03db0405484 >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>>> @@ -0,0 +1,59 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 AND MIT >>>> +/* >>>> + * Copyright © 2023 Intel Corporation >>>> + */ >>>> +#include "ttm_kunit_helpers.h" >>>> + >>>> +struct ttm_device_funcs ttm_dev_funcs = { >>>> +}; >>>> +EXPORT_SYMBOL_GPL(ttm_dev_funcs); >>>> + >>>> +int ttm_kunit_helper_alloc_device(struct kunit *test, >>> >>> Since this function is only initializing the ttm device I think we >>> should name it ttm_kunit_helper_init_device(). >>> >>> On the other hand I don't see a good reason why it can't also >>> allocate the device. >> >> I believe that's a leftover from times when I thought I'll reuse DRM >> device between the tests. No problem, I can put them into one function. >> >> All the best, >> Karolina >> >>> >>> Apart from that looks like a good start, >>> Christian. >>> >>>> + struct ttm_device *ttm, >>>> + bool use_dma_alloc, >>>> + bool use_dma32) >>>> +{ >>>> + struct ttm_test_devices_priv *priv = test->priv; >>>> + struct drm_device *drm = priv->drm; >>>> + int err; >>>> + >>>> + err = ttm_device_init(ttm, &ttm_dev_funcs, drm->dev, >>>> + drm->anon_inode->i_mapping, >>>> + drm->vma_offset_manager, >>>> + use_dma_alloc, use_dma32); >>>> + >>>> + return err; >>>> +} >>>> +EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); >>>> + >>>> +int ttm_test_devices_init(struct kunit *test) >>>> +{ >>>> + struct ttm_test_devices_priv *priv; >>>> + >>>> + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); >>>> + KUNIT_ASSERT_NOT_NULL(test, priv); >>>> + >>>> + test->priv = priv; >>>> + >>>> + priv->dev = drm_kunit_helper_alloc_device(test); >>>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev); >>>> + >>>> + priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev, >>>> + sizeof(*priv->drm), 0, >>>> + DRIVER_GEM); >>>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm); >>>> + >>>> + return 0; >>>> +} >>>> +EXPORT_SYMBOL_GPL(ttm_test_devices_init); >>>> + >>>> +void ttm_test_devices_fini(struct kunit *test) >>>> +{ >>>> + struct ttm_test_devices_priv *priv = test->priv; >>>> + >>>> + drm_kunit_helper_free_device(test, priv->dev); >>>> + drm_dev_put(priv->drm); >>>> +} >>>> +EXPORT_SYMBOL_GPL(ttm_test_devices_fini); >>>> + >>>> +MODULE_LICENSE("GPL"); >>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>>> new file mode 100644 >>>> index 000000000000..69fb03b9c4d2 >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>>> @@ -0,0 +1,29 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 AND MIT */ >>>> +/* >>>> + * Copyright © 2023 Intel Corporation >>>> + */ >>>> +#ifndef TTM_KUNIT_HELPERS_H >>>> +#define TTM_KUNIT_HELPERS_H >>>> + >>>> +#include <drm/drm_drv.h> >>>> +#include <drm/ttm/ttm_device.h> >>>> + >>>> +#include <drm/drm_kunit_helpers.h> >>>> +#include <kunit/test.h> >>>> + >>>> +extern struct ttm_device_funcs ttm_dev_funcs; >>>> + >>>> +struct ttm_test_devices_priv { >>>> + struct drm_device *drm; >>>> + struct device *dev; >>>> +}; >>>> + >>>> +int ttm_kunit_helper_alloc_device(struct kunit *test, >>>> + struct ttm_device *ttm, >>>> + bool use_dma_alloc, >>>> + bool use_dma32); >>>> + >>>> +int ttm_test_devices_init(struct kunit *test); >>>> +void ttm_test_devices_fini(struct kunit *test); >>>> + >>>> +#endif // TTM_KUNIT_HELPERS_H >>> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC v2 1/3] drm/ttm: Introduce KUnit tests 2023-06-30 11:18 ` Christian König @ 2023-06-30 12:35 ` Karolina Stolarek 0 siblings, 0 replies; 15+ messages in thread From: Karolina Stolarek @ 2023-06-30 12:35 UTC (permalink / raw) To: Christian König, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das On 30.06.2023 13:18, Christian König wrote: > Am 30.06.23 um 13:09 schrieb Karolina Stolarek: >> Hi Christian, >> >> I'm taking a second look at this, and I wonder what would be the >> benefit of combining the initialization of device and ttm_device. >> (drm_)device can be initialized indepedently from the test params, so >> we can utilize .init and .exit callbacks offered by KUnit[1] to >> prepare and release the device indepedently of our setup. If we were >> to change it so ttm_device is also initialized there, we'd have to >> manually call init/fini in every single test. What's more, ttm_pool >> tests don't depend on ttm_device, and I can imagine that some structs >> can be tested in a similar way. >> >> Would it be fine with you to rename ttm_kunit_helper_alloc_device(), >> but leave its definition as it is? > > Yeah, sure. It's perfectly up to you how to structurize that, I wasn't > even aware that there are .init and .exit callbacks. > > I just found that the function name didn't match what the function was > doing. Right, and that was a good call, thanks a lot. All the best, Karolina > > Regards, > Christian. > >> >> All the best, >> Karolina >> ------------------------------------------------------------------- >> [1] - >> https://kunit.dev/third_party/kernel/docs/api/test.html#c.kunit_suite >> >> On 29.06.2023 12:05, Karolina Stolarek wrote: >>> Hi Christian, >>> >>> Thanks a lot for taking a look at my patches. >>> >>> On 29.06.2023 09:50, Christian König wrote: >>>> Am 27.06.23 um 10:32 schrieb Karolina Stolarek: >>>>> Add the initial version of unit tests for ttm_device struct, together >>>>> with helper functions. >>>>> >>>>> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> >>>>> --- >>>>> drivers/gpu/drm/Kconfig | 15 +++++ >>>>> drivers/gpu/drm/ttm/Makefile | 1 + >>>>> drivers/gpu/drm/ttm/tests/.kunitconfig | 4 ++ >>>>> drivers/gpu/drm/ttm/tests/Makefile | 5 ++ >>>>> drivers/gpu/drm/ttm/tests/ttm_device_test.c | 54 +++++++++++++++++ >>>>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 59 >>>>> +++++++++++++++++++ >>>>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 29 +++++++++ >>>>> 7 files changed, 167 insertions(+) >>>>> create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig >>>>> create mode 100644 drivers/gpu/drm/ttm/tests/Makefile >>>>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c >>>>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>>>> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>>>> >>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>>>> index afb3b2f5f425..53024e44a2d5 100644 >>>>> --- a/drivers/gpu/drm/Kconfig >>>>> +++ b/drivers/gpu/drm/Kconfig >>>>> @@ -194,6 +194,21 @@ config DRM_TTM >>>>> GPU memory types. Will be enabled automatically if a device >>>>> driver >>>>> uses it. >>>>> +config DRM_TTM_KUNIT_TEST >>>>> + tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS >>>>> + default n >>>>> + depends on DRM && KUNIT >>>>> + select DRM_TTM >>>>> + select DRM_EXPORT_FOR_TESTS if m >>>>> + select DRM_KUNIT_TEST_HELPERS >>>>> + default KUNIT_ALL_TESTS >>>>> + help >>>>> + Enables unit tests for TTM, a GPU memory manager >>>>> subsystem used >>>>> + to manage memory buffers. This option is mostly useful >>>>> for kernel >>>>> + developers. >>>>> + >>>>> + If in doubt, say "N". >>>>> + >>>>> config DRM_BUDDY >>>>> tristate >>>>> depends on DRM >>>>> diff --git a/drivers/gpu/drm/ttm/Makefile >>>>> b/drivers/gpu/drm/ttm/Makefile >>>>> index f906b22959cf..dad298127226 100644 >>>>> --- a/drivers/gpu/drm/ttm/Makefile >>>>> +++ b/drivers/gpu/drm/ttm/Makefile >>>>> @@ -8,3 +8,4 @@ ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o >>>>> ttm_bo_vm.o ttm_module.o \ >>>>> ttm-$(CONFIG_AGP) += ttm_agp_backend.o >>>>> obj-$(CONFIG_DRM_TTM) += ttm.o >>>>> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += tests/ >>>>> diff --git a/drivers/gpu/drm/ttm/tests/.kunitconfig >>>>> b/drivers/gpu/drm/ttm/tests/.kunitconfig >>>>> new file mode 100644 >>>>> index 000000000000..75fdce0cd98e >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/ttm/tests/.kunitconfig >>>>> @@ -0,0 +1,4 @@ >>>>> +CONFIG_KUNIT=y >>>>> +CONFIG_DRM=y >>>>> +CONFIG_DRM_KUNIT_TEST_HELPERS=y >>>>> +CONFIG_DRM_TTM_KUNIT_TEST=y >>>>> diff --git a/drivers/gpu/drm/ttm/tests/Makefile >>>>> b/drivers/gpu/drm/ttm/tests/Makefile >>>>> new file mode 100644 >>>>> index 000000000000..7917805f37af >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/ttm/tests/Makefile >>>>> @@ -0,0 +1,5 @@ >>>>> +# SPDX-License-Identifier: GPL-2.0 AND MIT >>>>> + >>>>> +obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ >>>>> + ttm_device_test.o \ >>>>> + ttm_kunit_helpers.o >>>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>>>> b/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>>>> new file mode 100644 >>>>> index 000000000000..08d7314b1e35 >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c >>>>> @@ -0,0 +1,54 @@ >>>>> +// SPDX-License-Identifier: GPL-2.0 AND MIT >>>>> +/* >>>>> + * Copyright © 2023 Intel Corporation >>>>> + */ >>>>> +#include <drm/ttm/ttm_resource.h> >>>>> +#include <drm/ttm/ttm_device.h> >>>>> +#include <drm/ttm/ttm_placement.h> >>>>> + >>>>> +#include "ttm_kunit_helpers.h" >>>>> + >>>>> +static void ttm_device_init_basic(struct kunit *test) >>>>> +{ >>>>> + struct ttm_test_devices_priv *priv = test->priv; >>>>> + struct ttm_device *ttm_dev; >>>>> + struct ttm_resource_manager *ttm_sys_man; >>>>> + int err; >>>>> + >>>>> + ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); >>>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev); >>>>> + >>>>> + err = ttm_kunit_helper_alloc_device(test, ttm_dev, false, false); >>>>> + KUNIT_ASSERT_EQ(test, err, 0); >>>>> + >>>>> + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->funcs, &ttm_dev_funcs); >>>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->wq); >>>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_dev->man_drv[TTM_PL_SYSTEM]); >>>>> + >>>>> + ttm_sys_man = &ttm_dev->sysman; >>>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man); >>>>> + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_tt); >>>>> + KUNIT_EXPECT_TRUE(test, ttm_sys_man->use_type); >>>>> + KUNIT_ASSERT_NOT_NULL(test, ttm_sys_man->func); >>>>> + >>>>> + KUNIT_EXPECT_PTR_EQ(test, ttm_dev->dev_mapping, >>>>> + priv->drm->anon_inode->i_mapping); >>>>> + >>>>> + ttm_device_fini(ttm_dev); >>>>> +} >>>>> + >>>>> +static struct kunit_case ttm_device_test_cases[] = { >>>>> + KUNIT_CASE(ttm_device_init_basic), >>>>> + {} >>>>> +}; >>>>> + >>>>> +static struct kunit_suite ttm_device_test_suite = { >>>>> + .name = "ttm_device", >>>>> + .init = ttm_test_devices_init, >>>>> + .exit = ttm_test_devices_fini, >>>>> + .test_cases = ttm_device_test_cases, >>>>> +}; >>>>> + >>>>> +kunit_test_suites(&ttm_device_test_suite); >>>>> + >>>>> +MODULE_LICENSE("GPL"); >>>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>>>> new file mode 100644 >>>>> index 000000000000..d03db0405484 >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >>>>> @@ -0,0 +1,59 @@ >>>>> +// SPDX-License-Identifier: GPL-2.0 AND MIT >>>>> +/* >>>>> + * Copyright © 2023 Intel Corporation >>>>> + */ >>>>> +#include "ttm_kunit_helpers.h" >>>>> + >>>>> +struct ttm_device_funcs ttm_dev_funcs = { >>>>> +}; >>>>> +EXPORT_SYMBOL_GPL(ttm_dev_funcs); >>>>> + >>>>> +int ttm_kunit_helper_alloc_device(struct kunit *test, >>>> >>>> Since this function is only initializing the ttm device I think we >>>> should name it ttm_kunit_helper_init_device(). >>>> >>>> On the other hand I don't see a good reason why it can't also >>>> allocate the device. >>> >>> I believe that's a leftover from times when I thought I'll reuse DRM >>> device between the tests. No problem, I can put them into one function. >>> >>> All the best, >>> Karolina >>> >>>> >>>> Apart from that looks like a good start, >>>> Christian. >>>> >>>>> + struct ttm_device *ttm, >>>>> + bool use_dma_alloc, >>>>> + bool use_dma32) >>>>> +{ >>>>> + struct ttm_test_devices_priv *priv = test->priv; >>>>> + struct drm_device *drm = priv->drm; >>>>> + int err; >>>>> + >>>>> + err = ttm_device_init(ttm, &ttm_dev_funcs, drm->dev, >>>>> + drm->anon_inode->i_mapping, >>>>> + drm->vma_offset_manager, >>>>> + use_dma_alloc, use_dma32); >>>>> + >>>>> + return err; >>>>> +} >>>>> +EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); >>>>> + >>>>> +int ttm_test_devices_init(struct kunit *test) >>>>> +{ >>>>> + struct ttm_test_devices_priv *priv; >>>>> + >>>>> + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); >>>>> + KUNIT_ASSERT_NOT_NULL(test, priv); >>>>> + >>>>> + test->priv = priv; >>>>> + >>>>> + priv->dev = drm_kunit_helper_alloc_device(test); >>>>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev); >>>>> + >>>>> + priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev, >>>>> + sizeof(*priv->drm), 0, >>>>> + DRIVER_GEM); >>>>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm); >>>>> + >>>>> + return 0; >>>>> +} >>>>> +EXPORT_SYMBOL_GPL(ttm_test_devices_init); >>>>> + >>>>> +void ttm_test_devices_fini(struct kunit *test) >>>>> +{ >>>>> + struct ttm_test_devices_priv *priv = test->priv; >>>>> + >>>>> + drm_kunit_helper_free_device(test, priv->dev); >>>>> + drm_dev_put(priv->drm); >>>>> +} >>>>> +EXPORT_SYMBOL_GPL(ttm_test_devices_fini); >>>>> + >>>>> +MODULE_LICENSE("GPL"); >>>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>>>> new file mode 100644 >>>>> index 000000000000..69fb03b9c4d2 >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >>>>> @@ -0,0 +1,29 @@ >>>>> +/* SPDX-License-Identifier: GPL-2.0 AND MIT */ >>>>> +/* >>>>> + * Copyright © 2023 Intel Corporation >>>>> + */ >>>>> +#ifndef TTM_KUNIT_HELPERS_H >>>>> +#define TTM_KUNIT_HELPERS_H >>>>> + >>>>> +#include <drm/drm_drv.h> >>>>> +#include <drm/ttm/ttm_device.h> >>>>> + >>>>> +#include <drm/drm_kunit_helpers.h> >>>>> +#include <kunit/test.h> >>>>> + >>>>> +extern struct ttm_device_funcs ttm_dev_funcs; >>>>> + >>>>> +struct ttm_test_devices_priv { >>>>> + struct drm_device *drm; >>>>> + struct device *dev; >>>>> +}; >>>>> + >>>>> +int ttm_kunit_helper_alloc_device(struct kunit *test, >>>>> + struct ttm_device *ttm, >>>>> + bool use_dma_alloc, >>>>> + bool use_dma32); >>>>> + >>>>> +int ttm_test_devices_init(struct kunit *test); >>>>> +void ttm_test_devices_fini(struct kunit *test); >>>>> + >>>>> +#endif // TTM_KUNIT_HELPERS_H >>>> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC v2 2/3] drm/ttm/tests: Add tests for ttm_device 2023-06-27 8:32 [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Karolina Stolarek 2023-06-27 8:32 ` [RFC v2 1/3] drm/ttm: Introduce KUnit tests Karolina Stolarek @ 2023-06-27 8:32 ` Karolina Stolarek 2023-06-29 9:14 ` Christian König 2023-06-27 8:32 ` [RFC v2 3/3] drm/ttm/tests: Add tests for ttm_pool Karolina Stolarek 2023-06-29 7:39 ` [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Christian König 3 siblings, 1 reply; 15+ messages in thread From: Karolina Stolarek @ 2023-06-27 8:32 UTC (permalink / raw) To: dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Karolina Stolarek, Andi Shyti, Shuah Khan, Christian König, Nirmoy Das 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@intel.com> --- 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)); + } + } + } + } + + 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), {} }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC v2 2/3] drm/ttm/tests: Add tests for ttm_device 2023-06-27 8:32 ` [RFC v2 2/3] drm/ttm/tests: Add tests for ttm_device Karolina Stolarek @ 2023-06-29 9:14 ` Christian König 2023-06-29 10:09 ` Karolina Stolarek 0 siblings, 1 reply; 15+ messages in thread From: Christian König @ 2023-06-29 9:14 UTC (permalink / raw) To: Karolina Stolarek, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das 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@intel.com> > --- > 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. 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@amd.com>. 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), > {} > }; > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC v2 2/3] drm/ttm/tests: Add tests for ttm_device 2023-06-29 9:14 ` Christian König @ 2023-06-29 10:09 ` Karolina Stolarek 0 siblings, 0 replies; 15+ messages in thread From: Karolina Stolarek @ 2023-06-29 10:09 UTC (permalink / raw) To: Christian König, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das 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@intel.com> >> --- >> 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@amd.com>. 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), >> {} >> }; > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC v2 3/3] drm/ttm/tests: Add tests for ttm_pool 2023-06-27 8:32 [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Karolina Stolarek 2023-06-27 8:32 ` [RFC v2 1/3] drm/ttm: Introduce KUnit tests Karolina Stolarek 2023-06-27 8:32 ` [RFC v2 2/3] drm/ttm/tests: Add tests for ttm_device Karolina Stolarek @ 2023-06-27 8:32 ` Karolina Stolarek 2023-06-29 9:17 ` Christian König 2023-06-29 7:39 ` [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Christian König 3 siblings, 1 reply; 15+ messages in thread From: Karolina Stolarek @ 2023-06-27 8:32 UTC (permalink / raw) To: dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Karolina Stolarek, Andi Shyti, Shuah Khan, Christian König, Nirmoy Das Add KUnit tests that exercise page allocation using page pools and freeing pages, either by returning them to the pool or freeing them. Add a basic test for ttm_pool cleanup. Introduce helpers to create a dummy ttm_buffer_object. Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> --- drivers/gpu/drm/ttm/tests/Makefile | 1 + drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 29 ++ drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 5 + drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 406 ++++++++++++++++++ 4 files changed, 441 insertions(+) create mode 100644 drivers/gpu/drm/ttm/tests/ttm_pool_test.c diff --git a/drivers/gpu/drm/ttm/tests/Makefile b/drivers/gpu/drm/ttm/tests/Makefile index 7917805f37af..ec87c4fc1ad5 100644 --- a/drivers/gpu/drm/ttm/tests/Makefile +++ b/drivers/gpu/drm/ttm/tests/Makefile @@ -2,4 +2,5 @@ obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ ttm_device_test.o \ + ttm_pool_test.o \ ttm_kunit_helpers.o diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c index d03db0405484..6ccd3e858397 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c @@ -26,6 +26,35 @@ int ttm_kunit_helper_alloc_device(struct kunit *test, } EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); +struct ttm_buffer_object *ttm_kunit_helper_ttm_bo_init(struct kunit *test, + size_t size) +{ + struct ttm_test_devices_priv *priv = test->priv; + struct drm_gem_object *gem_obj; + struct ttm_buffer_object *bo; + + gem_obj = kunit_kzalloc(test, sizeof(*gem_obj), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, gem_obj); + + drm_gem_private_object_init(priv->drm, gem_obj, size); + + bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, bo); + + bo->sg = NULL; + bo->base = *gem_obj; + + return bo; +} +EXPORT_SYMBOL_GPL(ttm_kunit_helper_ttm_bo_init); + +void ttm_kunit_helper_ttm_bo_fini(struct ttm_buffer_object *bo) +{ + drm_gem_object_release(&bo->base); + ttm_bo_put(bo); +} +EXPORT_SYMBOL_GPL(ttm_kunit_helper_ttm_bo_fini); + int ttm_test_devices_init(struct kunit *test) { struct ttm_test_devices_priv *priv; diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h index 69fb03b9c4d2..abb8279f18c7 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h @@ -7,6 +7,7 @@ #include <drm/drm_drv.h> #include <drm/ttm/ttm_device.h> +#include <drm/ttm/ttm_bo.h> #include <drm/drm_kunit_helpers.h> #include <kunit/test.h> @@ -23,6 +24,10 @@ int ttm_kunit_helper_alloc_device(struct kunit *test, bool use_dma_alloc, bool use_dma32); +struct ttm_buffer_object *ttm_kunit_helper_ttm_bo_init(struct kunit *test, + size_t size); +void ttm_kunit_helper_ttm_bo_fini(struct ttm_buffer_object *bo); + int ttm_test_devices_init(struct kunit *test); void ttm_test_devices_fini(struct kunit *test); diff --git a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c new file mode 100644 index 000000000000..0bc6415c3067 --- /dev/null +++ b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c @@ -0,0 +1,406 @@ +// SPDX-License-Identifier: GPL-2.0 AND MIT +/* + * Copyright © 2023 Intel Corporation + */ +#include <linux/mm.h> + +#include <drm/ttm/ttm_tt.h> +#include <drm/ttm/ttm_pool.h> + +#include "ttm_kunit_helpers.h" + +struct ttm_pool_test_case { + const char *description; + unsigned int order; + bool use_dma_alloc; +}; + +static struct ttm_operation_ctx simple_ctx = { + .interruptible = true, + .no_wait_gpu = false, +}; + +static struct ttm_tt *mock_ttm_tt_init(struct kunit *test, + uint32_t page_flags, + enum ttm_caching caching, + size_t size) +{ + struct ttm_tt *tt; + struct ttm_buffer_object *bo; + int err; + + bo = ttm_kunit_helper_ttm_bo_init(test, size); + KUNIT_ASSERT_NOT_NULL(test, bo); + + tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, tt); + + err = ttm_tt_init(tt, bo, page_flags, caching, 0); + KUNIT_ASSERT_EQ(test, err, 0); + + /* We don't need this BO later, release it */ + ttm_kunit_helper_ttm_bo_fini(bo); + + return tt; +} + +static struct ttm_pool *ttm_pool_pre_populated(struct kunit *test, + size_t size, + enum ttm_caching caching) +{ + struct ttm_test_devices_priv *priv = test->priv; + struct ttm_pool *pool; + struct ttm_tt *tt; + int err; + unsigned long order = __fls(size / PAGE_SIZE); + + tt = mock_ttm_tt_init(test, order, caching, size); + KUNIT_ASSERT_NOT_NULL(test, tt); + + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, pool); + + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); + + err = ttm_pool_alloc(pool, tt, &simple_ctx); + KUNIT_ASSERT_EQ(test, err, 0); + + ttm_pool_free(pool, tt); + ttm_tt_fini(tt); + + return pool; +} + +static const struct ttm_pool_test_case ttm_pool_basic_cases[] = { + { + .description = "One page", + .order = 0, + }, + { + .description = "More than one page", + .order = 2, + }, + { + .description = "Above the allocation limit", + .order = MAX_ORDER + 1, + }, + { + .description = "One page, with coherent DMA mappings enabled", + .order = 0, + .use_dma_alloc = true, + }, + { + .description = "Above the allocation limit, with coherent DMA mappings enabled", + .order = MAX_ORDER + 1, + .use_dma_alloc = true, + }, +}; + +static void ttm_pool_alloc_case_desc(const struct ttm_pool_test_case *t, + char *desc) +{ + strscpy(desc, t->description, KUNIT_PARAM_DESC_SIZE); +} + +KUNIT_ARRAY_PARAM(ttm_pool_alloc_basic, ttm_pool_basic_cases, + ttm_pool_alloc_case_desc); + +static void ttm_pool_alloc_basic(struct kunit *test) +{ + struct ttm_test_devices_priv *priv = test->priv; + const struct ttm_pool_test_case *params = test->param_value; + struct ttm_tt *tt; + struct ttm_pool *pool; + struct page *fst_page, *last_page; + int err; + enum ttm_caching caching = ttm_uncached; + unsigned int expected_num_pages = 1 << params->order; + size_t size = expected_num_pages * PAGE_SIZE; + + tt = mock_ttm_tt_init(test, 0, caching, size); + KUNIT_ASSERT_NOT_NULL(test, tt); + + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, pool); + + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, params->use_dma_alloc, + false); + + KUNIT_ASSERT_PTR_EQ(test, pool->dev, priv->dev); + KUNIT_ASSERT_EQ(test, pool->nid, NUMA_NO_NODE); + KUNIT_ASSERT_EQ(test, pool->use_dma_alloc, params->use_dma_alloc); + + err = ttm_pool_alloc(pool, tt, &simple_ctx); + KUNIT_ASSERT_EQ(test, err, 0); + KUNIT_ASSERT_EQ(test, tt->num_pages, expected_num_pages); + + fst_page = tt->pages[0]; + last_page = tt->pages[tt->num_pages - 1]; + + if (params->order <= MAX_ORDER) { + if (params->use_dma_alloc) { + KUNIT_ASSERT_NOT_NULL(test, (void *)fst_page->private); + KUNIT_ASSERT_NOT_NULL(test, (void *)last_page->private); + } else { + KUNIT_ASSERT_EQ(test, fst_page->private, params->order); + } + } else { + if (params->use_dma_alloc) { + KUNIT_ASSERT_NOT_NULL(test, (void *)fst_page->private); + KUNIT_ASSERT_NULL(test, (void *)last_page->private); + } else { + /* + * We expect to alloc one big block, followed by + * order 0 blocks + */ + KUNIT_ASSERT_EQ(test, fst_page->private, + min_t(unsigned int, MAX_ORDER, + params->order)); + KUNIT_ASSERT_EQ(test, last_page->private, 0); + } + } + + ttm_pool_free(pool, tt); + ttm_tt_fini(tt); + ttm_pool_fini(pool); +} + +static void ttm_pool_alloc_basic_dma_addr(struct kunit *test) +{ + struct ttm_test_devices_priv *priv = test->priv; + const struct ttm_pool_test_case *params = test->param_value; + struct ttm_tt *tt; + struct ttm_pool *pool; + struct ttm_buffer_object *bo; + dma_addr_t dma1, dma2; + int err; + enum ttm_caching caching = ttm_uncached; + unsigned int expected_num_pages = 1 << params->order; + size_t size = expected_num_pages * PAGE_SIZE; + + tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, tt); + + bo = ttm_kunit_helper_ttm_bo_init(test, size); + KUNIT_ASSERT_NOT_NULL(test, bo); + + err = ttm_sg_tt_init(tt, bo, 0, caching); + KUNIT_ASSERT_EQ(test, err, 0); + + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, pool); + + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); + + err = ttm_pool_alloc(pool, tt, &simple_ctx); + KUNIT_ASSERT_EQ(test, err, 0); + KUNIT_ASSERT_EQ(test, tt->num_pages, expected_num_pages); + + dma1 = tt->dma_address[0]; + dma2 = tt->dma_address[tt->num_pages - 1]; + + KUNIT_ASSERT_NOT_NULL(test, (void *)dma1); + KUNIT_ASSERT_NOT_NULL(test, (void *)dma2); + + ttm_pool_free(pool, tt); + ttm_tt_fini(tt); + ttm_pool_fini(pool); +} + +static void ttm_pool_alloc_order_caching_match(struct kunit *test) +{ + struct ttm_tt *tt; + struct ttm_pool *pool; + struct ttm_pool_type *pt; + enum ttm_caching caching = ttm_uncached; + unsigned int order = 0; + size_t size = PAGE_SIZE; + int err; + + pool = ttm_pool_pre_populated(test, size, caching); + + pt = &pool->caching[caching].orders[order]; + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); + + tt = mock_ttm_tt_init(test, 0, caching, size); + KUNIT_ASSERT_NOT_NULL(test, tt); + + err = ttm_pool_alloc(pool, tt, &simple_ctx); + KUNIT_ASSERT_EQ(test, err, 0); + + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); + + ttm_pool_free(pool, tt); + ttm_tt_fini(tt); + ttm_pool_fini(pool); +} + +static void ttm_pool_alloc_caching_mismatch(struct kunit *test) +{ + struct ttm_tt *tt; + struct ttm_pool *pool; + struct ttm_pool_type *pt_pool, *pt_tt; + int err; + enum ttm_caching tt_caching = ttm_uncached; + enum ttm_caching pool_caching = ttm_cached; + size_t size = PAGE_SIZE; + unsigned int order = 0; + + pool = ttm_pool_pre_populated(test, size, pool_caching); + + pt_pool = &pool->caching[pool_caching].orders[order]; + pt_tt = &pool->caching[tt_caching].orders[order]; + + tt = mock_ttm_tt_init(test, 0, tt_caching, size); + KUNIT_ASSERT_NOT_NULL(test, tt); + + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); + KUNIT_ASSERT_TRUE(test, list_empty(&pt_tt->pages)); + + err = ttm_pool_alloc(pool, tt, &simple_ctx); + KUNIT_ASSERT_EQ(test, err, 0); + + ttm_pool_free(pool, tt); + ttm_tt_fini(tt); + + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); + KUNIT_ASSERT_FALSE(test, list_empty(&pt_tt->pages)); + + ttm_pool_fini(pool); +} + +static void ttm_pool_alloc_order_mismatch(struct kunit *test) +{ + struct ttm_tt *tt; + struct ttm_pool *pool; + struct ttm_pool_type *pt_pool, *pt_tt; + int err; + enum ttm_caching caching = ttm_uncached; + unsigned int order = 2; + size_t fst_size = (1 << order) * PAGE_SIZE; + size_t snd_size = PAGE_SIZE; + + pool = ttm_pool_pre_populated(test, fst_size, caching); + + pt_pool = &pool->caching[caching].orders[order]; + pt_tt = &pool->caching[caching].orders[0]; + + tt = mock_ttm_tt_init(test, 0, caching, snd_size); + KUNIT_ASSERT_NOT_NULL(test, tt); + + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); + KUNIT_ASSERT_TRUE(test, list_empty(&pt_tt->pages)); + + err = ttm_pool_alloc(pool, tt, &simple_ctx); + KUNIT_ASSERT_EQ(test, err, 0); + + ttm_pool_free(pool, tt); + ttm_tt_fini(tt); + + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); + KUNIT_ASSERT_FALSE(test, list_empty(&pt_tt->pages)); + + ttm_pool_fini(pool); +} + +static void ttm_pool_free_dma_alloc(struct kunit *test) +{ + struct ttm_test_devices_priv *priv = test->priv; + struct ttm_tt *tt; + struct ttm_pool *pool; + struct ttm_pool_type *pt; + enum ttm_caching caching = ttm_uncached; + unsigned int order = 2; + size_t size = (1 << order) * PAGE_SIZE; + + tt = mock_ttm_tt_init(test, 0, caching, size); + KUNIT_ASSERT_NOT_NULL(test, tt); + + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, pool); + + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); + ttm_pool_alloc(pool, tt, &simple_ctx); + + pt = &pool->caching[caching].orders[order]; + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); + + ttm_pool_free(pool, tt); + ttm_tt_fini(tt); + + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); + + ttm_pool_fini(pool); +} + +static void ttm_pool_free_no_dma_alloc(struct kunit *test) +{ + struct ttm_test_devices_priv *priv = test->priv; + struct ttm_tt *tt; + struct ttm_pool *pool; + struct ttm_pool_type *pt; + enum ttm_caching caching = ttm_uncached; + unsigned int order = 2; + size_t size = (1 << order) * PAGE_SIZE; + + tt = mock_ttm_tt_init(test, 0, caching, size); + KUNIT_ASSERT_NOT_NULL(test, tt); + + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, pool); + + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, false, false); + ttm_pool_alloc(pool, tt, &simple_ctx); + + pt = &pool->caching[caching].orders[order]; + KUNIT_ASSERT_TRUE(test, list_is_singular(&pt->pages)); + + ttm_pool_free(pool, tt); + ttm_tt_fini(tt); + + KUNIT_ASSERT_TRUE(test, list_is_singular(&pt->pages)); + + ttm_pool_fini(pool); +} + +static void ttm_pool_fini_basic(struct kunit *test) +{ + struct ttm_pool *pool; + struct ttm_pool_type *pt; + enum ttm_caching caching = ttm_uncached; + unsigned int order = 0; + size_t size = PAGE_SIZE; + + pool = ttm_pool_pre_populated(test, size, caching); + pt = &pool->caching[caching].orders[order]; + + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); + + ttm_pool_fini(pool); + + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); +} + +static struct kunit_case ttm_pool_test_cases[] = { + KUNIT_CASE_PARAM(ttm_pool_alloc_basic, ttm_pool_alloc_basic_gen_params), + KUNIT_CASE_PARAM(ttm_pool_alloc_basic_dma_addr, + ttm_pool_alloc_basic_gen_params), + KUNIT_CASE(ttm_pool_alloc_order_caching_match), + KUNIT_CASE(ttm_pool_alloc_caching_mismatch), + KUNIT_CASE(ttm_pool_alloc_order_mismatch), + KUNIT_CASE(ttm_pool_free_dma_alloc), + KUNIT_CASE(ttm_pool_free_no_dma_alloc), + KUNIT_CASE(ttm_pool_fini_basic), + {} +}; + +static struct kunit_suite ttm_pool_test_suite = { + .name = "ttm_pool", + .init = ttm_test_devices_init, + .exit = ttm_test_devices_fini, + .test_cases = ttm_pool_test_cases, +}; + +kunit_test_suites(&ttm_pool_test_suite); + +MODULE_LICENSE("GPL"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC v2 3/3] drm/ttm/tests: Add tests for ttm_pool 2023-06-27 8:32 ` [RFC v2 3/3] drm/ttm/tests: Add tests for ttm_pool Karolina Stolarek @ 2023-06-29 9:17 ` Christian König 2023-06-29 10:19 ` Karolina Stolarek 0 siblings, 1 reply; 15+ messages in thread From: Christian König @ 2023-06-29 9:17 UTC (permalink / raw) To: Karolina Stolarek, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das Am 27.06.23 um 10:32 schrieb Karolina Stolarek: > Add KUnit tests that exercise page allocation using page pools > and freeing pages, either by returning them to the pool or > freeing them. Add a basic test for ttm_pool cleanup. Introduce > helpers to create a dummy ttm_buffer_object. > > Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> > --- > drivers/gpu/drm/ttm/tests/Makefile | 1 + > drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 29 ++ > drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 5 + > drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 406 ++++++++++++++++++ > 4 files changed, 441 insertions(+) > create mode 100644 drivers/gpu/drm/ttm/tests/ttm_pool_test.c > > diff --git a/drivers/gpu/drm/ttm/tests/Makefile b/drivers/gpu/drm/ttm/tests/Makefile > index 7917805f37af..ec87c4fc1ad5 100644 > --- a/drivers/gpu/drm/ttm/tests/Makefile > +++ b/drivers/gpu/drm/ttm/tests/Makefile > @@ -2,4 +2,5 @@ > > obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ > ttm_device_test.o \ > + ttm_pool_test.o \ > ttm_kunit_helpers.o > diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c > index d03db0405484..6ccd3e858397 100644 > --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c > @@ -26,6 +26,35 @@ int ttm_kunit_helper_alloc_device(struct kunit *test, > } > EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); > > +struct ttm_buffer_object *ttm_kunit_helper_ttm_bo_init(struct kunit *test, > + size_t size) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + struct drm_gem_object *gem_obj; > + struct ttm_buffer_object *bo; > + > + gem_obj = kunit_kzalloc(test, sizeof(*gem_obj), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, gem_obj); > + > + drm_gem_private_object_init(priv->drm, gem_obj, size); > + > + bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, bo); > + > + bo->sg = NULL; > + bo->base = *gem_obj; Hui? Why that hack? That is clearly leaking the gem object. Apart from that looks really good to me, Christian. > + > + return bo; > +} > +EXPORT_SYMBOL_GPL(ttm_kunit_helper_ttm_bo_init); > + > +void ttm_kunit_helper_ttm_bo_fini(struct ttm_buffer_object *bo) > +{ > + drm_gem_object_release(&bo->base); > + ttm_bo_put(bo); > +} > +EXPORT_SYMBOL_GPL(ttm_kunit_helper_ttm_bo_fini); > + > int ttm_test_devices_init(struct kunit *test) > { > struct ttm_test_devices_priv *priv; > diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h > index 69fb03b9c4d2..abb8279f18c7 100644 > --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h > @@ -7,6 +7,7 @@ > > #include <drm/drm_drv.h> > #include <drm/ttm/ttm_device.h> > +#include <drm/ttm/ttm_bo.h> > > #include <drm/drm_kunit_helpers.h> > #include <kunit/test.h> > @@ -23,6 +24,10 @@ int ttm_kunit_helper_alloc_device(struct kunit *test, > bool use_dma_alloc, > bool use_dma32); > > +struct ttm_buffer_object *ttm_kunit_helper_ttm_bo_init(struct kunit *test, > + size_t size); > +void ttm_kunit_helper_ttm_bo_fini(struct ttm_buffer_object *bo); > + > int ttm_test_devices_init(struct kunit *test); > void ttm_test_devices_fini(struct kunit *test); > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c > new file mode 100644 > index 000000000000..0bc6415c3067 > --- /dev/null > +++ b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c > @@ -0,0 +1,406 @@ > +// SPDX-License-Identifier: GPL-2.0 AND MIT > +/* > + * Copyright © 2023 Intel Corporation > + */ > +#include <linux/mm.h> > + > +#include <drm/ttm/ttm_tt.h> > +#include <drm/ttm/ttm_pool.h> > + > +#include "ttm_kunit_helpers.h" > + > +struct ttm_pool_test_case { > + const char *description; > + unsigned int order; > + bool use_dma_alloc; > +}; > + > +static struct ttm_operation_ctx simple_ctx = { > + .interruptible = true, > + .no_wait_gpu = false, > +}; > + > +static struct ttm_tt *mock_ttm_tt_init(struct kunit *test, > + uint32_t page_flags, > + enum ttm_caching caching, > + size_t size) > +{ > + struct ttm_tt *tt; > + struct ttm_buffer_object *bo; > + int err; > + > + bo = ttm_kunit_helper_ttm_bo_init(test, size); > + KUNIT_ASSERT_NOT_NULL(test, bo); > + > + tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + err = ttm_tt_init(tt, bo, page_flags, caching, 0); > + KUNIT_ASSERT_EQ(test, err, 0); > + > + /* We don't need this BO later, release it */ > + ttm_kunit_helper_ttm_bo_fini(bo); > + > + return tt; > +} > + > +static struct ttm_pool *ttm_pool_pre_populated(struct kunit *test, > + size_t size, > + enum ttm_caching caching) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + struct ttm_pool *pool; > + struct ttm_tt *tt; > + int err; > + unsigned long order = __fls(size / PAGE_SIZE); > + > + tt = mock_ttm_tt_init(test, order, caching, size); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, pool); > + > + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); > + > + err = ttm_pool_alloc(pool, tt, &simple_ctx); > + KUNIT_ASSERT_EQ(test, err, 0); > + > + ttm_pool_free(pool, tt); > + ttm_tt_fini(tt); > + > + return pool; > +} > + > +static const struct ttm_pool_test_case ttm_pool_basic_cases[] = { > + { > + .description = "One page", > + .order = 0, > + }, > + { > + .description = "More than one page", > + .order = 2, > + }, > + { > + .description = "Above the allocation limit", > + .order = MAX_ORDER + 1, > + }, > + { > + .description = "One page, with coherent DMA mappings enabled", > + .order = 0, > + .use_dma_alloc = true, > + }, > + { > + .description = "Above the allocation limit, with coherent DMA mappings enabled", > + .order = MAX_ORDER + 1, > + .use_dma_alloc = true, > + }, > +}; > + > +static void ttm_pool_alloc_case_desc(const struct ttm_pool_test_case *t, > + char *desc) > +{ > + strscpy(desc, t->description, KUNIT_PARAM_DESC_SIZE); > +} > + > +KUNIT_ARRAY_PARAM(ttm_pool_alloc_basic, ttm_pool_basic_cases, > + ttm_pool_alloc_case_desc); > + > +static void ttm_pool_alloc_basic(struct kunit *test) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + const struct ttm_pool_test_case *params = test->param_value; > + struct ttm_tt *tt; > + struct ttm_pool *pool; > + struct page *fst_page, *last_page; > + int err; > + enum ttm_caching caching = ttm_uncached; > + unsigned int expected_num_pages = 1 << params->order; > + size_t size = expected_num_pages * PAGE_SIZE; > + > + tt = mock_ttm_tt_init(test, 0, caching, size); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, pool); > + > + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, params->use_dma_alloc, > + false); > + > + KUNIT_ASSERT_PTR_EQ(test, pool->dev, priv->dev); > + KUNIT_ASSERT_EQ(test, pool->nid, NUMA_NO_NODE); > + KUNIT_ASSERT_EQ(test, pool->use_dma_alloc, params->use_dma_alloc); > + > + err = ttm_pool_alloc(pool, tt, &simple_ctx); > + KUNIT_ASSERT_EQ(test, err, 0); > + KUNIT_ASSERT_EQ(test, tt->num_pages, expected_num_pages); > + > + fst_page = tt->pages[0]; > + last_page = tt->pages[tt->num_pages - 1]; > + > + if (params->order <= MAX_ORDER) { > + if (params->use_dma_alloc) { > + KUNIT_ASSERT_NOT_NULL(test, (void *)fst_page->private); > + KUNIT_ASSERT_NOT_NULL(test, (void *)last_page->private); > + } else { > + KUNIT_ASSERT_EQ(test, fst_page->private, params->order); > + } > + } else { > + if (params->use_dma_alloc) { > + KUNIT_ASSERT_NOT_NULL(test, (void *)fst_page->private); > + KUNIT_ASSERT_NULL(test, (void *)last_page->private); > + } else { > + /* > + * We expect to alloc one big block, followed by > + * order 0 blocks > + */ > + KUNIT_ASSERT_EQ(test, fst_page->private, > + min_t(unsigned int, MAX_ORDER, > + params->order)); > + KUNIT_ASSERT_EQ(test, last_page->private, 0); > + } > + } > + > + ttm_pool_free(pool, tt); > + ttm_tt_fini(tt); > + ttm_pool_fini(pool); > +} > + > +static void ttm_pool_alloc_basic_dma_addr(struct kunit *test) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + const struct ttm_pool_test_case *params = test->param_value; > + struct ttm_tt *tt; > + struct ttm_pool *pool; > + struct ttm_buffer_object *bo; > + dma_addr_t dma1, dma2; > + int err; > + enum ttm_caching caching = ttm_uncached; > + unsigned int expected_num_pages = 1 << params->order; > + size_t size = expected_num_pages * PAGE_SIZE; > + > + tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + bo = ttm_kunit_helper_ttm_bo_init(test, size); > + KUNIT_ASSERT_NOT_NULL(test, bo); > + > + err = ttm_sg_tt_init(tt, bo, 0, caching); > + KUNIT_ASSERT_EQ(test, err, 0); > + > + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, pool); > + > + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); > + > + err = ttm_pool_alloc(pool, tt, &simple_ctx); > + KUNIT_ASSERT_EQ(test, err, 0); > + KUNIT_ASSERT_EQ(test, tt->num_pages, expected_num_pages); > + > + dma1 = tt->dma_address[0]; > + dma2 = tt->dma_address[tt->num_pages - 1]; > + > + KUNIT_ASSERT_NOT_NULL(test, (void *)dma1); > + KUNIT_ASSERT_NOT_NULL(test, (void *)dma2); > + > + ttm_pool_free(pool, tt); > + ttm_tt_fini(tt); > + ttm_pool_fini(pool); > +} > + > +static void ttm_pool_alloc_order_caching_match(struct kunit *test) > +{ > + struct ttm_tt *tt; > + struct ttm_pool *pool; > + struct ttm_pool_type *pt; > + enum ttm_caching caching = ttm_uncached; > + unsigned int order = 0; > + size_t size = PAGE_SIZE; > + int err; > + > + pool = ttm_pool_pre_populated(test, size, caching); > + > + pt = &pool->caching[caching].orders[order]; > + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); > + > + tt = mock_ttm_tt_init(test, 0, caching, size); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + err = ttm_pool_alloc(pool, tt, &simple_ctx); > + KUNIT_ASSERT_EQ(test, err, 0); > + > + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); > + > + ttm_pool_free(pool, tt); > + ttm_tt_fini(tt); > + ttm_pool_fini(pool); > +} > + > +static void ttm_pool_alloc_caching_mismatch(struct kunit *test) > +{ > + struct ttm_tt *tt; > + struct ttm_pool *pool; > + struct ttm_pool_type *pt_pool, *pt_tt; > + int err; > + enum ttm_caching tt_caching = ttm_uncached; > + enum ttm_caching pool_caching = ttm_cached; > + size_t size = PAGE_SIZE; > + unsigned int order = 0; > + > + pool = ttm_pool_pre_populated(test, size, pool_caching); > + > + pt_pool = &pool->caching[pool_caching].orders[order]; > + pt_tt = &pool->caching[tt_caching].orders[order]; > + > + tt = mock_ttm_tt_init(test, 0, tt_caching, size); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); > + KUNIT_ASSERT_TRUE(test, list_empty(&pt_tt->pages)); > + > + err = ttm_pool_alloc(pool, tt, &simple_ctx); > + KUNIT_ASSERT_EQ(test, err, 0); > + > + ttm_pool_free(pool, tt); > + ttm_tt_fini(tt); > + > + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); > + KUNIT_ASSERT_FALSE(test, list_empty(&pt_tt->pages)); > + > + ttm_pool_fini(pool); > +} > + > +static void ttm_pool_alloc_order_mismatch(struct kunit *test) > +{ > + struct ttm_tt *tt; > + struct ttm_pool *pool; > + struct ttm_pool_type *pt_pool, *pt_tt; > + int err; > + enum ttm_caching caching = ttm_uncached; > + unsigned int order = 2; > + size_t fst_size = (1 << order) * PAGE_SIZE; > + size_t snd_size = PAGE_SIZE; > + > + pool = ttm_pool_pre_populated(test, fst_size, caching); > + > + pt_pool = &pool->caching[caching].orders[order]; > + pt_tt = &pool->caching[caching].orders[0]; > + > + tt = mock_ttm_tt_init(test, 0, caching, snd_size); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); > + KUNIT_ASSERT_TRUE(test, list_empty(&pt_tt->pages)); > + > + err = ttm_pool_alloc(pool, tt, &simple_ctx); > + KUNIT_ASSERT_EQ(test, err, 0); > + > + ttm_pool_free(pool, tt); > + ttm_tt_fini(tt); > + > + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); > + KUNIT_ASSERT_FALSE(test, list_empty(&pt_tt->pages)); > + > + ttm_pool_fini(pool); > +} > + > +static void ttm_pool_free_dma_alloc(struct kunit *test) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + struct ttm_tt *tt; > + struct ttm_pool *pool; > + struct ttm_pool_type *pt; > + enum ttm_caching caching = ttm_uncached; > + unsigned int order = 2; > + size_t size = (1 << order) * PAGE_SIZE; > + > + tt = mock_ttm_tt_init(test, 0, caching, size); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, pool); > + > + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); > + ttm_pool_alloc(pool, tt, &simple_ctx); > + > + pt = &pool->caching[caching].orders[order]; > + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); > + > + ttm_pool_free(pool, tt); > + ttm_tt_fini(tt); > + > + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); > + > + ttm_pool_fini(pool); > +} > + > +static void ttm_pool_free_no_dma_alloc(struct kunit *test) > +{ > + struct ttm_test_devices_priv *priv = test->priv; > + struct ttm_tt *tt; > + struct ttm_pool *pool; > + struct ttm_pool_type *pt; > + enum ttm_caching caching = ttm_uncached; > + unsigned int order = 2; > + size_t size = (1 << order) * PAGE_SIZE; > + > + tt = mock_ttm_tt_init(test, 0, caching, size); > + KUNIT_ASSERT_NOT_NULL(test, tt); > + > + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, pool); > + > + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, false, false); > + ttm_pool_alloc(pool, tt, &simple_ctx); > + > + pt = &pool->caching[caching].orders[order]; > + KUNIT_ASSERT_TRUE(test, list_is_singular(&pt->pages)); > + > + ttm_pool_free(pool, tt); > + ttm_tt_fini(tt); > + > + KUNIT_ASSERT_TRUE(test, list_is_singular(&pt->pages)); > + > + ttm_pool_fini(pool); > +} > + > +static void ttm_pool_fini_basic(struct kunit *test) > +{ > + struct ttm_pool *pool; > + struct ttm_pool_type *pt; > + enum ttm_caching caching = ttm_uncached; > + unsigned int order = 0; > + size_t size = PAGE_SIZE; > + > + pool = ttm_pool_pre_populated(test, size, caching); > + pt = &pool->caching[caching].orders[order]; > + > + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); > + > + ttm_pool_fini(pool); > + > + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); > +} > + > +static struct kunit_case ttm_pool_test_cases[] = { > + KUNIT_CASE_PARAM(ttm_pool_alloc_basic, ttm_pool_alloc_basic_gen_params), > + KUNIT_CASE_PARAM(ttm_pool_alloc_basic_dma_addr, > + ttm_pool_alloc_basic_gen_params), > + KUNIT_CASE(ttm_pool_alloc_order_caching_match), > + KUNIT_CASE(ttm_pool_alloc_caching_mismatch), > + KUNIT_CASE(ttm_pool_alloc_order_mismatch), > + KUNIT_CASE(ttm_pool_free_dma_alloc), > + KUNIT_CASE(ttm_pool_free_no_dma_alloc), > + KUNIT_CASE(ttm_pool_fini_basic), > + {} > +}; > + > +static struct kunit_suite ttm_pool_test_suite = { > + .name = "ttm_pool", > + .init = ttm_test_devices_init, > + .exit = ttm_test_devices_fini, > + .test_cases = ttm_pool_test_cases, > +}; > + > +kunit_test_suites(&ttm_pool_test_suite); > + > +MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC v2 3/3] drm/ttm/tests: Add tests for ttm_pool 2023-06-29 9:17 ` Christian König @ 2023-06-29 10:19 ` Karolina Stolarek 0 siblings, 0 replies; 15+ messages in thread From: Karolina Stolarek @ 2023-06-29 10:19 UTC (permalink / raw) To: Christian König, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das On 29.06.2023 11:17, Christian König wrote: > > > Am 27.06.23 um 10:32 schrieb Karolina Stolarek: >> Add KUnit tests that exercise page allocation using page pools >> and freeing pages, either by returning them to the pool or >> freeing them. Add a basic test for ttm_pool cleanup. Introduce >> helpers to create a dummy ttm_buffer_object. >> >> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> >> --- >> drivers/gpu/drm/ttm/tests/Makefile | 1 + >> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 29 ++ >> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 5 + >> drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 406 ++++++++++++++++++ >> 4 files changed, 441 insertions(+) >> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_pool_test.c >> >> diff --git a/drivers/gpu/drm/ttm/tests/Makefile >> b/drivers/gpu/drm/ttm/tests/Makefile >> index 7917805f37af..ec87c4fc1ad5 100644 >> --- a/drivers/gpu/drm/ttm/tests/Makefile >> +++ b/drivers/gpu/drm/ttm/tests/Makefile >> @@ -2,4 +2,5 @@ >> obj-$(CONFIG_DRM_TTM_KUNIT_TEST) += \ >> ttm_device_test.o \ >> + ttm_pool_test.o \ >> ttm_kunit_helpers.o >> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> index d03db0405484..6ccd3e858397 100644 >> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> @@ -26,6 +26,35 @@ int ttm_kunit_helper_alloc_device(struct kunit *test, >> } >> EXPORT_SYMBOL_GPL(ttm_kunit_helper_alloc_device); >> +struct ttm_buffer_object *ttm_kunit_helper_ttm_bo_init(struct kunit >> *test, >> + size_t size) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + struct drm_gem_object *gem_obj; >> + struct ttm_buffer_object *bo; >> + >> + gem_obj = kunit_kzalloc(test, sizeof(*gem_obj), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, gem_obj); >> + >> + drm_gem_private_object_init(priv->drm, gem_obj, size); >> + >> + bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, bo); >> + >> + bo->sg = NULL; >> + bo->base = *gem_obj; > > Hui? Why that hack? That is clearly leaking the gem object. Huh, you're right... I believe that I had some problems with either device cleanup or allocs. I'll double-check and see if I can get rid of it. > Apart from that looks really good to me, > Christian. Thank you. I had some questions around testing > MAX_ORDER in v1 email[1], could you take a look and see if checks in ttm_pool_alloc_basic() are correct? Many thanks, Karolina ---------------------------------------------------------------- [1] - message-id: b08037ef-156c-0f5d-1fde-e2c55d7655fb@intel.com > >> + >> + return bo; >> +} >> +EXPORT_SYMBOL_GPL(ttm_kunit_helper_ttm_bo_init); >> + >> +void ttm_kunit_helper_ttm_bo_fini(struct ttm_buffer_object *bo) >> +{ >> + drm_gem_object_release(&bo->base); >> + ttm_bo_put(bo); >> +} >> +EXPORT_SYMBOL_GPL(ttm_kunit_helper_ttm_bo_fini); >> + >> int ttm_test_devices_init(struct kunit *test) >> { >> struct ttm_test_devices_priv *priv; >> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> index 69fb03b9c4d2..abb8279f18c7 100644 >> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> @@ -7,6 +7,7 @@ >> #include <drm/drm_drv.h> >> #include <drm/ttm/ttm_device.h> >> +#include <drm/ttm/ttm_bo.h> >> #include <drm/drm_kunit_helpers.h> >> #include <kunit/test.h> >> @@ -23,6 +24,10 @@ int ttm_kunit_helper_alloc_device(struct kunit *test, >> bool use_dma_alloc, >> bool use_dma32); >> +struct ttm_buffer_object *ttm_kunit_helper_ttm_bo_init(struct kunit >> *test, >> + size_t size); >> +void ttm_kunit_helper_ttm_bo_fini(struct ttm_buffer_object *bo); >> + >> int ttm_test_devices_init(struct kunit *test); >> void ttm_test_devices_fini(struct kunit *test); >> diff --git a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c >> b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c >> new file mode 100644 >> index 000000000000..0bc6415c3067 >> --- /dev/null >> +++ b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c >> @@ -0,0 +1,406 @@ >> +// SPDX-License-Identifier: GPL-2.0 AND MIT >> +/* >> + * Copyright © 2023 Intel Corporation >> + */ >> +#include <linux/mm.h> >> + >> +#include <drm/ttm/ttm_tt.h> >> +#include <drm/ttm/ttm_pool.h> >> + >> +#include "ttm_kunit_helpers.h" >> + >> +struct ttm_pool_test_case { >> + const char *description; >> + unsigned int order; >> + bool use_dma_alloc; >> +}; >> + >> +static struct ttm_operation_ctx simple_ctx = { >> + .interruptible = true, >> + .no_wait_gpu = false, >> +}; >> + >> +static struct ttm_tt *mock_ttm_tt_init(struct kunit *test, >> + uint32_t page_flags, >> + enum ttm_caching caching, >> + size_t size) >> +{ >> + struct ttm_tt *tt; >> + struct ttm_buffer_object *bo; >> + int err; >> + >> + bo = ttm_kunit_helper_ttm_bo_init(test, size); >> + KUNIT_ASSERT_NOT_NULL(test, bo); >> + >> + tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + err = ttm_tt_init(tt, bo, page_flags, caching, 0); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + >> + /* We don't need this BO later, release it */ >> + ttm_kunit_helper_ttm_bo_fini(bo); >> + >> + return tt; >> +} >> + >> +static struct ttm_pool *ttm_pool_pre_populated(struct kunit *test, >> + size_t size, >> + enum ttm_caching caching) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + struct ttm_pool *pool; >> + struct ttm_tt *tt; >> + int err; >> + unsigned long order = __fls(size / PAGE_SIZE); >> + >> + tt = mock_ttm_tt_init(test, order, caching, size); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, pool); >> + >> + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); >> + >> + err = ttm_pool_alloc(pool, tt, &simple_ctx); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + >> + ttm_pool_free(pool, tt); >> + ttm_tt_fini(tt); >> + >> + return pool; >> +} >> + >> +static const struct ttm_pool_test_case ttm_pool_basic_cases[] = { >> + { >> + .description = "One page", >> + .order = 0, >> + }, >> + { >> + .description = "More than one page", >> + .order = 2, >> + }, >> + { >> + .description = "Above the allocation limit", >> + .order = MAX_ORDER + 1, >> + }, >> + { >> + .description = "One page, with coherent DMA mappings enabled", >> + .order = 0, >> + .use_dma_alloc = true, >> + }, >> + { >> + .description = "Above the allocation limit, with coherent DMA >> mappings enabled", >> + .order = MAX_ORDER + 1, >> + .use_dma_alloc = true, >> + }, >> +}; >> + >> +static void ttm_pool_alloc_case_desc(const struct ttm_pool_test_case *t, >> + char *desc) >> +{ >> + strscpy(desc, t->description, KUNIT_PARAM_DESC_SIZE); >> +} >> + >> +KUNIT_ARRAY_PARAM(ttm_pool_alloc_basic, ttm_pool_basic_cases, >> + ttm_pool_alloc_case_desc); >> + >> +static void ttm_pool_alloc_basic(struct kunit *test) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + const struct ttm_pool_test_case *params = test->param_value; >> + struct ttm_tt *tt; >> + struct ttm_pool *pool; >> + struct page *fst_page, *last_page; >> + int err; >> + enum ttm_caching caching = ttm_uncached; >> + unsigned int expected_num_pages = 1 << params->order; >> + size_t size = expected_num_pages * PAGE_SIZE; >> + >> + tt = mock_ttm_tt_init(test, 0, caching, size); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, pool); >> + >> + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, params->use_dma_alloc, >> + false); >> + >> + KUNIT_ASSERT_PTR_EQ(test, pool->dev, priv->dev); >> + KUNIT_ASSERT_EQ(test, pool->nid, NUMA_NO_NODE); >> + KUNIT_ASSERT_EQ(test, pool->use_dma_alloc, params->use_dma_alloc); >> + >> + err = ttm_pool_alloc(pool, tt, &simple_ctx); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + KUNIT_ASSERT_EQ(test, tt->num_pages, expected_num_pages); >> + >> + fst_page = tt->pages[0]; >> + last_page = tt->pages[tt->num_pages - 1]; >> + >> + if (params->order <= MAX_ORDER) { >> + if (params->use_dma_alloc) { >> + KUNIT_ASSERT_NOT_NULL(test, (void *)fst_page->private); >> + KUNIT_ASSERT_NOT_NULL(test, (void *)last_page->private); >> + } else { >> + KUNIT_ASSERT_EQ(test, fst_page->private, params->order); >> + } >> + } else { >> + if (params->use_dma_alloc) { >> + KUNIT_ASSERT_NOT_NULL(test, (void *)fst_page->private); >> + KUNIT_ASSERT_NULL(test, (void *)last_page->private); >> + } else { >> + /* >> + * We expect to alloc one big block, followed by >> + * order 0 blocks >> + */ >> + KUNIT_ASSERT_EQ(test, fst_page->private, >> + min_t(unsigned int, MAX_ORDER, >> + params->order)); >> + KUNIT_ASSERT_EQ(test, last_page->private, 0); >> + } >> + } >> + >> + ttm_pool_free(pool, tt); >> + ttm_tt_fini(tt); >> + ttm_pool_fini(pool); >> +} >> + >> +static void ttm_pool_alloc_basic_dma_addr(struct kunit *test) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + const struct ttm_pool_test_case *params = test->param_value; >> + struct ttm_tt *tt; >> + struct ttm_pool *pool; >> + struct ttm_buffer_object *bo; >> + dma_addr_t dma1, dma2; >> + int err; >> + enum ttm_caching caching = ttm_uncached; >> + unsigned int expected_num_pages = 1 << params->order; >> + size_t size = expected_num_pages * PAGE_SIZE; >> + >> + tt = kunit_kzalloc(test, sizeof(*tt), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + bo = ttm_kunit_helper_ttm_bo_init(test, size); >> + KUNIT_ASSERT_NOT_NULL(test, bo); >> + >> + err = ttm_sg_tt_init(tt, bo, 0, caching); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + >> + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, pool); >> + >> + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); >> + >> + err = ttm_pool_alloc(pool, tt, &simple_ctx); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + KUNIT_ASSERT_EQ(test, tt->num_pages, expected_num_pages); >> + >> + dma1 = tt->dma_address[0]; >> + dma2 = tt->dma_address[tt->num_pages - 1]; >> + >> + KUNIT_ASSERT_NOT_NULL(test, (void *)dma1); >> + KUNIT_ASSERT_NOT_NULL(test, (void *)dma2); >> + >> + ttm_pool_free(pool, tt); >> + ttm_tt_fini(tt); >> + ttm_pool_fini(pool); >> +} >> + >> +static void ttm_pool_alloc_order_caching_match(struct kunit *test) >> +{ >> + struct ttm_tt *tt; >> + struct ttm_pool *pool; >> + struct ttm_pool_type *pt; >> + enum ttm_caching caching = ttm_uncached; >> + unsigned int order = 0; >> + size_t size = PAGE_SIZE; >> + int err; >> + >> + pool = ttm_pool_pre_populated(test, size, caching); >> + >> + pt = &pool->caching[caching].orders[order]; >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); >> + >> + tt = mock_ttm_tt_init(test, 0, caching, size); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + err = ttm_pool_alloc(pool, tt, &simple_ctx); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + >> + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); >> + >> + ttm_pool_free(pool, tt); >> + ttm_tt_fini(tt); >> + ttm_pool_fini(pool); >> +} >> + >> +static void ttm_pool_alloc_caching_mismatch(struct kunit *test) >> +{ >> + struct ttm_tt *tt; >> + struct ttm_pool *pool; >> + struct ttm_pool_type *pt_pool, *pt_tt; >> + int err; >> + enum ttm_caching tt_caching = ttm_uncached; >> + enum ttm_caching pool_caching = ttm_cached; >> + size_t size = PAGE_SIZE; >> + unsigned int order = 0; >> + >> + pool = ttm_pool_pre_populated(test, size, pool_caching); >> + >> + pt_pool = &pool->caching[pool_caching].orders[order]; >> + pt_tt = &pool->caching[tt_caching].orders[order]; >> + >> + tt = mock_ttm_tt_init(test, 0, tt_caching, size); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); >> + KUNIT_ASSERT_TRUE(test, list_empty(&pt_tt->pages)); >> + >> + err = ttm_pool_alloc(pool, tt, &simple_ctx); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + >> + ttm_pool_free(pool, tt); >> + ttm_tt_fini(tt); >> + >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt_tt->pages)); >> + >> + ttm_pool_fini(pool); >> +} >> + >> +static void ttm_pool_alloc_order_mismatch(struct kunit *test) >> +{ >> + struct ttm_tt *tt; >> + struct ttm_pool *pool; >> + struct ttm_pool_type *pt_pool, *pt_tt; >> + int err; >> + enum ttm_caching caching = ttm_uncached; >> + unsigned int order = 2; >> + size_t fst_size = (1 << order) * PAGE_SIZE; >> + size_t snd_size = PAGE_SIZE; >> + >> + pool = ttm_pool_pre_populated(test, fst_size, caching); >> + >> + pt_pool = &pool->caching[caching].orders[order]; >> + pt_tt = &pool->caching[caching].orders[0]; >> + >> + tt = mock_ttm_tt_init(test, 0, caching, snd_size); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); >> + KUNIT_ASSERT_TRUE(test, list_empty(&pt_tt->pages)); >> + >> + err = ttm_pool_alloc(pool, tt, &simple_ctx); >> + KUNIT_ASSERT_EQ(test, err, 0); >> + >> + ttm_pool_free(pool, tt); >> + ttm_tt_fini(tt); >> + >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt_pool->pages)); >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt_tt->pages)); >> + >> + ttm_pool_fini(pool); >> +} >> + >> +static void ttm_pool_free_dma_alloc(struct kunit *test) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + struct ttm_tt *tt; >> + struct ttm_pool *pool; >> + struct ttm_pool_type *pt; >> + enum ttm_caching caching = ttm_uncached; >> + unsigned int order = 2; >> + size_t size = (1 << order) * PAGE_SIZE; >> + >> + tt = mock_ttm_tt_init(test, 0, caching, size); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, pool); >> + >> + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, true, false); >> + ttm_pool_alloc(pool, tt, &simple_ctx); >> + >> + pt = &pool->caching[caching].orders[order]; >> + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); >> + >> + ttm_pool_free(pool, tt); >> + ttm_tt_fini(tt); >> + >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); >> + >> + ttm_pool_fini(pool); >> +} >> + >> +static void ttm_pool_free_no_dma_alloc(struct kunit *test) >> +{ >> + struct ttm_test_devices_priv *priv = test->priv; >> + struct ttm_tt *tt; >> + struct ttm_pool *pool; >> + struct ttm_pool_type *pt; >> + enum ttm_caching caching = ttm_uncached; >> + unsigned int order = 2; >> + size_t size = (1 << order) * PAGE_SIZE; >> + >> + tt = mock_ttm_tt_init(test, 0, caching, size); >> + KUNIT_ASSERT_NOT_NULL(test, tt); >> + >> + pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); >> + KUNIT_ASSERT_NOT_NULL(test, pool); >> + >> + ttm_pool_init(pool, priv->dev, NUMA_NO_NODE, false, false); >> + ttm_pool_alloc(pool, tt, &simple_ctx); >> + >> + pt = &pool->caching[caching].orders[order]; >> + KUNIT_ASSERT_TRUE(test, list_is_singular(&pt->pages)); >> + >> + ttm_pool_free(pool, tt); >> + ttm_tt_fini(tt); >> + >> + KUNIT_ASSERT_TRUE(test, list_is_singular(&pt->pages)); >> + >> + ttm_pool_fini(pool); >> +} >> + >> +static void ttm_pool_fini_basic(struct kunit *test) >> +{ >> + struct ttm_pool *pool; >> + struct ttm_pool_type *pt; >> + enum ttm_caching caching = ttm_uncached; >> + unsigned int order = 0; >> + size_t size = PAGE_SIZE; >> + >> + pool = ttm_pool_pre_populated(test, size, caching); >> + pt = &pool->caching[caching].orders[order]; >> + >> + KUNIT_ASSERT_FALSE(test, list_empty(&pt->pages)); >> + >> + ttm_pool_fini(pool); >> + >> + KUNIT_ASSERT_TRUE(test, list_empty(&pt->pages)); >> +} >> + >> +static struct kunit_case ttm_pool_test_cases[] = { >> + KUNIT_CASE_PARAM(ttm_pool_alloc_basic, >> ttm_pool_alloc_basic_gen_params), >> + KUNIT_CASE_PARAM(ttm_pool_alloc_basic_dma_addr, >> + ttm_pool_alloc_basic_gen_params), >> + KUNIT_CASE(ttm_pool_alloc_order_caching_match), >> + KUNIT_CASE(ttm_pool_alloc_caching_mismatch), >> + KUNIT_CASE(ttm_pool_alloc_order_mismatch), >> + KUNIT_CASE(ttm_pool_free_dma_alloc), >> + KUNIT_CASE(ttm_pool_free_no_dma_alloc), >> + KUNIT_CASE(ttm_pool_fini_basic), >> + {} >> +}; >> + >> +static struct kunit_suite ttm_pool_test_suite = { >> + .name = "ttm_pool", >> + .init = ttm_test_devices_init, >> + .exit = ttm_test_devices_fini, >> + .test_cases = ttm_pool_test_cases, >> +}; >> + >> +kunit_test_suites(&ttm_pool_test_suite); >> + >> +MODULE_LICENSE("GPL"); > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC v2 0/3] Introduce KUnit tests for TTM subsystem 2023-06-27 8:32 [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Karolina Stolarek ` (2 preceding siblings ...) 2023-06-27 8:32 ` [RFC v2 3/3] drm/ttm/tests: Add tests for ttm_pool Karolina Stolarek @ 2023-06-29 7:39 ` Christian König 2023-06-29 7:42 ` Karolina Stolarek 3 siblings, 1 reply; 15+ messages in thread From: Christian König @ 2023-06-29 7:39 UTC (permalink / raw) To: Karolina Stolarek, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das Sorry for the delayed response, AMD had some mail server issues and I simply missed this through the web access interface. Going to take a look at this now, Christian. Am 27.06.23 um 10:32 schrieb Karolina Stolarek: > This series introduces KUnit[1] tests for TTM (Translation Table Manager) > subsystem, a memory manager used by graphics drivers to create and manage > memory buffers across different memory domains, such as system memory > or VRAM. > > Unit tests implemented here cover two data structures: > - ttm_device -- referred as a buffer object device, which stores > resource managers and page pools > - ttm_pool -- a struct of pools (ttm_pool_type) of different page > orders and caching attributes, with pages that can be reused on > the next buffer allocation > > Use kunit_tool script to manually run the tests: > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/ttm/tests > > The kunit tool might not work with older python versions. To fix that, > apply [2] patch. > > To build a kernel with TTM KUnit tests, enable CONFIG_DRM_TTM_KUNIT_TEST > symbol. > > As for now, tests are architecture-agnostic (i.e. KUnit runner uses UML > kernel), which means that we have limited coverage in some places. For > example, we can't fully test the initialization of global page pools, > such as global_write_combined. It is to be decided if we want to stick > to UML or use CONFIG_X86 (at least to some extent). > > These patches are just a beginning of the work to improve the test > coverage of TTM. Feel free to suggest changes, test cases or priorities. > > Many thanks, > Karolina > > v2: > - Add missing symbol exports in ttm_kunit_helpers.c > - Update helpers include to fix compilation issues (didn't catch it as > KUnit tests weren't enabled in the kernel I tested, an oversight > on my part) > - Add checks for ttm_pool fields in ttm_pool_alloc_basic(), including the > one for NUMA node id > - Rebase the changes on the top of drm-tip > > -------------------------------- > [1] - https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html > [2] - https://lore.kernel.org/lkml/20230610175618.82271-1-sj@kernel.org/T/ > > Karolina Stolarek (3): > drm/ttm: Introduce KUnit tests > drm/ttm/tests: Add tests for ttm_device > drm/ttm/tests: Add tests for ttm_pool > > drivers/gpu/drm/Kconfig | 15 + > drivers/gpu/drm/ttm/Makefile | 1 + > drivers/gpu/drm/ttm/tests/.kunitconfig | 4 + > drivers/gpu/drm/ttm/tests/Makefile | 6 + > drivers/gpu/drm/ttm/tests/ttm_device_test.c | 213 +++++++++ > drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 88 ++++ > drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 34 ++ > drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 406 ++++++++++++++++++ > 8 files changed, 767 insertions(+) > create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig > create mode 100644 drivers/gpu/drm/ttm/tests/Makefile > create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c > create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c > create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h > create mode 100644 drivers/gpu/drm/ttm/tests/ttm_pool_test.c > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC v2 0/3] Introduce KUnit tests for TTM subsystem 2023-06-29 7:39 ` [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Christian König @ 2023-06-29 7:42 ` Karolina Stolarek 0 siblings, 0 replies; 15+ messages in thread From: Karolina Stolarek @ 2023-06-29 7:42 UTC (permalink / raw) To: Christian König, dri-devel Cc: Thomas Hellström, Mauro Carvalho Chehab, Shuah Khan, Andi Shyti, Nirmoy Das On 29.06.2023 09:39, Christian König wrote: > Sorry for the delayed response, AMD had some mail server issues and I > simply missed this through the web access interface. No worries, that gave me an opportunity to fix my series :) All the best, Karolina > > Going to take a look at this now, > Christian. > > Am 27.06.23 um 10:32 schrieb Karolina Stolarek: >> This series introduces KUnit[1] tests for TTM (Translation Table Manager) >> subsystem, a memory manager used by graphics drivers to create and manage >> memory buffers across different memory domains, such as system memory >> or VRAM. >> >> Unit tests implemented here cover two data structures: >> - ttm_device -- referred as a buffer object device, which stores >> resource managers and page pools >> - ttm_pool -- a struct of pools (ttm_pool_type) of different page >> orders and caching attributes, with pages that can be reused on >> the next buffer allocation >> >> Use kunit_tool script to manually run the tests: >> >> $ ./tools/testing/kunit/kunit.py run >> --kunitconfig=drivers/gpu/drm/ttm/tests >> >> The kunit tool might not work with older python versions. To fix that, >> apply [2] patch. >> >> To build a kernel with TTM KUnit tests, enable CONFIG_DRM_TTM_KUNIT_TEST >> symbol. >> >> As for now, tests are architecture-agnostic (i.e. KUnit runner uses UML >> kernel), which means that we have limited coverage in some places. For >> example, we can't fully test the initialization of global page pools, >> such as global_write_combined. It is to be decided if we want to stick >> to UML or use CONFIG_X86 (at least to some extent). >> >> These patches are just a beginning of the work to improve the test >> coverage of TTM. Feel free to suggest changes, test cases or priorities. >> >> Many thanks, >> Karolina >> >> v2: >> - Add missing symbol exports in ttm_kunit_helpers.c >> - Update helpers include to fix compilation issues (didn't catch it as >> KUnit tests weren't enabled in the kernel I tested, an oversight >> on my part) >> - Add checks for ttm_pool fields in ttm_pool_alloc_basic(), >> including the >> one for NUMA node id >> - Rebase the changes on the top of drm-tip >> >> -------------------------------- >> [1] - https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html >> [2] - >> https://lore.kernel.org/lkml/20230610175618.82271-1-sj@kernel.org/T/ >> >> Karolina Stolarek (3): >> drm/ttm: Introduce KUnit tests >> drm/ttm/tests: Add tests for ttm_device >> drm/ttm/tests: Add tests for ttm_pool >> >> drivers/gpu/drm/Kconfig | 15 + >> drivers/gpu/drm/ttm/Makefile | 1 + >> drivers/gpu/drm/ttm/tests/.kunitconfig | 4 + >> drivers/gpu/drm/ttm/tests/Makefile | 6 + >> drivers/gpu/drm/ttm/tests/ttm_device_test.c | 213 +++++++++ >> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 88 ++++ >> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h | 34 ++ >> drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 406 ++++++++++++++++++ >> 8 files changed, 767 insertions(+) >> create mode 100644 drivers/gpu/drm/ttm/tests/.kunitconfig >> create mode 100644 drivers/gpu/drm/ttm/tests/Makefile >> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_device_test.c >> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c >> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h >> create mode 100644 drivers/gpu/drm/ttm/tests/ttm_pool_test.c >> > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-06-30 12:36 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-27 8:32 [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Karolina Stolarek 2023-06-27 8:32 ` [RFC v2 1/3] drm/ttm: Introduce KUnit tests Karolina Stolarek 2023-06-29 7:50 ` Christian König 2023-06-29 10:05 ` Karolina Stolarek 2023-06-30 11:09 ` Karolina Stolarek 2023-06-30 11:18 ` Christian König 2023-06-30 12:35 ` Karolina Stolarek 2023-06-27 8:32 ` [RFC v2 2/3] drm/ttm/tests: Add tests for ttm_device Karolina Stolarek 2023-06-29 9:14 ` Christian König 2023-06-29 10:09 ` Karolina Stolarek 2023-06-27 8:32 ` [RFC v2 3/3] drm/ttm/tests: Add tests for ttm_pool Karolina Stolarek 2023-06-29 9:17 ` Christian König 2023-06-29 10:19 ` Karolina Stolarek 2023-06-29 7:39 ` [RFC v2 0/3] Introduce KUnit tests for TTM subsystem Christian König 2023-06-29 7:42 ` Karolina Stolarek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox