On Thu, 02 Jan 2014, Chris Wilson <[email protected]> wrote: > In very rare cases (such as a memory failure stress test) it is possible > to fill the entire ring without emitting a request. Under this > circumstance, the outstanding request is flushed and waited upon. After > space on the ring is cleared, we return to emitting the new command - > except that we just cleared the seqno allocated for this operation and > trigger the sanity check that a request is only ever emitted with a > valid seqno. The fix is to rearrange the code to make sure the > allocation of the seqno for this operation is after any required flushes > of outstanding operations. > > The bug exists since the preallocation was introduced in > commit 9d7730914f4cd496e356acfab95b41075aa8eae8 > Author: Chris Wilson <[email protected]> > Date: Tue Nov 27 16:22:52 2012 +0000 > > drm/i915: Preallocate next seqno before touching the ring >
I can't claim to be an expert in the area, but the explanation and code make sense. Reviewed-by: Jani Nikula <[email protected]> > Signed-off-by: Chris Wilson <[email protected]> > Cc: Mika Kuoppala <[email protected]> > Cc: Daniel Vetter <[email protected]> > Cc: [email protected] > Signed-off-by: Chris Wilson <[email protected]> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 5744841669e4..6bb914e49bf0 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1610,8 +1610,8 @@ intel_ring_alloc_seqno(struct intel_ring_buffer *ring) > return i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno); > } > > -static int __intel_ring_begin(struct intel_ring_buffer *ring, > - int bytes) > +static int __intel_ring_prepare(struct intel_ring_buffer *ring, > + int bytes) > { > int ret; > > @@ -1627,7 +1627,6 @@ static int __intel_ring_begin(struct intel_ring_buffer > *ring, > return ret; > } > > - ring->space -= bytes; > return 0; > } > > @@ -1642,12 +1641,17 @@ int intel_ring_begin(struct intel_ring_buffer *ring, > if (ret) > return ret; > > + ret = __intel_ring_prepare(ring, num_dwords * sizeof(uint32_t)); > + if (ret) > + return ret; > + > /* Preallocate the olr before touching the ring */ > ret = intel_ring_alloc_seqno(ring); > if (ret) > return ret; > > - return __intel_ring_begin(ring, num_dwords * sizeof(uint32_t)); > + ring->space -= num_dwords * sizeof(uint32_t); > + return 0; > } > > void intel_ring_init_seqno(struct intel_ring_buffer *ring, u32 seqno) > -- > 1.8.5.2 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
