diff mbox

[v6] drm/i915/guc: Simplify intel_guc_init_hw()

Message ID 20170310114606.17793-1-arkadiusz.hiler@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Arkadiusz Hiler March 10, 2017, 11:46 a.m. UTC
Current version of intel_guc_init_hw() does a lot:
 - cares about submission
 - loads huc
 - implement WA

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

v2: rename guc_hw_reset and fix typo in define name (M. Wajdeczko)
v3: rename once again
v4: remove spurious comments and add some style (J. Lahtinen)
v5: flow changes, got rid of dead checks (M. Wajdeczko)
v6: rebase

Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |   2 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 144 +++-----------------------------
 drivers/gpu/drm/i915/intel_uc.c         | 113 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h         |   2 +
 4 files changed, 127 insertions(+), 134 deletions(-)

Comments

Michal Wajdeczko March 10, 2017, 12:11 p.m. UTC | #1
On Fri, Mar 10, 2017 at 12:46:06PM +0100, Arkadiusz Hiler wrote:
> Current version of intel_guc_init_hw() does a lot:
>  - cares about submission
>  - loads huc
>  - implement WA
> 
> This change offloads some of the logic to intel_uc_init_hw(), which now
> cares about the above.
> 
> v2: rename guc_hw_reset and fix typo in define name (M. Wajdeczko)
> v3: rename once again
> v4: remove spurious comments and add some style (J. Lahtinen)
> v5: flow changes, got rid of dead checks (M. Wajdeczko)
> v6: rebase
> 
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---

snip

> @@ -397,42 +379,22 @@ int intel_guc_init_hw(struct intel_guc *guc)
>  {
>  	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>  	const char *fw_path = 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_uc_fw_status_repr(guc->fw.fetch_status),
>  		intel_uc_fw_status_repr(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) {
> -		/* Device is known to have no uCode (e.g. no GuC) */
> -		err = -ENXIO;
> -		goto fail;
> +	if (!fw_path) {
> +		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.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) {
> -		err = -EIO;
> -		goto fail;
> -	} else if (guc->fw.load_status == INTEL_UC_FIRMWARE_FAIL) {
> -		err = -ENOEXEC;
> -		goto fail;
> -	}
> -
> -	guc_interrupts_release(dev_priv);
> -	gen9_reset_guc_interrupts(dev_priv);
> -
> -	/* We need to notify the guc whenever we change the GGTT */
> -	i915_ggtt_enable_guc(dev_priv);
> +	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
> +		return -EIO;

Above fw_path checks seem to be redundant as we also look for fetch status here.

...
 

> @@ -63,6 +84,98 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv)
>  	intel_guc_init_fw(&dev_priv->guc);
>  }
>  
> +int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> +{
> +	int ret, attempts;
> +	const int guc_wa_hash_check_not_set_attempts = 3;

In all other places, we put const vars definitions as first statements.
And maybe this const should be accompanied by the /* Wa */ comment
that is shown below. Also changing prefix to "gen9" maybe helpful

-Michal

> +
> +
> +	/* GuC not enabled, nothing to do */
> +	if (!i915.enable_guc_loading)
> +		return 0;
> +
> +	intel_guc_release_interrupts(dev_priv);
> +	gen9_reset_guc_interrupts(dev_priv);
> +
> +	/* We need to notify the guc whenever we change the GGTT */
> +	i915_ggtt_enable_guc(dev_priv);
> +
> +	if (i915.enable_guc_submission) {
> +		ret = i915_guc_submission_init(dev_priv);
> +		if (ret)
> +			goto fail;
> +	}
> +
> +	/* WaEnableuKernelHeaderValidFix:skl */
> +	/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
> +	if (IS_GEN9(dev_priv))
> +		attempts = guc_wa_hash_check_not_set_attempts;
> +	else
> +		attempts = 1;
> +
> +	while (attempts--) {
> +		/*
> +		 * Always reset the GuC just before (re)loading, so
> +		 * that the state and timing are fairly predictable
> +		 */
> +		ret = __intel_uc_reset_hw(dev_priv);
> +		if (ret)
> +			goto fail;
> +
> +		intel_huc_init_hw(&dev_priv->huc);
> +		ret = intel_guc_init_hw(&dev_priv->guc);
> +		if (ret == 0 || ret != -EAGAIN)
> +			break;
> +
> +		DRM_INFO("GuC fw load failed: %d; will reset and "
> +			 "retry %d more time(s)\n", ret, attempts);
> +	}
> +
> +	/* Did we succeded or run out of retries? */
> +	if (ret)
> +		goto fail;
> +
> +	intel_guc_auth_huc(dev_priv);
> +	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;
> +	}
> +
> +	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).
> +	 */
> +	DRM_ERROR("GuC init failed\n");
> +	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_NOTE("Falling back from GuC submission to execlist mode\n");
> +	}
> +
> +	i915_ggtt_disable_guc(dev_priv);
> +	intel_guc_release_interrupts(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 c5179ef..a1f4960 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -187,6 +187,7 @@ struct intel_huc {
>  void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
>  void intel_uc_init_early(struct drm_i915_private *dev_priv);
>  void intel_uc_init_fw(struct drm_i915_private *dev_priv);
> +int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>  void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
>  			 struct intel_uc_fw *uc_fw);
>  int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
> @@ -199,6 +200,7 @@ void intel_guc_fini(struct drm_i915_private *dev_priv);
>  const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
>  int intel_guc_suspend(struct drm_i915_private *dev_priv);
>  int intel_guc_resume(struct drm_i915_private *dev_priv);
> +void intel_guc_release_interrupts(struct drm_i915_private *dev_priv);
>  u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>  
>  /* i915_guc_submission.c */
> -- 
> 2.9.3
>
Joonas Lahtinen March 13, 2017, 9:38 a.m. UTC | #2
On pe, 2017-03-10 at 12:46 +0100, Arkadiusz Hiler wrote:
> Current version of intel_guc_init_hw() does a lot:
>  - cares about submission
>  - loads huc
>  - implement WA
> 
> This change offloads some of the logic to intel_uc_init_hw(), which now
> cares about the above.
> 
> v2: rename guc_hw_reset and fix typo in define name (M. Wajdeczko)
> v3: rename once again
> v4: remove spurious comments and add some style (J. Lahtinen)
> v5: flow changes, got rid of dead checks (M. Wajdeczko)
> v6: rebase
> 
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

<SNIP>

> +int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> +{
> +	int ret, attempts;
> +	const int guc_wa_hash_check_not_set_attempts = 3;

I wouldn't bother with this const, the Wa comment makes it pretty clear
already.

Extra newline here, too.

<SNIP>

> +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).
> +	 */
> +	DRM_ERROR("GuC init failed\n");
> +	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_NOTE("Falling back from GuC submission to execlist mode\n");
> +	}
> +

I'd drop the above code to the bottom.

> +	i915_ggtt_disable_guc(dev_priv);
> +	intel_guc_release_interrupts(dev_priv);
> +	i915_guc_submission_disable(dev_priv);
> +	i915_guc_submission_fini(dev_priv);

And make this teardown oniony with some more labels.

With those changes, looks good to me.

Regards, Joonas
Arkadiusz Hiler March 13, 2017, 12:43 p.m. UTC | #3
On Fri, Mar 10, 2017 at 01:11:06PM +0100, Michal Wajdeczko wrote:
> On Fri, Mar 10, 2017 at 12:46:06PM +0100, Arkadiusz Hiler wrote:
> > Current version of intel_guc_init_hw() does a lot:
> >  - cares about submission
> >  - loads huc
> >  - implement WA
> > 
> > This change offloads some of the logic to intel_uc_init_hw(), which now
> > cares about the above.
> > 
> > v2: rename guc_hw_reset and fix typo in define name (M. Wajdeczko)
> > v3: rename once again
> > v4: remove spurious comments and add some style (J. Lahtinen)
> > v5: flow changes, got rid of dead checks (M. Wajdeczko)
> > v6: rebase
> > 
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> 
> snip
> 
> > @@ -397,42 +379,22 @@ int intel_guc_init_hw(struct intel_guc *guc)
> >  {
> >  	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> >  	const char *fw_path = 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_uc_fw_status_repr(guc->fw.fetch_status),
> >  		intel_uc_fw_status_repr(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) {
> > -		/* Device is known to have no uCode (e.g. no GuC) */
> > -		err = -ENXIO;
> > -		goto fail;
> > +	if (!fw_path) {
> > +		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.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) {
> > -		err = -EIO;
> > -		goto fail;
> > -	} else if (guc->fw.load_status == INTEL_UC_FIRMWARE_FAIL) {
> > -		err = -ENOEXEC;
> > -		goto fail;
> > -	}
> > -
> > -	guc_interrupts_release(dev_priv);
> > -	gen9_reset_guc_interrupts(dev_priv);
> > -
> > -	/* We need to notify the guc whenever we change the GGTT */
> > -	i915_ggtt_enable_guc(dev_priv);
> > +	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
> > +		return -EIO;
> 
> Above fw_path checks seem to be redundant as we also look for fetch status here.

Got rid of it in the path simplifying patch. Thanks.

> > @@ -63,6 +84,98 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv)
> >  	intel_guc_init_fw(&dev_priv->guc);
> >  }
> >  
> > +int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> > +{
> > +	int ret, attempts;
> > +	const int guc_wa_hash_check_not_set_attempts = 3;
> 
> In all other places, we put const vars definitions as first statements.
> And maybe this const should be accompanied by the /* Wa */ comment
> that is shown below. Also changing prefix to "gen9" maybe helpful

Went with using it directly, as Joonas suggested. The current if-else
statement makes meaning obvious enough without having the need to name
it.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6ffaebc..3462459 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4540,7 +4540,7 @@  int 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_init_hw(&dev_priv->guc);
+	ret = intel_uc_init_hw(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 eaced63..1ccffda 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -90,7 +90,7 @@  const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 	}
 };
 
-static void guc_interrupts_release(struct drm_i915_private *dev_priv)
+void intel_guc_release_interrupts(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
@@ -362,24 +362,6 @@  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 ret;
-	u32 guc_status;
-
-	ret = intel_guc_reset(dev_priv);
-	if (ret) {
-		DRM_ERROR("GuC reset failed, ret = %d\n", ret);
-		return ret;
-	}
-
-	guc_status = I915_READ(GUC_STATUS);
-	WARN(!(guc_status & GS_MIA_IN_RESET),
-	     "GuC status: 0x%x, MIA core expected to be in reset\n", guc_status);
-
-	return ret;
-}
-
 /**
  * intel_guc_init_hw() - finish preparing the GuC for activity
  * @guc: intel_guc structure
@@ -397,42 +379,22 @@  int intel_guc_init_hw(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	const char *fw_path = 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_uc_fw_status_repr(guc->fw.fetch_status),
 		intel_uc_fw_status_repr(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) {
-		/* Device is known to have no uCode (e.g. no GuC) */
-		err = -ENXIO;
-		goto fail;
+	if (!fw_path) {
+		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.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) {
-		err = -EIO;
-		goto fail;
-	} else if (guc->fw.load_status == INTEL_UC_FIRMWARE_FAIL) {
-		err = -ENOEXEC;
-		goto fail;
-	}
-
-	guc_interrupts_release(dev_priv);
-	gen9_reset_guc_interrupts(dev_priv);
-
-	/* We need to notify the guc whenever we change the GGTT */
-	i915_ggtt_enable_guc(dev_priv);
+	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return -EIO;
 
 	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
 
@@ -440,103 +402,19 @@  int intel_guc_init_hw(struct intel_guc *guc)
 		intel_uc_fw_status_repr(guc->fw.fetch_status),
 		intel_uc_fw_status_repr(guc->fw.load_status));
 
-	err = i915_guc_submission_init(dev_priv);
-	if (err)
-		goto fail;
+	ret = guc_ucode_xfer(dev_priv);
 
-	/*
-	 * 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;
-
-		intel_huc_init_hw(&dev_priv->huc);
-		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.load_status = INTEL_UC_FIRMWARE_SUCCESS;
 
-	intel_guc_auth_huc(dev_priv);
-
-	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;
-	}
-
 	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
 		 i915.enable_guc_submission ? "submission enabled" : "loaded",
 		 guc->fw.path,
 		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
 
 	return 0;
-
-fail:
-	if (guc->fw.load_status == INTEL_UC_FIRMWARE_PENDING)
-		guc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
-
-	guc_interrupts_release(dev_priv);
-	i915_guc_submission_disable(dev_priv);
-	i915_guc_submission_fini(dev_priv);
-	i915_ggtt_disable_guc(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;
 }
 
 
@@ -595,7 +473,7 @@  void intel_guc_fini(struct drm_i915_private *dev_priv)
 	struct drm_i915_gem_object *obj;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
-	guc_interrupts_release(dev_priv);
+	intel_guc_release_interrupts(dev_priv);
 	i915_guc_submission_disable(dev_priv);
 	i915_guc_submission_fini(dev_priv);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f0a69d4..bbf74e04 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -26,6 +26,27 @@ 
 #include "intel_uc.h"
 #include <linux/firmware.h>
 
+/* Reset GuC providing us with fresh state for both GuC and HuC.
+ */
+static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
+{
+	int ret;
+	u32 guc_status;
+
+	ret = intel_guc_reset(dev_priv);
+	if (ret) {
+		DRM_ERROR("GuC reset failed, ret = %d\n", ret);
+		return ret;
+	}
+
+	guc_status = I915_READ(GUC_STATUS);
+	WARN(!(guc_status & GS_MIA_IN_RESET),
+	     "GuC status: 0x%x, MIA core expected to be in reset\n",
+	     guc_status);
+
+	return ret;
+}
+
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 {
 	if (!HAS_GUC(dev_priv)) {
@@ -63,6 +84,98 @@  void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 	intel_guc_init_fw(&dev_priv->guc);
 }
 
+int intel_uc_init_hw(struct drm_i915_private *dev_priv)
+{
+	int ret, attempts;
+	const int guc_wa_hash_check_not_set_attempts = 3;
+
+
+	/* GuC not enabled, nothing to do */
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	intel_guc_release_interrupts(dev_priv);
+	gen9_reset_guc_interrupts(dev_priv);
+
+	/* We need to notify the guc whenever we change the GGTT */
+	i915_ggtt_enable_guc(dev_priv);
+
+	if (i915.enable_guc_submission) {
+		ret = i915_guc_submission_init(dev_priv);
+		if (ret)
+			goto fail;
+	}
+
+	/* WaEnableuKernelHeaderValidFix:skl */
+	/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
+	if (IS_GEN9(dev_priv))
+		attempts = guc_wa_hash_check_not_set_attempts;
+	else
+		attempts = 1;
+
+	while (attempts--) {
+		/*
+		 * Always reset the GuC just before (re)loading, so
+		 * that the state and timing are fairly predictable
+		 */
+		ret = __intel_uc_reset_hw(dev_priv);
+		if (ret)
+			goto fail;
+
+		intel_huc_init_hw(&dev_priv->huc);
+		ret = intel_guc_init_hw(&dev_priv->guc);
+		if (ret == 0 || ret != -EAGAIN)
+			break;
+
+		DRM_INFO("GuC fw load failed: %d; will reset and "
+			 "retry %d more time(s)\n", ret, attempts);
+	}
+
+	/* Did we succeded or run out of retries? */
+	if (ret)
+		goto fail;
+
+	intel_guc_auth_huc(dev_priv);
+	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;
+	}
+
+	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).
+	 */
+	DRM_ERROR("GuC init failed\n");
+	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_NOTE("Falling back from GuC submission to execlist mode\n");
+	}
+
+	i915_ggtt_disable_guc(dev_priv);
+	intel_guc_release_interrupts(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 c5179ef..a1f4960 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -187,6 +187,7 @@  struct intel_huc {
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
 void intel_uc_init_fw(struct drm_i915_private *dev_priv);
+int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
 			 struct intel_uc_fw *uc_fw);
 int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
@@ -199,6 +200,7 @@  void intel_guc_fini(struct drm_i915_private *dev_priv);
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
 int intel_guc_suspend(struct drm_i915_private *dev_priv);
 int intel_guc_resume(struct drm_i915_private *dev_priv);
+void intel_guc_release_interrupts(struct drm_i915_private *dev_priv);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 
 /* i915_guc_submission.c */