On Fri, Dec 03, 2021 at 10:33:39AM -0800, John.C.Harrison@xxxxxxxxx wrote:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> 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@xxxxxxxxx>
Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> ---
> .../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
>
