Re: [Intel-gfx] [resend] drm/i915: Check incoming alignment for unfenced buffers (on i915gm)

2017-11-01 Thread Chris Wilson
Quoting Joonas Lahtinen (2017-11-01 13:17:11)
> On Tue, 2017-10-31 at 10:36 +, Chris Wilson wrote:
> > In case the object has changed tiling between calls to execbuf, we need
> > to check if the existing offset inside the GTT matches the new tiling
> > constraint. We even need to do this for "unfenced" tiled objects, where
> > the 3D commands use an implied fence and so the object still needs to
> > match the physical fence restrictions on alignment (only required for
> > gen2 and early gen3).
> > 
> > In commit 2889caa92321 ("drm/i915: Eliminate lots of iterations over
> > the execobjects array"), the idea was to remove the second guessing and
> > only set the NEEDS_MAP flag when required. However, the entire check
> > for an unusable offset for fencing was removed and not just the
> > secondary check. I.e.
> > 
> >   /* avoid costly ping-pong once a batch bo ended up non-mappable */
> > if (entry->flags & __EXEC_OBJECT_NEEDS_MAP &&
> > !i915_vma_is_map_and_fenceable(vma))
> > return !only_mappable_for_reloc(entry->flags);
> > 
> > was entirely removed as the ping-pong between execbuf passes was fixed,
> > but its primary purpose in forcing unaligned unfenced access to be
> > rebound was forgotten.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103502
> > Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the 
> > execobjects array")
> > Signed-off-by: Chris Wilson 
> > Cc: Joonas Lahtinen 
> 
> Reviewed-by: Joonas Lahtinen 

Ta. So I'd been pondering how to catch this in CI. In theory, this could
be caught by gem_render_tiled_blit, but it firstly requires checking
that the rendercopy routines use the implicit fence instructions and
secondary we need lots of repetitions with interleaved set-tiling to try
and cause an address conflict. And then we have only one machine in the
farm that is even susceptible to this bug...
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [resend] drm/i915: Check incoming alignment for unfenced buffers (on i915gm)

2017-11-01 Thread Joonas Lahtinen
On Tue, 2017-10-31 at 10:36 +, Chris Wilson wrote:
> In case the object has changed tiling between calls to execbuf, we need
> to check if the existing offset inside the GTT matches the new tiling
> constraint. We even need to do this for "unfenced" tiled objects, where
> the 3D commands use an implied fence and so the object still needs to
> match the physical fence restrictions on alignment (only required for
> gen2 and early gen3).
> 
> In commit 2889caa92321 ("drm/i915: Eliminate lots of iterations over
> the execobjects array"), the idea was to remove the second guessing and
> only set the NEEDS_MAP flag when required. However, the entire check
> for an unusable offset for fencing was removed and not just the
> secondary check. I.e.
> 
>   /* avoid costly ping-pong once a batch bo ended up non-mappable */
> if (entry->flags & __EXEC_OBJECT_NEEDS_MAP &&
> !i915_vma_is_map_and_fenceable(vma))
> return !only_mappable_for_reloc(entry->flags);
> 
> was entirely removed as the ping-pong between execbuf passes was fixed,
> but its primary purpose in forcing unaligned unfenced access to be
> rebound was forgotten.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103502
> Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the 
> execobjects array")
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [resend] drm/i915: Check incoming alignment for unfenced buffers (on i915gm)

2017-10-31 Thread Chris Wilson
In case the object has changed tiling between calls to execbuf, we need
to check if the existing offset inside the GTT matches the new tiling
constraint. We even need to do this for "unfenced" tiled objects, where
the 3D commands use an implied fence and so the object still needs to
match the physical fence restrictions on alignment (only required for
gen2 and early gen3).

In commit 2889caa92321 ("drm/i915: Eliminate lots of iterations over
the execobjects array"), the idea was to remove the second guessing and
only set the NEEDS_MAP flag when required. However, the entire check
for an unusable offset for fencing was removed and not just the
secondary check. I.e.

/* avoid costly ping-pong once a batch bo ended up non-mappable */
if (entry->flags & __EXEC_OBJECT_NEEDS_MAP &&
!i915_vma_is_map_and_fenceable(vma))
return !only_mappable_for_reloc(entry->flags);

was entirely removed as the ping-pong between execbuf passes was fixed,
but its primary purpose in forcing unaligned unfenced access to be
rebound was forgotten.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103502
Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the 
execobjects array")
Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3d7190764f10..5b8213b1a8c7 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -343,6 +343,10 @@ eb_vma_misplaced(const struct drm_i915_gem_exec_object2 
*entry,
(vma->node.start + vma->node.size - 1) >> 32)
return true;
 
+   if (flags & __EXEC_OBJECT_NEEDS_MAP &&
+   !i915_vma_is_map_and_fenceable(vma))
+   return true;
+
return false;
 }
 
-- 
2.15.0.rc2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx