On Thu, 12 Oct 2017 11:07:21 +0200, Chris Wilson <ch...@chris-wilson.co.uk> wrote:

Quoting Michal Wajdeczko (2017-10-10 15:51:32)
Time to remove less important info and make messages clear
and consistent.

Signed-off-by: Michal Wajdeczko <michal.wajdec...@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kam...@intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
---
drivers/gpu/drm/i915/intel_uc_fw.c | 73 +++++++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 482115b..b52d6b6 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -45,26 +45,33 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
        size_t size;
        int err;

+       DRM_DEBUG_DRIVER("%s fw fetch %s\n",
+ intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
+
        if (!uc_fw->path)
                return;

        uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
-
- DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
+       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));

        err = request_firmware(&fw, uc_fw->path, &pdev->dev);
-       if (err)
-               goto fail;
-       if (!fw)
+       if (err) {
+               DRM_NOTE("%s: Error while requesting firmware\n",
+                        intel_uc_fw_type_repr(uc_fw->type));

So what am I, the user, meant to do? Do I need to worry? What are the
consequences of this?

Yes, you can be little worried.
Being here means that driver decided to install *desired* firmware.

We don't know the consequences yet, as there might be a fallback
scenario available (like execlist submission, huc not used)

As we are jumping into fail label, which will start with similar
message, I can downgrade this message into DRM_DEBUG_DRIVER to
avoid duplication.


                goto fail;
+       }

...
fail:
+       DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
+                intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);

And then my "significant but normal" message has suddenly become a
warning the driver is crippled. Make up your mind, do I need to panic or
not?

Good point. This can be DRM_NOTE.

But maybe we should promote some other existing DRM_NOTEs into:

DRM_WARN("%s: Unexpected firmware size (%zu, min %zu)\n",
DRM_WARN("%s: Mismatched firmware header definition\n",
DRM_WARN("%s: Mismatched firmware header definition\n",
DRM_WARN("%s: Mismatched firmware RSA key size (%u)\n",
DRM_WARN("%s: Truncated firmware (%zu, expected %zu)\n",

as these indicates corrupted/mismatched data (and it's not normal)

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

Reply via email to