[PATCH 05/12] drm/i915/guc: Make GuC logging related functions depend only on guc_log_level

Sagar Arun Kamble sagar.a.kamble at intel.com
Tue Oct 17 19:28:40 UTC 2017


With guc_log_level parameter sanitized we can remove the GuC submission
checks from flush_guc_logs and i915_guc_log_register/unregister and
intel_uc_fini_hw. It is important to note that GuC log runtime/relay
channel data has to be freed during driver unregister.
Freeing of that data can't be gated by guc_log_level check because If we
free GuC log runtime only when log level >=0 then it will not be destroyed
when logging is disabled after enabling before driver unload and driver
will try to destroy during cleanup/fini_hw where relay will be NULL
already.

With this patch GuC interrupts are enabled first after GuC load if logging
is enabled. Earlier they were enabled only when submission was getting
enabled.

v2: Rebase. Updated check in i915_guc_log_unregister to be based on
guc_log_level. (Michal Wajdeczko)

v3: Updated guc_log_unregister again. Made all GuC log related functions
depend only guc_log_level. Updated uC init w.r.t enabling of GuC
interrupts.

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     |  3 ++-
 drivers/gpu/drm/i915/intel_guc_log.c | 12 ++++--------
 drivers/gpu/drm/i915/intel_uc.c      | 21 ++++++++++++---------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 31f25e5..959057a 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -274,7 +274,8 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
 	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return 0;
 
-	intel_disable_guc_interrupts(guc);
+	if (i915_modparams.guc_log_level >= 0)
+		intel_disable_guc_interrupts(guc);
 
 	ctx = dev_priv->kernel_context;
 
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 200f0a1..f87e9f5 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -504,8 +504,7 @@ static void guc_log_capture_logs(struct intel_guc *guc)
 
 static void guc_flush_logs(struct intel_guc *guc)
 {
-	if (!i915_modparams.enable_guc_submission ||
-	    (i915_modparams.guc_log_level < 0))
+	if (i915_modparams.guc_log_level < 0)
 		return;
 
 	/* First disable the interrupts, will be renabled afterwards */
@@ -645,8 +644,7 @@ 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)
 {
-	if (!i915_modparams.enable_guc_submission ||
-	    (i915_modparams.guc_log_level < 0))
+	if (i915_modparams.guc_log_level < 0)
 		return;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
@@ -656,12 +654,10 @@ void i915_guc_log_register(struct drm_i915_private *dev_priv)
 
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 {
-	if (!i915_modparams.enable_guc_submission)
-		return;
-
 	mutex_lock(&dev_priv->drm.struct_mutex);
 	/* GuC logging is currently the only user of Guc2Host interrupts */
-	intel_disable_guc_interrupts(&dev_priv->guc);
+	if (i915_modparams.guc_log_level >= 0)
+		intel_disable_guc_interrupts(&dev_priv->guc);
 	guc_log_runtime_destroy(&dev_priv->guc);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 }
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 69ea048..db5ac51 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -212,18 +212,18 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (ret)
 		goto err_log_capture;
 
+	if (i915_modparams.guc_log_level >= 0)
+		intel_enable_guc_interrupts(guc);
+
 	ret = guc_enable_communication(guc);
 	if (ret)
-		goto err_log_capture;
+		goto err_interrupts;
 
 	intel_huc_auth(&dev_priv->huc);
 	if (i915_modparams.enable_guc_submission) {
-		if (i915_modparams.guc_log_level >= 0)
-			intel_enable_guc_interrupts(guc);
-
 		ret = i915_guc_submission_enable(dev_priv);
 		if (ret)
-			goto err_interrupts;
+			goto err_comm;
 	}
 
 	dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version %u.%u])\n",
@@ -243,9 +243,11 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	 * nonfatal error (i.e. it doesn't prevent driver load, but
 	 * marks the GPU as wedged until reset).
 	 */
-err_interrupts:
+err_comm:
 	guc_disable_communication(guc);
-	intel_disable_guc_interrupts(guc);
+err_interrupts:
+	if (i915_modparams.guc_log_level >= 0)
+		intel_disable_guc_interrupts(guc);
 err_log_capture:
 	guc_capture_load_err_log(guc);
 err_submission:
@@ -285,10 +287,11 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 
 	guc_disable_communication(&dev_priv->guc);
 
-	if (i915_modparams.enable_guc_submission) {
+	if (i915_modparams.guc_log_level >= 0)
 		intel_disable_guc_interrupts(&dev_priv->guc);
+
+	if (i915_modparams.enable_guc_submission)
 		i915_guc_submission_fini(dev_priv);
-	}
 
 	i915_ggtt_disable_guc(dev_priv);
 }
-- 
1.9.1



More information about the Intel-gfx-trybot mailing list