diff mbox series

[v2] drm/i915/uc: Log fw status changes only under debug config

Message ID 20190813081559.23936-1-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915/uc: Log fw status changes only under debug config | expand

Commit Message

Michal Wajdeczko Aug. 13, 2019, 8:15 a.m. UTC
We don't care about internal firmware status changes unless
we are doing some real debugging. Note that our CI is not
using DRM_I915_DEBUG_GUC config by default so use it.

v2: protect against accidental overwrites (Chris)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c |  4 +-
 drivers/gpu/drm/i915/gt/uc/intel_huc.c    |  4 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 52 ++++++++++++++---------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  | 18 +++++++-
 drivers/gpu/drm/i915/i915_gpu_error.c     |  4 +-
 5 files changed, 55 insertions(+), 27 deletions(-)

Comments

Chris Wilson Aug. 13, 2019, 8:19 a.m. UTC | #1
Quoting Michal Wajdeczko (2019-08-13 09:15:59)
> We don't care about internal firmware status changes unless
> we are doing some real debugging. Note that our CI is not
> using DRM_I915_DEBUG_GUC config by default so use it.
> 
> v2: protect against accidental overwrites (Chris)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Chris Wilson Aug. 13, 2019, 9:50 a.m. UTC | #2
Quoting Patchwork (2019-08-13 09:53:21)
> == Series Details ==
> 
> Series: drm/i915/uc: Log fw status changes only under debug config (rev2)
> URL   : https://patchwork.freedesktop.org/series/65101/
> State : success
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_6691 -> Patchwork_13996
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.

Any pushed. Confident in the module load testing for something that
only happens at module load!
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 445dcea51e51..5528224448f6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -157,10 +157,10 @@  int intel_guc_fw_upload(struct intel_guc *guc)
 	if (ret)
 		goto out;
 
-	guc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
+	intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_RUNNING);
 	return 0;
 
 out:
-	guc->fw.status = INTEL_UC_FIRMWARE_FAIL;
+	intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_FAIL);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 75dade1cfbca..1edda1657411 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -147,12 +147,12 @@  int intel_huc_auth(struct intel_huc *huc)
 		goto fail;
 	}
 
-	huc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
+	intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_RUNNING);
 	return 0;
 
 fail:
 	i915_probe_error(gt->i915, "HuC: Authentication failed %d\n", ret);
-	huc->fw.status = INTEL_UC_FIRMWARE_FAIL;
+	intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_FAIL);
 	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 302eb1b909dc..d056e1f4bd6d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -11,6 +11,29 @@ 
 #include "intel_uc_fw_abi.h"
 #include "i915_drv.h"
 
+#ifdef CONFIG_DRM_I915_DEBUG_GUC
+static inline struct intel_gt *__uc_fw_to_gt(struct intel_uc_fw *uc_fw)
+{
+	GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);
+	if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
+		return container_of(uc_fw, struct intel_gt, uc.guc.fw);
+
+	GEM_BUG_ON(uc_fw->type != INTEL_UC_FW_TYPE_HUC);
+	return container_of(uc_fw, struct intel_gt, uc.huc.fw);
+}
+
+void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
+			       enum intel_uc_fw_status status)
+{
+	uc_fw->__status =  status;
+	DRM_DEV_DEBUG_DRIVER(__uc_fw_to_gt(uc_fw)->i915->drm.dev,
+			     "%s firmware -> %s\n",
+			     intel_uc_fw_type_repr(uc_fw->type),
+			     status == INTEL_UC_FIRMWARE_SELECTED ?
+			     uc_fw->path : intel_uc_fw_status_repr(status));
+}
+#endif
+
 /*
  * List of required GuC and HuC binaries per-platform.
  * Must be ordered based on platform + revid, from newer to older.
@@ -183,10 +206,9 @@  void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 		__uc_fw_user_override(uc_fw);
 	}
 
-	if (uc_fw->path && *uc_fw->path)
-		uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
-	else
-		uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
+	intel_uc_fw_change_status(uc_fw, uc_fw->path && *uc_fw->path ?
+				  INTEL_UC_FIRMWARE_SELECTED :
+				  INTEL_UC_FIRMWARE_NOT_SUPPORTED);
 }
 
 static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw,
@@ -343,20 +365,15 @@  int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 
 	uc_fw->obj = obj;
 	uc_fw->size = fw->size;
-	uc_fw->status = INTEL_UC_FIRMWARE_AVAILABLE;
-
-	DRM_DEV_DEBUG_DRIVER(dev, "%s firmware %s: %s\n",
-			     intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
-			     intel_uc_fw_status_repr(uc_fw->status));
+	intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_AVAILABLE);
 
 	release_firmware(fw);
 	return 0;
 
 fail:
-	if (err == -ENOENT)
-		uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
-	else
-		uc_fw->status = INTEL_UC_FIRMWARE_ERROR;
+	intel_uc_fw_change_status(uc_fw, err == -ENOENT ?
+				  INTEL_UC_FIRMWARE_MISSING :
+				  INTEL_UC_FIRMWARE_ERROR);
 
 	dev_notice(dev, "%s firmware %s: fetch failed with error %d\n",
 		   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
@@ -491,17 +508,14 @@  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
 	if (err)
 		goto fail;
 
-	uc_fw->status = INTEL_UC_FIRMWARE_TRANSFERRED;
-	DRM_DEV_DEBUG_DRIVER(gt->i915->drm.dev, "%s firmware %s: %s\n",
-			     intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
-			     intel_uc_fw_status_repr(uc_fw->status));
+	intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_TRANSFERRED);
 	return 0;
 
 fail:
 	i915_probe_error(gt->i915, "Failed to load %s firmware %s (%d)\n",
 			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
 			 err);
-	uc_fw->status = INTEL_UC_FIRMWARE_FAIL;
+	intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_FAIL);
 	return err;
 }
 
@@ -546,7 +560,7 @@  void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw)
 	if (obj)
 		i915_gem_object_put(obj);
 
-	uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
+	intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_SELECTED);
 }
 
 /**
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 20a5ddb753c3..ce8e83128a95 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -42,7 +42,10 @@  enum intel_uc_fw_type {
  */
 struct intel_uc_fw {
 	enum intel_uc_fw_type type;
-	enum intel_uc_fw_status status;
+	union {
+		const enum intel_uc_fw_status status;
+		enum intel_uc_fw_status __status; /* no accidental overwrites */
+	};
 	const char *path;
 	bool user_overridden;
 	size_t size;
@@ -62,6 +65,17 @@  struct intel_uc_fw {
 	u32 ucode_size;
 };
 
+#ifdef CONFIG_DRM_I915_DEBUG_GUC
+void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
+			       enum intel_uc_fw_status status);
+#else
+static inline void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
+					     enum intel_uc_fw_status status)
+{
+	uc_fw->__status = status;
+}
+#endif
+
 static inline
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 {
@@ -156,7 +170,7 @@  static inline bool intel_uc_fw_is_overridden(const struct intel_uc_fw *uc_fw)
 static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
 {
 	if (intel_uc_fw_is_loaded(uc_fw))
-		uc_fw->status = INTEL_UC_FIRMWARE_AVAILABLE;
+		intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_AVAILABLE);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 92986d3f6995..6718fde1ed4c 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1475,8 +1475,8 @@  capture_uc_state(struct i915_gpu_state *error, struct compress *compress)
 	if (!error->device_info.has_gt_uc)
 		return;
 
-	error_uc->guc_fw = uc->guc.fw;
-	error_uc->huc_fw = uc->huc.fw;
+	memcpy(&error_uc->guc_fw, &uc->guc.fw, sizeof(uc->guc.fw));
+	memcpy(&error_uc->huc_fw, &uc->huc.fw, sizeof(uc->huc.fw));
 
 	/* Non-default firmware paths will be specified by the modparam.
 	 * As modparams are generally accesible from the userspace make