diff mbox

[6/6] drm/i915/huc: Add BXT HuC Loading Support

Message ID 1466532685-5101-7-git-send-email-peter.antoine@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Antoine June 21, 2016, 6:11 p.m. UTC
This patch adds the HuC Loading for the BXT.
Version 1.x of the HuC firmware.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         | 13 +++----------
 drivers/gpu/drm/i915/intel_guc_loader.c | 29 +++++++++++++++++------------
 drivers/gpu/drm/i915/intel_huc_loader.c |  7 +++++++
 3 files changed, 27 insertions(+), 22 deletions(-)

Comments

Rodrigo Vivi June 21, 2016, 6:26 p.m. UTC | #1
Hi Peter,

On Tue, 2016-06-21 at 19:11 +0100, Peter Antoine wrote:
> This patch adds the HuC Loading for the BXT.

> Version 1.x of the HuC firmware.

> 

> Signed-off-by: Peter Antoine <peter.antoine@intel.com>

> ---

>  drivers/gpu/drm/i915/i915_gem.c         | 13 +++----------

>  drivers/gpu/drm/i915/intel_guc_loader.c | 29 +++++++++++++++++----

> --------

>  drivers/gpu/drm/i915/intel_huc_loader.c |  7 +++++++

>  3 files changed, 27 insertions(+), 22 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/i915_gem.c

> b/drivers/gpu/drm/i915/i915_gem.c

> index 549dd3f..6abd5e5 100644

> --- a/drivers/gpu/drm/i915/i915_gem.c

> +++ b/drivers/gpu/drm/i915/i915_gem.c

> @@ -5143,16 +5143,9 @@ i915_gem_init_hw(struct drm_device *dev)

>  	intel_mocs_init_l3cc_table(dev);

>  

>  	/* We can't enable contexts until all firmware is loaded */

> -	if (HAS_GUC(dev)) {

> -		/* init WOPCM */

> -		I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));

> -		I915_WRITE(DMA_GUC_WOPCM_OFFSET,

> GUC_WOPCM_OFFSET_VALUE);

> -

> -		intel_huc_load(dev);

> -		ret = intel_guc_setup(dev);

> -		if (ret)

> -			goto out;

> -	}

> +	ret = intel_guc_setup(dev);

> +	if (ret)

> +		goto out;

>  

>  	/*

>  	 * Increment the next seqno by 0x100 so we have a visible

> break

> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c

> b/drivers/gpu/drm/i915/intel_guc_loader.c

> index e876a23f..289b5b6 100644

> --- a/drivers/gpu/drm/i915/intel_guc_loader.c

> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c

> @@ -82,6 +82,17 @@ const char *intel_uc_fw_status_repr(enum

> intel_uc_fw_status status)

>  	}

>  };

>  

> +u32 guc_wopcm_size(struct drm_device *dev)

> +{

> +	u32 wopcm_size = GUC_WOPCM_TOP;

> +

> +	/* On BXT, the top of WOPCM is reserved for RC6 context */

> +	if (IS_BROXTON(dev))

> +		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;

> +

> +	return wopcm_size;

> +}

> +

>  static void direct_interrupts_to_host(struct drm_i915_private

> *dev_priv)

>  {

>  	struct intel_engine_cs *engine;

> @@ -296,17 +307,6 @@ static int guc_ucode_xfer_dma(struct

> drm_i915_private *dev_priv)

>  	return ret;

>  }

>  

> -u32 guc_wopcm_size(struct drm_device *dev)

> -{

> -	u32 wopcm_size = GUC_WOPCM_TOP;

> -

> -	/* On BXT, the top of WOPCM is reserved for RC6 context */

> -	if (IS_BROXTON(dev))

> -		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;

> -

> -	return wopcm_size;

> -}

> -

>  /*

>   * Load the GuC firmware blob into the MinuteIA.

>   */

> @@ -333,6 +333,10 @@ static int guc_ucode_xfer(struct

> drm_i915_private *dev_priv)

>  

>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);

>  

> +	/* init WOPCM */

> +	I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));

> +	I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE);

> +

>  	/* Enable MIA caching. GuC clock gating is disabled. */

>  	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);

>  

> @@ -483,6 +487,7 @@ int intel_guc_setup(struct drm_device *dev)

>  			goto fail;

>  		}

>  

> +		intel_huc_load(dev);

>  		err = guc_ucode_xfer(dev_priv);

>  		if (!err)

>  			break;

> @@ -638,7 +643,7 @@ void intel_uc_fw_fetch(struct drm_device *dev,

> struct intel_uc_fw *uc_fw)

>  		size = uc_fw->header_size + uc_fw->ucode_size;

>  

>  		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6

> context). */

> -		if (size > guc_wopcm_size(dev->dev_private)) {

> +		if (size > guc_wopcm_size(dev)) {

>  			DRM_ERROR("Firmware is too large to fit in

> WOPCM\n");

>  			goto fail;

>  		}

> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c

> b/drivers/gpu/drm/i915/intel_huc_loader.c

> index 7205e9e..7a43d4e 100644

> --- a/drivers/gpu/drm/i915/intel_huc_loader.c

> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c

> @@ -49,6 +49,9 @@

>  #define I915_SKL_HUC_UCODE "i915/skl_huc_ver1.bin"

>  MODULE_FIRMWARE(I915_SKL_HUC_UCODE);


We need to step away from symbolic links for all firmwares.

Specially with HuC that HuC team had never agreed with major_minor
where major increases if any change in API or ABI.

And also they have the major_minor_build number in the release like

intel/firmware/skl_huc_ver01_07_1398

So I believe we need to fix this first and then add the BXT in the
proper way without using any symbolic link.

>  

> +#define I915_BXT_HUC_UCODE "i915/bxt_huc_ver1.bin"

> +MODULE_FIRMWARE(I915_BXT_HUC_UCODE);


Also, where is this version? I believe I missed this release while I
was out. Could you please share with me?

Is it production and with all permissions to release at 01.org and
propagate to linux-firmware.git?

Thanks,
Rodrigo.

> +

>  /**

>   * intel_huc_load_ucode() - DMA's the firmware

>   * @dev: the drm device

> @@ -157,6 +160,10 @@ void intel_huc_init(struct drm_device *dev)

>  		fw_path = I915_SKL_HUC_UCODE;

>  		huc_fw->major_ver_wanted = 1;

>  		huc_fw->minor_ver_wanted = 5;

> +	} else if (IS_BROXTON(dev)) {

> +		fw_path = I915_BXT_HUC_UCODE;

> +		huc_fw->major_ver_wanted = 1;

> +		huc_fw->minor_ver_wanted = 0;

>  	}

>  

>  	if (fw_path == NULL)
Peter Antoine June 21, 2016, 9:28 p.m. UTC | #2
Hi Rodrigo,

I tested with an old version that has been knocking about for ages. It is shipped on Android so I think it is production. I may be wrong there as Tom was handling the firmware release process and is not available to talk to.

I am being "ACT'ed"/"ACT'ioned"*, so am not sure when I'll stop having access to the firmware/hardware, so added the BXT patch so it was available to others, it has been tested on multiple platforms/kernels including being tested on media enabled platforms.

I have seen the skl huc version with the full version and build. I am not sure what the release version of the BXT HuC will be. It might be worth simply dropping the BXT patch (after review) so that when the correct firmware is released we have a tested/reviewed version of the code to base the changes on.

I think Chris Harris, is trying to organize the firmware releases/packaging, but the release process and structure is still up in the air a little. It might be worth contacting him directly to find out the  correct version and if there is a releasable version for BXT.

Peter

*still under review (the position and not just the correct verb to use :) ).

-----Original Message-----
From: Vivi, Rodrigo 

Sent: Tuesday, June 21, 2016 7:26 PM
To: intel-gfx@lists.freedesktop.org; Antoine, Peter <peter.antoine@intel.com>
Cc: Prigent, Christophe <christophe.prigent@intel.com>; Kelley, Sean V <sean.v.kelley@intel.com>; Gordon, David S <david.s.gordon@intel.com>; Li, Lawrence T <lawrence.t.li@intel.com>
Subject: Re: [PATCH 6/6] drm/i915/huc: Add BXT HuC Loading Support

Hi Peter,

On Tue, 2016-06-21 at 19:11 +0100, Peter Antoine wrote:
> This patch adds the HuC Loading for the BXT.

> Version 1.x of the HuC firmware.

> 

> Signed-off-by: Peter Antoine <peter.antoine@intel.com>

> ---

>  drivers/gpu/drm/i915/i915_gem.c         | 13 +++----------

>  drivers/gpu/drm/i915/intel_guc_loader.c | 29 +++++++++++++++++----

> --------

>  drivers/gpu/drm/i915/intel_huc_loader.c |  7 +++++++

>  3 files changed, 27 insertions(+), 22 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/i915_gem.c 

> b/drivers/gpu/drm/i915/i915_gem.c index 549dd3f..6abd5e5 100644

> --- a/drivers/gpu/drm/i915/i915_gem.c

> +++ b/drivers/gpu/drm/i915/i915_gem.c

> @@ -5143,16 +5143,9 @@ i915_gem_init_hw(struct drm_device *dev)

>  	intel_mocs_init_l3cc_table(dev);

>  

>  	/* We can't enable contexts until all firmware is loaded */

> -	if (HAS_GUC(dev)) {

> -		/* init WOPCM */

> -		I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));

> -		I915_WRITE(DMA_GUC_WOPCM_OFFSET,

> GUC_WOPCM_OFFSET_VALUE);

> -

> -		intel_huc_load(dev);

> -		ret = intel_guc_setup(dev);

> -		if (ret)

> -			goto out;

> -	}

> +	ret = intel_guc_setup(dev);

> +	if (ret)

> +		goto out;

>  

>  	/*

>  	 * Increment the next seqno by 0x100 so we have a visible break diff 

> --git a/drivers/gpu/drm/i915/intel_guc_loader.c

> b/drivers/gpu/drm/i915/intel_guc_loader.c

> index e876a23f..289b5b6 100644

> --- a/drivers/gpu/drm/i915/intel_guc_loader.c

> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c

> @@ -82,6 +82,17 @@ const char *intel_uc_fw_status_repr(enum 

> intel_uc_fw_status status)

>  	}

>  };

>  

> +u32 guc_wopcm_size(struct drm_device *dev) {

> +	u32 wopcm_size = GUC_WOPCM_TOP;

> +

> +	/* On BXT, the top of WOPCM is reserved for RC6 context */

> +	if (IS_BROXTON(dev))

> +		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;

> +

> +	return wopcm_size;

> +}

> +

>  static void direct_interrupts_to_host(struct drm_i915_private

> *dev_priv)

>  {

>  	struct intel_engine_cs *engine;

> @@ -296,17 +307,6 @@ static int guc_ucode_xfer_dma(struct 

> drm_i915_private *dev_priv)

>  	return ret;

>  }

>  

> -u32 guc_wopcm_size(struct drm_device *dev) -{

> -	u32 wopcm_size = GUC_WOPCM_TOP;

> -

> -	/* On BXT, the top of WOPCM is reserved for RC6 context */

> -	if (IS_BROXTON(dev))

> -		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;

> -

> -	return wopcm_size;

> -}

> -

>  /*

>   * Load the GuC firmware blob into the MinuteIA.

>   */

> @@ -333,6 +333,10 @@ static int guc_ucode_xfer(struct drm_i915_private 

> *dev_priv)

>  

>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);

>  

> +	/* init WOPCM */

> +	I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));

> +	I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE);

> +

>  	/* Enable MIA caching. GuC clock gating is disabled. */

>  	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);

>  

> @@ -483,6 +487,7 @@ int intel_guc_setup(struct drm_device *dev)

>  			goto fail;

>  		}

>  

> +		intel_huc_load(dev);

>  		err = guc_ucode_xfer(dev_priv);

>  		if (!err)

>  			break;

> @@ -638,7 +643,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, 

> struct intel_uc_fw *uc_fw)

>  		size = uc_fw->header_size + uc_fw->ucode_size;

>  

>  		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */

> -		if (size > guc_wopcm_size(dev->dev_private)) {

> +		if (size > guc_wopcm_size(dev)) {

>  			DRM_ERROR("Firmware is too large to fit in WOPCM\n");

>  			goto fail;

>  		}

> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c

> b/drivers/gpu/drm/i915/intel_huc_loader.c

> index 7205e9e..7a43d4e 100644

> --- a/drivers/gpu/drm/i915/intel_huc_loader.c

> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c

> @@ -49,6 +49,9 @@

>  #define I915_SKL_HUC_UCODE "i915/skl_huc_ver1.bin"

>  MODULE_FIRMWARE(I915_SKL_HUC_UCODE);


We need to step away from symbolic links for all firmwares.

Specially with HuC that HuC team had never agreed with major_minor where major increases if any change in API or ABI.

And also they have the major_minor_build number in the release like

intel/firmware/skl_huc_ver01_07_1398

So I believe we need to fix this first and then add the BXT in the proper way without using any symbolic link.

>  

> +#define I915_BXT_HUC_UCODE "i915/bxt_huc_ver1.bin"

> +MODULE_FIRMWARE(I915_BXT_HUC_UCODE);


Also, where is this version? I believe I missed this release while I was out. Could you please share with me?

Is it production and with all permissions to release at 01.org and propagate to linux-firmware.git?

Thanks,
Rodrigo.

> +

>  /**

>   * intel_huc_load_ucode() - DMA's the firmware

>   * @dev: the drm device

> @@ -157,6 +160,10 @@ void intel_huc_init(struct drm_device *dev)

>  		fw_path = I915_SKL_HUC_UCODE;

>  		huc_fw->major_ver_wanted = 1;

>  		huc_fw->minor_ver_wanted = 5;

> +	} else if (IS_BROXTON(dev)) {

> +		fw_path = I915_BXT_HUC_UCODE;

> +		huc_fw->major_ver_wanted = 1;

> +		huc_fw->minor_ver_wanted = 0;

>  	}

>  

>  	if (fw_path == NULL)
Dave Gordon June 28, 2016, 3:20 p.m. UTC | #3
On 21/06/16 19:11, Peter Antoine wrote:
> This patch adds the HuC Loading for the BXT.
> Version 1.x of the HuC firmware.
>
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c         | 13 +++----------
>   drivers/gpu/drm/i915/intel_guc_loader.c | 29 +++++++++++++++++------------
>   drivers/gpu/drm/i915/intel_huc_loader.c |  7 +++++++
>   3 files changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 549dd3f..6abd5e5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5143,16 +5143,9 @@ i915_gem_init_hw(struct drm_device *dev)
>   	intel_mocs_init_l3cc_table(dev);
>
>   	/* We can't enable contexts until all firmware is loaded */
> -	if (HAS_GUC(dev)) {
> -		/* init WOPCM */
> -		I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));
> -		I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE);
> -
> -		intel_huc_load(dev);
> -		ret = intel_guc_setup(dev);
> -		if (ret)
> -			goto out;
> -	}
> +	ret = intel_guc_setup(dev);
> +	if (ret)
> +		goto out;

This reverts one of the earlier changes that I objected to!
Please squash them together so it isn't changed and then changed back.

>   	/*
>   	 * Increment the next seqno by 0x100 so we have a visible break
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index e876a23f..289b5b6 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -82,6 +82,17 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>   	}
>   };
>
> +u32 guc_wopcm_size(struct drm_device *dev)
> +{
> +	u32 wopcm_size = GUC_WOPCM_TOP;
> +
> +	/* On BXT, the top of WOPCM is reserved for RC6 context */
> +	if (IS_BROXTON(dev))
> +		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
> +
> +	return wopcm_size;
> +}
> +
>   static void direct_interrupts_to_host(struct drm_i915_private *dev_priv)
>   {
>   	struct intel_engine_cs *engine;
> @@ -296,17 +307,6 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
>   	return ret;
>   }
>
> -u32 guc_wopcm_size(struct drm_device *dev)
> -{
> -	u32 wopcm_size = GUC_WOPCM_TOP;
> -
> -	/* On BXT, the top of WOPCM is reserved for RC6 context */
> -	if (IS_BROXTON(dev))
> -		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
> -
> -	return wopcm_size;
> -}
> -

This is just being moved around? Does it need to be?

>   /*
>    * Load the GuC firmware blob into the MinuteIA.
>    */
> @@ -333,6 +333,10 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
>
>   	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>
> +	/* init WOPCM */
> +	I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));
> +	I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE);
> +

Is this being moved around (again?)
I think we should end up with GUC_WOPCM_SIZE being set just once.

>   	/* Enable MIA caching. GuC clock gating is disabled. */
>   	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
>
> @@ -483,6 +487,7 @@ int intel_guc_setup(struct drm_device *dev)
>   			goto fail;
>   		}
>
> +		intel_huc_load(dev);
>   		err = guc_ucode_xfer(dev_priv);
>   		if (!err)
>   			break;
> @@ -638,7 +643,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
>   		size = uc_fw->header_size + uc_fw->ucode_size;
>
>   		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
> -		if (size > guc_wopcm_size(dev->dev_private)) {
> +		if (size > guc_wopcm_size(dev)) {
>   			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
>   			goto fail;
>   		}
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> index 7205e9e..7a43d4e 100644
> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -49,6 +49,9 @@
>   #define I915_SKL_HUC_UCODE "i915/skl_huc_ver1.bin"
>   MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
>
> +#define I915_BXT_HUC_UCODE "i915/bxt_huc_ver1.bin"
> +MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
> +
>   /**
>    * intel_huc_load_ucode() - DMA's the firmware
>    * @dev: the drm device
> @@ -157,6 +160,10 @@ void intel_huc_init(struct drm_device *dev)
>   		fw_path = I915_SKL_HUC_UCODE;
>   		huc_fw->major_ver_wanted = 1;
>   		huc_fw->minor_ver_wanted = 5;
> +	} else if (IS_BROXTON(dev)) {
> +		fw_path = I915_BXT_HUC_UCODE;
> +		huc_fw->major_ver_wanted = 1;
> +		huc_fw->minor_ver_wanted = 0;
>   	}
>
>   	if (fw_path == NULL)

Overall: I think the general functionality is OK, but there are a few 
specific points that need to be fixed (unless they already are, by a 
later patch). In addition, the series need to be reorganised so that we 
don't have the same lines of code repeatedly shuffled from one place to 
another, or a change made and then reverted.

.Dave.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 549dd3f..6abd5e5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5143,16 +5143,9 @@  i915_gem_init_hw(struct drm_device *dev)
 	intel_mocs_init_l3cc_table(dev);
 
 	/* We can't enable contexts until all firmware is loaded */
-	if (HAS_GUC(dev)) {
-		/* init WOPCM */
-		I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));
-		I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE);
-
-		intel_huc_load(dev);
-		ret = intel_guc_setup(dev);
-		if (ret)
-			goto out;
-	}
+	ret = intel_guc_setup(dev);
+	if (ret)
+		goto out;
 
 	/*
 	 * Increment the next seqno by 0x100 so we have a visible break
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index e876a23f..289b5b6 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -82,6 +82,17 @@  const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 	}
 };
 
+u32 guc_wopcm_size(struct drm_device *dev)
+{
+	u32 wopcm_size = GUC_WOPCM_TOP;
+
+	/* On BXT, the top of WOPCM is reserved for RC6 context */
+	if (IS_BROXTON(dev))
+		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+
+	return wopcm_size;
+}
+
 static void direct_interrupts_to_host(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
@@ -296,17 +307,6 @@  static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-u32 guc_wopcm_size(struct drm_device *dev)
-{
-	u32 wopcm_size = GUC_WOPCM_TOP;
-
-	/* On BXT, the top of WOPCM is reserved for RC6 context */
-	if (IS_BROXTON(dev))
-		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
-
-	return wopcm_size;
-}
-
 /*
  * Load the GuC firmware blob into the MinuteIA.
  */
@@ -333,6 +333,10 @@  static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
 
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
+	/* init WOPCM */
+	I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));
+	I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE);
+
 	/* Enable MIA caching. GuC clock gating is disabled. */
 	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
 
@@ -483,6 +487,7 @@  int intel_guc_setup(struct drm_device *dev)
 			goto fail;
 		}
 
+		intel_huc_load(dev);
 		err = guc_ucode_xfer(dev_priv);
 		if (!err)
 			break;
@@ -638,7 +643,7 @@  void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
 		size = uc_fw->header_size + uc_fw->ucode_size;
 
 		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
-		if (size > guc_wopcm_size(dev->dev_private)) {
+		if (size > guc_wopcm_size(dev)) {
 			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
 			goto fail;
 		}
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 7205e9e..7a43d4e 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -49,6 +49,9 @@ 
 #define I915_SKL_HUC_UCODE "i915/skl_huc_ver1.bin"
 MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
 
+#define I915_BXT_HUC_UCODE "i915/bxt_huc_ver1.bin"
+MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
+
 /**
  * intel_huc_load_ucode() - DMA's the firmware
  * @dev: the drm device
@@ -157,6 +160,10 @@  void intel_huc_init(struct drm_device *dev)
 		fw_path = I915_SKL_HUC_UCODE;
 		huc_fw->major_ver_wanted = 1;
 		huc_fw->minor_ver_wanted = 5;
+	} else if (IS_BROXTON(dev)) {
+		fw_path = I915_BXT_HUC_UCODE;
+		huc_fw->major_ver_wanted = 1;
+		huc_fw->minor_ver_wanted = 0;
 	}
 
 	if (fw_path == NULL)