[v2,1/1] drm/i915/uc: Make GuC/HuC fw fetch and loading functions/file structure symmetric
diff mbox

Message ID 1519915642-20180-1-git-send-email-sagar.a.kamble@intel.com
State New
Headers show

Commit Message

sagar.a.kamble@intel.com March 1, 2018, 2:47 p.m. UTC
GuC load function is named intel_guc_fw_upload() and HuC load function is
named intel_huc_init_hw(). Make them consistent intel_*_fw_upload. Also
move HuC fw loading functions and declarations to separate files
intel_huc_fw.c|h like GuC.

While at this, do below changes
1. Update kernel-doc comment for intel_*_fw_upload() functions
2. s/huc_ucode_xfer/huc_fw_xfer
3. Introduce intel_huc_fw_init_early()

v2: Changed patch to update HuC functions instead of changing
    guc_fw_upload and update file structure. (Michal Wajdeczko)

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/Makefile       |   3 +-
 drivers/gpu/drm/i915/intel_guc_fw.c |  10 +-
 drivers/gpu/drm/i915/intel_huc.c    | 154 +-----------------------------
 drivers/gpu/drm/i915/intel_huc.h    |   2 +-
 drivers/gpu/drm/i915/intel_huc_fw.c | 184 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_huc_fw.h |  33 +++++++
 drivers/gpu/drm/i915/intel_uc.c     |   2 +-
 7 files changed, 227 insertions(+), 161 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_huc_fw.c
 create mode 100644 drivers/gpu/drm/i915/intel_huc_fw.h

Comments

Michal Wajdeczko March 1, 2018, 2:51 p.m. UTC | #1
On Thu, 01 Mar 2018 15:47:22 +0100, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> GuC load function is named intel_guc_fw_upload() and HuC load function is
> named intel_huc_init_hw(). Make them consistent intel_*_fw_upload. Also
> move HuC fw loading functions and declarations to separate files
> intel_huc_fw.c|h like GuC.
>
> While at this, do below changes
> 1. Update kernel-doc comment for intel_*_fw_upload() functions
> 2. s/huc_ucode_xfer/huc_fw_xfer
> 3. Introduce intel_huc_fw_init_early()
>
> v2: Changed patch to update HuC functions instead of changing
>     guc_fw_upload and update file structure. (Michal Wajdeczko)
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile       |   3 +-
>  drivers/gpu/drm/i915/intel_guc_fw.c |  10 +-
>  drivers/gpu/drm/i915/intel_huc.c    | 154 +-----------------------------
>  drivers/gpu/drm/i915/intel_huc.h    |   2 +-
>  drivers/gpu/drm/i915/intel_huc_fw.c | 184  
> ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_huc_fw.h |  33 +++++++
>  drivers/gpu/drm/i915/intel_uc.c     |   2 +-
>  7 files changed, 227 insertions(+), 161 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_huc_fw.c
>  create mode 100644 drivers/gpu/drm/i915/intel_huc_fw.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile  
> b/drivers/gpu/drm/i915/Makefile
> index 881d712..1bd9bc5 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -89,7 +89,8 @@ i915-y += intel_uc.o \
>  	  intel_guc_fw.o \
>  	  intel_guc_log.o \
>  	  intel_guc_submission.o \
> -	  intel_huc.o
> +	  intel_huc.o \
> +	  intel_huc_fw.o
> # autogenerated null render state
>  i915-y += intel_renderstate_gen6.o \
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c  
> b/drivers/gpu/drm/i915/intel_guc_fw.c
> index 3b09329..d07f2b9 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -269,15 +269,15 @@ static int guc_fw_xfer(struct intel_uc_fw *guc_fw,  
> struct i915_vma *vma)
>  }
> /**
> - * intel_guc_fw_upload() - finish preparing the GuC for activity
> + * intel_guc_fw_upload() - load GuC uCode to device
>   * @guc: intel_guc structure
>   *
> - * Called during driver loading and also after a GPU reset.
> + * Called from intel_uc_init_hw() during driver load, resume from sleep  
> and
> + * after a GPU reset.
>   *
> - * The main action required here it to load the GuC uCode into the  
> device.
>   * The firmware image should have already been fetched into memory by  
> the
> - * earlier call to intel_guc_init(), so here we need only check that
> - * worked, and then transfer the image to the h/w.
> + * earlier call to intel_uc_init_fw(), so here we need to only check  
> that
> + * fetch succeeded, and then transfer the image to the h/w.
>   *
>   * Return:	non-zero code on error
>   */
> diff --git a/drivers/gpu/drm/i915/intel_huc.c  
> b/drivers/gpu/drm/i915/intel_huc.c
> index ef9a05d..e37f58e 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -27,161 +27,9 @@
>  #include "intel_huc.h"
>  #include "i915_drv.h"
> -/**
> - * DOC: HuC Firmware
> - *
> - * Motivation:
> - * GEN9 introduces a new dedicated firmware for usage in media HEVC  
> (High
> - * Efficiency Video Coding) operations. Userspace can use the firmware
> - * capabilities by adding HuC specific commands to batch buffers.
> - *
> - * Implementation:
> - * The same firmware loader is used as the GuC. However, the actual
> - * loading to HW is deferred until GEM initialization is done.
> - *
> - * Note that HuC firmware loading must be done before GuC loading.
> - */
> -
> -#define BXT_HUC_FW_MAJOR 01
> -#define BXT_HUC_FW_MINOR 07
> -#define BXT_BLD_NUM 1398
> -
> -#define SKL_HUC_FW_MAJOR 01
> -#define SKL_HUC_FW_MINOR 07
> -#define SKL_BLD_NUM 1398
> -
> -#define KBL_HUC_FW_MAJOR 02
> -#define KBL_HUC_FW_MINOR 00
> -#define KBL_BLD_NUM 1810
> -
> -#define HUC_FW_PATH(platform, major, minor, bld_num) \
> -	"i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
> -	__stringify(minor) "_" __stringify(bld_num) ".bin"
> -
> -#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \
> -	SKL_HUC_FW_MINOR, SKL_BLD_NUM)
> -MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
> -
> -#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \
> -	BXT_HUC_FW_MINOR, BXT_BLD_NUM)
> -MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
> -
> -#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \
> -	KBL_HUC_FW_MINOR, KBL_BLD_NUM)
> -MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
> -
> -static void huc_fw_select(struct intel_uc_fw *huc_fw)
> -{
> -	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
> -	struct drm_i915_private *dev_priv = huc_to_i915(huc);
> -
> -	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
> -
> -	if (!HAS_HUC(dev_priv))
> -		return;
> -
> -	if (i915_modparams.huc_firmware_path) {
> -		huc_fw->path = i915_modparams.huc_firmware_path;
> -		huc_fw->major_ver_wanted = 0;
> -		huc_fw->minor_ver_wanted = 0;
> -	} else if (IS_SKYLAKE(dev_priv)) {
> -		huc_fw->path = I915_SKL_HUC_UCODE;
> -		huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
> -		huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
> -	} else if (IS_BROXTON(dev_priv)) {
> -		huc_fw->path = I915_BXT_HUC_UCODE;
> -		huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
> -		huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
> -	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> -		huc_fw->path = I915_KBL_HUC_UCODE;
> -		huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
> -		huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
> -	} else {
> -		DRM_WARN("%s: No firmware known for this platform!\n",
> -			 intel_uc_fw_type_repr(huc_fw->type));
> -	}
> -}
> -
> -/**
> - * intel_huc_init_early() - initializes HuC struct
> - * @huc: intel_huc struct
> - *
> - * On platforms with HuC selects firmware for uploading
> - */
>  void intel_huc_init_early(struct intel_huc *huc)
>  {
> -	struct intel_uc_fw *huc_fw = &huc->fw;
> -
> -	intel_uc_fw_init(huc_fw, INTEL_UC_FW_TYPE_HUC);
> -	huc_fw_select(huc_fw);
> -}
> -
> -/**
> - * huc_ucode_xfer() - DMA's the firmware
> - * @huc_fw: the firmware descriptor
> - * @vma: the firmware image (bound into the GGTT)
> - *
> - * Transfer the firmware image to RAM for execution by the  
> microcontroller.
> - *
> - * Return: 0 on success, non-zero on failure
> - */
> -static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma  
> *vma)
> -{
> -	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
> -	struct drm_i915_private *dev_priv = huc_to_i915(huc);
> -	unsigned long offset = 0;
> -	u32 size;
> -	int ret;
> -
> -	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
> -
> -	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> -
> -	/* Set the source address for the uCode */
> -	offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
> -	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> -	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
> -
> -	/* Hardware doesn't look at destination address for HuC. Set it to 0,
> -	 * but still program the correct address space.
> -	 */
> -	I915_WRITE(DMA_ADDR_1_LOW, 0);
> -	I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
> -
> -	size = huc_fw->header_size + huc_fw->ucode_size;
> -	I915_WRITE(DMA_COPY_SIZE, size);
> -
> -	/* Start the DMA */
> -	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
> -
> -	/* Wait for DMA to finish */
> -	ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0,  
> 100);
> -
> -	DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
> -
> -	/* Disable the bits once DMA is over */
> -	I915_WRITE(DMA_CTRL, _MASKED_BIT_DISABLE(HUC_UKERNEL));
> -
> -	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> -
> -	return ret;
> -}
> -
> -/**
> - * intel_huc_init_hw() - load HuC uCode to device
> - * @huc: intel_huc structure
> - *
> - * Called from intel_uc_init_hw() during driver loading and also after  
> a GPU
> - * reset. Be note that HuC loading must be done before GuC loading.
> - *
> - * The firmware image should have already been fetched into memory by  
> the
> - * earlier call to intel_uc_init_fw(), so here we need only check that
> - * is succeeded, and then transfer the image to the h/w.
> - *
> - */
> -int intel_huc_init_hw(struct intel_huc *huc)
> -{
> -	return intel_uc_fw_upload(&huc->fw, huc_ucode_xfer);
> +	intel_huc_fw_init_early(huc);
>  }
> /**
> diff --git a/drivers/gpu/drm/i915/intel_huc.h  
> b/drivers/gpu/drm/i915/intel_huc.h
> index 40039db..5d6e804 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -26,6 +26,7 @@
>  #define _INTEL_HUC_H_
> #include "intel_uc_fw.h"
> +#include "intel_huc_fw.h"

hmm, as we don't need anything from this header, maybe we can drop it ?

> struct intel_huc {
>  	/* Generic uC firmware management */
> @@ -35,7 +36,6 @@ struct intel_huc {
>  };
> void intel_huc_init_early(struct intel_huc *huc);
> -int intel_huc_init_hw(struct intel_huc *huc);
>  int intel_huc_auth(struct intel_huc *huc);
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c  
> b/drivers/gpu/drm/i915/intel_huc_fw.c
> new file mode 100644
> index 0000000..d83a63d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_huc_fw.c
> @@ -0,0 +1,184 @@
> +/*
> + * Copyright © 2016-2018 Intel Corporation

I think there was agreement to use SPDX license identifiers only.

> + *
> + * Permission is hereby granted, free of charge, to any person  
> obtaining a
> + * copy of this software and associated documentation files (the  
> "Software"),
> + * to deal in the Software without restriction, including without  
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,  
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the  
> next
> + * paragraph) shall be included in all copies or substantial portions  
> of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,  
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF  
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT  
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR  
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,  
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER  
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "intel_huc_fw.h"
> +#include "i915_drv.h"
> +
> +/**
> + * DOC: HuC Firmware
> + *
> + * Motivation:
> + * GEN9 introduces a new dedicated firmware for usage in media HEVC  
> (High
> + * Efficiency Video Coding) operations. Userspace can use the firmware
> + * capabilities by adding HuC specific commands to batch buffers.
> + *
> + * Implementation:
> + * The same firmware loader is used as the GuC. However, the actual
> + * loading to HW is deferred until GEM initialization is done.
> + *
> + * Note that HuC firmware loading must be done before GuC loading.
> + */
> +
> +#define BXT_HUC_FW_MAJOR 01
> +#define BXT_HUC_FW_MINOR 07
> +#define BXT_BLD_NUM 1398
> +
> +#define SKL_HUC_FW_MAJOR 01
> +#define SKL_HUC_FW_MINOR 07
> +#define SKL_BLD_NUM 1398
> +
> +#define KBL_HUC_FW_MAJOR 02
> +#define KBL_HUC_FW_MINOR 00
> +#define KBL_BLD_NUM 1810
> +
> +#define HUC_FW_PATH(platform, major, minor, bld_num) \
> +	"i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
> +	__stringify(minor) "_" __stringify(bld_num) ".bin"
> +
> +#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \
> +	SKL_HUC_FW_MINOR, SKL_BLD_NUM)
> +MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
> +
> +#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \
> +	BXT_HUC_FW_MINOR, BXT_BLD_NUM)
> +MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
> +
> +#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \
> +	KBL_HUC_FW_MINOR, KBL_BLD_NUM)
> +MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
> +
> +static void huc_fw_select(struct intel_uc_fw *huc_fw)
> +{
> +	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
> +	struct drm_i915_private *dev_priv = huc_to_i915(huc);
> +
> +	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
> +
> +	if (!HAS_HUC(dev_priv))
> +		return;
> +
> +	if (i915_modparams.huc_firmware_path) {
> +		huc_fw->path = i915_modparams.huc_firmware_path;
> +		huc_fw->major_ver_wanted = 0;
> +		huc_fw->minor_ver_wanted = 0;
> +	} else if (IS_SKYLAKE(dev_priv)) {
> +		huc_fw->path = I915_SKL_HUC_UCODE;
> +		huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
> +		huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
> +	} else if (IS_BROXTON(dev_priv)) {
> +		huc_fw->path = I915_BXT_HUC_UCODE;
> +		huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
> +		huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
> +	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> +		huc_fw->path = I915_KBL_HUC_UCODE;
> +		huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
> +		huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
> +	} else {
> +		DRM_WARN("%s: No firmware known for this platform!\n",
> +			 intel_uc_fw_type_repr(huc_fw->type));
> +	}
> +}
> +
> +/**
> + * intel_huc_fw_init_early() - initializes HuC firmware struct
> + * @huc: intel_huc struct
> + *
> + * On platforms with HuC selects firmware for uploading
> + */
> +void intel_huc_fw_init_early(struct intel_huc *huc)
> +{
> +	struct intel_uc_fw *huc_fw = &huc->fw;
> +
> +	intel_uc_fw_init(huc_fw, INTEL_UC_FW_TYPE_HUC);
> +	huc_fw_select(huc_fw);
> +}
> +
> +/**
> + * huc_fw_xfer() - DMA's the firmware
> + * @huc_fw: the firmware descriptor
> + * @vma: the firmware image (bound into the GGTT)
> + *
> + * Transfer the firmware image to RAM for execution by the  
> microcontroller.
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
> +{
> +	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
> +	struct drm_i915_private *dev_priv = huc_to_i915(huc);
> +	unsigned long offset = 0;
> +	u32 size;
> +	int ret;
> +
> +	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
> +
> +	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> +
> +	/* Set the source address for the uCode */
> +	offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
> +	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> +	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
> +
> +	/* Hardware doesn't look at destination address for HuC. Set it to 0,
> +	 * but still program the correct address space.
> +	 */
> +	I915_WRITE(DMA_ADDR_1_LOW, 0);
> +	I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
> +
> +	size = huc_fw->header_size + huc_fw->ucode_size;
> +	I915_WRITE(DMA_COPY_SIZE, size);
> +
> +	/* Start the DMA */
> +	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
> +
> +	/* Wait for DMA to finish */
> +	ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0,  
> 100);
> +
> +	DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
> +
> +	/* Disable the bits once DMA is over */
> +	I915_WRITE(DMA_CTRL, _MASKED_BIT_DISABLE(HUC_UKERNEL));
> +
> +	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> +
> +	return ret;
> +}
> +
> +/**
> + * intel_huc_fw_upload() - load HuC uCode to device
> + * @huc: intel_huc structure
> + *
> + * Called from intel_uc_init_hw() during driver load, resume from sleep  
> and
> + * after a GPU reset. Note that HuC must be loaded before GuC.
> + *
> + * The firmware image should have already been fetched into memory by  
> the
> + * earlier call to intel_uc_init_fw(), so here we need to only check  
> that
> + * fetch succeeded, and then transfer the image to the h/w.
> + *
> + * Return:	non-zero code on error
> + */
> +int intel_huc_fw_upload(struct intel_huc *huc)
> +{
> +	return intel_uc_fw_upload(&huc->fw, huc_fw_xfer);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.h  
> b/drivers/gpu/drm/i915/intel_huc_fw.h
> new file mode 100644
> index 0000000..ab0a0b9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_huc_fw.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright © 2014-2018 Intel Corporation

use SPDX

> + *
> + * Permission is hereby granted, free of charge, to any person  
> obtaining a
> + * copy of this software and associated documentation files (the  
> "Software"),
> + * to deal in the Software without restriction, including without  
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,  
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the  
> next
> + * paragraph) shall be included in all copies or substantial portions  
> of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,  
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF  
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT  
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR  
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,  
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER  
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef _INTEL_HUC_FW_H_
> +#define _INTEL_HUC_FW_H_
> +
> +struct intel_huc;
> +
> +void intel_huc_fw_init_early(struct intel_huc *huc);
> +int intel_huc_fw_upload(struct intel_huc *huc);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 9f1bac6..8e25474 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -361,7 +361,7 @@ int intel_uc_init_hw(struct drm_i915_private  
> *dev_priv)
>  			goto err_out;
> 		if (USES_HUC(dev_priv)) {
> -			ret = intel_huc_init_hw(huc);
> +			ret = intel_huc_fw_upload(huc);
>  			if (ret)
>  				goto err_out;
>  		}

LGTM, and with SPDX fixes,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
sagar.a.kamble@intel.com March 1, 2018, 3:01 p.m. UTC | #2
On 3/1/2018 8:21 PM, Michal Wajdeczko wrote:
> On Thu, 01 Mar 2018 15:47:22 +0100, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> GuC load function is named intel_guc_fw_upload() and HuC load 
>> function is
>> named intel_huc_init_hw(). Make them consistent intel_*_fw_upload. Also
>> move HuC fw loading functions and declarations to separate files
>> intel_huc_fw.c|h like GuC.
>>
>> While at this, do below changes
>> 1. Update kernel-doc comment for intel_*_fw_upload() functions
>> 2. s/huc_ucode_xfer/huc_fw_xfer
>> 3. Introduce intel_huc_fw_init_early()
>>
>> v2: Changed patch to update HuC functions instead of changing
>>     guc_fw_upload and update file structure. (Michal Wajdeczko)
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
<snip>
>> diff --git a/drivers/gpu/drm/i915/intel_huc.h 
>> b/drivers/gpu/drm/i915/intel_huc.h
>> index 40039db..5d6e804 100644
>> --- a/drivers/gpu/drm/i915/intel_huc.h
>> +++ b/drivers/gpu/drm/i915/intel_huc.h
>> @@ -26,6 +26,7 @@
>>  #define _INTEL_HUC_H_
>> #include "intel_uc_fw.h"
>> +#include "intel_huc_fw.h"
>
> hmm, as we don't need anything from this header, maybe we can drop it ?
fw_init_early called from huc.c and fw_upload from uc.c which can access 
through intel_huc.h.
Similar to GuC. Not including huc_fw.h directly in huc.c or uc.c
>
>> struct intel_huc {
>>      /* Generic uC firmware management */
>> @@ -35,7 +36,6 @@ struct intel_huc {
>>  };
>> void intel_huc_init_early(struct intel_huc *huc);
>> -int intel_huc_init_hw(struct intel_huc *huc);
>>  int intel_huc_auth(struct intel_huc *huc);
>> #endif
>> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c 
>> b/drivers/gpu/drm/i915/intel_huc_fw.c
>> new file mode 100644
>> index 0000000..d83a63d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_huc_fw.c
>> @@ -0,0 +1,184 @@
>> +/*
>> + * Copyright © 2016-2018 Intel Corporation
>
> I think there was agreement to use SPDX license identifiers only.
>
Ah. right. will update. Thanks
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial 
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +
>> +#include "intel_huc_fw.h"
>> +#include "i915_drv.h"
>> +
>> +/**
>> + * DOC: HuC Firmware
>> + *
>> + * Motivation:
>> + * GEN9 introduces a new dedicated firmware for usage in media HEVC 
>> (High
>> + * Efficiency Video Coding) operations. Userspace can use the firmware
>> + * capabilities by adding HuC specific commands to batch buffers.
>> + *
>> + * Implementation:
>> + * The same firmware loader is used as the GuC. However, the actual
>> + * loading to HW is deferred until GEM initialization is done.
>> + *
>> + * Note that HuC firmware loading must be done before GuC loading.
>> + */
>> +
>> +#define BXT_HUC_FW_MAJOR 01
>> +#define BXT_HUC_FW_MINOR 07
>> +#define BXT_BLD_NUM 1398
>> +
>> +#define SKL_HUC_FW_MAJOR 01
>> +#define SKL_HUC_FW_MINOR 07
>> +#define SKL_BLD_NUM 1398
>> +
>> +#define KBL_HUC_FW_MAJOR 02
>> +#define KBL_HUC_FW_MINOR 00
>> +#define KBL_BLD_NUM 1810
>> +
>> +#define HUC_FW_PATH(platform, major, minor, bld_num) \
>> +    "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
>> +    __stringify(minor) "_" __stringify(bld_num) ".bin"
>> +
>> +#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \
>> +    SKL_HUC_FW_MINOR, SKL_BLD_NUM)
>> +MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
>> +
>> +#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \
>> +    BXT_HUC_FW_MINOR, BXT_BLD_NUM)
>> +MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
>> +
>> +#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \
>> +    KBL_HUC_FW_MINOR, KBL_BLD_NUM)
>> +MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
>> +
>> +static void huc_fw_select(struct intel_uc_fw *huc_fw)
>> +{
>> +    struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
>> +    struct drm_i915_private *dev_priv = huc_to_i915(huc);
>> +
>> +    GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
>> +
>> +    if (!HAS_HUC(dev_priv))
>> +        return;
>> +
>> +    if (i915_modparams.huc_firmware_path) {
>> +        huc_fw->path = i915_modparams.huc_firmware_path;
>> +        huc_fw->major_ver_wanted = 0;
>> +        huc_fw->minor_ver_wanted = 0;
>> +    } else if (IS_SKYLAKE(dev_priv)) {
>> +        huc_fw->path = I915_SKL_HUC_UCODE;
>> +        huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
>> +        huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
>> +    } else if (IS_BROXTON(dev_priv)) {
>> +        huc_fw->path = I915_BXT_HUC_UCODE;
>> +        huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
>> +        huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
>> +    } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
>> +        huc_fw->path = I915_KBL_HUC_UCODE;
>> +        huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
>> +        huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
>> +    } else {
>> +        DRM_WARN("%s: No firmware known for this platform!\n",
>> +             intel_uc_fw_type_repr(huc_fw->type));
>> +    }
>> +}
>> +
>> +/**
>> + * intel_huc_fw_init_early() - initializes HuC firmware struct
>> + * @huc: intel_huc struct
>> + *
>> + * On platforms with HuC selects firmware for uploading
>> + */
>> +void intel_huc_fw_init_early(struct intel_huc *huc)
>> +{
>> +    struct intel_uc_fw *huc_fw = &huc->fw;
>> +
>> +    intel_uc_fw_init(huc_fw, INTEL_UC_FW_TYPE_HUC);
>> +    huc_fw_select(huc_fw);
>> +}
>> +
>> +/**
>> + * huc_fw_xfer() - DMA's the firmware
>> + * @huc_fw: the firmware descriptor
>> + * @vma: the firmware image (bound into the GGTT)
>> + *
>> + * Transfer the firmware image to RAM for execution by the 
>> microcontroller.
>> + *
>> + * Return: 0 on success, non-zero on failure
>> + */
>> +static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma 
>> *vma)
>> +{
>> +    struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
>> +    struct drm_i915_private *dev_priv = huc_to_i915(huc);
>> +    unsigned long offset = 0;
>> +    u32 size;
>> +    int ret;
>> +
>> +    GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
>> +
>> +    intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> +
>> +    /* Set the source address for the uCode */
>> +    offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
>> +    I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
>> +    I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
>> +
>> +    /* Hardware doesn't look at destination address for HuC. Set it 
>> to 0,
>> +     * but still program the correct address space.
>> +     */
>> +    I915_WRITE(DMA_ADDR_1_LOW, 0);
>> +    I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
>> +
>> +    size = huc_fw->header_size + huc_fw->ucode_size;
>> +    I915_WRITE(DMA_COPY_SIZE, size);
>> +
>> +    /* Start the DMA */
>> +    I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
>> +
>> +    /* Wait for DMA to finish */
>> +    ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 
>> 0, 100);
>> +
>> +    DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
>> +
>> +    /* Disable the bits once DMA is over */
>> +    I915_WRITE(DMA_CTRL, _MASKED_BIT_DISABLE(HUC_UKERNEL));
>> +
>> +    intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>> +
>> +    return ret;
>> +}
>> +
>> +/**
>> + * intel_huc_fw_upload() - load HuC uCode to device
>> + * @huc: intel_huc structure
>> + *
>> + * Called from intel_uc_init_hw() during driver load, resume from 
>> sleep and
>> + * after a GPU reset. Note that HuC must be loaded before GuC.
>> + *
>> + * The firmware image should have already been fetched into memory 
>> by the
>> + * earlier call to intel_uc_init_fw(), so here we need to only check 
>> that
>> + * fetch succeeded, and then transfer the image to the h/w.
>> + *
>> + * Return:    non-zero code on error
>> + */
>> +int intel_huc_fw_upload(struct intel_huc *huc)
>> +{
>> +    return intel_uc_fw_upload(&huc->fw, huc_fw_xfer);
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.h 
>> b/drivers/gpu/drm/i915/intel_huc_fw.h
>> new file mode 100644
>> index 0000000..ab0a0b9
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_huc_fw.h
>> @@ -0,0 +1,33 @@
>> +/*
>> + * Copyright © 2014-2018 Intel Corporation
>
> use SPDX
>
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial 
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +
>> +#ifndef _INTEL_HUC_FW_H_
>> +#define _INTEL_HUC_FW_H_
>> +
>> +struct intel_huc;
>> +
>> +void intel_huc_fw_init_early(struct intel_huc *huc);
>> +int intel_huc_fw_upload(struct intel_huc *huc);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 9f1bac6..8e25474 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -361,7 +361,7 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>              goto err_out;
>>         if (USES_HUC(dev_priv)) {
>> -            ret = intel_huc_init_hw(huc);
>> +            ret = intel_huc_fw_upload(huc);
>>              if (ret)
>>                  goto err_out;
>>          }
>
> LGTM, and with SPDX fixes,
>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Thanks for the review

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 881d712..1bd9bc5 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -89,7 +89,8 @@  i915-y += intel_uc.o \
 	  intel_guc_fw.o \
 	  intel_guc_log.o \
 	  intel_guc_submission.o \
-	  intel_huc.o
+	  intel_huc.o \
+	  intel_huc_fw.o
 
 # autogenerated null render state
 i915-y += intel_renderstate_gen6.o \
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 3b09329..d07f2b9 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -269,15 +269,15 @@  static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
 }
 
 /**
- * intel_guc_fw_upload() - finish preparing the GuC for activity
+ * intel_guc_fw_upload() - load GuC uCode to device
  * @guc: intel_guc structure
  *
- * Called during driver loading and also after a GPU reset.
+ * Called from intel_uc_init_hw() during driver load, resume from sleep and
+ * after a GPU reset.
  *
- * The main action required here it to load the GuC uCode into the device.
  * The firmware image should have already been fetched into memory by the
- * earlier call to intel_guc_init(), so here we need only check that
- * worked, and then transfer the image to the h/w.
+ * earlier call to intel_uc_init_fw(), so here we need to only check that
+ * fetch succeeded, and then transfer the image to the h/w.
  *
  * Return:	non-zero code on error
  */
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index ef9a05d..e37f58e 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -27,161 +27,9 @@ 
 #include "intel_huc.h"
 #include "i915_drv.h"
 
-/**
- * DOC: HuC Firmware
- *
- * Motivation:
- * GEN9 introduces a new dedicated firmware for usage in media HEVC (High
- * Efficiency Video Coding) operations. Userspace can use the firmware
- * capabilities by adding HuC specific commands to batch buffers.
- *
- * Implementation:
- * The same firmware loader is used as the GuC. However, the actual
- * loading to HW is deferred until GEM initialization is done.
- *
- * Note that HuC firmware loading must be done before GuC loading.
- */
-
-#define BXT_HUC_FW_MAJOR 01
-#define BXT_HUC_FW_MINOR 07
-#define BXT_BLD_NUM 1398
-
-#define SKL_HUC_FW_MAJOR 01
-#define SKL_HUC_FW_MINOR 07
-#define SKL_BLD_NUM 1398
-
-#define KBL_HUC_FW_MAJOR 02
-#define KBL_HUC_FW_MINOR 00
-#define KBL_BLD_NUM 1810
-
-#define HUC_FW_PATH(platform, major, minor, bld_num) \
-	"i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
-	__stringify(minor) "_" __stringify(bld_num) ".bin"
-
-#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \
-	SKL_HUC_FW_MINOR, SKL_BLD_NUM)
-MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
-
-#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \
-	BXT_HUC_FW_MINOR, BXT_BLD_NUM)
-MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
-
-#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \
-	KBL_HUC_FW_MINOR, KBL_BLD_NUM)
-MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
-
-static void huc_fw_select(struct intel_uc_fw *huc_fw)
-{
-	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
-	struct drm_i915_private *dev_priv = huc_to_i915(huc);
-
-	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
-
-	if (!HAS_HUC(dev_priv))
-		return;
-
-	if (i915_modparams.huc_firmware_path) {
-		huc_fw->path = i915_modparams.huc_firmware_path;
-		huc_fw->major_ver_wanted = 0;
-		huc_fw->minor_ver_wanted = 0;
-	} else if (IS_SKYLAKE(dev_priv)) {
-		huc_fw->path = I915_SKL_HUC_UCODE;
-		huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
-		huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
-	} else if (IS_BROXTON(dev_priv)) {
-		huc_fw->path = I915_BXT_HUC_UCODE;
-		huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
-		huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
-	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
-		huc_fw->path = I915_KBL_HUC_UCODE;
-		huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
-		huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
-	} else {
-		DRM_WARN("%s: No firmware known for this platform!\n",
-			 intel_uc_fw_type_repr(huc_fw->type));
-	}
-}
-
-/**
- * intel_huc_init_early() - initializes HuC struct
- * @huc: intel_huc struct
- *
- * On platforms with HuC selects firmware for uploading
- */
 void intel_huc_init_early(struct intel_huc *huc)
 {
-	struct intel_uc_fw *huc_fw = &huc->fw;
-
-	intel_uc_fw_init(huc_fw, INTEL_UC_FW_TYPE_HUC);
-	huc_fw_select(huc_fw);
-}
-
-/**
- * huc_ucode_xfer() - DMA's the firmware
- * @huc_fw: the firmware descriptor
- * @vma: the firmware image (bound into the GGTT)
- *
- * Transfer the firmware image to RAM for execution by the microcontroller.
- *
- * Return: 0 on success, non-zero on failure
- */
-static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
-{
-	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
-	struct drm_i915_private *dev_priv = huc_to_i915(huc);
-	unsigned long offset = 0;
-	u32 size;
-	int ret;
-
-	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
-
-	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
-
-	/* Set the source address for the uCode */
-	offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
-	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
-	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
-
-	/* Hardware doesn't look at destination address for HuC. Set it to 0,
-	 * but still program the correct address space.
-	 */
-	I915_WRITE(DMA_ADDR_1_LOW, 0);
-	I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
-
-	size = huc_fw->header_size + huc_fw->ucode_size;
-	I915_WRITE(DMA_COPY_SIZE, size);
-
-	/* Start the DMA */
-	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
-
-	/* Wait for DMA to finish */
-	ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0, 100);
-
-	DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
-
-	/* Disable the bits once DMA is over */
-	I915_WRITE(DMA_CTRL, _MASKED_BIT_DISABLE(HUC_UKERNEL));
-
-	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
-
-	return ret;
-}
-
-/**
- * intel_huc_init_hw() - load HuC uCode to device
- * @huc: intel_huc structure
- *
- * Called from intel_uc_init_hw() during driver loading and also after a GPU
- * reset. Be note that HuC loading must be done before GuC loading.
- *
- * The firmware image should have already been fetched into memory by the
- * earlier call to intel_uc_init_fw(), so here we need only check that
- * is succeeded, and then transfer the image to the h/w.
- *
- */
-int intel_huc_init_hw(struct intel_huc *huc)
-{
-	return intel_uc_fw_upload(&huc->fw, huc_ucode_xfer);
+	intel_huc_fw_init_early(huc);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 40039db..5d6e804 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -26,6 +26,7 @@ 
 #define _INTEL_HUC_H_
 
 #include "intel_uc_fw.h"
+#include "intel_huc_fw.h"
 
 struct intel_huc {
 	/* Generic uC firmware management */
@@ -35,7 +36,6 @@  struct intel_huc {
 };
 
 void intel_huc_init_early(struct intel_huc *huc);
-int intel_huc_init_hw(struct intel_huc *huc);
 int intel_huc_auth(struct intel_huc *huc);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c b/drivers/gpu/drm/i915/intel_huc_fw.c
new file mode 100644
index 0000000..d83a63d
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_huc_fw.c
@@ -0,0 +1,184 @@ 
+/*
+ * Copyright © 2016-2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "intel_huc_fw.h"
+#include "i915_drv.h"
+
+/**
+ * DOC: HuC Firmware
+ *
+ * Motivation:
+ * GEN9 introduces a new dedicated firmware for usage in media HEVC (High
+ * Efficiency Video Coding) operations. Userspace can use the firmware
+ * capabilities by adding HuC specific commands to batch buffers.
+ *
+ * Implementation:
+ * The same firmware loader is used as the GuC. However, the actual
+ * loading to HW is deferred until GEM initialization is done.
+ *
+ * Note that HuC firmware loading must be done before GuC loading.
+ */
+
+#define BXT_HUC_FW_MAJOR 01
+#define BXT_HUC_FW_MINOR 07
+#define BXT_BLD_NUM 1398
+
+#define SKL_HUC_FW_MAJOR 01
+#define SKL_HUC_FW_MINOR 07
+#define SKL_BLD_NUM 1398
+
+#define KBL_HUC_FW_MAJOR 02
+#define KBL_HUC_FW_MINOR 00
+#define KBL_BLD_NUM 1810
+
+#define HUC_FW_PATH(platform, major, minor, bld_num) \
+	"i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
+	__stringify(minor) "_" __stringify(bld_num) ".bin"
+
+#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \
+	SKL_HUC_FW_MINOR, SKL_BLD_NUM)
+MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
+
+#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \
+	BXT_HUC_FW_MINOR, BXT_BLD_NUM)
+MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
+
+#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \
+	KBL_HUC_FW_MINOR, KBL_BLD_NUM)
+MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
+
+static void huc_fw_select(struct intel_uc_fw *huc_fw)
+{
+	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
+
+	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
+
+	if (!HAS_HUC(dev_priv))
+		return;
+
+	if (i915_modparams.huc_firmware_path) {
+		huc_fw->path = i915_modparams.huc_firmware_path;
+		huc_fw->major_ver_wanted = 0;
+		huc_fw->minor_ver_wanted = 0;
+	} else if (IS_SKYLAKE(dev_priv)) {
+		huc_fw->path = I915_SKL_HUC_UCODE;
+		huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
+		huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
+	} else if (IS_BROXTON(dev_priv)) {
+		huc_fw->path = I915_BXT_HUC_UCODE;
+		huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
+		huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
+	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
+		huc_fw->path = I915_KBL_HUC_UCODE;
+		huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
+		huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
+	} else {
+		DRM_WARN("%s: No firmware known for this platform!\n",
+			 intel_uc_fw_type_repr(huc_fw->type));
+	}
+}
+
+/**
+ * intel_huc_fw_init_early() - initializes HuC firmware struct
+ * @huc: intel_huc struct
+ *
+ * On platforms with HuC selects firmware for uploading
+ */
+void intel_huc_fw_init_early(struct intel_huc *huc)
+{
+	struct intel_uc_fw *huc_fw = &huc->fw;
+
+	intel_uc_fw_init(huc_fw, INTEL_UC_FW_TYPE_HUC);
+	huc_fw_select(huc_fw);
+}
+
+/**
+ * huc_fw_xfer() - DMA's the firmware
+ * @huc_fw: the firmware descriptor
+ * @vma: the firmware image (bound into the GGTT)
+ *
+ * Transfer the firmware image to RAM for execution by the microcontroller.
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
+{
+	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
+	unsigned long offset = 0;
+	u32 size;
+	int ret;
+
+	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
+
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+
+	/* Set the source address for the uCode */
+	offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
+	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
+	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
+
+	/* Hardware doesn't look at destination address for HuC. Set it to 0,
+	 * but still program the correct address space.
+	 */
+	I915_WRITE(DMA_ADDR_1_LOW, 0);
+	I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
+
+	size = huc_fw->header_size + huc_fw->ucode_size;
+	I915_WRITE(DMA_COPY_SIZE, size);
+
+	/* Start the DMA */
+	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
+
+	/* Wait for DMA to finish */
+	ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0, 100);
+
+	DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
+
+	/* Disable the bits once DMA is over */
+	I915_WRITE(DMA_CTRL, _MASKED_BIT_DISABLE(HUC_UKERNEL));
+
+	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+
+	return ret;
+}
+
+/**
+ * intel_huc_fw_upload() - load HuC uCode to device
+ * @huc: intel_huc structure
+ *
+ * Called from intel_uc_init_hw() during driver load, resume from sleep and
+ * after a GPU reset. Note that HuC must be loaded before GuC.
+ *
+ * The firmware image should have already been fetched into memory by the
+ * earlier call to intel_uc_init_fw(), so here we need to only check that
+ * fetch succeeded, and then transfer the image to the h/w.
+ *
+ * Return:	non-zero code on error
+ */
+int intel_huc_fw_upload(struct intel_huc *huc)
+{
+	return intel_uc_fw_upload(&huc->fw, huc_fw_xfer);
+}
diff --git a/drivers/gpu/drm/i915/intel_huc_fw.h b/drivers/gpu/drm/i915/intel_huc_fw.h
new file mode 100644
index 0000000..ab0a0b9
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_huc_fw.h
@@ -0,0 +1,33 @@ 
+/*
+ * Copyright © 2014-2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _INTEL_HUC_FW_H_
+#define _INTEL_HUC_FW_H_
+
+struct intel_huc;
+
+void intel_huc_fw_init_early(struct intel_huc *huc);
+int intel_huc_fw_upload(struct intel_huc *huc);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9f1bac6..8e25474 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -361,7 +361,7 @@  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 			goto err_out;
 
 		if (USES_HUC(dev_priv)) {
-			ret = intel_huc_init_hw(huc);
+			ret = intel_huc_fw_upload(huc);
 			if (ret)
 				goto err_out;
 		}