On ma, 2016-03-14 at 16:59 +0200, Imre Deak wrote:
> Add support for forcing an error at selected places in the driver. As
> an
> example add 4 options to fail during driver loading.
> 
> Requested by Chris.
> 
> v2:
> - Add fault point for modeset initialization
> - Print debug message when injecting an error
> v3:
> - Rename inject_fault to inject_load_failure, rename the related
> macros
>   and helper accordingly (Chris)
> 
> CC: Chris Wilson <ch...@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.d...@intel.com>
> ---
> 
> [This depends on
>  https://lists.freedesktop.org/archives/intel-gfx/2016-March/089597.h
> tml]
> 
>  drivers/gpu/drm/i915/i915_dma.c    | 12 ++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h    | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/i915_params.c |  5 +++++
>  drivers/gpu/drm/i915/i915_params.h |  1 +
>  4 files changed, 34 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c
> index a5121cd..7490307 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -370,6 +370,9 @@ static int i915_load_modeset_init(struct
> drm_device *dev)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       int ret;
>  
> +     if (i915_inject_load_failure(I915_FAIL_INIT_MODESET))
> +             return -ENODEV;
> +
>       ret = intel_bios_init(dev_priv);
>       if (ret)
>               DRM_INFO("failed to find VBIOS tables\n");
> @@ -950,6 +953,9 @@ static int i915_driver_init_early(struct
> drm_i915_private *dev_priv,
>       struct intel_device_info *device_info;
>       int ret = 0;
>  
> +     if (i915_inject_load_failure(I915_FAIL_INIT_EARLY))
> +             return -ENODEV;
> +
>       dev_priv->dev = dev;
>  
>       /* Setup the write-once "constant" device info */
> @@ -1064,6 +1070,9 @@ static int i915_driver_init_mmio(struct
> drm_i915_private *dev_priv)
>       struct drm_device *dev = dev_priv->dev;
>       int ret;
>  
> +     if (i915_inject_load_failure(I915_FAIL_INIT_MMIO))
> +             return -ENODEV;
> +
>       if (i915_get_bridge_dev(dev))
>               return -EIO;
>  
> @@ -1107,6 +1116,9 @@ static int i915_driver_init_hw(struct
> drm_i915_private *dev_priv)
>       uint32_t aperture_size;
>       int ret;
>  
> +     if (i915_inject_load_failure(I915_FAIL_INIT_HW))
> +             return -ENODEV;
> +
>       intel_device_info_runtime_init(dev);
>  
>       ret = i915_gem_gtt_init(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 25274e1..e2d21d5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -98,6 +98,22 @@
>  #define I915_STATE_WARN_ON(x)                                        
>       \
>       I915_STATE_WARN((x), "%s", "WARN_ON(" __stringify(x) ")")
>  
> +#define I915_FAIL_INIT_EARLY BIT(0)
> +#define I915_FAIL_INIT_MMIO  BIT(1)
> +#define I915_FAIL_INIT_HW    BIT(2)
> +#define I915_FAIL_INIT_MODESET       BIT(3)
> +
> +static inline bool i915_inject_load_failure(unsigned int fail_mask)
> +{
> +     if (i915.inject_load_failure & fail_mask) {
> +             DRM_DEBUG_DRIVER("Injecting failure %08x\n",
> fail_mask);
> +
> +             return true;
> +     }
> +
> +     return false;
> +}
> +
>  static inline const char *yesno(bool v)
>  {
>       return v ? "yes" : "no";
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 278c9c4..4faeeed 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -56,6 +56,7 @@ struct i915_params i915 __read_mostly = {
>       .edp_vswing = 0,
>       .enable_guc_submission = false,
>       .guc_log_level = -1,
> +     .inject_load_failure = 0,
>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -201,3 +202,7 @@ MODULE_PARM_DESC(enable_guc_submission, "Enable
> GuC submission (default:false)")
>  module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
>  MODULE_PARM_DESC(guc_log_level,
>       "GuC firmware logging level (-1:disabled (default), 0-
> 3:enabled)");
> +
> +module_param_named(inject_load_failure, i915.inject_load_failure,
> uint, 0600);

This most definitely should be module_param_named_unsafe.

I think I'd also hope to see some kind of compile time option to enable
this. I don't think average user wants this by default.

> +MODULE_PARM_DESC(inject_load_failure,
> +     "Force an error at selected points (0:disabled,
> 0x1:INIT_EARLY, 0x2:INIT_MMIO, 0x4:INIT_HW, 0x8:INIT_MODESET)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h
> b/drivers/gpu/drm/i915/i915_params.h
> index bd5026b..b691026 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -59,6 +59,7 @@ struct i915_params {
>       bool enable_guc_submission;
>       bool verbose_state_checks;
>       bool nuclear_pageflip;
> +     unsigned int inject_load_failure;

Duh, add it above the bools, this struct was cleaned once already :P

Regards, Joonas

>  };
>  
>  extern struct i915_params i915 __read_mostly;
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to