[Intel-gfx] [RFC 2/3] drm/i915/guc: Omit guc_init_doorbell_hw during driver load

[Daniele Ceraolo Spurio]

On 15/11/17 10:30, Michel Thierry wrote:
> During driver load we create the GuC clients and allocate their
> doorbells just before executing guc_init_doorbell_hw; but since we just
> created these doorbells, how can they be out of sync?
> This code has had more than enough refactoring (2 more still in progress)
> so I would not be surprised if calling guc_init_doorbell_hw made sense at
> some point, but not anymore.
> 

I think the idea was to clean up the unallocated doorbells on takeover 
to be covered in case the previous occupant of the GPU didn't release 
them when leaving the HW. We do a full gpu reset during i915 load now in 
i915_gem_sanitize so the doorbell HW should be cleaned up by that, but 
there is still a possible issue when i915.reset=0. However, with reset=0 
this wouldn't be the only thing not sanitized and the only bad 
consequence would be extra irqs to GuC (which would be ignored), so I 
don't think it is worth worrying about that case.

Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>

Daniele

> The resume path is different, in this case the driver doesn't
> recreate clients, and it is still reasonable to validate/reallocate the
> doorbells in order to confirm that they still belong to the clients.
> 
> And probably guc_init_doorbell_hw is no longer the right name, but I'll
> leave that to someone else.
> 
> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Oscar Mateo <oscar.mateo at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 5d6576e01a91..d6762ca42cf1 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1424,16 +1424,16 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
>   	} else {
>   		guc_reset_wq(guc->execbuf_client);
>   		guc_reset_wq(guc->preempt_client);
> +
> +		err = guc_init_doorbell_hw(guc);
> +		if (err)
> +			goto err_free_clients;
>   	}
>   
>   	err = intel_guc_sample_forcewake(guc);
>   	if (err)
>   		goto err_free_clients;
>   
> -	err = guc_init_doorbell_hw(guc);
> -	if (err)
> -		goto err_free_clients;
> -
>   	/* Take over from manual control of ELSP (execlists) */
>   	guc_interrupts_capture(dev_priv);
>   
>