Re: [PATCH v3 1/2] drm/i915/uc: Start preparing GuC/HuC for reset

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Michal Wajdeczko (2018-03-05 14:29:16)
> Right after GPU reset there will be a small window of time during which
> some of GuC/HuC fields will still show state before reset. Let's start
> to fix that by sanitizing firmware status as we will use it shortly.
> 
> v2: s/reset_prepare/prepare_to_reset (Michel)
>     don't forget about gem_sanitize path (Daniele)
> v3: rebased
> 
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c    |  5 ++++-
>  drivers/gpu/drm/i915/intel_guc.h   |  5 +++++
>  drivers/gpu/drm/i915/intel_huc.h   |  5 +++++
>  drivers/gpu/drm/i915/intel_uc.c    | 14 ++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.h    |  1 +
>  drivers/gpu/drm/i915/intel_uc_fw.h |  6 ++++++
>  6 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a5bd073..aedb17d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2981,6 +2981,7 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
>         }
>  
>         i915_gem_revoke_fences(dev_priv);
> +       intel_uc_prepare_to_reset(dev_priv);
>  
>         return err;
>  }
> @@ -4882,8 +4883,10 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
>          * it may impact the display and we are uncertain about the stability
>          * of the reset, so this could be applied to even earlier gen.
>          */
> -       if (INTEL_GEN(i915) >= 5 && intel_has_gpu_reset(i915))
> +       if (INTEL_GEN(i915) >= 5 && intel_has_gpu_reset(i915)) {
> +               intel_uc_prepare_to_reset(i915);
>                 WARN_ON(intel_gpu_reset(i915, ALL_ENGINES));

This still feels wrong. If we accept that we will have to reload the fw
on resume, why are we not just sanitzing the uc state and forcing the
reload?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx