drm/i915/uc: Fix issues with overriding firmware files

The earlier update to support reduced versioning of firmware files
introduced an issue with the firmware override module parameter. A
self test would specify an invalid file name (invalid meaning not in
the table) both with and without setting the override flag. The
*non-override* case would cause an infinite loop. I.e. a situation
that is impossible to hit outside of the selftest because either the
file name has come from the table in first place or it came from an
override. However, the override case was also broken in that it would
bypass some of the later processing.

The first fix is to update the scanning loop code so that if an
invalid file is passed in, it will exit rather than loop forever. So
if the impossible situation did somehow occur in the future, it
wouldn't be such a big problem.

The second flips the logic on the override early exit to be negative
rather than positive. That way if an explicit override has been set,
then it won't try to scan for backup options (because there is no
point anyway - the user wanted X and if X is not available, that's
their problem). It also means that it won't skip code that still needs
to be run once a valid firmware file has been selected.

v2: Also remove ANSI colour codes that accidentally got left in an
error message in the original patch.

Fixes: 665ae9c9ca79 ("drm/i915/uc: Support for version reduced and multiple firmware files")
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220914005821.3702446-2-John.C.Harrison@Intel.com

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index af42591..1169e2a 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -232,6 +232,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
        u32 fw_count;
        u8 rev = INTEL_REVID(i915);
        int i;
+       bool found;
 
        /*
         * The only difference between the ADL GuC FWs is the HWConfig support.
@@ -246,6 +247,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
        fw_blobs = blobs_all[uc_fw->type].blobs;
        fw_count = blobs_all[uc_fw->type].count;
 
+       found = false;
        for (i = 0; i < fw_count && p <= fw_blobs[i].p; i++) {
                const struct uc_fw_blob *blob = &fw_blobs[i].blob;
 
@@ -266,9 +268,15 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
                uc_fw->file_wanted.path = blob->path;
                uc_fw->file_wanted.major_ver = blob->major;
                uc_fw->file_wanted.minor_ver = blob->minor;
+               found = true;
                break;
        }
 
+       if (!found && uc_fw->file_selected.path) {
+               /* Failed to find a match for the last attempt?! */
+               uc_fw->file_selected.path = NULL;
+       }
+
        /* make sure the list is ordered as expected */
        if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified) {
                verified = true;
@@ -322,7 +330,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
                                continue;
 
 bad:
-                       drm_err(&i915->drm, "\x1B[35;1mInvalid FW blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
+                       drm_err(&i915->drm, "Invalid FW blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
                                intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev,
                                fw_blobs[i - 1].blob.legacy ? "L" : "v",
                                fw_blobs[i - 1].blob.major,
@@ -553,10 +561,14 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
 
        err = firmware_request_nowarn(&fw, uc_fw->file_selected.path, dev);
        memcpy(&file_ideal, &uc_fw->file_wanted, sizeof(file_ideal));
-       if (!err || intel_uc_fw_is_overridden(uc_fw))
-               goto done;
+
+       /* Any error is terminal if overriding. Don't bother searching for older versions */
+       if (err && intel_uc_fw_is_overridden(uc_fw))
+               goto fail;
 
        while (err == -ENOENT) {
+               old_ver = true;
+
                __uc_fw_auto_select(i915, uc_fw);
                if (!uc_fw->file_selected.path) {
                        /*
@@ -576,8 +588,6 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
        if (err)
                goto fail;
 
-       old_ver = true;
-done:
        if (uc_fw->loaded_via_gsc)
                err = check_gsc_manifest(fw, uc_fw);
        else