[Intel-gfx] [PATCH v9 8/8] drm/i915/guc : Calling intel_guc_init in i915_gem_init

[Michal Wajdeczko]

On Sat, 11 Nov 2017 01:06:38 +0100, Sujaritha Sundaresan  
<sujaritha.sundaresan at intel.com> wrote:

> Placing the call to intel_guc_init after i915_gem_contexts_init,
> based on the dependency within i915_gem_init.
>
> Will move the function if required, depending on the review
> comments.
>
> Suggested by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Oscar Mateo <oscar.mateo at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c  
> b/drivers/gpu/drm/i915/i915_gem.c
> index 889ae88..c877a5d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -36,6 +36,7 @@
>  #include "intel_frontbuffer.h"
>  #include "intel_mocs.h"
>  #include "i915_gemfs.h"
> +#include "intel_guc.h"
>  #include <linux/dma-fence-array.h>
>  #include <linux/kthread.h>
>  #include <linux/reservation.h>
> @@ -4972,6 +4973,7 @@ bool intel_sanitize_semaphores(struct  
> drm_i915_private *dev_priv, int value)
> int i915_gem_init(struct drm_i915_private *dev_priv)
>  {
> +	struct intel_guc *guc = &dev_priv->guc;
>  	int ret;
> 	mutex_lock(&dev_priv->drm.struct_mutex);
> @@ -5015,6 +5017,18 @@ int i915_gem_init(struct drm_i915_private  
> *dev_priv)
>  	if (ret)
>  		goto out_unlock;
> +	ret = intel_guc_init(guc);
> +
> +		if (i915_modparams.enable_guc) {
> +			/*
> +			 * This is stuff we need to have available at fw load time
> +			 * if we are planning to enable submission later
> +			 */
> +			ret = intel_guc_init(guc);
> +			if (ret)
> +				goto err_shared;
> +		}
> +

Why there are two calls to guc_init ?

Also note that this approach breaks previous idea to hide GuC/HuC internals
to the rest of the driver by exposing only high level "uc" functions.

>  	ret = intel_engines_init(dev_priv);
>  	if (ret)
>  		goto out_unlock;
> @@ -5035,7 +5049,8 @@ int i915_gem_init(struct drm_i915_private  
> *dev_priv)
>  out_unlock:
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  	mutex_unlock(&dev_priv->drm.struct_mutex);
> -
> +err_shared:
> +	intel_guc_fini(guc);
>  	return ret;
>  }
> @@ -5192,6 +5207,7 @@ void i915_gem_load_cleanup(struct drm_i915_private  
> *dev_priv)
>  	rcu_barrier();
> 	i915_gemfs_fini(dev_priv);
> +	intel_guc_fini(&dev_priv->guc);
>  }
> int i915_gem_freeze(struct drm_i915_private *dev_priv)