* [PATCH 1/6] drm/i915: Add debugfs entry to read/write next_seqno
2012-12-04 13:11 [PATCH 0/6] ring seqno wrap handling Mika Kuoppala
@ 2012-12-04 13:12 ` Mika Kuoppala
2012-12-04 13:12 ` [PATCH 2/6] drm/i915: Fix debugfs seqno info print to use uint Mika Kuoppala
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Mika Kuoppala @ 2012-12-04 13:12 UTC (permalink / raw)
To: intel-gfx
This is needed for testing seqno wrapping. Be careful to not bump next_seqno
more than 0x7FFFFFFF at a time (between some handled requests) as
i915_seqno_passed() can't handle bigger difference in between.
v2: Address review comments from Chris Wilson.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 86 +++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ce60506..18de201 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -843,6 +843,86 @@ static const struct file_operations i915_error_state_fops = {
.release = i915_error_state_release,
};
+static ssize_t
+i915_next_seqno_read(struct file *filp,
+ char __user *ubuf,
+ size_t max,
+ loff_t *ppos)
+{
+ struct drm_device *dev = filp->private_data;
+ drm_i915_private_t *dev_priv = dev->dev_private;
+ char buf[80];
+ int len;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
+
+ len = snprintf(buf, sizeof(buf),
+ "next_seqno : 0x%x\n",
+ dev_priv->next_seqno);
+
+ mutex_unlock(&dev->struct_mutex);
+
+ if (len > sizeof(buf))
+ len = sizeof(buf);
+
+ return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+}
+
+static ssize_t
+i915_next_seqno_write(struct file *filp,
+ const char __user *ubuf,
+ size_t cnt,
+ 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;
+
+ if (cnt > 0) {
+ if (cnt > sizeof(buf) - 1)
+ return -EINVAL;
+
+ if (copy_from_user(buf, ubuf, cnt))
+ return -EFAULT;
+ buf[cnt] = 0;
+
+ ret = kstrtouint(buf, 0, &val);
+ if (ret < 0)
+ 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;
+ }
+
+ mutex_unlock(&dev->struct_mutex);
+
+ return ret ?: cnt;
+}
+
+static const struct file_operations i915_next_seqno_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .read = i915_next_seqno_read,
+ .write = i915_next_seqno_write,
+ .llseek = default_llseek,
+};
+
static int i915_rstdby_delays(struct seq_file *m, void *unused)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -2107,6 +2187,12 @@ int i915_debugfs_init(struct drm_minor *minor)
if (ret)
return ret;
+ ret = i915_debugfs_create(minor->debugfs_root, minor,
+ "i915_next_seqno",
+ &i915_next_seqno_fops);
+ if (ret)
+ return ret;
+
return drm_debugfs_create_files(i915_debugfs_list,
I915_DEBUGFS_ENTRIES,
minor->debugfs_root, minor);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/6] drm/i915: Fix debugfs seqno info print to use uint
2012-12-04 13:11 [PATCH 0/6] ring seqno wrap handling Mika Kuoppala
2012-12-04 13:12 ` [PATCH 1/6] drm/i915: Add debugfs entry to read/write next_seqno Mika Kuoppala
@ 2012-12-04 13:12 ` Mika Kuoppala
2012-12-04 13:12 ` [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened Mika Kuoppala
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Mika Kuoppala @ 2012-12-04 13:12 UTC (permalink / raw)
To: intel-gfx
seqno's are u32 so print accordingly
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 18de201..e51b89d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -389,7 +389,7 @@ static void i915_ring_seqno_info(struct seq_file *m,
struct intel_ring_buffer *ring)
{
if (ring->get_seqno) {
- seq_printf(m, "Current sequence (%s): %d\n",
+ seq_printf(m, "Current sequence (%s): %u\n",
ring->name, ring->get_seqno(ring, false));
}
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened
2012-12-04 13:11 [PATCH 0/6] ring seqno wrap handling Mika Kuoppala
2012-12-04 13:12 ` [PATCH 1/6] drm/i915: Add debugfs entry to read/write next_seqno Mika Kuoppala
2012-12-04 13:12 ` [PATCH 2/6] drm/i915: Fix debugfs seqno info print to use uint Mika Kuoppala
@ 2012-12-04 13:12 ` Mika Kuoppala
2012-12-05 20:44 ` Paulo Zanoni
2012-12-06 14:12 ` [PATCH] Detect wraparound using next-seqno rather than waiter->olr Chris Wilson
2012-12-04 13:12 ` [PATCH 4/6] drm/i915: Split intel_ring_begin Mika Kuoppala
` (3 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: Mika Kuoppala @ 2012-12-04 13:12 UTC (permalink / raw)
To: intel-gfx
If wrap just happened we need to prevent emitting waits for
pre wrap values. Detect this and emit no-ops instead.
v2: Use olr > seqno to detect wrap instead of *seqno == 0
as suggested by Chris Wilson.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 36e1e13a..2305af5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -626,11 +626,22 @@ gen6_ring_sync(struct intel_ring_buffer *waiter,
if (ret)
return ret;
- intel_ring_emit(waiter,
- dw1 | signaller->semaphore_register[waiter->id]);
- intel_ring_emit(waiter, seqno);
- intel_ring_emit(waiter, 0);
- intel_ring_emit(waiter, MI_NOOP);
+ BUG_ON(!waiter->outstanding_lazy_request);
+
+ /* If seqno wrap happened, omit the wait with no-ops */
+ if (likely(waiter->outstanding_lazy_request > seqno)) {
+ intel_ring_emit(waiter,
+ dw1 |
+ signaller->semaphore_register[waiter->id]);
+ intel_ring_emit(waiter, seqno);
+ intel_ring_emit(waiter, 0);
+ intel_ring_emit(waiter, MI_NOOP);
+ } else {
+ intel_ring_emit(waiter, MI_NOOP);
+ intel_ring_emit(waiter, MI_NOOP);
+ intel_ring_emit(waiter, MI_NOOP);
+ intel_ring_emit(waiter, MI_NOOP);
+ }
intel_ring_advance(waiter);
return 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened
2012-12-04 13:12 ` [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened Mika Kuoppala
@ 2012-12-05 20:44 ` Paulo Zanoni
2012-12-06 8:51 ` Daniel Vetter
2012-12-06 14:12 ` [PATCH] Detect wraparound using next-seqno rather than waiter->olr Chris Wilson
1 sibling, 1 reply; 15+ messages in thread
From: Paulo Zanoni @ 2012-12-05 20:44 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
Hi
2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
> If wrap just happened we need to prevent emitting waits for
> pre wrap values. Detect this and emit no-ops instead.
>
> v2: Use olr > seqno to detect wrap instead of *seqno == 0
> as suggested by Chris Wilson.
This commit introduces a bug on Haswell. Now when I'm typing my
password on GDM the screen keeps doing wrong rendering. It "blinks
blue". After logging in I don't see more problems.
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 36e1e13a..2305af5 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -626,11 +626,22 @@ gen6_ring_sync(struct intel_ring_buffer *waiter,
> if (ret)
> return ret;
>
> - intel_ring_emit(waiter,
> - dw1 | signaller->semaphore_register[waiter->id]);
> - intel_ring_emit(waiter, seqno);
> - intel_ring_emit(waiter, 0);
> - intel_ring_emit(waiter, MI_NOOP);
> + BUG_ON(!waiter->outstanding_lazy_request);
> +
> + /* If seqno wrap happened, omit the wait with no-ops */
> + if (likely(waiter->outstanding_lazy_request > seqno)) {
> + intel_ring_emit(waiter,
> + dw1 |
> + signaller->semaphore_register[waiter->id]);
> + intel_ring_emit(waiter, seqno);
> + intel_ring_emit(waiter, 0);
> + intel_ring_emit(waiter, MI_NOOP);
> + } else {
> + intel_ring_emit(waiter, MI_NOOP);
> + intel_ring_emit(waiter, MI_NOOP);
> + intel_ring_emit(waiter, MI_NOOP);
> + intel_ring_emit(waiter, MI_NOOP);
> + }
> intel_ring_advance(waiter);
>
> return 0;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened
2012-12-05 20:44 ` Paulo Zanoni
@ 2012-12-06 8:51 ` Daniel Vetter
2012-12-06 11:41 ` Paulo Zanoni
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2012-12-06 8:51 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Wed, Dec 5, 2012 at 9:44 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
>> If wrap just happened we need to prevent emitting waits for
>> pre wrap values. Detect this and emit no-ops instead.
>>
>> v2: Use olr > seqno to detect wrap instead of *seqno == 0
>> as suggested by Chris Wilson.
>
> This commit introduces a bug on Haswell. Now when I'm typing my
> password on GDM the screen keeps doing wrong rendering. It "blinks
> blue". After logging in I don't see more prodrm/i915: Set initial seqno value close to wrap boundaryblems.
Just now I've taken out "drm/i915: Set initial seqno value close to
wrap boundary" since QA complained that it regresses things. Does that
help for you, too?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened
2012-12-06 8:51 ` Daniel Vetter
@ 2012-12-06 11:41 ` Paulo Zanoni
2012-12-06 12:14 ` Daniel Vetter
0 siblings, 1 reply; 15+ messages in thread
From: Paulo Zanoni @ 2012-12-06 11:41 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
Hi
2012/12/6 Daniel Vetter <daniel@ffwll.ch>:
> On Wed, Dec 5, 2012 at 9:44 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
>> 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
>>> If wrap just happened we need to prevent emitting waits for
>>> pre wrap values. Detect this and emit no-ops instead.
>>>
>>> v2: Use olr > seqno to detect wrap instead of *seqno == 0
>>> as suggested by Chris Wilson.
>>
>> This commit introduces a bug on Haswell. Now when I'm typing my
>> password on GDM the screen keeps doing wrong rendering. It "blinks
>> blue". After logging in I don't see more prodrm/i915: Set initial seqno value close to wrap boundaryblems.
>
> Just now I've taken out "drm/i915: Set initial seqno value close to
> wrap boundary" since QA complained that it regresses things. Does that
> help for you, too?
It helps: besides the "wrong rendering at GDM screen" I was also
getting GPU hangs (when starting X, when running dmesg, when alt+tab,
etc), and it seems with today's dinq I don't get the gpu hangs
anymore. I still get the "wrong rendering" problem and it goes away if
we revert the "Don't emit semaphore wait if wrap happened".
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened
2012-12-06 11:41 ` Paulo Zanoni
@ 2012-12-06 12:14 ` Daniel Vetter
2012-12-06 12:24 ` Chris Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2012-12-06 12:14 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Thu, Dec 6, 2012 at 12:41 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> 2012/12/6 Daniel Vetter <daniel@ffwll.ch>:
>> On Wed, Dec 5, 2012 at 9:44 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
>>> 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
>>>> If wrap just happened we need to prevent emitting waits for
>>>> pre wrap values. Detect this and emit no-ops instead.
>>>>
>>>> v2: Use olr > seqno to detect wrap instead of *seqno == 0
>>>> as suggested by Chris Wilson.
>>>
>>> This commit introduces a bug on Haswell. Now when I'm typing my
>>> password on GDM the screen keeps doing wrong rendering. It "blinks
>>> blue". After logging in I don't see more prodrm/i915: Set initial seqno value close to wrap boundaryblems.
>>
>> Just now I've taken out "drm/i915: Set initial seqno value close to
>> wrap boundary" since QA complained that it regresses things. Does that
>> help for you, too?
>
> It helps: besides the "wrong rendering at GDM screen" I was also
> getting GPU hangs (when starting X, when running dmesg, when alt+tab,
> etc), and it seems with today's dinq I don't get the gpu hangs
> anymore. I still get the "wrong rendering" problem and it goes away if
> we revert the "Don't emit semaphore wait if wrap happened".
Ok, looks like we have still some fish left to fry here. I've backed
out the 2nd patch, too. And I guess we need some more tests in i-g-t
to check semaphore correctness, we seem to have some serious gaps ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened
2012-12-06 12:14 ` Daniel Vetter
@ 2012-12-06 12:24 ` Chris Wilson
0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2012-12-06 12:24 UTC (permalink / raw)
To: Daniel Vetter, Paulo Zanoni; +Cc: intel-gfx
On Thu, 6 Dec 2012 13:14:09 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Dec 6, 2012 at 12:41 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> > 2012/12/6 Daniel Vetter <daniel@ffwll.ch>:
> >> On Wed, Dec 5, 2012 at 9:44 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> >>> 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
> >>>> If wrap just happened we need to prevent emitting waits for
> >>>> pre wrap values. Detect this and emit no-ops instead.
> >>>>
> >>>> v2: Use olr > seqno to detect wrap instead of *seqno == 0
> >>>> as suggested by Chris Wilson.
> >>>
> >>> This commit introduces a bug on Haswell. Now when I'm typing my
> >>> password on GDM the screen keeps doing wrong rendering. It "blinks
> >>> blue". After logging in I don't see more prodrm/i915: Set initial seqno value close to wrap boundaryblems.
> >>
> >> Just now I've taken out "drm/i915: Set initial seqno value close to
> >> wrap boundary" since QA complained that it regresses things. Does that
> >> help for you, too?
> >
> > It helps: besides the "wrong rendering at GDM screen" I was also
> > getting GPU hangs (when starting X, when running dmesg, when alt+tab,
> > etc), and it seems with today's dinq I don't get the gpu hangs
> > anymore. I still get the "wrong rendering" problem and it goes away if
> > we revert the "Don't emit semaphore wait if wrap happened".
>
> Ok, looks like we have still some fish left to fry here. I've backed
> out the 2nd patch, too. And I guess we need some more tests in i-g-t
> to check semaphore correctness, we seem to have some serious gaps ...
I wouldn't back that out too quickly as it seems that Paulo has some
debugging to do first. A few WARNs would be a good start...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] Detect wraparound using next-seqno rather than waiter->olr
2012-12-04 13:12 ` [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened Mika Kuoppala
2012-12-05 20:44 ` Paulo Zanoni
@ 2012-12-06 14:12 ` Chris Wilson
1 sibling, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2012-12-06 14:12 UTC (permalink / raw)
To: intel-gfx
As the olr may have been allocated long in advance for a much earlier
write to the ring, waiter->olr may genuinely be less than the semaphore
seqno. So use the global next_seqno for wraparound detection instead.
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d5d5177..29f9e2c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -595,6 +595,12 @@ gen6_add_request(struct intel_ring_buffer *ring)
return 0;
}
+static inline u32 i915_gem_next_seqno(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ return dev_priv->next_seqno;
+}
+
/**
* intel_ring_sync - sync the waiter to the signaller on seqno
*
@@ -628,7 +634,7 @@ gen6_ring_sync(struct intel_ring_buffer *waiter,
BUG_ON(!waiter->outstanding_lazy_request);
/* If seqno wrap happened, omit the wait with no-ops */
- if (likely(waiter->outstanding_lazy_request > seqno)) {
+ if (likely(i915_gem_next_seqno(waiter->dev) > seqno)) {
intel_ring_emit(waiter,
dw1 |
signaller->semaphore_register[waiter->id]);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/6] drm/i915: Split intel_ring_begin
2012-12-04 13:11 [PATCH 0/6] ring seqno wrap handling Mika Kuoppala
` (2 preceding siblings ...)
2012-12-04 13:12 ` [PATCH 3/6] drm/i915: Don't emit semaphore wait if wrap happened Mika Kuoppala
@ 2012-12-04 13:12 ` Mika Kuoppala
2012-12-04 13:12 ` [PATCH 5/6] drm/i915: Add intel_ring_handle_seqno wrap Mika Kuoppala
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Mika Kuoppala @ 2012-12-04 13:12 UTC (permalink / raw)
To: intel-gfx
In preparation for handling ring seqno wrapping, split
intel_ring_begin into helper part which doesn't allocate
seqno.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 37 ++++++++++++++++++-------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2305af5..9342bb2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1374,11 +1374,31 @@ intel_ring_alloc_seqno(struct intel_ring_buffer *ring)
return i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_request);
}
+static int __intel_ring_begin(struct intel_ring_buffer *ring,
+ int bytes)
+{
+ int ret;
+
+ if (unlikely(ring->tail + bytes > ring->effective_size)) {
+ ret = intel_wrap_ring_buffer(ring);
+ if (unlikely(ret))
+ return ret;
+ }
+
+ if (unlikely(ring->space < bytes)) {
+ ret = ring_wait_for_space(ring, bytes);
+ if (unlikely(ret))
+ return ret;
+ }
+
+ ring->space -= bytes;
+ return 0;
+}
+
int intel_ring_begin(struct intel_ring_buffer *ring,
int num_dwords)
{
drm_i915_private_t *dev_priv = ring->dev->dev_private;
- int n = 4*num_dwords;
int ret;
ret = i915_gem_check_wedge(dev_priv, dev_priv->mm.interruptible);
@@ -1390,20 +1410,7 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
if (ret)
return ret;
- if (unlikely(ring->tail + n > ring->effective_size)) {
- ret = intel_wrap_ring_buffer(ring);
- if (unlikely(ret))
- return ret;
- }
-
- if (unlikely(ring->space < n)) {
- ret = ring_wait_for_space(ring, n);
- if (unlikely(ret))
- return ret;
- }
-
- ring->space -= n;
- return 0;
+ return __intel_ring_begin(ring, num_dwords * sizeof(uint32_t));
}
void intel_ring_advance(struct intel_ring_buffer *ring)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/6] drm/i915: Add intel_ring_handle_seqno wrap
2012-12-04 13:11 [PATCH 0/6] ring seqno wrap handling Mika Kuoppala
` (3 preceding siblings ...)
2012-12-04 13:12 ` [PATCH 4/6] drm/i915: Split intel_ring_begin Mika Kuoppala
@ 2012-12-04 13:12 ` Mika Kuoppala
2012-12-04 13:12 ` [PATCH 6/6] drm/i915: Set initial seqno value close to wrap boundary Mika Kuoppala
2012-12-04 14:05 ` [PATCH 0/6] ring seqno wrap handling Chris Wilson
6 siblings, 0 replies; 15+ messages in thread
From: Mika Kuoppala @ 2012-12-04 13:12 UTC (permalink / raw)
To: intel-gfx
If there are pre-wrap values in semaphore-mbox registers after wrap,
syncing against some after-wrap request will complete immediately.
Fix this by emitting ring commands to set mbox registers to zero
when the wrap happens.
v2: Use __intel_ring_begin to emit ring commands, from
Chris Wilson.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 4 ++++
drivers/gpu/drm/i915/intel_ringbuffer.c | 20 ++++++++++++++++++++
drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +-
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cb94144..02d3151 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1952,6 +1952,10 @@ i915_gem_handle_seqno_wrap(struct drm_device *dev)
i915_gem_retire_requests(dev);
for_each_ring(ring, dev_priv, i) {
+ ret = intel_ring_handle_seqno_wrap(ring);
+ if (ret)
+ return ret;
+
for (j = 0; j < ARRAY_SIZE(ring->sync_seqno); j++)
ring->sync_seqno[j] = 0;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9342bb2..23587b2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1413,6 +1413,26 @@ 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)
+{
+ int ret;
+
+ 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;
+
+ update_mboxes(ring, ring->signal_mbox[0]);
+ update_mboxes(ring, ring->signal_mbox[1]);
+ intel_ring_advance(ring);
+
+ return 0;
+}
+
void intel_ring_advance(struct intel_ring_buffer *ring)
{
struct drm_i915_private *dev_priv = ring->dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d4b7416..b4a533e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -196,7 +196,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);
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 6/6] drm/i915: Set initial seqno value close to wrap boundary
2012-12-04 13:11 [PATCH 0/6] ring seqno wrap handling Mika Kuoppala
` (4 preceding siblings ...)
2012-12-04 13:12 ` [PATCH 5/6] drm/i915: Add intel_ring_handle_seqno wrap Mika Kuoppala
@ 2012-12-04 13:12 ` Mika Kuoppala
2012-12-04 14:05 ` [PATCH 0/6] ring seqno wrap handling Chris Wilson
6 siblings, 0 replies; 15+ messages in thread
From: Mika Kuoppala @ 2012-12-04 13:12 UTC (permalink / raw)
To: intel-gfx
To gain confidence in the wrap handling, make it happen quite
soon after the boot.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 02d3151..70bb4e8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3947,7 +3947,7 @@ i915_gem_init_hw(struct drm_device *dev)
goto cleanup_bsd_ring;
}
- dev_priv->next_seqno = 1;
+ dev_priv->next_seqno = (u32)-1 - 0x1000;
/*
* XXX: There was some w/a described somewhere suggesting loading
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 0/6] ring seqno wrap handling
2012-12-04 13:11 [PATCH 0/6] ring seqno wrap handling Mika Kuoppala
` (5 preceding siblings ...)
2012-12-04 13:12 ` [PATCH 6/6] drm/i915: Set initial seqno value close to wrap boundary Mika Kuoppala
@ 2012-12-04 14:05 ` Chris Wilson
2012-12-04 22:03 ` Daniel Vetter
6 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2012-12-04 14:05 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
On Tue, 4 Dec 2012 15:11:59 +0200, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Hi,
>
> As Ben suggested, explicitly syncing across wrap boundary
> revealed yet another set of issues that gem_stress didn't find.
>
> So here is a patchset which addresses the syncing problems
> across wrap boundary. Chris helped me to carve out seqno handling
> ugliness from my initial attempts to handle the ring wrapping.
>
> As I can't break this anymore, i added 6/6.
> --Mika
>
> Mika Kuoppala (6):
> drm/i915: Fix debugfs seqno info print to use uint
> drm/i915: Don't emit semaphore wait if wrap happened
> drm/i915: Split intel_ring_begin
> drm/i915: Add intel_ring_handle_seqno wrap
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> drm/i915: Add debugfs entry to read/write next_seqno
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> drm/i915: Set initial seqno value close to wrap boundary
*washes hands*
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/6] ring seqno wrap handling
2012-12-04 14:05 ` [PATCH 0/6] ring seqno wrap handling Chris Wilson
@ 2012-12-04 22:03 ` Daniel Vetter
0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2012-12-04 22:03 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Tue, Dec 04, 2012 at 02:05:42PM +0000, Chris Wilson wrote:
> On Tue, 4 Dec 2012 15:11:59 +0200, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> > Hi,
> >
> > As Ben suggested, explicitly syncing across wrap boundary
> > revealed yet another set of issues that gem_stress didn't find.
> >
> > So here is a patchset which addresses the syncing problems
> > across wrap boundary. Chris helped me to carve out seqno handling
> > ugliness from my initial attempts to handle the ring wrapping.
> >
> > As I can't break this anymore, i added 6/6.
> > --Mika
> >
> > Mika Kuoppala (6):
> > drm/i915: Fix debugfs seqno info print to use uint
> > drm/i915: Don't emit semaphore wait if wrap happened
> > drm/i915: Split intel_ring_begin
> > drm/i915: Add intel_ring_handle_seqno wrap
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> > drm/i915: Add debugfs entry to read/write next_seqno
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Merged them all, thanks for patches&review.
>
> > drm/i915: Set initial seqno value close to wrap boundary
> *washes hands*
I'm feeling stupid/lucky/bold/whatever, also merged.
/me hides
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 15+ messages in thread