From: Matthew Brost <matthew.brost@intel.com>
To: John.C.Harrison@Intel.com
Cc: Intel-GFX@Lists.FreeDesktop.Org, DRI-Devel@Lists.FreeDesktop.Org
Subject: Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: Improve GuC loading status check/error reports
Date: Fri, 3 Dec 2021 12:22:57 -0800 [thread overview]
Message-ID: <20211203202257.GA32634@jons-linux-dev-box> (raw)
In-Reply-To: <20211203183339.3276250-6-John.C.Harrison@Intel.com>
On Fri, Dec 03, 2021 at 10:33:39AM -0800, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> If the GuC fails to load, it is useful to know what firmware file /
> version was attempted. So move the version info report to before the
> load attempt rather than only after a successful load.
>
> If the GuC does fail to load, then make the error messages visible
> rather than being 'debug' prints that do not appears in dmesg output
> by default.
>
> When waiting for the GuC to load, it used to be necessary to check for
> two different states - READY and (LAPIC_DONE | MIA_CORE). Apparently
> the second signified init complete on RC6 exit. However, in more
> recent GuC versions the RC6 exit sequence now finishes with status
> READY as well. So the test can be simplified.
>
> Also, add an enum giving all the current status codes that GuC loading
> can report as a reference without having to pull and search through
> the GuC source files.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> ---
> .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h | 23 ++++++++++++++
> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 17 +++++-----
> drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 4 ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 1 +
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 31 ++++++++++---------
> 5 files changed, 48 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
> index 488b6061ee89..c20658ee85a5 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
> @@ -11,4 +11,27 @@ enum intel_guc_response_status {
> INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
> };
>
> +enum intel_guc_load_status {
> + INTEL_GUC_LOAD_STATUS_DEFAULT = 0x00,
> + INTEL_GUC_LOAD_STATUS_START = 0x01,
> + INTEL_GUC_LOAD_STATUS_ERROR_DEVID_BUILD_MISMATCH = 0x02,
> + INTEL_GUC_LOAD_STATUS_GUC_PREPROD_BUILD_MISMATCH = 0x03,
> + INTEL_GUC_LOAD_STATUS_ERROR_DEVID_INVALID_GUCTYPE = 0x04,
> + INTEL_GUC_LOAD_STATUS_GDT_DONE = 0x10,
> + INTEL_GUC_LOAD_STATUS_IDT_DONE = 0x20,
> + INTEL_GUC_LOAD_STATUS_LAPIC_DONE = 0x30,
> + INTEL_GUC_LOAD_STATUS_GUCINT_DONE = 0x40,
> + INTEL_GUC_LOAD_STATUS_DPC_READY = 0x50,
> + INTEL_GUC_LOAD_STATUS_DPC_ERROR = 0x60,
> + INTEL_GUC_LOAD_STATUS_EXCEPTION = 0x70,
> + INTEL_GUC_LOAD_STATUS_INIT_DATA_INVALID = 0x71,
> + INTEL_GUC_LOAD_STATUS_PXP_TEARDOWN_CTRL_ENABLED = 0x72,
> + INTEL_GUC_LOAD_STATUS_INVALID_INIT_DATA_RANGE_START,
> + INTEL_GUC_LOAD_STATUS_MPU_DATA_INVALID = 0x73,
> + INTEL_GUC_LOAD_STATUS_INIT_MMIO_SAVE_RESTORE_INVALID = 0x74,
> + INTEL_GUC_LOAD_STATUS_INVALID_INIT_DATA_RANGE_END,
> +
> + INTEL_GUC_LOAD_STATUS_READY = 0xF0,
> +};
> +
> #endif /* _ABI_GUC_ERRORS_ABI_H */
> 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 196424be0998..d3cee01d07e0 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -70,11 +70,10 @@ static int guc_xfer_rsa(struct intel_uc_fw *guc_fw,
> static inline bool guc_ready(struct intel_uncore *uncore, u32 *status)
> {
> u32 val = intel_uncore_read(uncore, GUC_STATUS);
> - u32 uk_val = val & GS_UKERNEL_MASK;
> + u32 uk_val = REG_FIELD_GET(GS_UKERNEL_MASK, val);
>
> *status = val;
> - return (uk_val == GS_UKERNEL_READY) ||
> - ((val & GS_MIA_CORE_STATE) && (uk_val == GS_UKERNEL_LAPIC_DONE));
> + return uk_val == INTEL_GUC_LOAD_STATUS_READY;
> }
>
> static int guc_wait_ucode(struct intel_uncore *uncore)
> @@ -94,8 +93,8 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
> if (ret) {
> struct drm_device *drm = &uncore->i915->drm;
>
> - drm_dbg(drm, "GuC load failed: status = 0x%08X\n", status);
> - drm_dbg(drm, "GuC load failed: status: Reset = %d, "
> + drm_info(drm, "GuC load failed: status = 0x%08X\n", status);
> + drm_info(drm, "GuC load failed: status: Reset = %d, "
> "BootROM = 0x%02X, UKernel = 0x%02X, "
> "MIA = 0x%02X, Auth = 0x%02X\n",
> REG_FIELD_GET(GS_MIA_IN_RESET, status),
> @@ -105,13 +104,13 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
> REG_FIELD_GET(GS_AUTH_STATUS_MASK, status));
>
> if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
> - drm_dbg(drm, "GuC firmware signature verification failed\n");
> + drm_info(drm, "GuC firmware signature verification failed\n");
> ret = -ENOEXEC;
> }
>
> - if ((status & GS_UKERNEL_MASK) == GS_UKERNEL_EXCEPTION) {
> - drm_dbg(drm, "GuC firmware exception. EIP: %#x\n",
> - intel_uncore_read(uncore, SOFT_SCRATCH(13)));
> + if (REG_FIELD_GET(GS_UKERNEL_MASK, status) == INTEL_GUC_LOAD_STATUS_EXCEPTION) {
> + drm_info(drm, "GuC firmware exception. EIP: %#x\n",
> + intel_uncore_read(uncore, SOFT_SCRATCH(13)));
> ret = -ENXIO;
> }
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> index b37fc2ffaef2..e6bd66d6ce5a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> @@ -22,10 +22,6 @@
> #define GS_BOOTROM_JUMP_PASSED (0x76 << GS_BOOTROM_SHIFT)
> #define GS_UKERNEL_SHIFT 8
> #define GS_UKERNEL_MASK (0xFF << GS_UKERNEL_SHIFT)
> -#define GS_UKERNEL_LAPIC_DONE (0x30 << GS_UKERNEL_SHIFT)
> -#define GS_UKERNEL_DPC_ERROR (0x60 << GS_UKERNEL_SHIFT)
> -#define GS_UKERNEL_EXCEPTION (0x70 << GS_UKERNEL_SHIFT)
> -#define GS_UKERNEL_READY (0xF0 << GS_UKERNEL_SHIFT)
> #define GS_MIA_SHIFT 16
> #define GS_MIA_MASK (0x07 << GS_MIA_SHIFT)
> #define GS_MIA_CORE_STATE (0x01 << GS_MIA_SHIFT)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index ff4b6869b80b..cef406dd937e 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -195,6 +195,7 @@ int intel_huc_auth(struct intel_huc *huc)
> }
>
> intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_RUNNING);
> + drm_info(>->i915->drm, "HuC authenticated\n");
> return 0;
>
> fail:
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 2fef3b0bbe95..27b709860afc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -431,6 +431,15 @@ static int __uc_check_hw(struct intel_uc *uc)
> return 0;
> }
>
> +static void print_fw_ver(struct intel_uc *uc, struct intel_uc_fw *fw)
> +{
> + struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> +
> + drm_info(&i915->drm, "%s firmware %s version %u.%u\n",
> + intel_uc_fw_type_repr(fw->type), fw->path,
> + fw->major_ver_found, fw->minor_ver_found);
> +}
> +
> static int __uc_init_hw(struct intel_uc *uc)
> {
> struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> @@ -441,6 +450,11 @@ static int __uc_init_hw(struct intel_uc *uc)
> GEM_BUG_ON(!intel_uc_supports_guc(uc));
> GEM_BUG_ON(!intel_uc_wants_guc(uc));
>
> + print_fw_ver(uc, &guc->fw);
> +
> + if (intel_uc_uses_huc(uc))
> + print_fw_ver(uc, &huc->fw);
> +
> if (!intel_uc_fw_is_loadable(&guc->fw)) {
> ret = __uc_check_hw(uc) ||
> intel_uc_fw_is_overridden(&guc->fw) ||
> @@ -501,24 +515,11 @@ static int __uc_init_hw(struct intel_uc *uc)
> goto err_submission;
> }
>
> - drm_info(&i915->drm, "%s firmware %s version %u.%u %s:%s\n",
> - intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_GUC), guc->fw.path,
> - guc->fw.major_ver_found, guc->fw.minor_ver_found,
> - "submission",
> + drm_info(&i915->drm, "GuC submission %s\n",
> enableddisabled(intel_uc_uses_guc_submission(uc)));
> -
> - drm_info(&i915->drm, "GuC SLPC: %s\n",
> + drm_info(&i915->drm, "GuC SLPC %s\n",
> enableddisabled(intel_uc_uses_guc_slpc(uc)));
>
> - if (intel_uc_uses_huc(uc)) {
> - drm_info(&i915->drm, "%s firmware %s version %u.%u %s:%s\n",
> - intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_HUC),
> - huc->fw.path,
> - huc->fw.major_ver_found, huc->fw.minor_ver_found,
> - "authenticated",
> - yesno(intel_huc_is_authenticated(huc)));
> - }
> -
> return 0;
>
> /*
> --
> 2.25.1
>
next prev parent reply other threads:[~2021-12-03 20:28 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-03 18:33 [Intel-gfx] [PATCH 0/5] Update to GuC version 69.0.0 John.C.Harrison
2021-12-03 18:33 ` [Intel-gfx] [PATCH 1/5] drm/i915/uc: Allow platforms to have GuC but not HuC John.C.Harrison
2021-12-03 19:01 ` Lucas De Marchi
2021-12-06 22:09 ` Daniele Ceraolo Spurio
2021-12-03 18:33 ` [Intel-gfx] [PATCH 2/5] drm/i915/guc: Increase GuC log size for CONFIG_DEBUG_GEM John.C.Harrison
2021-12-03 20:25 ` Matthew Brost
2021-12-03 18:33 ` [Intel-gfx] [PATCH 3/5] drm/i915/guc: Don't go bang in GuC log if no GuC John.C.Harrison
2021-12-03 18:33 ` [Intel-gfx] [PATCH 4/5] drm/i915/guc: Update to GuC version 69.0.0 John.C.Harrison
2021-12-03 22:28 ` Michal Wajdeczko
2021-12-03 23:56 ` Matthew Brost
2021-12-03 18:33 ` [Intel-gfx] [PATCH 5/5] drm/i915/guc: Improve GuC loading status check/error reports John.C.Harrison
2021-12-03 20:22 ` Matthew Brost [this message]
2021-12-03 22:22 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Update to GuC version 69.0.0 Patchwork
2021-12-03 22:23 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-12-03 22:27 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2021-12-06 19:29 ` John Harrison
2021-12-06 22:34 ` Michal Wajdeczko
2021-12-03 22:50 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-12-04 9:55 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211203202257.GA32634@jons-linux-dev-box \
--to=matthew.brost@intel.com \
--cc=DRI-Devel@Lists.FreeDesktop.Org \
--cc=Intel-GFX@Lists.FreeDesktop.Org \
--cc=John.C.Harrison@Intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox