[Intel-xe] [PATCH 1/7] drm/xe: Add basic unit tests for rtp

Sat Apr 1 08:59:32 UTC 2023

On Fri, Mar 31, 2023 at 07:43:34AM -0600, Lucas De Marchi wrote:
>On Mon, Mar 27, 2023 at 07:56:01PM +0200, Michał Winiarski wrote:
>>On Tue, Mar 21, 2023 at 03:05:21PM -0700, Lucas De Marchi wrote:
>>>Add some basic unit tests for rtp. This is intended to prove the
>>>functionality of the rtp itself, like coalescing entries, rejecting
>>>non-disjoint values, etc.
>>>
>>>Contrary to the other tests in xe, this is a unit test to test the
>>>sw-side only, so it can be executed on any machine - it doesn't interact
>>>with the real hardware. For that a mock was provided with the minimum
>>>initialization needed. Running it produces the following output:
>>>
>>>	$ ./tools/testing/kunit/kunit.py run --raw_output  \
>>>		--kunitconfig drivers/gpu/drm/xe/.kunitconfig xe_rtp
>>>	...
>>>	[13:46:14] Starting KUnit Kernel (1/1)...
>>>	KTAP version 1
>>>	1..1
>>>	    KTAP version 1
>>>	    # Subtest: xe_rtp
>>>	    1..3
>>>	    ok 1 xe_rtp_process_basic
>>>	[drm:xe_reg_sr_add] *ERROR* Discarding save-restore reg 0001 (clear: 00000001, set: 00000001, masked: no): ret=-22
>>>	[drm:xe_reg_sr_add] *ERROR* Discarding save-restore reg 0001 (clear: 00000003, set: 00000003, masked: no): ret=-22
>>>	    ok 2 xe_rtp_process_dup
>>>	[drm:xe_reg_sr_add] *ERROR* Discarding save-restore reg 0001 (clear: 00000001, set: 00000001, masked: no): ret=-22
>>>	[drm:xe_reg_sr_add] *ERROR* Discarding save-restore reg 0001 (clear: 00000003, set: 00000003, masked: no): ret=-22
>>>	    ok 3 xe_rtp_process_incompat_types
>>>	# xe_rtp: pass:3 fail:0 skip:0 total:3
>>>	# Totals: pass:3 fail:0 skip:0 total:3
>>>	ok 1 xe_rtp
>>>	[13:46:14] Elapsed time: 6.800s total, 0.002s configuring, 6.682s building, 0.073s running
>>>
>>>Note that the ERRORs in the kernel log are expected since it's testing
>>>non-disjoint or incompatible entries.
>>>
>>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>>Reviewed-by: Maarten Lankhorst<maarten.lankhorst at linux.intel.com>
>>>---
>>> drivers/gpu/drm/xe/Kconfig.debug       |   1 +
>>> drivers/gpu/drm/xe/tests/Makefile      |   7 +-
>>> drivers/gpu/drm/xe/tests/xe_rtp.c      | 201 +++++++++++++++++++++++++
>>> drivers/gpu/drm/xe/tests/xe_rtp_test.c |  64 ++++++++
>>> drivers/gpu/drm/xe/tests/xe_rtp_test.h |  15 ++
>>> drivers/gpu/drm/xe/xe_rtp.c            |   4 +
>>> 6 files changed, 290 insertions(+), 2 deletions(-)
>>> create mode 100644 drivers/gpu/drm/xe/tests/xe_rtp.c
>>> create mode 100644 drivers/gpu/drm/xe/tests/xe_rtp_test.c
>>> create mode 100644 drivers/gpu/drm/xe/tests/xe_rtp_test.h
>>>
>>>diff --git a/drivers/gpu/drm/xe/Kconfig.debug b/drivers/gpu/drm/xe/Kconfig.debug
>>>index 93b284cdd0a2..11bb13c73e7b 100644
>>>--- a/drivers/gpu/drm/xe/Kconfig.debug
>>>+++ b/drivers/gpu/drm/xe/Kconfig.debug
>>>@@ -66,6 +66,7 @@ config DRM_XE_KUNIT_TEST
>>> 	depends on DRM_XE && KUNIT && DEBUG_FS
>>> 	default KUNIT_ALL_TESTS
>>> 	select DRM_EXPORT_FOR_TESTS if m
>>>+	select DRM_KUNIT_TEST_HELPERS
>>> 	help
>>> 	  Choose this option to allow the driver to perform selftests under
>>> 	  the kunit framework
>>
>>But now we're mixing proper isolated tests with HW-interacting ones.
>>Can we move the HW-interacting ones to a separate Kconfig which is not
>>included in .kunitconfig and doesn't set its default value using
>>KUNIT_ALL_TESTS?
>
>I'm not sure moving them to a separate kconfig is good, because that
>would mean people building without one or the other. The only case it
>makes sense to build without the hw-interacting ones would be for when
>we are building for UML.
>
>I want to make sure the kconfig from CI includes both and people can
>run under !UML both of them.
>
>On the positive side for your approach, we'd be moving the
>*HW-interacting* ones to a separate config. This means we can create the
>split between the true unit tests and the ones just using the kunit
>infrastructure. I will give this a try and see how it looks in the end.
>
>I've been advocating for a split on a name-basis, calling these unit
>tests as sw_* and the others as hw_*. Then running just the sw ones is
>simply a matter of passing it as argument to kunit.py

I will leave this for later as I don't want to stall the tests on
something that can be converted later and... this is not making the
situation any worse: actually running this in CI will provide an output
a little more useful than it actually is giving: a skip on all tests.

I'm almost convinced we can make it work with separate configs as you
suggested, but we will need to sync with CI to make sure it fits
everyody.

>
>>>diff --git a/drivers/gpu/drm/xe/tests/Makefile b/drivers/gpu/drm/xe/tests/Makefile
>>>index 47056b6459e3..c5c2f108d017 100644
>>>--- a/drivers/gpu/drm/xe/tests/Makefile
>>>+++ b/drivers/gpu/drm/xe/tests/Makefile
>>>@@ -1,4 +1,7 @@
>>> # SPDX-License-Identifier: GPL-2.0
>>>
>>>-obj-$(CONFIG_DRM_XE_KUNIT_TEST) += xe_bo_test.o xe_dma_buf_test.o \
>>>-	xe_migrate_test.o
>>>+obj-$(CONFIG_DRM_XE_KUNIT_TEST) += \
>>>+	xe_bo_test.o \
>>>+	xe_dma_buf_test.o \
>>>+	xe_migrate_test.o \
>>>+	xe_rtp_test.o
>>>diff --git a/drivers/gpu/drm/xe/tests/xe_rtp.c b/drivers/gpu/drm/xe/tests/xe_rtp.c
>>>new file mode 100644
>>>index 000000000000..92e2c2b8b44d
>>>--- /dev/null
>>>+++ b/drivers/gpu/drm/xe/tests/xe_rtp.c
>>>@@ -0,0 +1,201 @@
>>>+// SPDX-License-Identifier: GPL-2.0 AND MIT
>>>+/*
>>>+ * Copyright © 2023 Intel Corporation
>>>+ */
>>>+
>>>+#include "tests/xe_rtp_test.h"
>>>+
>>>+#include <linux/string.h>
>>>+#include <linux/xarray.h>
>>
>>This test is not using xarray.

it actually is. See the uses of xa_for_each

thanks
Lucas De Marchi