[Intel-gfx] [PATCH 3/5] drm/i915/guc: Simplify intel_guc_load()

Arkadiusz Hiler arkadiusz.hiler at intel.com
Thu Dec 15 15:47:06 UTC 2016


Current version of intel_guc_load() does a lot:
 - cares about submission
 - loads huc
 - implement WA

This change offloads some of the logic to intel_uc_load(), which now
cares about the above.

Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
Cc: Jeff McGee <jeff.mcgee at intel.com>
Cc: Michal Winiarski <michal.winiarski at intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |   2 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 126 +++++---------------------------
 drivers/gpu/drm/i915/intel_uc.c         |  83 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h         |   8 ++
 4 files changed, 110 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6af4e85..76b52c6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4412,7 +4412,7 @@ i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	intel_mocs_init_l3cc_table(dev_priv);
 
 	/* We can't enable contexts until all firmware is loaded */
-	ret = intel_guc_load(dev_priv);
+	ret = intel_uc_load(dev_priv);
 	if (ret)
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index f8b28b1..b76b556 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -97,7 +97,7 @@ const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status)
 	}
 };
 
-static void guc_interrupts_release(struct drm_i915_private *dev_priv)
+void guc_interrupts_release(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
@@ -115,7 +115,7 @@ static void guc_interrupts_release(struct drm_i915_private *dev_priv)
 	I915_WRITE(GUC_WD_VECS_IER, 0);
 }
 
-static void guc_interrupts_capture(struct drm_i915_private *dev_priv)
+void guc_interrupts_capture(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
@@ -334,7 +334,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
-static u32 guc_wopcm_size(struct drm_i915_private *dev_priv)
+u32 guc_wopcm_size(struct drm_i915_private *dev_priv)
 {
 	u32 wopcm_size = GUC_WOPCM_TOP;
 
@@ -417,7 +417,7 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-static int guc_hw_reset(struct drm_i915_private *dev_priv)
+int guc_hw_reset(struct drm_i915_private *dev_priv)
 {
 	int ret;
 	u32 guc_status;
@@ -452,75 +452,37 @@ int intel_guc_load(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
 	const char *fw_path = guc_fw->guc_fw_path;
-	int retries, ret, err;
+	int ret;
 
 	DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
 		fw_path,
 		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
-	/* Loading forbidden, or no firmware to load? */
-	if (!i915.enable_guc_loading) {
-		err = 0;
-		goto fail;
-	} else if (fw_path == NULL) {
+	if (fw_path == NULL) {
 		/* Device is known to have no uCode (e.g. no GuC) */
-		err = -ENXIO;
-		goto fail;
+		return -ENXIO;
 	} else if (*fw_path == '\0') {
 		/* Device has a GuC but we don't know what f/w to load? */
 		WARN(1, "No GuC firmware known for this platform!\n");
-		err = -ENODEV;
-		goto fail;
+		return -ENODEV;
 	}
 
 	/* Fetch failed, or already fetched but failed to load? */
 	if (guc_fw->guc_fw_fetch_status != GUC_FIRMWARE_SUCCESS) {
-		err = -EIO;
-		goto fail;
+		return -EIO;
 	} else if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_FAIL) {
-		err = -ENOEXEC;
-		goto fail;
+		return -ENOEXEC;
 	}
 
-	guc_interrupts_release(dev_priv);
-	gen9_reset_guc_interrupts(dev_priv);
-
 	guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
 
-	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
-		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
-		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
-
-	err = i915_guc_submission_init(dev_priv);
-	if (err)
-		goto fail;
-
-	/*
-	 * WaEnableuKernelHeaderValidFix:skl,bxt
-	 * For BXT, this is only upto B0 but below WA is required for later
-	 * steppings also so this is extended as well.
-	 */
 	/* WaEnableGuCBootHashCheckNotSet:skl,bxt */
-	for (retries = 3; ; ) {
-		/*
-		 * Always reset the GuC just before (re)loading, so
-		 * that the state and timing are fairly predictable
-		 */
-		err = guc_hw_reset(dev_priv);
-		if (err)
-			goto fail;
+	/* we may want to retry guc ucode transfer */
+	ret = guc_ucode_xfer(dev_priv);
 
-		err = guc_ucode_xfer(dev_priv);
-		if (!err)
-			break;
-
-		if (--retries == 0)
-			goto fail;
-
-		DRM_INFO("GuC fw load failed: %d; will reset and "
-			 "retry %d more time(s)\n", err, retries);
-	}
+	if (ret)
+		return -EAGAIN;
 
 	guc_fw->guc_fw_load_status = GUC_FIRMWARE_SUCCESS;
 
@@ -528,63 +490,7 @@ int intel_guc_load(struct drm_i915_private *dev_priv)
 		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
-	if (i915.enable_guc_submission) {
-		if (i915.guc_log_level >= 0)
-			gen9_enable_guc_interrupts(dev_priv);
-
-		err = i915_guc_submission_enable(dev_priv);
-		if (err)
-			goto fail;
-		guc_interrupts_capture(dev_priv);
-	}
-
 	return 0;
-
-fail:
-	if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_PENDING)
-		guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
-
-	guc_interrupts_release(dev_priv);
-	i915_guc_submission_disable(dev_priv);
-	i915_guc_submission_fini(dev_priv);
-
-	/*
-	 * We've failed to load the firmware :(
-	 *
-	 * Decide whether to disable GuC submission and fall back to
-	 * execlist mode, and whether to hide the error by returning
-	 * zero or to return -EIO, which the caller will treat as a
-	 * nonfatal error (i.e. it doesn't prevent driver load, but
-	 * marks the GPU as wedged until reset).
-	 */
-	if (i915.enable_guc_loading > 1) {
-		ret = -EIO;
-	} else if (i915.enable_guc_submission > 1) {
-		ret = -EIO;
-	} else {
-		ret = 0;
-	}
-
-	if (err == 0 && !HAS_GUC_UCODE(dev_priv))
-		;	/* Don't mention the GuC! */
-	else if (err == 0)
-		DRM_INFO("GuC firmware load skipped\n");
-	else if (ret != -EIO)
-		DRM_NOTE("GuC firmware load failed: %d\n", err);
-	else
-		DRM_WARN("GuC firmware load failed: %d\n", err);
-
-	if (i915.enable_guc_submission) {
-		if (fw_path == NULL)
-			DRM_INFO("GuC submission without firmware not supported\n");
-		if (ret == 0)
-			DRM_NOTE("Falling back from GuC submission to execlist mode\n");
-		else
-			DRM_ERROR("GuC init failed: %d\n", ret);
-	}
-	i915.enable_guc_submission = 0;
-
-	return ret;
 }
 
 static void guc_fw_fetch(struct drm_i915_private *dev_priv,
@@ -757,6 +663,10 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
 	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
 	guc_fw->guc_fw_load_status = GUC_FIRMWARE_NONE;
 
+	/* can't enable guc submission without guc */
+	if (!i915.enable_guc_loading)
+		i915.enable_guc_submission = 0;
+
 	/* Early (and silent) return if GuC loading is disabled */
 	if (!i915.enable_guc_loading)
 		return;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 8eec035..4e184edb 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -35,6 +35,89 @@ void intel_uc_init(struct drm_i915_private *dev_priv)
 	intel_guc_init(dev_priv);
 }
 
+int intel_uc_load(struct drm_i915_private *dev_priv)
+{
+	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
+	int ret, retries;
+
+	/* guc not enabled, nothing to do */
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	guc_interrupts_release(dev_priv);
+	gen9_reset_guc_interrupts(dev_priv);
+
+	guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
+
+	if (i915.enable_guc_submission) {
+		ret = i915_guc_submission_init(dev_priv);
+		if (ret)
+			goto fail;
+	}
+
+	/* WaEnableGuCBootHashCheckNotSet:skl,bxt */
+	retries = GUC_WA_HASH_CHECK_NOT_SET_ATTEPMTS;
+	while (retries--) {
+		/*
+		 * Always reset the GuC just before (re)loading, so
+		 * that the state and timing are fairly predictable
+		 */
+		ret = guc_hw_reset(dev_priv);
+		if (ret)
+			goto fail;
+
+		ret = intel_guc_load(dev_priv);
+		if (ret == 0 || ret != -EAGAIN)
+			break;
+
+		DRM_INFO("GuC fw load failed: %d; will reset and "
+			 "retry %d more time(s)\n", ret, retries);
+	}
+
+	/* did we succeded or run out of retries? */
+	if (ret)
+		goto fail;
+
+	if (i915.enable_guc_submission) {
+		if (i915.guc_log_level >= 0)
+			gen9_enable_guc_interrupts(dev_priv);
+
+		ret = i915_guc_submission_enable(dev_priv);
+		if (ret)
+			goto fail;
+		guc_interrupts_capture(dev_priv);
+	}
+
+	return 0;
+
+fail:
+	/*
+	 * We've failed to load the firmware :(
+	 *
+	 * Decide whether to disable GuC submission and fall back to
+	 * execlist mode, and whether to hide the error by returning
+	 * zero or to return -EIO, which the caller will treat as a
+	 * nonfatal error (i.e. it doesn't prevent driver load, but
+	 * marks the GPU as wedged until reset).
+	 */
+	if (i915.enable_guc_loading > 1 || i915.enable_guc_submission > 1)
+		ret = -EIO;
+	else
+		ret = 0;
+
+	if (i915.enable_guc_submission) {
+		i915.enable_guc_submission = 0;
+		DRM_INFO("GuC submission without firmware not supported\n");
+		DRM_NOTE("Falling back from GuC submission to execlist mode\n");
+	}
+
+	guc_interrupts_release(dev_priv);
+	i915_guc_submission_disable(dev_priv);
+	i915_guc_submission_fini(dev_priv);
+
+	return ret;
+}
+
 /*
  * Read GuC command/status register (SOFT_SCRATCH_0)
  * Return true if it contains a response rather than a command
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index ec1a5b2..89c7e1f 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -28,6 +28,9 @@
 #include "i915_guc_reg.h"
 #include "intel_ringbuffer.h"
 
+/* WaEnableGuCBootHashCheckNotSet:skl,bxt */
+#define GUC_WA_HASH_CHECK_NOT_SET_ATTEPMTS 3
+
 struct drm_i915_gem_request;
 
 /*
@@ -171,6 +174,7 @@ struct intel_guc {
 /* intel_uc.c */
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
 void intel_uc_init(struct drm_i915_private *dev_priv);
+int intel_uc_load(struct drm_i915_private *dev_priv);
 bool intel_guc_recv(struct drm_i915_private *dev_priv, u32 *status);
 int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
@@ -185,6 +189,10 @@ extern void intel_guc_fini(struct drm_i915_private *dev_priv);
 extern const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status);
 extern int intel_guc_suspend(struct drm_i915_private *dev_priv);
 extern int intel_guc_resume(struct drm_i915_private *dev_priv);
+void guc_interrupts_release(struct drm_i915_private *dev_priv);
+void guc_interrupts_capture(struct drm_i915_private *dev_priv);
+int guc_hw_reset(struct drm_i915_private *dev_priv);
+u32 guc_wopcm_size(struct drm_i915_private *dev_priv);
 
 /* i915_guc_submission.c */
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
-- 
2.9.3



More information about the Intel-gfx mailing list