[Intel-xe] [PATCH 1/5] drm/xe: Introduce new selftests framework

[Lucas De Marchi]

On Mon, Mar 13, 2023 at 09:13:12AM +0100, Michal Wajdeczko wrote:
>Our initial attempt to run Kunit based tests on live devices is hard
>to use and will not work well with multiple devices attached.
>
>Instead of relying on separate test module and duplicating support
>for multiple live devices in every test entry point, create new set
>of helpers and use test->priv to pass reference of the device under
>test (DUT) directly to every test function.
>
>The same use pattern is supported for both mock and live selftests.
>
>By using bus_notifier we can start testing each new bound device
>separatelly, subject to filtering using dut_filter modparam.
>
>All test cases can be included directly from .c file, allowing test
>code to access and use any private functions.
>
>As Kunit will attempt to run all suites on module load, including
>our new live test suites, we may want to allow to execute them on
>any available device. This is controlled by anonymous_dut modparam.
>
>Support for either mock or live selftests, only one at time, is
>controlled by new Kconfig options.
>
>Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>---
> drivers/gpu/drm/xe/Kconfig.debug              |  22 ++++
> drivers/gpu/drm/xe/Makefile                   |  11 ++
> drivers/gpu/drm/xe/selftests/xe_selftests.h   |  35 ++++++
> .../gpu/drm/xe/selftests/xe_selftests_auto.c  |  46 +++++++
> .../drm/xe/selftests/xe_selftests_executor.c  | 118 ++++++++++++++++++
> .../drm/xe/selftests/xe_selftests_helpers.c   | 101 +++++++++++++++
> .../drm/xe/selftests/xe_selftests_internal.h  |  22 ++++
> .../gpu/drm/xe/selftests/xe_selftests_mock.c  |  32 +++++
> .../drm/xe/selftests/xe_selftests_params.c    |  23 ++++
> drivers/gpu/drm/xe/xe_module.c                |   2 +
> 10 files changed, 412 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests.h
> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_auto.c
> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_executor.c
> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_helpers.c
> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_internal.h
> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_mock.c
> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_params.c
>
>diff --git a/drivers/gpu/drm/xe/Kconfig.debug b/drivers/gpu/drm/xe/Kconfig.debug
>index 93b284cdd0a2..aeba43977dca 100644
>--- a/drivers/gpu/drm/xe/Kconfig.debug
>+++ b/drivers/gpu/drm/xe/Kconfig.debug
>@@ -61,6 +61,28 @@ config DRM_XE_SIMPLE_ERROR_CAPTURE
>
> 	  If in doubt, say "N".
>
>+config DRM_XE_SELFTESTS_MOCK
>+	bool "Mock selftests for Xe" if !KUNIT_ALL_TESTS
>+	depends on DRM_XE && KUNIT
>+	default KUNIT_ALL_TESTS
>+	help
>+		Choose this option to allow the driver to perform mock
>+		selftests based on the Kunit framework.
>+
>+		Recommended for driver developers only.
>+		If in doubt, say "N".
>+
>+config DRM_XE_SELFTESTS_LIVE
>+	bool "Live selftests for Xe" if !DRM_XE_SELFTESTS_MOCK

why?  I think we should aim to run live tests in *addition* to
mock ones.  The mock tests are more testing our SW infra and can be used
in a true "unit test" sense. While the live ones are testing the
integration with the HW and making sure they work together.

I have changes pending to test some WA and RTP infra we added,
regardless of the HW. But I don't want to have to choose either one or
the other.

Also Cc Thomas as original author of the kunit integration.

We also have some overloaded terms that need better explanation:
mock, selftest, live.

