On 06/07/16 15:24, Peter Antoine wrote:
Rename some of the GuC fw loading code to make them more general. We
will utilise them for HuC loading as well.
s/intel_guc_fw/intel_uc_fw/g
s/GUC_FIRMWARE/UC_FIRMWARE/g
Struct intel_guc_fw is renamed to intel_uc_fw. Prefix of tts members,
such as 'guc' or 'guc_fw' either is renamed to 'uc' or removed for
same purpose.
v2: rebased on top of nightly.
reapplied the search/replace as upstream code as changed.
v3: rebased again on drm-nightly.
Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx>
Signed-off-by: Peter Antoine <peter.antoine@xxxxxxxxx>
Reviewed-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_debugfs.c | 12 +--
drivers/gpu/drm/i915/i915_guc_submission.c | 4 +-
drivers/gpu/drm/i915/intel_guc.h | 39 ++++----
drivers/gpu/drm/i915/intel_guc_loader.c | 146 ++++++++++++++---------------
4 files changed, 101 insertions(+), 100 deletions(-)
As of yesterday, the odd-numbered patches 1 & 3 no longer apply cleanly
and will need rebasing (again).
Also (as of last week's Tech Forum) any series containing more than a
single patch should preferably have a cover letter that at least gives a
summary of the patchset as a whole.
However the main problem with this patch it not what it changes, as what
it fails to change. As I previously suggested (and provided code for)
you need to change all the messages so they don't say "GuC" when we're
actually dealing with the HuC. Updated fixup-patch attached ...
.Dave.
>From 8a2e98098ff48ed1a9abec3159e630b3a8c18f64 Mon Sep 17 00:00:00 2001
From: Dave Gordon <david.s.gordon@xxxxxxxxx>
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@xxxxxxxxx>
---
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
