[Intel-gfx] [RFC 24/39] drm/i915: Support for 'unflushed' ring idle

[Daniel Vetter]

On Fri, Jul 17, 2015 at 03:33:33PM +0100, John.C.Harrison at Intel.com wrote:
> From: John Harrison <John.C.Harrison at Intel.com>
> 
> When the seqno wraps around zero, the entire GPU is forced to be idle
> for some reason (possibly only to work around issues with hardware
> semaphores but no-one seems too sure!). This causes a problem if the

It is for semaphores, and since 9d7730914f4cd we have extensive testcases
for this case thanks to Mika. Originally this was added in 1ec14ad313270
but somehow the details in the comments where lost.

So maybe no human knows it any more, but git log surely still does ;-)
-Daniel

> force idle occurs at an inopportune moment such as in the middle of
> submitting a batch buffer. Specifically, it would lead to recursive
> submits - submitting work requires a new seqno, the new seqno requires
> idling the ring, idling the ring requires submitting work, submitting
> work requires a new seqno...
> 
> This change adds a 'flush' parameter to the idle function call which
> specifies whether the scheduler queues should be flushed out. I.e. is
> the call intended to just idle the ring as it is right now (no flush)
> or is it intended to force all outstanding work out of the system
> (with flush).
> 
> In the seqno wrap case, pending work is not an issue because the next
> operation will be to submit it. However, in other cases, the intention
> is to make sure everything that could be done has been done.
> 
> Change-Id: I182e9a5853666c64ecc9e84d8a8b820a7f8e8836
> For: VIZ-1587
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c         |  4 ++--
>  drivers/gpu/drm/i915/intel_lrc.c        |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 17 +++++++++++++++--
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
>  4 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6d72caa..20c696f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2474,7 +2474,7 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
>  
>  	/* Carefully retire all requests without writing to the rings */
>  	for_each_ring(ring, dev_priv, i) {
> -		ret = intel_ring_idle(ring);
> +		ret = intel_ring_idle(ring, false);
>  		if (ret)
>  			return ret;
>  	}
> @@ -3732,7 +3732,7 @@ int i915_gpu_idle(struct drm_device *dev)
>  			i915_add_request_no_flush(req);
>  		}
>  
> -		ret = intel_ring_idle(ring);
> +		ret = intel_ring_idle(ring, true);
>  		if (ret)
>  			return ret;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 76d5023..a811d0b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -990,7 +990,7 @@ void intel_logical_ring_stop(struct intel_engine_cs *ring)
>  	if (!intel_ring_initialized(ring))
>  		return;
>  
> -	ret = intel_ring_idle(ring);
> +	ret = intel_ring_idle(ring, true);
>  	if (ret && !i915_reset_in_progress(&to_i915(ring->dev)->gpu_error))
>  		DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
>  			  ring->name, ret);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index e0992b7..afb04de 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2177,9 +2177,22 @@ static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
>  	intel_ring_update_space(ringbuf);
>  }
>  
> -int intel_ring_idle(struct intel_engine_cs *ring)
> +int intel_ring_idle(struct intel_engine_cs *ring, bool flush)
>  {
>  	struct drm_i915_gem_request *req;
> +	int ret;
> +
> +	/*
> +	 * NB: Must not flush the scheduler if this idle request is from
> +	 * within an execbuff submission (i.e. due to 'get_seqno' calling
> +	 * 'wrap_seqno' calling 'idle'). As that would lead to recursive
> +	 * flushes!
> +	 */
> +	if (flush) {
> +		ret = i915_scheduler_flush(ring, true);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	/* Wait upon the last request to be completed */
>  	if (list_empty(&ring->request_list))
> @@ -2983,7 +2996,7 @@ intel_stop_ring_buffer(struct intel_engine_cs *ring)
>  	if (!intel_ring_initialized(ring))
>  		return;
>  
> -	ret = intel_ring_idle(ring);
> +	ret = intel_ring_idle(ring, true);
>  	if (ret && !i915_reset_in_progress(&to_i915(ring->dev)->gpu_error))
>  		DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
>  			  ring->name, ret);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 9457774..2f30900 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -487,7 +487,7 @@ void intel_ring_update_space(struct intel_ringbuffer *ringbuf);
>  int intel_ring_space(struct intel_ringbuffer *ringbuf);
>  bool intel_ring_stopped(struct intel_engine_cs *ring);
>  
> -int __must_check intel_ring_idle(struct intel_engine_cs *ring);
> +int __must_check intel_ring_idle(struct intel_engine_cs *ring, bool flush);
>  void intel_ring_init_seqno(struct intel_engine_cs *ring, u32 seqno);
>  int intel_ring_flush_all_caches(struct drm_i915_gem_request *req);
>  int intel_ring_invalidate_all_caches(struct drm_i915_gem_request *req);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch