[Intel-xe] [PATCH v2 7/8] drm/xe: Invalidate TLB on all affected GTs during GGTT updates

[Lucas De Marchi]

On Fri, Apr 14, 2023 at 02:09:04PM -0700, Lucas De Marchi wrote:
>On Wed, Apr 12, 2023 at 03:52:47PM -0700, Matt Roper wrote:
>>Platforms with a standalone media GT share a single GGTT between the
>>primary and media GTs.  However each of these GTs has its own TLBs
>>caching the page table, and each needs to be invalidated separately.
>>Convert ggtt->gt into a list of GTs so that we can iterate and
>>invalidate them all.
>>
>>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>
>it's not clear to me if the call to xe_ggtt_invalidate() in
>drivers/gpu/drm/xe/display/xe_fb_pin.c needs to consider all GTs or if
>it's ok to keep that one on the first GT only.  +Maarten

which leads me to wonder if xe_ggtt_invalidate() shouldn't actually
receive struct xe_ggtt as argument and let the loop happen inside it.
AFAICS this is also the behavior in i915: guc_ggtt_invalidate().

Lucas De Marchi

>
>Anyway, it should be done with one additional fixup commit due to the
>way we are moving display up on rebases.
>
>
>Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
>
>
>Lucas De Marchi
>
>>---
>>drivers/gpu/drm/xe/xe_ggtt.c       | 15 +++++++++++----
>>drivers/gpu/drm/xe/xe_ggtt_types.h |  2 +-
>>drivers/gpu/drm/xe/xe_gt.c         |  4 ++++
>>drivers/gpu/drm/xe/xe_gt_types.h   |  2 ++
>>4 files changed, 18 insertions(+), 5 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
>>index 0ae373b604f6..374830a7a0cf 100644
>>--- a/drivers/gpu/drm/xe/xe_ggtt.c
>>+++ b/drivers/gpu/drm/xe/xe_ggtt.c
>>@@ -98,7 +98,7 @@ int xe_ggtt_init_noalloc(struct xe_gt *gt, struct xe_ggtt *ggtt)
>>
>>	XE_BUG_ON(xe_gt_is_media_type(gt));
>>
>>-	ggtt->gt = gt;
>>+	list_add_tail(&gt->mem.ggtt_link, &ggtt->gt_list);
>>
>>	gsm_size = probe_gsm_size(pdev);
>>	if (gsm_size == 0) {
>>@@ -139,6 +139,7 @@ int xe_ggtt_init_noalloc(struct xe_gt *gt, struct xe_ggtt *ggtt)
>>static void xe_ggtt_initial_clear(struct xe_ggtt *ggtt)
>>{
>>	struct drm_mm_node *hole;
>>+	struct xe_gt *gt;
>>	u64 start, end;
>>
>>	/* Display may have allocated inside ggtt, so be careful with clearing here */
>>@@ -146,7 +147,8 @@ static void xe_ggtt_initial_clear(struct xe_ggtt *ggtt)
>>	drm_mm_for_each_hole(hole, &ggtt->mm, start, end)
>>		xe_ggtt_clear(ggtt, start, end - start);
>>
>>-	xe_ggtt_invalidate(ggtt->gt);
>>+	list_for_each_entry(gt, &ggtt->gt_list, mem.ggtt_link)
>>+		xe_ggtt_invalidate(gt);
>>	mutex_unlock(&ggtt->lock);
>>}
>>
>>@@ -263,6 +265,7 @@ int xe_ggtt_insert_special_node(struct xe_ggtt *ggtt, struct drm_mm_node *node,
>>
>>void xe_ggtt_map_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>>{
>>+	struct xe_gt *gt;
>>	u64 start = bo->ggtt_node.start;
>>	u64 offset, pte;
>>
>>@@ -271,7 +274,8 @@ void xe_ggtt_map_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>>		xe_ggtt_set_pte(ggtt, start + offset, pte);
>>	}
>>
>>-	xe_ggtt_invalidate(ggtt->gt);
>>+	list_for_each_entry(gt, &ggtt->gt_list, mem.ggtt_link)
>>+		xe_ggtt_invalidate(gt);
>>}
>>
>>static int __xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
>>@@ -316,13 +320,16 @@ int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>>
>>void xe_ggtt_remove_node(struct xe_ggtt *ggtt, struct drm_mm_node *node)
>>{
>>+	struct xe_gt *gt;
>>+
>>	mutex_lock(&ggtt->lock);
>>
>>	xe_ggtt_clear(ggtt, node->start, node->size);
>>	drm_mm_remove_node(node);
>>	node->size = 0;
>>
>>-	xe_ggtt_invalidate(ggtt->gt);
>>+	list_for_each_entry(gt, &ggtt->gt_list, mem.ggtt_link)
>>+		xe_ggtt_invalidate(gt);
>>
>>	mutex_unlock(&ggtt->lock);
>>}
>>diff --git a/drivers/gpu/drm/xe/xe_ggtt_types.h b/drivers/gpu/drm/xe/xe_ggtt_types.h
>>index ea70aaef4b31..fd527179f81b 100644
>>--- a/drivers/gpu/drm/xe/xe_ggtt_types.h
>>+++ b/drivers/gpu/drm/xe/xe_ggtt_types.h
>>@@ -12,7 +12,7 @@ struct xe_bo;
>>struct xe_gt;
>>
>>struct xe_ggtt {
>>-	struct xe_gt *gt;
>>+	struct list_head gt_list;
>>
>>	u64 size;
>>
>>diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>>index 4186f7f0d42f..a07d17af3db0 100644
>>--- a/drivers/gpu/drm/xe/xe_gt.c
>>+++ b/drivers/gpu/drm/xe/xe_gt.c
>>@@ -71,6 +71,8 @@ int xe_gt_alloc(struct xe_device *xe, struct xe_gt *gt)
>>		if (!gt->mem.ggtt)
>>			return -ENOMEM;
>>
>>+		INIT_LIST_HEAD(&gt->mem.ggtt->gt_list);
>>+
>>		gt->mem.vram_mgr = drmm_kzalloc(drm, sizeof(*gt->mem.vram_mgr),
>>						GFP_KERNEL);
>>		if (!gt->mem.vram_mgr)
>>@@ -81,6 +83,8 @@ int xe_gt_alloc(struct xe_device *xe, struct xe_gt *gt)
>>
>>		gt->mem.ggtt = full_gt->mem.ggtt;
>>		gt->mem.vram_mgr = full_gt->mem.vram_mgr;
>>+
>>+		list_add_tail(&gt->mem.ggtt_link, &full_gt->mem.ggtt->gt_list);
>>	}
>>
>>	gt->ordered_wq = alloc_ordered_workqueue("gt-ordered-wq", 0);
>>diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
>>index 7c47d67aa8be..1eff57f43731 100644
>>--- a/drivers/gpu/drm/xe/xe_gt_types.h
>>+++ b/drivers/gpu/drm/xe/xe_gt_types.h
>>@@ -164,6 +164,8 @@ struct xe_gt {
>>		struct xe_ttm_vram_mgr *vram_mgr;
>>		/** @ggtt: Global graphics translation table */
>>		struct xe_ggtt *ggtt;
>>+		/** @ggtt_link: node in GGTT's GT list */
>>+		struct list_head ggtt_link;
>>	} mem;
>>
>>	/** @reset: state for GT resets */
>>-- 
>>2.39.2
>>