[Intel-gfx] [PATCH v2 07/10] drm/i915/guc: Apply new uC status tracking to GuC submission as well

Michal Wajdeczko michal.wajdeczko at intel.com
Tue Feb 4 18:06:25 UTC 2020


On Tue, 04 Feb 2020 00:28:35 +0100, Daniele Ceraolo Spurio  
<daniele.ceraolospurio at intel.com> wrote:

> To be able to differentiate the before and after of our commitment to
> GuC submission, which will be used in follow-up patches to early set-up
> the submission structures.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc.c        | 12 ++++----
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h        | 17 +++++++++--
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  9 ++----
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c         | 14 ++++-----
>  drivers/gpu/drm/i915/gt/uc/intel_uc.h         | 29 +++++++------------
>  drivers/gpu/drm/i915/gvt/scheduler.c          |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h               |  6 ----
>  drivers/gpu/drm/i915/intel_gvt.c              |  2 +-
>  8 files changed, 42 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c  
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index c4c1523da7a6..f96d1bdf4bf2 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -207,7 +207,7 @@ static u32 guc_ctl_feature_flags(struct intel_guc  
> *guc)
>  {
>  	u32 flags = 0;
> -	if (!intel_guc_is_submission_supported(guc))
> +	if (!intel_guc_submission_is_used(guc))
>  		flags |= GUC_CTL_DISABLE_SCHEDULER;
> 	return flags;
> @@ -217,7 +217,7 @@ static u32 guc_ctl_ctxinfo_flags(struct intel_guc  
> *guc)
>  {
>  	u32 flags = 0;
> -	if (intel_guc_is_submission_supported(guc)) {
> +	if (intel_guc_submission_is_used(guc)) {
>  		u32 ctxnum, base;
> 		base = intel_guc_ggtt_offset(guc, guc->stage_desc_pool);
> @@ -348,7 +348,7 @@ int intel_guc_init(struct intel_guc *guc)
>  	if (ret)
>  		goto err_ads;
> -	if (intel_guc_is_submission_supported(guc)) {
> +	if (intel_guc_submission_is_used(guc)) {
>  		/*
>  		 * This is stuff we need to have available at fw load time
>  		 * if we are planning to enable submission later
> @@ -389,7 +389,7 @@ void intel_guc_fini(struct intel_guc *guc)
> 	i915_ggtt_disable_guc(gt->ggtt);
> -	if (intel_guc_is_submission_supported(guc))
> +	if (intel_guc_submission_is_used(guc))
>  		intel_guc_submission_fini(guc);
> 	intel_guc_ct_fini(&guc->ct);
> @@ -544,7 +544,7 @@ int intel_guc_suspend(struct intel_guc *guc)
>  	 * If GuC communication is enabled but submission is not supported,
>  	 * we do not need to suspend the GuC.
>  	 */
> -	if (!intel_guc_submission_is_enabled(guc))
> +	if (!intel_guc_submission_is_used(guc) || !intel_guc_is_ready(guc))
>  		return 0;
> 	/*
> @@ -609,7 +609,7 @@ int intel_guc_resume(struct intel_guc *guc)
>  	 * we do not need to resume the GuC but we do need to enable the
>  	 * GuC communication on resume (above).
>  	 */
> -	if (!intel_guc_submission_is_enabled(guc))
> +	if (!intel_guc_submission_is_used(guc) || !intel_guc_is_ready(guc))
>  		return 0;
> 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h  
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index f6b33745ae0b..1aba4d2c15b3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -39,7 +39,7 @@ struct intel_guc {
>  		void (*disable)(struct intel_guc *guc);
>  	} interrupts;
> -	bool submission_supported;
> +	bool submission_selected;
> 	struct i915_vma *ads_vma;
>  	struct __guc_ads_blob *ads_blob;
> @@ -172,9 +172,20 @@ static inline int intel_guc_sanitize(struct  
> intel_guc *guc)
>  	return 0;
>  }
> -static inline bool intel_guc_is_submission_supported(struct intel_guc  
> *guc)
> +static inline bool intel_guc_submission_is_supported(struct intel_guc  
> *guc)
>  {
> -	return guc->submission_supported;
> +	/* XXX: GuC submission is unavailable for now */
> +	return false;
> +}
> +
> +static inline bool intel_guc_submission_is_wanted(struct intel_guc *guc)
> +{
> +	return guc->submission_selected;
> +}
> +
> +static inline bool intel_guc_submission_is_used(struct intel_guc *guc)
> +{
> +	return intel_guc_is_used(guc) && intel_guc_submission_is_wanted(guc);
>  }

can we keep all intel_guc_submission_xx() functions in  
intel_guc_submission.h?
or is it circular-dependency issue ? but these functions are not on  
hot-path so
maybe can be moved to .c to break that dependency ?

or is it due to auto generator changes below ?

> static inline void intel_guc_enable_msg(struct intel_guc *guc, u32 mask)
> 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 9e42324fdecd..1beaa77f9bb6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -660,12 +660,9 @@ void intel_guc_submission_disable(struct intel_guc  
> *guc)
>  	guc_proc_desc_fini(guc);
>  }
> -static bool __guc_submission_support(struct intel_guc *guc)
> +static bool __guc_submission_selected(struct intel_guc *guc)
>  {
> -	/* XXX: GuC submission is unavailable for now */
> -	return false;
> -
> -	if (!intel_guc_is_supported(guc))
> +	if (!intel_guc_submission_is_supported(guc))
>  		return false;
> 	return i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION;
> @@ -673,7 +670,7 @@ static bool __guc_submission_support(struct  
> intel_guc *guc)
> void intel_guc_submission_init_early(struct intel_guc *guc)
>  {
> -	guc->submission_supported = __guc_submission_support(guc);
> +	guc->submission_selected = __guc_submission_selected(guc);
>  }
> bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs  
> *engine)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c  
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 654d7c0c757a..175fa6361ff2 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -49,7 +49,7 @@ static void __confirm_options(struct intel_uc *uc)
>  			     "enable_guc=%d (guc:%s submission:%s huc:%s)\n",
>  			     i915_modparams.enable_guc,
>  			     yesno(intel_uc_wants_guc(uc)),
> -			     yesno(intel_uc_uses_guc_submission(uc)),
> +			     yesno(intel_uc_wants_guc_submission(uc)),
>  			     yesno(intel_uc_wants_huc(uc)));
> 	if (i915_modparams.enable_guc == -1)
> @@ -57,7 +57,7 @@ static void __confirm_options(struct intel_uc *uc)
> 	if (i915_modparams.enable_guc == 0) {
>  		GEM_BUG_ON(intel_uc_wants_guc(uc));
> -		GEM_BUG_ON(intel_uc_uses_guc_submission(uc));
> +		GEM_BUG_ON(intel_uc_wants_guc_submission(uc));
>  		GEM_BUG_ON(intel_uc_wants_huc(uc));
>  		return;
>  	}
> @@ -285,7 +285,7 @@ static void __uc_init(struct intel_uc *uc)
>  		return;
> 	/* XXX: GuC submission is unavailable for now */
> -	GEM_BUG_ON(intel_uc_supports_guc_submission(uc));
> +	GEM_BUG_ON(intel_uc_uses_guc_submission(uc));
> 	ret = intel_guc_init(guc);
>  	if (ret) {
> @@ -410,7 +410,7 @@ static int __uc_init_hw(struct intel_uc *uc)
>  	if (!intel_uc_fw_is_available(&guc->fw)) {
>  		ret = __uc_check_hw(uc) ||
>  		      intel_uc_fw_is_overridden(&guc->fw) ||
> -		      intel_uc_supports_guc_submission(uc) ?
> +		      intel_uc_wants_guc_submission(uc) ?
>  		      intel_uc_fw_status_to_error(guc->fw.status) : 0;
>  		goto err_out;
>  	}
> @@ -462,14 +462,14 @@ static int __uc_init_hw(struct intel_uc *uc)
>  	if (ret)
>  		goto err_communication;
> -	if (intel_uc_supports_guc_submission(uc))
> +	if (intel_uc_uses_guc_submission(uc))
>  		intel_guc_submission_enable(guc);
> 	dev_info(i915->drm.dev, "%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",
> -		 enableddisabled(intel_uc_supports_guc_submission(uc)));
> +		 enableddisabled(intel_uc_uses_guc_submission(uc)));
> 	if (intel_uc_uses_huc(uc)) {
>  		dev_info(i915->drm.dev, "%s firmware %s version %u.%u %s:%s\n",
> @@ -511,7 +511,7 @@ static void __uc_fini_hw(struct intel_uc *uc)
>  	if (!intel_guc_is_fw_running(guc))
>  		return;
> -	if (intel_uc_supports_guc_submission(uc))
> +	if (intel_uc_uses_guc_submission(uc))
>  		intel_guc_submission_disable(guc);
> 	if (guc_communication_enabled(guc))
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h  
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> index a41aaf353f88..35ce8a6be88b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> @@ -61,33 +61,24 @@ int intel_uc_runtime_resume(struct intel_uc *uc);
>   * - In use: wanted + firmware found on the system and successfully  
> fetched.
>   */
> -#define __uc_state_checker(x, state, required) \
> -static inline bool intel_uc_##state##_##x(struct intel_uc *uc) \
> +#define __uc_state_checker(x, func, state, required) \
> +static inline bool intel_uc_##state##_##func(struct intel_uc *uc) \
>  { \
> -	return intel_##x##_is_##required(&uc->x); \
> +	return intel_##func##_is_##required(&uc->x); \
>  }
> -#define uc_state_checkers(x) \
> -__uc_state_checker(x, supports, supported) \
> -__uc_state_checker(x, wants, wanted) \
> -__uc_state_checker(x, uses, used)
> +#define uc_state_checkers(x, func) \
> +__uc_state_checker(x, func, supports, supported) \
> +__uc_state_checker(x, func, wants, wanted) \
> +__uc_state_checker(x, func, uses, used)
> -uc_state_checkers(guc);
> -uc_state_checkers(huc);
> +uc_state_checkers(guc, guc);
> +uc_state_checkers(huc, huc);
> +uc_state_checkers(guc, guc_submission);
> #undef uc_state_checkers
>  #undef __uc_state_checker
> -static inline bool intel_uc_supports_guc_submission(struct intel_uc *uc)
> -{
> -	return intel_guc_is_submission_supported(&uc->guc);
> -}
> -
> -static inline bool intel_uc_uses_guc_submission(struct intel_uc *uc)
> -{
> -	return intel_guc_is_submission_supported(&uc->guc);
> -}
> -
>  #define intel_uc_ops_function(_NAME, _OPS, _TYPE, _RET) \
>  static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
>  { \
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c  
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index 5fe00ee6bd1b..e8c0885df978 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -1247,7 +1247,7 @@ int intel_vgpu_setup_submission(struct intel_vgpu  
> *vgpu)
>  		intel_context_set_single_submission(ce);
> 		/* Max ring buffer size */
> -		if (!intel_uc_uses_guc_submission(&engine->gt->uc)) {
> +		if (!intel_uc_wants_guc_submission(&engine->gt->uc)) {
>  			const unsigned int ring_size = 512 * SZ_4K;
> 			ce->ring = __intel_context_ring_size(ring_size);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h  
> b/drivers/gpu/drm/i915/i915_drv.h
> index 0a45240af38d..d21039ba2b4f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2010,10 +2010,4 @@ i915_coherent_map_type(struct drm_i915_private  
> *i915)
>  	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
>  }
> -static inline bool intel_guc_submission_is_enabled(struct intel_guc  
> *guc)
> -{
> -	return intel_guc_is_submission_supported(guc) &&
> -	       intel_guc_is_ready(guc);
> -}
> -
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c  
> b/drivers/gpu/drm/i915/intel_gvt.c
> index 5c189bc31da5..e73fd752adef 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -105,7 +105,7 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
>  		return 0;
>  	}
> -	if (intel_uc_uses_guc_submission(&dev_priv->gt.uc)) {
> +	if (intel_uc_wants_guc_submission(&dev_priv->gt.uc)) {
>  		drm_err(&dev_priv->drm,
>  			"i915 GVT-g loading failed due to Graphics virtualization is not yet  
> supported with GuC submission\n");
>  		return -EIO;


More information about the Intel-gfx mailing list