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

[Daniele Ceraolo Spurio]

On 22/02/18 11:28, Daniele Ceraolo Spurio wrote:
> 
> 
> On 22/02/18 10:45, Michal Wajdeczko wrote:
>> 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.
>>
>> 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>
>> ---
>>   drivers/gpu/drm/i915/i915_gem.c    |  1 +
>>   drivers/gpu/drm/i915/intel_guc.h   |  5 +++++
>>   drivers/gpu/drm/i915/intel_huc.h   |  5 +++++
>>   drivers/gpu/drm/i915/intel_uc.c    | 11 +++++++++++
>>   drivers/gpu/drm/i915/intel_uc.h    |  1 +
>>   drivers/gpu/drm/i915/intel_uc_fw.h |  6 ++++++
>>   6 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c 
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index 14c855b..9f91625 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_reset_prepare(dev_priv);
>>       return err;
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index 52856a9..ffa528f 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -132,4 +132,9 @@ static inline u32 guc_ggtt_offset(struct i915_vma 
>> *vma)
>>   struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 
>> size);
>>   u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>> +static inline void intel_guc_reset_prepare(struct intel_guc *guc)
>> +{
> 
> We could also move guc_disable_communication and reset_guc_interrupts 
> here and remove them from intel_uc_init_hw (I'm assuming there is no 
> need to do either of the 2 the first time GuC is loaded).

Actually after looking at the flow I think we do still need those in 
intel_uc_init_hw for when we come out of suspension. we call 
intel_gpu_reset and not the full i915_reset in gem_sanitize, so 
reset_prepare is not called. for the same reason uc_fw->load_status will 
probably be incorrect after resume as well.

Daniele

> 
> Patch looks good to me apart from the above note.
> 
> Daniele
> 
>> +    intel_uc_fw_reset_prepare(&guc->fw);
>> +}
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/intel_huc.h 
>> b/drivers/gpu/drm/i915/intel_huc.h
>> index 40039db..c1ee5b5 100644
>> --- a/drivers/gpu/drm/i915/intel_huc.h
>> +++ b/drivers/gpu/drm/i915/intel_huc.h
>> @@ -38,4 +38,9 @@ struct intel_huc {
>>   int intel_huc_init_hw(struct intel_huc *huc);
>>   int intel_huc_auth(struct intel_huc *huc);
>> +static inline void intel_huc_reset_prepare(struct intel_huc *huc)
>> +{
>> +    intel_uc_fw_reset_prepare(&huc->fw);
>> +}
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 9f1bac6..64ae0a9 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -445,3 +445,14 @@ void intel_uc_fini_hw(struct drm_i915_private 
>> *dev_priv)
>>       if (USES_GUC_SUBMISSION(dev_priv))
>>           gen9_disable_guc_interrupts(dev_priv);
>>   }
>> +
>> +void intel_uc_reset_prepare(struct drm_i915_private *i915)
>> +{
>> +    if (!USES_GUC(i915))
>> +        return;
>> +
>> +    GEM_BUG_ON(!HAS_GUC(i915));
>> +
>> +    intel_huc_reset_prepare(&i915->huc);
>> +    intel_guc_reset_prepare(&i915->guc);
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h 
>> b/drivers/gpu/drm/i915/intel_uc.h
>> index f2984e0..5c10253 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -39,6 +39,7 @@
>>   void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
>>   int intel_uc_init(struct drm_i915_private *dev_priv);
>>   void intel_uc_fini(struct drm_i915_private *dev_priv);
>> +void intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
>>   static inline bool intel_uc_is_using_guc(void)
>>   {
>> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h 
>> b/drivers/gpu/drm/i915/intel_uc_fw.h
>> index d5fd460..68bc304 100644
>> --- a/drivers/gpu/drm/i915/intel_uc_fw.h
>> +++ b/drivers/gpu/drm/i915/intel_uc_fw.h
>> @@ -115,6 +115,12 @@ static inline bool intel_uc_fw_is_selected(struct 
>> intel_uc_fw *uc_fw)
>>       return uc_fw->path != NULL;
>>   }
>> +static inline void intel_uc_fw_reset_prepare(struct intel_uc_fw *uc_fw)
>> +{
>> +    if (uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS)
>> +        uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
>> +}
>> +
>>   void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>>                  struct intel_uc_fw *uc_fw);
>>   int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx