On Tue, 13 Nov 2012 15:51:36 +0200
Mika Kuoppala <mika.kuoppala at linux.intel.com> wrote:
> If seqno has wrapped, normal compare operation will give wrong results.
> i915_seqno_passed can handle the wrap so use it instead.
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index cdcf19d..35d5db8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2138,7 +2138,7 @@ i915_gem_retire_requests_ring(struct intel_ring_buffer *ring)
> seqno = ring->get_seqno(ring, true);
>
> for (i = 0; i < ARRAY_SIZE(ring->sync_seqno); i++)
> - if (seqno >= ring->sync_seqno[i])
> + if (i915_seqno_passed(seqno, ring->sync_seqno[i]))
> ring->sync_seqno[i] = 0;
>
> while (!list_empty(&ring->request_list)) {
> @@ -2376,7 +2376,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
> idx = intel_ring_sync_index(from, to);
>
> seqno = obj->last_read_seqno;
> - if (seqno <= from->sync_seqno[idx])
> + if (i915_seqno_passed(from->sync_seqno[idx], seqno))
> return 0;
>
> ret = i915_gem_check_olr(obj->ring, seqno);
Can you add another patch on top of this to have the starting seqno be a
near pre-wrap value so we can catch bugs even without igt.
Also as an overall comment, I want the patches to guarantee to catch
the bug you found, which I think with the randomness of
gem_stress - isn't. Specifically, we want the waiting ring to be
waiting on a pre-wrapped value. Maybe I missed that guarantee, but if
there is a quick/dirty way to make that happen, that would better than
running an arbitrary number of gem_stress tests.
--
Ben Widawsky, Intel Open Source Technology Center
