[PATCH 03/16] drm/i915/guc: Create common entry points for log register/unregister

Michał Winiarski michal.winiarski at intel.com
Mon Feb 26 17:12:20 UTC 2018


We have multiple different functions responsible for allocating
different parts of runtime called from multiple places.
Let's stick with keeping everything in guc_log_register instead.

Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |   6 +-
 drivers/gpu/drm/i915/i915_gem.c      |   4 +-
 drivers/gpu/drm/i915/intel_guc_log.c | 148 ++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_guc_log.h |   6 +-
 drivers/gpu/drm/i915/intel_uc.c      |  39 ++++-----
 drivers/gpu/drm/i915/intel_uc.h      |   6 +-
 6 files changed, 91 insertions(+), 118 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index aaa861b51024..719b2be73292 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -636,7 +636,7 @@ static void i915_gem_fini(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_fini(dev_priv);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-	intel_uc_fini_misc(dev_priv);
+	intel_uc_fini_wq(dev_priv);
 	i915_gem_cleanup_userptr(dev_priv);
 
 	i915_gem_drain_freed_objects(dev_priv);
@@ -1237,7 +1237,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	/* Reveal our presence to userspace */
 	if (drm_dev_register(dev, 0) == 0) {
 		i915_debugfs_register(dev_priv);
-		i915_guc_log_register(dev_priv);
+		intel_uc_log_register(dev_priv);
 		i915_setup_sysfs(dev_priv);
 
 		/* Depends on sysfs having been initialized */
@@ -1297,7 +1297,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	i915_pmu_unregister(dev_priv);
 
 	i915_teardown_sysfs(dev_priv);
-	i915_guc_log_unregister(dev_priv);
+	intel_uc_log_unregister(dev_priv);
 	drm_dev_unregister(&dev_priv->drm);
 
 	i915_gem_shrinker_unregister(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 14c855b1a3a4..4bf5f25b29e2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5279,7 +5279,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
-	ret = intel_uc_init_misc(dev_priv);
+	ret = intel_uc_init_wq(dev_priv);
 	if (ret)
 		return ret;
 
@@ -5375,7 +5375,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-	intel_uc_fini_misc(dev_priv);
+	intel_uc_fini_wq(dev_priv);
 
 	if (ret != -EIO)
 		i915_gem_cleanup_userptr(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 22a05320817b..f1cab43d334e 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -441,7 +441,7 @@ void intel_guc_log_init_early(struct intel_guc *guc)
 	INIT_WORK(&guc->log.runtime.flush_work, capture_logs_work);
 }
 
-int intel_guc_log_relay_create(struct intel_guc *guc)
+int guc_log_relay_create(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct rchan *guc_log_relay_chan;
@@ -494,7 +494,7 @@ int intel_guc_log_relay_create(struct intel_guc *guc)
 	return ret;
 }
 
-void intel_guc_log_relay_destroy(struct intel_guc *guc)
+void guc_log_relay_destroy(struct intel_guc *guc)
 {
 	mutex_lock(&guc->log.runtime.relay_lock);
 
@@ -512,49 +512,6 @@ void intel_guc_log_relay_destroy(struct intel_guc *guc)
 	mutex_unlock(&guc->log.runtime.relay_lock);
 }
 
-static int guc_log_late_setup(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	int ret;
-
-	if (!guc_log_has_runtime(guc)) {
-		/*
-		 * If log was disabled at boot time, then setup needed to handle
-		 * log buffer flush interrupts would not have been done yet, so
-		 * do that now.
-		 */
-		ret = intel_guc_log_relay_create(guc);
-		if (ret)
-			goto err;
-
-		mutex_lock(&dev_priv->drm.struct_mutex);
-		intel_runtime_pm_get(dev_priv);
-		ret = guc_log_runtime_create(guc);
-		intel_runtime_pm_put(dev_priv);
-		mutex_unlock(&dev_priv->drm.struct_mutex);
-
-		if (ret)
-			goto err_relay;
-	}
-
-	ret = guc_log_relay_file_create(guc);
-	if (ret)
-		goto err_runtime;
-
-	return 0;
-
-err_runtime:
-	mutex_lock(&dev_priv->drm.struct_mutex);
-	guc_log_runtime_destroy(guc);
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-err_relay:
-	intel_guc_log_relay_destroy(guc);
-err:
-	/* logging will remain off */
-	i915_modparams.guc_log_level = 0;
-	return ret;
-}
-
 static void guc_log_capture_logs(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -574,16 +531,6 @@ static void guc_flush_logs(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	if (!USES_GUC_SUBMISSION(dev_priv) || !i915_modparams.guc_log_level)
-		return;
-
-	/* First disable the interrupts, will be renabled afterwards */
-	mutex_lock(&dev_priv->drm.struct_mutex);
-	intel_runtime_pm_get(dev_priv);
-	gen9_disable_guc_interrupts(dev_priv);
-	intel_runtime_pm_put(dev_priv);
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-
 	/*
 	 * Before initiating the forceful flush, wait for any pending/ongoing
 	 * flush to complete otherwise forceful flush may not actually happen.
@@ -626,12 +573,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) |
@@ -643,8 +584,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;
@@ -703,24 +642,14 @@ int intel_guc_log_control_set(struct intel_guc *guc, u64 val)
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
 	if (GUC_LOG_IS_ENABLED(val) && !guc_log_has_runtime(guc)) {
-		ret = guc_log_late_setup(guc);
-		if (ret)
+		ret = intel_guc_log_register(guc);
+		if (ret) {
+			/* logging will remain off */
+			i915_modparams.guc_log_level = 0;
 			goto out;
-
-		/* GuC logging is currently the only user of Guc2Host interrupts */
-		mutex_lock(&dev_priv->drm.struct_mutex);
-		intel_runtime_pm_get(dev_priv);
-		gen9_enable_guc_interrupts(dev_priv);
-		intel_runtime_pm_put(dev_priv);
-		mutex_unlock(&dev_priv->drm.struct_mutex);
+		}
 	} else if (!GUC_LOG_IS_ENABLED(val) && guc_log_has_runtime(guc)) {
-		/*
-		 * Once logging is disabled, GuC won't generate logs & send an
-		 * interrupt. But there could be some data in the log buffer
-		 * which is yet to be captured. So request GuC to update the log
-		 * buffer state and then collect the left over logs.
-		 */
-		guc_flush_logs(guc);
+		intel_guc_log_unregister(guc);
 	}
 
 	return 0;
@@ -731,20 +660,63 @@ int intel_guc_log_control_set(struct intel_guc *guc, u64 val)
 	return ret;
 }
 
-void i915_guc_log_register(struct drm_i915_private *dev_priv)
+int intel_guc_log_register(struct intel_guc *guc)
 {
-	if (!USES_GUC_SUBMISSION(dev_priv) || !i915_modparams.guc_log_level)
-		return;
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	int ret;
+
+	GEM_BUG_ON(guc_log_has_runtime(guc));
+
+	/*
+	 * If log was disabled at boot time, then setup needed to handle
+	 * log buffer flush interrupts would not have been done yet, so
+	 * do that now.
+	 */
+	ret = guc_log_relay_create(guc);
+	if (ret)
+		goto err;
+
+	mutex_lock(&dev_priv->drm.struct_mutex);
+	ret = guc_log_runtime_create(guc);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+
+	if (ret)
+		goto err_relay;
+
+	ret = guc_log_relay_file_create(guc);
+	if (ret)
+		goto err_runtime;
 
-	guc_log_late_setup(&dev_priv->guc);
+	/* GuC logging is currently the only user of Guc2Host interrupts */
+	mutex_lock(&dev_priv->drm.struct_mutex);
+	intel_runtime_pm_get(dev_priv);
+	gen9_enable_guc_interrupts(dev_priv);
+	intel_runtime_pm_put(dev_priv);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+
+	return 0;
+
+err_runtime:
+	mutex_lock(&dev_priv->drm.struct_mutex);
+	guc_log_runtime_destroy(guc);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+err_relay:
+	guc_log_relay_destroy(guc);
+err:
+	return ret;
 }
 
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
+void intel_guc_log_unregister(struct intel_guc *guc)
 {
-	struct intel_guc *guc = &dev_priv->guc;
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	if (!USES_GUC_SUBMISSION(dev_priv))
-		return;
+	/*
+	 * Once logging is disabled, GuC won't generate logs & send an
+	 * interrupt. But there could be some data in the log buffer
+	 * which is yet to be captured. So request GuC to update the log
+	 * buffer state and then collect the left over logs.
+	 */
+	guc_flush_logs(guc);
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
 	/* GuC logging is currently the only user of Guc2Host interrupts */
@@ -755,5 +727,5 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 	guc_log_runtime_destroy(guc);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-	intel_guc_log_relay_destroy(guc);
+	guc_log_relay_destroy(guc);
 }
diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
index 141ce9ca22ce..09dd2ef1933d 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -62,11 +62,9 @@ struct intel_guc_log {
 int intel_guc_log_create(struct intel_guc *guc);
 void intel_guc_log_destroy(struct intel_guc *guc);
 void intel_guc_log_init_early(struct intel_guc *guc);
-int intel_guc_log_relay_create(struct intel_guc *guc);
-void intel_guc_log_relay_destroy(struct intel_guc *guc);
 int intel_guc_log_control_get(struct intel_guc *guc);
 int intel_guc_log_control_set(struct intel_guc *guc, 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);
+int intel_guc_log_register(struct intel_guc *guc);
+void intel_guc_log_unregister(struct intel_guc *guc);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9f1bac6398fb..3f41ef525e26 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -219,6 +219,22 @@ static void guc_free_load_err_log(struct intel_guc *guc)
 		i915_gem_object_put(guc->load_err_log);
 }
 
+int intel_uc_log_register(struct drm_i915_private *dev_priv)
+{
+	if (!USES_GUC(dev_priv) || !i915_modparams.guc_log_level)
+		return 0;
+
+	return intel_guc_log_register(&dev_priv->guc);
+}
+
+void intel_uc_log_unregister(struct drm_i915_private *dev_priv)
+{
+	if (!USES_GUC(dev_priv) || !i915_modparams.guc_log_level)
+		return;
+
+	intel_guc_log_unregister(&dev_priv->guc);
+}
+
 static int guc_enable_communication(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -240,7 +256,7 @@ static void guc_disable_communication(struct intel_guc *guc)
 	guc->send = intel_guc_send_nop;
 }
 
-int intel_uc_init_misc(struct drm_i915_private *dev_priv)
+int intel_uc_init_wq(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 	int ret;
@@ -249,26 +265,13 @@ int intel_uc_init_misc(struct drm_i915_private *dev_priv)
 		return 0;
 
 	ret = intel_guc_init_wq(guc);
-	if (ret) {
-		DRM_ERROR("Couldn't allocate workqueues for GuC\n");
-		goto err;
-	}
-
-	ret = intel_guc_log_relay_create(guc);
-	if (ret) {
-		DRM_ERROR("Couldn't allocate relay for GuC log\n");
-		goto err_relay;
-	}
+	if (ret)
+		return ret;
 
 	return 0;
-
-err_relay:
-	intel_guc_fini_wq(guc);
-err:
-	return ret;
 }
 
-void intel_uc_fini_misc(struct drm_i915_private *dev_priv)
+void intel_uc_fini_wq(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 
@@ -276,8 +279,6 @@ void intel_uc_fini_misc(struct drm_i915_private *dev_priv)
 		return;
 
 	intel_guc_fini_wq(guc);
-
-	intel_guc_log_relay_destroy(guc);
 }
 
 int intel_uc_init(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index f2984e01e257..9bc22103396d 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -31,10 +31,12 @@
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
 void intel_uc_init_mmio(struct drm_i915_private *dev_priv);
+int intel_uc_log_register(struct drm_i915_private *dev_priv);
+void intel_uc_log_unregister(struct drm_i915_private *dev_priv);
 void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
-int intel_uc_init_misc(struct drm_i915_private *dev_priv);
-void intel_uc_fini_misc(struct drm_i915_private *dev_priv);
+int intel_uc_init_wq(struct drm_i915_private *dev_priv);
+void intel_uc_fini_wq(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_uc_init(struct drm_i915_private *dev_priv);
-- 
2.14.3



More information about the Intel-gfx-trybot mailing list