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

[Sagar Arun Kamble <sagar.a.kamble@intel.com>]

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.

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

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 7278bde..9dac3ee 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 84ae6f8..d866645 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -356,7 +356,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;
@@ -364,6 +364,9 @@ static int guc_log_runtime_create(struct intel_guc *guc)
 	size_t n_subbufs, subbuf_size;
 	int ret;
 
+	if (i915_modparams.guc_log_level < 0)
+		return 0;
+
 	lockdep_assert_held(&dev_priv->drm.struct_mutex);
 
 	GEM_BUG_ON(guc_log_has_runtime(guc));
@@ -419,7 +422,7 @@ static int guc_log_runtime_create(struct intel_guc *guc)
 	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
@@ -445,7 +448,7 @@ static int guc_log_late_setup(struct intel_guc *guc)
 		 * handle 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;
 	}
@@ -457,7 +460,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 = -1;
@@ -535,12 +538,6 @@ int intel_guc_log_create(struct intel_guc *guc)
 
 	guc->log.vma = vma;
 
-	if (i915_modparams.guc_log_level >= 0) {
-		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) |
@@ -552,8 +549,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 = -1;
@@ -562,7 +557,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);
 }
 
@@ -638,6 +632,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 */
 	intel_guc_disable_interrupts(&dev_priv->guc);
-	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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx