diff mbox series

[4/4] drm/i915/uc: Use -EIO code for GuC initialization failures

Message ID 20190811195132.9660-5-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show
Series Use -EIO code for GuC initialization failures | expand

Commit Message

Michal Wajdeczko Aug. 11, 2019, 7:51 p.m. UTC
Since commit 6ca9a2beb54a ("drm/i915: Unwind i915_gem_init() failure")
we believed that we correctly handle all errors encountered during
GuC initialization, including special one that indicates request to
run driver with disabled GPU submission (-EIO).

Unfortunately since commit 121981fafe69 ("drm/i915/guc: Combine
enable_guc_loading|submission modparams") we stopped using that
error code to avoid unwanted fallback to execlist submission mode.

In result any GuC initialization failure was treated as non-recoverable
error leading to driver load abort, so we could not even read related
GuC error log to investigate cause of the problem.

For now always return -EIO on any uC hardware related failure.

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_uc.c | 11 +++--------
 drivers/gpu/drm/i915/i915_gem.c       | 14 ++++++++------
 2 files changed, 11 insertions(+), 14 deletions(-)

Comments

Chris Wilson Aug. 11, 2019, 8:26 p.m. UTC | #1
Quoting Michal Wajdeczko (2019-08-11 20:51:32)
> Since commit 6ca9a2beb54a ("drm/i915: Unwind i915_gem_init() failure")
> we believed that we correctly handle all errors encountered during
> GuC initialization, including special one that indicates request to
> run driver with disabled GPU submission (-EIO).
> 
> Unfortunately since commit 121981fafe69 ("drm/i915/guc: Combine
> enable_guc_loading|submission modparams") we stopped using that
> error code to avoid unwanted fallback to execlist submission mode.
> 
> In result any GuC initialization failure was treated as non-recoverable
> error leading to driver load abort, so we could not even read related
> GuC error log to investigate cause of the problem.
> 
> For now always return -EIO on any uC hardware related failure.
> 
> 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>

Works for me,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 2acf7907287c..3b33fb275efb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -530,15 +530,10 @@  int intel_uc_init_hw(struct intel_uc *uc)
 err_out:
 	__uc_sanitize(uc);
 
-	/*
-	 * Note that there is no fallback as either user explicitly asked for
-	 * the GuC or driver default option was to run with the GuC enabled.
-	 */
-	if (GEM_WARN_ON(ret == -EIO))
-		ret = -EINVAL;
-
 	i915_probe_error(i915, "GuC initialization failed %d\n", ret);
-	return ret;
+
+	/* We want to keep KMS alive */
+	return -EIO;
 }
 
 void intel_uc_fini_hw(struct intel_uc *uc)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6ff01a404346..4752a3bf9636 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1463,8 +1463,10 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 	intel_init_gt_powersave(dev_priv);
 
 	ret = intel_uc_init(&dev_priv->gt.uc);
-	if (ret)
+	if (ret) {
+		GEM_BUG_ON(ret == -EIO);
 		goto err_pm;
+	}
 
 	ret = i915_gem_init_hw(dev_priv);
 	if (ret)
@@ -1526,7 +1528,8 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 err_init_hw:
 	intel_uc_fini_hw(&dev_priv->gt.uc);
 err_uc_init:
-	intel_uc_fini(&dev_priv->gt.uc);
+	if (ret != -EIO)
+		intel_uc_fini(&dev_priv->gt.uc);
 err_pm:
 	if (ret != -EIO) {
 		intel_cleanup_gt_powersave(dev_priv);
@@ -1542,9 +1545,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);
 
-	intel_uc_cleanup_firmwares(&dev_priv->gt.uc);
-
 	if (ret != -EIO) {
+		intel_uc_cleanup_firmwares(&dev_priv->gt.uc);
 		i915_gem_cleanup_userptr(dev_priv);
 		intel_timelines_fini(dev_priv);
 	}
@@ -1553,8 +1555,8 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 		mutex_lock(&dev_priv->drm.struct_mutex);
 
 		/*
-		 * Allow engine initialisation to fail by marking the GPU as
-		 * wedged. But we only want to do this where the GPU is angry,
+		 * Allow engines or uC initialisation to fail by marking the GPU
+		 * as wedged. But we only want to do this when the GPU is angry,
 		 * for all other failure, such as an allocation failure, bail.
 		 */
 		if (!intel_gt_is_wedged(&dev_priv->gt)) {