All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Mark untiled BLT commands as fenced on gen2/3
@ 2012-03-21 10:48 Chris Wilson
  2012-03-23 19:03 ` Daniel Vetter
  2012-03-23 20:49 ` Chris Wilson
  0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2012-03-21 10:48 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

The BLT commands on gen2/3 utilize the fence registers and so we cannot
modify any fences for the object whilst those commands are in flight.
Currently we marked tiled commands as occupying a fence, but forgot to
restrict the untiled commands from preventing a fence being assigned
before they were completed.

One side-effect is that we ten have to double check that a fence was
allocated for a fenced buffer during move-to-active.

Reported-by: Jiri Slaby <jirislaby@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43427
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@kernel.org
---
 drivers/gpu/drm/i915/i915_gem.c            |   15 +++++++++------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f79bd02..0918f02 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1656,16 +1656,19 @@ i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
 	list_move_tail(&obj->ring_list, &ring->active_list);
 
 	obj->last_rendering_seqno = seqno;
-	if (obj->fenced_gpu_access) {
-		struct drm_i915_fence_reg *reg;
-
-		BUG_ON(obj->fence_reg == I915_FENCE_REG_NONE);
 
+	if (obj->fenced_gpu_access) {
 		obj->last_fenced_seqno = seqno;
 		obj->last_fenced_ring = ring;
 
-		reg = &dev_priv->fence_regs[obj->fence_reg];
-		list_move_tail(&reg->lru_list, &dev_priv->mm.fence_list);
+		/* Bump MRU to take account of the delayed flush */
+		if (obj->fence_reg != I915_FENCE_REG_NONE) {
+			struct drm_i915_fence_reg *reg;
+
+			reg = &dev_priv->fence_regs[obj->fence_reg];
+			list_move_tail(&reg->lru_list,
+				       &dev_priv->mm.fence_list);
+		}
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 21fd5a4..9b33e10 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -498,8 +498,8 @@ pin_and_fence_object(struct drm_i915_gem_object *obj,
 				if (ret)
 					goto err_unpin;
 			}
+			obj->pending_fenced_gpu_access = true;
 		}
-		obj->pending_fenced_gpu_access = need_fence;
 	}
 
 	entry->offset = obj->gtt_offset;
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915: Mark untiled BLT commands as fenced on gen2/3
  2012-03-21 10:48 [PATCH] drm/i915: Mark untiled BLT commands as fenced on gen2/3 Chris Wilson
@ 2012-03-23 19:03 ` Daniel Vetter
  2012-03-28 11:20   ` Daniel Vetter
  2012-03-23 20:49 ` Chris Wilson
  1 sibling, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2012-03-23 19:03 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, stable

On Wed, Mar 21, 2012 at 10:48:18AM +0000, Chris Wilson wrote:
> The BLT commands on gen2/3 utilize the fence registers and so we cannot
> modify any fences for the object whilst those commands are in flight.
> Currently we marked tiled commands as occupying a fence, but forgot to
> restrict the untiled commands from preventing a fence being assigned
> before they were completed.
> 
> One side-effect is that we ten have to double check that a fence was
> allocated for a fenced buffer during move-to-active.
> 
> Reported-by: Jiri Slaby <jirislaby@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43427
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@kernel.org

I've had a few comments on irc about a preliminary version of this patch,
and Chris addressed them all. So

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Futhermore I've stitched together a little testcase in i-g-t, and it
indeed blows up without this patch. So

Testcase: i-g-t/tests/gem_tiled_after_untiled_blt
Tested-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915: Mark untiled BLT commands as fenced on gen2/3
  2012-03-21 10:48 [PATCH] drm/i915: Mark untiled BLT commands as fenced on gen2/3 Chris Wilson
  2012-03-23 19:03 ` Daniel Vetter
@ 2012-03-23 20:49 ` Chris Wilson
  1 sibling, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2012-03-23 20:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

On Wed, 21 Mar 2012 10:48:18 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> One side-effect is that we ten have to double check that a fence was
s/ten/then/
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915: Mark untiled BLT commands as fenced on gen2/3
  2012-03-23 19:03 ` Daniel Vetter
@ 2012-03-28 11:20   ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2012-03-28 11:20 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, stable

On Fri, Mar 23, 2012 at 08:03:23PM +0100, Daniel Vetter wrote:
> On Wed, Mar 21, 2012 at 10:48:18AM +0000, Chris Wilson wrote:
> > The BLT commands on gen2/3 utilize the fence registers and so we cannot
> > modify any fences for the object whilst those commands are in flight.
> > Currently we marked tiled commands as occupying a fence, but forgot to
> > restrict the untiled commands from preventing a fence being assigned
> > before they were completed.
> > 
> > One side-effect is that we ten have to double check that a fence was
> > allocated for a fenced buffer during move-to-active.
> > 
> > Reported-by: Jiri Slaby <jirislaby@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43427
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: stable@kernel.org
> 
> I've had a few comments on irc about a preliminary version of this patch,
> and Chris addressed them all. So
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Futhermore I've stitched together a little testcase in i-g-t, and it
> indeed blows up without this patch. So
> 
> Testcase: i-g-t/tests/gem_tiled_after_untiled_blt
> Tested-by: Daniel Vetter <daniel.vetter@ffwll.ch>

... and now also the report from our QA:

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47990
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-03-28 11:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-21 10:48 [PATCH] drm/i915: Mark untiled BLT commands as fenced on gen2/3 Chris Wilson
2012-03-23 19:03 ` Daniel Vetter
2012-03-28 11:20   ` Daniel Vetter
2012-03-23 20:49 ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.