Re: [Intel-gfx] [PATCH] drm/ivb: Flush outstanding requests before allocating new seqno

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 02 Jan 2014, Chris Wilson <chris@chris-wilson.co.uk> 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 <chris@chris-wilson.co.uk>
> 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 <jani.nikula@intel.com>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  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
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center