[PATCH 3/6] drm/i915/uc: replace uc init/fini misc

Daniele Ceraolo Spurio daniele.ceraolospurio at intel.com
Fri Jun 28 23:44:14 UTC 2019


The "misc" terminology doesn't make it very clear what we intend to cover
in this phase. The only thing we do in there apart from FW fetch is
initializing the workqueues. We can move the latter to init_early (which
already covers init of other workqueues) and rename the function to
clarify that they only fetch/release the blobs.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
---
 drivers/gpu/drm/i915/gt/intel_guc.c     | 82 +++++++++----------------
 drivers/gpu/drm/i915/gt/intel_guc.h     |  5 +-
 drivers/gpu/drm/i915/gt/intel_guc_log.c | 32 +++++++++-
 drivers/gpu/drm/i915/gt/intel_guc_log.h |  3 +-
 drivers/gpu/drm/i915/gt/intel_huc.c     |  8 ---
 drivers/gpu/drm/i915/gt/intel_huc.h     |  6 --
 drivers/gpu/drm/i915/gt/intel_uc.c      | 50 ++++++---------
 drivers/gpu/drm/i915/gt/intel_uc.h      |  6 +-
 drivers/gpu/drm/i915/i915_drv.c         |  7 ++-
 drivers/gpu/drm/i915/i915_gem.c         | 12 ++--
 10 files changed, 97 insertions(+), 114 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_guc.c b/drivers/gpu/drm/i915/gt/intel_guc.c
index c40a6efdd33a..e6558d786241 100644
--- a/drivers/gpu/drm/i915/gt/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/intel_guc.c
@@ -74,54 +74,10 @@ void intel_guc_init_send_regs(struct intel_guc *guc)
 	guc->send_regs.fw_domains = fw_domains;
 }
 
-void intel_guc_init_early(struct intel_guc *guc)
-{
-	struct drm_i915_private *i915 = guc_to_i915(guc);
-
-	intel_guc_fw_init_early(guc);
-	intel_guc_ct_init_early(&guc->ct);
-	intel_guc_log_init_early(&guc->log);
-
-	mutex_init(&guc->send_mutex);
-	spin_lock_init(&guc->irq_lock);
-	guc->send = intel_guc_send_nop;
-	guc->handler = intel_guc_to_host_event_handler_nop;
-	if (INTEL_GEN(i915) >= 11) {
-		guc->notify = gen11_guc_raise_irq;
-		guc->interrupts.reset = gen11_reset_guc_interrupts;
-		guc->interrupts.enable = gen11_enable_guc_interrupts;
-		guc->interrupts.disable = gen11_disable_guc_interrupts;
-	} else {
-		guc->notify = gen8_guc_raise_irq;
-		guc->interrupts.reset = gen9_reset_guc_interrupts;
-		guc->interrupts.enable = gen9_enable_guc_interrupts;
-		guc->interrupts.disable = gen9_disable_guc_interrupts;
-	}
-}
-
 static int guc_init_wq(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	/*
-	 * GuC log buffer flush work item has to do register access to
-	 * send the ack to GuC and this work item, if not synced before
-	 * suspend, can potentially get executed after the GFX device is
-	 * suspended.
-	 * By marking the WQ as freezable, we don't have to bother about
-	 * flushing of this work item from the suspend hooks, the pending
-	 * work item if any will be either executed before the suspend
-	 * or scheduled later on resume. This way the handling of work
-	 * item can be kept same between system suspend & rpm suspend.
-	 */
-	guc->log.relay.flush_wq =
-		alloc_ordered_workqueue("i915-guc_log",
-					WQ_HIGHPRI | WQ_FREEZABLE);
-	if (!guc->log.relay.flush_wq) {
-		DRM_ERROR("Couldn't allocate workqueue for GuC log\n");
-		return -ENOMEM;
-	}
-
 	/*
 	 * Even though both sending GuC action, and adding a new workitem to
 	 * GuC workqueue are serialized (each with its own locking), since
@@ -157,30 +113,50 @@ static void guc_fini_wq(struct intel_guc *guc)
 	wq = fetch_and_zero(&guc->preempt_wq);
 	if (wq)
 		destroy_workqueue(wq);
-
-	wq = fetch_and_zero(&guc->log.relay.flush_wq);
-	if (wq)
-		destroy_workqueue(wq);
 }
 
-int intel_guc_init_misc(struct intel_guc *guc)
+int intel_guc_init_early(struct intel_guc *guc)
 {
 	struct drm_i915_private *i915 = guc_to_i915(guc);
 	int ret;
 
-	ret = guc_init_wq(guc);
+	intel_guc_fw_init_early(guc);
+	intel_guc_ct_init_early(&guc->ct);
+	ret = intel_guc_log_init_early(&guc->log);
 	if (ret)
 		return ret;
 
-	intel_uc_fw_fetch(i915, &guc->fw);
+	ret = guc_init_wq(guc);
+	if (ret)
+		goto err_log;
 
+	mutex_init(&guc->send_mutex);
+	spin_lock_init(&guc->irq_lock);
+	guc->send = intel_guc_send_nop;
+	guc->handler = intel_guc_to_host_event_handler_nop;
+	if (INTEL_GEN(i915) >= 11) {
+		guc->notify = gen11_guc_raise_irq;
+		guc->interrupts.reset = gen11_reset_guc_interrupts;
+		guc->interrupts.enable = gen11_enable_guc_interrupts;
+		guc->interrupts.disable = gen11_disable_guc_interrupts;
+	} else {
+		guc->notify = gen8_guc_raise_irq;
+		guc->interrupts.reset = gen9_reset_guc_interrupts;
+		guc->interrupts.enable = gen9_enable_guc_interrupts;
+		guc->interrupts.disable = gen9_disable_guc_interrupts;
+	}
 	return 0;
+
+err_log:
+	intel_guc_log_fini_early(&guc->log);
+
+	return ret;
 }
 
-void intel_guc_fini_misc(struct intel_guc *guc)
+void intel_guc_fini_early(struct intel_guc *guc)
 {
-	intel_uc_fw_cleanup_fetch(&guc->fw);
 	guc_fini_wq(guc);
+	intel_guc_log_fini_early(&guc->log);
 }
 
 static int guc_shared_data_create(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/gt/intel_guc.h b/drivers/gpu/drm/i915/gt/intel_guc.h
index d6a75bc3d7f4..55185bd33f9f 100644
--- a/drivers/gpu/drm/i915/gt/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/intel_guc.h
@@ -155,13 +155,12 @@ static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
 	return offset;
 }
 
-void intel_guc_init_early(struct intel_guc *guc);
+int intel_guc_init_early(struct intel_guc *guc);
+void intel_guc_fini_early(struct intel_guc *guc);
 void intel_guc_init_send_regs(struct intel_guc *guc);
 void intel_guc_init_params(struct intel_guc *guc);
-int intel_guc_init_misc(struct intel_guc *guc);
 int intel_guc_init(struct intel_guc *guc);
 void intel_guc_fini(struct intel_guc *guc);
-void intel_guc_fini_misc(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
 		       u32 *response_buf, u32 response_buf_size);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
diff --git a/drivers/gpu/drm/i915/gt/intel_guc_log.c b/drivers/gpu/drm/i915/gt/intel_guc_log.c
index e3b83ecb90b5..4b1c7a9f23e7 100644
--- a/drivers/gpu/drm/i915/gt/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/gt/intel_guc_log.c
@@ -374,10 +374,40 @@ static void guc_log_unmap(struct intel_guc_log *log)
 	log->relay.buf_addr = NULL;
 }
 
-void intel_guc_log_init_early(struct intel_guc_log *log)
+int intel_guc_log_init_early(struct intel_guc_log *log)
 {
 	mutex_init(&log->relay.lock);
 	INIT_WORK(&log->relay.flush_work, capture_logs_work);
+
+	/*
+	 * GuC log buffer flush work item has to do register access to
+	 * send the ack to GuC and this work item, if not synced before
+	 * suspend, can potentially get executed after the GFX device is
+	 * suspended.
+	 * By marking the WQ as freezable, we don't have to bother about
+	 * flushing of this work item from the suspend hooks, the pending
+	 * work item if any will be either executed before the suspend
+	 * or scheduled later on resume. This way the handling of work
+	 * item can be kept same between system suspend & rpm suspend.
+	 */
+	log->relay.flush_wq =
+		alloc_ordered_workqueue("i915-guc_log",
+					WQ_HIGHPRI | WQ_FREEZABLE);
+	if (!log->relay.flush_wq) {
+		DRM_ERROR("Couldn't allocate workqueue for GuC log\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+void intel_guc_log_fini_early(struct intel_guc_log *log)
+{
+	struct workqueue_struct *wq;
+
+	wq = fetch_and_zero(&log->relay.flush_wq);
+	if (wq)
+		destroy_workqueue(wq);
 }
 
 static int guc_log_relay_create(struct intel_guc_log *log)
diff --git a/drivers/gpu/drm/i915/gt/intel_guc_log.h b/drivers/gpu/drm/i915/gt/intel_guc_log.h
index 7bc763f10c03..8b5e2fc7df24 100644
--- a/drivers/gpu/drm/i915/gt/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/gt/intel_guc_log.h
@@ -80,7 +80,8 @@ struct intel_guc_log {
 	} stats[GUC_MAX_LOG_BUFFER];
 };
 
-void intel_guc_log_init_early(struct intel_guc_log *log);
+int intel_guc_log_init_early(struct intel_guc_log *log);
+void intel_guc_log_fini_early(struct intel_guc_log *log);
 int intel_guc_log_create(struct intel_guc_log *log);
 void intel_guc_log_destroy(struct intel_guc_log *log);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_huc.c b/drivers/gpu/drm/i915/gt/intel_huc.c
index 64d879341811..581c9c3d4fc0 100644
--- a/drivers/gpu/drm/i915/gt/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/intel_huc.c
@@ -44,14 +44,6 @@ void intel_huc_init_early(struct intel_huc *huc)
 	}
 }
 
-int intel_huc_init_misc(struct intel_huc *huc)
-{
-	struct drm_i915_private *i915 = huc_to_i915(huc);
-
-	intel_uc_fw_fetch(i915, &huc->fw);
-	return 0;
-}
-
 static int intel_huc_rsa_data_create(struct intel_huc *huc)
 {
 	struct drm_i915_private *i915 = huc_to_i915(huc);
diff --git a/drivers/gpu/drm/i915/gt/intel_huc.h b/drivers/gpu/drm/i915/gt/intel_huc.h
index 2a6c94e79f17..9fa3d4629f2e 100644
--- a/drivers/gpu/drm/i915/gt/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/intel_huc.h
@@ -45,17 +45,11 @@ struct intel_huc {
 };
 
 void intel_huc_init_early(struct intel_huc *huc);
-int intel_huc_init_misc(struct intel_huc *huc);
 int intel_huc_init(struct intel_huc *huc);
 void intel_huc_fini(struct intel_huc *huc);
 int intel_huc_auth(struct intel_huc *huc);
 int intel_huc_check_status(struct intel_huc *huc);
 
-static inline void intel_huc_fini_misc(struct intel_huc *huc)
-{
-	intel_uc_fw_cleanup_fetch(&huc->fw);
-}
-
 static inline int intel_huc_sanitize(struct intel_huc *huc)
 {
 	intel_uc_fw_sanitize(&huc->fw);
diff --git a/drivers/gpu/drm/i915/gt/intel_uc.c b/drivers/gpu/drm/i915/gt/intel_uc.c
index 27a15cb377e5..b6acce9c6703 100644
--- a/drivers/gpu/drm/i915/gt/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/intel_uc.c
@@ -171,15 +171,19 @@ static void sanitize_options_early(struct drm_i915_private *i915)
 	GEM_BUG_ON(i915_modparams.guc_log_level < 0);
 }
 
-void intel_uc_init_early(struct drm_i915_private *i915)
+int intel_uc_init_early(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &i915->gt.uc.guc;
-	struct intel_huc *huc = &i915->gt.uc.huc;
+	int ret;
+
+	ret = intel_guc_init_early(&i915->gt.uc.guc);
+	if (ret)
+		return ret;
 
-	intel_guc_init_early(guc);
-	intel_huc_init_early(huc);
+	intel_huc_init_early(&i915->gt.uc.huc);
 
 	sanitize_options_early(i915);
+
+	return 0;
 }
 
 void intel_uc_cleanup_early(struct drm_i915_private *i915)
@@ -187,6 +191,8 @@ void intel_uc_cleanup_early(struct drm_i915_private *i915)
 	struct intel_guc *guc = &i915->gt.uc.guc;
 
 	guc_free_load_err_log(guc);
+
+	intel_guc_fini_early(guc);
 }
 
 /**
@@ -345,44 +351,26 @@ static void guc_disable_communication(struct intel_guc *guc)
 	DRM_INFO("GuC communication disabled\n");
 }
 
-int intel_uc_init_misc(struct drm_i915_private *i915)
+void intel_uc_fetch_firmwares(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &i915->gt.uc.guc;
-	struct intel_huc *huc = &i915->gt.uc.huc;
-	int ret;
-
 	if (!USES_GUC(i915))
-		return 0;
-
-	ret = intel_guc_init_misc(guc);
-	if (ret)
-		return ret;
+		return;
 
-	if (USES_HUC(i915)) {
-		ret = intel_huc_init_misc(huc);
-		if (ret)
-			goto err_guc;
-	}
+	intel_uc_fw_fetch(i915, &i915->gt.uc.guc.fw);
 
-	return 0;
-
-err_guc:
-	intel_guc_fini_misc(guc);
-	return ret;
+	if (USES_HUC(i915))
+		intel_uc_fw_fetch(i915, &i915->gt.uc.huc.fw);
 }
 
-void intel_uc_fini_misc(struct drm_i915_private *i915)
+void intel_uc_cleanup_firmwares(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &i915->gt.uc.guc;
-	struct intel_huc *huc = &i915->gt.uc.huc;
-
 	if (!USES_GUC(i915))
 		return;
 
 	if (USES_HUC(i915))
-		intel_huc_fini_misc(huc);
+		intel_uc_fw_cleanup_fetch(&i915->gt.uc.huc.fw);
 
-	intel_guc_fini_misc(guc);
+	intel_uc_fw_cleanup_fetch(&i915->gt.uc.guc.fw);
 }
 
 int intel_uc_init(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/gt/intel_uc.h b/drivers/gpu/drm/i915/gt/intel_uc.h
index a8373b3e818c..7934faea2dd2 100644
--- a/drivers/gpu/drm/i915/gt/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/intel_uc.h
@@ -33,11 +33,11 @@ struct intel_uc {
 	struct intel_huc huc;
 };
 
-void intel_uc_init_early(struct drm_i915_private *dev_priv);
+int intel_uc_init_early(struct drm_i915_private *dev_priv);
 void intel_uc_cleanup_early(struct drm_i915_private *dev_priv);
 void intel_uc_init_mmio(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);
+void intel_uc_fetch_firmwares(struct drm_i915_private *dev_priv);
+void intel_uc_cleanup_firmwares(struct drm_i915_private *dev_priv);
 void intel_uc_sanitize(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);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 14ec0d9f71f3..72d13b3c4428 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -934,7 +934,11 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
 	intel_detect_pch(dev_priv);
 
 	intel_wopcm_init_early(&dev_priv->wopcm);
-	intel_uc_init_early(dev_priv);
+
+	ret = intel_uc_init_early(dev_priv);
+	if (ret)
+		goto err_gem;
+
 	intel_pm_setup(dev_priv);
 	intel_init_dpio(dev_priv);
 	ret = intel_power_domains_init(dev_priv);
@@ -953,6 +957,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
 
 err_uc:
 	intel_uc_cleanup_early(dev_priv);
+err_gem:
 	i915_gem_cleanup_early(dev_priv);
 err_workqueues:
 	i915_workqueues_cleanup(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b7f290b77f8f..46a75844f303 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1429,13 +1429,11 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
-	ret = intel_uc_init_misc(dev_priv);
-	if (ret)
-		return ret;
+	intel_uc_fetch_firmwares(dev_priv);
 
 	ret = intel_wopcm_init(&dev_priv->wopcm);
 	if (ret)
-		goto err_uc_misc;
+		goto err_uc_fw;
 
 	/* This is just a security blanket to placate dragons.
 	 * On some systems, we very sporadically observe that the first TLBs
@@ -1561,8 +1559,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-err_uc_misc:
-	intel_uc_fini_misc(dev_priv);
+err_uc_fw:
+	intel_uc_cleanup_firmwares(dev_priv);
 
 	if (ret != -EIO) {
 		i915_gem_cleanup_userptr(dev_priv);
@@ -1628,7 +1626,7 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
 
 	intel_cleanup_gt_powersave(dev_priv);
 
-	intel_uc_fini_misc(dev_priv);
+	intel_uc_cleanup_firmwares(dev_priv);
 	i915_gem_cleanup_userptr(dev_priv);
 	intel_timelines_fini(dev_priv);
 
-- 
2.20.1



More information about the Intel-gfx-trybot mailing list