Re: [PATCH] drm/i915: Preallocate next seqno before touching the ring

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

On Tue, 27 Nov 2012 09:48:50 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Based on the work by Mika Kuoppala, we realised that we need to handle
> seqno wraparound prior to committing our changes to the ring. The most
> obvious point then is to grab the seqno inside intel_ring_begin(), and
> then to reuse that seqno for all ring operations until the next request.
> As intel_ring_begin() can fail, the callers must already be prepared to
> handle such failure and so we can safely add further checks.
> 
> This patch looks like it should be split up into the interface
> changes and the tweaks to move seqno wrapping from the execbuffer into
> the core seqno increment. However, I found no easy way to break it into
> incremental steps without introducing further broken behaviour.
> 
> v2: Mika found a silly mistake and a subtle error in the existing code;
> inside i915_gem_retire_requests() we were resetting the sync_seqno of
> the target ring based on the seqno from this ring - which are only
> related by the order of their allocation, not retirement. Hence we were
> applying the optimisation that the rings were synchronised too early,
> fortunately the only real casualty there is the handling of seqno
> wrapping.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=863861

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>