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

[Michal Wajdeczko]

On Thu, 08 Mar 2018 06:47:35 +0100, Sagar Arun Kamble  
<sagar.a.kamble at intel.com> wrote:

>
>
> On 3/5/2018 8:13 PM, Chris Wilson wrote:
>> 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 at intel.com>
>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Michel Thierry <michel.thierry at intel.com>
>>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at 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?
> Hi Chris,
>
> intel_uc_prepare_to_reset() is sanitizing uc state and reload is  
> happening through gem_init_hw in resume path.
> what are we missing?

Maybe to make everyone happy, I'll introduce intel_uc_sanitize()
leaving to doubt how we will handle reset/suspend/resume scenarios.
And then uc_resume will likely be nop as we will do full reload on
gem_init_hw.

/Michal