From 91e55e545ab8bbedd3a518dbe6124c10388ff92f Mon Sep 17 00:00:00 2001 From: Daniele Ceraolo Spurio Date: Wed, 24 Jul 2019 17:18:09 -0700 Subject: [PATCH] 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 Signed-off-by: Daniele Ceraolo Spurio Cc: Michal Wajdeczko Cc: Chris Wilson Reviewed-by: Michal Wajdeczko Signed-off-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20190725001813.4740-5-daniele.ceraolospurio@intel.com --- drivers/gpu/drm/i915/gt/uc/intel_guc.h | 4 +- drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 6 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_huc.c | 8 +- drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 ++ drivers/gpu/drm/i915/gt/uc/intel_uc.c | 10 +-- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 78 +++++++------------ drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 63 ++++++++++----- 8 files changed, 92 insertions(+), 84 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index ac6333ad7102..714e9892aaff 100644 --- 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 99f44d8ae026..eec767383e92 100644 --- 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 a0f2a01365bc..b4238fe16a03 100644 --- 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 ab6c1564b6a7..a45976e56af7 100644 --- 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 = >->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 9fa3d4629f2e..ea340f85bc46 100644 --- 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 3f672ea7456b..b1815abecf30 100644 --- 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 9206d4221789..1e7df2c19265 100644 --- 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 c93e271917c9..f6aa2e3e4d1f 100644 --- 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 ""; } @@ -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; -- 2.30.2