drm/i915/uc: Unify uc_fw status tracking

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.

v2: rename states, add the running state (Michal), drop some logs in
    the fetch path (Michal, Chris)

v3: re-rename states, extend early status check to all helpers (Michal)

Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190725001813.4740-5-daniele.ceraolospurio@intel.com

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index ac6333ad7102840173d6930a136679cccb357578..714e9892aaffc8fab9ea96316bb3aeb238bfa2ed 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -172,9 +172,9 @@ int intel_guc_suspend(struct intel_guc *guc);
 int intel_guc_resume(struct intel_guc *guc);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 
-static inline bool intel_guc_is_loaded(struct intel_guc *guc)
+static inline bool intel_guc_is_running(struct intel_guc *guc)
 {
-       return intel_uc_fw_is_loaded(&guc->fw);
+       return intel_uc_fw_is_running(&guc->fw);
 }
 
 static inline int intel_guc_sanitize(struct intel_guc *guc)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 99f44d8ae0266a127c9976fa2a5a3b8904c0861c..eec767383e92c56192cc128a2682a4879e4ed55b 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -230,5 +230,9 @@ static int guc_fw_xfer(struct intel_uc_fw *guc_fw)
  */
 int intel_guc_fw_upload(struct intel_guc *guc)
 {
-       return intel_uc_fw_upload(&guc->fw, guc_fw_xfer);
+       int ret = intel_uc_fw_upload(&guc->fw, guc_fw_xfer);
+       if (!ret)
+               guc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
+
+       return ret;
 }

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0f2a01365bce4c3acc3b36ed5e5a4db07f76245..b4238fe16a036a1a33ec3ff9bed3955aeb8fc1e5 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -941,7 +941,7 @@ static void __guc_client_disable(struct intel_guc_client *client)
         * the case, instead of trying (in vain) to communicate with it, let's
         * just cleanup the doorbell HW and our internal state.
         */
-       if (intel_guc_is_loaded(client->guc))
+       if (intel_guc_is_running(client->guc))
                destroy_doorbell(client);
        else
                __fini_doorbell(client);

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index ab6c1564b6a7c5a7657f63d941ccc25760872d1c..a45976e56af77bac647ef10d60d74cb9269131bb 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -117,8 +117,8 @@ 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)
-               return -ENOEXEC;
+       GEM_BUG_ON(!intel_uc_fw_is_loaded(&huc->fw));
+       GEM_BUG_ON(intel_huc_is_authenticated(huc));
 
        ret = intel_guc_auth_huc(guc,
                                 intel_guc_ggtt_offset(guc, huc->rsa_data));
@@ -138,10 +138,12 @@ int intel_huc_auth(struct intel_huc *huc)
                goto fail;
        }
 
+       huc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
+
        return 0;
 
 fail:
-       huc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
+       huc->fw.status = INTEL_UC_FIRMWARE_FAIL;
 
        DRM_ERROR("HuC: Authentication failed %d\n", ret);
        return ret;

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 9fa3d4629f2e97a7d6cdd7457697c0c2a7d03f2e..ea340f85bc46da7ff80a9ec7e76fec6a9839fae7 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -56,4 +56,9 @@ static inline int intel_huc_sanitize(struct intel_huc *huc)
        return 0;
 }
 
+static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
+{
+       return intel_uc_fw_is_running(&huc->fw);
+}
+
 #endif

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 3f672ea7456b97e21032ac6ea76bd234538eacac..b1815abecf308e4adfcd79b1d36431d83001192e 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -560,7 +560,7 @@ void intel_uc_fini_hw(struct intel_uc *uc)
 {
        struct intel_guc *guc = &uc->guc;
 
-       if (!intel_guc_is_loaded(guc))
+       if (!intel_guc_is_running(guc))
                return;
 
        GEM_BUG_ON(!intel_uc_fw_supported(&guc->fw));
@@ -582,7 +582,7 @@ void intel_uc_reset_prepare(struct intel_uc *uc)
 {
        struct intel_guc *guc = &uc->guc;
 
-       if (!intel_guc_is_loaded(guc))
+       if (!intel_guc_is_running(guc))
                return;
 
        guc_stop_communication(guc);
@@ -594,7 +594,7 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
        struct intel_guc *guc = &uc->guc;
        int err;
 
-       if (!intel_guc_is_loaded(guc))
+       if (!intel_guc_is_running(guc))
                return;
 
        err = intel_guc_suspend(guc);
@@ -609,7 +609,7 @@ void intel_uc_suspend(struct intel_uc *uc)
        struct intel_guc *guc = &uc->guc;
        intel_wakeref_t wakeref;
 
-       if (!intel_guc_is_loaded(guc))
+       if (!intel_guc_is_running(guc))
                return;
 
        with_intel_runtime_pm(&uc_to_gt(uc)->i915->runtime_pm, wakeref)
@@ -621,7 +621,7 @@ int intel_uc_resume(struct intel_uc *uc)
        struct intel_guc *guc = &uc->guc;
        int err;
 
-       if (!intel_guc_is_loaded(guc))
+       if (!intel_guc_is_running(guc))
                return 0;
 
        guc_enable_communication(guc);

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 9206d42217897257ce6db7bd9594436b8d94da7e..1e7df2c19265e7f35cf280ce67da2f9b3779eae2 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -162,12 +162,11 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
                            struct drm_i915_private *i915)
 {
        /*
-        * 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);
-       GEM_BUG_ON(uc_fw->fetch_status);
-       GEM_BUG_ON(uc_fw->load_status);
+       GEM_BUG_ON(uc_fw->status);
        GEM_BUG_ON(uc_fw->path);
 
        uc_fw->type = type;
@@ -176,13 +175,10 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
                __uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform,
                                    INTEL_REVID(i915));
 
-       if (uc_fw->path) {
-               uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
-               uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED;
-       } else {
-               uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
-               uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
-       }
+       if (uc_fw->path)
+               uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
+       else
+               uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
 }
 
 /**
@@ -205,20 +201,9 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 
        GEM_BUG_ON(!intel_uc_fw_supported(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",
-                                intel_uc_fw_type_repr(uc_fw->type), err);
+       if (err)
                goto fail;
-       }
 
        DRM_DEBUG_DRIVER("%s fw size %zu ptr %p\n",
                         intel_uc_fw_type_repr(uc_fw->type), fw->size, fw);
@@ -320,19 +305,13 @@ 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_AVAILABLE;
 
        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_MISSING;
 
        DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
                 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
@@ -388,14 +367,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;
-
-       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));
+       /* make sure the status was cleared the last time we reset the uc */
+       GEM_BUG_ON(intel_uc_fw_is_loaded(uc_fw));
 
+       if (!intel_uc_fw_is_available(uc_fw))
+               return -ENOEXEC;
        /* Call custom loader */
        intel_uc_fw_ggtt_bind(uc_fw);
        err = xfer(uc_fw);
@@ -403,10 +379,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_TRANSFERRED;
+       DRM_DEBUG_DRIVER("%s fw xfer 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),
@@ -416,10 +391,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_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);
@@ -431,7 +405,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(intel_uc_fw_is_loaded(uc_fw));
+
+       if (!intel_uc_fw_is_available(uc_fw))
                return -ENOEXEC;
 
        err = i915_gem_object_pin_pages(uc_fw->obj);
@@ -444,7 +421,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 (!intel_uc_fw_is_available(uc_fw))
                return;
 
        i915_gem_object_unpin_pages(uc_fw->obj);
@@ -478,7 +455,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_SELECTED;
 }
 
 /**
@@ -492,9 +469,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 c93e271917c9ab6535dfc46737eeee6a6d0f33dd..f6aa2e3e4d1f80e88ed47dab72e13edb23291439 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -35,12 +35,14 @@ 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_FAIL = -3, /* failed to xfer or init/auth the fw */
+       INTEL_UC_FIRMWARE_MISSING = -2, /* blob not found on the system */
+       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_SELECTED, /* selected the blob we want to load */
+       INTEL_UC_FIRMWARE_AVAILABLE, /* blob found and copied in mem */
+       INTEL_UC_FIRMWARE_TRANSFERRED, /* dma xfer done */
+       INTEL_UC_FIRMWARE_RUNNING /* init/auth done */
 };
 
 enum intel_uc_fw_type {
@@ -57,8 +59,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
@@ -83,18 +84,22 @@ static inline
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 {
        switch (status) {
-       case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
-               return "N/A - uc HW not available";
        case INTEL_UC_FIRMWARE_FAIL:
                return "FAIL";
+       case INTEL_UC_FIRMWARE_MISSING:
+               return "MISSING";
+       case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
+               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_SELECTED:
+               return "SELECTED";
+       case INTEL_UC_FIRMWARE_AVAILABLE:
+               return "AVAILABLE";
+       case INTEL_UC_FIRMWARE_TRANSFERRED:
+               return "TRANSFERRED";
+       case INTEL_UC_FIRMWARE_RUNNING:
+               return "RUNNING";
        }
        return "<invalid>";
 }
@@ -110,22 +115,38 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
        return "uC";
 }
 
+static inline enum intel_uc_fw_status
+__intel_uc_fw_status(struct intel_uc_fw *uc_fw)
+{
+       /* shouldn't call this before checking hw/blob availability */
+       GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);
+       return uc_fw->status;
+}
+
+static inline bool intel_uc_fw_is_available(struct intel_uc_fw *uc_fw)
+{
+       return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_AVAILABLE;
+}
+
 static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
 {
-       return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
+       return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_TRANSFERRED;
+}
+
+static inline bool intel_uc_fw_is_running(struct intel_uc_fw *uc_fw)
+{
+       return __intel_uc_fw_status(uc_fw) == INTEL_UC_FIRMWARE_RUNNING;
 }
 
 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;
+       return __intel_uc_fw_status(uc_fw) != 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_AVAILABLE;
 }
 
 /**
@@ -138,7 +159,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 (!intel_uc_fw_is_available(uc_fw))
                return 0;
 
        return uc_fw->header_size + uc_fw->ucode_size;