Re: [Intel-gfx] [PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine

2019-09-27 Thread Chris Wilson
Quoting Chris Wilson (2019-09-27 20:14:42)
> -static reset_func intel_get_gpu_reset(struct drm_i915_private *i915)
> +static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
>  {
> -   if (INTEL_GEN(i915) >= 8)
> +   struct drm_i915_private *i915 = gt->i915;
> +
> +   if (is_mock_gt(gt))

Actually this highlights an issue with using gt->awake == -1 as our
indicator.

Hmm. I wonder...
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine

2019-09-27 Thread Chris Wilson
Quoting Andi Shyti (2019-09-27 21:41:19)
> Hi Chris,
> 
> On Fri, Sep 27, 2019 at 08:14:42PM +0100, Chris Wilson wrote:
> > For those mock tests that may wish to pretend triggering a GPU reset and
> > processing the cleanup.
> 
> The patch is OK, per se, but I think it should be split in two
> parts:
> 
>  - the i915 to gt conversion (that is the biggest part of the
>patch)
>  - the mock-reset part (baskically the function)
> 
> right?

But you read it all already...
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine

2019-09-27 Thread Andi Shyti
Hi Chris,

On Fri, Sep 27, 2019 at 08:14:42PM +0100, Chris Wilson wrote:
> For those mock tests that may wish to pretend triggering a GPU reset and
> processing the cleanup.

The patch is OK, per se, but I think it should be split in two
parts:

 - the i915 to gt conversion (that is the biggest part of the
   patch)
 - the mock-reset part (baskically the function)

right?

Andi

> 
> Signed-off-by: Chris Wilson 
> Cc: Andi Shyti 
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c | 32 +--
>  drivers/gpu/drm/i915/gt/intel_reset.h |  5 ++-
>  drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 12 +++
>  drivers/gpu/drm/i915/gt/selftest_lrc.c|  2 +-
>  drivers/gpu/drm/i915/gt/selftest_reset.c  |  4 +--
>  .../gpu/drm/i915/gt/selftest_workarounds.c|  8 ++---
>  drivers/gpu/drm/i915/i915_getparam.c  |  4 +--
>  8 files changed, 41 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 8f125f1624bd..7758a3744626 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4273,7 +4273,7 @@ __intel_display_resume(struct drm_device *dev,
>  static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
>  {
>   return (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display &&
> - intel_has_gpu_reset(dev_priv));
> + intel_has_gpu_reset(_priv->gt));
>  }
>  
>  void intel_prepare_reset(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
> b/drivers/gpu/drm/i915/gt/intel_reset.c
> index d08226f5bea5..76938fa3a1b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -542,13 +542,24 @@ static int gen8_reset_engines(struct intel_gt *gt,
>   return ret;
>  }
>  
> +static int mock_reset(struct intel_gt *gt,
> +   intel_engine_mask_t mask,
> +   unsigned int retry)
> +{
> + return 0;
> +}
> +
>  typedef int (*reset_func)(struct intel_gt *,
> intel_engine_mask_t engine_mask,
> unsigned int retry);
>  
> -static reset_func intel_get_gpu_reset(struct drm_i915_private *i915)
> +static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
>  {
> - if (INTEL_GEN(i915) >= 8)
> + struct drm_i915_private *i915 = gt->i915;
> +
> + if (is_mock_gt(gt))
> + return mock_reset;
> + else if (INTEL_GEN(i915) >= 8)
>   return gen8_reset_engines;
>   else if (INTEL_GEN(i915) >= 6)
>   return gen6_reset_engines;
> @@ -571,7 +582,7 @@ int __intel_gt_reset(struct intel_gt *gt, 
> intel_engine_mask_t engine_mask)
>   int ret = -ETIMEDOUT;
>   int retry;
>  
> - reset = intel_get_gpu_reset(gt->i915);
> + reset = intel_get_gpu_reset(gt);
>   if (!reset)
>   return -ENODEV;
>  
> @@ -591,17 +602,20 @@ int __intel_gt_reset(struct intel_gt *gt, 
> intel_engine_mask_t engine_mask)
>   return ret;
>  }
>  
> -bool intel_has_gpu_reset(struct drm_i915_private *i915)
> +bool intel_has_gpu_reset(const struct intel_gt *gt)
>  {
>   if (!i915_modparams.reset)
>   return NULL;
>  
> - return intel_get_gpu_reset(i915);
> + return intel_get_gpu_reset(gt);
>  }
>  
> -bool intel_has_reset_engine(struct drm_i915_private *i915)
> +bool intel_has_reset_engine(const struct intel_gt *gt)
>  {
> - return INTEL_INFO(i915)->has_reset_engine && i915_modparams.reset >= 2;
> + if (i915_modparams.reset < 2)
> + return false;
> +
> + return INTEL_INFO(gt->i915)->has_reset_engine;
>  }
>  
>  int intel_reset_guc(struct intel_gt *gt)
> @@ -958,7 +972,7 @@ void intel_gt_reset(struct intel_gt *gt,
>  
>   awake = reset_prepare(gt);
>  
> - if (!intel_has_gpu_reset(gt->i915)) {
> + if (!intel_has_gpu_reset(gt)) {
>   if (i915_modparams.reset)
>   dev_err(gt->i915->drm.dev, "GPU reset not supported\n");
>   else
> @@ -1179,7 +1193,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
>* Try engine reset when available. We fall back to full reset if
>* single reset fails.
>*/
> - if (intel_has_reset_engine(gt->i915) && !intel_gt_is_wedged(gt)) {
> + if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
>   for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
>   BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
>   if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h 
> b/drivers/gpu/drm/i915/gt/intel_reset.h
> index 0b6ff1ee7f06..8e8d5f761166 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.h
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.h
> @@ -14,7 +14,6 @@
>