[9/9] drm/i915/uc: Unify uC firmware upload
diff mbox series

Message ID 20190722232048.9970-10-daniele.ceraolospurio@intel.com
State New
Headers show
Series
  • uC fw path unification + misc clean-up
Related show

Commit Message

Daniele Ceraolo Spurio July 22, 2019, 11:20 p.m. UTC
The way we load the firmwares is the same for both GuC and HuC, the only
difference is in the wopcm destination address and the dma flags, so we
easily can move the logic to a common function and pass in offset and
flags. The only other difference in the uplaod path are some the extra
steps that guc does before and after the xfer, but those don't require
the guc fw to be pinned in ggtt and can safely be performed before
calling the uc_upload function.

Note that this patch re-introduces the dma xfer wait for guc loading that
was removed with "drm/i915/guc: Propagate the fw xfer timeout". This is
not going to slow us down on a successful load (the dma has to complete
before fw init can start), but could slightly increase the timeout in case
of a fw init error.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 100 +++++-----------------
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c |  57 +-----------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  |  79 +++++++++++++----
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  |   4 +-
 4 files changed, 86 insertions(+), 154 deletions(-)

Comments

Chris Wilson July 23, 2019, 8:45 a.m. UTC | #1
Quoting Daniele Ceraolo Spurio (2019-07-23 00:20:48)
> The way we load the firmwares is the same for both GuC and HuC, the only
> difference is in the wopcm destination address and the dma flags, so we
> easily can move the logic to a common function and pass in offset and
> flags. The only other difference in the uplaod path are some the extra
> steps that guc does before and after the xfer, but those don't require
> the guc fw to be pinned in ggtt and can safely be performed before
> calling the uc_upload function.
> 
> Note that this patch re-introduces the dma xfer wait for guc loading that
> was removed with "drm/i915/guc: Propagate the fw xfer timeout". This is
> not going to slow us down on a successful load (the dma has to complete
> before fw init can start), but could slightly increase the timeout in case
> of a fw init error.

Should not be a problem if all is well. And if not, picking up on an
error earlier is beneficial.

> +static int uc_fw_xfer(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
> +                     u32 wopcm_offset, u32 dma_flags)
> +{
> +       struct intel_uncore *uncore = gt->uncore;
> +       u64 offset;
> +       int ret;
> +
> +       intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
> +
> +       /* Set the source address for the uCode */
> +       offset = uc_fw_ggtt_offset(uc_fw, gt->ggtt) + uc_fw->header_offset;
> +       GEM_BUG_ON(upper_32_bits(offset) & 0xFFFF0000);
> +       intel_uncore_write(uncore, DMA_ADDR_0_LOW, lower_32_bits(offset));

You've taken an explicit fw, so these can all be _fw.

> +       intel_uncore_write(uncore, DMA_ADDR_0_HIGH, upper_32_bits(offset));
> +
> +       /* Set the DMA destination */
> +       intel_uncore_write(uncore, DMA_ADDR_1_LOW, wopcm_offset);
> +       intel_uncore_write(uncore, DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
> +
> +       /*
> +        * Set the trasfer size. The header plus uCode will be copied to WOPCM
> +        * via DMA, excluding any other components
> +        */
> +       intel_uncore_write(uncore, DMA_COPY_SIZE,
> +                          uc_fw->header_size + uc_fw->ucode_size);
> +
> +       /* Start the DMA */
> +       intel_uncore_write(uncore, DMA_CTRL,
> +                          _MASKED_BIT_ENABLE(dma_flags | START_DMA));
> +
> +       /* Wait for DMA to finish */
> +       ret = intel_wait_for_register_fw(uncore, DMA_CTRL, START_DMA, 0, 100);
> +       if (ret)
> +               DRM_ERROR("DMA for %s fw failed, err=%d\n",
> +                         intel_uc_fw_type_repr(uc_fw->type), ret);
> +
> +       /* Disable the bits once DMA is over */
> +       intel_uncore_write(uncore, DMA_CTRL, _MASKED_BIT_DISABLE(dma_flags));
> +
> +       intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
> +
> +       return ret;
> +}

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index c9bd55e23eec..a1e071958822 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -113,13 +113,6 @@  static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
 		intel_uncore_write(uncore, UOS_RSA_SCRATCH(i), rsa[i]);
 }
 
-static bool guc_xfer_completed(struct intel_uncore *uncore, u32 *status)
-{
-	/* Did we complete the xfer? */
-	*status = intel_uncore_read(uncore, DMA_CTRL);
-	return !(*status & START_DMA);
-}
-
 /*
  * Read the GuC status register (GUC_STATUS) and store it in the
  * specified location; then return a boolean indicating whether
@@ -166,65 +159,27 @@  static int guc_wait_ucode(struct intel_uncore *uncore)
 		ret = -ENXIO;
 	}
 
-	if (ret == 0 && !guc_xfer_completed(uncore, &status)) {
-		DRM_ERROR("GuC is ready, but the xfer %08x is incomplete\n",
-			  status);
-		ret = -ENXIO;
-	}
-
 	return ret;
 }
 
-/*
- * Transfer the firmware image to RAM for execution by the microcontroller.
+/**
+ * intel_guc_fw_upload() - load GuC uCode to device
+ * @guc: intel_guc structure
  *
- * Architecturally, the DMA engine is bidirectional, and can potentially even
- * transfer between GTT locations. This functionality is left out of the API
- * for now as there is no need for it.
- */
-static int guc_xfer_ucode(struct intel_uc_fw *guc_fw,
-			  struct intel_gt *gt)
-{
-	struct intel_uncore *uncore = gt->uncore;
-	unsigned long offset;
-
-	/*
-	 * The header plus uCode will be copied to WOPCM via DMA, excluding any
-	 * other components
-	 */
-	intel_uncore_write(uncore, DMA_COPY_SIZE,
-			   guc_fw->header_size + guc_fw->ucode_size);
-
-	/* Set the source address for the new blob */
-	offset = intel_uc_fw_ggtt_offset(guc_fw, gt->ggtt) + guc_fw->header_offset;
-	intel_uncore_write(uncore, DMA_ADDR_0_LOW, lower_32_bits(offset));
-	intel_uncore_write(uncore, DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
-
-	/*
-	 * Set the DMA destination. Current uCode expects the code to be
-	 * loaded at 8k; locations below this are used for the stack.
-	 */
-	intel_uncore_write(uncore, DMA_ADDR_1_LOW, 0x2000);
-	intel_uncore_write(uncore, DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
-
-	/* Finally start the DMA */
-	intel_uncore_write(uncore, DMA_CTRL,
-			   _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
-
-	return guc_wait_ucode(uncore);
-}
-/*
- * Load the GuC firmware blob into the MinuteIA.
+ * Called from intel_uc_init_hw() during driver load, resume from sleep and
+ * after a GPU reset.
+ *
+ * The firmware image should have already been fetched into memory, so only
+ * check that fetch succeeded, and then transfer the image to the h/w.
+ *
+ * Return:	non-zero code on error
  */
-static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct intel_gt *gt)
+int intel_guc_fw_upload(struct intel_guc *guc)
 {
+	struct intel_gt *gt = guc_to_gt(guc);
 	struct intel_uncore *uncore = gt->uncore;
 	int ret;
 
-	GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
-
-	intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
-
 	guc_prepare_xfer(uncore);
 
 	/*
@@ -232,28 +187,15 @@  static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct intel_gt *gt)
 	 * by the DMA engine in one operation, whereas the RSA signature is
 	 * loaded via MMIO.
 	 */
-	guc_xfer_rsa(guc_fw, uncore);
+	guc_xfer_rsa(&guc->fw, uncore);
 
-	ret = guc_xfer_ucode(guc_fw, gt);
-
-	intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
-
-	return ret;
-}
+	/*
+	 * Current uCode expects the code to be loaded at 8k; locations below
+	 * this are used for the stack.
+	 */
+	ret = intel_uc_fw_upload(&guc->fw, gt, 0x2000, UOS_MOVE);
+	if (ret)
+		return ret;
 
-/**
- * intel_guc_fw_upload() - load GuC uCode to device
- * @guc: intel_guc structure
- *
- * Called from intel_uc_init_hw() during driver load, resume from sleep and
- * after a GPU reset.
- *
- * The firmware image should have already been fetched into memory, so only
- * check that fetch succeeded, and then transfer the image to the h/w.
- *
- * Return:	non-zero code on error
- */
-int intel_guc_fw_upload(struct intel_guc *guc)
-{
-	return intel_uc_fw_upload(&guc->fw, guc_to_gt(guc), guc_fw_xfer);
+	return guc_wait_ucode(uncore);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 41e032149f7e..44d2b7e7e845 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -62,60 +62,6 @@  void intel_huc_fw_init_early(struct intel_huc *huc)
 			   huc_fw_blobs, ARRAY_SIZE(huc_fw_blobs));
 }
 
-/**
- * huc_fw_xfer() - DMA's the firmware
- * @huc_fw: the firmware descriptor
- *
- * Transfer the firmware image to RAM for execution by the microcontroller.
- *
- * Return: 0 on success, non-zero on failure
- */
-static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct intel_gt *gt)
-{
-	struct intel_uncore *uncore = gt->uncore;
-	unsigned long offset = 0;
-	u32 size;
-	int ret;
-
-	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
-
-	intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
-
-	/* Set the source address for the uCode */
-	offset = intel_uc_fw_ggtt_offset(huc_fw, gt->ggtt) +
-		 huc_fw->header_offset;
-	intel_uncore_write(uncore, DMA_ADDR_0_LOW,
-			   lower_32_bits(offset));
-	intel_uncore_write(uncore, 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.
-	 */
-	intel_uncore_write(uncore, DMA_ADDR_1_LOW, 0);
-	intel_uncore_write(uncore, DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
-
-	size = huc_fw->header_size + huc_fw->ucode_size;
-	intel_uncore_write(uncore, DMA_COPY_SIZE, size);
-
-	/* Start the DMA */
-	intel_uncore_write(uncore, DMA_CTRL,
-			   _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
-
-	/* Wait for DMA to finish */
-	ret = intel_wait_for_register_fw(uncore, DMA_CTRL, START_DMA, 0, 100);
-
-	DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
-
-	/* Disable the bits once DMA is over */
-	intel_uncore_write(uncore, DMA_CTRL, _MASKED_BIT_DISABLE(HUC_UKERNEL));
-
-	intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
-
-	return ret;
-}
-
 /**
  * intel_huc_fw_upload() - load HuC uCode to device
  * @huc: intel_huc structure
@@ -130,5 +76,6 @@  static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct intel_gt *gt)
  */
 int intel_huc_fw_upload(struct intel_huc *huc)
 {
-	return intel_uc_fw_upload(&huc->fw, huc_to_gt(huc), huc_fw_xfer);
+	/* HW doesn't look at destination address for HuC, so set it to 0 */
+	return intel_uc_fw_upload(&huc->fw, huc_to_gt(huc), 0, HUC_UKERNEL);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index bb6fb64c3936..5a6e239498d2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -227,13 +227,24 @@  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	release_firmware(fw);		/* OK even if fw is NULL */
 }
 
+static u32 uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
+{
+	struct drm_mm_node *node = &ggtt->uc_fw;
+
+	GEM_BUG_ON(!node->allocated);
+	GEM_BUG_ON(upper_32_bits(node->start));
+	GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
+
+	return lower_32_bits(node->start);
+}
+
 static void intel_uc_fw_ggtt_bind(struct intel_uc_fw *uc_fw,
 				  struct intel_gt *gt)
 {
 	struct drm_i915_gem_object *obj = uc_fw->obj;
 	struct i915_ggtt *ggtt = gt->ggtt;
 	struct i915_vma dummy = {
-		.node.start = intel_uc_fw_ggtt_offset(uc_fw, ggtt),
+		.node.start = uc_fw_ggtt_offset(uc_fw, ggtt),
 		.node.size = obj->base.size,
 		.pages = obj->mm.pages,
 		.vm = &ggtt->vm,
@@ -253,23 +264,68 @@  static void intel_uc_fw_ggtt_unbind(struct intel_uc_fw *uc_fw,
 {
 	struct drm_i915_gem_object *obj = uc_fw->obj;
 	struct i915_ggtt *ggtt = gt->ggtt;
-	u64 start = intel_uc_fw_ggtt_offset(uc_fw, ggtt);
+	u64 start = uc_fw_ggtt_offset(uc_fw, ggtt);
 
 	ggtt->vm.clear_range(&ggtt->vm, start, obj->base.size);
 }
 
+static int uc_fw_xfer(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
+		      u32 wopcm_offset, u32 dma_flags)
+{
+	struct intel_uncore *uncore = gt->uncore;
+	u64 offset;
+	int ret;
+
+	intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
+
+	/* Set the source address for the uCode */
+	offset = uc_fw_ggtt_offset(uc_fw, gt->ggtt) + uc_fw->header_offset;
+	GEM_BUG_ON(upper_32_bits(offset) & 0xFFFF0000);
+	intel_uncore_write(uncore, DMA_ADDR_0_LOW, lower_32_bits(offset));
+	intel_uncore_write(uncore, DMA_ADDR_0_HIGH, upper_32_bits(offset));
+
+	/* Set the DMA destination */
+	intel_uncore_write(uncore, DMA_ADDR_1_LOW, wopcm_offset);
+	intel_uncore_write(uncore, DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
+
+	/*
+	 * Set the trasfer size. The header plus uCode will be copied to WOPCM
+	 * via DMA, excluding any other components
+	 */
+	intel_uncore_write(uncore, DMA_COPY_SIZE,
+			   uc_fw->header_size + uc_fw->ucode_size);
+
+	/* Start the DMA */
+	intel_uncore_write(uncore, DMA_CTRL,
+			   _MASKED_BIT_ENABLE(dma_flags | START_DMA));
+
+	/* Wait for DMA to finish */
+	ret = intel_wait_for_register_fw(uncore, DMA_CTRL, START_DMA, 0, 100);
+	if (ret)
+		DRM_ERROR("DMA for %s fw failed, err=%d\n",
+			  intel_uc_fw_type_repr(uc_fw->type), ret);
+
+	/* Disable the bits once DMA is over */
+	intel_uncore_write(uncore, DMA_CTRL, _MASKED_BIT_DISABLE(dma_flags));
+
+	intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
+
+	return ret;
+}
+
 /**
  * intel_uc_fw_upload - load uC firmware using custom loader
  * @uc_fw: uC firmware
  * @gt: the intel_gt structure
- * @xfer: custom uC firmware loader function
+ * @wopcm_offset: destination offset in wopcm
+ * @dma_flags: flags for flags for dma ctrl
  *
- * Loads uC firmware using custom loader and updates internal flags.
+ * Loads uC firmware and updates internal flags.
  *
  * Return: 0 on success, non-zero on failure.
  */
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
-		       int (*xfer)(struct intel_uc_fw *uc_fw, struct intel_gt *gt))
+		       u32 wopcm_offset, u32 dma_flags)
 {
 	int err;
 
@@ -284,7 +340,7 @@  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
 
 	/* Call custom loader */
 	intel_uc_fw_ggtt_bind(uc_fw, gt);
-	err = xfer(uc_fw, gt);
+	err = uc_fw_xfer(uc_fw, gt, wopcm_offset, dma_flags);
 	intel_uc_fw_ggtt_unbind(uc_fw, gt);
 	if (err)
 		goto fail;
@@ -337,17 +393,6 @@  void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
 	i915_gem_object_unpin_pages(uc_fw->obj);
 }
 
-u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
-{
-	struct drm_mm_node *node = &ggtt->uc_fw;
-
-	GEM_BUG_ON(!node->allocated);
-	GEM_BUG_ON(upper_32_bits(node->start));
-	GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
-
-	return lower_32_bits(node->start);
-}
-
 /**
  * intel_uc_fw_cleanup_fetch - cleanup uC firmware
  *
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 4043da69db60..d5db8d12a2c2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -32,7 +32,6 @@ 
 struct drm_printer;
 struct drm_i915_private;
 struct intel_gt;
-struct i915_ggtt;
 
 /* Home of GuC, HuC and DMC firmwares */
 #define INTEL_UC_FIRMWARE_URL "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915"
@@ -197,10 +196,9 @@  void intel_uc_fw_fetch(struct drm_i915_private *i915,
 		       struct intel_uc_fw *uc_fw);
 void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
-		       int (*xfer)(struct intel_uc_fw *uc_fw, struct intel_gt *gt));
+		       u32 wopcm_offset, u32 dma_flags);
 int intel_uc_fw_init(struct intel_uc_fw *uc_fw);
 void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
-u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt);
 void intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len);
 void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p);