<div class="gmail_quote">On Wed, Feb 8, 2012 at 16:06, Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1784,12 +1784,20 @@ i915_add_request(struct intel_ring_buffer *ring, { drm_i915_private_t *dev_priv = ring->dev->dev_private; uint32_t seqno; + u32 request_ring_position; int was_empty; int ret; BUG_ON(request == NULL); seqno = i915_gem_next_request_seqno(ring); + /* Record the position of the start of the request so that + * should we detect the updated seqno part-way through the + * GPU processing the request, we never over-estimate the + * position of the head. + */ + request_ring_position = intel_ring_get_tail(ring); </blockquote><div> Perhaps a stupid question within the bikeshedding spirit, but why do we need intel_ring_get_tail()? Wouldn't just: request_ring_position = ring->tail; be self-explainable? Or some additional logic could be involved there at some point? </div></div> But in overall, I like this, so: Reviewed-by: Eugeni Dodonov <<a href="mailto:eugeni.dodonov@intel.com">eugeni.dodonov@intel.com</a>> -- Eugeni Dodonov<a href="http://eugeni.dodonov.net/" target="_blank"> </a>