All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915/uc: correctly track uc_fw init failure
Date: Thu, 9 Dec 2021 11:04:11 -0800	[thread overview]
Message-ID: <20211209190410.GA21453@jons-linux-dev-box> (raw)
In-Reply-To: <20211209005610.1499729-2-daniele.ceraolospurio@intel.com>

On Wed, Dec 08, 2021 at 04:56:08PM -0800, Daniele Ceraolo Spurio wrote:
> The FAILURE state of uc_fw currently implies that the fw is loadable
> (i.e init completed), so we can't use it for init failures and instead
> need a dedicated error code.
> 
> Note that this currently does not cause any issues because if we fail to
> init any of the firmwares we abort the load, but better be accurate
> anyway in case things change in the future.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c |  2 +-
>  drivers/gpu/drm/i915/gt/uc/intel_huc.c    |  2 +-
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  |  4 ++--
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  | 17 +++++++++++------
>  4 files changed, 15 insertions(+), 10 deletions(-)
> 
> 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..796483a41353 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -164,6 +164,6 @@ int intel_guc_fw_upload(struct intel_guc *guc)
>  	return 0;
>  
>  out:
> -	intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_FAIL);
> +	intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index ff4b6869b80b..c10736dddfb4 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -199,7 +199,7 @@ int intel_huc_auth(struct intel_huc *huc)
>  
>  fail:
>  	i915_probe_error(gt->i915, "HuC: Authentication failed %d\n", ret);
> -	intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_FAIL);
> +	intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
>  	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 3aa87be4f2e4..b7fa51aefdff 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -540,7 +540,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, u32 dst_offset, u32 dma_flags)
>  	i915_probe_error(gt->i915, "Failed to load %s firmware %s (%d)\n",
>  			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
>  			 err);
> -	intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_FAIL);
> +	intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
>  	return err;
>  }
>  
> @@ -558,7 +558,7 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
>  	if (err) {
>  		DRM_DEBUG_DRIVER("%s fw pin-pages err=%d\n",
>  				 intel_uc_fw_type_repr(uc_fw->type), err);
> -		intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_FAIL);
> +		intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_INIT_FAIL);
>  	}
>  
>  	return err;
> 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 1e00bf65639e..fd17abf2ab02 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -32,11 +32,12 @@ struct intel_gt;
>   * |            |    MISSING <--/    |    \--> ERROR                |
>   * |   fetch    |                    V                              |
>   * |            |                 AVAILABLE                         |
> - * +------------+-                   |                             -+
> + * +------------+-                   |   \                         -+
> + * |            |                    |    \--> INIT FAIL            |
>   * |   init     |                    V                              |
>   * |            |        /------> LOADABLE <----<-----------\       |
>   * +------------+-       \         /    \        \           \     -+
> - * |            |         FAIL <--<      \--> TRANSFERRED     \     |
> + * |            |    LOAD FAIL <--<      \--> TRANSFERRED     \     |
>   * |   upload   |                  \           /   \          /     |
>   * |            |                   \---------/     \--> RUNNING    |
>   * +------------+---------------------------------------------------+
> @@ -50,8 +51,9 @@ enum intel_uc_fw_status {
>  	INTEL_UC_FIRMWARE_MISSING, /* blob not found on the system */
>  	INTEL_UC_FIRMWARE_ERROR, /* invalid format or version */
>  	INTEL_UC_FIRMWARE_AVAILABLE, /* blob found and copied in mem */
> +	INTEL_UC_FIRMWARE_INIT_FAIL, /* failed to prepare fw objects for load */
>  	INTEL_UC_FIRMWARE_LOADABLE, /* all fw-required objects are ready */
> -	INTEL_UC_FIRMWARE_FAIL, /* failed to xfer or init/auth the fw */
> +	INTEL_UC_FIRMWARE_LOAD_FAIL, /* failed to xfer or init/auth the fw */
>  	INTEL_UC_FIRMWARE_TRANSFERRED, /* dma xfer done */
>  	INTEL_UC_FIRMWARE_RUNNING /* init/auth done */
>  };
> @@ -130,10 +132,12 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>  		return "ERROR";
>  	case INTEL_UC_FIRMWARE_AVAILABLE:
>  		return "AVAILABLE";
> +	case INTEL_UC_FIRMWARE_INIT_FAIL:
> +		return "INIT FAIL";
>  	case INTEL_UC_FIRMWARE_LOADABLE:
>  		return "LOADABLE";
> -	case INTEL_UC_FIRMWARE_FAIL:
> -		return "FAIL";
> +	case INTEL_UC_FIRMWARE_LOAD_FAIL:
> +		return "LOAD FAIL";
>  	case INTEL_UC_FIRMWARE_TRANSFERRED:
>  		return "TRANSFERRED";
>  	case INTEL_UC_FIRMWARE_RUNNING:
> @@ -155,7 +159,8 @@ static inline int intel_uc_fw_status_to_error(enum intel_uc_fw_status status)
>  		return -ENOENT;
>  	case INTEL_UC_FIRMWARE_ERROR:
>  		return -ENOEXEC;
> -	case INTEL_UC_FIRMWARE_FAIL:
> +	case INTEL_UC_FIRMWARE_INIT_FAIL:
> +	case INTEL_UC_FIRMWARE_LOAD_FAIL:
>  		return -EIO;
>  	case INTEL_UC_FIRMWARE_SELECTED:
>  		return -ESTALE;
> -- 
> 2.25.1
> 

  reply	other threads:[~2021-12-09 19:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09  0:56 [Intel-gfx] [PATCH 0/3] Support bigger GuC RSA keys Daniele Ceraolo Spurio
2021-12-09  0:56 ` [Intel-gfx] [PATCH 1/3] drm/i915/uc: correctly track uc_fw init failure Daniele Ceraolo Spurio
2021-12-09 19:04   ` Matthew Brost [this message]
2021-12-09  0:56 ` [Intel-gfx] [PATCH 2/3] drm/i915/uc: Prepare for different firmware key sizes Daniele Ceraolo Spurio
2021-12-09 18:57   ` Matthew Brost
2021-12-09  0:56 ` [Intel-gfx] [PATCH 3/3] drm/i915/guc: support bigger RSA keys Daniele Ceraolo Spurio
2021-12-09 19:47   ` Matthew Brost
2021-12-09 23:27 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Support bigger GuC " Patchwork
2021-12-09 23:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-12-10  9:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " 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=20211209190410.GA21453@jons-linux-dev-box \
    --to=matthew.brost@intel.com \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.