Re: [PATCH v4 5/6] drm/i915/guc: disable GuC submission earlier during GuC (re)load

[Yu Dai <yu.dai@intel.com>]

LGTM.

Reviewed-by: Alex Dai <yu.dai@intel.com>

On 04/07/2016 10:21 AM, Dave Gordon wrote:
> When resetting and reloading the GuC, the GuC submission management code
> also needs to destroy and recreate the GuC client(s). Currently this is
> done by a separate call from the GuC loader, but really, it's just an
> internal detail of the submission code. So here we remove the call from
> the loader (which is too late, really, because the GuC has already been
> reloaded at this point) and put it into guc_submission_init() instead.
> This means that any preexisting client is destroyed *before* the GuC
> (re)load and then recreated after, iff the firmware was successfully
> loaded. If the GuC reload fails, we don't recreate the client, so fallback
> to execlists mode (if active) won't leak the client object (previously,
> the now-unusable client would have been left allocated, and leaked if
> the driver were unloaded).
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 6 ++++--
>   drivers/gpu/drm/i915/intel_guc_loader.c    | 3 ---
>   2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index f466eab..a8717f7 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -981,6 +981,10 @@ int i915_guc_submission_init(struct drm_device *dev)
>   	const size_t gemsize = round_up(poolsize, PAGE_SIZE);
>   	struct intel_guc *guc = &dev_priv->guc;
>   
> +	/* Wipe bitmap & delete client in case of reinitialisation */
> +	bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
> +	i915_guc_submission_disable(dev);
> +
>   	if (!i915.enable_guc_submission)
>   		return 0; /* not enabled  */
>   
> @@ -992,9 +996,7 @@ int i915_guc_submission_init(struct drm_device *dev)
>   		return -ENOMEM;
>   
>   	ida_init(&guc->ctx_ids);
> -
>   	guc_create_log(guc);
> -
>   	guc_create_ads(guc);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 876e5da..3e14a9a 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -470,9 +470,6 @@ int intel_guc_ucode_load(struct drm_device *dev)
>   		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
>   
>   	if (i915.enable_guc_submission) {
> -		/* The execbuf_client will be recreated. Release it first. */
> -		i915_guc_submission_disable(dev);
> -
>   		err = i915_guc_submission_enable(dev);
>   		if (err)
>   			goto fail;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx