On Wed, 25 Jan 2012 14:03:58 +0100, Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > Currently we reserve seqnos only when we emit the request to the ring > (by bumping dev_priv->next_seqno), but start using it much earlier for > ring->oustanding_lazy_request. When 2 threads compete for the gpu and > run on two different rings (e.g. ddx on blitter vs. compositor) > hilarity ensued, especially when we get constantly interrupted while > reserving buffers. > > Breakage seems to have been introduced in > > commit 6f392d548658a17600da7faaf8a5df25ee5f01f6 > Author: Chris Wilson <chris at chris-wilson.co.uk> > Date: Sat Aug 7 11:01:22 2010 +0100 > > drm/i915: Use a common seqno for all rings. > > This patch fixes up the seqno reservation logic by moving it into > i915_gem_next_request_seqno. The ring->add_request functions now > superflously still return the new seqno through a pointer, that will > be refactored in the next patch. > > v2: Keep i915_gem_get_seqno (but move it to i915_gem.c) to make it > clear that we only have one seqno counter for all rings. Suggested by > Chris Wilson. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45181 > Tested-by: Nicolas Kalkhof nkalkhof()at()web.de > Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > --- > static int > render_ring_flush(struct intel_ring_buffer *ring, > u32 invalidate_domains, > @@ -467,7 +453,7 @@ gen6_add_request(struct intel_ring_buffer *ring, > mbox1_reg = ring->signal_mbox[0]; > mbox2_reg = ring->signal_mbox[1]; > > - *seqno = i915_gem_get_seqno(ring->dev); > + *seqno = ring->outstanding_lazy_request; In discussing this patch with Daniel, I made the mistake of reading that as i915_gem_get_next_request_seqno() instead of get_seqno(). I'd suggest the patch makes that change and hide the ugly ring->o_l_r. Then since we do i915_gem_get_next_request_seqno() both here and in the caller, it becomes much clearer that we are able to remove it. Daniel, apologies for the confusion! -Chris -- Chris Wilson, Intel Open Source Technology Centre
