Re: [PATCH] drm/i915/guc: Move firmware size check out of generic code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 06 Oct 2017 12:43:10 +0200, Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> wrote:



On 10/6/2017 2:31 PM, Michal Wajdeczko wrote:
Checking GuC firmware size can be done in GuC specific code
right before DMA copy as it is unlikely to fail anyway.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_guc_loader.c | 14 +++++++++++---
  drivers/gpu/drm/i915/intel_uc_fw.c      |  8 --------
  2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c7a800a..f245aa5 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -198,6 +198,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
  	unsigned long offset;
  	struct sg_table *sg = vma->pages;
  	u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
+	u32 size = guc_fw->header_size + guc_fw->ucode_size;
  	int i, ret = 0;
    	/* where RSA signature starts */
@@ -208,9 +209,16 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
  	for (i = 0; i < UOS_RSA_SCRATCH_MAX_COUNT; i++)
  		I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
- /* The header plus uCode will be copied to WOPCM via DMA, excluding any
-	 * other components */
-	I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
+	/*
+ * The header plus uCode will be copied to WOPCM via DMA, excluding any
+	 * other components. Make sure that firmware fits there.
+	 */
+	if (unlikely(size > intel_guc_wopcm_size(dev_priv))) {
+		DRM_ERROR("GuC: Firmware is too large (%dB) to fit in WOPCM\n",
+			  size);
+		return -EFBIG;
Top level function is converting this to -EAGAIN and would be unnecessary to retry in that case.

Hmm, top level function may receive following errors:
-ETIMEDOUT
-ENOEXEC (signature verification failed)
timeout > 0
-EINTR
-EFAULT
-EINVAL
-ENOMEM
-EFBIG (not only from above case)
-EEXIST
-EIO
-ENOSPC
-E2BIG
...
and unconditionally converts all of them.
Note that there are other cases when retry will not help.

So maybe we should:
1) let guc_ucode_xfer[_dma] decide which failed step can be retried
   (by converting any transient error into -EAGAIN)
or,
2) let intel_uc_init_hw decide when to retry based on error codes
   (retry only on EAGAIN EINTR ETIMEDOUT)
or,
3) ignore any unlikely error duplications caused by retry
   (note that today we retry only due to WA)

Michal

+	}
+	I915_WRITE(DMA_COPY_SIZE, size);
    	/* Set the source address for the new blob */
  	offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 766b1cb..482115b 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -108,14 +108,6 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
  	 */
  	switch (uc_fw->type) {
  	case INTEL_UC_FW_TYPE_GUC:
-		/* Header and uCode will be loaded to WOPCM. Size of the two. */
-		size = uc_fw->header_size + uc_fw->ucode_size;
-
-		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
This comment was not correct. So removal makes sense.
-		if (size > intel_guc_wopcm_size(dev_priv)) {
-			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
-			goto fail;
-		}
  		uc_fw->major_ver_found = css->guc.sw_version >> 16;
  		uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF;
  		break;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux