Re: [PATCH 52/53] drm/i915: Remove the now obsolete 'outstanding_lazy_request'

[Tomas Elf <tomas.elf@intel.com>]

On 19/02/2015 17:18, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> The outstanding_lazy_request is no longer used anywhere in the driver.
> Everything that was looking at it now has a request explicitly passed in from on
> high. Everything that was relying upon behind the scenes is now explicitly
> creating/passing/submitting it's own private request. Thus the OLR can be
> removed.
>

"Everything that was relying upon behind the scenes is now explicitly 
creating/passing/submitting it's ..."
--->
"Everything that was relying on it behind the scenes is now explicitly 
creating/passing/submitting its ..." ?


> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c            |   16 +---------------
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |    4 +---
>   drivers/gpu/drm/i915/intel_lrc.c           |    6 ++----
>   drivers/gpu/drm/i915/intel_ringbuffer.c    |   17 ++---------------
>   drivers/gpu/drm/i915/intel_ringbuffer.h    |    4 ----
>   5 files changed, 6 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 60f6671..8e7418b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1156,15 +1156,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
>   int
>   i915_gem_check_olr(struct drm_i915_gem_request *req)
>   {
> -	int ret;
> -
>   	WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
>
> -	ret = 0;
> -	if (req == req->ring->outstanding_lazy_request)
> -		ret = i915_add_request(req);
> -
> -	return ret;
> +	return 0;
>   }
>
>   static void fake_irq(unsigned long data)
> @@ -2424,8 +2418,6 @@ int __i915_add_request(struct drm_i915_gem_request *request,
>   	dev_priv = ring->dev->dev_private;
>   	ringbuf = request->ringbuf;
>
> -	WARN_ON(request != ring->outstanding_lazy_request);
> -
>   	request_start = intel_ring_get_tail(ringbuf);
>   	/*
>   	 * Emit any outstanding flushes - execbuf can fail to emit the flush
> @@ -2486,7 +2478,6 @@ int __i915_add_request(struct drm_i915_gem_request *request,
>   	}
>
>   	trace_i915_gem_request_add(request);
> -	ring->outstanding_lazy_request = NULL;
>
>   	i915_queue_hangcheck(ring->dev);
>
> @@ -2672,9 +2663,6 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
>
>   		i915_gem_free_request(request);
>   	}
> -
> -	/* This may not have been flushed before the reset, so clean it now */
> -	i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
>   }
>
>   void i915_gem_restore_fences(struct drm_device *dev)
> @@ -3124,8 +3112,6 @@ int i915_gpu_idle(struct drm_device *dev)
>   			}
>   		}
>
> -		WARN_ON(ring->outstanding_lazy_request);
> -
>   		ret = intel_ring_idle(ring);
>   		if (ret)
>   			return ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 6a703e6..0eae592 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1571,10 +1571,8 @@ err:
>   	 * must be freed again. If it was submitted then it is being tracked
>   	 * on the active request list and no clean up is required here.
>   	 */
> -	if (ret && params->request) {
> +	if (ret && params->request)
>   		i915_gem_request_unreference(params->request);
> -		ring->outstanding_lazy_request = NULL;
> -	}
>
>   	mutex_unlock(&dev->struct_mutex);
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 2911cf6..db63ea0 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1032,8 +1032,7 @@ int intel_logical_ring_alloc_request(struct intel_engine_cs *ring,
>   	if (!req_out)
>   		return -EINVAL;
>
> -	if ((*req_out = ring->outstanding_lazy_request) != NULL)
> -		return 0;
> +	*req_out = NULL;
>
>   	request = kzalloc(sizeof(*request), GFP_KERNEL);
>   	if (request == NULL)
> @@ -1067,7 +1066,7 @@ int intel_logical_ring_alloc_request(struct intel_engine_cs *ring,
>   	i915_gem_context_reference(request->ctx);
>   	request->ringbuf = ctx->engine[ring->id].ringbuf;
>
> -	*req_out = ring->outstanding_lazy_request = request;
> +	*req_out = request;
>   	return 0;
>   }
>
> @@ -1393,7 +1392,6 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
>
>   	intel_logical_ring_stop(ring);
>   	WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
> -	i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
>
>   	if (ring->cleanup)
>   		ring->cleanup(ring);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 5eef02e..85daa18 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2034,7 +2034,6 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
>
>   	intel_unpin_ringbuffer_obj(ringbuf);
>   	intel_destroy_ringbuffer_obj(ringbuf);
> -	i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
>
>   	if (ring->cleanup)
>   		ring->cleanup(ring);
> @@ -2153,15 +2152,6 @@ static int intel_wrap_ring_buffer(struct intel_engine_cs *ring)
>   int intel_ring_idle(struct intel_engine_cs *ring)
>   {
>   	struct drm_i915_gem_request *req;
> -	int ret;
> -
> -	/* We need to add any requests required to flush the objects and ring */
> -	WARN_ON(ring->outstanding_lazy_request);
> -	if (ring->outstanding_lazy_request) {
> -		ret = i915_add_request(ring->outstanding_lazy_request);
> -		if (ret)
> -			return ret;
> -	}
>
>   	/* Wait upon the last request to be completed */
>   	if (list_empty(&ring->request_list))
> @@ -2186,8 +2176,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring,
>   	if (!req_out)
>   		return -EINVAL;
>
> -	if ((*req_out = ring->outstanding_lazy_request) != NULL)
> -		return 0;
> +	*req_out = NULL;
>
>   	request = kzalloc(sizeof(*request), GFP_KERNEL);
>   	if (request == NULL)
> @@ -2206,7 +2195,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring,
>   		return ret;
>   	}
>
> -	*req_out = ring->outstanding_lazy_request = request;
> +	*req_out = request;
>   	return 0;
>   }
>
> @@ -2281,8 +2270,6 @@ void intel_ring_init_seqno(struct intel_engine_cs *ring, u32 seqno)
>   	struct drm_device *dev = ring->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>
> -	BUG_ON(ring->outstanding_lazy_request);
> -
>   	if (INTEL_INFO(dev)->gen == 6 || INTEL_INFO(dev)->gen == 7) {
>   		I915_WRITE(RING_SYNC_0(ring->mmio_base), 0);
>   		I915_WRITE(RING_SYNC_1(ring->mmio_base), 0);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 3b0261f..d2c6427 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -258,10 +258,6 @@ struct  intel_engine_cs {
>   	 */
>   	struct list_head request_list;
>
> -	/**
> -	 * Do we have some not yet emitted requests outstanding?
> -	 */
> -	struct drm_i915_gem_request *outstanding_lazy_request;
>   	bool gpu_caches_dirty;
>   	bool fbc_dirty;
>
>

Since we're removing the i915_add_request from i915_check_olr and 
thereby removing the OLR emission the following comment at 
i915_gem_object_flush_active is no longer valid:

         /**
          * Ensures that an object will eventually get non-busy by 
flushing any required
          * write domains, emitting any outstanding lazy request and 
retiring and
          * completed requests.
          */
          static int
          i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
          {
                 struct intel_engine_cs *ring;
                 int ret;

                 if (obj->active) {
                         ring = 
i915_gem_request_get_ring(obj->last_read_req);
                         ret = i915_gem_check_olr(obj->last_read_req);


Thanks,
Tomas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx