Commit a5f978c3 authored by Michal Wajdeczko's avatar Michal Wajdeczko Committed by Chris Wilson
Browse files

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



Since commit 6ca9a2be ("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 121981fa ("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: default avatarMichal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190811195132.9660-5-michal.wajdeczko@intel.com
parent 07587c34

drivers/gpu/drm/i915/gt/uc/intel_uc.c

+3 −8
Original line number Diff line number Diff line
@@ -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)

drivers/gpu/drm/i915/i915_gem.c

+8 −6
Original line number Diff line number Diff line
@@ -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,6 +1528,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) 
err_init_hw:

	intel_uc_fini_hw(&dev_priv->gt.uc);

err_uc_init:

	if (ret != -EIO)

		intel_uc_fini(&dev_priv->gt.uc);

err_pm:

	if (ret != -EIO) {
@@ -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)) {