[Intel-gfx] [PATCH 3/8] drm/i915/huc: Add HuC fw loading support
Arkadiusz Hiler
arkadiusz.hiler at intel.com
Wed Jan 4 15:15:55 UTC 2017
On Tue, Jan 03, 2017 at 06:59:11PM +0000, Srivatsa, Anusha wrote:
>
>
> >-----Original Message-----
> >From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of
> >Srivatsa, Anusha
> >Sent: Monday, January 2, 2017 4:09 PM
> >To: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>
> >Cc: intel-gfx at lists.freedesktop.org; Alex Dai <yu.dai at intel.com>; Peter Antoine
> ><peter.antoine at intel.com>
> >Subject: Re: [Intel-gfx] [PATCH 3/8] drm/i915/huc: Add HuC fw loading support
> >
> >
> >
> >>-----Original Message-----
> >>From: Wajdeczko, Michal
> >>Sent: Tuesday, December 27, 2016 9:51 AM
> >>To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
> >>Cc: intel-gfx at lists.freedesktop.org; Alex Dai <yu.dai at intel.com>; Peter
> >>Antoine <peter.antoine at intel.com>
> >>Subject: Re: [Intel-gfx] [PATCH 3/8] drm/i915/huc: Add HuC fw loading
> >>support
> >>
> >>On Thu, Dec 22, 2016 at 03:12:19PM -0800, Anusha Srivatsa wrote:
> >>> 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.
> >>>
> >>> v2: rebased on-top of drm-intel-nightly.
> >>> removed if(HAS_GUC()) before the guc call. (D.Gordon)
> >>> update huc_version number of format.
> >>> v3: rebased to drm-intel-nightly, changed the file name format to
> >>> match the one in the huc package.
> >>> Changed dev->dev_private to to_i915()
> >>> v4: moved function back to where it was.
> >>> change wait_for_atomic to wait_for.
> >>> v5: rebased + comment changes.
> >>> v7: rebased.
> >>> v8: rebased.
> >>> v9: rebased. Changed the year in the copyright message to reflect the
> >>> right year.Correct the comments,remove the unwanted WARN message,
> >>> replace drm_gem_object_unreference() with i915_gem_object_put().Make
> >>> the prototypes in intel_huc.h non-extern.
> >>> v10: rebased. Update the file construction done by HuC. It is similar
> >>> to GuC.Adopted the approach used in-
> >>> https://patchwork.freedesktop.org/patch/104355/ <Tvrtko Ursulin>
> >>> v11: Fix warnings remove old declaration
> >>> v12: Change dev to dev_priv in macro definition.
> >>> Corrected comments.
> >>> v13: rebased.
> >>> v14: rebased on top of drm-tip
> >>> v15: rebased. Updated functions intel_huc_load(),intel_huc_init() and
> >>> intel_uc_fw_fetch() to accept dev_priv instead of dev. Moved contents
> >>> of intel_huc.h to intel_uc.h
> >>> v16: change SKL_FW_ to SKL_HUC_FW_. Add intel_ prefix to
> >>guc_wopcm_size().
> >>> Remove unwanted checks in intel_uc.h. Rename huc_fw in struct
> >>> intel_huc to simply fw to avoid redundency.
> >>> v17: rebased.
> >>>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>> Tested-by: Xiang Haihao <haihao.xiang at intel.com>
> >>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
> >>> Signed-off-by: Alex Dai <yu.dai at intel.com>
> >>> Signed-off-by: Peter Antoine <peter.antoine at intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/Makefile | 1 +
> >>> drivers/gpu/drm/i915/i915_drv.c | 4 +-
> >>> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> >>> drivers/gpu/drm/i915/i915_guc_reg.h | 3 +
> >>> drivers/gpu/drm/i915/intel_guc_loader.c | 11 +-
> >>> drivers/gpu/drm/i915/intel_huc_loader.c | 263
> >>++++++++++++++++++++++++++++++++
> >>> drivers/gpu/drm/i915/intel_uc.h | 18 +++
> >>> 7 files changed, 296 insertions(+), 7 deletions(-) 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 5196509..45ae124 100644
> >>> --- a/drivers/gpu/drm/i915/Makefile
> >>> +++ b/drivers/gpu/drm/i915/Makefile
> >>> @@ -57,6 +57,7 @@ i915-y += i915_cmd_parser.o \ # general-purpose
> >>> microcontroller (GuC) support i915-y += intel_uc.o \
> >>> intel_guc_loader.o \
> >>> + intel_huc_loader.o \
> >>> i915_guc_submission.o
> >>>
> >>> # autogenerated null render state
> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> >>> b/drivers/gpu/drm/i915/i915_drv.c index 6428588..85a47c2 100644
> >>> --- a/drivers/gpu/drm/i915/i915_drv.c
> >>> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >>> @@ -600,6 +600,7 @@ static int i915_load_modeset_init(struct
> >>> drm_device
> >>*dev)
> >>> if (ret)
> >>> goto cleanup_irq;
> >>>
> >>> + intel_huc_init(dev_priv);
> >>> intel_guc_init(dev_priv);
> >>>
> >>> ret = i915_gem_init(dev_priv);
> >>> @@ -627,6 +628,7 @@ static int i915_load_modeset_init(struct
> >>> drm_device
> >>*dev)
> >>> DRM_ERROR("failed to idle hardware; continuing to unload!\n");
> >>> i915_gem_fini(dev_priv);
> >>> cleanup_irq:
> >>> + intel_huc_fini(dev);
> >>> intel_guc_fini(dev_priv);
> >>> drm_irq_uninstall(dev);
> >>> intel_teardown_gmbus(dev_priv);
> >>> @@ -1313,7 +1315,7 @@ void i915_driver_unload(struct drm_device *dev)
> >>>
> >>> /* Flush any outstanding unpin_work. */
> >>> drain_workqueue(dev_priv->wq);
> >>> -
> >>> + intel_huc_fini(dev);
> >>> intel_guc_fini(dev_priv);
> >>> i915_gem_fini(dev_priv);
> >>> 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 1a91409..7ac7730 100644
> >>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>> @@ -2147,6 +2147,7 @@ struct drm_i915_private {
> >>>
> >>> struct intel_gvt *gvt;
> >>>
> >>> + struct intel_huc huc;
> >>> struct intel_guc guc;
> >>>
> >>> struct intel_csr csr;
> >>> @@ -2921,7 +2922,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> >>> #define HAS_GUC(dev_priv) ((dev_priv)->info.has_guc)
> >>> #define HAS_GUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
> >>> #define HAS_GUC_SCHED(dev_priv) (HAS_GUC(dev_priv))
> >>> -
> >>> +#define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
> >>> #define HAS_RESOURCE_STREAMER(dev_priv)
> >>> ((dev_priv)->info.has_resource_streamer)
> >>>
> >>> #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu)
> >>> diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h
> >>> b/drivers/gpu/drm/i915/i915_guc_reg.h
> >>> index 5e638fc..f9829f6 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_loader.c
> >>> b/drivers/gpu/drm/i915/intel_guc_loader.c
> >>> index 06e3e5c..8c77e94 100644
> >>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> >>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> >>> @@ -309,8 +309,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.
> >>> * NB: Docs recommend not using the interrupt for completion.
> >>> @@ -334,7 +334,7 @@ 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 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
> >>> {
> >>> u32 wopcm_size = GUC_WOPCM_TOP;
> >>>
> >>> @@ -372,7 +372,7 @@ 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(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
> >>> I915_WRITE(DMA_GUC_WOPCM_OFFSET,
> >>GUC_WOPCM_OFFSET_VALUE);
> >>>
> >>> /* Enable MIA caching. GuC clock gating is disabled. */ @@ -511,6
> >>> +511,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
> >>> if (err)
> >>> goto fail;
> >>>
> >>> + intel_huc_load(dev_priv);
> >>> err = guc_ucode_xfer(dev_priv);
> >>> if (!err)
> >>> break;
> >>> @@ -658,7 +659,7 @@ void intel_uc_fw_fetch(struct drm_i915_private
> >>*dev_priv,
> >>> 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_priv)) {
> >>> + if (size > intel_guc_wopcm_size(dev_priv)) {
> >>> 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
> >>> new file mode 100644
> >>> index 0000000..98d631c
> >>> --- /dev/null
> >>> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> >>> @@ -0,0 +1,263 @@
> >>> +/*
> >>> + * Copyright (c) 2016 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_uc.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 SKL_HUC_FW_MAJOR 01
> >>> +#define SKL_HUC_FW_MINOR 07
> >>> +#define SKL_BLD_NUM 1398
> >>> +
> >>> +#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);
> >>> +
> >>> +/**
> >>> + * huc_ucode_xfer() - DMA's the firmware
> >>> + * @dev_priv: the drm device
> >>> + *
> >>> + * This function takes the gem object containing the firmware, sets
> >>> +up the DMA
> >>
> >>Hmm, this function takes just dev_priv...
> >
> >Oops.... will change the comment.
> >
> >Anusha
> >>
> >>> + * 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.fw;
> >>> + struct i915_vma *vma;
> >>> + 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;
> >>> + }
> >>> +
> >>> + vma = i915_gem_object_ggtt_pin(huc_fw->uc_fw_obj, NULL, 0, 0, 0);
> >>> + if (IS_ERR(vma)) {
> >>> + DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma));
> >>> + return PTR_ERR(vma);
> >>> + }
> >>> +
> >>> + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
> >>> + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> >>> +
> >>> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> >>> +
> >>> + /* init WOPCM */
> >>> + I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
> >>> + I915_WRITE(DMA_GUC_WOPCM_OFFSET,
> >>GUC_WOPCM_OFFSET_VALUE |
> >>> + HUC_LOADING_AGENT_GUC);
> >>> +
> >>> + /* Set the source address for the uCode */
> >>> + offset = i915_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 = wait_for((I915_READ(DMA_CTRL) & START_DMA) == 0, 100);
> >>> +
> >>> + 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_vma_unpin(vma);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +
> >>> +/**
> >>> + * intel_huc_init() - initiate HuC firmware loading request
> >>> + * @dev_priv: the drm_i915_private 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 INTEL_UC_FIRMWARE_NONE either
> >>> +because HW
> >>> + * is not capable or driver yet support it. And there will be no
> >>> +error message
> >>> + * for INTEL_UC_FIRMWARE_NONE cases.
> >>> + *
> >>> + * The DMA-copying to HW is done later when intel_huc_load() is called.
> >>> + */
> >>> +void intel_huc_init(struct drm_i915_private *dev_priv) {
> >>> + struct intel_huc *huc = &dev_priv->huc;
> >>> + struct intel_uc_fw *huc_fw = &huc->fw;
> >>> + const char *fw_path = NULL;
> >>> +
> >>> + huc_fw->uc_fw_path = NULL;
> >>> + huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
> >>> + huc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
> >>> + huc_fw->fw_type = INTEL_UC_FW_TYPE_HUC;
> >>> +
> >>> + if (!HAS_HUC_UCODE(dev_priv))
> >>> + return;
> >>> +
> >>> + if (IS_SKYLAKE(dev_priv)) {
> >>> + fw_path = I915_SKL_HUC_UCODE;
> >>> + huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
> >>> + huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
> >>> + }
> >>> +
> >>> + huc_fw->uc_fw_path = fw_path;
> >>> + huc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
> >>> +
> >>> + DRM_DEBUG_DRIVER("HuC firmware pending, path %s\n", fw_path);
> >>> +
> >>> + intel_uc_fw_fetch(dev_priv, huc_fw); }
> >>> +
> >>> +/**
> >>> + * intel_huc_load() - load HuC uCode to device
> >>> + * @dev_priv: the drm_i915_private 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_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_i915_private *dev_priv) {
> >>> + struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
> >>> + int err;
> >>> +
> >>> + if (huc_fw->fetch_status == INTEL_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 == INTEL_UC_FIRMWARE_SUCCESS &&
> >>> + huc_fw->load_status == INTEL_UC_FIRMWARE_FAIL)
> >>> + return -ENOEXEC;
> >>> +
> >>> + huc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
> >>> +
> >>> + switch (huc_fw->fetch_status) {
> >>> + case INTEL_UC_FIRMWARE_FAIL:
> >>> + /* something went wrong :( */
> >>> + err = -EIO;
> >>> + goto fail;
> >>> +
> >>> + case INTEL_UC_FIRMWARE_NONE:
> >>> + case INTEL_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 INTEL_UC_FIRMWARE_SUCCESS:
> >>> + break;
> >>> + }
> >>> +
> >>> + err = huc_ucode_xfer(dev_priv);
> >>> + if (err)
> >>> + goto fail;
> >>> +
> >>> + huc_fw->load_status = INTEL_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));
> >>
> >>Hmm, this message will always display "fetch SUCCESS load SUCCESS"
> >>as all other cases all handled as fail below... is it expected ?
>
> Yes. I think we need a message for the case when there is no failure.
>
> >>> +
> >>> + return 0;
> >>> +
> >>> +fail:
> >>> + if (huc_fw->load_status == INTEL_UC_FIRMWARE_PENDING)
> >>> + huc_fw->load_status = INTEL_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)
> >>
> >>Why this function takes dev? All other functions take dev_priv.
>
> Last I heard this was the only function that took dev and there some WIP before we make it take dev_priv.
> Arek, can we change this now?
IIRC the refactoring that happened changed dev to dev_priv everywhere
where the latter was used exclusively. This patchset was changed
accordingly and this function was left with dev, as it uses it for lock
access.
But I see your point with being consistent.
> >>Michal
> >>
> >>> +{
> >>> + struct drm_i915_private *dev_priv = to_i915(dev);
> >>> + struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
> >>> +
> >>> + mutex_lock(&dev->struct_mutex);
> >>> + if (huc_fw->uc_fw_obj)
> >>> + i915_gem_object_put(huc_fw->uc_fw_obj);
> >>> + huc_fw->uc_fw_obj = NULL;
> >>> + mutex_unlock(&dev->struct_mutex);
> >>> +
> >>> + huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; }
> >>> +
> >>> diff --git a/drivers/gpu/drm/i915/intel_uc.h
> >>> b/drivers/gpu/drm/i915/intel_uc.h index ad140e2..57aef56 100644
> >>> --- a/drivers/gpu/drm/i915/intel_uc.h
> >>> +++ b/drivers/gpu/drm/i915/intel_uc.h
> >>> @@ -24,6 +24,9 @@
> >>> #ifndef _INTEL_UC_H_
> >>> #define _INTEL_UC_H_
> >>>
> >>> +#define HUC_STATUS2 _MMIO(0xD3B0)
> >>> +#define HUC_FW_VERIFIED (1<<7)
> >>> +
> >>> #include "intel_guc_fwif.h"
> >>> #include "i915_guc_reg.h"
> >>> #include "intel_ringbuffer.h"
> >>> @@ -174,6 +177,13 @@ struct intel_guc {
> >>> struct mutex send_mutex;
> >>> };
> >>>
> >>> +struct intel_huc {
> >>> + /* Generic uC firmware management */
> >>> + struct intel_uc_fw fw;
> >>> +
> >>> + /* HuC-specific additions */
> >>> +};
> >>> +
> >>> /* intel_uc.c */
> >>> void intel_uc_init_early(struct drm_i915_private *dev_priv); bool
> >>> intel_guc_recv(struct drm_i915_private *dev_priv, u32 *status); @@
> >>> -190,6 +200,9 @@ extern void intel_guc_fini(struct drm_i915_private
> >>> *dev_priv); extern const char *intel_uc_fw_status_repr(enum
> >>> intel_uc_fw_status status); extern int intel_guc_suspend(struct
> >>> drm_i915_private *dev_priv); extern int intel_guc_resume(struct
> >>> drm_i915_private *dev_priv);
> >>> +void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> >>> + struct intel_uc_fw *uc_fw);
> >>> +u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
> >>>
> >>> /* i915_guc_submission.c */
> >>> int i915_guc_submission_init(struct drm_i915_private *dev_priv); @@
> >>> -204,4 +217,9 @@ void i915_guc_register(struct drm_i915_private
> >>> *dev_priv); void i915_guc_unregister(struct drm_i915_private
> >>> *dev_priv); int i915_guc_log_control(struct drm_i915_private
> >>> *dev_priv, u64 control_val);
> >>>
> >>> +/* intel_huc_loader.c */
> >>> +void intel_huc_init(struct drm_i915_private *dev_priv); void
> >>> +intel_huc_fini(struct drm_device *dev); int intel_huc_load(struct
> >>> +drm_i915_private *dev_priv);
> >>> +
> >>> #endif
> >>> --
> >>> 2.7.4
> >>>
> >>> _______________________________________________
> >>> Intel-gfx mailing list
> >>> Intel-gfx at lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >_______________________________________________
> >Intel-gfx mailing list
> >Intel-gfx at lists.freedesktop.org
> >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Cheers,
Arek
More information about the Intel-gfx
mailing list