>-----Original Message----- >From: Tvrtko Ursulin [mailto:tvrtko.ursulin@xxxxxxxxxxxxxxx] >Sent: Tuesday, November 15, 2016 2:27 AM >To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>; intel- >gfx@xxxxxxxxxxxxxxxxxxxxx >Cc: Alex Dai <yu.dai@xxxxxxxxx>; Peter Antoine <peter.antoine@xxxxxxxxx> >Subject: Re: [PATCH-v11] drm/i915/huc: Add HuC fw loading support > > >On 15/11/2016 00:39, Anusha Srivatsa wrote: >> From: Peter Antoine <peter.antoine@xxxxxxxxx> >> >> 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. >> >> 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> >> Reviewed-by: Dave Gordon <david.s.gordon@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/Makefile | 1 + >> drivers/gpu/drm/i915/i915_drv.h | 3 + >> 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 | 6 +- >> drivers/gpu/drm/i915/intel_huc.h | 42 +++++ >> drivers/gpu/drm/i915/intel_huc_loader.c | 269 >> ++++++++++++++++++++++++++++++++ >> 7 files changed, 323 insertions(+), 2 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 3dea46a..cfea925 100644 >> --- a/drivers/gpu/drm/i915/Makefile >> +++ b/drivers/gpu/drm/i915/Makefile >> @@ -56,6 +56,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_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h index c4a14de..3a6c412 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -56,6 +56,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" >> >> @@ -1791,6 +1792,7 @@ struct drm_i915_private { >> >> struct intel_gvt *gvt; >> >> + struct intel_huc huc; >> struct intel_guc guc; >> >> struct intel_csr csr; >> @@ -2607,6 +2609,7 @@ struct drm_i915_cmd_table { >> #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) (HAS_GUC(dev)) > >Parameter name should be dev_priv. > >> #define HAS_RESOURCE_STREAMER(dev_priv) >> ((dev_priv)->info.has_resource_streamer) >> >> diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h >> b/drivers/gpu/drm/i915/i915_guc_reg.h >> index a47e1e4..64e942a 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 46990a0..338e803 100644 >> --- a/drivers/gpu/drm/i915/intel_guc.h >> +++ b/drivers/gpu/drm/i915/intel_guc.h >> @@ -184,6 +184,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_i915_private *dev_priv); >> >> /* 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 70b965d..7f3fdb0 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_loader.c >> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c >> @@ -309,7 +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. >> @@ -334,7 +335,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 guc_wopcm_size(struct drm_i915_private *dev_priv) >> { >> u32 wopcm_size = GUC_WOPCM_TOP; >> >> @@ -512,6 +513,7 @@ int intel_guc_setup(struct drm_device *dev) >> if (err) >> goto fail; >> >> + intel_huc_load(dev); >> err = guc_ucode_xfer(dev_priv); >> if (!err) >> break; >> diff --git a/drivers/gpu/drm/i915/intel_huc.h >> b/drivers/gpu/drm/i915/intel_huc.h >> new file mode 100644 >> index 0000000..3ce0299 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_huc.h >> @@ -0,0 +1,42 @@ >> +/* >> + * Copyright © 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. >> + * >> + */ >> +#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 */ >> +}; >> + >> +void intel_huc_init(struct drm_device *dev); void >> +intel_huc_fini(struct drm_device *dev); int intel_huc_load(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..ce50306 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c >> @@ -0,0 +1,269 @@ >> +/* >> + * Copyright © 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_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: >> + * 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_FW_MAJOR 01 >> +#define SKL_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" > >Apologies if I missed it, but was it ever discussed why do we need the build >number in the filename? More so, for the driver to know about it when it >otherwise does not use it? > >> + >> +#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_FW_MAJOR, \ >> + SKL_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 >> + * 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; >> + 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, 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; >> +} > >It may be possible to extract some commonality between this and >guc_ucode_xfer(_dma), but it can be done as a followup. > >> + >> +/** >> + * 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_load() is called. >> + */ >> +void intel_huc_init(struct drm_device *dev) { >> + struct drm_i915_private *dev_priv = to_i915(dev); >> + 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_priv)) >> + return; >> + >> + if (IS_SKYLAKE(dev_priv)) { >> + fw_path = I915_SKL_HUC_UCODE; >> + huc_fw->major_ver_wanted = SKL_FW_MAJOR; >> + huc_fw->minor_ver_wanted = SKL_FW_MINOR; >> + } > >Why not "else return" instead of the fw_path check below? (Could remove >fw_path = NULL initializer then). Yes. I agree. Will remove that block. >> + >> + 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 > >intel_huc_ucode_init exists anywhere? > >> + * 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 = to_i915(dev); >> + 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; >> + } >> + >> + 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 = to_i915(dev); >> + struct intel_uc_fw *huc_fw = &dev_priv->huc.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 = UC_FIRMWARE_NONE; } >> > >Regards, > >Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
