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 5/9] drm/i915/uc: Unify uc_fw status tracking
Date: Mon, 22 Jul 2019 16:20:44 -0700	[thread overview]
Message-ID: <20190722232048.9970-6-daniele.ceraolospurio@intel.com> (raw)
In-Reply-To: <20190722232048.9970-1-daniele.ceraolospurio@intel.com>

We currently track fetch and load status separately, but the 2 are
actually sequential in the uc lifetime (fetch must complete before we
can attempt the load!). Unifying the 2 variables we can better follow
the sequential states and improve our trackng of the uC state.

Also, sprinkle some GEM_BUG_ON to make sure we transition correctly
between states.

Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/uc/intel_huc.c   |  4 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 74 ++++++++++--------------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 50 ++++++++--------
 3 files changed, 57 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index bc14439173d7..1868f676d890 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -117,7 +117,7 @@ int intel_huc_auth(struct intel_huc *huc)
 	struct intel_guc *guc = &gt->uc.guc;
 	int ret;
 
-	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (huc->fw.status != INTEL_UC_FIRMWARE_LOADED)
 		return -ENOEXEC;
 
 	ret = intel_guc_auth_huc(guc,
@@ -141,7 +141,7 @@ int intel_huc_auth(struct intel_huc *huc)
 	return 0;
 
 fail:
-	huc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
+	huc->fw.status = INTEL_UC_FIRMWARE_LOAD_FAIL;
 
 	DRM_ERROR("HuC: Authentication failed %d\n", ret);
 	return ret;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index faa96748aed3..91b1d9663481 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -45,15 +45,13 @@ void intel_uc_fw_select(struct drm_i915_private *i915,
 			const struct intel_uc_platform_requirement *list,
 			unsigned int length)
 {
-	GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED);
+	GEM_BUG_ON(uc_fw->status != INTEL_UC_FIRMWARE_UNINITIALIZED);
 
 	if (!HAS_UC(i915)) {
-		uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
+		uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
 		return;
 	}
 
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
-
 	if (unlikely(i915_modparams.guc_firmware_path &&
 		     uc_fw->type == INTEL_UC_FW_TYPE_GUC)) {
 		uc_fw->path = i915_modparams.guc_firmware_path;
@@ -74,6 +72,8 @@ void intel_uc_fw_select(struct drm_i915_private *i915,
 			}
 		}
 	}
+
+	uc_fw->status = INTEL_UC_FIRMWARE_SELECTION_DONE;
 }
 
 /**
@@ -95,23 +95,11 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	int err;
 
 	GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
-
-	if (!uc_fw->path) {
-		dev_info(dev_priv->drm.dev,
-			 "%s: No firmware was defined for %s!\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 intel_platform_name(INTEL_INFO(dev_priv)->platform));
-		return;
-	}
+	GEM_BUG_ON(!intel_uc_fw_is_selected(uc_fw));
 
 	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
 			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
 
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
-	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 intel_uc_fw_status_repr(uc_fw->fetch_status));
-
 	err = request_firmware(&fw, uc_fw->path, &pdev->dev);
 	if (err) {
 		DRM_DEBUG_DRIVER("%s fw request_firmware err=%d\n",
@@ -219,19 +207,17 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 
 	uc_fw->obj = obj;
 	uc_fw->size = fw->size;
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
-	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 intel_uc_fw_status_repr(uc_fw->fetch_status));
+	uc_fw->status = INTEL_UC_FIRMWARE_FETCHED;
+	DRM_DEBUG_DRIVER("%s fw fetch done\n",
+			 intel_uc_fw_type_repr(uc_fw->type));
 
 	release_firmware(fw);
 	return;
 
 fail:
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
-	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 intel_uc_fw_status_repr(uc_fw->fetch_status));
+	uc_fw->status = INTEL_UC_FIRMWARE_FETCH_FAIL;
+	DRM_DEBUG_DRIVER("%s fw fetch failed\n",
+			 intel_uc_fw_type_repr(uc_fw->type));
 
 	DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
 		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
@@ -287,13 +273,11 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 	DRM_DEBUG_DRIVER("%s fw load %s\n",
 			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
 
-	if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return -ENOEXEC;
+	/* make sure the status was cleared the last time we reset the uc */
+	GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_LOADED);
 
