diff mbox

[04/12] drm/i915/guc: The Additional Data Struct (ADS) should get enabled together with GuC submission

Message ID 1490086977-9282-5-git-send-email-oscar.mateo@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

oscar.mateo@intel.com March 21, 2017, 9:02 a.m. UTC 
It's mandatory and it gets created if and only if GuC submission is enabled, so that should be
the condition for informing the GuC about it.

Also s/guc_addon_create/guc_ads_create and s/guc_addon_destroy/guc_ads_destroy and, while
at it, add an explanation of what things go inside the ADS object.

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_guc_submission.c | 17 ++++++++++++-----
 drivers/gpu/drm/i915/intel_guc_loader.c    | 10 ++++------
 2 files changed, 16 insertions(+), 11 deletions(-)

Comments

Daniele Ceraolo Spurio March 21, 2017, 5:51 p.m. UTC | #1 
On 3/21/2017 2:02 AM, Oscar Mateo wrote:
> It's mandatory and it gets created if and only if GuC submission is enabled, so that should be
> the condition for informing the GuC about it.

This is true now, but GuC might actually require the ADS even if GuC 
submission is disabled, depending on the features being used. In the 
future we might therefore want to gate ADS logic based on the value of 
guc_loading instead of guc_submission.

This change is coherent with the current flow and makes things cleaner, so:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

>
> Also s/guc_addon_create/guc_ads_create and s/guc_addon_destroy/guc_ads_destroy and, while
> at it, add an explanation of what things go inside the ADS object.
>
> 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>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b24602b..ff8235a1 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -62,6 +62,13 @@ 
  * ELSP context descriptor dword into Work Item.
  * See guc_wq_item_append()
  *
+ * ADS:
+ * The Additional Data Struct (ADS) has pointers for different buffers used by
+ * the GuC. One single gem object contains the ADS struct itself (guc_ads), the
+ * scheduling policies (guc_policies), a structure describing a collection of
+ * register sets (guc_mmio_reg_state) and some extra pages for the GuC to save
+ * its internal state for sleep.
+ *
  */
 
 static inline bool is_high_priority(struct i915_guc_client* client)
@@ -937,7 +944,7 @@  static void guc_policies_init(struct guc_policies *policies)
 	policies->is_valid = 1;
 }
 
-static int guc_addon_create(struct intel_guc *guc)
+static int guc_ads_create(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct i915_vma *vma;
@@ -1000,7 +1007,7 @@  static int guc_addon_create(struct intel_guc *guc)
 	return 0;
 }
 
-static void guc_addon_destroy(struct intel_guc *guc)
+static void guc_ads_destroy(struct intel_guc *guc)
 {
 	i915_vma_unpin_and_release(&guc->ads_vma);
 }
@@ -1050,7 +1057,7 @@  int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 	if (ret < 0)
 		goto err_vaddr;
 
-	ret = guc_addon_create(guc);
+	ret = guc_ads_create(guc);
 	if (ret < 0)
 		goto err_log;
 
@@ -1069,7 +1076,7 @@  int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 	return 0;
 
 err_ads:
-	guc_addon_destroy(guc);
+	guc_ads_destroy(guc);
 err_log:
 	intel_guc_log_destroy(guc);
 err_vaddr:
@@ -1089,7 +1096,7 @@  void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
 	guc_client_free(guc->execbuf_client);
 	guc->execbuf_client = NULL;
 	ida_destroy(&guc->ctx_ids);
-	guc_addon_destroy(guc);
+	guc_ads_destroy(guc);
 	intel_guc_log_destroy(guc);
 	i915_gem_object_unpin_map(guc->ctx_pool->obj);
 	i915_vma_unpin_and_release(&guc->ctx_pool);
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index b8ba28d..62ebf56 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -148,17 +148,15 @@  static void guc_params_init(struct drm_i915_private *dev_priv)
 	} else
 		params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
 
-	if (guc->ads_vma) {
-		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
-		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
-		params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
-	}
-
 	/* If GuC submission is enabled, set up additional parameters here */
 	if (i915.enable_guc_submission) {
+		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
 		u32 pgs = guc_ggtt_offset(dev_priv->guc.ctx_pool);
 		u32 ctx_in_16 = GUC_MAX_GPU_CONTEXTS / 16;
 
+		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
+		params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
+
 		pgs >>= PAGE_SHIFT;
 		params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
 			(ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);