Re: [Intel-gfx] [PATCH 3/3] drm/i915/gt: Apply workaround 22016122933 correctly

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Jonathan,

On Wed, Jul 26, 2023 at 08:53:56AM -0700, Jonathan Cavitt wrote:
> WA_22016122933 was recently applied to all MeteorLake engines, which is
> simultaneously too broad (should only apply to Media engines) and too
> specific (should apply to all platforms that use the same media engine
> as MeteorLake).  Correct this in cases where coherency settings are
> modified.
> 
> There were also two additional places where the workaround was applied
> unconditionally.  The change was confirmed as necessary for all
> platforms, so the workaround label was removed.
> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Suggested-by: Matt Roper <matthew.d.roper@intel.com>

nitpick: the tag part is in chronological order. First Matt
suggested it and then you developed and sent.

> ---
>  drivers/gpu/drm/i915/gt/intel_gt.c        | 5 +++--
>  drivers/gpu/drm/i915/gt/intel_gt.h        | 6 ++++++
>  drivers/gpu/drm/i915/gt/intel_lrc.c       | 7 ++++---
>  drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c | 4 ----
>  drivers/gpu/drm/i915/gt/uc/intel_guc.c    | 7 ++++---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 4 ----
>  6 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 62eda0ab9bfc..b0f029f2380d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -1139,9 +1139,10 @@ enum i915_map_type intel_gt_coherent_map_type(struct intel_gt *gt,
>  					      bool always_coherent)
>  {
>  	/*
> -	 * Wa_22016122933: always return I915_MAP_WC for MTL
> +	 * Wa_22016122933: always return I915_MAP_WC for Media
> +	 * version 13.0 when the object is on the Media GT
>  	 */
> -	if (i915_gem_object_is_lmem(obj) || IS_METEORLAKE(gt->i915))
> +	if (i915_gem_object_is_lmem(obj) || intel_gt_needs_wa_22016122933(gt))
>  		return I915_MAP_WC;
>  	if (HAS_LLC(gt->i915) || always_coherent)
>  		return I915_MAP_WB;
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
> index adb442aaa522..2444ceb42b1b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
> @@ -6,6 +6,7 @@
>  #ifndef __INTEL_GT__
>  #define __INTEL_GT__
>  
> +#include "i915_drv.h"
>  #include "intel_engine_types.h"
>  #include "intel_gt_types.h"
>  #include "intel_reset.h"
> @@ -24,6 +25,11 @@ static inline bool gt_is_root(struct intel_gt *gt)
>  	return !gt->info.id;
>  }
>  
> +static inline bool intel_gt_needs_wa_22016122933(struct intel_gt *gt)
> +{
> +	return MEDIA_VER_FULL(gt->i915) == IP_VER(13, 0) && gt->type == GT_MEDIA;
> +}
> +

I like this format! I think we should make it a standard way to
handle workarounds.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> 

Andi

[...]