[PATCH v2 1/4] drm/i915: Always do WOPCM partitioning based on real firmware sizes

Jackie Li yaodong.li at intel.com
Thu Mar 22 20:52:05 UTC 2018


By using enable_guc modparam, we will run into problems when first loading
module with only GuC FW then trying to reload module with both GuC and HuC
FW because WOPCM registers were locked without considering the HuC FW size.
Since we need both GuC and HuC FW sizes to determine the final layout of
the WOPCM, we need to know both GuC and HuC firmware sizes if the firmwares
are present on a specific platform and we need enable/disable HuC FW
loading dynamically with enable_guc modparam.

This patch splits uC firmware fetching into two stages. First stage is to
fetch the firmware image and verify the firmware header. uC firmware will
be marked as verified and this will make FW info available for following
WOPCM layout calculation. The second stage is to create and copy the FW
data into a GEM object which only available when GuC/HuC is enabled by
enable_guc. This will guarantee that the WOPCM layout will be always be
calculated correctly without any assumption to the GuC and HuC firmware
sizes.

Signed-off-by: Jackie Li <yaodong.li at intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
Cc: Michał Winiarski <michal.winiarski at intel.com>
Cc: John Spotswood <john.a.spotswood at intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c    | 10 +++-------
 drivers/gpu/drm/i915/intel_uc_fw.c | 30 +++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_uc_fw.h |  7 +++++--
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 34f8a2c..5946aa3 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -175,10 +175,8 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 	if (!USES_GUC(dev_priv))
 		return;
 
-	if (USES_HUC(dev_priv))
-		intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
-
-	intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw);
+	intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw, USES_HUC(dev_priv));
+	intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw, true);
 }
 
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
@@ -187,9 +185,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
 		return;
 
 	intel_uc_fw_fini(&dev_priv->guc.fw);
-
-	if (USES_HUC(dev_priv))
-		intel_uc_fw_fini(&dev_priv->huc.fw);
+	intel_uc_fw_fini(&dev_priv->huc.fw);
 
 	guc_free_load_err_log(&dev_priv->guc);
 }
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 30c7324..5f58903 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -33,11 +33,12 @@
  *
  * @dev_priv: device private
  * @uc_fw: uC firmware
+ * @copy: whether should fetch uC firmware into GEM obj
  *
- * Fetch uC firmware into GEM obj.
+ * Fetch and verify uC firmware and copy data into GEM obj if @copy is true.
  */
 void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
-		       struct intel_uc_fw *uc_fw)
+		       struct intel_uc_fw *uc_fw, bool copy)
 {
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 	struct drm_i915_gem_object *obj;
@@ -154,17 +155,24 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 		goto fail;
 	}
 
-	obj = i915_gem_object_create_from_data(dev_priv, fw->data, fw->size);
-	if (IS_ERR(obj)) {
-		err = PTR_ERR(obj);
-		DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
-				 intel_uc_fw_type_repr(uc_fw->type), err);
-		goto fail;
+	uc_fw->size = fw->size;
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_VERIFIED;
+
+	if (copy) {
+		obj = i915_gem_object_create_from_data(dev_priv, fw->data,
+						       fw->size);
+		if (IS_ERR(obj)) {
+			err = PTR_ERR(obj);
+			DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
+					 intel_uc_fw_type_repr(uc_fw->type),
+					 err);
+			goto fail;
+		}
+
+		uc_fw->obj = obj;
+		uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
 	}
 
-	uc_fw->obj = obj;
-	uc_fw->size = fw->size;
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
 	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
 			 intel_uc_fw_type_repr(uc_fw->type),
 			 intel_uc_fw_status_repr(uc_fw->fetch_status));
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index dc33b12..3a6147a 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -36,6 +36,7 @@ enum intel_uc_fw_status {
 	INTEL_UC_FIRMWARE_FAIL = -1,
 	INTEL_UC_FIRMWARE_NONE = 0,
 	INTEL_UC_FIRMWARE_PENDING,
+	INTEL_UC_FIRMWARE_VERIFIED,
 	INTEL_UC_FIRMWARE_SUCCESS
 };
 
@@ -84,6 +85,8 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 		return "NONE";
 	case INTEL_UC_FIRMWARE_PENDING:
 		return "PENDING";
+	case INTEL_UC_FIRMWARE_VERIFIED:
+		return "VERIFIED";
 	case INTEL_UC_FIRMWARE_SUCCESS:
 		return "SUCCESS";
 	}
@@ -131,14 +134,14 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
  */
 static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
 {
-	if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (uc_fw->fetch_status < INTEL_UC_FIRMWARE_VERIFIED)
 		return 0;
 
 	return uc_fw->header_size + uc_fw->ucode_size;
 }
 
 void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
-		       struct intel_uc_fw *uc_fw);
+		       struct intel_uc_fw *uc_fw, bool copy);
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 		       int (*xfer)(struct intel_uc_fw *uc_fw,
 				   struct i915_vma *vma));
-- 
2.7.4



More information about the Intel-gfx-trybot mailing list