On 9/12/2022 5:23 PM, Lucas De Marchi wrote:
On Tue, Sep 06, 2022 at 04:01:46PM -0700, Daniele Ceraolo Spurio wrote:
@@ -184,49 +247,94 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
    fw_count = blobs_all[uc_fw->type].count;

    for (i = 0; i < fw_count && p <= fw_blobs[i].p; i++) {
-        if (p == fw_blobs[i].p && rev >= fw_blobs[i].rev) {
-            const struct uc_fw_blob *blob = &fw_blobs[i].blob;
-            uc_fw->path = blob->path;
-            uc_fw->wanted_path = blob->path;
-            uc_fw->major_ver_wanted = blob->major;
-            uc_fw->minor_ver_wanted = blob->minor;
-            break;
-        }
-    }
+        const struct uc_fw_blob *blob = &fw_blobs[i].blob;

-    if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) {
-        const struct uc_fw_platform_requirement *blobs = blobs_guc_fallback;
-        u32 count = ARRAY_SIZE(blobs_guc_fallback);
+        if (p != fw_blobs[i].p)
+            continue;

-        for (i = 0; i < count && p <= blobs[i].p; i++) {
-            if (p == blobs[i].p && rev >= blobs[i].rev) {
-                const struct uc_fw_blob *blob = &blobs[i].blob;
+        if (rev < fw_blobs[i].rev)
+            continue;

-                uc_fw->fallback.path = blob->path;
-                uc_fw->fallback.major_ver = blob->major;
-                uc_fw->fallback.minor_ver = blob->minor;
-                break;
-            }
+        if (uc_fw->file_selected.path) {
+            if (uc_fw->file_selected.path == blob->path)
+                uc_fw->file_selected.path = NULL;
+
+            continue;
        }
+
+        uc_fw->file_selected.path = blob->path;
+        uc_fw->file_wanted.path = blob->path;
+        uc_fw->file_wanted.major_ver = blob->major;
+        uc_fw->file_wanted.minor_ver = blob->minor;
+        break;
    }

    /* make sure the list is ordered as expected */
-    if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) {
+    if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified) {
+        verified = true;
+
        for (i = 1; i < fw_count; i++) {
+            /* Next platform is good: */
            if (fw_blobs[i].p < fw_blobs[i - 1].p)
                continue;

+            /* Next platform revision is good: */
            if (fw_blobs[i].p == fw_blobs[i - 1].p &&
                fw_blobs[i].rev < fw_blobs[i - 1].rev)
                continue;

-            drm_err(&i915->drm, "Invalid FW blob order: %s r%u comes before %s r%u\n",
-                intel_platform_name(fw_blobs[i - 1].p),
-                fw_blobs[i - 1].rev,
-                intel_platform_name(fw_blobs[i].p),
-                fw_blobs[i].rev);
+            /* Platform/revision must be in order: */
+            if (fw_blobs[i].p != fw_blobs[i - 1].p ||
+                fw_blobs[i].rev != fw_blobs[i - 1].rev)
+                goto bad;
+
+            /* Next major version is good: */
+            if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
+                continue;
+
+            /* New must be before legacy: */
+            if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
+                goto bad;
+
+            /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */ +            if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) {
+                if (!fw_blobs[i - 1].blob.major)
+                    continue;
+
+                if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major)
+                    continue;
+            }
+
+            /* Major versions must be in order: */
+            if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
+                goto bad;
+
+            /* Next minor version is good: */
+            if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
+                continue;

-            uc_fw->path = NULL;
+            /* Minor versions must be in order: */
+            if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
+                goto bad;
+
+            /* Patch versions must be in order: */
+            if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
+                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",

what is this \x1B[35;1m? Probably something that went bad while
writing/pasting this?

Looks like it slipped in. Fix coming soon (John will send a v2 of " drm/i915/uc: Fix issues with overriding firmware files" and include it there).

Daniele


Lucas De Marchi

+ 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,
+                fw_blobs[i - 1].blob.minor,
+                fw_blobs[i - 1].blob.patch,
+                intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
+                fw_blobs[i].blob.legacy ? "L" : "v",
+                fw_blobs[i].blob.major,
+                fw_blobs[i].blob.minor,
+                fw_blobs[i].blob.patch);
+
+            uc_fw->file_selected.path = NULL;
        }
    }
}
@@ -259,7 +367,7 @@ static void __uc_fw_user_override(struct drm_i915_private *i915, struct intel_uc

Reply via email to