All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Widawsky <ben@bwidawsk.net>
To: intel-gfx@lists.freedesktop.org
Cc: Ben Widawsky <ben@bwidawsk.net>
Subject: [PATCH] drm/i915: Dumb down the semaphore logic
Date: Thu,  1 Sep 2011 20:55:35 -0700	[thread overview]
Message-ID: <1314935735-5451-1-git-send-email-ben@bwidawsk.net> (raw)

While I think the previous code is correct, it was hard to follow and
hard to debug. Since we already have a ring abstraction, might as well
use it to handle the semaphore updates and compares.

I don't expect this code to make semaphores better or worse, but you
never know...

Cc: Andrew Lutomirski <luto@mit.edu>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    3 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c    |  164 +++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    7 +-
 3 files changed, 119 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 4934cf8..3693e83 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -784,7 +784,8 @@ i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
 	}
 
 	from->sync_seqno[idx] = seqno;
-	return intel_ring_sync(to, from, seqno - 1);
+
+	return to->sync_to(to, from, seqno - 1);
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c30626e..c3d3906 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -313,81 +313,137 @@ static void render_ring_cleanup(struct intel_ring_buffer *ring)
 
 	cleanup_pipe_control(ring);
 }
+#define MBOX_UPDATE(ring, seqno) \
+		intel_ring_emit(ring, \
+				MI_SEMAPHORE_MBOX | \
+				MI_SEMAPHORE_GLOBAL_GTT | /* Should be ignored */ \
+				MI_SEMAPHORE_REGISTER | \
+				MI_SEMAPHORE_UPDATE); \
+		intel_ring_emit(ring, seqno)
 
-static void
-update_semaphore(struct intel_ring_buffer *ring, int i, u32 seqno)
-{
-	struct drm_device *dev = ring->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int id;
-
-	/*
-	 * cs -> 1 = vcs, 0 = bcs
-	 * vcs -> 1 = bcs, 0 = cs,
-	 * bcs -> 1 = cs, 0 = vcs.
-	 */
-	id = ring - dev_priv->ring;
-	id += 2 - i;
-	id %= 3;
-
-	intel_ring_emit(ring,
-			MI_SEMAPHORE_MBOX |
-			MI_SEMAPHORE_REGISTER |
-			MI_SEMAPHORE_UPDATE);
-	intel_ring_emit(ring, seqno);
-	intel_ring_emit(ring,
-			RING_SYNC_0(dev_priv->ring[id].mmio_base) + 4*i);
-}
-
-static int
-gen6_add_request(struct intel_ring_buffer *ring,
-		 u32 *result)
+static u32
+update_semaphore(struct intel_ring_buffer *ring,
+		 u32 first,
+		 u32 second)
 {
 	u32 seqno;
 	int ret;
+	seqno = i915_gem_get_seqno(ring->dev);
 
 	ret = intel_ring_begin(ring, 10);
 	if (ret)
 		return ret;
 
-	seqno = i915_gem_get_seqno(ring->dev);
-	update_semaphore(ring, 0, seqno);
-	update_semaphore(ring, 1, seqno);
-
+	MBOX_UPDATE(ring, seqno);
+	intel_ring_emit(ring, first);
+	MBOX_UPDATE(ring, seqno);
+	intel_ring_emit(ring, second);
 	intel_ring_emit(ring, MI_STORE_DWORD_INDEX);
 	intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT);
 	intel_ring_emit(ring, seqno);
 	intel_ring_emit(ring, MI_USER_INTERRUPT);
 	intel_ring_advance(ring);
 
-	*result = seqno;
+	return seqno;
+}
+
+static int
+gen6_blt_add_request(struct intel_ring_buffer *ring,
+		 u32 *result)
+{
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	*result = update_semaphore(ring,
+				   dev_priv->ring[RCS].mmio_base + 0x44,
+				   dev_priv->ring[VCS].mmio_base + 0x40);
 	return 0;
 }
 
-int
-intel_ring_sync(struct intel_ring_buffer *ring,
-		struct intel_ring_buffer *to,
-		u32 seqno)
+static int
+gen6_bsd_add_request(struct intel_ring_buffer *ring,
+		 u32 *result)
+{
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	*result = update_semaphore(ring,
+				   dev_priv->ring[RCS].mmio_base + 0x40,
+				   dev_priv->ring[BCS].mmio_base + 0x44);
+	return 0;
+}
+
+static int
+gen6_render_add_request(struct intel_ring_buffer *ring,
+		 u32 *result)
+{
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	*result = update_semaphore(ring,
+				   dev_priv->ring[VCS].mmio_base + 0x44,
+				   dev_priv->ring[BCS].mmio_base + 0x40);
+	return 0;
+}
+
+static int
+intel_ring_sync(struct intel_ring_buffer *comparer,
+		struct intel_ring_buffer *updater,
+		u32 seqno,
+		u32 semaphore_register)
 {
 	int ret;
+	u32 temp = MI_SEMAPHORE_MBOX |
+		   MI_SEMAPHORE_GLOBAL_GTT | /* Not needed */
+		   MI_SEMAPHORE_COMPARE;
 
-	ret = intel_ring_begin(ring, 4);
+	ret = intel_ring_begin(comparer, 4);
 	if (ret)
 		return ret;
 
-	intel_ring_emit(ring,
-			MI_SEMAPHORE_MBOX |
-			MI_SEMAPHORE_REGISTER |
-			intel_ring_sync_index(ring, to) << 17 |
-			MI_SEMAPHORE_COMPARE);
-	intel_ring_emit(ring, seqno);
-	intel_ring_emit(ring, 0);
-	intel_ring_emit(ring, MI_NOOP);
-	intel_ring_advance(ring);
+	temp |= MI_SEMAPHORE_REGISTER;
+
+	intel_ring_emit(comparer, temp | semaphore_register);
+	intel_ring_emit(comparer, seqno);
+	intel_ring_emit(comparer, 0);
+	intel_ring_emit(comparer, MI_NOOP);
+	intel_ring_advance(comparer);
 
 	return 0;
 }
 
