[Intel-gfx] [PATCH v2] drm/i915/gt: Temporarily force MTL into uncached mode

[John Harrison]

On 10/10/2023 07:36, Jonathan Cavitt wrote:
> FIXME: CAT errors are cropping up on MTL.  This removes them,
> but the real root cause must still be diagnosed.
I think 'hides' would be more accurate than 'removes'. At least until we 
have a better understanding of the issue.

Also, is there any performance penalty with this change? If we are going 
from fully cached to write combined then one assumes that something, 
somewhere is going to notice? Do we have any benchmark results or other 
tests that show an impact?

John.


>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> ---
>
> v2: Apply FIXME to shmem_utils as well.
>
>   drivers/gpu/drm/i915/gt/intel_gt.c     | 6 +++++-
>   drivers/gpu/drm/i915/gt/intel_lrc.c    | 5 ++++-
>   drivers/gpu/drm/i915/gt/shmem_utils.c  | 8 +++++++-
>   drivers/gpu/drm/i915/gt/uc/intel_guc.c | 5 ++++-
>   4 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index ed32bf5b15464..b52c8eb0b033f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -1026,8 +1026,12 @@ enum i915_map_type intel_gt_coherent_map_type(struct intel_gt *gt,
>   	/*
>   	 * Wa_22016122933: always return I915_MAP_WC for Media
>   	 * version 13.0 when the object is on the Media GT
> +	 *
> +	 * FIXME: CAT errors are cropping up on MTL.  This removes them,
> +	 * but the real root cause must still be diagnosed.
>   	 */
> -	if (i915_gem_object_is_lmem(obj) || intel_gt_needs_wa_22016122933(gt))
> +	if (i915_gem_object_is_lmem(obj) || intel_gt_needs_wa_22016122933(gt) ||
> +	    IS_METEORLAKE(gt->i915))
>   		return I915_MAP_WC;
>   	if (HAS_LLC(gt->i915) || always_coherent)
>   		return I915_MAP_WB;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index eaf66d9031665..8aaa4df84cb3e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1124,8 +1124,11 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine)
>   		 * Wa_22016122933: For Media version 13.0, all Media GT shared
>   		 * memory needs to be mapped as WC on CPU side and UC (PAT
>   		 * index 2) on GPU side.
> +		 *
> +		 * FIXME: CAT errors are cropping up on MTL.  This removes them,
> +		 * but the real root cause must still be diagnosed.
>   		 */
> -		if (intel_gt_needs_wa_22016122933(engine->gt))
> +		if (intel_gt_needs_wa_22016122933(engine->gt) || IS_METEORLAKE(engine->i915))
>   			i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE);
>   	}
>   
> diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c b/drivers/gpu/drm/i915/gt/shmem_utils.c
> index bccc3a1200bc6..a026c216fd286 100644
> --- a/drivers/gpu/drm/i915/gt/shmem_utils.c
> +++ b/drivers/gpu/drm/i915/gt/shmem_utils.c
> @@ -43,7 +43,13 @@ struct file *shmem_create_from_object(struct drm_i915_gem_object *obj)
>   		return file;
>   	}
>   
> -	map_type = i915_gem_object_is_lmem(obj) ? I915_MAP_WC : I915_MAP_WB;
> +	/*
> +	 * FIXME: CAT errors are cropping up on MTL.  This removes them,
> +	 * but the real root cause must still be diagnosed.
> +	 */
> +	map_type = i915_gem_object_is_lmem(obj) ||
> +		   IS_METEORLAKE(to_i915(obj->base.dev)) ?
> +		   I915_MAP_WC : I915_MAP_WB;
>   	ptr = i915_gem_object_pin_map_unlocked(obj, map_type);
>   	if (IS_ERR(ptr))
>   		return ERR_CAST(ptr);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 27df41c53b890..e3a7d61506188 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -774,8 +774,11 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
>   	 * Wa_22016122933: For Media version 13.0, all Media GT shared
>   	 * memory needs to be mapped as WC on CPU side and UC (PAT
>   	 * index 2) on GPU side.
> +	 *
> +	 * FIXME: CAT errors are cropping up on MTL.  This removes them,
> +	 * but the real root cause must still be diagnosed.
>   	 */
> -	if (intel_gt_needs_wa_22016122933(gt))
> +	if (intel_gt_needs_wa_22016122933(gt) || IS_METEORLAKE(gt->i915))
>   		i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE);
>   
>   	vma = i915_vma_instance(obj, &gt->ggtt->vm, NULL);