Re: [03/11 v3] drm/i915/guc: Add onion teardown to the GuC setup

[Arkadiusz Hiler <arkadiusz.hiler@intel.com>]

On Fri, Mar 10, 2017 at 08:28:50AM -0800, Oscar Mateo wrote:
> Starting with intel_guc_loader, down to intel_guc_submission
> and finally to intel_guc_log.
> 
> v2:
>   - Null execbuf client outside guc_client_free (Daniele)
>   - Assert if things try to get allocated twice (Daniele/Joonas)
>   - Null guc->log.buf_addr when destroyed (Daniele)
>   - Newline between returning success and error labels (Joonas)
>   - Remove some unnecessary comments (Joonas)
>   - Keep guc_log_create_extras naming convention (Joonas)
>   - Helper function guc_log_has_extras (Joonas)
>   - No need for separate relay_channel create/destroy. It's just another extra.
>   - No need to nullify guc->log.flush_wq when destroyed (Joonas)
>   - Hoist the check for has_extras out of guc_log_create_extras (Joonas)
>   - Try to do i915_guc_log_register/unregister calls (kind of) symmetric (Daniele)
>   - Make sure initel_guc_fini is not called before init is ever called (Daniele)
> 
> v3:
>   - Remove unnecessary parenthesis (Joonas)
>   - Check for logs enabled on debugfs registration
>   - Rebase on top of Tvrtko's "Fix request re-submission after reset"
> 
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c            |   7 +-
>  drivers/gpu/drm/i915/i915_guc_submission.c |  94 ++++----
>  drivers/gpu/drm/i915/intel_guc_loader.c    |  32 +--
>  drivers/gpu/drm/i915/intel_guc_log.c       | 362 ++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_uc.h            |   8 +-
>  5 files changed, 261 insertions(+), 242 deletions(-)
> 

<SNIP>

> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index b1ebfce..f9e183a 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -443,7 +443,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
>  
>  	err = i915_guc_submission_init(dev_priv);
>  	if (err)
> -		goto fail;
> +		goto err_guc;
>  
>  	/*
>  	 * WaEnableuKernelHeaderValidFix:skl,bxt
> @@ -458,7 +458,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
>  		 */
>  		err = guc_hw_reset(dev_priv);
>  		if (err)
> -			goto fail;
> +			goto err_submission;
>  
>  		intel_huc_load(dev_priv);
>  		err = guc_ucode_xfer(dev_priv);
> @@ -466,7 +466,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
>  			break;
>  
>  		if (--retries == 0)
> -			goto fail;
> +			goto err_submission;
>  
>  		DRM_INFO("GuC fw load failed: %d; will reset and "
>  			 "retry %d more time(s)\n", err, retries);
> @@ -482,7 +482,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
>  
>  		err = i915_guc_submission_enable(dev_priv);
>  		if (err)
> -			goto fail;
> +			goto err_interrupts;
>  	}
>  
>  	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
> @@ -492,15 +492,16 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
>  
>  	return 0;
>  
> +err_interrupts:
> +	gen9_disable_guc_interrupts(dev_priv);

It's no longer called from this place as of:

commit 7762ebb9a455db18eef5c366da5496fb38429d56
Author: Sagar Arun Kamble <sagar.a.kamble@intel.com>

drm/i915/guc: Release GuC interrupts in i915_guc_submission_disable

Also, I did similar thing in my GuC series (for guc_loader.c only). It
should be easy to rebase one on top of the other depending which will
get there first.

Othere changes here LGTM

-- 
Cheers,
Arek


> +err_submission:
> +	i915_guc_submission_fini(dev_priv);
> +err_guc:
> +	i915_ggtt_disable_guc(dev_priv);
>  fail:
>  	if (guc_fw->load_status == INTEL_UC_FIRMWARE_PENDING)
>  		guc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
>  
> -	guc_interrupts_release(dev_priv);
> -	i915_guc_submission_disable(dev_priv);
> -	i915_guc_submission_fini(dev_priv);
> -	i915_ggtt_disable_guc(dev_priv);
> -
>  	/*
>  	 * We've failed to load the firmware :(
>  	 *
> @@ -540,6 +541,13 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
>  	return ret;
>  }
>  

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