+/* VCS->RCS (RVSYNC) or BCS->RCS (RBSYNC) */
+int
+render_ring_sync_to(struct intel_ring_buffer *comparer,
+		struct intel_ring_buffer *updater,
+		u32 seqno)
+{
+	WARN_ON(updater->semaphore_register[RCS] == 1);
+	return intel_ring_sync(comparer, updater, seqno,
+			       updater->semaphore_register[RCS]);
+}
+
+/* RCS->VCS (VRSYNC) or BCS->VCS (VBSYNC) */
+int
+gen6_bsd_ring_sync_to(struct intel_ring_buffer *comparer,
+		struct intel_ring_buffer *updater,
+		u32 seqno)
+{
+	WARN_ON(updater->semaphore_register[VCS] == 1);
+	return intel_ring_sync(comparer, updater, seqno,
+			       updater->semaphore_register[VCS]);
+}
+
+/* RCS->BCS (BRSYNC) or VCS->BCS (BVSYNC) */
+int
+gen6_blt_ring_sync_to(struct intel_ring_buffer *comparer,
+		struct intel_ring_buffer *updater,
+		u32 seqno)
+{
+	WARN_ON(updater->semaphore_register[BCS] == 1);
+	return intel_ring_sync(comparer, updater, seqno,
+			       updater->semaphore_register[BCS]);
+}
+
+
+
 #define PIPE_CONTROL_FLUSH(ring__, addr__)					\
 do {									\
 	intel_ring_emit(ring__, GFX_OP_PIPE_CONTROL | PIPE_CONTROL_QW_WRITE |		\
@@ -1027,6 +1083,8 @@ static const struct intel_ring_buffer render_ring = {
 	.irq_put		= render_ring_put_irq,
 	.dispatch_execbuffer	= render_ring_dispatch_execbuffer,
        .cleanup			= render_ring_cleanup,
+	.sync_to		= render_ring_sync_to,
+	.semaphore_register	= {1, 2 << 16, 0 << 16}, /* invalid, RVSYNC, RBSYNC */
 };
 
 /* ring buffer for bit-stream decoder */
@@ -1149,11 +1207,13 @@ static const struct intel_ring_buffer gen6_bsd_ring = {
 	.init			= init_ring_common,
 	.write_tail		= gen6_bsd_ring_write_tail,
 	.flush			= gen6_ring_flush,
-	.add_request		= gen6_add_request,
+	.add_request		= gen6_bsd_add_request,
 	.get_seqno		= ring_get_seqno,
 	.irq_get		= gen6_bsd_ring_get_irq,
 	.irq_put		= gen6_bsd_ring_put_irq,
 	.dispatch_execbuffer	= gen6_ring_dispatch_execbuffer,
+	.sync_to		= gen6_bsd_ring_sync_to,
+	.semaphore_register	= {0 << 16, 1, 2 << 16}, /* VRSYNC, 0, VBSYNC */
 };
 
 /* Blitter support (SandyBridge+) */
@@ -1279,12 +1339,14 @@ static const struct intel_ring_buffer gen6_blt_ring = {
        .init			= blt_ring_init,
        .write_tail		= ring_write_tail,
        .flush			= blt_ring_flush,
-       .add_request		= gen6_add_request,
+       .add_request		= gen6_blt_add_request,
        .get_seqno		= ring_get_seqno,
        .irq_get			= blt_ring_get_irq,
        .irq_put			= blt_ring_put_irq,
        .dispatch_execbuffer	= gen6_ring_dispatch_execbuffer,
        .cleanup			= blt_ring_cleanup,
+	.sync_to		= gen6_blt_ring_sync_to,
+	.semaphore_register	= {2 << 16, 0 << 16, 1}, /* BRSYNC, BVSYNC, 0 */
 };
 
 int intel_init_render_ring_buffer(struct drm_device *dev)
@@ -1294,7 +1356,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 
 	*ring = render_ring;
 	if (INTEL_INFO(dev)->gen >= 6) {
-		ring->add_request = gen6_add_request;
+		ring->add_request = gen6_render_add_request;
 		ring->irq_get = gen6_render_ring_get_irq;
 		ring->irq_put = gen6_render_ring_put_irq;
 	} else if (IS_GEN5(dev)) {
@@ -1317,7 +1379,7 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size)
 
 	*ring = render_ring;
 	if (INTEL_INFO(dev)->gen >= 6) {
-		ring->add_request = gen6_add_request;
+		ring->add_request = gen6_render_add_request;
 		ring->irq_get = gen6_render_ring_get_irq;
 		ring->irq_put = gen6_render_ring_put_irq;
 	} else if (IS_GEN5(dev)) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 39ac2b6..98052fd 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -75,7 +75,11 @@ struct  intel_ring_buffer {
 	int		(*dispatch_execbuffer)(struct intel_ring_buffer *ring,
 					       u32 offset, u32 length);
 	void		(*cleanup)(struct intel_ring_buffer *ring);
+	int		(*sync_to)(struct intel_ring_buffer *ring,
+				   struct intel_ring_buffer *to,
+				   u32 seqno);
 
+	u32		semaphore_register[3];
 	/**
 	 * List of objects currently involved in rendering from the
 	 * ringbuffer.
@@ -180,9 +184,6 @@ static inline void intel_ring_emit(struct intel_ring_buffer *ring,
 void intel_ring_advance(struct intel_ring_buffer *ring);
 
 u32 intel_ring_get_seqno(struct intel_ring_buffer *ring);
-int intel_ring_sync(struct intel_ring_buffer *ring,
-		    struct intel_ring_buffer *to,
-		    u32 seqno);
 
 int intel_init_render_ring_buffer(struct drm_device *dev);
 int intel_init_bsd_ring_buffer(struct drm_device *dev);
-- 
1.7.6.1

             reply	other threads:[~2011-09-02  3:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-02  3:55 Ben Widawsky [this message]
2011-09-02  8:54 ` [PATCH] drm/i915: Dumb down the semaphore logic Chris Wilson
2011-09-02 14:02   ` Ben Widawsky
2011-09-02 19:10 ` Eric Anholt
2011-09-03 20:09   ` Ben Widawsky
2011-09-04  2:55     ` Keith Packard
2011-09-05  3:52 Ben Widawsky
2011-09-05  6:54 ` Ben Widawsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1314935735-5451-1-git-send-email-ben@bwidawsk.net \
    --to=ben@bwidawsk.net \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.