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

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

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)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx