diff mbox series

[v3,2/2] drm/i915: Don't use BAR mappings for ring buffers with LLC

Message ID 20230216011101.1909009-3-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show
Series Don't use stolen memory or BAR mappings for ring buffers | expand

Commit Message

John Harrison Feb. 16, 2023, 1:11 a.m. UTC
From: John Harrison <John.C.Harrison@Intel.com>

Direction from hardware is that ring buffers should never be mapped
via the BAR on systems with LLC. There are too many caching pitfalls
due to the way BAR accesses are routed. So it is safest to just not
use it.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v4.9+
---
 drivers/gpu/drm/i915/gt/intel_ring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniele Ceraolo Spurio Feb. 17, 2023, 3:50 p.m. UTC | #1
On 2/15/2023 5:11 PM, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Direction from hardware is that ring buffers should never be mapped
> via the BAR on systems with LLC. There are too many caching pitfalls
> due to the way BAR accesses are routed. So it is safest to just not
> use it.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v4.9+

I've double-checked the original patches to make sure the 4.9 fixes tag 
was correct for both (which dim confirmed), because if we backport this 
fix without the previous one then the driver would break. Also, the 
original patches were merged as part of the same series 
(https://patchwork.freedesktop.org/series/11278/), so we should be 
guaranteed that they're always there as a pair.

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> ---
>   drivers/gpu/drm/i915/gt/intel_ring.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
> index fb1d2595392ed..fb99143be98e7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring.c
> @@ -53,7 +53,7 @@ int intel_ring_pin(struct intel_ring *ring, struct i915_gem_ww_ctx *ww)
>   	if (unlikely(ret))
>   		goto err_unpin;
>   
> -	if (i915_vma_is_map_and_fenceable(vma)) {
> +	if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915)) {
>   		addr = (void __force *)i915_vma_pin_iomap(vma);
>   	} else {
>   		int type = i915_coherent_map_type(vma->vm->i915, vma->obj, false);
> @@ -98,7 +98,7 @@ void intel_ring_unpin(struct intel_ring *ring)
>   		return;
>   
>   	i915_vma_unset_ggtt_write(vma);
> -	if (i915_vma_is_map_and_fenceable(vma))
> +	if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
>   		i915_vma_unpin_iomap(vma);
>   	else
>   		i915_gem_object_unpin_map(vma->obj);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
index fb1d2595392ed..fb99143be98e7 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -53,7 +53,7 @@  int intel_ring_pin(struct intel_ring *ring, struct i915_gem_ww_ctx *ww)
 	if (unlikely(ret))
 		goto err_unpin;
 
-	if (i915_vma_is_map_and_fenceable(vma)) {
+	if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915)) {
 		addr = (void __force *)i915_vma_pin_iomap(vma);
 	} else {
 		int type = i915_coherent_map_type(vma->vm->i915, vma->obj, false);
@@ -98,7 +98,7 @@  void intel_ring_unpin(struct intel_ring *ring)
 		return;
 
 	i915_vma_unset_ggtt_write(vma);
-	if (i915_vma_is_map_and_fenceable(vma))
+	if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
 		i915_vma_unpin_iomap(vma);
 	else
 		i915_gem_object_unpin_map(vma->obj);