[PATCH] drm/i915: small semaphore fix

[PATCH] drm/i915: small semaphore fix

[Ben Widawsky]

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(-)

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);
-- 
1.7.6.1

Re: [PATCH] drm/i915: small semaphore fix

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 330 bytes --]

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.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: small semaphore fix

[Ben Widawsky]

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.

Re: [PATCH] drm/i915: small semaphore fix

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: small semaphore fix

[Ben Widawsky]

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