-	uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
-	DRM_DEBUG_DRIVER("%s fw load %s\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 intel_uc_fw_status_repr(uc_fw->load_status));
+	if (uc_fw->status < INTEL_UC_FIRMWARE_FETCHED)
+		return -ENOEXEC;
 
 	/* Call custom loader */
 	intel_uc_fw_ggtt_bind(uc_fw);
@@ -302,10 +286,9 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 	if (err)
 		goto fail;
 
-	uc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
-	DRM_DEBUG_DRIVER("%s fw load %s\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 intel_uc_fw_status_repr(uc_fw->load_status));
+	uc_fw->status = INTEL_UC_FIRMWARE_LOADED;
+	DRM_DEBUG_DRIVER("%s fw load completed\n",
+			 intel_uc_fw_type_repr(uc_fw->type));
 
 	DRM_INFO("%s: Loaded firmware %s (version %u.%u)\n",
 		 intel_uc_fw_type_repr(uc_fw->type),
@@ -315,10 +298,9 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 	return 0;
 
 fail:
-	uc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
-	DRM_DEBUG_DRIVER("%s fw load %s\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 intel_uc_fw_status_repr(uc_fw->load_status));
+	uc_fw->status = INTEL_UC_FIRMWARE_LOAD_FAIL;
+	DRM_DEBUG_DRIVER("%s fw load failed\n",
+			 intel_uc_fw_type_repr(uc_fw->type));
 
 	DRM_WARN("%s: Failed to load firmware %s (error %d)\n",
 		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
@@ -330,7 +312,10 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
 {
 	int err;
 
-	if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+	/* this should happen before the load! */
+	GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_LOADED);
+
+	if (uc_fw->status < INTEL_UC_FIRMWARE_FETCHED)
 		return -ENOEXEC;
 
 	err = i915_gem_object_pin_pages(uc_fw->obj);
@@ -343,7 +328,7 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
 
 void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
 {
-	if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (uc_fw->status < INTEL_UC_FIRMWARE_FETCHED)
 		return;
 
 	i915_gem_object_unpin_pages(uc_fw->obj);
@@ -377,7 +362,7 @@ void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw)
 	if (obj)
 		i915_gem_object_put(obj);
 
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+	uc_fw->status = INTEL_UC_FIRMWARE_SELECTION_DONE;
 }
 
 /**
@@ -391,9 +376,8 @@ void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p)
 {
 	drm_printf(p, "%s firmware: %s\n",
 		   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
-	drm_printf(p, "\tstatus: fetch %s, load %s\n",
-		   intel_uc_fw_status_repr(uc_fw->fetch_status),
-		   intel_uc_fw_status_repr(uc_fw->load_status));
+	drm_printf(p, "\tstatus: %s\n",
+		   intel_uc_fw_status_repr(uc_fw->status));
 	drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n",
 		   uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted,
 		   uc_fw->major_ver_found, uc_fw->minor_ver_found);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 550b626afb15..e0c5a38685e6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -36,12 +36,13 @@ struct drm_i915_private;
 #define INTEL_UC_FIRMWARE_URL "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915"
 
 enum intel_uc_fw_status {
-	INTEL_UC_FIRMWARE_NOT_SUPPORTED = -2, /* no uc HW */
-	INTEL_UC_FIRMWARE_FAIL = -1,
+	INTEL_UC_FIRMWARE_LOAD_FAIL = -3,
+	INTEL_UC_FIRMWARE_FETCH_FAIL = -2,
+	INTEL_UC_FIRMWARE_NOT_SUPPORTED = -1, /* no uc HW */
 	INTEL_UC_FIRMWARE_UNINITIALIZED = 0, /* used to catch checks done too early */
-	INTEL_UC_FIRMWARE_NOT_STARTED = 1,
-	INTEL_UC_FIRMWARE_PENDING,
-	INTEL_UC_FIRMWARE_SUCCESS
+	INTEL_UC_FIRMWARE_SELECTION_DONE, /* selection include the "no FW" case */
+	INTEL_UC_FIRMWARE_FETCHED,
+	INTEL_UC_FIRMWARE_LOADED
 };
 
 enum intel_uc_fw_type {
@@ -77,8 +78,7 @@ struct intel_uc_fw {
 	const char *path;
 	size_t size;
 	struct drm_i915_gem_object *obj;
-	enum intel_uc_fw_status fetch_status;
-	enum intel_uc_fw_status load_status;
+	enum intel_uc_fw_status status;
 
 	/*
 	 * The firmware build process will generate a version header file with major and
@@ -103,18 +103,20 @@ static inline
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 {
 	switch (status) {
+	case INTEL_UC_FIRMWARE_LOAD_FAIL:
+		return "LOAD FAIL";
+	case INTEL_UC_FIRMWARE_FETCH_FAIL:
+		return "FETCH FAIL";
 	case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
-		return "N/A - uc HW not available";
-	case INTEL_UC_FIRMWARE_FAIL:
-		return "FAIL";
+		return "N/A";
 	case INTEL_UC_FIRMWARE_UNINITIALIZED:
 		return "UNINITIALIZED";
-	case INTEL_UC_FIRMWARE_NOT_STARTED:
-		return "NOT_STARTED";
-	case INTEL_UC_FIRMWARE_PENDING:
-		return "PENDING";
-	case INTEL_UC_FIRMWARE_SUCCESS:
-		return "SUCCESS";
+	case INTEL_UC_FIRMWARE_SELECTION_DONE:
+		return "SELECTION DONE";
+	case INTEL_UC_FIRMWARE_FETCHED:
+		return "FETCHED";
+	case INTEL_UC_FIRMWARE_LOADED:
+		return "LOADED";
 	}
 	return "<invalid>";
 }
@@ -135,38 +137,38 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 			    enum intel_uc_fw_type type)
 {
 	/*
-	 * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status
+	 * we use FIRMWARE_UNINITIALIZED to detect checks against uc_fw->status
 	 * before we're looked at the HW caps to see if we have uc support
 	 */
 	BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED);
 
 	uc_fw->path = NULL;
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED;
-	uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+	uc_fw->status = INTEL_UC_FIRMWARE_UNINITIALIZED;
 	uc_fw->type = type;
 }
 
 static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw)
 {
+	GEM_BUG_ON(uc_fw->path && uc_fw->status < INTEL_UC_FIRMWARE_SELECTION_DONE);
 	return uc_fw->path != NULL;
 }
 
 static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
 {
-	return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
+	return uc_fw->status == INTEL_UC_FIRMWARE_LOADED;
 }
 
 static inline bool intel_uc_fw_supported(struct intel_uc_fw *uc_fw)
 {
 	/* shouldn't call this before checking hw/blob availability */
-	GEM_BUG_ON(uc_fw->fetch_status == INTEL_UC_FIRMWARE_UNINITIALIZED);
-	return uc_fw->fetch_status != INTEL_UC_FIRMWARE_NOT_SUPPORTED;
+	GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);
+	return uc_fw->status != INTEL_UC_FIRMWARE_NOT_SUPPORTED;
 }
 
 static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
 {
 	if (intel_uc_fw_is_loaded(uc_fw))
-		uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+		uc_fw->status = INTEL_UC_FIRMWARE_FETCHED;
 }
 
 /**
@@ -179,7 +181,7 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
  */
 static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
 {
-	if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (uc_fw->status < INTEL_UC_FIRMWARE_FETCHED)
 		return 0;
 
 	return uc_fw->header_size + uc_fw->ucode_size;
-- 
2.22.0

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

  parent reply	other threads:[~2019-07-22 23:21 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-22 23:20 [PATCH 0/9] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
2019-07-22 23:20 ` [PATCH 1/9] drm/i915/uc: Gt-fy uc reset Daniele Ceraolo Spurio
2019-07-23  7:47   ` Chris Wilson
2019-07-22 23:20 ` [PATCH 2/9] drm/i915/uc: Unify uC platform check Daniele Ceraolo Spurio
2019-07-23  8:21   ` Chris Wilson
2019-07-23 11:19   ` Michal Wajdeczko
2019-07-23 14:52     ` Daniele Ceraolo Spurio
2019-07-23 21:15       ` Daniele Ceraolo Spurio
2019-07-22 23:20 ` [PATCH 3/9] drm/i915/uc: Unify uC FW selection Daniele Ceraolo Spurio
2019-07-23  8:26   ` Chris Wilson
2019-07-23 13:22   ` Michal Wajdeczko
2019-07-23 15:01     ` Daniele Ceraolo Spurio
2019-07-22 23:20 ` [PATCH 4/9] drm/i915/uc: Sanitize uC when GT is sanitized Daniele Ceraolo Spurio
2019-07-23  8:00   ` Chris Wilson
2019-07-22 23:20 ` Daniele Ceraolo Spurio [this message]
2019-07-23  8:28   ` [PATCH 5/9] drm/i915/uc: Unify uc_fw status tracking Chris Wilson
2019-07-23 14:20   ` Michal Wajdeczko
2019-07-22 23:20 ` [PATCH 6/9] drm/i915/uc: Move xfer rsa logic to common function Daniele Ceraolo Spurio
2019-07-23  8:34   ` Chris Wilson
2019-07-22 23:20 ` [PATCH 7/9] drm/i915/huc: Copy huc rsa only once Daniele Ceraolo Spurio
2019-07-23  8:37   ` Chris Wilson
2019-07-22 23:20 ` [PATCH 8/9] drm/i915/uc: Plumb the gt through fw_upload Daniele Ceraolo Spurio
2019-07-23  8:39   ` Chris Wilson
2019-07-22 23:20 ` [PATCH 9/9] drm/i915/uc: Unify uC firmware upload Daniele Ceraolo Spurio
2019-07-23  8:45   ` Chris Wilson
2019-07-23  0:14 ` ✗ Fi.CI.CHECKPATCH: warning for uC fw path unification + misc clean-up Patchwork
2019-07-23  0:19 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-07-23  1:08 ` ✓ Fi.CI.BAT: success " Patchwork
2019-07-23  4:54 ` ✓ Fi.CI.IGT: " Patchwork
2019-07-23  8:01 ` [PATCH 0/9] " 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=20190722232048.9970-6-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.