[Intel-gfx] [PATCH v2 1/2] drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init

Michel Thierry michel.thierry at intel.com
Mon Jun 18 16:02:14 UTC 2018


On 06/18/2018 03:39 AM, Michal Wajdeczko wrote:
> We're fetching GuC/HuC firmwares directly from uc level during
> init_early stage but this breaks guc/huc struct isolation and
> also strict SW-only initialization rule. Move fw fetching to
> init phase and do it separately per guc/huc struct.
> 
> v2: don't forget to move wopcm_init - Michele
					I'm not Italian ;)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c  |  8 ++++----
>   drivers/gpu/drm/i915/intel_guc.c |  7 ++++++-
>   drivers/gpu/drm/i915/intel_huc.c |  8 ++++++++
>   drivers/gpu/drm/i915/intel_huc.h |  6 ++++++
>   drivers/gpu/drm/i915/intel_uc.c  | 37 ++++++++++++++++++++-----------------
>   5 files changed, 44 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 822abf4..6a0fa53 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5456,10 +5456,6 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>   	if (ret)
>   		return ret;
>   
> -	ret = intel_wopcm_init(&dev_priv->wopcm);
> -	if (ret)
> -		return ret;
> -
>   	ret = intel_uc_init_misc(dev_priv);
>   	if (ret)
>   		return ret;
> @@ -5497,6 +5493,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)

Not visible, but the previous line before this chunk is

         ret = intel_uc_init(dev_priv);
>   	if (ret)
>   		goto err_pm;
>
> +	ret = intel_wopcm_init(&dev_priv->wopcm);
> +	if (ret)
> +		goto err_uc_init;
> +
>   	ret = i915_gem_init_hw(dev_priv);
>   	if (ret)
>   		goto err_uc_init;

So,

Reviewed-by: Michel Thierry <michel.thierry at intel.com>

> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 1aff30b..a63a86f 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -167,9 +167,11 @@ int intel_guc_init(struct intel_guc *guc)
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   	int ret;
>   
> +	intel_uc_fw_fetch(dev_priv, &guc->fw);
> +
>   	ret = guc_shared_data_create(guc);
>   	if (ret)
> -		return ret;
> +		goto err_fetch;
>   	GEM_BUG_ON(!guc->shared_data);
>   
>   	ret = intel_guc_log_create(&guc->log);
> @@ -190,6 +192,8 @@ int intel_guc_init(struct intel_guc *guc)
>   	intel_guc_log_destroy(&guc->log);
>   err_shared:
>   	guc_shared_data_destroy(guc);
> +err_fetch:
> +	intel_uc_fw_fini(&guc->fw);
>   	return ret;
>   }
>   
> @@ -201,6 +205,7 @@ void intel_guc_fini(struct intel_guc *guc)
>   	intel_guc_ads_destroy(guc);
>   	intel_guc_log_destroy(&guc->log);
>   	guc_shared_data_destroy(guc);
> +	intel_uc_fw_fini(&guc->fw);
>   }
>   
>   static u32 guc_ctl_debug_flags(struct intel_guc *guc)
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 2912852..8a884d7 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -32,6 +32,14 @@ void intel_huc_init_early(struct intel_huc *huc)
>   	intel_huc_fw_init_early(huc);
>   }
>   
> +int intel_huc_init(struct intel_huc *huc)
> +{
> +	struct drm_i915_private *i915 = huc_to_i915(huc);
> +
> +	intel_uc_fw_fetch(i915, &huc->fw);
> +	return 0;
> +}
> +
>   /**
>    * intel_huc_auth() - Authenticate HuC uCode
>    * @huc: intel_huc structure
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> index aa85490..21e600c 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -36,9 +36,15 @@ struct intel_huc {
>   };
>   
>   void intel_huc_init_early(struct intel_huc *huc);
> +int intel_huc_init(struct intel_huc *huc);
>   int intel_huc_auth(struct intel_huc *huc);
>   int intel_huc_check_status(struct intel_huc *huc);
>   
> +static inline void intel_huc_fini(struct intel_huc *huc)
> +{
> +	intel_uc_fw_fini(&huc->fw);
> +}
> +
>   static inline int intel_huc_sanitize(struct intel_huc *huc)
>   {
>   	intel_uc_fw_sanitize(&huc->fw);
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 94e8863..ec3c37c 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -171,24 +171,11 @@ void intel_uc_init_early(struct drm_i915_private *i915)
>   	intel_huc_init_early(huc);
>   
>   	sanitize_options_early(i915);
> -
> -	if (USES_GUC(i915))
> -		intel_uc_fw_fetch(i915, &guc->fw);
> -
> -	if (USES_HUC(i915))
> -		intel_uc_fw_fetch(i915, &huc->fw);
>   }
>   
>   void intel_uc_cleanup_early(struct drm_i915_private *i915)
>   {
>   	struct intel_guc *guc = &i915->guc;
> -	struct intel_huc *huc = &i915->huc;
> -
> -	if (USES_HUC(i915))
> -		intel_uc_fw_fini(&huc->fw);
> -
> -	if (USES_GUC(i915))
> -		intel_uc_fw_fini(&guc->fw);
>   
>   	guc_free_load_err_log(guc);
>   }
> @@ -279,6 +266,7 @@ void intel_uc_fini_misc(struct drm_i915_private *i915)
>   int intel_uc_init(struct drm_i915_private *i915)
>   {
>   	struct intel_guc *guc = &i915->guc;
> +	struct intel_huc *huc = &i915->huc;
>   	int ret;
>   
>   	if (!USES_GUC(i915))
> @@ -291,24 +279,36 @@ int intel_uc_init(struct drm_i915_private *i915)
>   	if (ret)
>   		return ret;
>   
> +	if (USES_HUC(i915)) {
> +		ret = intel_huc_init(huc);
> +		if (ret)
> +			goto err_guc;
> +	}
> +
>   	if (USES_GUC_SUBMISSION(i915)) {
>   		/*
>   		 * This is stuff we need to have available at fw load time
>   		 * if we are planning to enable submission later
>   		 */
>   		ret = intel_guc_submission_init(guc);
> -		if (ret) {
> -			intel_guc_fini(guc);
> -			return ret;
> -		}
> +		if (ret)
> +			goto err_huc;
>   	}
>   
>   	return 0;
> +
> +err_huc:
> +	if (USES_HUC(i915))
> +		intel_huc_fini(huc);
> +err_guc:
> +	intel_guc_fini(guc);
> +	return ret;
>   }
>   
>   void intel_uc_fini(struct drm_i915_private *i915)
>   {
>   	struct intel_guc *guc = &i915->guc;
> +	struct intel_huc *huc = &i915->huc;
>   
>   	if (!USES_GUC(i915))
>   		return;
> @@ -318,6 +318,9 @@ void intel_uc_fini(struct drm_i915_private *i915)
>   	if (USES_GUC_SUBMISSION(i915))
>   		intel_guc_submission_fini(guc);
>   
> +	if (USES_HUC(i915))
> +		intel_huc_fini(huc);
> +
>   	intel_guc_fini(guc);
>   }
>   
> 


More information about the Intel-gfx mailing list