>+	depends on DRM_XE && KUNIT && EXPERT
>+	default n
>+	help
>+		Choose this option to allow the driver to perform live
>+		selftests based on the Kunit framework.
>+
>+		Recommended for driver developers only.
>+		If in doubt, say "N".
>+
> config DRM_XE_KUNIT_TEST
>         tristate "KUnit tests for the drm xe driver" if !KUNIT_ALL_TESTS
> 	depends on DRM_XE && KUNIT && DEBUG_FS
>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>index b94e65c91101..b193af2d9930 100644
>--- a/drivers/gpu/drm/xe/Makefile
>+++ b/drivers/gpu/drm/xe/Makefile
>@@ -211,6 +211,17 @@ ifeq ($(CONFIG_DRM_FBDEV_EMULATION),y)
> 	xe-$(CONFIG_DRM_XE_DISPLAY) += display/intel_fbdev.o
> endif
>
>+xe-$(CONFIG_DRM_XE_SELFTESTS_MOCK) += \
>+	selftests/xe_selftests_mock.o \
>+
>+xe-$(CONFIG_DRM_XE_SELFTESTS_LIVE) += \
>+	selftests/xe_selftests_auto.o \
>+	selftests/xe_selftests_executor.o \
>+	selftests/xe_selftests_params.o \
>+
>+xe-$(CONFIG_KUNIT) += \
>+	selftests/xe_selftests_helpers.o \
>+
> obj-$(CONFIG_DRM_XE) += xe.o
> obj-$(CONFIG_DRM_XE_KUNIT_TEST) += tests/
>
>diff --git a/drivers/gpu/drm/xe/selftests/xe_selftests.h b/drivers/gpu/drm/xe/selftests/xe_selftests.h
>new file mode 100644
>index 000000000000..6b13b998e789
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/selftests/xe_selftests.h
>@@ -0,0 +1,35 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2023 Intel Corporation
>+ */
>+
>+#ifndef _XE_SELFTESTS_H_
>+#define _XE_SELFTESTS_H_
>+
>+struct kunit_suite;
>+struct kunit;
>+
>+int xe_selftests_run(const char *dev_name);
>+
>+#if IS_ENABLED(CONFIG_DRM_XE_SELFTESTS_LIVE)
>+int init_xe_live_selftest_suite(struct kunit_suite *suite);
>+#endif
>+
>+int init_xe_device_selftest(struct kunit *test);
>+void exit_xe_device_selftest(struct kunit *test);
>+
>+int init_xe_gt_selftest(struct kunit *test);
>+void exit_xe_gt_selftest(struct kunit *test);
>+
>+int init_xe_guc_selftest(struct kunit *test);
>+void exit_xe_guc_selftest(struct kunit *test);
>+
>+#if IS_ENABLED(CONFIG_DRM_XE_SELFTESTS_LIVE)
>+int xe_selftests_module_init(void);
>+void xe_selftests_module_exit(void);
>+#else
>+static inline int xe_selftests_module_init(void) { return 0; }
>+static inline void xe_selftests_module_exit(void) { }
>+#endif
>+
>+#endif /* _XE_SELFTESTS_H_ */
>diff --git a/drivers/gpu/drm/xe/selftests/xe_selftests_auto.c b/drivers/gpu/drm/xe/selftests/xe_selftests_auto.c
>new file mode 100644
>index 000000000000..ef98f3599eac
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/selftests/xe_selftests_auto.c
>@@ -0,0 +1,46 @@
>+// SPDX-License-Identifier: MIT
>+/*
>+ * Copyright © 2023 Intel Corporation
>+ */
>+
>+#include <linux/device.h>
>+#include <linux/glob.h>
>+#include <linux/pci.h>
>+
>+#include "xe_drv.h"
>+#include "selftests/xe_selftests.h"
>+#include "selftests/xe_selftests_internal.h"
>+
>+static int bus_cb(struct notifier_block *nb, unsigned long action, void *data)
>+{
>+	struct device *dev = data;
>+	struct device_driver *drv = dev->driver;
>+
>+	if (action != BUS_NOTIFY_BOUND_DRIVER)
>+		return 0;
>+
>+	if (strcmp(DRIVER_NAME, drv->name))
>+		return 0;
>+
>+	/* only matching DUT names will be tested */
>+	if (!dut_filter || !glob_match(dut_filter, dev_name(dev)))

"DUT" name could be avoided and just use what exactly it means.
When I see "DUT" I'm wondering... is this the platform name, the pci ID,
the pci slot, the device node?

Lucas De Marchi