>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Srivatsa, Anusha
>Sent: Monday, January 2, 2017 4:09 PM
>To: Wajdeczko, Michal <michal.wajdec...@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Alex Dai <yu....@intel.com>; Peter Antoine
><peter.anto...@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.sriva...@intel.com>
>>Cc: intel-gfx@lists.freedesktop.org; Alex Dai <yu....@intel.com>; Peter
>>Antoine <peter.anto...@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.ursu...@intel.com>
>>> Tested-by: Xiang Haihao <haihao.xi...@intel.com>
>>> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>
>>> Signed-off-by: Alex Dai <yu....@intel.com>
>>> Signed-off-by: Peter Antoine <peter.anto...@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?

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

Reply via email to