[Intel-gfx] [PATCH 05/46] drm/i915: Provide a hook for selftests

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Fri Feb 10 10:19:01 UTC 2017


On 02/02/2017 09:08, Chris Wilson wrote:
> Some pieces of code are independent of hardware but are very tricky to
> exercise through the normal userspace ABI or via debugfs hooks. Being
> able to create mock unit tests and execute them through CI is vital.
> Start by adding a central point where we can execute unit tests and
> a parameter to enable them. This is disabled by default as the
> expectation is that these tests will occasionally explode.
>
> To facilitate integration with igt, any parameter beginning with
> i915.igt__ is interpreted as a subtest executable independently via
> igt/drv_selftest.
>
> Two classes of selftests are recognised: mock unit tests and integration
> tests. Mock unit tests are run as soon as the module is loaded, before
> the device is probed. At that point there is no driver instantiated and
> all hw interactions must be "mocked". This is very useful for writing
> universal tests to exercise code not typically run on a broad range of
> architectures. Alternatively, you can hook into the live selftests and
> run when the device has been instantiated - hw interactions are real.
>
> v2: Add a macro for compiling conditional code for mock objects inside
> real objects.
> v3: Differentiate between mock unit tests and late integration test.
> v4: List the tests in natural order, use igt to sort after modparam.
> v5: s/late/live/
> v6: s/unsigned long/unsigned int/
> v7: Use igt_ prefixes for long helpers.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com> #v1
> ---
>  drivers/gpu/drm/i915/Kconfig.debug                 |  16 ++
>  drivers/gpu/drm/i915/Makefile                      |   3 +
>  drivers/gpu/drm/i915/i915_pci.c                    |  31 ++-
>  drivers/gpu/drm/i915/i915_selftest.h               | 102 +++++++++
>  .../gpu/drm/i915/selftests/i915_live_selftests.h   |  11 +
>  .../gpu/drm/i915/selftests/i915_mock_selftests.h   |  11 +
>  drivers/gpu/drm/i915/selftests/i915_random.c       |  63 ++++++
>  drivers/gpu/drm/i915/selftests/i915_random.h       |  50 +++++
>  drivers/gpu/drm/i915/selftests/i915_selftest.c     | 250 +++++++++++++++++++++
>  tools/testing/selftests/drivers/gpu/i915.sh        |   1 +
>  10 files changed, 531 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_selftest.h
>  create mode 100644 drivers/gpu/drm/i915/selftests/i915_live_selftests.h
>  create mode 100644 drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
>  create mode 100644 drivers/gpu/drm/i915/selftests/i915_random.c
>  create mode 100644 drivers/gpu/drm/i915/selftests/i915_random.h
>  create mode 100644 drivers/gpu/drm/i915/selftests/i915_selftest.c
>
> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> index 598551dbf62c..a4d8cfd77c3c 100644
> --- a/drivers/gpu/drm/i915/Kconfig.debug
> +++ b/drivers/gpu/drm/i915/Kconfig.debug
> @@ -26,6 +26,7 @@ config DRM_I915_DEBUG
>          select DRM_DEBUG_MM if DRM=y
>  	select DRM_DEBUG_MM_SELFTEST
>  	select DRM_I915_SW_FENCE_DEBUG_OBJECTS
> +	select DRM_I915_SELFTEST
>          default n
>          help
>            Choose this option to turn on extra driver debugging that may affect
> @@ -59,3 +60,18 @@ config DRM_I915_SW_FENCE_DEBUG_OBJECTS
>            Recommended for driver developers only.
>
>            If in doubt, say "N".
> +
> +config DRM_I915_SELFTEST
> +	bool "Enable selftests upon driver load"
> +	depends on DRM_I915
> +	default n
> +	select PRIME_NUMBERS
> +	help
> +	  Choose this option to allow the driver to perform selftests upon
> +	  loading; also requires the i915.selftest=1 module parameter. To
> +	  exit the module after running the selftests (i.e. to prevent normal
> +	  module initialisation afterwards) use i915.selftest=-1.
> +
> +	  Recommended for driver developers only.
> +
> +	  If in doubt, say "N".
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index c62ab45683c0..bac62fd5b438 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -116,6 +116,9 @@ i915-y += dvo_ch7017.o \
>
>  # Post-mortem debug and GPU hang state capture
>  i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
> +i915-$(CONFIG_DRM_I915_SELFTEST) += \
> +	selftests/i915_random.o \
> +	selftests/i915_selftest.o
>
>  # virtual gpu code
>  i915-y += i915_vgpu.o
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index df2051b41fa1..732101ed57fb 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -27,6 +27,7 @@
>  #include <linux/vga_switcheroo.h>
>
>  #include "i915_drv.h"
> +#include "i915_selftest.h"
>
>  #define GEN_DEFAULT_PIPEOFFSETS \
>  	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> @@ -473,10 +474,19 @@ static const struct pci_device_id pciidlist[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, pciidlist);
>
> +static void i915_pci_remove(struct pci_dev *pdev)
> +{
> +	struct drm_device *dev = pci_get_drvdata(pdev);
> +
> +	i915_driver_unload(dev);
> +	drm_dev_unref(dev);
> +}
> +
>  static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	struct intel_device_info *intel_info =
>  		(struct intel_device_info *) ent->driver_data;
> +	int err;
>
>  	if (IS_ALPHA_SUPPORT(intel_info) && !i915.alpha_support) {
>  		DRM_INFO("The driver support for your hardware in this kernel version is alpha quality\n"
> @@ -500,15 +510,17 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (vga_switcheroo_client_probe_defer(pdev))
>  		return -EPROBE_DEFER;
>
> -	return i915_driver_load(pdev, ent);
> -}
> +	err = i915_driver_load(pdev, ent);
> +	if (err)
> +		return err;
>
> -static void i915_pci_remove(struct pci_dev *pdev)
> -{
> -	struct drm_device *dev = pci_get_drvdata(pdev);
> +	err = i915_live_selftests(pdev);
> +	if (err) {
> +		i915_pci_remove(pdev);
> +		return err > 0 ? -ENOTTY : err;
> +	}
>
> -	i915_driver_unload(dev);
> -	drm_dev_unref(dev);
> +	return 0;
>  }
>
>  static struct pci_driver i915_pci_driver = {
> @@ -522,6 +534,11 @@ static struct pci_driver i915_pci_driver = {
>  static int __init i915_init(void)
>  {
>  	bool use_kms = true;
> +	int err;
> +
> +	err = i915_mock_selftests();
> +	if (err)
> +		return err > 0 ? 0 : err;
>
>  	/*
>  	 * Enable KMS by default, unless explicitly overriden by
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> new file mode 100644
> index 000000000000..8b5994caa301
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -0,0 +1,102 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#ifndef __I915_SELFTEST_H__
> +#define __I915_SELFTEST_H__
> +
> +struct pci_dev;
> +struct drm_i915_private;
> +
> +struct i915_selftest {
> +	unsigned long timeout_jiffies;
> +	unsigned int timeout_ms;
> +	unsigned int random_seed;
> +	int mock;
> +	int live;
> +};
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +extern struct i915_selftest i915_selftest;
> +
> +int i915_mock_selftests(void);
> +int i915_live_selftests(struct pci_dev *pdev);
> +
> +/* We extract the function declarations from i915_mock_selftests.h and
> + * i915_live_selftests.h Add your unit test declarations there!
> + *
> + * Mock unit tests are run very early upon module load, before the driver
> + * is probed. All hardware interactions, as well as other subsystems, must
> + * be "mocked".
> + *
> + * Live unit tests are run after the driver is loaded - all hardware
> + * interactions are real.
> + */
> +#define selftest(name, func) int func(void);
> +#include "selftests/i915_mock_selftests.h"
> +#undef selftest
> +#define selftest(name, func) int func(struct drm_i915_private *i915);
> +#include "selftests/i915_live_selftests.h"
> +#undef selftest
> +
> +struct i915_subtest {
> +	int (*func)(void *data);
> +	const char *name;
> +};
> +
> +int __i915_subtests(const char *caller,
> +		    const struct i915_subtest *st,
> +		    unsigned int count,
> +		    void *data);
> +#define i915_subtests(T, data) \
> +	__i915_subtests(__func__, T, ARRAY_SIZE(T), data)
> +
> +#define SUBTEST(x) { x, #x }
> +
> +#define I915_SELFTEST_DECLARE(x) x
> +#define I915_SELFTEST_ONLY(x) unlikely(x)
> +
> +#else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
> +
> +static inline int i915_mock_selftests(void) { return 0; }
> +static inline int i915_live_selftests(struct pci_dev *pdev) { return 0; }
> +
> +#define I915_SELFTEST_DECLARE(x)
> +#define I915_SELFTEST_ONLY(x) 0
> +
> +#endif
> +
> +/* Using the i915_selftest_ prefix becomes a little unwieldy with the helpers.
> + * Instead we use the igt_ shorthand, in reference to the intel-gpu-tools
> + * suite of uabi test cases (which includes a test runner for our selftests).
> + */
> +
> +#define IGT_TIMEOUT(name__) \
> +	unsigned long name__ = jiffies + i915_selftest.timeout_jiffies
> +
> +__printf(2, 3)
> +bool __igt_timeout(unsigned long timeout, const char *fmt, ...);
> +
> +#define igt_timeout(t, fmt, ...) \
> +	__igt_timeout((t), KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> +
> +#endif /* !__I915_SELFTEST_H__ */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> new file mode 100644
> index 000000000000..f3e17cb10e05
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> @@ -0,0 +1,11 @@
> +/* List each unit test as selftest(name, function)
> + *
> + * The name is used as both an enum and expanded as subtest__name to create
> + * a module parameter. It must be unique and legal for a C identifier.
> + *
> + * The function should be of type int function(void). It may be conditionally
> + * compiled using #if IS_ENABLED(DRM_I915_SELFTEST).
> + *
> + * Tests are executed in order by igt/drv_selftest
> + */
> +selftest(sanitycheck, i915_live_sanitycheck) /* keep first (igt selfcheck) */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> new file mode 100644
> index 000000000000..69e97a2ba4a6
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> @@ -0,0 +1,11 @@
> +/* List each unit test as selftest(name, function)
> + *
> + * The name is used as both an enum and expanded as subtest__name to create
> + * a module parameter. It must be unique and legal for a C identifier.
> + *
> + * The function should be of type int function(void). It may be conditionally
> + * compiled using #if IS_ENABLED(DRM_I915_SELFTEST).
> + *
> + * Tests are executed in order by igt/drv_selftest
> + */
> +selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt selfcheck) */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_random.c b/drivers/gpu/drm/i915/selftests/i915_random.c
> new file mode 100644
> index 000000000000..606a237fed17
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/i915_random.c
> @@ -0,0 +1,63 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/kernel.h>
> +#include <linux/random.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include "i915_random.h"
> +
> +static inline u32 i915_prandom_u32_max_state(u32 ep_ro, struct rnd_state *state)
> +{
> +	return upper_32_bits((u64)prandom_u32_state(state) * ep_ro);

What is ep_ro?

> +}
> +
> +void i915_random_reorder(unsigned int *order, unsigned int count,
> +			 struct rnd_state *state)
> +{
> +	unsigned int i, j;
> +
> +	for (i = 0; i < count; ++i) {
> +		BUILD_BUG_ON(sizeof(unsigned int) > sizeof(u32));

? :)

> +		j = i915_prandom_u32_max_state(count, state);
> +		swap(order[i], order[j]);
> +	}
> +}
> +
> +unsigned int *i915_random_order(unsigned int count, struct rnd_state *state)
> +{
> +	unsigned int *order, i;
> +
> +	order = kmalloc_array(count, sizeof(*order), GFP_TEMPORARY);
> +	if (!order)
> +		return order;
> +
> +	for (i = 0; i < count; i++)
> +		order[i] = i;
> +
> +	i915_random_reorder(order, count, state);
> +	return order;
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/i915_random.h b/drivers/gpu/drm/i915/selftests/i915_random.h
> new file mode 100644
> index 000000000000..b9c334ce6cd9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/i915_random.h
> @@ -0,0 +1,50 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef __I915_SELFTESTS_RANDOM_H__
> +#define __I915_SELFTESTS_RANDOM_H__
> +
> +#include <linux/random.h>
> +
> +#include "../i915_selftest.h"
> +
> +#define I915_RND_STATE_INITIALIZER(x) ({				\
> +	struct rnd_state state__;					\
> +	prandom_seed_state(&state__, (x));				\
> +	state__;							\
> +})
> +
> +#define I915_RND_STATE(name__) \
> +	struct rnd_state name__ = I915_RND_STATE_INITIALIZER(i915_selftest.random_seed)
> +
> +#define I915_RND_SUBSTATE(name__, parent__) \
> +	struct rnd_state name__ = I915_RND_STATE_INITIALIZER(prandom_u32_state(&(parent__)))
> +
> +unsigned int *i915_random_order(unsigned int count,
> +				struct rnd_state *state);
> +void i915_random_reorder(unsigned int *order,
> +			 unsigned int count,
> +			 struct rnd_state *state);
> +
> +#endif /* !__I915_SELFTESTS_RANDOM_H__ */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> new file mode 100644
> index 000000000000..6ba3abb10c6f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -0,0 +1,250 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include <linux/random.h>
> +
> +#include "../i915_drv.h"
> +#include "../i915_selftest.h"
> +
> +struct i915_selftest i915_selftest __read_mostly = {
> +	.timeout_ms = 1000,
> +};
> +
> +int i915_mock_sanitycheck(void)
> +{
> +	pr_info(DRIVER_NAME ": %s() - ok!\n", __func__);
> +	return 0;
> +}
> +
> +int i915_live_sanitycheck(struct drm_i915_private *i915)
> +{
> +	pr_info("%s: %s() - ok!\n", i915->drm.driver->name, __func__);
> +	return 0;
> +}
> +
> +enum {
> +#define selftest(name, func) mock_##name,
> +#include "i915_mock_selftests.h"
> +#undef selftest
> +};
> +
> +enum {
> +#define selftest(name, func) live_##name,
> +#include "i915_live_selftests.h"
> +#undef selftest
> +};
> +
> +struct selftest {
> +	bool enabled;
> +	const char *name;
> +	union {
> +		int (*mock)(void);
> +		int (*live)(struct drm_i915_private *);
> +	};
> +};
> +
> +#define selftest(n, f) [mock_##n] = { .name = #n, .mock = f },
> +static struct selftest mock_selftests[] = {
> +#include "i915_mock_selftests.h"
> +};
> +#undef selftest
> +
> +#define selftest(n, f) [live_##n] = { .name = #n, .live = f },
> +static struct selftest live_selftests[] = {
> +#include "i915_live_selftests.h"
> +};
> +#undef selftest
> +
> +/* Embed the line number into the parameter name so that we can order tests */
> +#define selftest(n, func) selftest_0(n, func, param(n))
> +#define param(n) __PASTE(igt__, __PASTE(__LINE__, __mock_##n))
> +#define selftest_0(n, func, id) \
> +module_param_named(id, mock_selftests[mock_##n].enabled, bool, 0400);
> +#include "i915_mock_selftests.h"
> +#undef selftest_0
> +#undef param
> +
> +#define param(n) __PASTE(igt__, __PASTE(__LINE__, __live_##n))
> +#define selftest_0(n, func, id) \
> +module_param_named(id, live_selftests[live_##n].enabled, bool, 0400);
> +#include "i915_live_selftests.h"
> +#undef selftest_0
> +#undef param
> +#undef selftest
> +
> +static void set_default_test_all(struct selftest *st, unsigned int count)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < count; i++)
> +		if (st[i].enabled)
> +			return;
> +
> +	for (i = 0; i < count; i++)
> +		st[i].enabled = true;
> +}
> +
> +static int __run_selftests(const char *name,
> +			   struct selftest *st,
> +			   unsigned int count,
> +			   void *data)
> +{
> +	int err = 0;
> +
> +	while (!i915_selftest.random_seed)
> +		i915_selftest.random_seed = get_random_int();
> +
> +	i915_selftest.timeout_jiffies =
> +		i915_selftest.timeout_ms ?
> +		msecs_to_jiffies_timeout(i915_selftest.timeout_ms) :
> +		MAX_SCHEDULE_TIMEOUT;
> +
> +	set_default_test_all(st, count);
> +
> +	pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x st_timeout=%u\n",
> +		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
> +
> +	/* Tests are listed in order in i915_*_selftests.h */
> +	for (; count--; st++) {
> +		if (!st->enabled)
> +			continue;
> +
> +		cond_resched();
> +		if (signal_pending(current))
> +			return -EINTR;
> +
> +		pr_debug(DRIVER_NAME ": Running %s\n", st->name);
> +		if (data)
> +			err = st->live(data);
> +		else
> +			err = st->mock();
> +		if (err == -EINTR && !signal_pending(current))

What is the expected use for this?

> +			err = 0;
> +		if (err)
> +			break;
> +	}
> +
> +	if (WARN(err > 0 || err == -ENOTTY,
> +		 "%s returned %d, conflicting with selftest's magic values!\n",
> +		 st->name, err))
> +		err = -1;
> +
> +	return err;
> +}
> +
> +#define run_selftests(x, data) \
> +	__run_selftests(#x, x##_selftests, ARRAY_SIZE(x##_selftests), data)
> +
> +int i915_mock_selftests(void)
> +{
> +	int err;
> +
> +	if (!i915_selftest.mock)
> +		return 0;
> +
> +	err = run_selftests(mock, NULL);
> +	if (err) {
> +		i915_selftest.mock = err;
> +		return err;
> +	}
> +
> +	if (i915_selftest.mock < 0) {
> +		i915_selftest.mock = -ENOTTY;
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +int i915_live_selftests(struct pci_dev *pdev)
> +{
> +	int err;
> +
> +	if (!i915_selftest.live)
> +		return 0;
> +
> +	err = run_selftests(live, to_i915(pci_get_drvdata(pdev)));
> +	if (err) {
> +		i915_selftest.live = err;
> +		return err;
> +	}
> +
> +	if (i915_selftest.live < 0) {
> +		i915_selftest.live = -ENOTTY;
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +int __i915_subtests(const char *caller,
> +		    const struct i915_subtest *st,
> +		    unsigned int count,
> +		    void *data)
> +{
> +	int err;
> +
> +	for (; count--; st++) {
> +		cond_resched();
> +		if (signal_pending(current))
> +			return -EINTR;
> +
> +		pr_debug(DRIVER_NAME ": Running %s/%s\n", caller, st->name);
> +		err = st->func(data);
> +		if (err && err != -EINTR) {
> +			pr_err(DRIVER_NAME "/%s: %s failed with error %d\n",
> +			       caller, st->name, err);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +bool __igt_timeout(unsigned long timeout, const char *fmt, ...)
> +{
> +	va_list va;
> +
> +	if (!signal_pending(current)) {
> +		cond_resched();
> +		if (time_before(jiffies, timeout))
> +			return false;
> +	}
> +
> +	if (fmt) {
> +		va_start(va, fmt);
> +		vprintk(fmt, va);
> +		va_end(va);
> +	}
> +
> +	return true;
> +}
> +
> +module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
> +module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);

I think selftest_ prefix would be better to match the ones below and 
just in general.

> +
> +module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
> +MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then exit module)");
> +
> +module_param_named_unsafe(live_selftests, i915_selftest.live, int, 0400);
> +MODULE_PARM_DESC(live_selftests, "Run selftests after driver initialisation on the live system (0:disabled [default], 1:run tests then continue, -1:run tests then exit module)");
> diff --git a/tools/testing/selftests/drivers/gpu/i915.sh b/tools/testing/selftests/drivers/gpu/i915.sh
> index d407f0fa1e3a..c06d6e8a8dcc 100755
> --- a/tools/testing/selftests/drivers/gpu/i915.sh
> +++ b/tools/testing/selftests/drivers/gpu/i915.sh
> @@ -7,6 +7,7 @@ if ! /sbin/modprobe -q -r i915; then
>  fi
>
>  if /sbin/modprobe -q i915 mock_selftests=-1; then
> +	/sbin/modprobe -q -r i915
>  	echo "drivers/gpu/i915: ok"
>  else
>  	echo "drivers/gpu/i915: [FAIL]"
>

No serious complaints. Bikeshed or not (but preferred for modparams):

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko


More information about the Intel-gfx mailing list