Re: [PATCH 3/6] drm/i915/huc: Add HuC fw loading support

Daniel Vetter
2016-06-22T08:31:53Z

On Tue, Jun 21, 2016 at 07:11:22PM +0100, Peter Antoine wrote:
> From: Alex Dai <yu.dai@intel.com>
>
> The HuC loading process is similar to GuC. The intel_uc_fw_fetch()
> is used for both cases.
>
> HuC loading needs to be before GuC loading. The WOPCM setting must
> be done early before loading any of them.
>
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_dma.c | 3 +
> drivers/gpu/drm/i915/i915_drv.h | 3 +
> drivers/gpu/drm/i915/i915_gem.c | 13 +-
> drivers/gpu/drm/i915/i915_guc_reg.h | 3 +
> drivers/gpu/drm/i915/intel_guc.h | 1 +
> drivers/gpu/drm/i915/intel_guc_loader.c | 11 +-
> drivers/gpu/drm/i915/intel_huc.h | 44 ++++++
> drivers/gpu/drm/i915/intel_huc_loader.c | 262 ++++++++++++++++++++++++++++++++
> 9 files changed, 331 insertions(+), 10 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_huc.h
> create mode 100644 drivers/gpu/drm/i915/intel_huc_loader.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 276abf1..7384db0 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -44,6 +44,7 @@ i915-y += i915_cmd_parser.o \
>
> # general-purpose microcontroller (GuC) support
> i915-y += intel_guc_loader.o \
> + intel_huc_loader.o \
> i915_guc_submission.o
>
> # autogenerated null render state
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index e28c0dd..a449a59 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -507,6 +507,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> * working irqs for e.g. gmbus and dp aux transfers. */
> intel_modeset_init(dev);
>
> + intel_huc_init(dev);
> intel_guc_init(dev);
>
> ret = i915_gem_init(dev);
> @@ -532,6 +533,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> cleanup_gem:
> i915_gem_fini(dev);
> cleanup_irq:
> + intel_huc_fini(dev);
> intel_guc_fini(dev);
> drm_irq_uninstall(dev);
> intel_teardown_gmbus(dev);
> @@ -1563,6 +1565,7 @@ int i915_driver_unload(struct drm_device *dev)
> /* Flush any outstanding unpin_work. */
> flush_workqueue(dev_priv->wq);
>
> + intel_huc_fini(dev);
> intel_guc_fini(dev);
> i915_gem_fini(dev);
> intel_fbc_cleanup_cfb(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7c81757..8b88227 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -54,6 +54,7 @@
> #include "intel_bios.h"
> #include "intel_dpll_mgr.h"
> #include "intel_guc.h"
> +#include "intel_huc.h"
> #include "intel_lrc.h"
> #include "intel_ringbuffer.h"
>
> @@ -1749,6 +1750,7 @@ struct drm_i915_private {
>
> struct intel_gvt gvt;
>
> + struct intel_huc huc;
> struct intel_guc guc;
>
> struct intel_csr csr;
> @@ -2841,6 +2843,7 @@ struct drm_i915_cmd_table {
> #define HAS_GUC(dev) (IS_GEN9(dev) && !IS_KABYLAKE(dev))
> #define HAS_GUC_UCODE(dev) (HAS_GUC(dev))
> #define HAS_GUC_SCHED(dev) (HAS_GUC(dev))
> +#define HAS_HUC_UCODE(dev) (HAS_GUC(dev))
>
> #define HAS_RESOURCE_STREAMER(dev) (IS_HASWELL(dev) || \
> INTEL_INFO(dev)->gen >= 8)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6abd5e5..549dd3f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5143,9 +5143,16 @@ i915_gem_init_hw(struct drm_device *dev)
> intel_mocs_init_l3cc_table(dev);
>
> /* We can't enable contexts until all firmware is loaded */
> - ret = intel_guc_setup(dev);
> - if (ret)
> - goto out;
> + 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;
> + }
>
> /*
> * Increment the next seqno by 0x100 so we have a visible break
> diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h
> index cf5a65b..51533f1 100644
> --- a/drivers/gpu/drm/i915/i915_guc_reg.h
> +++ b/drivers/gpu/drm/i915/i915_guc_reg.h
> @@ -61,9 +61,12 @@
> #define DMA_ADDRESS_SPACE_GTT (8 << 16)
> #define DMA_COPY_SIZE _MMIO(0xc310)
> #define DMA_CTRL _MMIO(0xc314)
> +#define HUC_UKERNEL (1<<9)
> #define UOS_MOVE (1<<4)
> #define START_DMA (1<<0)
> #define DMA_GUC_WOPCM_OFFSET _MMIO(0xc340)
> +#define HUC_LOADING_AGENT_VCR (0<<1)
> +#define HUC_LOADING_AGENT_GUC (1<<1)
> #define GUC_WOPCM_OFFSET_VALUE 0x80000 /* 512KB */
> #define GUC_MAX_IDLE_COUNT _MMIO(0xC3E4)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 0b4ed88..c4b92f3 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -160,6 +160,7 @@ extern const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
> extern int intel_guc_suspend(struct drm_device *dev);
> extern int intel_guc_resume(struct drm_device *dev);
> void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw);
> +u32 guc_wopcm_size(struct drm_device *dev);
>
> /* i915_guc_submission.c */
> int i915_guc_submission_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 70575bd..c4a210d 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -270,7 +270,8 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
> I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
>
> /* Finally start the DMA */
> - I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
> + I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA) |
> + _MASKED_BIT_DISABLE(HUC_UKERNEL));
>
> /*
> * Wait for the DMA to complete & the GuC to start up.
> @@ -295,12 +296,12 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
> return ret;
> }
>
> -static u32 guc_wopcm_size(struct drm_i915_private *dev_priv)
> +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_priv))
> + if (IS_BROXTON(dev))
> wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
>
> return wopcm_size;
> @@ -332,10 +333,6 @@ 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_priv));
> - 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);
>
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> new file mode 100644
> index 0000000..946caa7
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright © 2014 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_H_
> +#define _INTEL_HUC_H_
> +
> +#include "intel_guc.h"
> +
> +#define HUC_STATUS2 _MMIO(0xD3B0)
> +#define HUC_FW_VERIFIED (1<<7)
> +
> +struct intel_huc {
> + /* Generic uC firmware management */
> + struct intel_uc_fw huc_fw;
> +
> + /* HuC-specific additions */
> +};
> +
> +extern void intel_huc_init(struct drm_device *dev);
> +extern int intel_huc_load(struct drm_device *dev);
> +extern void intel_huc_auth(struct drm_device *dev);
> +extern void intel_huc_fini(struct drm_device *dev);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> new file mode 100644
> index 0000000..472fabe
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -0,0 +1,262 @@
> +/*
> + * Copyright © 2014 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 <linux/firmware.h>
> +#include "i915_drv.h"
> +#include "intel_huc.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:
> + * On supported platforms, i915's job is to load the firmware stored on the
> + * file system and assist with authentication. It is up to userspace to
> + * detect the presence of HuC support on a platform, on their own.
> + * For debugging, i915 provides a debugfs file, i915_huc_load_status_info
> + * which displays the firmware load status.
> + *
> + * The unified uC firmware loader is used. Firmware binary is fetched by the
> + * loader asynchronously from the driver init process. However, the actual
> + * loading to HW is deferred until GEM initialization is done. Be note that HuC
> + * firmware loading must be done before GuC loading.
> + */
> +
> +#define I915_SKL_HUC_UCODE "i915/skl_huc_ver1.bin"
> +MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
> +
> +/**
> + * intel_huc_load_ucode() - DMA's the firmware
> + * @dev: the drm device
> + *
> + * This function takes the gem object containing the firmware, sets up the DMA
> + * engine MMIO, triggers the DMA operation and waits for it to finish.
> + *
> + * 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 drm_i915_private *dev_priv)
> +{
> + struct intel_uc_fw *huc_fw = &dev_priv->huc.huc_fw;
> + unsigned long offset = 0;
> + u32 size;
> + int ret;
> +
> + ret = i915_gem_object_set_to_gtt_domain(huc_fw->uc_fw_obj, false);
> + if (ret) {
> + DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
> + return ret;
> + }
> +
> + ret = i915_gem_obj_ggtt_pin(huc_fw->uc_fw_obj, 0, 0);
> + if (ret) {
> + DRM_DEBUG_DRIVER("pin failed %d\n", ret);
> + return ret;
> + }
> +
> + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
> + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> +
> + WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> +
> + /* Set the source address for the uCode */
> + offset = i915_gem_obj_ggtt_offset(huc_fw->uc_fw_obj) +
> + 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 = wait_for_atomic((I915_READ(DMA_CTRL) & START_DMA) == 0, 50);
> +
> + DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
> +
> + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> +
> + /*
> + * We keep the object pages for reuse during resume. But we can unpin it
> + * now that DMA has completed, so it doesn't continue to take up space.
> + */
> + i915_gem_object_ggtt_unpin(huc_fw->uc_fw_obj);
> +
> + return ret;
> +}
> +
> +/**
> + * intel_huc_init() - initiate HuC firmware loading request
> + * @dev: the drm device
> + *
> + * Called early during driver load, but after GEM is initialised. The loading
> + * will continue only when driver explicitly specify firmware name and version.
> + * All other cases are considered as UC_FIRMWARE_NONE either because HW is not
> + * capable or driver yet support it. And there will be no error message for
> + * UC_FIRMWARE_NONE cases.
> + *
> + * The DMA-copying to HW is done later when intel_huc_ucode_load() is called.
> + */
> +void intel_huc_init(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_huc *huc = &dev_priv->huc;
> + struct intel_uc_fw *huc_fw = &huc->huc_fw;
> + const char *fw_path = NULL;
> +
> + huc_fw->uc_dev = dev;
> + huc_fw->uc_fw_path = NULL;
> + huc_fw->fetch_status = UC_FIRMWARE_NONE;
> + huc_fw->load_status = UC_FIRMWARE_NONE;
> + huc_fw->fw_type = UC_FW_TYPE_HUC;
> +
> + if (!HAS_HUC_UCODE(dev))
> + return;
> +
> + if (IS_SKYLAKE(dev)) {
> + fw_path = I915_SKL_HUC_UCODE;
> + huc_fw->major_ver_wanted = 1;
> + huc_fw->minor_ver_wanted = 5;
> + }
> +
> + if (fw_path == NULL)
> + return;
> +
> + huc_fw->uc_fw_path = fw_path;
> + huc_fw->fetch_status = UC_FIRMWARE_PENDING;
> +
> + DRM_DEBUG_DRIVER("HuC firmware pending, path %s\n", fw_path);
> +
> + intel_uc_fw_fetch(dev, huc_fw);
> +}
> +
> +/**
> + * intel_huc_load() - load HuC uCode to device
> + * @dev: the drm device
> + *
> + * Called from gem_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_huc_ucode_init(), so here we need only check that
> + * is succeeded, and then transfer the image to the h/w.
> + *
> + * Return: non-zero code on error
> + */
> +int intel_huc_load(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_uc_fw *huc_fw = &dev_priv->huc.huc_fw;
> + int err;
> +
> + if (huc_fw->fetch_status == UC_FIRMWARE_NONE)
> + return 0;
> +
> + DRM_DEBUG_DRIVER("%s fw status: fetch %s, load %s\n",
> + huc_fw->uc_fw_path,
> + intel_uc_fw_status_repr(huc_fw->fetch_status),
> + intel_uc_fw_status_repr(huc_fw->load_status));
> +
> + if (huc_fw->fetch_status == UC_FIRMWARE_SUCCESS &&
> + huc_fw->load_status == UC_FIRMWARE_FAIL)
> + return -ENOEXEC;
> +
> + huc_fw->load_status = UC_FIRMWARE_PENDING;
> +
> + switch (huc_fw->fetch_status) {
> + case UC_FIRMWARE_FAIL:
> + /* something went wrong :( */
> + err = -EIO;
> + goto fail;
> +
> + case UC_FIRMWARE_NONE:
> + case UC_FIRMWARE_PENDING:
> + default:
> + /* "can't happen" */
> + WARN_ONCE(1, "HuC fw %s invalid fetch_status %s [%d]\n",
> + huc_fw->uc_fw_path,
> + intel_uc_fw_status_repr(huc_fw->fetch_status),
> + huc_fw->fetch_status);
> + err = -ENXIO;
> + goto fail;
> +
> + case UC_FIRMWARE_SUCCESS:
> + break;
> + }

Can we please remove all that indirection through fetch_status? This
really shouldn't be that complicated that you need pages of code to
generate/decode status. I know it's copypaste from guc, but I didn't like
it there, and I definitely don't want 2 copies of this. Please simplify
both places.

The correct fix is to give the uc_fw_fetch function an error code, and rip
out all that cargo culted status handling. All of it.
-Daniel

> +
> + err = huc_ucode_xfer(dev_priv);
> + if (err)
> + goto fail;
> +
> + huc_fw->load_status = UC_FIRMWARE_SUCCESS;
> +
> + DRM_DEBUG_DRIVER("%s fw status: fetch %s, load %s\n",
> + huc_fw->uc_fw_path,
> + intel_uc_fw_status_repr(huc_fw->fetch_status),
> + intel_uc_fw_status_repr(huc_fw->load_status));
> +
> + return 0;
> +
> +fail:
> + if (huc_fw->load_status == UC_FIRMWARE_PENDING)
> + huc_fw->load_status = UC_FIRMWARE_FAIL;
> +
> + DRM_ERROR("Failed to complete HuC uCode load with ret %d\n", err);
> +
> + return err;
> +}
> +
> +/**
> + * intel_huc_fini() - clean up resources allocated for HuC
> + * @dev: the drm device
> + *
> + * Cleans up by releasing the huc firmware GEM obj.
> + */
> +void intel_huc_fini(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_uc_fw *huc_fw = &dev_priv->huc.huc_fw;
> +
> + mutex_lock(&dev->struct_mutex);
> + if (huc_fw->uc_fw_obj)
> + drm_gem_object_unreference(&huc_fw->uc_fw_obj->base);
> + huc_fw->uc_fw_obj = NULL;
> + mutex_unlock(&dev->struct_mutex);
> +
> + huc_fw->fetch_status = UC_FIRMWARE_NONE;
> +}
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation