On 19/07/16 13:20, Dave Gordon wrote:
Some downgraded from DRM_ERROR() to DRM_WARN() or DRM_NOTE(),
a few upgraded from DRM_INFO() to DRM_NOTE() or DRM_WARN(),
and one eliminated completely.
v2: different permutation of levels :)
Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_guc_loader.c | 34 ++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 605c696..a2f4fa4 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -140,12 +140,14 @@ static u32 get_gttype(struct drm_i915_private *dev_priv)
static u32 get_core_family(struct drm_i915_private *dev_priv)
{
- switch (INTEL_INFO(dev_priv)->gen) {
+ u32 gen = INTEL_GEN(dev_priv);
+
+ switch (gen) {
case 9:
return GFXCORE_FAMILY_GEN9;
default:
- DRM_ERROR("GUC: unsupported core family\n");
+ DRM_WARN("GEN%d does not support GuC operation\n", gen);
This looks more like a WARN_ON condition to me, something developers
need to notice extremely easily in development and will never happen in
deployment.
return GFXCORE_FAMILY_UNKNOWN;
}
}
@@ -433,7 +435,7 @@ int intel_guc_setup(struct drm_device *dev)
goto fail;
} else if (*fw_path == '\0') {
/* Device has a GuC but we don't know what f/w to load? */
- DRM_INFO("No GuC firmware known for this platform\n");
+ DRM_WARN("No GuC firmware known for this platform\n");
This looks the same to me (WARN_ON), it can only happen if someone
messes up things in development.
err = -ENODEV;
goto fail;
}
@@ -471,10 +473,8 @@ int intel_guc_setup(struct drm_device *dev)
* that the state and timing are fairly predictable
*/
err = i915_reset_guc(dev_priv);
- if (err) {
- DRM_ERROR("GuC reset failed: %d\n", err);
+ if (err)
goto fail;
- }
err = guc_ucode_xfer(dev_priv);
if (!err)
@@ -532,15 +532,15 @@ int intel_guc_setup(struct drm_device *dev)
else if (err == 0)
DRM_INFO("GuC firmware load skipped\n");
else if (ret != -EIO)
- DRM_INFO("GuC firmware load failed: %d\n", err);
+ DRM_NOTE("GuC firmware load failed: %d\n", err);
else
- DRM_ERROR("GuC firmware load failed: %d\n", err);
+ DRM_WARN("GuC firmware load failed: %d\n", err);
if (i915.enable_guc_submission) {
if (fw_path == NULL)
DRM_INFO("GuC submission without firmware not supported\n");
if (ret == 0)
- DRM_INFO("Falling back from GuC submission to execlist mode\n");
+ DRM_NOTE("Falling back from GuC submission to execlist mode\n");
else
DRM_ERROR("GuC init failed: %d\n", ret);
}
@@ -571,7 +571,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
/* Check the size of the blob before examining buffer contents */
if (fw->size < sizeof(struct guc_css_header)) {
- DRM_ERROR("Firmware header is missing\n");
+ DRM_NOTE("Firmware header is missing\n");
goto fail;
}
@@ -583,7 +583,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
if (guc_fw->header_size != sizeof(struct guc_css_header)) {
- DRM_ERROR("CSS header definition mismatch\n");
+ DRM_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -593,7 +593,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
/* now RSA */
if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
- DRM_ERROR("RSA key size is bad\n");
+ DRM_NOTE("RSA key size is bad\n");
goto fail;
}
guc_fw->rsa_offset = guc_fw->ucode_offset + guc_fw->ucode_size;
@@ -602,14 +602,14 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
/* At least, it should have header, uCode and RSA. Size of all three. */
size = guc_fw->header_size + guc_fw->ucode_size + guc_fw->rsa_size;
if (fw->size < size) {
- DRM_ERROR("Missing firmware components\n");
+ DRM_NOTE("Missing firmware components\n");
goto fail;
}
/* Header and uCode will be loaded to WOPCM. Size of the two. */
size = guc_fw->header_size + guc_fw->ucode_size;
if (size > guc_wopcm_size(to_i915(dev))) {
- DRM_ERROR("Firmware is too large to fit in WOPCM\n");
+ DRM_NOTE("Firmware is too large to fit in WOPCM\n");
goto fail;
}
@@ -624,7 +624,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
if (guc_fw->guc_fw_major_found != guc_fw->guc_fw_major_wanted ||
guc_fw->guc_fw_minor_found < guc_fw->guc_fw_minor_wanted) {
- DRM_ERROR("GuC firmware version %d.%d, required %d.%d\n",
+ DRM_NOTE("GuC firmware version %d.%d, required %d.%d\n",
guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
err = -ENOEXEC;
@@ -654,10 +654,10 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
return;
fail:
+ DRM_WARN("Failed to fetch valid GuC firmware from %s (error %d)\n",
+ guc_fw->guc_fw_path, err);
DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n",
err, fw, guc_fw->guc_fw_obj);
As commented before, I would tidy these two into one while dropping the
uninteresting debug like object pointe and fw name pointer.
- DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n",
- guc_fw->guc_fw_path, err);
mutex_lock(&dev->struct_mutex);
obj = guc_fw->guc_fw_obj;
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Also, you can keep it if you decide to act on my suggestions from above.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
