Re: [Intel-gfx] [PATCH v2] drm/i915/mtl: Media GT and Render GT share common GGTT

2022-11-09 Thread Jani Nikula
On Wed, 09 Nov 2022, Aravind Iddamsetty  wrote:
> On XE_LPM+ platforms the media engines are carved out into a separate
> GT but have a common GGTMMADR address range which essentially makes
> the GGTT address space to be shared between media and render GT. As a
> result any updates in GGTT shall invalidate TLB of GTs sharing it and
> similarly any operation on GGTT requiring an action on a GT will have to
> involve all GTs sharing it. setup_private_pat was being done on a per
> GGTT based as that doesn't touch any GGTT structures moved it to per GT
> based.
>
> BSPEC: 63834
>
> v2:
> 1. Add details to commit msg
> 2. includes fix for failure to add item to ggtt->gt_list, as suggested
> by Lucas
> 3. as ggtt_flush() is used only for ggtt drop i915_is_ggtt check within
> it.
> 4. setup_private_pat moved out of intel_gt_tiles_init
>
> Cc: Matt Roper 
> Signed-off-by: Aravind Iddamsetty 
> ---
>  drivers/gpu/drm/i915/gt/intel_ggtt.c  | 47 ++---
>  drivers/gpu/drm/i915/gt/intel_gt.c| 13 +-
>  drivers/gpu/drm/i915/gt/intel_gt_types.h  |  3 ++
>  drivers/gpu/drm/i915/gt/intel_gtt.h   |  4 ++
>  drivers/gpu/drm/i915/i915_driver.c| 22 +++---
>  drivers/gpu/drm/i915/i915_gem_evict.c | 51 +--
>  drivers/gpu/drm/i915/i915_vma.c   |  5 ++-
>  drivers/gpu/drm/i915/selftests/i915_gem.c |  2 +
>  8 files changed, 112 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
> b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index 2518cebbf931..6ba7e9e8e2ca 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -8,6 +8,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -196,10 +197,13 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
>  
>  void i915_ggtt_suspend(struct i915_ggtt *ggtt)
>  {
> + struct intel_gt *gt;
> +
>   i915_ggtt_suspend_vm(&ggtt->vm);
>   ggtt->invalidate(ggtt);
>  
> - intel_gt_check_and_clear_faults(ggtt->vm.gt);
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_check_and_clear_faults(gt);
>  }
>  
>  void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
> @@ -225,16 +229,21 @@ static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt)
>  
>  static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)
>  {
> - struct intel_uncore *uncore = ggtt->vm.gt->uncore;
>   struct drm_i915_private *i915 = ggtt->vm.i915;
>  
>   gen8_ggtt_invalidate(ggtt);
>  
> - if (GRAPHICS_VER(i915) >= 12)
> - intel_uncore_write_fw(uncore, GEN12_GUC_TLB_INV_CR,
> -   GEN12_GUC_TLB_INV_CR_INVALIDATE);
> - else
> - intel_uncore_write_fw(uncore, GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> + if (GRAPHICS_VER(i915) >= 12) {
> + struct intel_gt *gt;
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_uncore_write_fw(gt->uncore,
> +   GEN12_GUC_TLB_INV_CR,
> +   GEN12_GUC_TLB_INV_CR_INVALIDATE);
> + } else {
> + intel_uncore_write_fw(ggtt->vm.gt->uncore,
> +   GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> + }
>  }
>  
>  u64 gen8_ggtt_pte_encode(dma_addr_t addr,
> @@ -986,8 +995,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>  
>   ggtt->vm.pte_encode = gen8_ggtt_pte_encode;
>  
> - setup_private_pat(ggtt->vm.gt);
> -
>   return ggtt_probe_common(ggtt, size);
>  }
>  
> @@ -1186,7 +1193,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct 
> intel_gt *gt)
>   (u64)ggtt->mappable_end >> 20);
>   drm_dbg(&i915->drm, "DSM size = %lluM\n",
>   (u64)resource_size(&intel_graphics_stolen_res) >> 20);
> -
> + INIT_LIST_HEAD(&ggtt->gt_list);
>   return 0;
>  }
>  
> @@ -1208,6 +1215,19 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915)
>   return 0;
>  }
>  
> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915)
> +{
> + struct i915_ggtt *ggtt;
> +
> + ggtt = drmm_kzalloc(&i915->drm, sizeof(*ggtt), GFP_KERNEL);
> + if (!ggtt)
> + return ERR_PTR(-ENOMEM);
> +
> + INIT_LIST_HEAD(&ggtt->gt_list);
> +
> + return ggtt;
> +}
> +
>  int i915_ggtt_enable_hw(struct drm_i915_private *i915)
>  {
>   if (GRAPHICS_VER(i915) < 6)
> @@ -1296,9 +1316,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
>  
>  void i915_ggtt_resume(struct i915_ggtt *ggtt)
>  {
> + struct intel_gt *gt;
>   bool flush;
>  
> - intel_gt_check_and_clear_faults(ggtt->vm.gt);
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_check_and_clear_faults(gt);
>  
>   flush = i915_ggtt_resume_vm(&ggtt->vm);
>  
> @@ -1307,9 +1329,6 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt)
>   if (flush)
>   wbinvd_on_all_c

[PATCH v2] drm/i915/mtl: Media GT and Render GT share common GGTT

2022-11-09 Thread Aravind Iddamsetty
On XE_LPM+ platforms the media engines are carved out into a separate
GT but have a common GGTMMADR address range which essentially makes
the GGTT address space to be shared between media and render GT. As a
result any updates in GGTT shall invalidate TLB of GTs sharing it and
similarly any operation on GGTT requiring an action on a GT will have to
involve all GTs sharing it. setup_private_pat was being done on a per
GGTT based as that doesn't touch any GGTT structures moved it to per GT
based.

BSPEC: 63834

v2:
1. Add details to commit msg
2. includes fix for failure to add item to ggtt->gt_list, as suggested
by Lucas
3. as ggtt_flush() is used only for ggtt drop i915_is_ggtt check within
it.
4. setup_private_pat moved out of intel_gt_tiles_init

Cc: Matt Roper 
Signed-off-by: Aravind Iddamsetty 
---
 drivers/gpu/drm/i915/gt/intel_ggtt.c  | 47 ++---
 drivers/gpu/drm/i915/gt/intel_gt.c| 13 +-
 drivers/gpu/drm/i915/gt/intel_gt_types.h  |  3 ++
 drivers/gpu/drm/i915/gt/intel_gtt.h   |  4 ++
 drivers/gpu/drm/i915/i915_driver.c| 22 +++---
 drivers/gpu/drm/i915/i915_gem_evict.c | 51 +--
 drivers/gpu/drm/i915/i915_vma.c   |  5 ++-
 drivers/gpu/drm/i915/selftests/i915_gem.c |  2 +
 8 files changed, 112 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 2518cebbf931..6ba7e9e8e2ca 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -196,10 +197,13 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 
 void i915_ggtt_suspend(struct i915_ggtt *ggtt)
 {
+   struct intel_gt *gt;
+
i915_ggtt_suspend_vm(&ggtt->vm);
ggtt->invalidate(ggtt);
 
-   intel_gt_check_and_clear_faults(ggtt->vm.gt);
+   list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+   intel_gt_check_and_clear_faults(gt);
 }
 
 void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
@@ -225,16 +229,21 @@ static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt)
 
 static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)
 {
-   struct intel_uncore *uncore = ggtt->vm.gt->uncore;
struct drm_i915_private *i915 = ggtt->vm.i915;
 
gen8_ggtt_invalidate(ggtt);
 
-   if (GRAPHICS_VER(i915) >= 12)
-   intel_uncore_write_fw(uncore, GEN12_GUC_TLB_INV_CR,
- GEN12_GUC_TLB_INV_CR_INVALIDATE);
-   else
-   intel_uncore_write_fw(uncore, GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+   if (GRAPHICS_VER(i915) >= 12) {
+   struct intel_gt *gt;
+
+   list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+   intel_uncore_write_fw(gt->uncore,
+ GEN12_GUC_TLB_INV_CR,
+ GEN12_GUC_TLB_INV_CR_INVALIDATE);
+   } else {
+   intel_uncore_write_fw(ggtt->vm.gt->uncore,
+ GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+   }
 }
 
 u64 gen8_ggtt_pte_encode(dma_addr_t addr,
@@ -986,8 +995,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
ggtt->vm.pte_encode = gen8_ggtt_pte_encode;
 
-   setup_private_pat(ggtt->vm.gt);
-
return ggtt_probe_common(ggtt, size);
 }
 
@@ -1186,7 +1193,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct 
intel_gt *gt)
(u64)ggtt->mappable_end >> 20);
drm_dbg(&i915->drm, "DSM size = %lluM\n",
(u64)resource_size(&intel_graphics_stolen_res) >> 20);
-
+   INIT_LIST_HEAD(&ggtt->gt_list);
return 0;
 }
 
