diff mbox

[v2,03/12] drm/i915/guc: Allocate separate shared data object for GuC communication

Message ID 20171025200020.16636-4-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michał Winiarski Oct. 25, 2017, 8 p.m. UTC 
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)

Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@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(-)

Comments

Michel Thierry Oct. 25, 2017, 9:04 p.m. UTC | #1 
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@intel.com>
> 
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Jeff McGee <jeff.mcgee@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
diff mbox

Patch

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;