On Tue, 27 Nov 2012 09:48:50 +0000, Chris Wilson <chris at 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 at chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala at intel.com> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=863861 Reviewed-by: Mika Kuoppala <mika.kuoppala at intel.com>