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@xxxxxxxxx>
> Suggested-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
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@xxxxxxxxxxxxxxx>
Andi
[...]
