On Thu, Oct 13, 2016 at 10:42:42AM -0700, Jeff McGee wrote: > On Mon, Oct 03, 2016 at 11:42:57AM -0700, 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. > > > > 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.c | 3 + > > 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 | 44 ++++++ > > drivers/gpu/drm/i915/intel_huc_loader.c | 268 ++++++++++++++++++++++++++++++++ > > 8 files changed, 327 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 e6fe004..6e99c51 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -53,6 +53,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.c b/drivers/gpu/drm/i915/i915_drv.c > > index 31b2b63..7af7bd6 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -613,6 +613,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > > * working irqs for e.g. gmbus and dp aux transfers. */ > > intel_modeset_init(dev); > > > > + intel_huc_init(dev); > > intel_guc_init(dev); > > > > ret = i915_gem_init(dev); > > @@ -638,6 +639,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > > cleanup_gem: > > i915_gem_fini(dev); > > cleanup_irq: > > + intel_huc_fini(dev); > > intel_guc_fini(dev); > > drm_irq_uninstall(dev); > > intel_teardown_gmbus(dev); > > @@ -1315,6 +1317,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); > > i915_gem_fini(dev); > > 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 e0cb71c..625aa92 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -55,6 +55,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" > > > > @@ -1766,6 +1767,7 @@ struct drm_i915_private { > > > > struct intel_gvt gvt; > > > > + struct intel_huc huc; > > struct intel_guc guc; > > > > struct intel_csr csr; > > @@ -2822,6 +2824,7 @@ struct drm_i915_cmd_table { > > #define HAS_GUC(dev) (INTEL_INFO(dev)->has_guc) > > #define HAS_GUC_UCODE(dev) (HAS_GUC(dev)) > > #define HAS_GUC_SCHED(dev) (HAS_GUC(dev)) > > +#define HAS_HUC_UCODE(dev) (HAS_GUC(dev)) > > > > #define HAS_RESOURCE_STREAMER(dev) (INTEL_INFO(dev)->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 812e4ca..af76ffe 100644 > > --- a/drivers/gpu/drm/i915/intel_guc.h > > +++ b/drivers/gpu/drm/i915/intel_guc.h > > @@ -160,6 +160,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 0b863a1..0d214b4 100644 > > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > > @@ -305,7 +305,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. > > @@ -330,7 +331,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; > > > > @@ -508,6 +509,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..946caa7 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_huc.h > > @@ -0,0 +1,44 @@ > > +/* > > + * Copyright © 2014 Intel Corporation > > Should this be 2016? > > > + * > > + * 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 */ > > +}; > > + > > +extern void intel_huc_init(struct drm_device *dev); > > +extern int intel_huc_load(struct drm_device *dev); > > +extern void intel_huc_auth(struct drm_device *dev); > > +extern void intel_huc_fini(struct drm_device *dev); These don't need extern. Please remove. Also intel_huc_auth comes in with later patch in the series so would be better to drop from here. > > + > > +#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..1aaff67 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_huc_loader.c > > @@ -0,0 +1,268 @@ > > +/* > > + * Copyright © 2014 Intel Corporation > > Should this be 2016? > > > + * > > + * 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: > > + * On supported platforms, i915's job is to load the firmware stored on the > > + * file system and assist with authentication. It is up to userspace to > > + * detect the presence of HuC support on a platform, on their own. > > + * For debugging, i915 provides a debugfs file, i915_huc_load_status_info > > + * which displays the firmware load status. > Above comment refers to features added in subsequent patches and is not > entirely accurate at that. So I would just remove this whole block. Less > commenting is better than misleading commenting. > > > + * > > + * The same firmware loader is used as the GuC. Firmware * binary is fetched by > > + * the loader asynchronously from the driver init process. However, the actual > Remove * from "Firmware * binary". Loading is not asynchronous so remove this > statement. > > > + * loading to HW is deferred until GEM initialization is done. > > + * > > + * Note that HuC firmware loading must be done before GuC loading. > > + */ > > + > > +#define I915_SKL_HUC_UCODE "i915/skl_huc_ver01_07_1398.bin" > > +MODULE_FIRMWARE(I915_SKL_HUC_UCODE); > Please update to use the new approach used by GuC for specifying > required version and constructing the file path. See intel_guc_loader.c. > HuC firmware versioning includes the build number after the minor so > some modification will be needed. > > > + > > +/** > > + * intel_huc_load_ucode() - DMA's the firmware > > + * @dev: the drm device > Fix name and parameter > > > + * > > + * 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); > > + > > + WARN_ON(!mutex_is_locked(&dev_priv->drm.struct_mutex)); > I don't see reason for this WARN_ON check. This function is called > in well defined paths that require locking way up the chain. > I think we can remove it. > > > + 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; > > +} > > + > > +/** > > + * 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_ucode_load() is called. > intel_huc_ucode_load() should be intel_huc_load(). > > > + */ > > +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 = 1; > > + huc_fw->minor_ver_wanted = 7; > > + } > > + > > + 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 > > + * 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) > > + drm_gem_object_unreference(&huc_fw->uc_fw_obj->base); > These are now replaced with 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; > > +} > > -- > > 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