[Intel-gfx] [PATCH v2 03/12] drm/i915/guc: Allocate separate shared data object for GuC communication

Michel Thierry michel.thierry at intel.com
Wed Oct 25 21:04:46 UTC 2017


On 25/10/17 13:00, Michał Winiarski wrote:
> We were using first page of kernel context render state for sharing data
> with GuC. While it's justified by the fact that those pages are not used
> (note, GuC still enforces this layout and refuses to work if we remove
> the extra page in front), it's also confusing (why are we using this
> particular page?). Let's allocate a separate object instead.
> 
> v2: Drop kernel_context from GuC suspend/resume action handlers (Michel)

v2 Reviewed-by: Michel Thierry <michel.thierry at intel.com>
> 
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Jeff McGee <jeff.mcgee at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Oscar Mateo <oscar.mateo at intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 36 +++++++++++++++++++++++++++++-
>   drivers/gpu/drm/i915/intel_guc.c           | 14 ++----------
>   drivers/gpu/drm/i915/intel_guc.h           |  2 ++
>   3 files changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index e195bdee0473..d1a5613da24c 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -437,6 +437,33 @@ static void guc_stage_desc_fini(struct intel_guc *guc,
>          memset(desc, 0, sizeof(*desc));
>   }
> 
> +static int guc_shared_data_create(struct intel_guc *guc)
> +{
> +       struct i915_vma *vma;
> +       void *vaddr;
> +
> +       vma = intel_guc_allocate_vma(guc, PAGE_SIZE);
> +       if (IS_ERR(vma))
> +               return PTR_ERR(vma);
> +
> +       vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
> +       if (IS_ERR(vaddr)) {
> +               i915_vma_unpin_and_release(&vma);
> +               return PTR_ERR(vaddr);
> +       }
> +
> +       guc->shared_data = vma;
> +       guc->shared_data_vaddr = vaddr;
> +
> +       return 0;
> +}
> +
> +static void guc_shared_data_destroy(struct intel_guc *guc)
> +{
> +       i915_gem_object_unpin_map(guc->shared_data->obj);
> +       i915_vma_unpin_and_release(&guc->shared_data);
> +}
> +
>   /* Construct a Work Item and append it to the GuC's Work Queue */
>   static void guc_wq_item_append(struct i915_guc_client *client,
>                                 struct drm_i915_gem_request *rq)
> @@ -993,9 +1020,13 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
>          if (ret)
>                  return ret;
> 
> +       ret = guc_shared_data_create(guc);
> +       if (ret)
> +               goto err_stage_desc_pool;
> +
>          ret = intel_guc_log_create(guc);
>          if (ret < 0)
> -               goto err_stage_desc_pool;
> +               goto err_shared_data;
> 
>          ret = guc_ads_create(guc);
>          if (ret < 0)
> @@ -1005,6 +1036,8 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
> 
>   err_log:
>          intel_guc_log_destroy(guc);
> +err_shared_data:
> +       guc_shared_data_destroy(guc);
>   err_stage_desc_pool:
>          guc_stage_desc_pool_destroy(guc);
>          return ret;
> @@ -1016,6 +1049,7 @@ void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
> 
>          guc_ads_destroy(guc);
>          intel_guc_log_destroy(guc);
> +       guc_shared_data_destroy(guc);
>          guc_stage_desc_pool_destroy(guc);
>   }
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 10037c0fdf95..f74d50fdaeb0 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -268,7 +268,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset)
>   int intel_guc_suspend(struct drm_i915_private *dev_priv)
>   {
>          struct intel_guc *guc = &dev_priv->guc;
> -       struct i915_gem_context *ctx;
>          u32 data[3];
> 
>          if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> @@ -276,14 +275,10 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
> 
>          gen9_disable_guc_interrupts(dev_priv);
> 
> -       ctx = dev_priv->kernel_context;
> -
>          data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
>          /* any value greater than GUC_POWER_D0 */
>          data[1] = GUC_POWER_D1;
> -       /* first page is shared data with GuC */
> -       data[2] = guc_ggtt_offset(ctx->engine[RCS].state) +
> -                 LRC_GUCSHR_PN * PAGE_SIZE;
> +       data[2] = guc_ggtt_offset(guc->shared_data);
> 
>          return intel_guc_send(guc, data, ARRAY_SIZE(data));
>   }
> @@ -295,7 +290,6 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
>   int intel_guc_resume(struct drm_i915_private *dev_priv)
>   {
>          struct intel_guc *guc = &dev_priv->guc;
> -       struct i915_gem_context *ctx;
>          u32 data[3];
> 
>          if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> @@ -304,13 +298,9 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
>          if (i915_modparams.guc_log_level >= 0)
>                  gen9_enable_guc_interrupts(dev_priv);
> 
> -       ctx = dev_priv->kernel_context;
> -
>          data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
>          data[1] = GUC_POWER_D0;
> -       /* first page is shared data with GuC */
> -       data[2] = guc_ggtt_offset(ctx->engine[RCS].state) +
> -                 LRC_GUCSHR_PN * PAGE_SIZE;
> +       data[2] = guc_ggtt_offset(guc->shared_data);
> 
>          return intel_guc_send(guc, data, ARRAY_SIZE(data));
>   }
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 418450b1ae27..aa1583167b0a 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -54,6 +54,8 @@ struct intel_guc {
>          struct i915_vma *stage_desc_pool;
>          void *stage_desc_pool_vaddr;
>          struct ida stage_ids;
> +       struct i915_vma *shared_data;
> +       void *shared_data_vaddr;
> 
>          struct i915_guc_client *execbuf_client;
> 
> --
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


More information about the Intel-gfx mailing list