Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/mtl: Add module parameter override for Wa_16019325821/Wa_14019159160

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Oct 27, 2023 at 02:18:14PM -0700, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> These w/a's can have signficant performance implications for any
> workload which uses both RCS and CCS. On the other hand, the hang
> itself is only seen in one or two very specific workloads. So add a
> module parameter to control whether the w/a's are enabled or not and
> default to not.

No, we are not adding a module parameter for a hardware workaround.

we need data to decide what's the impact and decide if we will live
the workaround enabled or disabled and we are not allowing toggle.

we also need to push back on hardware teams to continue pursuing
better and more feasible workaround.

or we need to disable the feature itself. But no module parameter
for hardware workarounds.

> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc.c            | 3 ++-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c        | 3 ++-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 3 ++-
>  drivers/gpu/drm/i915/i915_params.c                | 3 +++
>  drivers/gpu/drm/i915/i915_params.h                | 1 +
>  5 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 6252f32d67011..4c89983b1e907 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -296,7 +296,8 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc)
>  
>  	/* Wa_16019325821 */
>  	/* Wa_14019159160 */
> -	if (IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71)))
> +	if (gt->i915->params.enable_mtl_rcs_ccs_wa &&
> +	    IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71)))
>  		flags |= GUC_WA_RCS_CCS_SWITCHOUT;
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> index 8f7298cbbc322..78757e78bce88 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> @@ -845,7 +845,8 @@ static void guc_waklv_init(struct intel_guc *guc)
>  	remain = guc_ads_waklv_size(guc);
>  
>  	/* Wa_14019159160 */
> -	if (IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71))) {
> +	if (gt->i915->params.enable_mtl_rcs_ccs_wa &&
> +	    IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71))) {
>  		size = guc_waklv_ra_mode(guc, offset, remain);
>  		offset += size;
>  		remain -= size;
> 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 225812b299524..4de54a100c451 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -4384,7 +4384,8 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine)
>  
>  	/* Wa_16019325821 */
>  	/* Wa_14019159160 */
> -	if ((engine->class == COMPUTE_CLASS || engine->class == RENDER_CLASS) &&
> +	if (engine->i915->params.enable_mtl_rcs_ccs_wa &&
> +	    (engine->class == COMPUTE_CLASS || engine->class == RENDER_CLASS) &&
>  	    IS_GFX_GT_IP_RANGE(engine->gt, IP_VER(12, 70), IP_VER(12, 71)))
>  		engine->flags |= I915_ENGINE_USES_WA_HOLD_SWITCHOUT;
>  
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index de43048543e8b..1004171d99943 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -134,6 +134,9 @@ i915_param_named_unsafe(lmem_size, uint, 0400,
>  i915_param_named_unsafe(lmem_bar_size, uint, 0400,
>  			"Set the lmem bar size(in MiB).");
>  
> +i915_param_named(enable_mtl_rcs_ccs_wa, bool, 0400,
> +	"Enable the RCS/CCS switchout hold workaround for MTL (only some workloads are affected by issue and w/a has a performance penalty) (default:false)");
> +
>  static void _param_print_bool(struct drm_printer *p, const char *name,
>  			      bool val)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 1315d7fac850f..971a765d74f56 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -62,6 +62,7 @@ struct drm_printer;
>  	param(unsigned int, lmem_size, 0, 0400) \
>  	param(unsigned int, lmem_bar_size, 0, 0400) \
>  	/* leave bools at the end to not create holes */ \
> +	param(bool, enable_mtl_rcs_ccs_wa, false, 0x400) \
>  	param(bool, enable_hangcheck, true, 0600) \
>  	param(bool, error_capture, true, IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) ? 0600 : 0) \
>  	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0)
> -- 
> 2.41.0
>