From 8a2e98098ff48ed1a9abec3159e630b3a8c18f64 Mon Sep 17 00:00:00 2001
From: Dave Gordon <david.s.gordon@intel.com>
Date: Tue, 28 Jun 2016 13:09:54 +0100
Subject: [PATCH v3] Tweaks to GuC/HuC loader code
Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ

1. Add uc_name to intel_uc_fw structure, so we can use the appropriate
name ("GuC" or "HuC") everywhere (obviously, it should match the fw_type
field).

2. Change all messages in intel_uc_fw_fetch() to use the uc_name.

3. Validate fw_type at the beginning to intel_uc_fw_fetch() rather than
midway. Is there a firmware type in the CSS header -- if so we should
use that and the relevant definitions.

4. Refactor printing the firmware fetch/load status into a macro.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h        |  3 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 84 +++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_huc_loader.c |  3 +-
 3 files changed, 57 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index c7b2745..c04aef8 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -106,9 +106,10 @@ enum intel_uc_fw_status {
  */
 struct intel_uc_fw {
 	struct drm_device *uc_dev;
+	const char *uc_name;
 	const char *uc_fw_path;
-	size_t uc_fw_size;
 	struct drm_i915_gem_object *uc_fw_obj;
+	size_t uc_fw_size;
 	enum intel_uc_fw_status fetch_status;
 	enum intel_uc_fw_status load_status;
 
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c08b81a..b8c0df7 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -68,6 +68,15 @@
 #define I915_KBL_GUC_UCODE "i915/kbl_guc_ver9_14.bin"
 MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
 
+#define	DEBUG_UC_FW_STATUS(uc_fw)					\
+	{								\
+		DRM_DEBUG_DRIVER("%s fw status: path %s, fetch %s, load %s\n",	\
+			uc_fw->uc_name,					\
+			uc_fw->uc_fw_path,				\
+			intel_uc_fw_status_repr(uc_fw->fetch_status),	\
+			intel_uc_fw_status_repr(uc_fw->load_status));	\
+	}
+
 /* User-friendly representation of an enum */
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 {
@@ -156,7 +165,7 @@ static u32 get_core_family(struct drm_i915_private *dev_priv)
 		return GFXCORE_FAMILY_GEN9;
 
 	default:
-		DRM_ERROR("GUC: unsupported core family\n");
+		DRM_ERROR("GuC: unsupported core family\n");
 		return GFXCORE_FAMILY_UNKNOWN;
 	}
 }
@@ -421,10 +430,7 @@ int intel_guc_setup(struct drm_device *dev)
 	const char *fw_path = guc_fw->uc_fw_path;
 	int retries, ret, err;
 
-	DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
-		fw_path,
-		intel_uc_fw_status_repr(guc_fw->fetch_status),
-		intel_uc_fw_status_repr(guc_fw->load_status));
+	DEBUG_UC_FW_STATUS(guc_fw);
 
 	/* Loading forbidden, or no firmware to load? */
 	if (!i915.enable_guc_loading) {
@@ -454,9 +460,7 @@ int intel_guc_setup(struct drm_device *dev)
 
 	guc_fw->load_status = UC_FIRMWARE_PENDING;
 
-	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
-		intel_uc_fw_status_repr(guc_fw->fetch_status),
-		intel_uc_fw_status_repr(guc_fw->load_status));
+	DEBUG_UC_FW_STATUS(guc_fw);
 
 	err = i915_guc_submission_init(dev_priv);
 	if (err)
@@ -493,9 +497,7 @@ int intel_guc_setup(struct drm_device *dev)
 
 	guc_fw->load_status = UC_FIRMWARE_SUCCESS;
 
-	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
-		intel_uc_fw_status_repr(guc_fw->fetch_status),
-		intel_uc_fw_status_repr(guc_fw->load_status));
+	DEBUG_UC_FW_STATUS(guc_fw);
 
 	intel_huc_auth(dev);
 
@@ -563,8 +565,20 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
 	size_t size;
 	int err;
 
-	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
-		intel_uc_fw_status_repr(uc_fw->fetch_status));
+	/* Validate fw_type first of all, so we can name the uC */
+	switch (uc_fw->fw_type) {
+	case UC_FW_TYPE_GUC:
+	case UC_FW_TYPE_HUC:
+		break;
+
+	default:
+		DRM_ERROR("Unknown firmware type %d\n", uc_fw->fw_type);
+		err = -ENOEXEC;
+		goto fail;
+	}
+
+	DRM_DEBUG_DRIVER("%s fw fetch status %s before requesting firmware\n",
+		uc_fw->uc_name, intel_uc_fw_status_repr(uc_fw->fetch_status));
 
 	err = request_firmware(&fw, uc_fw->uc_fw_path, &dev->pdev->dev);
 	if (err)
@@ -572,12 +586,12 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
 	if (!fw)
 		goto fail;
 
-	DRM_DEBUG_DRIVER("fetch GuC fw from %s succeeded, fw %p\n",
-		uc_fw->uc_fw_path, fw);
+	DRM_DEBUG_DRIVER("%s fw fetch from %s succeeded, fw %p\n",
+		uc_fw->uc_name, uc_fw->uc_fw_path, fw);
 
 	/* Check the size of the blob before examining buffer contents */
 	if (fw->size < sizeof(struct uc_css_header)) {
-		DRM_ERROR("Firmware header is missing\n");
+		DRM_ERROR("%s firmware header is missing\n", uc_fw->uc_name);
 		goto fail;
 	}
 
@@ -589,7 +603,8 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
 		css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
 
 	if (uc_fw->header_size != sizeof(struct uc_css_header)) {
-		DRM_ERROR("CSS header definition mismatch\n");
+		DRM_ERROR("%s firmware CSS header definition mismatch\n",
+			uc_fw->uc_name);
 		goto fail;
 	}
 
@@ -599,7 +614,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
 
 	/* now RSA */
 	if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
-		DRM_ERROR("RSA key size is bad\n");
+		DRM_ERROR("%s firmware RSA key size is bad\n", uc_fw->uc_name);
 		goto fail;
 	}
 	uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size;
@@ -608,7 +623,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
 	/* At least, it should have header, uCode and RSA. Size of all three. */
 	size = uc_fw->header_size + uc_fw->ucode_size + uc_fw->rsa_size;
 	if (fw->size < size) {
-		DRM_ERROR("Missing firmware components\n");
+		DRM_ERROR("%s firmware missing components\n", uc_fw->uc_name);
 		goto fail;
 	}
 
@@ -625,35 +640,40 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
 
 		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
 		if (size > guc_wopcm_size(to_i915(dev))) {
-			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
+			DRM_ERROR("%s firmware is too large to fit in WOPCM\n",
+				uc_fw->uc_name);
 			goto fail;
 		}
 
 		uc_fw->major_ver_found = css->guc_sw_version >> 16;
 		uc_fw->minor_ver_found = css->guc_sw_version & 0xFFFF;
 		break;
+
 	case UC_FW_TYPE_HUC:
 		uc_fw->major_ver_found = css->huc_sw_version >> 16;
 		uc_fw->minor_ver_found = css->huc_sw_version & 0xFFFF;
 		break;
+
 	default:
-		DRM_ERROR("Unknown firmware type %d\n", uc_fw->fw_type);
+		/*NOTREACHED*/
 		err = -ENOEXEC;
 		goto fail;
 	}
 
 	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
 	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-		DRM_ERROR("GuC firmware version %d.%d, required %d.%d\n",
+		DRM_ERROR("Found %s firmware version %d.%d, required version %d.%d\n",
+			uc_fw->uc_name,
 			uc_fw->major_ver_found, uc_fw->minor_ver_found,
 			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
 		err = -ENOEXEC;
 		goto fail;
 	}
 
-	DRM_DEBUG_DRIVER("firmware version %d.%d OK (minimum %d.%d)\n",
-			uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
+	DRM_DEBUG_DRIVER("%s firmware version %d.%d OK (minimum %d.%d)\n",
+		uc_fw->uc_name,
+		uc_fw->major_ver_found, uc_fw->minor_ver_found,
+		uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
 
 	mutex_lock(&dev->struct_mutex);
 	obj = i915_gem_object_create_from_data(dev, fw->data, fw->size);
@@ -666,18 +686,18 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
 	uc_fw->uc_fw_obj = obj;
 	uc_fw->uc_fw_size = fw->size;
 
-	DRM_DEBUG_DRIVER("GuC fw fetch status SUCCESS, obj %p\n",
-			uc_fw->uc_fw_obj);
+	DRM_DEBUG_DRIVER("%s fw fetch status SUCCESS, obj %p\n",
+		uc_fw->uc_name, uc_fw->uc_fw_obj);
 
 	release_firmware(fw);
 	uc_fw->fetch_status = UC_FIRMWARE_SUCCESS;
 	return;
 
 fail:
-	DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n",
-		err, fw, uc_fw->uc_fw_obj);
-	DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n",
-		  uc_fw->uc_fw_path, err);
+	DRM_DEBUG_DRIVER("%s fw fetch status FAIL; err %d, fw %p, obj %p\n",
+		uc_fw->uc_name, err, fw, uc_fw->uc_fw_obj);
+	DRM_ERROR("Failed to fetch %s firmware from %s (error %d)\n",
+		uc_fw->uc_name, uc_fw->uc_fw_path, err);
 
 	mutex_lock(&dev->struct_mutex);
 	obj = uc_fw->uc_fw_obj;
@@ -730,6 +750,8 @@ void intel_guc_init(struct drm_device *dev)
 	}
 
 	guc_fw->uc_dev = dev;
+	guc_fw->uc_name = "GuC";
+	guc_fw->fw_type = UC_FW_TYPE_GUC;
 	guc_fw->uc_fw_path = fw_path;
 	guc_fw->fetch_status = UC_FIRMWARE_NONE;
 	guc_fw->load_status = UC_FIRMWARE_NONE;
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index c6d53b3..2ae0542 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -148,10 +148,11 @@ void intel_huc_init(struct drm_device *dev)
 	const char *fw_path = NULL;
 
 	huc_fw->uc_dev = dev;
+	huc_fw->uc_name = "HuC";
+	huc_fw->fw_type = UC_FW_TYPE_HUC;
 	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;
-- 
1.9.1

