@@ -5444,8 +5444,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
intel_init_gt_powersave(dev_priv);
ret = intel_uc_init(dev_priv);
- if (ret)
+ if (ret) {
+ GEM_BUG_ON(ret == -EIO);
goto err_pm;
+ }
ret = i915_gem_init_hw(dev_priv);
if (ret)
@@ -5492,7 +5494,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
i915_gem_contexts_lost(dev_priv);
i915_gem_fini_hw(dev_priv);
err_uc_init:
- intel_uc_fini(dev_priv);
+ if (ret != -EIO)
+ intel_uc_fini(dev_priv);
err_pm:
if (ret != -EIO) {
intel_cleanup_gt_powersave(dev_priv);
@@ -5506,15 +5509,15 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
mutex_unlock(&dev_priv->drm.struct_mutex);
- intel_uc_fini_misc(dev_priv);
-
- if (ret != -EIO)
+ if (ret != -EIO) {
+ intel_uc_fini_misc(dev_priv);
i915_gem_cleanup_userptr(dev_priv);
+ }
if (ret == -EIO) {
/*
- * 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 initialization 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 (!i915_terminally_wedged(&dev_priv->gpu_error)) {
@@ -176,6 +176,11 @@ static inline int intel_guc_sanitize(struct intel_guc *guc)
return 0;
}
+static inline bool intel_guc_is_loaded(struct intel_guc *guc)
+{
+ return intel_uc_fw_is_loaded(&guc->fw);
+}
+
static inline void intel_guc_enable_msg(struct intel_guc *guc, u32 mask)
{
spin_lock_irq(&guc->irq_lock);
@@ -332,6 +332,8 @@ static void __uc_sanitize(struct drm_i915_private *i915)
intel_huc_sanitize(huc);
intel_guc_sanitize(guc);
+ GEM_BUG_ON(intel_guc_is_loaded(guc));
+
__intel_uc_reset_hw(i915);
}
@@ -420,11 +422,13 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
* 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;
-
dev_err(dev_priv->drm.dev, "GuC initialization failed %d\n", ret);
- return ret;
+
+ /* Sanitize GuC/HuC to avoid clean-up on wedged */
+ __uc_sanitize(dev_priv);
+
+ /* We want to disable GPU submission but keep KMS alive */
+ return -EIO;
}
void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
@@ -436,6 +440,9 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
GEM_BUG_ON(!HAS_GUC(dev_priv));
+ if (!intel_guc_is_loaded(guc))
+ return;
+
if (USES_GUC_SUBMISSION(dev_priv))
intel_guc_submission_disable(guc);
@@ -121,6 +121,11 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
}
+static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
+{
+ return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
+}
+
/**
* intel_uc_fw_get_upload_size() - Get size of firmware needed to be uploaded.
* @uc_fw: uC firmware.
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. Fix that by always returning -EIO on uC hardware related failure. v2: don't allow -EIO from uc_init don't call uc_fini[_misc] on -EIO mark guc fw as failed on hw init failure prepare uc_fini_hw to run after earlier -EIO v3: update comments (Sagar) use sanitize functions on failure in init_hw (Michal) and also sanitize guc/huc fw in fini_hw (Michal) v4: rebase v5: rebase Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michal Winiarski <michal.winiarski@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++------- drivers/gpu/drm/i915/intel_guc.h | 5 +++++ drivers/gpu/drm/i915/intel_uc.c | 15 +++++++++++---- drivers/gpu/drm/i915/intel_uc_fw.h | 5 +++++ 4 files changed, 31 insertions(+), 11 deletions(-)