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

[Sujaritha]

On 11/12/2017 09:22 AM, Michal Wajdeczko wrote:
> 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.
>

I will call intel_guc_init (dev_priv) inside intel_uc_init(dev_priv). I 
will send the corrected patch asap.
I'm also considering splitting the series. I will split the module 
parameter changes and the decoupling
changes into two series.
>>      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)