diff mbox series

[v2] drm/i915: Avoid GGTT flushing on non-GGTT paths of i915_vma_pin_iomap

Message ID 20230725132946.1539075-1-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915: Avoid GGTT flushing on non-GGTT paths of i915_vma_pin_iomap | expand

Commit Message

Tvrtko Ursulin July 25, 2023, 1:29 p.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Commit 4bc91dbde0da ("drm/i915/lmem: Bypass aperture when lmem is available")
added a code path which does not map via GGTT, but was still setting the
ggtt write bit, and so triggering the GGTT flushing.

Fix it by not setting that bit unless the GGTT mapping path was used, and
replace the flush with wmb() in i915_vma_flush_writes().

This also works for the i915_gem_object_pin_map path added in
d976521a995a ("drm/i915: extend i915_vma_pin_iomap()").

It is hard to say if the fix has any observable effect, given that the
write-combine buffer gets flushed from intel_gt_flush_ggtt_writes too, but
apart from code clarity, skipping the needless GGTT flushing could be
beneficial on platforms with non-coherent GGTT. (See the code flow in
intel_gt_flush_ggtt_writes().)

v2:
 * Improve comment in i915_vma_flush_writes(). (Andi)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: 4bc91dbde0da ("drm/i915/lmem: Bypass aperture when lmem is available")
References: d976521a995a ("drm/i915: extend i915_vma_pin_iomap()")
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: <stable@vger.kernel.org> # v5.14+
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_vma.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index ffb425ba591c..7788b03b86d6 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -602,7 +602,9 @@  void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 	if (err)
 		goto err_unpin;
 
-	i915_vma_set_ggtt_write(vma);
+	if (!i915_gem_object_is_lmem(vma->obj) &&
+	    i915_vma_is_map_and_fenceable(vma))
+		i915_vma_set_ggtt_write(vma);
 
 	/* NB Access through the GTT requires the device to be awake. */
 	return page_mask_bits(ptr);
@@ -615,8 +617,19 @@  void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 
 void i915_vma_flush_writes(struct i915_vma *vma)
 {
+	/*
+	 * i915_vma_iomap() could have mapped the underlying memory in one
+	 * of the three ways, depending on which we have to choose the most
+	 * appropriate flushing mechanism.
+	 *
+	 * If the mapping method was via the aperture the appropriate flag will
+	 * be set via i915_vma_set_ggtt_write(), and if not then we know it is
+	 * enough to simply flush the CPU side write-combine buffer.
+	 */
 	if (i915_vma_unset_ggtt_write(vma))
 		intel_gt_flush_ggtt_writes(vma->vm->gt);
+	else
+		wmb();
 }
 
 void i915_vma_unpin_iomap(struct i915_vma *vma)