From patchwork Mon Jul 22 23:20:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniele Ceraolo Spurio X-Patchwork-Id: 11053683 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D686C112C for ; Mon, 22 Jul 2019 23:21:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C67DA2853C for ; Mon, 22 Jul 2019 23:21:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BAE74285B8; Mon, 22 Jul 2019 23:21:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 0A8161FFB7 for ; Mon, 22 Jul 2019 23:21:32 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9047389F4F; Mon, 22 Jul 2019 23:21:30 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id F053489F45 for ; Mon, 22 Jul 2019 23:21:22 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Jul 2019 16:21:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,296,1559545200"; d="scan'208";a="188709314" Received: from dceraolo-linux.fm.intel.com ([10.1.27.145]) by fmsmga001.fm.intel.com with ESMTP; 22 Jul 2019 16:21:22 -0700 From: Daniele Ceraolo Spurio To: intel-gfx@lists.freedesktop.org Date: Mon, 22 Jul 2019 16:20:44 -0700 Message-Id: <20190722232048.9970-6-daniele.ceraolospurio@intel.com> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190722232048.9970-1-daniele.ceraolospurio@intel.com> References: <20190722232048.9970-1-daniele.ceraolospurio@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 5/9] drm/i915/uc: Unify uc_fw status tracking X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP 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 Signed-off-by: Daniele Ceraolo Spurio Cc: Michal Wajdeczko Cc: Chris Wilson --- 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 = >->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 ""; } @@ -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;