On Tue, Jan 03, 2017 at 06:59:11PM +0000, Srivatsa, Anusha wrote: > > > >-----Original Message----- > >From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > >Srivatsa, Anusha > >Sent: Monday, January 2, 2017 4:09 PM > >To: Wajdeczko, Michal <Michal.Wajdeczko@xxxxxxxxx> > >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Alex Dai <yu.dai@xxxxxxxxx>; Peter Antoine > ><peter.antoine@xxxxxxxxx> > >Subject: Re: [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@xxxxxxxxx> > >>Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Alex Dai <yu.dai@xxxxxxxxx>; Peter > >>Antoine <peter.antoine@xxxxxxxxx> > >>Subject: Re: [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@xxxxxxxxx> > >>> Tested-by: Xiang Haihao <haihao.xiang@xxxxxxxxx> > >>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx> > >>> Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx> > >>> Signed-off-by: Peter Antoine <peter.antoine@xxxxxxxxx> > >>> --- > >>> 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@xxxxxxxxxxxxxxxxxxxxx > >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >_______________________________________________ > >Intel-gfx mailing list > >Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Cheers, Arek _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
