All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 03/12] drm/i915/uc: replace uc init/fini misc
Date: Tue,  9 Jul 2019 17:54:28 -0700	[thread overview]
Message-ID: <20190710005437.3496-4-daniele.ceraolospurio@intel.com> (raw)
In-Reply-To: <20190710005437.3496-1-daniele.ceraolospurio@intel.com>

The "misc" terminology doesn't clearly explain what we intend to cover
in this phase. The only thing we do in there apart from FW fetch is
initializing the log workqueue, with the latter being required only in
the very rare case where we enable the log relay. To clean this up, we
can move the wq init to when the relay is enabled and rename the
function to clarify that they only fetch/release the blobs.

v2: only create log wq when needed (Michal), reword commit msg
accordingly

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c      | 12 +++----
 drivers/gpu/drm/i915/intel_guc.c     | 53 ----------------------------
 drivers/gpu/drm/i915/intel_guc.h     |  2 --
 drivers/gpu/drm/i915/intel_guc_log.c | 31 +++++++++++++++-
 drivers/gpu/drm/i915/intel_huc.c     |  8 -----
 drivers/gpu/drm/i915/intel_huc.h     |  6 ----
 drivers/gpu/drm/i915/intel_uc.c      | 34 +++++-------------
 drivers/gpu/drm/i915/intel_uc.h      |  4 +--
 8 files changed, 45 insertions(+), 105 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7ade42b8ec99..e6e8e4d5ebb1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1433,13 +1433,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
@@ -1565,8 +1563,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);
@@ -1632,7 +1630,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);
 
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 501b74f44374..4173b35bf104 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -99,59 +99,6 @@ void intel_guc_init_early(struct intel_guc *guc)
 	}
 }
 
-static int guc_init_wq(struct intel_guc *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;
-	}
-
-	return 0;
-}
-
-static void guc_fini_wq(struct intel_guc *guc)
-{
-	struct workqueue_struct *wq;
-
-	wq = fetch_and_zero(&guc->log.relay.flush_wq);
-	if (wq)
-		destroy_workqueue(wq);
-}
-
-int intel_guc_init_misc(struct intel_guc *guc)
-{
-	struct drm_i915_private *i915 = guc_to_i915(guc);
-	int ret;
-
-	ret = guc_init_wq(guc);
-	if (ret)
-		return ret;
-
-	intel_uc_fw_fetch(i915, &guc->fw);
-
-	return 0;
-}
-
-void intel_guc_fini_misc(struct intel_guc *guc)
-{
-	intel_uc_fw_cleanup_fetch(&guc->fw);
-	guc_fini_wq(guc);
-}
-
 static int guc_shared_data_create(struct intel_guc *guc)
 {
 	struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index ec1038c1f50e..91d538fd5f65 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -153,10 +153,8 @@ static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
 void intel_guc_init_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/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 06c09ac52c74..0355724ee997 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -79,6 +79,9 @@ static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
 
 static void guc_log_enable_flush_events(struct intel_guc_log *log)
 {
+	/* relay wq must be ready for us to queue flush events */
+	GEM_BUG_ON(!log->relay.flush_wq);
+
 	intel_guc_enable_msg(log_to_guc(log),
 			     INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
 			     INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED);
@@ -86,6 +89,9 @@ static void guc_log_enable_flush_events(struct intel_guc_log *log)
 
 static void guc_log_disable_flush_events(struct intel_guc_log *log)
 {
+	/* relay wq must stick around until we disable flush events */
+	GEM_BUG_ON(!log->relay.flush_wq);
+
 	intel_guc_disable_msg(log_to_guc(log),
 			      INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
 			      INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED);
@@ -561,9 +567,29 @@ int intel_guc_log_relay_open(struct intel_guc_log *log)
 		goto out_unlock;
 	}
 
+	/*
+	 * 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");
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
+
 	ret = guc_log_relay_create(log);
 	if (ret)
-		goto out_unlock;
+		goto out_wq;
 
 	ret = guc_log_map(log);
 	if (ret)
@@ -584,6 +610,8 @@ int intel_guc_log_relay_open(struct intel_guc_log *log)
 
 out_relay:
 	guc_log_relay_destroy(log);
+out_wq:
+	destroy_workqueue(fetch_and_zero(&log->relay.flush_wq));
 out_unlock:
 	mutex_unlock(&log->relay.lock);
 
@@ -623,6 +651,7 @@ void intel_guc_log_relay_close(struct intel_guc_log *log)
 	GEM_BUG_ON(!intel_guc_log_relay_enabled(log));
 	guc_log_unmap(log);
 	guc_log_relay_destroy(log);
+	destroy_workqueue(fetch_and_zero(&log->relay.flush_wq));
 	mutex_unlock(&log->relay.lock);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index fb6f693d3cac..2a41ee89a16d 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/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/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 2a6c94e79f17..9fa3d4629f2e 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/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/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index fdf00f1ebb57..789b0bccfb41 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -345,44 +345,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->guc;
-	struct intel_huc *huc = &i915->huc;
-	int ret;
-
 	if (!USES_GUC(i915))
-		return 0;
-
-	ret = intel_guc_init_misc(guc);
-	if (ret)
-		return ret;
-
-	if (USES_HUC(i915)) {
-		ret = intel_huc_init_misc(huc);
-		if (ret)
-			goto err_guc;
-	}
+		return;
 
-	return 0;
+	intel_uc_fw_fetch(i915, &i915->guc.fw);
 
-err_guc:
-	intel_guc_fini_misc(guc);
-	return ret;
+	if (USES_HUC(i915))
+		intel_uc_fw_fetch(i915, &i915->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->guc;
-	struct intel_huc *huc = &i915->huc;
-
 	if (!USES_GUC(i915))
 		return;
 
 	if (USES_HUC(i915))
-		intel_huc_fini_misc(huc);
+		intel_uc_fw_cleanup_fetch(&i915->huc.fw);
 
-	intel_guc_fini_misc(guc);
+	intel_uc_fw_cleanup_fetch(&i915->guc.fw);
 }
 
 int intel_uc_init(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 3ea06c87dfcd..5a1383e192dd 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -31,8 +31,8 @@
 void 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);
-- 
2.20.1

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

  parent reply	other threads:[~2019-07-10  0:55 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-10  0:54 [PATCH 00/12] GT-fy the uc code Daniele Ceraolo Spurio
2019-07-10  0:54 ` [PATCH 01/12] drm/i915/guc: Remove preemption support for current fw Daniele Ceraolo Spurio
2019-07-10 15:45   ` Michał Winiarski
2019-07-10  0:54 ` [PATCH 02/12] drm/i915/guc: simplify guc client Daniele Ceraolo Spurio
2019-07-10 15:52   ` Michał Winiarski
2019-07-10  0:54 ` Daniele Ceraolo Spurio [this message]
2019-07-10  0:54 ` [PATCH 04/12] drm/i915/uc: introduce intel_uc_fw_supported Daniele Ceraolo Spurio
2019-07-10 16:57   ` Michal Wajdeczko
2019-07-10 21:46     ` Daniele Ceraolo Spurio
2019-07-11 12:48       ` Michal Wajdeczko
2019-07-10  0:54 ` [PATCH 05/12] drm/i915/guc: move guc irq functions to intel_guc parameter Daniele Ceraolo Spurio
2019-07-10  0:54 ` [PATCH 06/12] drm/i915/guc: unify guc irq handling Daniele Ceraolo Spurio
2019-07-10 17:04   ` Michal Wajdeczko
2019-07-10  0:54 ` [PATCH 07/12] drm/i915/uc: move GuC and HuC files under gt/uc/ Daniele Ceraolo Spurio
2019-07-10 17:52   ` Michal Wajdeczko
2019-07-10  0:54 ` [PATCH 08/12] drm/i915/uc: move GuC/HuC inside intel_gt under a new intel_uc Daniele Ceraolo Spurio
2019-07-10 18:22   ` Michal Wajdeczko
2019-07-10  0:54 ` [PATCH 09/12] drm/i915/uc: Move intel functions to intel_uc Daniele Ceraolo Spurio
2019-07-10 18:26   ` Michal Wajdeczko
2019-07-10  0:54 ` [PATCH 10/12] drm/i915/uc: prefer intel_gt over i915 in GuC/HuC paths Daniele Ceraolo Spurio
2019-07-10 18:35   ` Michal Wajdeczko
2019-07-10  0:54 ` [PATCH 11/12] drm/i915/guc: prefer intel_gt in guc interrupt functions Daniele Ceraolo Spurio
2019-07-10  0:54 ` [PATCH 12/12] drm/i915/uc: kill <g,h>uc_to_i915 Daniele Ceraolo Spurio
2019-07-10 18:37   ` Michal Wajdeczko
2019-07-10  1:12 ` ✗ Fi.CI.CHECKPATCH: warning for GT-fy the uc code Patchwork
2019-07-10  1:19 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-07-10  1:35 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-07-10 13:02 ` [PATCH 00/12] " Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190710005437.3496-4-daniele.ceraolospurio@intel.com \
    --to=daniele.ceraolospurio@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.