[Intel-xe] [PATCH 1/5] drm/xe: Introduce new selftests framework
Mauro Carvalho Chehab
mauro.chehab at linux.intel.com
Fri Mar 17 14:29:36 UTC 2023
On Fri, 17 Mar 2023 08:20:46 +0100
Thomas Hellström <thomas.hellstrom at linux.intel.com> wrote:
> Hi,
>
> On 3/16/23 07:46, Lucas De Marchi wrote:
> > On Mon, Mar 13, 2023 at 07:18:34PM +0100, Michal Wajdeczko wrote:
> >>
> >>
> >> On 13.03.2023 17:23, Lucas De Marchi wrote:
> >>> 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've added that just to simplify the code logic as I assumed we will be
> >> running mock tests only in "true kunit" fashion as part of the build.
> >>
> >> So then we could avoid mixing mock/live "kunit suites" in single driver
> >> module, especially that as of today there is no nice support to filter
> >> out undesired suites from the execution. Thus adding compile time flag
> >> to only have one category of selftests at time was looking promising.
> >
> > I'm using this in the patches I'm sending to unit test rtp and wa parts:
> >
> > ./tools/testing/kunit/kunit.py run --raw_output \
> > --kunitconfig drivers/gpu/drm/xe/.kunitconfig xe_rtp
> >
> >>
> >> So I was using .kunitconfig with
> >> CONFIG_DRM_XE_SELFTESTS_MOCK=y
> >> and normal .config with
> >> CONFIG_DRM_XE_SELFTESTS_LIVE=y
> >>
> >>>
> >>> 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.
> >>
> >> And I guess you still can do that, just it wont be automatically
> >> compiled out, nor you be able to benefit from the helpers, but they will
> >> be executed
> >>
> >>>
> >>> Also Cc Thomas as original author of the kunit integration.
> >>>
> >>> We also have some overloaded terms that need better explanation:
> >>> mock, selftest, live.
> >>
> >> I reused terms from i915, assumed everyone is familiar with that.
> >>
> >> "mock selftest" - a kunit test suite to be run on abstract xe device
> >> "live selftest" - a kunit test suite to be executed on real xe device
> >
> > Things I like on the current kunit integration we have in xe:
> >
> > 1) The ability to move unit test code out to a separate module
> > 2) Not having a big infra that is xe-only
> > 3) Share terminology with the rest of drm subsystem
> >
> > This patch series is basically killing (1) and (3). And for (2) it seems
> > to be heading another direction. Looking at the current available
> > drivers, I think we should be more like the drm tests and the vc4 tests
> > rather than the i915 ones. If we need more helpers we may probably
> > extend the ones in DRM_KUNIT_TEST_HELPERS
> >
> > I'd love to hear others' opinions though.
>
> I basically agree with Lucas here. Some points:
>
> * For mock selftests IMO we don't need anything on top of what we
> already have, except perhaps consolidate initialization of a mock
> device as a helper each test can optionally use. I think, though,
> that Mauro's suggestion of coalescing multiple mock tests together
> in a single separate kernel module with multiple test suites to
> avoid too many separate kernel module might be worth considering if
> needed.
IMO there's one important feature here that it is missing upstream:
support for filtering out KUnit tests.
Yet, the patch is adding it inside Xe. The right solution is, IMO, to
add support as much as possible inside the KUnit core and having
additional variables inside the module(s) implementing KUnit to setup
such filters.
This is more important for live selftests, but having this also for
mock won't hurt
> * For live selftests it seems like where the big disconnect is. I
> still believe that we should have separate modules, and depending on
> how the IGT runner end up working have enough information in that
> module for IGT to execute the tests with the --run-subtest and
> --dynamic-subtest options.
KUnit patch series is currently in rev. 5. It sounds almost ready for
being merged. In any case, to properly support dynamic subtests, we
need filtering capabilities. Once the filters get added, having multiple
test suites on the same module or on different modules won't make any
difference for IGT.
Regards,
Mauro
More information about the Intel-xe
mailing list