On Wed, Sep 19, 2012 at 04:08:51PM +0100, Chris Wilson wrote:
> In commit 69c2fc891343cb5217c866d10709343cff190bdc
> Author: Chris Wilson <chris at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
