[PATCH v3 1/5] drm/i915: Introduce ring set_seqno

[PATCH v3 1/5] drm/i915: Introduce ring set_seqno

[Mika Kuoppala]

In preparation for setting per ring initial seqno values
add ring::set_seqno().

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   20 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h |    9 +++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 69bbe7b..f536a99 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -727,6 +727,12 @@ ring_get_seqno(struct intel_ring_buffer *ring, bool lazy_coherency)
 	return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
 }
 
+static void
+ring_set_seqno(struct intel_ring_buffer *ring, u32 seqno)
+{
+	intel_write_status_page(ring, I915_GEM_HWS_INDEX, seqno);
+}
+
 static u32
 pc_render_get_seqno(struct intel_ring_buffer *ring, bool lazy_coherency)
 {
@@ -734,6 +740,13 @@ pc_render_get_seqno(struct intel_ring_buffer *ring, bool lazy_coherency)
 	return pc->cpu_page[0];
 }
 
+static void
+pc_render_set_seqno(struct intel_ring_buffer *ring, u32 seqno)
+{
+	struct pipe_control *pc = ring->private;
+	pc->cpu_page[0] = seqno;
+}
+
 static bool
 gen5_ring_get_irq(struct intel_ring_buffer *ring)
 {
@@ -1602,6 +1615,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->irq_put = gen6_ring_put_irq;
 		ring->irq_enable_mask = GT_USER_INTERRUPT;
 		ring->get_seqno = gen6_ring_get_seqno;
+		ring->set_seqno = ring_set_seqno;
 		ring->sync_to = gen6_ring_sync;
 		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_INVALID;
 		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_RV;
@@ -1612,6 +1626,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->add_request = pc_render_add_request;
 		ring->flush = gen4_render_ring_flush;
 		ring->get_seqno = pc_render_get_seqno;
+		ring->set_seqno = pc_render_set_seqno;
 		ring->irq_get = gen5_ring_get_irq;
 		ring->irq_put = gen5_ring_put_irq;
 		ring->irq_enable_mask = GT_USER_INTERRUPT | GT_PIPE_NOTIFY;
@@ -1622,6 +1637,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		else
 			ring->flush = gen4_render_ring_flush;
 		ring->get_seqno = ring_get_seqno;
+		ring->set_seqno = ring_set_seqno;
 		if (IS_GEN2(dev)) {
 			ring->irq_get = i8xx_ring_get_irq;
 			ring->irq_put = i8xx_ring_put_irq;
@@ -1672,6 +1688,7 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size)
 	else
 		ring->flush = gen4_render_ring_flush;
 	ring->get_seqno = ring_get_seqno;
+	ring->set_seqno = ring_set_seqno;
 	if (IS_GEN2(dev)) {
 		ring->irq_get = i8xx_ring_get_irq;
 		ring->irq_put = i8xx_ring_put_irq;
@@ -1732,6 +1749,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->flush = gen6_ring_flush;
 		ring->add_request = gen6_add_request;
 		ring->get_seqno = gen6_ring_get_seqno;
+		ring->set_seqno = ring_set_seqno;
 		ring->irq_enable_mask = GEN6_BSD_USER_INTERRUPT;
 		ring->irq_get = gen6_ring_get_irq;
 		ring->irq_put = gen6_ring_put_irq;
@@ -1747,6 +1765,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->flush = bsd_ring_flush;
 		ring->add_request = i9xx_add_request;
 		ring->get_seqno = ring_get_seqno;
+		ring->set_seqno = ring_set_seqno;
 		if (IS_GEN5(dev)) {
 			ring->irq_enable_mask = GT_BSD_USER_INTERRUPT;
 			ring->irq_get = gen5_ring_get_irq;
@@ -1776,6 +1795,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	ring->flush = blt_ring_flush;
 	ring->add_request = gen6_add_request;
 	ring->get_seqno = gen6_ring_get_seqno;
+	ring->set_seqno = ring_set_seqno;
 	ring->irq_enable_mask = GEN6_BLITTER_USER_INTERRUPT;
 	ring->irq_get = gen6_ring_get_irq;
 	ring->irq_put = gen6_ring_put_irq;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b4a533e..4a7cd67 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -79,6 +79,8 @@ struct  intel_ring_buffer {
 	 */
 	u32		(*get_seqno)(struct intel_ring_buffer *ring,
 				     bool lazy_coherency);
+	void		(*set_seqno)(struct intel_ring_buffer *ring,
+				     u32 seqno);
 	int		(*dispatch_execbuffer)(struct intel_ring_buffer *ring,
 					       u32 offset, u32 length,
 					       unsigned flags);
@@ -166,6 +168,13 @@ intel_read_status_page(struct intel_ring_buffer *ring,
 	return ring->status_page.page_addr[reg];
 }
 
+static inline void
+intel_write_status_page(struct intel_ring_buffer *ring,
+			int reg, u32 value)
+{
+	ring->status_page.page_addr[reg] = value;
+}
+
 /**
  * Reads a dword out of the status page, which is written to from the command
  * queue by automatic updates, MI_REPORT_HEAD, MI_STORE_DATA_INDEX, or
-- 
1.7.9.5

[PATCH v3 2/5] drm/i915: Initialize hardware semaphore state on ring init

[Mika Kuoppala]

Hardware status page needs to have proper seqno set
as our initial seqno can be arbitrary. If initial seqno is close
to wrap boundary on init and i915_seqno_passed() (31bit space)
refers to hw status page which contains zero, errorneous result
will be returned.

v2: clear mboxes and set hws page directly instead of going
through rings. Suggested by Chris Wilson.

v3: hws needs to be updated for all gens. Noticed by Chris
Wilson.

References: https://bugs.freedesktop.org/show_bug.cgi?id=58230
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |    8 +++-----
 drivers/gpu/drm/i915/intel_ringbuffer.c |   24 +++++++++---------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |    2 +-
 3 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d15c862..396be20 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1954,9 +1954,7 @@ i915_gem_handle_seqno_wrap(struct drm_device *dev)
 
 	/* Finally reset hw state */
 	for_each_ring(ring, dev_priv, i) {
-		ret = intel_ring_handle_seqno_wrap(ring);
-		if (ret)
-			return ret;
+		intel_ring_init_seqno(ring, 0);
 
 		for (j = 0; j < ARRAY_SIZE(ring->sync_seqno); j++)
 			ring->sync_seqno[j] = 0;
@@ -3933,6 +3931,8 @@ i915_gem_init_hw(struct drm_device *dev)
 
 	i915_gem_init_swizzling(dev);
 
+	dev_priv->next_seqno = dev_priv->last_seqno = (u32)~0 - 0x1000;
+
 	ret = intel_init_render_ring_buffer(dev);
 	if (ret)
 		return ret;
@@ -3949,8 +3949,6 @@ i915_gem_init_hw(struct drm_device *dev)
 			goto cleanup_bsd_ring;
 	}
 
-	dev_priv->next_seqno = (u32)-1 - 0x1000;
-
 	/*
 	 * XXX: There was some w/a described somewhere suggesting loading
 	 * contexts before PPGTT.
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index f536a99..2bd074a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1184,6 +1184,8 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 	if (IS_I830(ring->dev) || IS_845G(ring->dev))
 		ring->effective_size -= 128;
 
+	intel_ring_init_seqno(ring, dev_priv->last_seqno);
+
 	return 0;
 
 err_unmap:
@@ -1431,26 +1433,18 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
 	return __intel_ring_begin(ring, num_dwords * sizeof(uint32_t));
 }
 
-int intel_ring_handle_seqno_wrap(struct intel_ring_buffer *ring)
+void intel_ring_init_seqno(struct intel_ring_buffer *ring, u32 seqno)
 {
-	int ret;
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 
 	BUG_ON(ring->outstanding_lazy_request);
 
-	if (INTEL_INFO(ring->dev)->gen < 6)
-		return 0;
-
-	ret = __intel_ring_begin(ring, 6 * sizeof(uint32_t));
-	if (ret)
-		return ret;
-
-	/* Leaving a stale, pre-wrap seqno behind in the mboxes will result in
-	 * post-wrap semaphore waits completing immediately. Clear them. */
-	update_mboxes(ring, ring->signal_mbox[0]);
-	update_mboxes(ring, ring->signal_mbox[1]);
-	intel_ring_advance(ring);
+	if (INTEL_INFO(ring->dev)->gen >= 6) {
+		I915_WRITE(RING_SYNC_0(ring->mmio_base), 0);
+		I915_WRITE(RING_SYNC_1(ring->mmio_base), 0);
+	}
 
-	return 0;
+	ring->set_seqno(ring, seqno);
 }
 
 void intel_ring_advance(struct intel_ring_buffer *ring)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 4a7cd67..e7b9a6a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -205,7 +205,7 @@ static inline void intel_ring_emit(struct intel_ring_buffer *ring,
 }
 void intel_ring_advance(struct intel_ring_buffer *ring);
 int __must_check intel_ring_idle(struct intel_ring_buffer *ring);
-int __must_check intel_ring_handle_seqno_wrap(struct intel_ring_buffer *ring);
+void intel_ring_init_seqno(struct intel_ring_buffer *ring, u32 seqno);
 int intel_ring_flush_all_caches(struct intel_ring_buffer *ring);
 int intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring);
 
-- 
1.7.9.5

[PATCH v3 3/5] drm/i915: Always clear semaphore mboxes on seqno wrap

[Mika Kuoppala]

In preparation for setting the seqno to arbitrary value on init or
through debugfs. We need to always clear the semaphores and set the
hws page seqno index by calling intel_ring_init_seqno().

v2: rewrote the commit message as suggested by Chris Wilson.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c |   12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 396be20..53442ea 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1932,18 +1932,6 @@ i915_gem_handle_seqno_wrap(struct drm_device *dev)
 	struct intel_ring_buffer *ring;
 	int ret, i, j;
 
-	/* The hardware uses various monotonic 32-bit counters, if we
-	 * detect that they will wraparound we need to idle the GPU
-	 * and reset those counters.
-	 */
-	ret = 0;
-	for_each_ring(ring, dev_priv, i) {
-		for (j = 0; j < ARRAY_SIZE(ring->sync_seqno); j++)
-			ret |= ring->sync_seqno[j] != 0;
-	}
-	if (ret == 0)
-		return ret;
-
 	/* Carefully retire all requests without writing to the rings */
 	for_each_ring(ring, dev_priv, i) {
 		ret = intel_ring_idle(ring);
-- 
1.7.9.5

[PATCH v3 4/5] drm/i915: Introduce i915_gem_set_seqno()

[Mika Kuoppala]

This function can be used to set the driver's next_seqno
to arbitrary value.

i915_gem_set_seqno() will idle the gpu, retire outstanding
requests, clear the semaphore mailboxes and set the hardware
status page's seqno index.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |    4 ++--
 drivers/gpu/drm/i915/i915_gem.c |   32 +++++++++++++++++++++++++++++---
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 514aee8..b0faa91 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1479,8 +1479,8 @@ i915_seqno_passed(uint32_t seq1, uint32_t seq2)
 	return (int32_t)(seq1 - seq2) >= 0;
 }
 
-extern int i915_gem_get_seqno(struct drm_device *dev, u32 *seqno);
-
+int __must_check i915_gem_get_seqno(struct drm_device *dev, u32 *seqno);
+int __must_check i915_gem_set_seqno(struct drm_device *dev, u32 seqno);
 int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
 int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 53442ea..8291896 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1926,7 +1926,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
 }
 
 static int
-i915_gem_handle_seqno_wrap(struct drm_device *dev)
+i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_ring_buffer *ring;
@@ -1942,7 +1942,7 @@ i915_gem_handle_seqno_wrap(struct drm_device *dev)
 
 	/* Finally reset hw state */
 	for_each_ring(ring, dev_priv, i) {
-		intel_ring_init_seqno(ring, 0);
+		intel_ring_init_seqno(ring, seqno);
 
 		for (j = 0; j < ARRAY_SIZE(ring->sync_seqno); j++)
 			ring->sync_seqno[j] = 0;
@@ -1951,6 +1951,32 @@ i915_gem_handle_seqno_wrap(struct drm_device *dev)
 	return 0;
 }
 
+int i915_gem_set_seqno(struct drm_device *dev, u32 seqno)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	if (seqno == 0)
+		return -EINVAL;
+
+	/* HWS page needs to be set less than what we
+	 * will inject to ring
+	 */
+	ret = i915_gem_init_seqno(dev, seqno - 1);
+	if (ret)
+		return ret;
+
+	/* Carefully set the last_seqno value so that wrap
+	 * detection still works
+	 */
+	dev_priv->next_seqno = seqno;
+	dev_priv->last_seqno = seqno - 1;
+	if (dev_priv->last_seqno == 0)
+		dev_priv->last_seqno--;
+
+	return 0;
+}
+
 int
 i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 {
@@ -1958,7 +1984,7 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 
 	/* reserve 0 for non-seqno */
 	if (dev_priv->next_seqno == 0) {
-		int ret = i915_gem_handle_seqno_wrap(dev);
+		int ret = i915_gem_init_seqno(dev, 0);
 		if (ret)
 			return ret;
 
-- 
1.7.9.5

[PATCH v3 5/5] drm/i915: Make next_seqno debugs entry to use i915_gem_set_seqno

[Mika Kuoppala]

This debugs entry can be used to set arbitrary value to next_seqno.
Use i915_gem_set_seqno instead of poking next_seqno.

v2: nasty details of next_seqno and last_seqno handling
moved inside i915_gem_set_seqno as suggested by Chris Wilson.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7047c4a..882a735 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -878,7 +878,6 @@ i915_next_seqno_write(struct file *filp,
 		      loff_t *ppos)
 {
 	struct drm_device *dev = filp->private_data;
-	drm_i915_private_t *dev_priv = dev->dev_private;
 	char buf[20];
 	u32 val = 1;
 	int ret;
@@ -896,19 +895,11 @@ i915_next_seqno_write(struct file *filp,
 			return ret;
 	}
 
-	if (val == 0)
-		return -EINVAL;
-
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
 		return ret;
 
-	if (i915_seqno_passed(val, dev_priv->next_seqno)) {
-		dev_priv->next_seqno = val;
-		DRM_DEBUG_DRIVER("Advancing seqno to %u\n", val);
-	} else {
-		ret = -EINVAL;
-	}
+	ret = i915_gem_set_seqno(dev, val);
 
 	mutex_unlock(&dev->struct_mutex);
 
-- 
1.7.9.5

Re: [PATCH v3 5/5] drm/i915: Make next_seqno debugs entry to use i915_gem_set_seqno

[Chris Wilson]

On Wed, 19 Dec 2012 11:13:09 +0200, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> This debugs entry can be used to set arbitrary value to next_seqno.
> Use i915_gem_set_seqno instead of poking next_seqno.
> 
> v2: nasty details of next_seqno and last_seqno handling
> moved inside i915_gem_set_seqno as suggested by Chris Wilson.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Series looks good,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v3 5/5] drm/i915: Make next_seqno debugs entry to use i915_gem_set_seqno

[Daniel Vetter]

On Wed, Dec 19, 2012 at 09:57:58AM +0000, Chris Wilson wrote:
> On Wed, 19 Dec 2012 11:13:09 +0200, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> > This debugs entry can be used to set arbitrary value to next_seqno.
> > Use i915_gem_set_seqno instead of poking next_seqno.
> > 
> > v2: nasty details of next_seqno and last_seqno handling
> > moved inside i915_gem_set_seqno as suggested by Chris Wilson.
> > 
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Series looks good,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

And queued for -next, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch