[PATCH 3/8] drm/i915/guc: Separate creation/release of runtime logging data from base logging data

Sagar Arun Kamble sagar.a.kamble at intel.com
Sat Jan 13 05:58:36 UTC 2018


GuC log runtime/relay channel data will get released during i915
unregister, and only GuC log vma needs to be released during fini. To
achieve this, prepare separate helpers to create/destroy base and runtime
logging.
This separation is also needed to couple runtime log data and interrupts
handling together. In future we might not want to consider runtime data
creation failure as catastrophic to abort GuC load. Then we can ignore
the return error codes from intel_guc_log_runtime_create().

v2: Rebase.

v3: Refined usage of intel_guc_log_destroy and created new function
intel_guc_log_runtime_destroy. (Tvrtko)
Added intel_guc_log_runtime_create to separate the creation part as well.

v4: Rebase.

v5: Rebase. Moved i915_has_memcpy_from_wc check to guc_log_runtime_create.
Error handling updates to guc_log_runtime_create.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c     |  8 +++++-
 drivers/gpu/drm/i915/intel_guc_log.c | 48 +++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_guc_log.h |  2 ++
 3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 4681537..64e70c1 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -168,9 +168,13 @@ int intel_guc_init(struct intel_guc *guc)
 	if (ret)
 		goto err_shared;
 
-	ret = intel_guc_ads_create(guc);
+	ret = intel_guc_log_runtime_create(guc);
 	if (ret)
 		goto err_log;
+
+	ret = intel_guc_ads_create(guc);
+	if (ret)
+		goto err_log_runtime;
 	GEM_BUG_ON(!guc->ads_vma);
 
 	/* We need to notify the guc whenever we change the GGTT */
@@ -178,6 +182,8 @@ int intel_guc_init(struct intel_guc *guc)
 
 	return 0;
 
+err_log_runtime:
+	intel_guc_log_runtime_destroy(guc);
 err_log:
 	intel_guc_log_destroy(guc);
 err_shared:
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 1719f1e..fd73ba3 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -359,7 +359,7 @@ static bool guc_log_has_runtime(struct intel_guc *guc)
 	return guc->log.runtime.buf_addr != NULL;
 }
 
-static int guc_log_runtime_create(struct intel_guc *guc)
+int intel_guc_log_runtime_create(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	void *vaddr;
@@ -367,13 +367,25 @@ static int guc_log_runtime_create(struct intel_guc *guc)
 	size_t n_subbufs, subbuf_size;
 	int ret;
 
+	if (!i915_modparams.guc_log_level)
+		return 0;
+
 	lockdep_assert_held(&dev_priv->drm.struct_mutex);
 
 	GEM_BUG_ON(guc_log_has_runtime(guc));
 
+	/* We require SSE 4.1 for fast reads from the GuC log buffer and
+	 * it should be present on the chipsets supporting GuC based
+	 * submisssions.
+	 */
+	if (WARN_ON(!i915_has_memcpy_from_wc())) {
+		ret = -EINVAL;
+		goto err;
+	}
+
 	ret = i915_gem_object_set_to_wc_domain(guc->log.vma->obj, true);
 	if (ret)
-		return ret;
+		goto err;
 
 	/* Create a WC (Uncached for read) vmalloc mapping of log
 	 * buffer pages, so that we can directly get the data
@@ -382,7 +394,8 @@ static int guc_log_runtime_create(struct intel_guc *guc)
 	vaddr = i915_gem_object_pin_map(guc->log.vma->obj, I915_MAP_WC);
 	if (IS_ERR(vaddr)) {
 		DRM_ERROR("Couldn't map log buffer pages %d\n", ret);
-		return PTR_ERR(vaddr);
+		ret = PTR_ERR(vaddr);
+		goto err;
 	}
 
 	guc->log.runtime.buf_addr = vaddr;
@@ -419,10 +432,13 @@ static int guc_log_runtime_create(struct intel_guc *guc)
 err_vaddr:
 	i915_gem_object_unpin_map(guc->log.vma->obj);
 	guc->log.runtime.buf_addr = NULL;
+err:
+	/* logging will be off */
+	i915_modparams.guc_log_level = 0;
 	return ret;
 }
 
-static void guc_log_runtime_destroy(struct intel_guc *guc)
+void intel_guc_log_runtime_destroy(struct intel_guc *guc)
 {
 	/*
 	 * It's possible that the runtime stuff was never allocated because
@@ -449,7 +465,7 @@ static int guc_log_late_setup(struct intel_guc *guc)
 		 * log buffer flush interrupts would not have been done yet, so
 		 * do that now.
 		 */
-		ret = guc_log_runtime_create(guc);
+		ret = intel_guc_log_runtime_create(guc);
 		if (ret)
 			goto err;
 	}
@@ -461,7 +477,7 @@ static int guc_log_late_setup(struct intel_guc *guc)
 	return 0;
 
 err_runtime:
-	guc_log_runtime_destroy(guc);
+	intel_guc_log_runtime_destroy(guc);
 err:
 	/* logging will remain off */
 	i915_modparams.guc_log_level = 0;
@@ -520,15 +536,6 @@ int intel_guc_log_create(struct intel_guc *guc)
 		GUC_LOG_ISR_PAGES + 1 +
 		GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT;
 
-	/* We require SSE 4.1 for fast reads from the GuC log buffer and
-	 * it should be present on the chipsets supporting GuC based
-	 * submisssions.
-	 */
-	if (WARN_ON(!i915_has_memcpy_from_wc())) {
-		ret = -EINVAL;
-		goto err;
-	}
-
 	vma = intel_guc_allocate_vma(guc, size);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
@@ -537,12 +544,6 @@ int intel_guc_log_create(struct intel_guc *guc)
 
 	guc->log.vma = vma;
 
-	if (i915_modparams.guc_log_level) {
-		ret = guc_log_runtime_create(guc);
-		if (ret < 0)
-			goto err_vma;
-	}
-
 	/* each allocated unit is a page */
 	flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL |
 		(GUC_LOG_DPC_PAGES << GUC_LOG_DPC_SHIFT) |
@@ -554,8 +555,6 @@ int intel_guc_log_create(struct intel_guc *guc)
 
 	return 0;
 
-err_vma:
-	i915_vma_unpin_and_release(&guc->log.vma);
 err:
 	/* logging will be off */
 	i915_modparams.guc_log_level = 0;
@@ -564,7 +563,6 @@ int intel_guc_log_create(struct intel_guc *guc)
 
 void intel_guc_log_destroy(struct intel_guc *guc)
 {
-	guc_log_runtime_destroy(guc);
 	i915_vma_unpin_and_release(&guc->log.vma);
 }
 
@@ -640,6 +638,6 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 	mutex_lock(&dev_priv->drm.struct_mutex);
 	/* GuC logging is currently the only user of Guc2Host interrupts */
 	gen9_disable_guc_interrupts(dev_priv);
-	guc_log_runtime_destroy(&dev_priv->guc);
+	intel_guc_log_runtime_destroy(&dev_priv->guc);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 }
diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
index f512cf7..345447a 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -52,6 +52,8 @@ struct intel_guc_log {
 
 int intel_guc_log_create(struct intel_guc *guc);
 void intel_guc_log_destroy(struct intel_guc *guc);
+int intel_guc_log_runtime_create(struct intel_guc *guc);
+void intel_guc_log_runtime_destroy(struct intel_guc *guc);
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void i915_guc_log_register(struct drm_i915_private *dev_priv);
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
-- 
1.9.1



More information about the Intel-gfx-trybot mailing list