On Tue, Nov 15, 2016 at 10:26:46AM +0000, Tvrtko Ursulin wrote: > > 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? > VPG Media team that provides HuC firmware is using this format. It seems that build number really is a release minor. We are seeing new firmware from them with bug fixes where only build number is incremented. We can push back on this approach if it is a concern. -Jeff > >+ > >+#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). > > >+ > >+ 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
