[PATCH] drm/i915/guc: Fix a fw content lost issue after it is evicted

* [PATCH] drm/i915/guc: Fix a fw content lost issue after it is evicted
@ 2015-11-06 21:55 yu.dai
  2015-11-06 22:07 ` Chris Wilson
  0 siblings, 1 reply; 18+ messages in thread
From: yu.dai @ 2015-11-06 21:55 UTC (permalink / raw)
  To: intel-gfx

From: Alex Dai <yu.dai@intel.com>

We keep a copy of GuC fw in a GEM obj. However its content is lost
if the GEM obj is evicted (igt/gem_evict_*). Therefore, the later
fw loading during GPU reset will fail.

Now we keep the copy in a memory block rather than a GEM objet.
During fw loading, we will wrap up a GEM obj with fw data for DMA
copy. The GEM obj will be freed DMA is done.

Signed-off-by: Alex Dai <yu.dai@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h        |  2 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 84 +++++++++++++++------------------
 2 files changed, 39 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 5ba5866..2714106 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -68,7 +68,7 @@ struct intel_guc_fw {
 	struct drm_device *		guc_dev;
 	const char *			guc_fw_path;
 	size_t				guc_fw_size;
-	struct drm_i915_gem_object *	guc_fw_obj;
+	uint32_t			*guc_fw_data;
 	enum intel_guc_fw_status	guc_fw_fetch_status;
 	enum intel_guc_fw_status	guc_fw_load_status;
 
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 550921f..ebf15e8 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -218,26 +218,45 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
 static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
-	struct drm_i915_gem_object *fw_obj = guc_fw->guc_fw_obj;
+	struct drm_i915_gem_object *fw_obj;
 	unsigned long offset;
-	struct sg_table *sg = fw_obj->pages;
-	u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
+	u32 status, *rsa;
 	int i, ret = 0;
 
 	/* where RSA signature starts */
-	offset = guc_fw->rsa_offset;
+	rsa = (u32 *)((u8 *)guc_fw->guc_fw_data + guc_fw->rsa_offset);
 
 	/* Copy RSA signature from the fw image to HW for verification */
-	sg_pcopy_to_buffer(sg->sgl, sg->nents, rsa, sizeof(rsa), offset);
 	for (i = 0; i < UOS_RSA_SCRATCH_MAX_COUNT; i++)
-		I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
+		I915_WRITE(UOS_RSA_SCRATCH(i), *(rsa + i));
+
+	/* Wrap up a gem obj filled with header + ucode */
+	fw_obj = i915_gem_object_create_from_data(dev_priv->dev,
+			(u8 *)guc_fw->guc_fw_data + guc_fw->header_offset,
+			guc_fw->header_size + guc_fw->ucode_size);
+	if (IS_ERR_OR_NULL(fw_obj)) {
+		ret = fw_obj ? PTR_ERR(fw_obj) : -ENOMEM;
+		return ret;
+	}
+
+	ret = i915_gem_object_set_to_gtt_domain(fw_obj, false);
+	if (ret) {
+		DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
+		return ret;
+	}
+
+	ret = i915_gem_obj_ggtt_pin(fw_obj, 0, 0);
+	if (ret) {
+		DRM_DEBUG_DRIVER("pin failed %d\n", ret);
+		return ret;
+	}
 
 	/* 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);
 
 	/* Set the source address for the new blob */
-	offset = i915_gem_obj_ggtt_offset(fw_obj) + guc_fw->header_offset;
+	offset = i915_gem_obj_ggtt_offset(fw_obj);
 	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
 	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
 
@@ -269,6 +288,10 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
 		ret = -ENOEXEC;
 	}
 
+	/* We can free it now that DMA has completed */
+	i915_gem_object_ggtt_unpin(fw_obj);
+	drm_gem_object_unreference(&fw_obj->base);
+
 	DRM_DEBUG_DRIVER("returning %d\n", ret);
 
 	return ret;
@@ -279,22 +302,9 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
  */
 static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
 {
-	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
 	struct drm_device *dev = dev_priv->dev;
 	int ret;
 
-	ret = i915_gem_object_set_to_gtt_domain(guc_fw->guc_fw_obj, false);
-	if (ret) {
-		DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
-		return ret;
-	}
-
-	ret = i915_gem_obj_ggtt_pin(guc_fw->guc_fw_obj, 0, 0);
-	if (ret) {
-		DRM_DEBUG_DRIVER("pin failed %d\n", ret);
-		return ret;
-	}
-
 	/* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
 	I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
 
@@ -337,12 +347,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
 
 	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_gem_object_ggtt_unpin(guc_fw->guc_fw_obj);
-
 	return ret;
 }
 
@@ -444,7 +448,6 @@ fail:
 
 static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
 {
-	struct drm_i915_gem_object *obj;
 	const struct firmware *fw;
 	struct guc_css_header *css;
 	size_t size;
@@ -530,34 +533,27 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
 			guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
 			guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
 
-	mutex_lock(&dev->struct_mutex);
-	obj = i915_gem_object_create_from_data(dev, fw->data, fw->size);
-	mutex_unlock(&dev->struct_mutex);
-	if (IS_ERR_OR_NULL(obj)) {
-		err = obj ? PTR_ERR(obj) : -ENOMEM;
+	guc_fw->guc_fw_data = kmalloc(fw->size, GFP_KERNEL);
+	if (guc_fw->guc_fw_data == NULL) {
+		err = -ENOMEM;
 		goto fail;
 	}
 
-	guc_fw->guc_fw_obj = obj;
+	memcpy(guc_fw->guc_fw_data, fw->data, fw->size);
 	guc_fw->guc_fw_size = fw->size;
 
-	DRM_DEBUG_DRIVER("GuC fw fetch status SUCCESS, obj %p\n",
-			guc_fw->guc_fw_obj);
+	DRM_DEBUG_DRIVER("GuC fw fetch status SUCCESS\n");
 
 	release_firmware(fw);
 	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_SUCCESS;
 	return;
 
 fail:
-	DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n",
-		err, fw, guc_fw->guc_fw_obj);
+	DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p\n", err, fw);
 	DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n",
 		  guc_fw->guc_fw_path, err);
 
-	obj = guc_fw->guc_fw_obj;
-	if (obj)
-		drm_gem_object_unreference(&obj->base);
-	guc_fw->guc_fw_obj = NULL;
+	guc_fw->guc_fw_data = NULL;
 
 	release_firmware(fw);		/* OK even if fw is NULL */
 	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_FAIL;
@@ -627,11 +623,7 @@ void intel_guc_ucode_fini(struct drm_device *dev)
 	direct_interrupts_to_host(dev_priv);
 	i915_guc_submission_fini(dev);
 
-	mutex_lock(&dev->struct_mutex);
-	if (guc_fw->guc_fw_obj)
-		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
-	guc_fw->guc_fw_obj = NULL;
-	mutex_unlock(&dev->struct_mutex);
+	kfree(guc_fw->guc_fw_data);
 
 	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
 }
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread