diff mbox

drm/i915: small semaphore fix

Message ID 1314820042-6169-1-git-send-email-ben@bwidawsk.net (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky Aug. 31, 2011, 7:47 p.m. UTC
Assertion + unsigned helps catch potential issues.

From the docs it is hard to tell if the global GTT flag is actually
needed, but it shouldn't hurt.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

Comments

Eric Anholt Sept. 1, 2011, 6:03 p.m. UTC | #1
On Wed, 31 Aug 2011 12:47:22 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> Assertion + unsigned helps catch potential issues.
> 
> From the docs it is hard to tell if the global GTT flag is actually
> needed, but it shouldn't hurt.

We're updating a register, not the GTT, so I don't see why the flag
would be relevant.
Ben Widawsky Sept. 2, 2011, 2:51 a.m. UTC | #2
On Thu, 01 Sep 2011 11:03:07 -0700
Eric Anholt <eric@anholt.net> wrote:

> On Wed, 31 Aug 2011 12:47:22 -0700, Ben Widawsky <ben@bwidawsk.net>
> wrote:
> > Assertion + unsigned helps catch potential issues.
> > 
> > From the docs it is hard to tell if the global GTT flag is actually
> > needed, but it shouldn't hurt.
> 
> We're updating a register, not the GTT, so I don't see why the flag
> would be relevant.

The patch is as (ir)relevant as without it which implies PPGTT. The
reason it was there is in another branch I was planning to implement
memory based semaphores.

I have the refactor patch coming soon, should probably ignore this.
Chris Wilson Sept. 2, 2011, 8:56 a.m. UTC | #3
On Thu, 1 Sep 2011 19:51:11 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> On Thu, 01 Sep 2011 11:03:07 -0700
> Eric Anholt <eric@anholt.net> wrote:
> 
> > On Wed, 31 Aug 2011 12:47:22 -0700, Ben Widawsky <ben@bwidawsk.net>
> > wrote:
> > > Assertion + unsigned helps catch potential issues.
> > > 
> > > From the docs it is hard to tell if the global GTT flag is actually
> > > needed, but it shouldn't hurt.
> > 
> > We're updating a register, not the GTT, so I don't see why the flag
> > would be relevant.
> 
> The patch is as (ir)relevant as without it which implies PPGTT. The
> reason it was there is in another branch I was planning to implement
> memory based semaphores.

Memory based semaphores required working ppgtt, afaict.
-Chris
Ben Widawsky Sept. 2, 2011, 2:18 p.m. UTC | #4
On Fri, Sep 02, 2011 at 09:56:31AM +0100, Chris Wilson wrote:
> On Thu, 1 Sep 2011 19:51:11 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > On Thu, 01 Sep 2011 11:03:07 -0700
> > Eric Anholt <eric@anholt.net> wrote:
> > 
> > > On Wed, 31 Aug 2011 12:47:22 -0700, Ben Widawsky <ben@bwidawsk.net>
> > > wrote:
> > > > Assertion + unsigned helps catch potential issues.
> > > > 
> > > > From the docs it is hard to tell if the global GTT flag is actually
> > > > needed, but it shouldn't hurt.
> > > 
> > > We're updating a register, not the GTT, so I don't see why the flag
> > > would be relevant.
> > 
> > The patch is as (ir)relevant as without it which implies PPGTT. The
> > reason it was there is in another branch I was planning to implement
> > memory based semaphores.
> 
> Memory based semaphores required working ppgtt, afaict.
> -Chris

Hmm, nothing in the docs suggested to me that it wouldn't work. I know for sure
it won't work on the simulator. So long as the semaphore commands are being
issued from a secure batch, I don't see why ppgtt *should* be a requirement. I
think the whole point of the gGTT flag is to allow inter-contexct
synchronization without needing to have identical ppgtt mappings.

I was planning to give it a shot today, but based on the simulator I suspect it
won't work.

Ben
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c30626e..eb1ab0c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -315,12 +315,14 @@  static void render_ring_cleanup(struct intel_ring_buffer *ring)
 }
 
 static void
-update_semaphore(struct intel_ring_buffer *ring, int i, u32 seqno)
+update_semaphore(struct intel_ring_buffer *ring, unsigned int i, u32 seqno)
 {
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int id;
 
+	BUG_ON(IS_GEN6(dev) && i > 1);
+
 	/*
 	 * cs -> 1 = vcs, 0 = bcs
 	 * vcs -> 1 = bcs, 0 = cs,
@@ -332,6 +334,7 @@  update_semaphore(struct intel_ring_buffer *ring, int i, u32 seqno)
 
 	intel_ring_emit(ring,
 			MI_SEMAPHORE_MBOX |
+			MI_SEMAPHORE_GLOBAL_GTT |
 			MI_SEMAPHORE_REGISTER |
 			MI_SEMAPHORE_UPDATE);
 	intel_ring_emit(ring, seqno);