[Intel-gfx] [PATCH dii-client 2/2] drm/i915/gt: Apply workaround 22016122933 correctly

2023-07-25 Thread Jonathan Cavitt 
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 
Suggested-by: Matt Roper 
---
 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 6faf1dae965f..207bfc0ff939 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;
+}
+
 static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)
 {
return container_of(uc, struct intel_gt, uc);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index e5a83d4932c8..9f0a2d828a2a 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1095,10 +1095,11 @@ __lrc_alloc_state(struct intel_context *ce, struct 
intel_engine_cs *engine)
if (IS_ERR(obj)) {
obj = i915_gem_object_create_shmem(engine->i915, context_size);
/*
-* Wa_22016122933: For MTL the shared memory needs to be mapped
-* as WC on CPU side and UC (PAT index 2) on GPU side
+* 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.
 */
-   if (IS_METEORLAKE(engine->i915))
+   if (intel_gt_needs_wa_22016122933(engine->gt))
i915_gem_object_set_cache_coherency(obj, 
I915_CACHE_NONE);
}
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c 
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
index 6efb86c93bfc..52652a0350c6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
@@ -284,10 +284,6 @@ static int gsc_fw_load_prepare(struct intel_gsc_uc *gsc)
memcpy_toio(gsc->local_vaddr, src, gsc->fw.size);
memset_io(gsc->local_vaddr + gsc->fw.size, 0, gsc->local->size - 
gsc->fw.size);
 
-   /*
-* Wa_22016122933: Making sure the data in dst is
-* visible to GSC right away
-*/
intel_guc_write_barrier(>->uc.guc);
 
i915_gem_object_unpin_map(gsc->fw.obj);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index c0fa9d232205..63bdc000d76b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -745,10 +745,11 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc 
*guc, u32 size)
return ERR_CAST(obj);
 
/*
-* Wa_22016122933: For MTL the shared memory needs to be mapped
-* as WC on CPU side and UC (PAT index 2) on GPU side
+* 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.
 */
-   if (IS_METEORLAKE(gt->i915))
+   if (intel_gt_needs_wa_2201612293

[Intel-gfx] [PATCH dii-client 2/2] drm/i915/gt: Apply workaround 22016122933 correctly

2023-07-21 Thread Jonathan Cavitt 
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 
Suggested-by: Matt Roper 
---
 drivers/gpu/drm/i915/gem/i915_gem_pages.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/gem/i915_gem_pages.c 
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 44d93ead96ff..4acdd008d1d3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -470,9 +470,10 @@ enum i915_map_type i915_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 d2f4fbde5f9f..4eb41a3b6e8b 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;
+}
+
 static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)
 {
return container_of(uc, struct intel_gt, uc);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index c45e6d8cbaac..668ed3fc7076 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1095,10 +1095,11 @@ __lrc_alloc_state(struct intel_context *ce, struct 
intel_engine_cs *engine)
if (IS_ERR(obj)) {
obj = i915_gem_object_create_shmem(engine->i915, context_size);
/*
-* Wa_22016122933: For MTL the shared memory needs to be mapped
-* as WC on CPU side and UC (PAT index 2) on GPU side
+* 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.
 */
-   if (IS_METEORLAKE(engine->i915))
+   if (intel_gt_needs_wa_22016122933(engine->gt))
i915_gem_object_set_cache_coherency(obj, 
I915_CACHE_NONE);
}
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c 
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
index fb4933543f31..1093b47d3e06 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
@@ -284,10 +284,6 @@ static int gsc_fw_load_prepare(struct intel_gsc_uc *gsc)
memcpy_toio(gsc->local_vaddr, src, gsc->fw.size);
memset_io(gsc->local_vaddr + gsc->fw.size, 0, gsc->local->size - 
gsc->fw.size);
 
-   /*
-* Wa_22016122933: Making sure the data in dst is
-* visible to GSC right away
-*/
intel_guc_write_barrier(>->uc.guc);
 
i915_gem_object_unpin_map(gsc->fw.obj);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index effb37727093..846f6029 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -745,10 +745,11 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc 
*guc, u32 size)
return ERR_CAST(obj);
 
/*
-* Wa_22016122933: For MTL the shared memory needs to be mapped
-* as WC on CPU side and UC (PAT index 2) on GPU side
+* 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.
 */
-   if (IS_METEORLAKE(gt->i915))
+   if (intel_gt_n

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

2023-07-25 Thread Yang, Fei 
> Subject: [PATCH dii-client 2/2] drm/i915/gt: Apply workaround 22016122933 
> correctly

Remove dii-client from the subject.
Otherwise LGTM.

Acked-by: Fei Yang 

> 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 
> Suggested-by: Matt Roper 
> ---
>  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 6faf1dae965f..207bfc0ff939 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; }
> +
>  static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)  {
>   return container_of(uc, struct intel_gt, uc); diff --git 
> a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index e5a83d4932c8..9f0a2d828a2a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1095,10 +1095,11 @@ __lrc_alloc_state(struct intel_context *ce, struct 
> intel_engine_cs *engine)
>   if (IS_ERR(obj)) {
>   obj = i915_gem_object_create_shmem(engine->i915, context_size);
>   /*
> -  * Wa_22016122933: For MTL the shared memory needs to be mapped
> -  * as WC on CPU side and UC (PAT index 2) on GPU side
> +  * 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.
>*/
> - if (IS_METEORLAKE(engine->i915))
> + if (intel_gt_needs_wa_22016122933(engine->gt))
>   i915_gem_object_set_cache_coherency(obj, 
> I915_CACHE_NONE);
>   }
>   if (IS_ERR(obj))
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> index 6efb86c93bfc..52652a0350c6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> @@ -284,10 +284,6 @@ static int gsc_fw_load_prepare(struct intel_gsc_uc *gsc)
>   memcpy_toio(gsc->local_vaddr, src, gsc->fw.size);
>   memset_io(gsc->local_vaddr + gsc->fw.size, 0, gsc->local->size - 
> gsc->fw.size);
>
> - /*
> -  * Wa_22016122933: Making sure the data in dst is
> -  * visible to GSC right away
> -  */
>   intel_guc_write_barrier(>->uc.guc);
>
>   i915_gem_object_unpin_map(gsc->fw.obj);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index c0fa9d232205..63bdc000d76b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -745,10 +745,11 @@ struct i915_vma *intel_guc_allocate_vma(struct 
> intel_guc *guc, u32 size)
>   return ERR_CAST(obj);
>
>   /*
> -  * Wa_22016122933: For MTL the shared memory needs to be mapped
> -  * as WC on CPU side

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

2023-07-21 Thread Yang, Fei 
> 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 
> Suggested-by: Matt Roper 

Reviewed-by: Fei Yang 

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_pages.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/gem/i915_gem_pages.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> index 44d93ead96ff..4acdd008d1d3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> @@ -470,9 +470,10 @@ enum i915_map_type i915_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 d2f4fbde5f9f..4eb41a3b6e8b 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; }
> +
>  static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)  {
>   return container_of(uc, struct intel_gt, uc);
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index c45e6d8cbaac..668ed3fc7076 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1095,10 +1095,11 @@ __lrc_alloc_state(struct intel_context *ce, struct 
> intel_engine_cs *engine)
>   if (IS_ERR(obj)) {
>   obj = i915_gem_object_create_shmem(engine->i915, context_size);
>   /*
> -  * Wa_22016122933: For MTL the shared memory needs to be mapped
> -  * as WC on CPU side and UC (PAT index 2) on GPU side
> +  * 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.
>*/
> - if (IS_METEORLAKE(engine->i915))
> + if (intel_gt_needs_wa_22016122933(engine->gt))
>   i915_gem_object_set_cache_coherency(obj, 
> I915_CACHE_NONE);
>   }
>   if (IS_ERR(obj))
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> index fb4933543f31..1093b47d3e06 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> @@ -284,10 +284,6 @@ static int gsc_fw_load_prepare(struct intel_gsc_uc *gsc)
>   memcpy_toio(gsc->local_vaddr, src, gsc->fw.size);
>   memset_io(gsc->local_vaddr + gsc->fw.size, 0, gsc->local->size - 
> gsc->fw.size);
>
> - /*
> -  * Wa_22016122933: Making sure the data in dst is
> -  * visible to GSC right away
> -  */
>   intel_guc_write_barrier(>->uc.guc);
>
>   i915_gem_object_unpin_map(gsc->fw.obj);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index effb37727093..846f6029 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -745,10 +745,11 @@ struct i915_vma *intel_guc_allocate_vma(struct 
> intel_guc *guc, u32 size)
>   return ERR_CAST(obj);
>
>   /*
> -  * Wa_22016122933: For MTL the shared memory needs to be mapped
> -  * as WC on CPU side and UC (PAT index 2) on GPU side
> +  * Wa_22016122933: For Media version 13.0, all Media GT shared
> +  * mem

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

2023-07-24 Thread Tvrtko Ursulin 



On 21/07/2023 15:05, 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 
Suggested-by: Matt Roper 
---
  drivers/gpu/drm/i915/gem/i915_gem_pages.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/gem/i915_gem_pages.c 
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 44d93ead96ff..4acdd008d1d3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -470,9 +470,10 @@ enum i915_map_type i915_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 d2f4fbde5f9f..4eb41a3b6e8b 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;


Ah this is something Matt was recntly telling me is coming.

Would it make sense to store ipver in the gt? And then we could have:

return gt->type == GT_MEDIA && gt->info.ipver == IP_VER(13,0);

?

(Give or take the gt->info part, and macro or not.)

Reads a bit better to me to ask the same component about it's type and 
version.


Regards,

Tvrtko


+}
+
  static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)
  {
return container_of(uc, struct intel_gt, uc);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index c45e6d8cbaac..668ed3fc7076 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1095,10 +1095,11 @@ __lrc_alloc_state(struct intel_context *ce, struct 
intel_engine_cs *engine)
if (IS_ERR(obj)) {
obj = i915_gem_object_create_shmem(engine->i915, context_size);
/*
-* Wa_22016122933: For MTL the shared memory needs to be mapped
-* as WC on CPU side and UC (PAT index 2) on GPU side
+* 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.
 */
-   if (IS_METEORLAKE(engine->i915))
+   if (intel_gt_needs_wa_22016122933(engine->gt))
i915_gem_object_set_cache_coherency(obj, 
I915_CACHE_NONE);
}
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c 
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
index fb4933543f31..1093b47d3e06 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
@@ -284,10 +284,6 @@ static int gsc_fw_load_prepare(struct intel_gsc_uc *gsc)
memcpy_toio(gsc->local_vaddr, src, gsc->fw.size);
memset_io(gsc->local_vaddr + gsc->fw.size, 0, gsc->local->size - 
gsc->fw.size);
  
-	/*

-* Wa_22016122933: Making sure the data in dst is
-* visible to GSC right away
-*/
intel_guc_write_barrier(>->uc.guc);
  
  	i915_gem_object_unpin_map(gsc->fw.obj);

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index effb37727093..846f6029 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -745,10 +745,11 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc 
*guc, u32 size)