[Intel-gfx] [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup

[Sagar Arun Kamble]

On 3/28/2018 2:05 AM, Michal Wajdeczko wrote:
> In commit 9192d4fb811e ("drm/i915/guc: Extract doorbell creation
> from client allocation") we introduced asymmetry in doorbell cleanup
> to avoid warnings due to failed communication with already reset GuC.
> As we improved our reset/unload paths, we can restore symmetry in
> doorbell cleanup, as GuC should be still active by now.
>
> Suggested-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Michal Winiarski <michal.winiarski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
This looks good.
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>

We should extend this functionality further to consider cases when GuC 
doorbell deactivation flow is to be
invoked (when GuC is in sane state) and not to be invoked (when GuC is 
hung) as was prepared in patchset
https://patchwork.freedesktop.org/series/33209/ but that can be done 
after this series if we agree on need for such handling.

Thanks,
Sagar
> ---
>   drivers/gpu/drm/i915/intel_guc_submission.c | 15 +++------------
>   1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 207cda0..26985d8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -835,18 +835,9 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
>   
>   static void guc_clients_doorbell_fini(struct intel_guc *guc)
>   {
> -	/*
> -	 * By the time we're here, GuC has already been reset.
> -	 * Instead of trying (in vain) to communicate with it, let's just
> -	 * cleanup the doorbell HW and our internal state.
> -	 */
> -	if (guc->preempt_client) {
> -		__destroy_doorbell(guc->preempt_client);
> -		__update_doorbell_desc(guc->preempt_client,
> -				       GUC_DOORBELL_INVALID);
> -	}
> -	__destroy_doorbell(guc->execbuf_client);
> -	__update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
> +	if (guc->preempt_client)
> +		destroy_doorbell(guc->preempt_client);
> +	destroy_doorbell(guc->execbuf_client);
>   }
>   
>   /**

-- 
Thanks,
Sagar