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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

On 10/03/2015 10:18, Daniel Vetter wrote:

On Mon, Mar 09, 2015 at 11:51:26PM +0000, Tomas Elf wrote:

On 19/02/2015 17:18, John.C.Harrison@xxxxxxxxx wrote:

From: John Harrison <John.C.Harrison@xxxxxxxxx>

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@xxxxxxxxx>
---
  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);

Nice catch! On top of that the int return value is no longer needed, so a
follow-up patch should simplify it to void.
-Daniel
The comment also talks about flushing write domains but there is no obvious flush call. All it does is the check_olr (which is now a no-op) and the retire (which won't do anything unless the request has already completed). So is there any need for this function at all anymore? Or should it just be removed and replaced with a call to retire instead? 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux