[Intel-gfx] [PATCH 6/6] drm/i915/huc: Add BXT HuC Loading Support
Antoine, Peter
peter.antoine at intel.com
Tue Jun 21 21:28:19 UTC 2016
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 at lists.freedesktop.org; Antoine, Peter <peter.antoine at intel.com>
Cc: Prigent, Christophe <christophe.prigent at intel.com>; Kelley, Sean V <sean.v.kelley at intel.com>; Gordon, David S <david.s.gordon at intel.com>; Li, Lawrence T <lawrence.t.li at 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 at 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)
More information about the Intel-gfx
mailing list