We currently track fetch and load status separately, but the 2 are
actually sequential in the uc lifetime (fetch must complete before we
can attempt the load!). Unifying the 2 variables we can better follow
the sequential states and improve our trackng of the uC state.

Also, sprinkle some GEM_BUG_ON to make sure we transition correctly
between states.

Suggested-by: Michal Wajdeczko <michal.wajdec...@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/uc/intel_huc.c   |  4 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 74 ++++++++++--------------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 50 ++++++++--------
 3 files changed, 57 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index bc14439173d7..1868f676d890 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -117,7 +117,7 @@ int intel_huc_auth(struct intel_huc *huc)
        struct intel_guc *guc = &gt->uc.guc;
        int ret;
 
-       if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+       if (huc->fw.status != INTEL_UC_FIRMWARE_LOADED)
                return -ENOEXEC;
 
        ret = intel_guc_auth_huc(guc,
@@ -141,7 +141,7 @@ int intel_huc_auth(struct intel_huc *huc)
        return 0;
 
 fail:
-       huc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
+       huc->fw.status = INTEL_UC_FIRMWARE_LOAD_FAIL;
 
        DRM_ERROR("HuC: Authentication failed %d\n", ret);
        return ret;
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 faa96748aed3..91b1d9663481 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -45,15 +45,13 @@ void intel_uc_fw_select(struct drm_i915_private *i915,
                        const struct intel_uc_platform_requirement *list,
                        unsigned int length)
 {
-       GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED);
+       GEM_BUG_ON(uc_fw->status != INTEL_UC_FIRMWARE_UNINITIALIZED);
 
        if (!HAS_UC(i915)) {
-               uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
+               uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
                return;
        }
 
-       uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
-
        if (unlikely(i915_modparams.guc_firmware_path &&
                     uc_fw->type == INTEL_UC_FW_TYPE_GUC)) {
                uc_fw->path = i915_modparams.guc_firmware_path;
@@ -74,6 +72,8 @@ void intel_uc_fw_select(struct drm_i915_private *i915,
                        }
                }
        }
+
+       uc_fw->status = INTEL_UC_FIRMWARE_SELECTION_DONE;
 }
 
 /**
@@ -95,23 +95,11 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
        int err;
 
        GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
-
-       if (!uc_fw->path) {
-               dev_info(dev_priv->drm.dev,
-                        "%s: No firmware was defined for %s!\n",
-                        intel_uc_fw_type_repr(uc_fw->type),
-                        intel_platform_name(INTEL_INFO(dev_priv)->platform));
-               return;
-       }
+       GEM_BUG_ON(!intel_uc_fw_is_selected(uc_fw));
 
        DRM_DEBUG_DRIVER("%s fw fetch %s\n",
                         intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
 
-       uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
-       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) {
                DRM_DEBUG_DRIVER("%s fw request_firmware err=%d\n",
@@ -219,19 +207,17 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 
        uc_fw->obj = obj;
        uc_fw->size = fw->size;
-       uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
-       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));
+       uc_fw->status = INTEL_UC_FIRMWARE_FETCHED;
+       DRM_DEBUG_DRIVER("%s fw fetch done\n",
+                        intel_uc_fw_type_repr(uc_fw->type));
 
        release_firmware(fw);
        return;
 
 fail:
-       uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
-       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));
+       uc_fw->status = INTEL_UC_FIRMWARE_FETCH_FAIL;
+       DRM_DEBUG_DRIVER("%s fw fetch failed\n",
+                        intel_uc_fw_type_repr(uc_fw->type));
 
        DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
                 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
@@ -287,13 +273,11 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
        DRM_DEBUG_DRIVER("%s fw load %s\n",
                         intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
 
-       if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
-               return -ENOEXEC;
+       /* make sure the status was cleared the last time we reset the uc */
+       GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_LOADED);
 
-       uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
-       DRM_DEBUG_DRIVER("%s fw load %s\n",
-                        intel_uc_fw_type_repr(uc_fw->type),
-                        intel_uc_fw_status_repr(uc_fw->load_status));
+       if (uc_fw->status < INTEL_UC_FIRMWARE_FETCHED)
+               return -ENOEXEC;
 
        /* Call custom loader */
        intel_uc_fw_ggtt_bind(uc_fw);
@@ -302,10 +286,9 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
        if (err)
                goto fail;
 
-       uc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
-       DRM_DEBUG_DRIVER("%s fw load %s\n",
-                        intel_uc_fw_type_repr(uc_fw->type),
-                        intel_uc_fw_status_repr(uc_fw->load_status));
+       uc_fw->status = INTEL_UC_FIRMWARE_LOADED;
+       DRM_DEBUG_DRIVER("%s fw load completed\n",
+                        intel_uc_fw_type_repr(uc_fw->type));
 
        DRM_INFO("%s: Loaded firmware %s (version %u.%u)\n",
                 intel_uc_fw_type_repr(uc_fw->type),
@@ -315,10 +298,9 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
        return 0;
 
 fail:
-       uc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
-       DRM_DEBUG_DRIVER("%s fw load %s\n",
-                        intel_uc_fw_type_repr(uc_fw->type),
-                        intel_uc_fw_status_repr(uc_fw->load_status));
+       uc_fw->status = INTEL_UC_FIRMWARE_LOAD_FAIL;
+       DRM_DEBUG_DRIVER("%s fw load failed\n",
+                        intel_uc_fw_type_repr(uc_fw->type));
 
        DRM_WARN("%s: Failed to load firmware %s (error %d)\n",
                 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
@@ -330,7 +312,10 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
 {
        int err;
 
-       if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+       /* this should happen before the load! */
+       GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_LOADED);
+
+       if (uc_fw->status < INTEL_UC_FIRMWARE_FETCHED)
                return -ENOEXEC;
 
        err = i915_gem_object_pin_pages(uc_fw->obj);
@@ -343,7 +328,7 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
 
 void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
 {
-       if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+       if (uc_fw->status < INTEL_UC_FIRMWARE_FETCHED)
                return;
 
        i915_gem_object_unpin_pages(uc_fw->obj);
@@ -377,7 +362,7 @@ void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw)
        if (obj)
                i915_gem_object_put(obj);
 
-       uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+       uc_fw->status = INTEL_UC_FIRMWARE_SELECTION_DONE;
 }
 
 /**
@@ -391,9 +376,8 @@ void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, 
struct drm_printer *p)
 {
        drm_printf(p, "%s firmware: %s\n",
                   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
-       drm_printf(p, "\tstatus: fetch %s, load %s\n",
-                  intel_uc_fw_status_repr(uc_fw->fetch_status),
-                  intel_uc_fw_status_repr(uc_fw->load_status));
+       drm_printf(p, "\tstatus: %s\n",
+                  intel_uc_fw_status_repr(uc_fw->status));
        drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n",
                   uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted,
                   uc_fw->major_ver_found, uc_fw->minor_ver_found);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h 
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 550b626afb15..e0c5a38685e6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -36,12 +36,13 @@ struct drm_i915_private;
 #define INTEL_UC_FIRMWARE_URL 
"https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915";
 
 enum intel_uc_fw_status {
-       INTEL_UC_FIRMWARE_NOT_SUPPORTED = -2, /* no uc HW */
-       INTEL_UC_FIRMWARE_FAIL = -1,
+       INTEL_UC_FIRMWARE_LOAD_FAIL = -3,
+       INTEL_UC_FIRMWARE_FETCH_FAIL = -2,
+       INTEL_UC_FIRMWARE_NOT_SUPPORTED = -1, /* no uc HW */
        INTEL_UC_FIRMWARE_UNINITIALIZED = 0, /* used to catch checks done too 
early */
-       INTEL_UC_FIRMWARE_NOT_STARTED = 1,
-       INTEL_UC_FIRMWARE_PENDING,
-       INTEL_UC_FIRMWARE_SUCCESS
+       INTEL_UC_FIRMWARE_SELECTION_DONE, /* selection include the "no FW" case 
*/
+       INTEL_UC_FIRMWARE_FETCHED,
+       INTEL_UC_FIRMWARE_LOADED
 };
 
 enum intel_uc_fw_type {
@@ -77,8 +78,7 @@ struct intel_uc_fw {
        const char *path;
        size_t size;
        struct drm_i915_gem_object *obj;
-       enum intel_uc_fw_status fetch_status;
-       enum intel_uc_fw_status load_status;
+       enum intel_uc_fw_status status;
 
        /*
         * The firmware build process will generate a version header file with 
major and
@@ -103,18 +103,20 @@ static inline
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 {
        switch (status) {
+       case INTEL_UC_FIRMWARE_LOAD_FAIL:
+               return "LOAD FAIL";
+       case INTEL_UC_FIRMWARE_FETCH_FAIL:
+               return "FETCH FAIL";
        case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
-               return "N/A - uc HW not available";
-       case INTEL_UC_FIRMWARE_FAIL:
-               return "FAIL";
+               return "N/A";
        case INTEL_UC_FIRMWARE_UNINITIALIZED:
                return "UNINITIALIZED";
-       case INTEL_UC_FIRMWARE_NOT_STARTED:
-               return "NOT_STARTED";
-       case INTEL_UC_FIRMWARE_PENDING:
-               return "PENDING";
-       case INTEL_UC_FIRMWARE_SUCCESS:
-               return "SUCCESS";
+       case INTEL_UC_FIRMWARE_SELECTION_DONE:
+               return "SELECTION DONE";
+       case INTEL_UC_FIRMWARE_FETCHED:
+               return "FETCHED";
+       case INTEL_UC_FIRMWARE_LOADED:
+               return "LOADED";
        }
        return "<invalid>";
 }
@@ -135,38 +137,38 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
                            enum intel_uc_fw_type type)
 {
        /*
-        * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status
+        * we use FIRMWARE_UNINITIALIZED to detect checks against uc_fw->status
         * before we're looked at the HW caps to see if we have uc support
         */
        BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED);
 
        uc_fw->path = NULL;
-       uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED;
-       uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+       uc_fw->status = INTEL_UC_FIRMWARE_UNINITIALIZED;
        uc_fw->type = type;
 }
 
 static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw)
 {
+       GEM_BUG_ON(uc_fw->path && uc_fw->status < 
INTEL_UC_FIRMWARE_SELECTION_DONE);
        return uc_fw->path != NULL;
 }
 
 static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
 {
-       return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
+       return uc_fw->status == INTEL_UC_FIRMWARE_LOADED;
 }
 
 static inline bool intel_uc_fw_supported(struct intel_uc_fw *uc_fw)
 {
        /* shouldn't call this before checking hw/blob availability */
-       GEM_BUG_ON(uc_fw->fetch_status == INTEL_UC_FIRMWARE_UNINITIALIZED);
-       return uc_fw->fetch_status != INTEL_UC_FIRMWARE_NOT_SUPPORTED;
+       GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);
+       return uc_fw->status != INTEL_UC_FIRMWARE_NOT_SUPPORTED;
 }
 
 static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
 {
        if (intel_uc_fw_is_loaded(uc_fw))
-               uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+               uc_fw->status = INTEL_UC_FIRMWARE_FETCHED;
 }
 
 /**
@@ -179,7 +181,7 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw 
*uc_fw)
  */
 static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
 {
-       if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+       if (uc_fw->status < INTEL_UC_FIRMWARE_FETCHED)
                return 0;
 
        return uc_fw->header_size + uc_fw->ucode_size;
-- 
2.22.0

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

Reply via email to