[Intel-gfx] [PATCH v2 3/8] drm/i915/uc: Unify uc_fw status tracking

Michal Wajdeczko michal.wajdeczko at intel.com
Wed Jul 24 12:35:51 UTC 2019


On Wed, 24 Jul 2019 04:21:48 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio at intel.com> wrote:

> 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)
>
> Suggested-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  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      | 86 +++++++------------
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h      | 58 ++++++++-----
>  8 files changed, 89 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h  
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index 6852352381ce..f51c4c3c1d0b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -169,9 +169,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 a027deb80330..085e7842ef8a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -232,5 +232,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;

we should already know that in guc_fw_xfer/guc_xfer_ucode
see below for details

> +
> +	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..7804ea5f699c 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 = &gt->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_LOAD_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 d60c56fd72e5..b761809946b1 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -559,7 +559,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));
> @@ -581,7 +581,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);
> @@ -593,7 +593,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);
> @@ -608,7 +608,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)
> @@ -620,7 +620,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 48100dff466d..9fc72c2e50d1 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -130,7 +130,7 @@ __uc_fw_select(struct intel_uc_fw *uc_fw, enum  
> intel_platform p, u8 rev)
>  			       fw_blobs[i].first_rev);
> 			uc_fw->path = NULL;
> -			uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> +			uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
>  			return;
>  		}
>  	}
> @@ -139,15 +139,13 @@ __uc_fw_select(struct intel_uc_fw *uc_fw, enum  
> intel_platform p, u8 rev)
>  static void
>  uc_fw_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
>  {
> -	GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED);
> +	GEM_BUG_ON(uc_fw->status != INTEL_UC_FIRMWARE_UNINITIALIZED);
> 	if (!HAS_GT_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;
> @@ -156,6 +154,8 @@ uc_fw_select(struct drm_i915_private *i915, struct  
> intel_uc_fw *uc_fw)
>  		uc_fw->path = i915_modparams.huc_firmware_path;
>  	else
>  		__uc_fw_select(uc_fw, INTEL_INFO(i915)->platform, INTEL_REVID(i915));
> +
> +	uc_fw->status = INTEL_UC_FIRMWARE_SELECTION_DONE;
>  }
> /**
> @@ -172,14 +172,13 @@ void intel_uc_fw_init_early(struct  
> drm_i915_private *i915,
>  			    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;
> 	uc_fw_select(i915, uc_fw);
> @@ -204,29 +203,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;
> -	}
> -
> -	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));
> +	GEM_BUG_ON(!intel_uc_fw_is_selected(uc_fw));
> 	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);
> @@ -328,19 +309,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_FETCH_FAIL;
> 	DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
>  		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
> @@ -396,14 +371,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);
> @@ -411,10 +383,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;

maybe we can slightly modify xfer function agreement and use
-EINPROGRESS to indicate whether fw is just loaded (HuC) or
is already authenticated and running (GuC):

	if (!err)
		uc_fw->status = INTEL_UC_FIRMWARE_RUNNING;
	else if (err == -EINPROGRESS)
		uc_fw->status = INTEL_UC_FIRMWARE_LOADED;
	else
		goto fail;

> +	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),
> @@ -424,10 +395,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);
> @@ -439,7 +409,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);
> @@ -452,7 +425,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);
> @@ -486,7 +459,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;
>  }
> /**
> @@ -500,9 +473,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 c2868ef15eda..ecdec4320260 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_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 */

why do you want to keep "No FW" case here ?
when we know that there is no fw, we should not attempt to fetch it.
so this is different state than "fw was selected, awaiting fetch"

> +	INTEL_UC_FIRMWARE_AVAILABLE, /* fetch done */
> +	INTEL_UC_FIRMWARE_LOADED, /* dma xfer done */
> +	INTEL_UC_FIRMWARE_RUNNING /* fw 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_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";

nit: this is not my favorite, what was wrong with
"PENDING" (known, awaiting fetch/load, look it's transient state!)
"SELECTED" (shorter, applies to this fw object vs step)

> +	case INTEL_UC_FIRMWARE_AVAILABLE:
> +		return "AVAILABLE";
> +	case INTEL_UC_FIRMWARE_LOADED:
> +		return "LOADED";
> +	case INTEL_UC_FIRMWARE_RUNNING:
> +		return "RUNNING";
>  	}
>  	return "<invalid>";
>  }
> @@ -113,25 +118,36 @@ static inline const char  
> *intel_uc_fw_type_repr(enum intel_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_available(struct intel_uc_fw *uc_fw)
> +{
> +	return uc_fw->status >= 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 uc_fw->status >= INTEL_UC_FIRMWARE_LOADED;
> +}
> +
> +static inline bool intel_uc_fw_is_running(struct intel_uc_fw *uc_fw)
> +{
> +	return uc_fw->status == 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;
> +	GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);

shouldn't we have this check on all uc_fw_is_xxx() functions ?

> +	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_AVAILABLE;
>  }
> /**
> @@ -144,7 +160,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;


More information about the Intel-gfx mailing list