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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 29, 2022 at 11:33:15AM +0530, Iddamsetty, Aravind wrote:


On 29-11-2022 11:24, Lucas De Marchi wrote:
On Wed, Nov 23, 2022 at 09:47:03AM +0530, Iddamsetty, Aravind wrote:


On 23-11-2022 05:29, Matt Roper wrote:
On Tue, Nov 22, 2022 at 12:31:26PM +0530, 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

v3:
1. Move out for_each_gt from i915_driver.c (Jani Nikula)

v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list
(Matt Roper)

Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxx>

Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>

Thanks Matt, could you also help with merging the change.

Regards,
Aravind.

---
 drivers/gpu/drm/i915/gt/intel_ggtt.c      | 54 +++++++++++++++++------
 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        | 12 ++---
 drivers/gpu/drm/i915/i915_gem.c           |  2 +
 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 +
 9 files changed, 111 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 8145851ad23d..7644738b9cdb 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -8,6 +8,7 @@
 #include <linux/types.h>
 #include <linux/stop_machine.h>

+#include <drm/drm_managed.h>
 #include <drm/i915_drm.h>
 #include <drm/intel-gtt.h>

@@ -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);
 }

@@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt
*ggtt, struct intel_gt *gt)
  */
 int i915_ggtt_probe_hw(struct drm_i915_private *i915)
 {
-    int ret;
+    struct intel_gt *gt;
+    int ret, i;
+
+    for_each_gt(gt, i915, i) {
+        ret = intel_gt_assign_ggtt(gt);

in v3 the intel_gt_assign_ggtt() call is not in i915_driver.c anymore but
rather moved here. We could make i915_ggtt_create() static, doing the
allocation here and intel_gt_assign_ggtt() would be in charge of just
assigning the ggtt. Not very important though and can be done later.

well we call intel_gt_assign_ggtt in i915_gem_gtt_mock_selftests but not
i915_ggtt_probe_hw.

makes sense, let's leave it as is.

Lucas De Marchi



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux