[Intel-gfx] [PATCH 5/6] drm/i915: move load failure injection to selftests
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Mon Dec 31 13:13:26 UTC 2018
On 27/12/2018 14:33, Jani Nikula wrote:
> Seems like selftests is a better home for everything related to load
> failure injection, including the module parameter.
Hm not sure I would immediately want to couple the two, however..
> The failure injection code gets moved from under DRM_I915_DEBUG to
> DRM_I915_SELFTEST config option. This should be of no consequence, as
> the former selects the latter.
... I also don't see why debug would auto-select self-tests.
So don't know. Personally I'd have selftests separate from debug, and
load failure injection separate from selftests. But I don't feel that
strongly about it. At least I agree that losing failure injection from
the error state is not an issue. (Since all of them are strictly during
driver load phase.)
Regards,
Tvrtko
>
> With the parameter no longer part of i915_params, its value will not be
> recorded in error capture or debugfs param dump. Note that the value
> would have been zeroed anyway if a selftest had been hit, so there
> should not be meaningful information loss here, especially with all the
> logging around failure injection.
>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 25 -------------------------
> drivers/gpu/drm/i915/i915_drv.h | 15 ---------------
> drivers/gpu/drm/i915/i915_params.c | 5 -----
> drivers/gpu/drm/i915/i915_params.h | 1 -
> drivers/gpu/drm/i915/i915_selftest.h | 10 ++++++++++
> drivers/gpu/drm/i915/selftests/i915_selftest.c | 26 ++++++++++++++++++++++++++
> 6 files changed, 36 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index caa055ac9472..559a1b11f3e4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -57,31 +57,6 @@
>
> static struct drm_driver driver;
>
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
> -static unsigned int i915_load_fail_count;
> -
> -bool __i915_inject_load_failure(const char *func, int line)
> -{
> - if (i915_load_fail_count >= i915_modparams.inject_load_failure)
> - return false;
> -
> - if (++i915_load_fail_count == i915_modparams.inject_load_failure) {
> - DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
> - i915_modparams.inject_load_failure, func, line);
> - i915_modparams.inject_load_failure = 0;
> - return true;
> - }
> -
> - return false;
> -}
> -
> -bool i915_error_injected(void)
> -{
> - return i915_load_fail_count && !i915_modparams.inject_load_failure;
> -}
> -
> -#endif
> -
> #define FDO_BUG_URL "https://bugs.freedesktop.org/enter_bug.cgi?product=DRI"
> #define FDO_BUG_MSG "Please file a bug at " FDO_BUG_URL " against DRM/Intel " \
> "providing the dmesg log by booting with drm.debug=0xf"
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c60cf17e50a0..1f29f3933625 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -111,21 +111,6 @@
> #define I915_STATE_WARN_ON(x) \
> I915_STATE_WARN((x), "%s", "WARN_ON(" __stringify(x) ")")
>
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
> -
> -bool __i915_inject_load_failure(const char *func, int line);
> -#define i915_inject_load_failure() \
> - __i915_inject_load_failure(__func__, __LINE__)
> -
> -bool i915_error_injected(void);
> -
> -#else
> -
> -#define i915_inject_load_failure() false
> -#define i915_error_injected() false
> -
> -#endif
> -
> #define i915_load_error(i915, fmt, ...) \
> __i915_printk(i915, i915_error_injected() ? KERN_DEBUG : KERN_ERR, \
> fmt, ##__VA_ARGS__)
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 9f0539bdaa39..2853b54570eb 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -159,11 +159,6 @@ i915_param_named_unsafe(dmc_firmware_path, charp, 0400,
> i915_param_named_unsafe(enable_dp_mst, bool, 0600,
> "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)");
>
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
> -i915_param_named_unsafe(inject_load_failure, uint, 0400,
> - "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)");
> -#endif
> -
> i915_param_named(enable_dpcd_backlight, bool, 0600,
> "Enable support for DPCD backlight control (default:false)");
>
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 93f665eced16..85a9007c0ed6 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -53,7 +53,6 @@ struct drm_printer;
> param(int, mmio_debug, 0) \
> param(int, edp_vswing, 0) \
> param(int, reset, 2) \
> - param(unsigned int, inject_load_failure, 0) \
> /* leave bools at the end to not create holes */ \
> param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
> param(bool, enable_hangcheck, true) \
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index a73472dd12fd..1266cef8bf17 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -33,6 +33,7 @@ struct i915_selftest {
> unsigned int random_seed;
> int mock;
> int live;
> + unsigned int inject_load_failure;
> };
>
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> @@ -77,6 +78,12 @@ int __i915_subtests(const char *caller,
> #define I915_SELFTEST_DECLARE(x) x
> #define I915_SELFTEST_ONLY(x) unlikely(x)
>
> +bool __i915_inject_load_failure(const char *func, int line);
> +#define i915_inject_load_failure() \
> + __i915_inject_load_failure(__func__, __LINE__)
> +
> +bool i915_error_injected(void);
> +
> #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
>
> static inline int i915_mock_selftests(void) { return 0; }
> @@ -85,6 +92,9 @@ static inline int i915_live_selftests(struct pci_dev *pdev) { return 0; }
> #define I915_SELFTEST_DECLARE(x)
> #define I915_SELFTEST_ONLY(x) 0
>
> +static inline int i915_inject_load_failure(void) { return false; }
> +static inline int i915_error_injected(void) { return false; }
> +
> #endif
>
> /* Using the i915_selftest_ prefix becomes a little unwieldy with the helpers.
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> index 86c54ea37f48..9e556fc4707d 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -250,3 +250,29 @@ MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardw
>
> 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)");
> +
> +module_param_named_unsafe(inject_load_failure, i915_selftest.inject_load_failure, uint, 0400);
> +MODULE_PARM_DESC(inject_load_failure,
> + "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)");
> +
> +static unsigned int i915_load_fail_count;
> +
> +bool __i915_inject_load_failure(const char *func, int line)
> +{
> + if (i915_load_fail_count >= i915_selftest.inject_load_failure)
> + return false;
> +
> + if (++i915_load_fail_count == i915_selftest.inject_load_failure) {
> + DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
> + i915_selftest.inject_load_failure, func, line);
> + i915_selftest.inject_load_failure = 0;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +bool i915_error_injected(void)
> +{
> + return i915_load_fail_count && !i915_selftest.inject_load_failure;
> +}
>
More information about the Intel-gfx
mailing list