@@ -1208,6 +1215,19 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915)
return 0;
 }
 
+struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915)
+{
+   struct i915_ggtt *ggtt;
+
+   ggtt = drmm_kzalloc(&i915->drm, sizeof(*ggtt), GFP_KERNEL);
+   if (!ggtt)
+   return ERR_PTR(-ENOMEM);
+
+   INIT_LIST_HEAD(&ggtt->gt_list);
+
+   return ggtt;
+}
+
 int i915_ggtt_enable_hw(struct drm_i915_private *i915)
 {
if (GRAPHICS_VER(i915) < 6)
@@ -1296,9 +1316,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
 
 void i915_ggtt_resume(struct i915_ggtt *ggtt)
 {
+   struct intel_gt *gt;
bool flush;
 
-   intel_gt_check_and_clear_faults(ggtt->vm.gt);
+   list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+   intel_gt_check_and_clear_faults(gt);
 
flush = i915_ggtt_resume_vm(&ggtt->vm);
 
@@ -1307,9 +1329,6 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt)
if (flush)
wbinvd_on_all_cpus();
 
-   if (GRAPHICS_VER(ggtt->vm.i915) >= 8)
-   setup_private_pat(ggtt->vm.gt);
-
intel_ggtt_restore_fences(ggtt);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c 
b/drivers/gpu/drm/i915/gt/intel_gt