[Intel-gfx] [PATCH 4/7] drm/i915/uc: Stop talking with GuC when resetting

[Michal Wajdeczko]

On Fri, 17 May 2019 18:30:40 +0200, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2019-05-17 17:22:24)
>> Knowing that GuC will be reset soon, we may stop all communication
>> immediately without doing graceful cleanup as it is not needed.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_guc_ct.h |  5 +++++
>>  drivers/gpu/drm/i915/intel_uc.c     | 13 ++++++++++++-
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h  
>> b/drivers/gpu/drm/i915/intel_guc_ct.h
>> index f5e7f0663304..41ba593a4df7 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_ct.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.h
>> @@ -96,4 +96,9 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct);
>>  int intel_guc_ct_enable(struct intel_guc_ct *ct);
>>  void intel_guc_ct_disable(struct intel_guc_ct *ct);
>>
>> +static inline void intel_guc_ct_stop(struct intel_guc_ct *ct)
>> +{
>> +       ct->host_channel.enabled = false;
>> +}
>> +
>>  #endif /* _INTEL_GUC_CT_H_ */
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 9d86cd831ea7..86edfa5ad72e 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -224,6 +224,17 @@ static int guc_enable_communication(struct  
>> intel_guc *guc)
>>         return 0;
>>  }
>>
>> +static void guc_stop_communication(struct intel_guc *guc)
>> +{
>> +       struct drm_i915_private *i915 = guc_to_i915(guc);
>> +
>> +       if (HAS_GUC_CT(i915))
>
> Does this not fall out of some local knowledge? Though for the moment it
> is harmless to stop a non-existent intel_guc_ct.

We do have CT init/enable/disable/fini functions (all guarded by  
HAS_GUC_CT)
so at the moment we are here, we know that CT was already enabled.

>
>> +               intel_guc_ct_stop(&guc->ct);
>
> Any serialisation required here?

This function might be called from atomic context, so rather not.

> Or caveats for callers to observe?

As we are resetting the GuC, we simply does not allow any further
communication with it, any abusers will see something like this:

<4> [322.401614] Unexpected send: action=0x4000
<4> [322.401648] WARNING: CPU: 3 PID: 1357 at  
drivers/gpu/drm/i915/intel_guc.c:405 intel_guc_send_nop+0xe/0x20 [i915]

>
>> +       guc->send = intel_guc_send_nop;
>> +       guc->handler = intel_guc_to_host_event_handler_nop;
>> +}