diff mbox

drm/i915: Wait upon the last request seqno, rather than a future seqno

Message ID 1348067331-6927-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Sept. 19, 2012, 3:08 p.m. UTC
In commit 69c2fc891343cb5217c866d10709343cff190bdc
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jul 20 12:41:03 2012 +0100

    drm/i915: Remove the per-ring write list

the explicit flush was removed from i915_ring_idle(). However, we
continued to wait upon the next seqno which now did not correspond to
any request (except for the unusual condition of a failure to queue a
request after execbuffer) and so would wait indefinitely.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---

This time the complete patch, merging the forgotten partial commit.

---
 drivers/gpu/drm/i915/i915_gem.c |   23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Sept. 20, 2012, 7:49 a.m. UTC | #1
On Wed, Sep 19, 2012 at 04:08:51PM +0100, Chris Wilson wrote:
> In commit 69c2fc891343cb5217c866d10709343cff190bdc
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri Jul 20 12:41:03 2012 +0100
> 
>     drm/i915: Remove the per-ring write list
> 
> the explicit flush was removed from i915_ring_idle(). However, we
> continued to wait upon the next seqno which now did not correspond to
> any request (except for the unusual condition of a failure to queue a
> request after execbuffer) and so would wait indefinitely.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Afaict gem_next_request_seqno sets ring->olr and i915_wait_seqno does
check whether olr is set and then adds the request - which for ring_idle is
pretty much guaranteed to be missing ;-)

So tricky code, but I can't see the bug (and since both module unload and
suspend works, it'd be surprised if there is one). What am I missing here?
-Daniel


> ---
> 
> This time the complete patch, merging the forgotten partial commit.
> 
> ---
>  drivers/gpu/drm/i915/i915_gem.c |   23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 242f58b..39de523 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2593,10 +2593,29 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
>  
>  static int i915_ring_idle(struct intel_ring_buffer *ring)
>  {
> -	if (list_empty(&ring->active_list))
> +	u32 seqno;
> +	int ret;
> +
> +	/* We need to add any requests required to flush the objects */
> +	if (!list_empty(&ring->active_list)) {
> +		seqno = list_entry(ring->active_list.prev,
> +				   struct drm_i915_gem_object,
> +				   ring_list)->last_read_seqno;
> +
> +		ret = i915_gem_check_olr(ring, seqno);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Wait upon the last request to be completed */
> +	if (list_empty(&ring->request_list))
>  		return 0;
>  
> -	return i915_wait_seqno(ring, i915_gem_next_request_seqno(ring));

> +	seqno = list_entry(ring->request_list.prev,
> +			   struct drm_i915_gem_request,
> +			   list)->seqno;
> +
> +	return i915_wait_seqno(ring, seqno);
>  }
>  
>  int i915_gpu_idle(struct drm_device *dev)
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 242f58b..39de523 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2593,10 +2593,29 @@  i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 
 static int i915_ring_idle(struct intel_ring_buffer *ring)
 {
-	if (list_empty(&ring->active_list))
+	u32 seqno;
+	int ret;
+
+	/* We need to add any requests required to flush the objects */
+	if (!list_empty(&ring->active_list)) {
+		seqno = list_entry(ring->active_list.prev,
+				   struct drm_i915_gem_object,
+				   ring_list)->last_read_seqno;
+
+		ret = i915_gem_check_olr(ring, seqno);
+		if (ret)
+			return ret;
+	}
+
+	/* Wait upon the last request to be completed */
+	if (list_empty(&ring->request_list))
 		return 0;
 
-	return i915_wait_seqno(ring, i915_gem_next_request_seqno(ring));
+	seqno = list_entry(ring->request_list.prev,
+			   struct drm_i915_gem_request,
+			   list)->seqno;
+
+	return i915_wait_seqno(ring, seqno);
 }
 
 int i915_gpu_idle(struct drm_device *dev)