[PATCH 00/29] Replace seqno values with request structures

[PATCH 00/29] Replace seqno values with request structures

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Work in progress for replacing seqno usage with requst structures.

There is a general feeling that it is better to move away from using a simple
integer 'seqno' value to track batch buffer completion. Instead, the request
structure should be used. That provides for much more flexibility going
forwards. Especially which things like a GPU scheduler (which can re-order batch
buffers and hence seqnos after submission to the hardware), Android sync points
and other such features which potentially make seqno usage more and more
complex.

The current set of patches do most of the seqno to request structure conversion.
There are still a couple of direct seqno comparisons in the semaphore code. The
final conversion of a seqno test into a 'completed' flag inside the request
structure is still do to as well. Along with whatever changes are required to
maintain such a flag.

The patches are being posted now to make sure that the various people involved
agree that it is heading in the right direction.

[Patches against drm-intel-nightly tree fetched 11/09/2014]

John Harrison (29):
  drm/i915: Remove redundant parameter to i915_gem_object_wait_rendering__tail()
  drm/i915: Ensure OLS & PLR are always in sync
  drm/i915: Add reference count to request structure
  drm/i915: Add helper functions to aid seqno -> request transition
  drm/i915: Replace last_[rwf]_seqno with last_[rwf]_req
  drm/i915: Convert i915_gem_ring_throttle to use requests
  drm/i915: Ensure requests stick around during waits
  drm/i915: Remove 'outstanding_lazy_seqno'
  drm/i915: Make 'i915_gem_check_olr' actually check by request not seqno
  drm/i915: Convert 'last_flip_req' to be a request not a seqno
  drm/i915: Convert i915_wait_seqno to i915_wait_request
  drm/i915: Convert __wait_seqno() to __wait_request()
  drm/i915: Remove obsolete seqno parameter from 'i915_add_request'
  drm/i915: Convert mmio_flip::seqno to struct request
  drm/i915: Add IRQ friendly request deference facility
  drm/i915: Convert 'flip_queued_seqno' into 'flip_queued_request'
  drm/i915: Convert trace functions from seqno to request
  drm/i915: Convert 'trace_irq' to use requests rather than seqnos
  drm/i915: Convert 'ring_idle()' to use requests not seqnos
  drm/i915: Connect requests to rings at creation not submission
  drm/i915: Convert 'i915_seqno_passed' calls into 'i915_gem_request_completed'
  drm/i915: Remove the now redundant 'obj->ring'
  drm/i915: Cache request completion status
  drm/i915: Zero fill the request structure
  drm/i915: Spinlock protection for request list
  drm/i915: Add uniq id to request structure for debugging
  drm/i915: Interrupt driven request completion
  drm/i915: Remove obsolete parameter to i915_gem_request_completed()
  WIP: Defer seqno allocation until actual hardware submission time

 drivers/gpu/drm/i915/i915_debugfs.c          |   20 +-
 drivers/gpu/drm/i915/i915_drv.h              |   89 ++++++-
 drivers/gpu/drm/i915/i915_gem.c              |  356 +++++++++++++++++---------
 drivers/gpu/drm/i915/i915_gem_context.c      |    3 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c   |   10 +-
 drivers/gpu/drm/i915/i915_gem_gtt.h          |    4 +-
 drivers/gpu/drm/i915/i915_gem_render_state.c |    2 +-
 drivers/gpu/drm/i915/i915_gem_tiling.c       |    2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c        |    7 +-
 drivers/gpu/drm/i915/i915_irq.c              |   14 +-
 drivers/gpu/drm/i915/i915_trace.h            |   47 ++--
 drivers/gpu/drm/i915/intel_display.c         |   53 ++--
 drivers/gpu/drm/i915/intel_drv.h             |    4 +-
 drivers/gpu/drm/i915/intel_lrc.c             |   59 +++--
 drivers/gpu/drm/i915/intel_overlay.c         |   24 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c      |   82 +++---
 drivers/gpu/drm/i915/intel_ringbuffer.h      |   25 +-
 17 files changed, 505 insertions(+), 296 deletions(-)

-- 
1.7.9.5

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

[PATCH 01/29] drm/i915: Remove redundant parameter to i915_gem_object_wait_rendering__tail()

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

An earlier commit (c8725f3dc0911d4354315a65150aecd8b7d0d74a: Do not call
retire_requests from wait_for_rendering) removed the use of the ring parameter
within wait_rendering__tail() but did not remove the parameter itself. As the
plan is to remove obj->ring which is where this parameter comes from, it is
simpler to just remove the parameter completely than to update it with a new
source.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
CC: Brad Volkin <bradley.d.volkin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7e91978..c2456c2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1281,8 +1281,7 @@ i915_wait_seqno(struct intel_engine_cs *ring, uint32_t seqno)
 }
 
 static int
-i915_gem_object_wait_rendering__tail(struct drm_i915_gem_object *obj,
-				     struct intel_engine_cs *ring)
+i915_gem_object_wait_rendering__tail(struct drm_i915_gem_object *obj)
 {
 	if (!obj->active)
 		return 0;
@@ -1319,7 +1318,7 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
 	if (ret)
 		return ret;
 
-	return i915_gem_object_wait_rendering__tail(obj, ring);
+	return i915_gem_object_wait_rendering__tail(obj);
 }
 
 /* A nonblocking variant of the above wait. This is a highly dangerous routine
@@ -1359,7 +1358,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 	if (ret)
 		return ret;
 
-	return i915_gem_object_wait_rendering__tail(obj, ring);
+	return i915_gem_object_wait_rendering__tail(obj);
 }
 
 /**
-- 
1.7.9.5

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

Re: [PATCH 01/29] drm/i915: Remove redundant parameter to i915_gem_object_wait_rendering__tail()

[Daniel Vetter]

On Thu, Oct 30, 2014 at 06:40:53PM +0000, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> An earlier commit (c8725f3dc0911d4354315a65150aecd8b7d0d74a: Do not call
> retire_requests from wait_for_rendering) removed the use of the ring parameter
> within wait_rendering__tail() but did not remove the parameter itself. As the
> plan is to remove obj->ring which is where this parameter comes from, it is
> simpler to just remove the parameter completely than to update it with a new
> source.
> 
> For: VIZ-4377
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Brad Volkin <bradley.d.volkin@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/29] drm/i915: Ensure OLS & PLR are always in sync

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The aim is to replace seqno values with request structures. A step along the way
is to switch to using the PLR in preference to the OLS. That requires the PLR to
only be valid when and only when the OLS is also valid. I.e., the two must be
kept in lock step. Then, code which was using the OLS can be safely switched
over to using the PLR instead.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        |   42 ++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_ringbuffer.c |   29 +++++++++++++++------
 2 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index cd74e5c..2aa65c8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -796,27 +796,39 @@ void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf)
 static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
 				    struct intel_context *ctx)
 {
-	if (ring->outstanding_lazy_seqno)
-		return 0;
+	struct drm_i915_gem_request *request;
+	int ret;
 
-	if (ring->preallocated_lazy_request == NULL) {
-		struct drm_i915_gem_request *request;
+	/* XXX: The aim is to replace seqno values with request structures.
+	 * A step along the way is to switch to using the PLR in preference
+	 * to the OLS. That requires the PLR to only be valid when the OLS is
+	 * also valid. I.e., the two must be kept in step. */
 
-		request = kmalloc(sizeof(*request), GFP_KERNEL);
-		if (request == NULL)
-			return -ENOMEM;
+	if (ring->outstanding_lazy_seqno) {
+		WARN_ON(ring->preallocated_lazy_request == NULL);
+		return 0;
+	}
+	WARN_ON(ring->preallocated_lazy_request != NULL);
 
-		/* Hold a reference to the context this request belongs to
-		 * (we will need it when the time comes to emit/retire the
-		 * request).
-		 */
-		request->ctx = ctx;
-		i915_gem_context_reference(request->ctx);
+	request = kmalloc(sizeof(*request), GFP_KERNEL);
+	if (request == NULL)
+		return -ENOMEM;
 
-		ring->preallocated_lazy_request = request;
+	ret = i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
+	if (ret) {
+		kfree(request);
+		return ret;
 	}
 
-	return i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
+	/* Hold a reference to the context this request belongs to
+	 * (we will need it when the time comes to emit/retire the
+	 * request).
+	 */
+	request->ctx = ctx;
+	i915_gem_context_reference(request->ctx);
+
+	ring->preallocated_lazy_request = request;
+	return 0;
 }
 
 static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a8f72e8..153de1c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2016,20 +2016,33 @@ int intel_ring_idle(struct intel_engine_cs *ring)
 static int
 intel_ring_alloc_seqno(struct intel_engine_cs *ring)
 {
-	if (ring->outstanding_lazy_seqno)
+	int ret;
+	struct drm_i915_gem_request *request;
+
+	/* XXX: The aim is to replace seqno values with request structures.
+	 * A step along the way is to switch to using the PLR in preference
+	 * to the OLS. That requires the PLR to only be valid when the OLS
+	 * is also valid. I.e., the two must be kept in step. */
+
+	if (ring->outstanding_lazy_seqno) {
+		WARN_ON(ring->preallocated_lazy_request == NULL);
 		return 0;
+	}
 
-	if (ring->preallocated_lazy_request == NULL) {
-		struct drm_i915_gem_request *request;
+	WARN_ON(ring->preallocated_lazy_request != NULL);
 
-		request = kmalloc(sizeof(*request), GFP_KERNEL);
-		if (request == NULL)
-			return -ENOMEM;
+	request = kmalloc(sizeof(*request), GFP_KERNEL);
+	if (request == NULL)
+		return -ENOMEM;
 
-		ring->preallocated_lazy_request = request;
+	ret = i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
+	if (ret) {
+		kfree(request);
+		return ret;
 	}
 
-	return i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
+	ring->preallocated_lazy_request = request;
+	return 0;
 }
 
 static int __intel_ring_prepare(struct intel_engine_cs *ring,
-- 
1.7.9.5

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

[PATCH 03/29] drm/i915: Add reference count to request structure

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The plan is to use request structures everywhere that seqno values were
previously used. This means saving pointers to structures in places that used to
be simple integers. In turn, that means that the target structure now needs much
more stringent lifetime tracking. That is, it must not be freed while some other
random object still holds a pointer to it.

To achieve this tracking, a reference count needs to be added. Whenever a
pointer to the structure is saved away, the count must be incremented and the
free must only occur when all references have been released.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |   28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem.c         |   17 ++++++++++++-----
 drivers/gpu/drm/i915/intel_lrc.c        |    4 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.c |    4 +++-
 4 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6a73803..6f92977 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1922,6 +1922,8 @@ void i915_gem_track_fb(struct drm_i915_gem_object *old,
  * an emission time with seqnos for tracking how far ahead of the GPU we are.
  */
 struct drm_i915_gem_request {
+	struct kref ref;
+
 	/** On Which ring this request was generated */
 	struct intel_engine_cs *ring;
 
@@ -1951,6 +1953,32 @@ struct drm_i915_gem_request {
 	struct list_head client_list;
 };
 
+void i915_gem_request_free(struct kref *req_ref);
+
+static inline void
+i915_gem_request_reference(struct drm_i915_gem_request *req)
+{
+	kref_get(&req->ref);
+}
+
+static inline void
+i915_gem_request_unreference(struct drm_i915_gem_request *req)
+{
+	kref_put(&req->ref, i915_gem_request_free);
+}
+
+static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
+					   struct drm_i915_gem_request *src)
+{
+	if (src)
+		i915_gem_request_reference(src);
+
+	if (*pdst)
+		i915_gem_request_unreference(*pdst);
+
+	*pdst = src;
+}
+
 struct drm_i915_file_private {
 	struct drm_i915_private *dev_priv;
 	struct drm_file *file;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c2456c2..d46a70d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2496,10 +2496,18 @@ static void i915_gem_free_request(struct drm_i915_gem_request *request)
 	list_del(&request->list);
 	i915_gem_request_remove_from_client(request);
 
-	if (request->ctx)
-		i915_gem_context_unreference(request->ctx);
+	i915_gem_request_unreference(request);
+}
+
+void i915_gem_request_free(struct kref *req_ref)
+{
+	struct drm_i915_gem_request *req = container_of(req_ref,
+						 typeof(*req), ref);
 
-	kfree(request);
+	if (req->ctx)
+		i915_gem_context_unreference(req->ctx);
+
+	kfree(req);
 }
 
 struct drm_i915_gem_request *
@@ -2582,8 +2590,7 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 	}
 
 	/* These may not have been flush before the reset, do so now */
-	kfree(ring->preallocated_lazy_request);
-	ring->preallocated_lazy_request = NULL;
+	i915_gem_request_assign(&ring->preallocated_lazy_request, NULL);
 	ring->outstanding_lazy_seqno = 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2aa65c8..5b4054e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -814,6 +814,8 @@ static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
 	if (request == NULL)
 		return -ENOMEM;
 
+	kref_init(&request->ref);
+
 	ret = i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
 	if (ret) {
 		kfree(request);
@@ -1233,7 +1235,7 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
 
 	intel_logical_ring_stop(ring);
 	WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
-	ring->preallocated_lazy_request = NULL;
+	i915_gem_request_assign(&ring->preallocated_lazy_request, NULL);
 	ring->outstanding_lazy_seqno = 0;
 
 	if (ring->cleanup)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 153de1c..6659a08 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1855,7 +1855,7 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
 	WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
 
 	intel_destroy_ringbuffer_obj(ringbuf);
-	ring->preallocated_lazy_request = NULL;
+	i915_gem_request_assign(&ring->preallocated_lazy_request, NULL);
 	ring->outstanding_lazy_seqno = 0;
 
 	if (ring->cleanup)
@@ -2035,6 +2035,8 @@ intel_ring_alloc_seqno(struct intel_engine_cs *ring)
 	if (request == NULL)
 		return -ENOMEM;
 
+	kref_init(&request->ref);
+
 	ret = i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
 	if (ret) {
 		kfree(request);
-- 
1.7.9.5

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

[PATCH 04/29] drm/i915: Add helper functions to aid seqno -> request transition

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Added helper functions for retreiving the ring and seqno entries from a request
structure. This allows the internal workings of the request structure to be
hidden from code that is using these. It also allows for useful
workarounds/debug code to be added as or when necessary.

Note that it is intended that the majority (if not all) uses of the seqno
accessor will disappear eventually as code is updated to use the request
structure itself rather than working with seqno values.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |   12 ++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h |    7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6f92977..7a043b8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1955,6 +1955,18 @@ struct drm_i915_gem_request {
 
 void i915_gem_request_free(struct kref *req_ref);
 
+static inline uint32_t
+i915_gem_request_get_seqno(struct drm_i915_gem_request *req)
+{
+	return req ? req->seqno : 0;
+}
+
+static inline struct intel_engine_cs *
+i915_gem_request_get_ring(struct drm_i915_gem_request *req)
+{
+	return req ? req->ring : NULL;
+}
+
 static inline void
 i915_gem_request_reference(struct drm_i915_gem_request *req)
 {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 96479c8..cc1b62f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -435,6 +435,13 @@ static inline u32 intel_ring_get_seqno(struct intel_engine_cs *ring)
 	return ring->outstanding_lazy_seqno;
 }
 
+static inline struct drm_i915_gem_request *
+intel_ring_get_request(struct intel_engine_cs *ring)
+{
+	BUG_ON(ring->preallocated_lazy_request == NULL);
+	return ring->preallocated_lazy_request;
+}
+
 static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
 {
 	if (ring->trace_irq_seqno == 0 && ring->irq_get(ring))
-- 
1.7.9.5

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

[PATCH 05/29] drm/i915: Replace last_[rwf]_seqno with last_[rwf]_req

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The object structure contains the last read, write and fenced seqno values for
use in syncrhonisation operations. These have now been replaced with their
request structure counterparts.

Note that to ensure that objects do not end up with dangling pointers, the
assignments of last_*_req include reference count updates. Thus a request cannot
be freed if an object is still hanging on to it for any reason.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |    6 +--
 drivers/gpu/drm/i915/i915_drv.h            |    6 +--
 drivers/gpu/drm/i915/i915_gem.c            |   68 ++++++++++++++++------------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    6 +--
 drivers/gpu/drm/i915/i915_gem_gtt.h        |    4 +-
 drivers/gpu/drm/i915/i915_gem_tiling.c     |    2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c      |    4 +-
 drivers/gpu/drm/i915/intel_display.c       |   10 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    2 +-
 9 files changed, 58 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a79f83c..79ce5c0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -133,9 +133,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 		   obj->base.size / 1024,
 		   obj->base.read_domains,
 		   obj->base.write_domain,
-		   obj->last_read_seqno,
-		   obj->last_write_seqno,
-		   obj->last_fenced_seqno,
+		   i915_gem_request_get_seqno(obj->last_read_req),
+		   i915_gem_request_get_seqno(obj->last_write_req),
+		   i915_gem_request_get_seqno(obj->last_fenced_req),
 		   i915_cache_level_str(to_i915(obj->base.dev), obj->cache_level),
 		   obj->dirty ? " dirty" : "",
 		   obj->madv == I915_MADV_DONTNEED ? " purgeable" : "");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7a043b8..059ec43 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1871,10 +1871,10 @@ struct drm_i915_gem_object {
 	struct intel_engine_cs *ring;
 
 	/** Breadcrumb of last rendering to the buffer. */
-	uint32_t last_read_seqno;
-	uint32_t last_write_seqno;
+	struct drm_i915_gem_request *last_read_req;
+	struct drm_i915_gem_request *last_write_req;
 	/** Breadcrumb of last fenced GPU access to the buffer. */
-	uint32_t last_fenced_seqno;
+	struct drm_i915_gem_request *last_fenced_req;
 
 	/** Current tiling stride for the object, if it's tiled. */
 	uint32_t stride;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d46a70d..a28432c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1289,11 +1289,11 @@ i915_gem_object_wait_rendering__tail(struct drm_i915_gem_object *obj)
 	/* Manually manage the write flush as we may have not yet
 	 * retired the buffer.
 	 *
-	 * Note that the last_write_seqno is always the earlier of
-	 * the two (read/write) seqno, so if we haved successfully waited,
+	 * Note that the last_write_req is always the earlier of
+	 * the two (read/write) requests, so if we haved successfully waited,
 	 * we know we have passed the last write.
 	 */
-	obj->last_write_seqno = 0;
+	i915_gem_request_assign(&obj->last_write_req, NULL);
 
 	return 0;
 }
@@ -1306,14 +1306,18 @@ static __must_check int
 i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
 			       bool readonly)
 {
+	struct drm_i915_gem_request *req;
 	struct intel_engine_cs *ring = obj->ring;
 	u32 seqno;
 	int ret;
 
-	seqno = readonly ? obj->last_write_seqno : obj->last_read_seqno;
-	if (seqno == 0)
+	req = readonly ? obj->last_write_req : obj->last_read_req;
+	if (!req)
 		return 0;
 
+	seqno = i915_gem_request_get_seqno(req);
+	WARN_ON(seqno == 0);
+
 	ret = i915_wait_seqno(ring, seqno);
 	if (ret)
 		return ret;
@@ -1329,6 +1333,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 					    struct drm_i915_file_private *file_priv,
 					    bool readonly)
 {
+	struct drm_i915_gem_request *req;
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *ring = obj->ring;
@@ -1339,10 +1344,13 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
 	BUG_ON(!dev_priv->mm.interruptible);
 
-	seqno = readonly ? obj->last_write_seqno : obj->last_read_seqno;
-	if (seqno == 0)
+	req = readonly ? obj->last_write_req : obj->last_read_req;
+	if (!req)
 		return 0;
 
+	seqno = i915_gem_request_get_seqno(req);
+	WARN_ON(seqno == 0);
+
 	ret = i915_gem_check_wedge(&dev_priv->gpu_error, true);
 	if (ret)
 		return ret;
@@ -2179,12 +2187,12 @@ static void
 i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
 			       struct intel_engine_cs *ring)
 {
-	u32 seqno = intel_ring_get_seqno(ring);
+	struct drm_i915_gem_request *req = intel_ring_get_request(ring);
 
 	BUG_ON(ring == NULL);
-	if (obj->ring != ring && obj->last_write_seqno) {
-		/* Keep the seqno relative to the current ring */
-		obj->last_write_seqno = seqno;
+	if (obj->ring != ring && obj->last_write_req) {
+		/* Keep the request relative to the current ring */
+		i915_gem_request_assign(&obj->last_write_req, req);
 	}
 	obj->ring = ring;
 
@@ -2196,7 +2204,7 @@ i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
 
 	list_move_tail(&obj->ring_list, &ring->active_list);
 
-	obj->last_read_seqno = seqno;
+	i915_gem_request_assign(&obj->last_read_req, req);
 }
 
 void i915_vma_move_to_active(struct i915_vma *vma,
@@ -2227,11 +2235,11 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
 	list_del_init(&obj->ring_list);
 	obj->ring = NULL;
 
-	obj->last_read_seqno = 0;
-	obj->last_write_seqno = 0;
+	i915_gem_request_assign(&obj->last_read_req, NULL);
+	i915_gem_request_assign(&obj->last_write_req, NULL);
 	obj->base.write_domain = 0;
 
-	obj->last_fenced_seqno = 0;
+	i915_gem_request_assign(&obj->last_fenced_req, NULL);
 
 	obj->active = 0;
 	drm_gem_object_unreference(&obj->base);
@@ -2248,7 +2256,7 @@ i915_gem_object_retire(struct drm_i915_gem_object *obj)
 		return;
 
 	if (i915_seqno_passed(ring->get_seqno(ring, true),
-			      obj->last_read_seqno))
+			      i915_gem_request_get_seqno(obj->last_read_req)))
 		i915_gem_object_move_to_inactive(obj);
 }
 
@@ -2663,7 +2671,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 				      struct drm_i915_gem_object,
 				      ring_list);
 
-		if (!i915_seqno_passed(seqno, obj->last_read_seqno))
+		if (!i915_seqno_passed(seqno, i915_gem_request_get_seqno(obj->last_read_req)))
 			break;
 
 		i915_gem_object_move_to_inactive(obj);
@@ -2773,7 +2781,7 @@ i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
 	int ret;
 
 	if (obj->active) {
-		ret = i915_gem_check_olr(obj->ring, obj->last_read_seqno);
+		ret = i915_gem_check_olr(obj->ring, i915_gem_request_get_seqno(obj->last_read_req));
 		if (ret)
 			return ret;
 
@@ -2834,13 +2842,12 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	if (ret)
 		goto out;
 
-	if (obj->active) {
-		seqno = obj->last_read_seqno;
-		ring = obj->ring;
-	}
+	if (!obj->active || !obj->last_read_req)
+		goto out;
 
-	if (seqno == 0)
-		 goto out;
+	seqno = i915_gem_request_get_seqno(obj->last_read_req);
+	WARN_ON(seqno == 0);
+	ring = obj->ring;
 
 	/* Do this after OLR check to make sure we make forward progress polling
 	 * on this IOCTL with a timeout <=0 (like busy ioctl)
@@ -2891,7 +2898,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
 
 	idx = intel_ring_sync_index(from, to);
 
-	seqno = obj->last_read_seqno;
+	seqno = i915_gem_request_get_seqno(obj->last_read_req);
 	/* Optimization: Avoid semaphore sync when we are sure we already
 	 * waited for an object with higher seqno */
 	if (seqno <= from->semaphore.sync_seqno[idx])
@@ -2904,11 +2911,12 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
 	trace_i915_gem_ring_sync_to(from, to, seqno);
 	ret = to->semaphore.sync_to(to, from, seqno);
 	if (!ret)
-		/* We use last_read_seqno because sync_to()
+		/* We use last_read_req because sync_to()
 		 * might have just caused seqno wrap under
 		 * the radar.
 		 */
-		from->semaphore.sync_seqno[idx] = obj->last_read_seqno;
+		from->semaphore.sync_seqno[idx] =
+				i915_gem_request_get_seqno(obj->last_read_req);
 
 	return ret;
 }
@@ -3222,12 +3230,12 @@ static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj,
 static int
 i915_gem_object_wait_fence(struct drm_i915_gem_object *obj)
 {
-	if (obj->last_fenced_seqno) {
-		int ret = i915_wait_seqno(obj->ring, obj->last_fenced_seqno);
+	if (obj->last_fenced_req) {
+		int ret = i915_wait_seqno(obj->ring, i915_gem_request_get_seqno(obj->last_fenced_req));
 		if (ret)
 			return ret;
 
-		obj->last_fenced_seqno = 0;
+		i915_gem_request_assign(&obj->last_fenced_req, NULL);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 4b7f5c1..7ed09fc 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -943,7 +943,7 @@ void
 i915_gem_execbuffer_move_to_active(struct list_head *vmas,
 				   struct intel_engine_cs *ring)
 {
-	u32 seqno = intel_ring_get_seqno(ring);
+	struct drm_i915_gem_request *req = intel_ring_get_request(ring);
 	struct i915_vma *vma;
 
 	list_for_each_entry(vma, vmas, exec_list) {
@@ -960,7 +960,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
 		i915_vma_move_to_active(vma, ring);
 		if (obj->base.write_domain) {
 			obj->dirty = 1;
-			obj->last_write_seqno = seqno;
+			i915_gem_request_assign(&obj->last_write_req, req);
 
 			intel_fb_obj_invalidate(obj, ring);
 
@@ -968,7 +968,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
 			obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
 		}
 		if (entry->flags & EXEC_OBJECT_NEEDS_FENCE) {
-			obj->last_fenced_seqno = seqno;
+			i915_gem_request_assign(&obj->last_fenced_req, req);
 			if (entry->flags & __EXEC_OBJECT_HAS_FENCE) {
 				struct drm_i915_private *dev_priv = to_i915(ring->dev);
 				list_move_tail(&dev_priv->fence_regs[obj->fence_reg].lru_list,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index d0562d0..69d9ba1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -182,7 +182,7 @@ struct i915_address_space {
 	 * List of objects currently involved in rendering.
 	 *
 	 * Includes buffers having the contents of their GPU caches
-	 * flushed, not necessarily primitives.  last_rendering_seqno
+	 * flushed, not necessarily primitives.  last_rendering_req
 	 * represents when the rendering involved will be completed.
 	 *
 	 * A reference is held on the buffer while on this list.
@@ -193,7 +193,7 @@ struct i915_address_space {
 	 * LRU list of objects which are not in the ringbuffer and
 	 * are ready to unbind, but are still in the GTT.
 	 *
-	 * last_rendering_seqno is 0 while an object is in this list.
+	 * last_rendering_req is NULL while an object is in this list.
 	 *
 	 * A reference is not held on the buffer while on this list,
 	 * as merely being GTT-bound shouldn't prevent its being
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index d1e7a3e..243ba12 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -394,7 +394,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 
 		if (ret == 0) {
 			obj->fence_dirty =
-				obj->last_fenced_seqno ||
+				obj->last_fenced_req ||
 				obj->fence_reg != I915_FENCE_REG_NONE;
 
 			obj->tiling_mode = args->tiling_mode;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index d17360b..10ba188 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -668,8 +668,8 @@ static void capture_bo(struct drm_i915_error_buffer *err,
 
 	err->size = obj->base.size;
 	err->name = obj->base.name;
-	err->rseqno = obj->last_read_seqno;
-	err->wseqno = obj->last_write_seqno;
+	err->rseqno = i915_gem_request_get_seqno(obj->last_read_req);
+	err->wseqno = i915_gem_request_get_seqno(obj->last_write_req);
 	err->gtt_offset = vma->node.start;
 	err->read_domains = obj->base.read_domains;
 	err->write_domain = obj->base.write_domain;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 92f0005..51aae75 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9341,16 +9341,16 @@ static int intel_postpone_flip(struct drm_i915_gem_object *obj)
 
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
 
-	if (!obj->last_write_seqno)
+	if (!obj->last_write_req)
 		return 0;
 
 	ring = obj->ring;
 
 	if (i915_seqno_passed(ring->get_seqno(ring, true),
-			      obj->last_write_seqno))
+			      i915_gem_request_get_seqno(obj->last_write_req)))
 		return 0;
 
-	ret = i915_gem_check_olr(ring, obj->last_write_seqno);
+	ret = i915_gem_check_olr(ring, i915_gem_request_get_seqno(obj->last_write_req));
 	if (ret)
 		return ret;
 
@@ -9412,7 +9412,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
 	}
 
 	spin_lock_irq(&dev_priv->mmio_flip_lock);
-	intel_crtc->mmio_flip.seqno = obj->last_write_seqno;
+	intel_crtc->mmio_flip.seqno = i915_gem_request_get_seqno(obj->last_write_req);
 	intel_crtc->mmio_flip.ring_id = obj->ring->id;
 	spin_unlock_irq(&dev_priv->mmio_flip_lock);
 
@@ -9616,7 +9616,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		if (ret)
 			goto cleanup_unpin;
 
-		work->flip_queued_seqno = obj->last_write_seqno;
+		work->flip_queued_seqno = i915_gem_request_get_seqno(obj->last_write_req);
 		work->flip_queued_ring = obj->ring;
 	} else {
 		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index cc1b62f..d98b964 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -249,7 +249,7 @@ struct  intel_engine_cs {
 	 * ringbuffer.
 	 *
 	 * Includes buffers having the contents of their GPU caches
-	 * flushed, not necessarily primitives.  last_rendering_seqno
+	 * flushed, not necessarily primitives.  last_rendering_req
 	 * represents when the rendering involved will be completed.
 	 *
 	 * A reference is held on the buffer while on this list.
-- 
1.7.9.5

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

Re: [PATCH 05/29] drm/i915: Replace last_[rwf]_seqno with last_[rwf]_req

[Daniel, Thomas]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of John.C.Harrison@Intel.com
> Sent: Thursday, October 30, 2014 6:41 PM
> To: Intel-GFX@Lists.FreeDesktop.Org
> Subject: [Intel-gfx] [PATCH 05/29] drm/i915: Replace last_[rwf]_seqno with
> last_[rwf]_req
> 
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> The object structure contains the last read, write and fenced seqno values
> for use in syncrhonisation operations. These have now been replaced with
> their request structure counterparts.
> 
> Note that to ensure that objects do not end up with dangling pointers, the
> assignments of last_*_req include reference count updates. Thus a request
> cannot be freed if an object is still hanging on to it for any reason.
> 
> For: VIZ-4377
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c        |    6 +--
>  drivers/gpu/drm/i915/i915_drv.h            |    6 +--
>  drivers/gpu/drm/i915/i915_gem.c            |   68 ++++++++++++++++------------
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |    6 +--
>  drivers/gpu/drm/i915/i915_gem_gtt.h        |    4 +-
>  drivers/gpu/drm/i915/i915_gem_tiling.c     |    2 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c      |    4 +-
>  drivers/gpu/drm/i915/intel_display.c       |   10 ++--
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |    2 +-
>  9 files changed, 58 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index a79f83c..79ce5c0 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -133,9 +133,9 @@ describe_obj(struct seq_file *m, struct
> drm_i915_gem_object *obj)
>  		   obj->base.size / 1024,
>  		   obj->base.read_domains,
>  		   obj->base.write_domain,
> -		   obj->last_read_seqno,
> -		   obj->last_write_seqno,
> -		   obj->last_fenced_seqno,
> +		   i915_gem_request_get_seqno(obj->last_read_req),
> +		   i915_gem_request_get_seqno(obj->last_write_req),
> +		   i915_gem_request_get_seqno(obj->last_fenced_req),
>  		   i915_cache_level_str(to_i915(obj->base.dev), obj-
> >cache_level),
>  		   obj->dirty ? " dirty" : "",
>  		   obj->madv == I915_MADV_DONTNEED ? " purgeable" : "");
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 7a043b8..059ec43 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1871,10 +1871,10 @@ struct drm_i915_gem_object {
>  	struct intel_engine_cs *ring;
> 
>  	/** Breadcrumb of last rendering to the buffer. */
> -	uint32_t last_read_seqno;
> -	uint32_t last_write_seqno;
> +	struct drm_i915_gem_request *last_read_req;
> +	struct drm_i915_gem_request *last_write_req;
>  	/** Breadcrumb of last fenced GPU access to the buffer. */
> -	uint32_t last_fenced_seqno;
> +	struct drm_i915_gem_request *last_fenced_req;
> 
>  	/** Current tiling stride for the object, if it's tiled. */
>  	uint32_t stride;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c index d46a70d..a28432c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1289,11 +1289,11 @@ i915_gem_object_wait_rendering__tail(struct
> drm_i915_gem_object *obj)
>  	/* Manually manage the write flush as we may have not yet
>  	 * retired the buffer.
>  	 *
> -	 * Note that the last_write_seqno is always the earlier of
> -	 * the two (read/write) seqno, so if we haved successfully waited,
> +	 * Note that the last_write_req is always the earlier of
> +	 * the two (read/write) requests, so if we haved successfully waited,
>  	 * we know we have passed the last write.
>  	 */
> -	obj->last_write_seqno = 0;
> +	i915_gem_request_assign(&obj->last_write_req, NULL);
> 
>  	return 0;
>  }
> @@ -1306,14 +1306,18 @@ static __must_check int
> i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
>  			       bool readonly)
>  {
> +	struct drm_i915_gem_request *req;
>  	struct intel_engine_cs *ring = obj->ring;
>  	u32 seqno;
>  	int ret;
> 
> -	seqno = readonly ? obj->last_write_seqno : obj->last_read_seqno;
> -	if (seqno == 0)
> +	req = readonly ? obj->last_write_req : obj->last_read_req;
> +	if (!req)
>  		return 0;
> 
> +	seqno = i915_gem_request_get_seqno(req);
> +	WARN_ON(seqno == 0);
> +
>  	ret = i915_wait_seqno(ring, seqno);
>  	if (ret)
>  		return ret;
> @@ -1329,6 +1333,7 @@
> i915_gem_object_wait_rendering__nonblocking(struct
> drm_i915_gem_object *obj,
>  					    struct drm_i915_file_private
> *file_priv,
>  					    bool readonly)
>  {
> +	struct drm_i915_gem_request *req;
>  	struct drm_device *dev = obj->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_engine_cs *ring = obj->ring; @@ -1339,10 +1344,13 @@
> i915_gem_object_wait_rendering__nonblocking(struct
> drm_i915_gem_object *obj,
>  	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
>  	BUG_ON(!dev_priv->mm.interruptible);
> 
> -	seqno = readonly ? obj->last_write_seqno : obj->last_read_seqno;
> -	if (seqno == 0)
> +	req = readonly ? obj->last_write_req : obj->last_read_req;
> +	if (!req)
>  		return 0;
> 
> +	seqno = i915_gem_request_get_seqno(req);
> +	WARN_ON(seqno == 0);
> +
>  	ret = i915_gem_check_wedge(&dev_priv->gpu_error, true);
>  	if (ret)
>  		return ret;
> @@ -2179,12 +2187,12 @@ static void
>  i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
>  			       struct intel_engine_cs *ring)  {
> -	u32 seqno = intel_ring_get_seqno(ring);
> +	struct drm_i915_gem_request *req = intel_ring_get_request(ring);
> 
>  	BUG_ON(ring == NULL);
> -	if (obj->ring != ring && obj->last_write_seqno) {
> -		/* Keep the seqno relative to the current ring */
> -		obj->last_write_seqno = seqno;
> +	if (obj->ring != ring && obj->last_write_req) {
> +		/* Keep the request relative to the current ring */
> +		i915_gem_request_assign(&obj->last_write_req, req);
>  	}
>  	obj->ring = ring;
> 
> @@ -2196,7 +2204,7 @@ i915_gem_object_move_to_active(struct
> drm_i915_gem_object *obj,
> 
>  	list_move_tail(&obj->ring_list, &ring->active_list);
> 
> -	obj->last_read_seqno = seqno;
> +	i915_gem_request_assign(&obj->last_read_req, req);
>  }
> 
>  void i915_vma_move_to_active(struct i915_vma *vma, @@ -2227,11
> +2235,11 @@ i915_gem_object_move_to_inactive(struct
> drm_i915_gem_object *obj)
>  	list_del_init(&obj->ring_list);
>  	obj->ring = NULL;
> 
> -	obj->last_read_seqno = 0;
> -	obj->last_write_seqno = 0;
> +	i915_gem_request_assign(&obj->last_read_req, NULL);
> +	i915_gem_request_assign(&obj->last_write_req, NULL);
>  	obj->base.write_domain = 0;
> 
> -	obj->last_fenced_seqno = 0;
> +	i915_gem_request_assign(&obj->last_fenced_req, NULL);
> 
>  	obj->active = 0;
>  	drm_gem_object_unreference(&obj->base);
> @@ -2248,7 +2256,7 @@ i915_gem_object_retire(struct
> drm_i915_gem_object *obj)
>  		return;
> 
>  	if (i915_seqno_passed(ring->get_seqno(ring, true),
> -			      obj->last_read_seqno))
> +			      i915_gem_request_get_seqno(obj-
> >last_read_req)))
>  		i915_gem_object_move_to_inactive(obj);
>  }
> 
> @@ -2663,7 +2671,7 @@ i915_gem_retire_requests_ring(struct
> intel_engine_cs *ring)
>  				      struct drm_i915_gem_object,
>  				      ring_list);
> 
> -		if (!i915_seqno_passed(seqno, obj->last_read_seqno))
> +		if (!i915_seqno_passed(seqno,
> +i915_gem_request_get_seqno(obj->last_read_req)))
>  			break;
> 
>  		i915_gem_object_move_to_inactive(obj);
> @@ -2773,7 +2781,7 @@ i915_gem_object_flush_active(struct
> drm_i915_gem_object *obj)
>  	int ret;
> 
>  	if (obj->active) {
> -		ret = i915_gem_check_olr(obj->ring, obj->last_read_seqno);
> +		ret = i915_gem_check_olr(obj->ring,
> +i915_gem_request_get_seqno(obj->last_read_req));
>  		if (ret)
>  			return ret;
> 
> @@ -2834,13 +2842,12 @@ i915_gem_wait_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file)
>  	if (ret)
>  		goto out;
> 
> -	if (obj->active) {
> -		seqno = obj->last_read_seqno;
> -		ring = obj->ring;
> -	}
> +	if (!obj->active || !obj->last_read_req)
> +		goto out;
> 
> -	if (seqno == 0)
> -		 goto out;
> +	seqno = i915_gem_request_get_seqno(obj->last_read_req);
> +	WARN_ON(seqno == 0);
> +	ring = obj->ring;
> 
>  	/* Do this after OLR check to make sure we make forward progress
> polling
>  	 * on this IOCTL with a timeout <=0 (like busy ioctl) @@ -2891,7
> +2898,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
> 
>  	idx = intel_ring_sync_index(from, to);
> 
> -	seqno = obj->last_read_seqno;
> +	seqno = i915_gem_request_get_seqno(obj->last_read_req);
>  	/* Optimization: Avoid semaphore sync when we are sure we
> already
>  	 * waited for an object with higher seqno */
>  	if (seqno <= from->semaphore.sync_seqno[idx]) @@ -2904,11
> +2911,12 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
>  	trace_i915_gem_ring_sync_to(from, to, seqno);
>  	ret = to->semaphore.sync_to(to, from, seqno);
>  	if (!ret)
> -		/* We use last_read_seqno because sync_to()
> +		/* We use last_read_req because sync_to()
>  		 * might have just caused seqno wrap under
>  		 * the radar.
>  		 */
> -		from->semaphore.sync_seqno[idx] = obj->last_read_seqno;
> +		from->semaphore.sync_seqno[idx] =
> +				i915_gem_request_get_seqno(obj-
> >last_read_req);
> 
>  	return ret;
>  }
> @@ -3222,12 +3230,12 @@ static void
> i915_gem_object_update_fence(struct drm_i915_gem_object *obj,  static
> int  i915_gem_object_wait_fence(struct drm_i915_gem_object *obj)  {
> -	if (obj->last_fenced_seqno) {
> -		int ret = i915_wait_seqno(obj->ring, obj-
> >last_fenced_seqno);
> +	if (obj->last_fenced_req) {
> +		int ret = i915_wait_seqno(obj->ring,
> +i915_gem_request_get_seqno(obj->last_fenced_req));
>  		if (ret)
>  			return ret;
> 
> -		obj->last_fenced_seqno = 0;
> +		i915_gem_request_assign(&obj->last_fenced_req, NULL);
>  	}
> 
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 4b7f5c1..7ed09fc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -943,7 +943,7 @@ void
>  i915_gem_execbuffer_move_to_active(struct list_head *vmas,
>  				   struct intel_engine_cs *ring)
>  {
> -	u32 seqno = intel_ring_get_seqno(ring);
> +	struct drm_i915_gem_request *req = intel_ring_get_request(ring);
>  	struct i915_vma *vma;
> 
>  	list_for_each_entry(vma, vmas, exec_list) { @@ -960,7 +960,7 @@
> i915_gem_execbuffer_move_to_active(struct list_head *vmas,
>  		i915_vma_move_to_active(vma, ring);
>  		if (obj->base.write_domain) {
>  			obj->dirty = 1;
> -			obj->last_write_seqno = seqno;
> +			i915_gem_request_assign(&obj->last_write_req,
> req);
> 
>  			intel_fb_obj_invalidate(obj, ring);
> 
> @@ -968,7 +968,7 @@ i915_gem_execbuffer_move_to_active(struct
> list_head *vmas,
>  			obj->base.write_domain &=
> ~I915_GEM_GPU_DOMAINS;
>  		}
>  		if (entry->flags & EXEC_OBJECT_NEEDS_FENCE) {
> -			obj->last_fenced_seqno = seqno;
> +			i915_gem_request_assign(&obj->last_fenced_req,
> req);
>  			if (entry->flags & __EXEC_OBJECT_HAS_FENCE) {
>  				struct drm_i915_private *dev_priv =
> to_i915(ring->dev);
>  				list_move_tail(&dev_priv->fence_regs[obj-
> >fence_reg].lru_list,
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index d0562d0..69d9ba1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -182,7 +182,7 @@ struct i915_address_space {
>  	 * List of objects currently involved in rendering.
>  	 *
>  	 * Includes buffers having the contents of their GPU caches
> -	 * flushed, not necessarily primitives.  last_rendering_seqno
> +	 * flushed, not necessarily primitives.  last_rendering_req
Last_rendering_req does not exist.  Do you mean last_read_req?

>  	 * represents when the rendering involved will be completed.
>  	 *
>  	 * A reference is held on the buffer while on this list.
> @@ -193,7 +193,7 @@ struct i915_address_space {
>  	 * LRU list of objects which are not in the ringbuffer and
>  	 * are ready to unbind, but are still in the GTT.
>  	 *
> -	 * last_rendering_seqno is 0 while an object is in this list.
> +	 * last_rendering_req is NULL while an object is in this list.
Same again.

Thomas.

>  	 *
>  	 * A reference is not held on the buffer while on this list,
>  	 * as merely being GTT-bound shouldn't prevent its being diff --git
> a/drivers/gpu/drm/i915/i915_gem_tiling.c
> b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index d1e7a3e..243ba12 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -394,7 +394,7 @@ i915_gem_set_tiling(struct drm_device *dev, void
> *data,
> 
>  		if (ret == 0) {
>  			obj->fence_dirty =
> -				obj->last_fenced_seqno ||
> +				obj->last_fenced_req ||
>  				obj->fence_reg != I915_FENCE_REG_NONE;
> 
>  			obj->tiling_mode = args->tiling_mode; diff --git
> a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index d17360b..10ba188 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -668,8 +668,8 @@ static void capture_bo(struct drm_i915_error_buffer
> *err,
> 
>  	err->size = obj->base.size;
>  	err->name = obj->base.name;
> -	err->rseqno = obj->last_read_seqno;
> -	err->wseqno = obj->last_write_seqno;
> +	err->rseqno = i915_gem_request_get_seqno(obj->last_read_req);
> +	err->wseqno = i915_gem_request_get_seqno(obj->last_write_req);
>  	err->gtt_offset = vma->node.start;
>  	err->read_domains = obj->base.read_domains;
>  	err->write_domain = obj->base.write_domain; diff --git
> a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 92f0005..51aae75 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9341,16 +9341,16 @@ static int intel_postpone_flip(struct
> drm_i915_gem_object *obj)
> 
>  	lockdep_assert_held(&obj->base.dev->struct_mutex);
> 
> -	if (!obj->last_write_seqno)
> +	if (!obj->last_write_req)
>  		return 0;
> 
>  	ring = obj->ring;
> 
>  	if (i915_seqno_passed(ring->get_seqno(ring, true),
> -			      obj->last_write_seqno))
> +			      i915_gem_request_get_seqno(obj-
> >last_write_req)))
>  		return 0;
> 
> -	ret = i915_gem_check_olr(ring, obj->last_write_seqno);
> +	ret = i915_gem_check_olr(ring,
> +i915_gem_request_get_seqno(obj->last_write_req));
>  	if (ret)
>  		return ret;
> 
> @@ -9412,7 +9412,7 @@ static int intel_queue_mmio_flip(struct drm_device
> *dev,
>  	}
> 
>  	spin_lock_irq(&dev_priv->mmio_flip_lock);
> -	intel_crtc->mmio_flip.seqno = obj->last_write_seqno;
> +	intel_crtc->mmio_flip.seqno =
> +i915_gem_request_get_seqno(obj->last_write_req);
>  	intel_crtc->mmio_flip.ring_id = obj->ring->id;
>  	spin_unlock_irq(&dev_priv->mmio_flip_lock);
> 
> @@ -9616,7 +9616,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  		if (ret)
>  			goto cleanup_unpin;
> 
> -		work->flip_queued_seqno = obj->last_write_seqno;
> +		work->flip_queued_seqno =
> +i915_gem_request_get_seqno(obj->last_write_req);
>  		work->flip_queued_ring = obj->ring;
>  	} else {
>  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring, diff
> --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index cc1b62f..d98b964 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -249,7 +249,7 @@ struct  intel_engine_cs {
>  	 * ringbuffer.
>  	 *
>  	 * Includes buffers having the contents of their GPU caches
> -	 * flushed, not necessarily primitives.  last_rendering_seqno
> +	 * flushed, not necessarily primitives.  last_rendering_req
>  	 * represents when the rendering involved will be completed.
>  	 *
>  	 * A reference is held on the buffer while on this list.
> --
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/29] drm/i915: Convert i915_gem_ring_throttle to use requests

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Convert the throttle code to use the request structure rather than extracting a 
ring/seqno pair from it and using those. This is in preparation for
__wait_seqno() becoming __wait_request().

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a28432c..79a01b7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4014,10 +4014,8 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 	unsigned long recent_enough = jiffies - msecs_to_jiffies(20);
-	struct drm_i915_gem_request *request;
-	struct intel_engine_cs *ring = NULL;
+	struct drm_i915_gem_request *request, *target = NULL;
 	unsigned reset_counter;
-	u32 seqno = 0;
 	int ret;
 
 	ret = i915_gem_wait_for_error(&dev_priv->gpu_error);
@@ -4033,16 +4031,17 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 		if (time_after_eq(request->emitted_jiffies, recent_enough))
 			break;
 
-		ring = request->ring;
-		seqno = request->seqno;
+		target = request;
 	}
 	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
 	spin_unlock(&file_priv->mm.lock);
 
-	if (seqno == 0)
+	if (target == NULL)
 		return 0;
 
-	ret = __wait_seqno(ring, seqno, reset_counter, true, NULL, NULL);
+	ret = __wait_seqno(i915_gem_request_get_ring(target),
+			   i915_gem_request_get_seqno(target),
+			   reset_counter, true, NULL, NULL);
 	if (ret == 0)
 		queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
 
-- 
1.7.9.5

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

[PATCH 07/29] drm/i915: Ensure requests stick around during waits

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Added reference counting of the request structure around __wait_seqno() calls.
This is a precursor to updating the wait code itself to take the request rather
than a seqno. At that point, it would be a Bad Idea for a request object to be
retired and freed while the wait code is still using it.

Note that i915_wait_request() is safe without reference counting as the driver
mutex lock is held for the duration. Thus it is not possible for a free to occur
asynchronously.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 79a01b7..b63c605 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1360,9 +1360,11 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 		return ret;
 
 	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
+	i915_gem_request_reference(req);
 	mutex_unlock(&dev->struct_mutex);
 	ret = __wait_seqno(ring, seqno, reset_counter, true, NULL, file_priv);
 	mutex_lock(&dev->struct_mutex);
+	i915_gem_request_unreference(req);
 	if (ret)
 		return ret;
 
@@ -2819,6 +2821,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_wait *args = data;
 	struct drm_i915_gem_object *obj;
+	struct drm_i915_gem_request *req;
 	struct intel_engine_cs *ring = NULL;
 	unsigned reset_counter;
 	u32 seqno = 0;
@@ -2845,7 +2848,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	if (!obj->active || !obj->last_read_req)
 		goto out;
 
-	seqno = i915_gem_request_get_seqno(obj->last_read_req);
+	req = obj->last_read_req;
+	seqno = i915_gem_request_get_seqno(req);
 	WARN_ON(seqno == 0);
 	ring = obj->ring;
 
@@ -2859,10 +2863,15 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 
 	drm_gem_object_unreference(&obj->base);
 	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
+	i915_gem_request_reference(req);
 	mutex_unlock(&dev->struct_mutex);
 
-	return __wait_seqno(ring, seqno, reset_counter, true, &args->timeout_ns,
-			    file->driver_priv);
+	ret = __wait_seqno(ring, seqno, reset_counter, true, &args->timeout_ns,
+			   file->driver_priv);
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_request_unreference(req);
+	mutex_unlock(&dev->struct_mutex);
+	return ret;
 
 out:
 	drm_gem_object_unreference(&obj->base);
@@ -4034,6 +4043,8 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 		target = request;
 	}
 	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
+	if (target)
+		i915_gem_request_reference(target);
 	spin_unlock(&file_priv->mm.lock);
 
 	if (target == NULL)
@@ -4045,6 +4056,10 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 	if (ret == 0)
 		queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
 
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_request_unreference(target);
+	mutex_unlock(&dev->struct_mutex);
+
 	return ret;
 }
 
-- 
1.7.9.5

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

[PATCH 08/29] drm/i915: Remove 'outstanding_lazy_seqno'

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The OLS value is now obsolete. Exactly the same value is guarateed to be always
available as PLR->seqno. Thus it is safe to remove the OLS completely. And also
to rename the PLR to OLR to keep the 'outstanding lazy ...' naming convention
valid.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c            |   13 +++----
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
 drivers/gpu/drm/i915/intel_display.c       |    2 +-
 drivers/gpu/drm/i915/intel_lrc.c           |   26 +++++---------
 drivers/gpu/drm/i915/intel_ringbuffer.c    |   52 +++++++++++++++-------------
 drivers/gpu/drm/i915/intel_ringbuffer.h    |   13 ++-----
 6 files changed, 46 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b63c605..0a4452b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1108,7 +1108,7 @@ i915_gem_check_olr(struct intel_engine_cs *ring, u32 seqno)
 	BUG_ON(!mutex_is_locked(&ring->dev->struct_mutex));
 
 	ret = 0;
-	if (seqno == ring->outstanding_lazy_seqno)
+	if (seqno == i915_gem_request_get_seqno(ring->outstanding_lazy_request))
 		ret = i915_add_request(ring, NULL);
 
 	return ret;
@@ -2343,7 +2343,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	u32 request_ring_position, request_start;
 	int ret;
 
-	request = ring->preallocated_lazy_request;
+	request = ring->outstanding_lazy_request;
 	if (WARN_ON(request == NULL))
 		return -ENOMEM;
 
@@ -2388,7 +2388,6 @@ int __i915_add_request(struct intel_engine_cs *ring,
 			return ret;
 	}
 
-	request->seqno = intel_ring_get_seqno(ring);
 	request->ring = ring;
 	request->head = request_start;
 	request->tail = request_ring_position;
@@ -2425,8 +2424,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	}
 
 	trace_i915_gem_request_add(ring, request->seqno);
-	ring->outstanding_lazy_seqno = 0;
-	ring->preallocated_lazy_request = NULL;
+	ring->outstanding_lazy_request = NULL;
 
 	if (!dev_priv->ums.mm_suspended) {
 		i915_queue_hangcheck(ring->dev);
@@ -2599,9 +2597,8 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 		kfree(submit_req);
 	}
 
-	/* These may not have been flush before the reset, do so now */
-	i915_gem_request_assign(&ring->preallocated_lazy_request, NULL);
-	ring->outstanding_lazy_seqno = 0;
+	/* 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)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 7ed09fc..4833c35 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1167,7 +1167,7 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
 			return ret;
 	}
 
-	trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags);
+	trace_i915_gem_ring_dispatch(ring, i915_gem_request_get_seqno(intel_ring_get_request(ring)), flags);
 
 	i915_gem_execbuffer_move_to_active(vmas, ring);
 	i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 51aae75..a69b74e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9624,7 +9624,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		if (ret)
 			goto cleanup_unpin;
 
-		work->flip_queued_seqno = intel_ring_get_seqno(ring);
+		work->flip_queued_seqno = i915_gem_request_get_seqno(intel_ring_get_request(ring));
 		work->flip_queued_ring = ring;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 5b4054e..cf5e1f5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -793,22 +793,14 @@ void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf)
 	execlists_context_queue(ring, ctx, ringbuf->tail);
 }
 
-static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
-				    struct intel_context *ctx)
+static int logical_ring_alloc_request(struct intel_engine_cs *ring,
+				      struct intel_context *ctx)
 {
 	struct drm_i915_gem_request *request;
 	int ret;
 
-	/* XXX: The aim is to replace seqno values with request structures.
-	 * A step along the way is to switch to using the PLR in preference
-	 * to the OLS. That requires the PLR to only be valid when the OLS is
-	 * also valid. I.e., the two must be kept in step. */
-
-	if (ring->outstanding_lazy_seqno) {
-		WARN_ON(ring->preallocated_lazy_request == NULL);
+	if (ring->outstanding_lazy_request)
 		return 0;
-	}
-	WARN_ON(ring->preallocated_lazy_request != NULL);
 
 	request = kmalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
@@ -816,7 +808,7 @@ static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
 
 	kref_init(&request->ref);
 
-	ret = i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
+	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
 		kfree(request);
 		return ret;
@@ -829,7 +821,7 @@ static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
 	request->ctx = ctx;
 	i915_gem_context_reference(request->ctx);
 
-	ring->preallocated_lazy_request = request;
+	ring->outstanding_lazy_request = request;
 	return 0;
 }
 
@@ -997,7 +989,7 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords)
 		return ret;
 
 	/* Preallocate the olr before touching the ring */
-	ret = logical_ring_alloc_seqno(ring, ringbuf->FIXME_lrc_ctx);
+	ret = logical_ring_alloc_request(ring, ringbuf->FIXME_lrc_ctx);
 	if (ret)
 		return ret;
 
@@ -1212,7 +1204,8 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf)
 				(ring->status_page.gfx_addr +
 				(I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT)));
 	intel_logical_ring_emit(ringbuf, 0);
-	intel_logical_ring_emit(ringbuf, ring->outstanding_lazy_seqno);
+	intel_logical_ring_emit(ringbuf,
+		i915_gem_request_get_seqno(ring->outstanding_lazy_request));
 	intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
 	intel_logical_ring_emit(ringbuf, MI_NOOP);
 	intel_logical_ring_advance_and_submit(ringbuf);
@@ -1235,8 +1228,7 @@ 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->preallocated_lazy_request, NULL);
-	ring->outstanding_lazy_seqno = 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 6659a08..9b536b2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -914,17 +914,20 @@ static int gen8_rcs_signal(struct intel_engine_cs *signaller,
 		return ret;
 
 	for_each_ring(waiter, dev_priv, i) {
+		u32 seqno;
 		u64 gtt_offset = signaller->semaphore.signal_ggtt[i];
 		if (gtt_offset == MI_SEMAPHORE_SYNC_INVALID)
 			continue;
 
+		seqno = i915_gem_request_get_seqno(
+					   signaller->outstanding_lazy_request);
 		intel_ring_emit(signaller, GFX_OP_PIPE_CONTROL(6));
 		intel_ring_emit(signaller, PIPE_CONTROL_GLOBAL_GTT_IVB |
 					   PIPE_CONTROL_QW_WRITE |
 					   PIPE_CONTROL_FLUSH_ENABLE);
 		intel_ring_emit(signaller, lower_32_bits(gtt_offset));
 		intel_ring_emit(signaller, upper_32_bits(gtt_offset));
-		intel_ring_emit(signaller, signaller->outstanding_lazy_seqno);
+		intel_ring_emit(signaller, seqno);
 		intel_ring_emit(signaller, 0);
 		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
 					   MI_SEMAPHORE_TARGET(waiter->id));
@@ -952,16 +955,19 @@ static int gen8_xcs_signal(struct intel_engine_cs *signaller,
 		return ret;
 
 	for_each_ring(waiter, dev_priv, i) {
+		u32 seqno;
 		u64 gtt_offset = signaller->semaphore.signal_ggtt[i];
 		if (gtt_offset == MI_SEMAPHORE_SYNC_INVALID)
 			continue;
 
+		seqno = i915_gem_request_get_seqno(
+					   signaller->outstanding_lazy_request);
 		intel_ring_emit(signaller, (MI_FLUSH_DW + 1) |
 					   MI_FLUSH_DW_OP_STOREDW);
 		intel_ring_emit(signaller, lower_32_bits(gtt_offset) |
 					   MI_FLUSH_DW_USE_GTT);
 		intel_ring_emit(signaller, upper_32_bits(gtt_offset));
-		intel_ring_emit(signaller, signaller->outstanding_lazy_seqno);
+		intel_ring_emit(signaller, seqno);
 		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
 					   MI_SEMAPHORE_TARGET(waiter->id));
 		intel_ring_emit(signaller, 0);
@@ -990,9 +996,11 @@ static int gen6_signal(struct intel_engine_cs *signaller,
 	for_each_ring(useless, dev_priv, i) {
 		u32 mbox_reg = signaller->semaphore.mbox.signal[i];
 		if (mbox_reg != GEN6_NOSYNC) {
+			u32 seqno = i915_gem_request_get_seqno(
+					   signaller->outstanding_lazy_request);
 			intel_ring_emit(signaller, MI_LOAD_REGISTER_IMM(1));
 			intel_ring_emit(signaller, mbox_reg);
-			intel_ring_emit(signaller, signaller->outstanding_lazy_seqno);
+			intel_ring_emit(signaller, seqno);
 		}
 	}
 
@@ -1027,7 +1035,8 @@ gen6_add_request(struct intel_engine_cs *ring)
 
 	intel_ring_emit(ring, MI_STORE_DWORD_INDEX);
 	intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT);
-	intel_ring_emit(ring, ring->outstanding_lazy_seqno);
+	intel_ring_emit(ring,
+		    i915_gem_request_get_seqno(ring->outstanding_lazy_request));
 	intel_ring_emit(ring, MI_USER_INTERRUPT);
 	__intel_ring_advance(ring);
 
@@ -1145,7 +1154,8 @@ pc_render_add_request(struct intel_engine_cs *ring)
 			PIPE_CONTROL_WRITE_FLUSH |
 			PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE);
 	intel_ring_emit(ring, ring->scratch.gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
-	intel_ring_emit(ring, ring->outstanding_lazy_seqno);
+	intel_ring_emit(ring,
+		    i915_gem_request_get_seqno(ring->outstanding_lazy_request));
 	intel_ring_emit(ring, 0);
 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
 	scratch_addr += 2 * CACHELINE_BYTES; /* write to separate cachelines */
@@ -1164,7 +1174,8 @@ pc_render_add_request(struct intel_engine_cs *ring)
 			PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE |
 			PIPE_CONTROL_NOTIFY);
 	intel_ring_emit(ring, ring->scratch.gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
-	intel_ring_emit(ring, ring->outstanding_lazy_seqno);
+	intel_ring_emit(ring,
+		    i915_gem_request_get_seqno(ring->outstanding_lazy_request));
 	intel_ring_emit(ring, 0);
 	__intel_ring_advance(ring);
 
@@ -1404,7 +1415,8 @@ i9xx_add_request(struct intel_engine_cs *ring)
 
 	intel_ring_emit(ring, MI_STORE_DWORD_INDEX);
 	intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT);
-	intel_ring_emit(ring, ring->outstanding_lazy_seqno);
+	intel_ring_emit(ring,
+		    i915_gem_request_get_seqno(ring->outstanding_lazy_request));
 	intel_ring_emit(ring, MI_USER_INTERRUPT);
 	__intel_ring_advance(ring);
 
@@ -1855,8 +1867,7 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
 	WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
 
 	intel_destroy_ringbuffer_obj(ringbuf);
-	i915_gem_request_assign(&ring->preallocated_lazy_request, NULL);
-	ring->outstanding_lazy_seqno = 0;
+	i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
 
 	if (ring->cleanup)
 		ring->cleanup(ring);
@@ -1996,7 +2007,7 @@ int intel_ring_idle(struct intel_engine_cs *ring)
 	int ret;
 
 	/* We need to add any requests required to flush the objects and ring */
-	if (ring->outstanding_lazy_seqno) {
+	if (ring->outstanding_lazy_request) {
 		ret = i915_add_request(ring, NULL);
 		if (ret)
 			return ret;
@@ -2014,22 +2025,13 @@ int intel_ring_idle(struct intel_engine_cs *ring)
 }
 
 static int
-intel_ring_alloc_seqno(struct intel_engine_cs *ring)
+intel_ring_alloc_request(struct intel_engine_cs *ring)
 {
 	int ret;
 	struct drm_i915_gem_request *request;
 
-	/* XXX: The aim is to replace seqno values with request structures.
-	 * A step along the way is to switch to using the PLR in preference
-	 * to the OLS. That requires the PLR to only be valid when the OLS
-	 * is also valid. I.e., the two must be kept in step. */
-
-	if (ring->outstanding_lazy_seqno) {
-		WARN_ON(ring->preallocated_lazy_request == NULL);
+	if (ring->outstanding_lazy_request)
 		return 0;
-	}
-
-	WARN_ON(ring->preallocated_lazy_request != NULL);
 
 	request = kmalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
@@ -2037,13 +2039,13 @@ intel_ring_alloc_seqno(struct intel_engine_cs *ring)
 
 	kref_init(&request->ref);
 
-	ret = i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
+	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
 		kfree(request);
 		return ret;
 	}
 
-	ring->preallocated_lazy_request = request;
+	ring->outstanding_lazy_request = request;
 	return 0;
 }
 
@@ -2084,7 +2086,7 @@ int intel_ring_begin(struct intel_engine_cs *ring,
 		return ret;
 
 	/* Preallocate the olr before touching the ring */
-	ret = intel_ring_alloc_seqno(ring);
+	ret = intel_ring_alloc_request(ring);
 	if (ret)
 		return ret;
 
@@ -2119,7 +2121,7 @@ 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_seqno);
+	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);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d98b964..98b219d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -265,8 +265,7 @@ struct  intel_engine_cs {
 	/**
 	 * Do we have some not yet emitted requests outstanding?
 	 */
-	struct drm_i915_gem_request *preallocated_lazy_request;
-	u32 outstanding_lazy_seqno;
+	struct drm_i915_gem_request *outstanding_lazy_request;
 	bool gpu_caches_dirty;
 	bool fbc_dirty;
 
@@ -429,17 +428,11 @@ static inline u32 intel_ring_get_tail(struct intel_ringbuffer *ringbuf)
 	return ringbuf->tail;
 }
 
-static inline u32 intel_ring_get_seqno(struct intel_engine_cs *ring)
-{
-	BUG_ON(ring->outstanding_lazy_seqno == 0);
-	return ring->outstanding_lazy_seqno;
-}
-
 static inline struct drm_i915_gem_request *
 intel_ring_get_request(struct intel_engine_cs *ring)
 {
-	BUG_ON(ring->preallocated_lazy_request == NULL);
-	return ring->preallocated_lazy_request;
+	BUG_ON(ring->outstanding_lazy_request == NULL);
+	return ring->outstanding_lazy_request;
 }
 
 static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
-- 
1.7.9.5

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

[PATCH 09/29] drm/i915: Make 'i915_gem_check_olr' actually check by request not seqno

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Updated the _check_olr() function to actually take a requst object and compare
it to the OLR rather than extracting seqnos and comparing those.

Note that there is one use case where the request object being processed is no
longer available at that point in the call stack. Hence a temporary copy of the
original function is still present (but called _check_ols() instead). This will
be removed in a subsequent patch.

Also, downgraded a BUG_ON to a WARN_ON as apparently the former is frowned upon
for shipping code.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   17 ++++++++++++++++-
 drivers/gpu/drm/i915/i915_gem.c      |   25 ++++++++++---------------
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 059ec43..dfd263d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2523,7 +2523,7 @@ bool i915_gem_retire_requests(struct drm_device *dev);
 void i915_gem_retire_requests_ring(struct intel_engine_cs *ring);
 int __must_check i915_gem_check_wedge(struct i915_gpu_error *error,
 				      bool interruptible);
-int __must_check i915_gem_check_olr(struct intel_engine_cs *ring, u32 seqno);
+int __must_check i915_gem_check_olr(struct drm_i915_gem_request *req);
 
 static inline bool i915_reset_in_progress(struct i915_gpu_error *error)
 {
@@ -3056,4 +3056,19 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 	}
 }
 
+/* XXX: Temporary solution to be removed later in patch series. */
+static inline int __must_check i915_gem_check_ols(struct intel_engine_cs *ring, u32 seqno)
+{
+	int ret;
+
+	WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+
+	ret = 0;
+	if (seqno == i915_gem_request_get_seqno(ring->outstanding_lazy_request))
+		ret = i915_add_request(ring, NULL);
+
+	return ret;
+}
+/* XXX: Temporary solution to be removed later in patch series. */
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0a4452b..97eec33 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1097,19 +1097,18 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
 }
 
 /*
- * Compare seqno against outstanding lazy request. Emit a request if they are
- * equal.
+ * Compare arbitrary request against outstanding lazy request. Emit on match.
  */
 int
-i915_gem_check_olr(struct intel_engine_cs *ring, u32 seqno)
+i915_gem_check_olr(struct drm_i915_gem_request *req)
 {
 	int ret;
 
-	BUG_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
 
 	ret = 0;
-	if (seqno == i915_gem_request_get_seqno(ring->outstanding_lazy_request))
-		ret = i915_add_request(ring, NULL);
+	if (req == req->ring->outstanding_lazy_request)
+		ret = i915_add_request(req->ring, NULL);
 
 	return ret;
 }
@@ -1271,7 +1270,7 @@ i915_wait_seqno(struct intel_engine_cs *ring, uint32_t seqno)
 	if (ret)
 		return ret;
 
-	ret = i915_gem_check_olr(ring, seqno);
+	ret = i915_gem_check_ols(ring, seqno);
 	if (ret)
 		return ret;
 
@@ -1338,7 +1337,6 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *ring = obj->ring;
 	unsigned reset_counter;
-	u32 seqno;
 	int ret;
 
 	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -1348,21 +1346,18 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 	if (!req)
 		return 0;
 
-	seqno = i915_gem_request_get_seqno(req);
-	WARN_ON(seqno == 0);
-
 	ret = i915_gem_check_wedge(&dev_priv->gpu_error, true);
 	if (ret)
 		return ret;
 
-	ret = i915_gem_check_olr(ring, seqno);
+	ret = i915_gem_check_olr(req);
 	if (ret)
 		return ret;
 
 	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
 	i915_gem_request_reference(req);
 	mutex_unlock(&dev->struct_mutex);
-	ret = __wait_seqno(ring, seqno, reset_counter, true, NULL, file_priv);
+	ret = __wait_seqno(ring, i915_gem_request_get_seqno(req), reset_counter, true, NULL, file_priv);
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_request_unreference(req);
 	if (ret)
@@ -2780,7 +2775,7 @@ i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
 	int ret;
 
 	if (obj->active) {
-		ret = i915_gem_check_olr(obj->ring, i915_gem_request_get_seqno(obj->last_read_req));
+		ret = i915_gem_check_olr(obj->last_read_req);
 		if (ret)
 			return ret;
 
@@ -2910,7 +2905,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
 	if (seqno <= from->semaphore.sync_seqno[idx])
 		return 0;
 
-	ret = i915_gem_check_olr(obj->ring, seqno);
+	ret = i915_gem_check_olr(obj->last_read_req);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a69b74e..fd488d2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9350,7 +9350,7 @@ static int intel_postpone_flip(struct drm_i915_gem_object *obj)
 			      i915_gem_request_get_seqno(obj->last_write_req)))
 		return 0;
 
-	ret = i915_gem_check_olr(ring, i915_gem_request_get_seqno(obj->last_write_req));
+	ret = i915_gem_check_olr(obj->last_write_req);
 	if (ret)
 		return ret;
 
-- 
1.7.9.5

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

[PATCH 10/29] drm/i915: Convert 'last_flip_req' to be a request not a seqno

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Converted 'last_flip_req' to be an actual request rather than a seqno value as
part of the on going seqno to request changes. This includes reference counting
the request being saved away to ensure it can not be retired and freed while the
overlay code is still waiting on it.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c |   19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index dc2f4f2..e6cda7e 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -182,7 +182,7 @@ struct intel_overlay {
 	u32 flip_addr;
 	struct drm_i915_gem_object *reg_bo;
 	/* flip handling */
-	uint32_t last_flip_req;
+	struct drm_i915_gem_request *last_flip_req;
 	void (*flip_tail)(struct intel_overlay *);
 };
 
@@ -217,17 +217,18 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
 	int ret;
 
 	BUG_ON(overlay->last_flip_req);
-	ret = i915_add_request(ring, &overlay->last_flip_req);
+	i915_gem_request_assign(&overlay->last_flip_req, ring->outstanding_lazy_request);
+	ret = i915_add_request(ring, NULL);
 	if (ret)
 		return ret;
 
 	overlay->flip_tail = tail;
-	ret = i915_wait_seqno(ring, overlay->last_flip_req);
+	ret = i915_wait_seqno(ring, i915_gem_request_get_seqno(overlay->last_flip_req));
 	if (ret)
 		return ret;
 	i915_gem_retire_requests(dev);
 
-	overlay->last_flip_req = 0;
+	i915_gem_request_assign(&overlay->last_flip_req, NULL);
 	return 0;
 }
 
@@ -286,7 +287,9 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
 	intel_ring_emit(ring, flip_addr);
 	intel_ring_advance(ring);
 
-	return i915_add_request(ring, &overlay->last_flip_req);
+	WARN_ON(overlay->last_flip_req);
+	i915_gem_request_assign(&overlay->last_flip_req, ring->outstanding_lazy_request);
+	return i915_add_request(ring, NULL);
 }
 
 static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
@@ -366,10 +369,10 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
 	struct intel_engine_cs *ring = &dev_priv->ring[RCS];
 	int ret;
 
-	if (overlay->last_flip_req == 0)
+	if (overlay->last_flip_req == NULL)
 		return 0;
 
-	ret = i915_wait_seqno(ring, overlay->last_flip_req);
+	ret = i915_wait_seqno(ring, i915_gem_request_get_seqno(overlay->last_flip_req));
 	if (ret)
 		return ret;
 	i915_gem_retire_requests(dev);
@@ -377,7 +380,7 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
 	if (overlay->flip_tail)
 		overlay->flip_tail(overlay);
 
-	overlay->last_flip_req = 0;
+	i915_gem_request_assign(&overlay->last_flip_req, NULL);
 	return 0;
 }
 
-- 
1.7.9.5

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

[PATCH 11/29] drm/i915: Convert i915_wait_seqno to i915_wait_request

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Updated i915_wait_seqno() to take a request structure instead of a seqno value
and renamed it accordingly. Internally, it just pulls the seqno out of the
request and calls on to __wait_seqno() as before. However, all the code further
up the stack is now simplified as it can just pass the request object straight
through without having to peek inside.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |   18 +-----------------
 drivers/gpu/drm/i915/i915_gem.c         |   30 +++++++++++++++---------------
 drivers/gpu/drm/i915/intel_lrc.c        |    6 ++----
 drivers/gpu/drm/i915/intel_overlay.c    |    9 +++------
 drivers/gpu/drm/i915/intel_ringbuffer.c |   14 ++++++--------
 5 files changed, 27 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dfd263d..adf1402 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2570,8 +2570,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 		       u32 *seqno);
 #define i915_add_request(ring, seqno) \
 	__i915_add_request(ring, NULL, NULL, seqno)
-int __must_check i915_wait_seqno(struct intel_engine_cs *ring,
-				 uint32_t seqno);
+int __must_check i915_wait_request(struct drm_i915_gem_request *req);
 int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
 int __must_check
 i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj,
@@ -3056,19 +3055,4 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 	}
 }
 
-/* XXX: Temporary solution to be removed later in patch series. */
-static inline int __must_check i915_gem_check_ols(struct intel_engine_cs *ring, u32 seqno)
-{
-	int ret;
-
-	WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
-
-	ret = 0;
-	if (seqno == i915_gem_request_get_seqno(ring->outstanding_lazy_request))
-		ret = i915_add_request(ring, NULL);
-
-	return ret;
-}
-/* XXX: Temporary solution to be removed later in patch series. */
-
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 97eec33..31bd385 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1252,29 +1252,34 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
 }
 
 /**
- * Waits for a sequence number to be signaled, and cleans up the
+ * Waits for a request to be signaled, and cleans up the
  * request and object lists appropriately for that event.
  */
 int
-i915_wait_seqno(struct intel_engine_cs *ring, uint32_t seqno)
+i915_wait_request(struct drm_i915_gem_request *req)
 {
-	struct drm_device *dev = ring->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	bool interruptible = dev_priv->mm.interruptible;
+	struct drm_device *dev;
+	struct drm_i915_private *dev_priv;
+	bool interruptible;
 	int ret;
 
+	BUG_ON(req == NULL);
+
+	dev = req->ring->dev;
+	dev_priv = dev->dev_private;
+	interruptible = dev_priv->mm.interruptible;
+
 	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
-	BUG_ON(seqno == 0);
 
 	ret = i915_gem_check_wedge(&dev_priv->gpu_error, interruptible);
 	if (ret)
 		return ret;
 
-	ret = i915_gem_check_ols(ring, seqno);
+	ret = i915_gem_check_olr(req);
 	if (ret)
 		return ret;
 
-	return __wait_seqno(ring, seqno,
+	return __wait_seqno(req->ring, i915_gem_request_get_seqno(req),
 			    atomic_read(&dev_priv->gpu_error.reset_counter),
 			    interruptible, NULL, NULL);
 }
@@ -1306,18 +1311,13 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
 			       bool readonly)
 {
 	struct drm_i915_gem_request *req;
-	struct intel_engine_cs *ring = obj->ring;
-	u32 seqno;
 	int ret;
 
 	req = readonly ? obj->last_write_req : obj->last_read_req;
 	if (!req)
 		return 0;
 
-	seqno = i915_gem_request_get_seqno(req);
-	WARN_ON(seqno == 0);
-
-	ret = i915_wait_seqno(ring, seqno);
+	ret = i915_wait_request(req);
 	if (ret)
 		return ret;
 
@@ -3232,7 +3232,7 @@ static int
 i915_gem_object_wait_fence(struct drm_i915_gem_object *obj)
 {
 	if (obj->last_fenced_req) {
-		int ret = i915_wait_seqno(obj->ring, i915_gem_request_get_seqno(obj->last_fenced_req));
+		int ret = i915_wait_request(obj->last_fenced_req);
 		if (ret)
 			return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index cf5e1f5..52ee9ca 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -830,7 +830,6 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf,
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
 	struct drm_i915_gem_request *request;
-	u32 seqno = 0;
 	int ret;
 
 	if (ringbuf->last_retired_head != -1) {
@@ -845,15 +844,14 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf,
 	list_for_each_entry(request, &ring->request_list, list) {
 		if (__intel_ring_space(request->tail, ringbuf->tail,
 				       ringbuf->size) >= bytes) {
-			seqno = request->seqno;
 			break;
 		}
 	}
 
-	if (seqno == 0)
+	if (&request->list == &ring->request_list)
 		return -ENOSPC;
 
-	ret = i915_wait_seqno(ring, seqno);
+	ret = i915_wait_request(request);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index e6cda7e..a28eea9 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -223,7 +223,7 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
 		return ret;
 
 	overlay->flip_tail = tail;
-	ret = i915_wait_seqno(ring, i915_gem_request_get_seqno(overlay->last_flip_req));
+	ret = i915_wait_request(overlay->last_flip_req);
 	if (ret)
 		return ret;
 	i915_gem_retire_requests(dev);
@@ -364,18 +364,15 @@ static int intel_overlay_off(struct intel_overlay *overlay)
  * We have to be careful not to repeat work forever an make forward progess. */
 static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
 {
-	struct drm_device *dev = overlay->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_engine_cs *ring = &dev_priv->ring[RCS];
 	int ret;
 
 	if (overlay->last_flip_req == NULL)
 		return 0;
 
-	ret = i915_wait_seqno(ring, i915_gem_request_get_seqno(overlay->last_flip_req));
+	ret = i915_wait_request(overlay->last_flip_req);
 	if (ret)
 		return ret;
-	i915_gem_retire_requests(dev);
+	i915_gem_retire_requests(overlay->dev);
 
 	if (overlay->flip_tail)
 		overlay->flip_tail(overlay);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9b536b2..9db1029 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1884,7 +1884,6 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n)
 {
 	struct intel_ringbuffer *ringbuf = ring->buffer;
 	struct drm_i915_gem_request *request;
-	u32 seqno = 0;
 	int ret;
 
 	if (ringbuf->last_retired_head != -1) {
@@ -1899,15 +1898,14 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n)
 	list_for_each_entry(request, &ring->request_list, list) {
 		if (__intel_ring_space(request->tail, ringbuf->tail,
 				       ringbuf->size) >= n) {
-			seqno = request->seqno;
 			break;
 		}
 	}
 
-	if (seqno == 0)
+	if (&request->list == &ring->request_list)
 		return -ENOSPC;
 
-	ret = i915_wait_seqno(ring, seqno);
+	ret = i915_wait_request(request);
 	if (ret)
 		return ret;
 
@@ -2003,7 +2001,7 @@ static int intel_wrap_ring_buffer(struct intel_engine_cs *ring)
 
 int intel_ring_idle(struct intel_engine_cs *ring)
 {
-	u32 seqno;
+	struct drm_i915_gem_request *req;
 	int ret;
 
 	/* We need to add any requests required to flush the objects and ring */
@@ -2017,11 +2015,11 @@ int intel_ring_idle(struct intel_engine_cs *ring)
 	if (list_empty(&ring->request_list))
 		return 0;
 
-	seqno = list_entry(ring->request_list.prev,
+	req = list_entry(ring->request_list.prev,
 			   struct drm_i915_gem_request,
-			   list)->seqno;
+			   list);
 
-	return i915_wait_seqno(ring, seqno);
+	return i915_wait_request(req);
 }
 
 static int
-- 
1.7.9.5

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

[PATCH 12/29] drm/i915: Convert __wait_seqno() to __wait_request()

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Now that all code above is using request structures instead of seqno values, it
is possible to convert  __wait_seqno() itself. Internally, it is still calling
i915_seqno_passed(), this will be updated later in the series. This step is just
changing the parameter list and function name.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c |   50 ++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 31bd385..2dcb8bf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1133,10 +1133,9 @@ static bool can_wait_boost(struct drm_i915_file_private *file_priv)
 }
 
 /**
- * __wait_seqno - wait until execution of seqno has finished
- * @ring: the ring expected to report seqno
- * @seqno: duh!
- * @reset_counter: reset sequence associated with the given seqno
+ * __wait_request - wait until execution of request has finished
+ * @req: duh!
+ * @reset_counter: reset sequence associated with the given request
  * @interruptible: do an interruptible wait (normally yes)
  * @timeout: in - how long to wait (NULL forever); out - how much time remaining
  *
@@ -1147,15 +1146,16 @@ static bool can_wait_boost(struct drm_i915_file_private *file_priv)
  * reset_counter _must_ be read before, and an appropriate smp_rmb must be
  * inserted.
  *
- * Returns 0 if the seqno was found within the alloted time. Else returns the
+ * Returns 0 if the request was found within the alloted time. Else returns the
  * errno with remaining time filled in timeout argument.
  */
-static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
-			unsigned reset_counter,
-			bool interruptible,
-			s64 *timeout,
-			struct drm_i915_file_private *file_priv)
+static int __wait_request(struct drm_i915_gem_request *req,
+			  unsigned reset_counter,
+			  bool interruptible,
+			  s64 *timeout,
+			  struct drm_i915_file_private *file_priv)
 {
+	struct intel_engine_cs *ring = i915_gem_request_get_ring(req);
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	const bool irq_test_in_progress =
@@ -1167,7 +1167,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
 
 	WARN(!intel_irqs_enabled(dev_priv), "IRQs disabled");
 
-	if (i915_seqno_passed(ring->get_seqno(ring, true), seqno))
+	if (i915_seqno_passed(ring->get_seqno(ring, true), req->seqno))
 		return 0;
 
 	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
@@ -1184,7 +1184,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
 		return -ENODEV;
 
 	/* Record current time in case interrupted by signal, or wedged */
-	trace_i915_gem_request_wait_begin(ring, seqno);
+	trace_i915_gem_request_wait_begin(i915_gem_request_get_ring(req), i915_gem_request_get_seqno(req));
 	before = ktime_get_raw_ns();
 	for (;;) {
 		struct timer_list timer;
@@ -1203,7 +1203,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
 			break;
 		}
 
-		if (i915_seqno_passed(ring->get_seqno(ring, false), seqno)) {
+		if (i915_seqno_passed(ring->get_seqno(ring, false), req->seqno)) {
 			ret = 0;
 			break;
 		}
@@ -1235,7 +1235,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
 		}
 	}
 	now = ktime_get_raw_ns();
-	trace_i915_gem_request_wait_end(ring, seqno);
+	trace_i915_gem_request_wait_end(i915_gem_request_get_ring(req), i915_gem_request_get_seqno(req));
 
 	if (!irq_test_in_progress)
 		ring->irq_put(ring);
@@ -1279,9 +1279,9 @@ i915_wait_request(struct drm_i915_gem_request *req)
 	if (ret)
 		return ret;
 
-	return __wait_seqno(req->ring, i915_gem_request_get_seqno(req),
-			    atomic_read(&dev_priv->gpu_error.reset_counter),
-			    interruptible, NULL, NULL);
+	return __wait_request(req,
+			      atomic_read(&dev_priv->gpu_error.reset_counter),
+			      interruptible, NULL, NULL);
 }
 
 static int
@@ -1335,7 +1335,6 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 	struct drm_i915_gem_request *req;
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_engine_cs *ring = obj->ring;
 	unsigned reset_counter;
 	int ret;
 
@@ -1357,7 +1356,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
 	i915_gem_request_reference(req);
 	mutex_unlock(&dev->struct_mutex);
-	ret = __wait_seqno(ring, i915_gem_request_get_seqno(req), reset_counter, true, NULL, file_priv);
+	ret = __wait_request(req, reset_counter, true, NULL, file_priv);
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_request_unreference(req);
 	if (ret)
@@ -2814,9 +2813,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	struct drm_i915_gem_wait *args = data;
 	struct drm_i915_gem_object *obj;
 	struct drm_i915_gem_request *req;
-	struct intel_engine_cs *ring = NULL;
 	unsigned reset_counter;
-	u32 seqno = 0;
 	int ret = 0;
 
 	if (args->flags != 0)
@@ -2841,9 +2838,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		goto out;
 
 	req = obj->last_read_req;
-	seqno = i915_gem_request_get_seqno(req);
-	WARN_ON(seqno == 0);
-	ring = obj->ring;
 
 	/* Do this after OLR check to make sure we make forward progress polling
 	 * on this IOCTL with a timeout <=0 (like busy ioctl)
@@ -2858,8 +2852,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	i915_gem_request_reference(req);
 	mutex_unlock(&dev->struct_mutex);
 
-	ret = __wait_seqno(ring, seqno, reset_counter, true, &args->timeout_ns,
-			   file->driver_priv);
+	ret = __wait_request(req, reset_counter, true, &args->timeout_ns,
+			     file->driver_priv);
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_request_unreference(req);
 	mutex_unlock(&dev->struct_mutex);
@@ -4042,9 +4036,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 	if (target == NULL)
 		return 0;
 
-	ret = __wait_seqno(i915_gem_request_get_ring(target),
-			   i915_gem_request_get_seqno(target),
-			   reset_counter, true, NULL, NULL);
+	ret = __wait_request(target, reset_counter, true, NULL, NULL);
 	if (ret == 0)
 		queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
 
-- 
1.7.9.5

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

[PATCH 13/29] drm/i915: Remove obsolete seqno parameter from 'i915_add_request'

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

There is no longer any need to retreive a seqno value from an i915_add_request()
call. The calling code already knows which request structure is being processed
(it can only be ring->OLR). And as the request itself is now used in preference
to the basic seqno value, the latter is now redundant in this situation.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h              |    7 +++----
 drivers/gpu/drm/i915/i915_gem.c              |    7 ++-----
 drivers/gpu/drm/i915/i915_gem_execbuffer.c   |    2 +-
 drivers/gpu/drm/i915/i915_gem_render_state.c |    2 +-
 drivers/gpu/drm/i915/intel_lrc.c             |    2 +-
 drivers/gpu/drm/i915/intel_overlay.c         |    4 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c      |    2 +-
 7 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index adf1402..d184333 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2566,10 +2566,9 @@ int __must_check i915_gpu_idle(struct drm_device *dev);
 int __must_check i915_gem_suspend(struct drm_device *dev);
 int __i915_add_request(struct intel_engine_cs *ring,
 		       struct drm_file *file,
-		       struct drm_i915_gem_object *batch_obj,
-		       u32 *seqno);
-#define i915_add_request(ring, seqno) \
-	__i915_add_request(ring, NULL, NULL, seqno)
+		       struct drm_i915_gem_object *batch_obj);
+#define i915_add_request(ring) \
+	__i915_add_request(ring, NULL, NULL)
 int __must_check i915_wait_request(struct drm_i915_gem_request *req);
 int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
 int __must_check
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2dcb8bf..9130aa1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1108,7 +1108,7 @@ i915_gem_check_olr(struct drm_i915_gem_request *req)
 
 	ret = 0;
 	if (req == req->ring->outstanding_lazy_request)
-		ret = i915_add_request(req->ring, NULL);
+		ret = i915_add_request(req->ring);
 
 	return ret;
 }
@@ -2328,8 +2328,7 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 
 int __i915_add_request(struct intel_engine_cs *ring,
 		       struct drm_file *file,
-		       struct drm_i915_gem_object *obj,
-		       u32 *out_seqno)
+		       struct drm_i915_gem_object *obj)
 {
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct drm_i915_gem_request *request;
@@ -2430,8 +2429,6 @@ int __i915_add_request(struct intel_engine_cs *ring,
 		intel_mark_busy(dev_priv->dev);
 	}
 
-	if (out_seqno)
-		*out_seqno = request->seqno;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 4833c35..4212365 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -990,7 +990,7 @@ i915_gem_execbuffer_retire_commands(struct drm_device *dev,
 	ring->gpu_caches_dirty = true;
 
 	/* Add a breadcrumb for the completion of the batch buffer */
-	(void)__i915_add_request(ring, file, obj, NULL);
+	(void)__i915_add_request(ring, file, obj);
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 98dcd94..521548a 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -173,7 +173,7 @@ int i915_gem_render_state_init(struct intel_engine_cs *ring)
 
 	i915_vma_move_to_active(i915_gem_obj_to_ggtt(so.obj), ring);
 
-	ret = __i915_add_request(ring, NULL, so.obj, NULL);
+	ret = __i915_add_request(ring, NULL, so.obj);
 	/* __i915_add_request moves object to inactive if it fails */
 out:
 	i915_gem_render_state_fini(&so);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 52ee9ca..a6859ad 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1485,7 +1485,7 @@ int intel_lr_context_render_state_init(struct intel_engine_cs *ring,
 
 	i915_vma_move_to_active(i915_gem_obj_to_ggtt(so.obj), ring);
 
-	ret = __i915_add_request(ring, file, so.obj, NULL);
+	ret = __i915_add_request(ring, file, so.obj);
 	/* intel_logical_ring_add_request moves object to inactive if it
 	 * fails */
 out:
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index a28eea9..052e347 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -218,7 +218,7 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
 
 	BUG_ON(overlay->last_flip_req);
 	i915_gem_request_assign(&overlay->last_flip_req, ring->outstanding_lazy_request);
-	ret = i915_add_request(ring, NULL);
+	ret = i915_add_request(ring);
 	if (ret)
 		return ret;
 
@@ -289,7 +289,7 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
 
 	WARN_ON(overlay->last_flip_req);
 	i915_gem_request_assign(&overlay->last_flip_req, ring->outstanding_lazy_request);
-	return i915_add_request(ring, NULL);
+	return i915_add_request(ring);
 }
 
 static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9db1029..164236c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2006,7 +2006,7 @@ int intel_ring_idle(struct intel_engine_cs *ring)
 
 	/* We need to add any requests required to flush the objects and ring */
 	if (ring->outstanding_lazy_request) {
-		ret = i915_add_request(ring, NULL);
+		ret = i915_add_request(ring);
 		if (ret)
 			return ret;
 	}
-- 
1.7.9.5

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

[PATCH 14/29] drm/i915: Convert mmio_flip::seqno to struct request

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Converted the mmio_flip 'seqno' value to be a request structure as part of the
on going seqno to request changes. This includes reference counting the request
being saved away to ensure it can not be retired and freed while the flip code
is still waiting on it.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   11 ++++++-----
 drivers/gpu/drm/i915/intel_drv.h     |    2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd488d2..2b4c2bc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9374,15 +9374,15 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring)
 		struct intel_mmio_flip *mmio_flip;
 
 		mmio_flip = &intel_crtc->mmio_flip;
-		if (mmio_flip->seqno == 0)
+		if (mmio_flip->req == NULL)
 			continue;
 
 		if (ring->id != mmio_flip->ring_id)
 			continue;
 
-		if (i915_seqno_passed(seqno, mmio_flip->seqno)) {
+		if (i915_seqno_passed(seqno, i915_gem_request_get_seqno(mmio_flip->req))) {
 			intel_do_mmio_flip(intel_crtc);
-			mmio_flip->seqno = 0;
+			i915_gem_request_assign(&mmio_flip->req, NULL);
 			ring->irq_put(ring);
 		}
 	}
@@ -9400,7 +9400,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int ret;
 
-	if (WARN_ON(intel_crtc->mmio_flip.seqno))
+	if (WARN_ON(intel_crtc->mmio_flip.req))
 		return -EBUSY;
 
 	ret = intel_postpone_flip(obj);
@@ -9412,7 +9412,8 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
 	}
 
 	spin_lock_irq(&dev_priv->mmio_flip_lock);
-	intel_crtc->mmio_flip.seqno = i915_gem_request_get_seqno(obj->last_write_req);
+	i915_gem_request_assign(&intel_crtc->mmio_flip.req,
+				obj->last_write_req);
 	intel_crtc->mmio_flip.ring_id = obj->ring->id;
 	spin_unlock_irq(&dev_priv->mmio_flip_lock);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d53ac23..296b1a0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -400,7 +400,7 @@ struct intel_pipe_wm {
 };
 
 struct intel_mmio_flip {
-	u32 seqno;
+	struct drm_i915_gem_request *req;
 	u32 ring_id;
 };
 
-- 
1.7.9.5

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

[PATCH 15/29] drm/i915: Add IRQ friendly request deference facility

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The next patch in the series converts some display related seqno usage to
request structure usage. However, the request dereference introduced must be
done from interrupt context. As the dereference potentially involves freeing the
request structure and thus call lots of non-interrupt friendly code, this poses
a problem.

The solution is to add a IRQ friendly version of the dereference function. All
this does is to add the request structure to a 'delayed free' list and return.
The retire code, which is run periodically, then processes this list and does
the actual dereferencing of the request structures.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |    6 ++++++
 drivers/gpu/drm/i915/i915_gem.c         |   28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        |    2 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c |    3 +++
 drivers/gpu/drm/i915/intel_ringbuffer.h |    2 ++
 5 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d184333..3e6082b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1951,6 +1951,9 @@ struct drm_i915_gem_request {
 	struct drm_i915_file_private *file_priv;
 	/** file_priv list entry for this request */
 	struct list_head client_list;
+
+	/** deferred free list for dereferencing from IRQ context */
+	struct list_head free_list;
 };
 
 void i915_gem_request_free(struct kref *req_ref);
@@ -1976,9 +1979,12 @@ i915_gem_request_reference(struct drm_i915_gem_request *req)
 static inline void
 i915_gem_request_unreference(struct drm_i915_gem_request *req)
 {
+	WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
 	kref_put(&req->ref, i915_gem_request_free);
 }
 
+void i915_gem_request_unreference_irq(struct drm_i915_gem_request *req);
+
 static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
 					   struct drm_i915_gem_request *src)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9130aa1..e4cb253 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2635,6 +2635,18 @@ void i915_gem_reset(struct drm_device *dev)
 	i915_gem_restore_fences(dev);
 }
 
+void i915_gem_request_unreference_irq(struct drm_i915_gem_request *req)
+{
+	struct intel_engine_cs *ring = req->ring;
+	unsigned long flags;
+
+	/* Need to add the req to a deferred dereference list to be processed
+	 * outside of interrupt time */
+	spin_lock_irqsave(&ring->reqlist_lock, flags);
+	list_add_tail(&req->free_list, &ring->delayed_free_list);
+	spin_unlock_irqrestore(&ring->reqlist_lock, flags);
+}
+
 /**
  * This function clears the request list as sequence numbers are passed.
  */
@@ -2708,6 +2720,21 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 		ring->trace_irq_seqno = 0;
 	}
 
+	while (!list_empty(&ring->delayed_free_list)) {
+		struct drm_i915_gem_request *request;
+		unsigned long flags;
+
+		request = list_first_entry(&ring->delayed_free_list,
+					   struct drm_i915_gem_request,
+					   free_list);
+
+		spin_lock_irqsave(&request->ring->reqlist_lock, flags);
+		list_del(&request->free_list);
+		spin_unlock_irqrestore(&request->ring->reqlist_lock, flags);
+
+		i915_gem_request_unreference(request);
+	}
+
 	WARN_ON(i915_verify_lists(ring->dev));
 }
 
@@ -4948,6 +4975,7 @@ init_ring_lists(struct intel_engine_cs *ring)
 {
 	INIT_LIST_HEAD(&ring->active_list);
 	INIT_LIST_HEAD(&ring->request_list);
+	INIT_LIST_HEAD(&ring->delayed_free_list);
 }
 
 void i915_init_vm(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index a6859ad..ba0d7b8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1249,7 +1249,9 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
 	ring->dev = dev;
 	INIT_LIST_HEAD(&ring->active_list);
 	INIT_LIST_HEAD(&ring->request_list);
+	spin_lock_init(&ring->reqlist_lock);
 	init_waitqueue_head(&ring->irq_queue);
+	INIT_LIST_HEAD(&ring->delayed_free_list);
 
 	INIT_LIST_HEAD(&ring->execlist_queue);
 	spin_lock_init(&ring->execlist_lock);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 164236c..f225ba3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1807,6 +1807,8 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 	ring->dev = dev;
 	INIT_LIST_HEAD(&ring->active_list);
 	INIT_LIST_HEAD(&ring->request_list);
+	spin_lock_init(&ring->reqlist_lock);
+	INIT_LIST_HEAD(&ring->delayed_free_list);
 	INIT_LIST_HEAD(&ring->execlist_queue);
 	ringbuf->size = 32 * PAGE_SIZE;
 	ringbuf->ring = ring;
@@ -2506,6 +2508,7 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size)
 	ring->dev = dev;
 	INIT_LIST_HEAD(&ring->active_list);
 	INIT_LIST_HEAD(&ring->request_list);
+	INIT_LIST_HEAD(&ring->delayed_free_list);
 
 	ringbuf->size = size;
 	ringbuf->effective_size = ringbuf->size;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 98b219d..0451233 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -261,6 +261,8 @@ struct  intel_engine_cs {
 	 * outstanding.
 	 */
 	struct list_head request_list;
+	spinlock_t reqlist_lock;
+	struct list_head delayed_free_list;
 
 	/**
 	 * Do we have some not yet emitted requests outstanding?
-- 
1.7.9.5

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

Re: [PATCH 15/29] drm/i915: Add IRQ friendly request deference facility

[Daniel, Thomas]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of John.C.Harrison@Intel.com
> Sent: Thursday, October 30, 2014 6:41 PM
> To: Intel-GFX@Lists.FreeDesktop.Org
> Subject: [Intel-gfx] [PATCH 15/29] drm/i915: Add IRQ friendly request
> deference facility
> 
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> The next patch in the series converts some display related seqno usage to
> request structure usage. However, the request dereference introduced
> must be done from interrupt context. As the dereference potentially
> involves freeing the request structure and thus call lots of non-interrupt
> friendly code, this poses a problem.
> 
> The solution is to add a IRQ friendly version of the dereference function. All
> this does is to add the request structure to a 'delayed free' list and return.
> The retire code, which is run periodically, then processes this list and does
> the actual dereferencing of the request structures.
> 
> For: VIZ-4377
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |    6 ++++++
>  drivers/gpu/drm/i915/i915_gem.c         |   28
> ++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c        |    2 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    3 +++
>  drivers/gpu/drm/i915/intel_ringbuffer.h |    2 ++
>  5 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index d184333..3e6082b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1951,6 +1951,9 @@ struct drm_i915_gem_request {
>  	struct drm_i915_file_private *file_priv;
>  	/** file_priv list entry for this request */
>  	struct list_head client_list;
> +
> +	/** deferred free list for dereferencing from IRQ context */
> +	struct list_head free_list;
>  };
> 
>  void i915_gem_request_free(struct kref *req_ref); @@ -1976,9 +1979,12
> @@ i915_gem_request_reference(struct drm_i915_gem_request *req)
> static inline void  i915_gem_request_unreference(struct
> drm_i915_gem_request *req)  {
> +	WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
>  	kref_put(&req->ref, i915_gem_request_free);  }
> 
> +void i915_gem_request_unreference_irq(struct drm_i915_gem_request
> +*req);
> +
>  static inline void i915_gem_request_assign(struct drm_i915_gem_request
> **pdst,
>  					   struct drm_i915_gem_request *src)
> { diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c index 9130aa1..e4cb253 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2635,6 +2635,18 @@ void i915_gem_reset(struct drm_device *dev)
>  	i915_gem_restore_fences(dev);
>  }
> 
> +void i915_gem_request_unreference_irq(struct drm_i915_gem_request
> *req)
> +{
> +	struct intel_engine_cs *ring = req->ring;
> +	unsigned long flags;
> +
> +	/* Need to add the req to a deferred dereference list to be
> processed
> +	 * outside of interrupt time */
> +	spin_lock_irqsave(&ring->reqlist_lock, flags);
> +	list_add_tail(&req->free_list, &ring->delayed_free_list);
> +	spin_unlock_irqrestore(&ring->reqlist_lock, flags); }
> +
>  /**
>   * This function clears the request list as sequence numbers are passed.
>   */
> @@ -2708,6 +2720,21 @@ i915_gem_retire_requests_ring(struct
> intel_engine_cs *ring)
>  		ring->trace_irq_seqno = 0;
>  	}
> 
> +	while (!list_empty(&ring->delayed_free_list)) {
> +		struct drm_i915_gem_request *request;
> +		unsigned long flags;
> +
> +		request = list_first_entry(&ring->delayed_free_list,
> +					   struct drm_i915_gem_request,
> +					   free_list);
> +
> +		spin_lock_irqsave(&request->ring->reqlist_lock, flags);
> +		list_del(&request->free_list);
> +		spin_unlock_irqrestore(&request->ring->reqlist_lock, flags);
> +
> +		i915_gem_request_unreference(request);
> +	}
> +
>  	WARN_ON(i915_verify_lists(ring->dev));
>  }
> 
> @@ -4948,6 +4975,7 @@ init_ring_lists(struct intel_engine_cs *ring)  {
>  	INIT_LIST_HEAD(&ring->active_list);
>  	INIT_LIST_HEAD(&ring->request_list);
> +	INIT_LIST_HEAD(&ring->delayed_free_list);
Do we need this?
This list is initialized in logical_ring_init and init_ring_buffer anyway.

Thomas.

>  }
> 
>  void i915_init_vm(struct drm_i915_private *dev_priv, diff --git
> a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index a6859ad..ba0d7b8 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1249,7 +1249,9 @@ static int logical_ring_init(struct drm_device *dev,
> struct intel_engine_cs *rin
>  	ring->dev = dev;
>  	INIT_LIST_HEAD(&ring->active_list);
>  	INIT_LIST_HEAD(&ring->request_list);
> +	spin_lock_init(&ring->reqlist_lock);
>  	init_waitqueue_head(&ring->irq_queue);
> +	INIT_LIST_HEAD(&ring->delayed_free_list);
> 
>  	INIT_LIST_HEAD(&ring->execlist_queue);
>  	spin_lock_init(&ring->execlist_lock);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 164236c..f225ba3 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1807,6 +1807,8 @@ static int intel_init_ring_buffer(struct drm_device
> *dev,
>  	ring->dev = dev;
>  	INIT_LIST_HEAD(&ring->active_list);
>  	INIT_LIST_HEAD(&ring->request_list);
> +	spin_lock_init(&ring->reqlist_lock);
> +	INIT_LIST_HEAD(&ring->delayed_free_list);
>  	INIT_LIST_HEAD(&ring->execlist_queue);
>  	ringbuf->size = 32 * PAGE_SIZE;
>  	ringbuf->ring = ring;
> @@ -2506,6 +2508,7 @@ int intel_render_ring_init_dri(struct drm_device
> *dev, u64 start, u32 size)
>  	ring->dev = dev;
>  	INIT_LIST_HEAD(&ring->active_list);
>  	INIT_LIST_HEAD(&ring->request_list);
> +	INIT_LIST_HEAD(&ring->delayed_free_list);
> 
>  	ringbuf->size = size;
>  	ringbuf->effective_size = ringbuf->size; diff --git
> a/drivers/gpu/drm/i915/intel_ringbuffer.h
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 98b219d..0451233 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -261,6 +261,8 @@ struct  intel_engine_cs {
>  	 * outstanding.
>  	 */
>  	struct list_head request_list;
> +	spinlock_t reqlist_lock;
> +	struct list_head delayed_free_list;
> 
>  	/**
>  	 * Do we have some not yet emitted requests outstanding?
> --
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 16/29] drm/i915: Convert 'flip_queued_seqno' into 'flip_queued_request'

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Converted the flip_queued_seqno value to be a request structure as part of the
on going seqno to request changes. This includes reference counting the request
being saved away to ensure it can not be retired and freed while the flip code
is still waiting on it.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |    4 ++--
 drivers/gpu/drm/i915/intel_display.c |   19 +++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |    2 +-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 79ce5c0..15ad322 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -546,11 +546,11 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 			if (work->flip_queued_ring) {
 				seq_printf(m, "Flip queued on %s at seqno %u, next seqno %u [current breadcrumb %u], completed? %d\n",
 					   work->flip_queued_ring->name,
-					   work->flip_queued_seqno,
+					   i915_gem_request_get_seqno(work->flip_queued_req),
 					   dev_priv->next_seqno,
 					   work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
 					   i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
-							     work->flip_queued_seqno));
+							     i915_gem_request_get_seqno(work->flip_queued_req)));
 			} else
 				seq_printf(m, "Flip not associated with any ring\n");
 			seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2b4c2bc..44005a2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9450,10 +9450,15 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 		return false;
 
 	if (work->flip_ready_vblank == 0) {
-		if (work->flip_queued_ring &&
-		    !i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
-				       work->flip_queued_seqno))
-			return false;
+		if (work->flip_queued_ring) {
+			if (!i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
+				       i915_gem_request_get_seqno(work->flip_queued_req)))
+				return false;
+
+			i915_gem_request_unreference_irq(work->flip_queued_req);
+			work->flip_queued_req  = NULL;
+			work->flip_queued_ring = NULL;
+		}
 
 		work->flip_ready_vblank = drm_vblank_count(dev, intel_crtc->pipe);
 	}
@@ -9617,7 +9622,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		if (ret)
 			goto cleanup_unpin;
 
-		work->flip_queued_seqno = i915_gem_request_get_seqno(obj->last_write_req);
+		i915_gem_request_assign(&work->flip_queued_req,
+					obj->last_write_req);
 		work->flip_queued_ring = obj->ring;
 	} else {
 		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring,
@@ -9625,7 +9631,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		if (ret)
 			goto cleanup_unpin;
 
-		work->flip_queued_seqno = i915_gem_request_get_seqno(intel_ring_get_request(ring));
+		i915_gem_request_assign(&work->flip_queued_req,
+					intel_ring_get_request(ring));
 		work->flip_queued_ring = ring;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 296b1a0..8e585b4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -693,7 +693,7 @@ struct intel_unpin_work {
 	u32 flip_count;
 	u32 gtt_offset;
 	struct intel_engine_cs *flip_queued_ring;
-	u32 flip_queued_seqno;
+	struct drm_i915_gem_request *flip_queued_req;
 	int flip_queued_vblank;
 	int flip_ready_vblank;
 	bool enable_stall_check;
-- 
1.7.9.5

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

[PATCH 17/29] drm/i915: Convert trace functions from seqno to request

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

All the code above is now using requests not seqnos so it is possible to convert
the trace functions across. Note that rather than get into problematic reference
counting issues, the trace code only saves the seqno and ring values from the
request structure not the structure pointer itself.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c            |   10 +++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
 drivers/gpu/drm/i915/i915_trace.h          |   47 ++++++++++++++++------------
 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e4cb253..774ab9f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1184,7 +1184,7 @@ static int __wait_request(struct drm_i915_gem_request *req,
 		return -ENODEV;
 
 	/* Record current time in case interrupted by signal, or wedged */
-	trace_i915_gem_request_wait_begin(i915_gem_request_get_ring(req), i915_gem_request_get_seqno(req));
+	trace_i915_gem_request_wait_begin(req);
 	before = ktime_get_raw_ns();
 	for (;;) {
 		struct timer_list timer;
@@ -1235,7 +1235,7 @@ static int __wait_request(struct drm_i915_gem_request *req,
 		}
 	}
 	now = ktime_get_raw_ns();
-	trace_i915_gem_request_wait_end(i915_gem_request_get_ring(req), i915_gem_request_get_seqno(req));
+	trace_i915_gem_request_wait_end(req);
 
 	if (!irq_test_in_progress)
 		ring->irq_put(ring);
@@ -2416,7 +2416,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 		spin_unlock(&file_priv->mm.lock);
 	}
 
-	trace_i915_gem_request_add(ring, request->seqno);
+	trace_i915_gem_request_add(request);
 	ring->outstanding_lazy_request = NULL;
 
 	if (!dev_priv->ums.mm_suspended) {
@@ -2691,7 +2691,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 		if (!i915_seqno_passed(seqno, request->seqno))
 			break;
 
-		trace_i915_gem_request_retire(ring, request->seqno);
+		trace_i915_gem_request_retire(request);
 
 		/* This is one of the few common intersection points
 		 * between legacy ringbuffer submission and execlists:
@@ -2927,7 +2927,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
 	if (ret)
 		return ret;
 
-	trace_i915_gem_ring_sync_to(from, to, seqno);
+	trace_i915_gem_ring_sync_to(from, to, obj->last_read_req);
 	ret = to->semaphore.sync_to(to, from, seqno);
 	if (!ret)
 		/* We use last_read_req because sync_to()
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 4212365..532ca0d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1167,7 +1167,7 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
 			return ret;
 	}
 
-	trace_i915_gem_ring_dispatch(ring, i915_gem_request_get_seqno(intel_ring_get_request(ring)), flags);
+	trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), flags);
 
 	i915_gem_execbuffer_move_to_active(vmas, ring);
 	i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index f5aa006..66616f7 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -328,8 +328,8 @@ TRACE_EVENT(i915_gem_evict_vm,
 TRACE_EVENT(i915_gem_ring_sync_to,
 	    TP_PROTO(struct intel_engine_cs *from,
 		     struct intel_engine_cs *to,
-		     u32 seqno),
-	    TP_ARGS(from, to, seqno),
+		     struct drm_i915_gem_request *req),
+	    TP_ARGS(from, to, req),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
@@ -342,7 +342,7 @@ TRACE_EVENT(i915_gem_ring_sync_to,
 			   __entry->dev = from->dev->primary->index;
 			   __entry->sync_from = from->id;
 			   __entry->sync_to = to->id;
-			   __entry->seqno = seqno;
+			   __entry->seqno = i915_gem_request_get_seqno(req);
 			   ),
 
 	    TP_printk("dev=%u, sync-from=%u, sync-to=%u, seqno=%u",
@@ -352,8 +352,8 @@ TRACE_EVENT(i915_gem_ring_sync_to,
 );
 
 TRACE_EVENT(i915_gem_ring_dispatch,
-	    TP_PROTO(struct intel_engine_cs *ring, u32 seqno, u32 flags),
-	    TP_ARGS(ring, seqno, flags),
+	    TP_PROTO(struct drm_i915_gem_request *req, u32 flags),
+	    TP_ARGS(req, flags),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
@@ -363,11 +363,13 @@ TRACE_EVENT(i915_gem_ring_dispatch,
 			     ),
 
 	    TP_fast_assign(
+			   struct intel_engine_cs *ring =
+						i915_gem_request_get_ring(req);
 			   __entry->dev = ring->dev->primary->index;
 			   __entry->ring = ring->id;
-			   __entry->seqno = seqno;
+			   __entry->seqno = i915_gem_request_get_seqno(req);
 			   __entry->flags = flags;
-			   i915_trace_irq_get(ring, seqno);
+			   i915_trace_irq_get(ring, __entry->seqno);
 			   ),
 
 	    TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x",
@@ -398,8 +400,8 @@ TRACE_EVENT(i915_gem_ring_flush,
 );
 
 DECLARE_EVENT_CLASS(i915_gem_request,
-	    TP_PROTO(struct intel_engine_cs *ring, u32 seqno),
-	    TP_ARGS(ring, seqno),
+	    TP_PROTO(struct drm_i915_gem_request *req),
+	    TP_ARGS(req),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
@@ -408,9 +410,11 @@ DECLARE_EVENT_CLASS(i915_gem_request,
 			     ),
 
 	    TP_fast_assign(
+			   struct intel_engine_cs *ring =
+						i915_gem_request_get_ring(req);
 			   __entry->dev = ring->dev->primary->index;
 			   __entry->ring = ring->id;
-			   __entry->seqno = seqno;
+			   __entry->seqno = i915_gem_request_get_seqno(req);
 			   ),
 
 	    TP_printk("dev=%u, ring=%u, seqno=%u",
@@ -418,8 +422,8 @@ DECLARE_EVENT_CLASS(i915_gem_request,
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
-	    TP_PROTO(struct intel_engine_cs *ring, u32 seqno),
-	    TP_ARGS(ring, seqno)
+	    TP_PROTO(struct drm_i915_gem_request *req),
+	    TP_ARGS(req)
 );
 
 TRACE_EVENT(i915_gem_request_complete,
@@ -443,13 +447,13 @@ TRACE_EVENT(i915_gem_request_complete,
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_retire,
-	    TP_PROTO(struct intel_engine_cs *ring, u32 seqno),
-	    TP_ARGS(ring, seqno)
+	    TP_PROTO(struct drm_i915_gem_request *req),
+	    TP_ARGS(req)
 );
 
 TRACE_EVENT(i915_gem_request_wait_begin,
-	    TP_PROTO(struct intel_engine_cs *ring, u32 seqno),
-	    TP_ARGS(ring, seqno),
+	    TP_PROTO(struct drm_i915_gem_request *req),
+	    TP_ARGS(req),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
@@ -465,10 +469,13 @@ TRACE_EVENT(i915_gem_request_wait_begin,
 	     * less desirable.
 	     */
 	    TP_fast_assign(
+			   struct intel_engine_cs *ring =
+						i915_gem_request_get_ring(req);
 			   __entry->dev = ring->dev->primary->index;
 			   __entry->ring = ring->id;
-			   __entry->seqno = seqno;
-			   __entry->blocking = mutex_is_locked(&ring->dev->struct_mutex);
+			   __entry->seqno = i915_gem_request_get_seqno(req);
+			   __entry->blocking =
+				     mutex_is_locked(&ring->dev->struct_mutex);
 			   ),
 
 	    TP_printk("dev=%u, ring=%u, seqno=%u, blocking=%s",
@@ -477,8 +484,8 @@ TRACE_EVENT(i915_gem_request_wait_begin,
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_wait_end,
-	    TP_PROTO(struct intel_engine_cs *ring, u32 seqno),
-	    TP_ARGS(ring, seqno)
+	    TP_PROTO(struct drm_i915_gem_request *req),
+	    TP_ARGS(req)
 );
 
 DECLARE_EVENT_CLASS(i915_ring,
-- 
1.7.9.5

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

[PATCH 18/29] drm/i915: Convert 'trace_irq' to use requests rather than seqnos

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Updated the trace_irq code to use requests instead of seqnos. This includes
reference counting the request object to ensure it sticks around when required.
Note that getting access to the reference counting functions means moving the
inline i915_trace_irq_get() function from intel_ringbuffer.h to i915_drv.h.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |    7 +++++++
 drivers/gpu/drm/i915/i915_gem.c         |    6 +++---
 drivers/gpu/drm/i915/i915_trace.h       |    2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h |    8 +-------
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3e6082b..6137b51 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3060,4 +3060,11 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 	}
 }
 
+static inline void i915_trace_irq_get(struct intel_engine_cs *ring,
+				      struct drm_i915_gem_request *req)
+{
+	if (ring->trace_irq_req == NULL && ring->irq_get(ring))
+		i915_gem_request_assign(&ring->trace_irq_req, req);
+}
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 774ab9f..34dd703 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2714,10 +2714,10 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 		i915_gem_free_request(request);
 	}
 
-	if (unlikely(ring->trace_irq_seqno &&
-		     i915_seqno_passed(seqno, ring->trace_irq_seqno))) {
+	if (unlikely(ring->trace_irq_req &&
+		     i915_seqno_passed(seqno, i915_gem_request_get_seqno(ring->trace_irq_req)))) {
 		ring->irq_put(ring);
-		ring->trace_irq_seqno = 0;
+		i915_gem_request_assign(&ring->trace_irq_req, NULL);
 	}
 
 	while (!list_empty(&ring->delayed_free_list)) {
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 66616f7..0242089 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -369,7 +369,7 @@ TRACE_EVENT(i915_gem_ring_dispatch,
 			   __entry->ring = ring->id;
 			   __entry->seqno = i915_gem_request_get_seqno(req);
 			   __entry->flags = flags;
-			   i915_trace_irq_get(ring, __entry->seqno);
+			   i915_trace_irq_get(ring, req);
 			   ),
 
 	    TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x",
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0451233..eb754cf5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -142,7 +142,7 @@ struct  intel_engine_cs {
 
 	unsigned irq_refcount; /* protected by dev_priv->irq_lock */
 	u32		irq_enable_mask;	/* bitmask to enable ring interrupt */
-	u32		trace_irq_seqno;
+	struct drm_i915_gem_request *trace_irq_req;
 	bool __must_check (*irq_get)(struct intel_engine_cs *ring);
 	void		(*irq_put)(struct intel_engine_cs *ring);
 
@@ -437,12 +437,6 @@ intel_ring_get_request(struct intel_engine_cs *ring)
 	return ring->outstanding_lazy_request;
 }
 
-static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
-{
-	if (ring->trace_irq_seqno == 0 && ring->irq_get(ring))
-		ring->trace_irq_seqno = seqno;
-}
-
 /* DRI warts */
 int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size);
 
-- 
1.7.9.5

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

[PATCH 19/29] drm/i915: Convert 'ring_idle()' to use requests not seqnos

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

More seqno value to request structure conversions. Note, this change temporarily
moves the 'get_seqno()' call inside ring_idle() but this will disappear again in
a later patch when i915_seqno_passed() itself is converted.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a2b013d..003b268 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2716,18 +2716,19 @@ static void gen8_disable_vblank(struct drm_device *dev, int pipe)
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 
-static u32
-ring_last_seqno(struct intel_engine_cs *ring)
+static struct drm_i915_gem_request *
+ring_last_request(struct intel_engine_cs *ring)
 {
 	return list_entry(ring->request_list.prev,
-			  struct drm_i915_gem_request, list)->seqno;
+			  struct drm_i915_gem_request, list);
 }
 
 static bool
-ring_idle(struct intel_engine_cs *ring, u32 seqno)
+ring_idle(struct intel_engine_cs *ring)
 {
 	return (list_empty(&ring->request_list) ||
-		i915_seqno_passed(seqno, ring_last_seqno(ring)));
+		i915_seqno_passed(ring->get_seqno(ring, false),
+				  i915_gem_request_get_seqno(ring_last_request(ring))));
 }
 
 static bool
@@ -2947,7 +2948,7 @@ static void i915_hangcheck_elapsed(unsigned long data)
 		acthd = intel_ring_get_active_head(ring);
 
 		if (ring->hangcheck.seqno == seqno) {
-			if (ring_idle(ring, seqno)) {
+			if (ring_idle(ring)) {
 				ring->hangcheck.action = HANGCHECK_IDLE;
 
 				if (waitqueue_active(&ring->irq_queue)) {
-- 
1.7.9.5

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

[PATCH 20/29] drm/i915: Connect requests to rings at creation not submission

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

It makes a lot more sense (and makes future seqno -> request conversion patches
simpler) to fill in the 'ring' field of the request structure at the point of
creation rather than submission. Given that the request structure is assigned by
ring specific code and thus is locked to a ring from the start, there really is
no reason to defer this assignment.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |    1 -
 drivers/gpu/drm/i915/intel_lrc.c        |    1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |    1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 34dd703..917a01b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2381,7 +2381,6 @@ int __i915_add_request(struct intel_engine_cs *ring,
 			return ret;
 	}
 
-	request->ring = ring;
 	request->head = request_start;
 	request->tail = request_ring_position;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ba0d7b8..6e6a255 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -807,6 +807,7 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
 		return -ENOMEM;
 
 	kref_init(&request->ref);
+	request->ring = ring;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index f225ba3..63d35a6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2038,6 +2038,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
 		return -ENOMEM;
 
 	kref_init(&request->ref);
+	request->ring = ring;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
-- 
1.7.9.5

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

[PATCH 21/29] drm/i915: Convert 'i915_seqno_passed' calls into 'i915_gem_request_completed'

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Almost everywhere that caled i915_seqno_passed() was really asking 'has the
given seqno popped out of the hardware yet?'. Thus it had to query the current
hardware seqno and then do a signed delta comparison (which copes with wrapping
around zero but not with seqno values more than 2GB apart, although the latter
is unlikely!).

Now that the majority of seqno instances have been replaced with request
structures, it is possible to convert this test to be request based as well.
There is now a 'i915_gem_request_completed()' function which takes a request and
returns true or false as appropriate. The ultimate aim is that this will be
simply returning a cached internal value: e.g. '_completed(req) { return
req->compelted; }'. However, at the moment, the implementation actualy falls
back to the original 'seqno_passed()' mechanism. This is to be improved in later
patches in the series.

This checkin almost all _seqno_passed() calls. The only one left is in the
semaphore code which still requires seqnos not request structures.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |    3 +--
 drivers/gpu/drm/i915/i915_drv.h      |   18 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem.c      |   22 +++++++---------------
 drivers/gpu/drm/i915/i915_irq.c      |    3 +--
 drivers/gpu/drm/i915/intel_display.c |   11 +++--------
 5 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 15ad322..f26c2b2 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -549,8 +549,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 					   i915_gem_request_get_seqno(work->flip_queued_req),
 					   dev_priv->next_seqno,
 					   work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
-					   i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
-							     i915_gem_request_get_seqno(work->flip_queued_req)));
+					   i915_gem_request_completed(work->flip_queued_req, true));
 			} else
 				seq_printf(m, "Flip not associated with any ring\n");
 			seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6137b51..1d30dcb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1997,6 +1997,12 @@ static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
 	*pdst = src;
 }
 
+/*
+ * XXX: i915_gem_request_completed should be here but currently needs the
+ * definition of i915_seqno_passed() which is below. It will be moved in
+ * a later patch when the call to i915_seqno_passed() is obsoleted...
+ */
+
 struct drm_i915_file_private {
 	struct drm_i915_private *dev_priv;
 	struct drm_file *file;
@@ -3060,6 +3066,18 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 	}
 }
 
+static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
+					      bool lazy_coherency)
+{
+	u32 seqno;
+
+	BUG_ON(req == NULL);
+
+	seqno = req->ring->get_seqno(req->ring, lazy_coherency);
+
+	return i915_seqno_passed(seqno, req->seqno);
+}
+
 static inline void i915_trace_irq_get(struct intel_engine_cs *ring,
 				      struct drm_i915_gem_request *req)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 917a01b..2daafeb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1167,7 +1167,7 @@ static int __wait_request(struct drm_i915_gem_request *req,
 
 	WARN(!intel_irqs_enabled(dev_priv), "IRQs disabled");
 
-	if (i915_seqno_passed(ring->get_seqno(ring, true), req->seqno))
+	if (i915_gem_request_completed(req, true))
 		return 0;
 
 	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
@@ -1203,7 +1203,7 @@ static int __wait_request(struct drm_i915_gem_request *req,
 			break;
 		}
 
-		if (i915_seqno_passed(ring->get_seqno(ring, false), req->seqno)) {
+		if (i915_gem_request_completed(req, false)) {
 			ret = 0;
 			break;
 		}
@@ -2251,8 +2251,7 @@ i915_gem_object_retire(struct drm_i915_gem_object *obj)
 	if (ring == NULL)
 		return;
 
-	if (i915_seqno_passed(ring->get_seqno(ring, true),
-			      i915_gem_request_get_seqno(obj->last_read_req)))
+	if (i915_gem_request_completed(obj->last_read_req, true))
 		i915_gem_object_move_to_inactive(obj);
 }
 
@@ -2512,12 +2511,9 @@ struct drm_i915_gem_request *
 i915_gem_find_active_request(struct intel_engine_cs *ring)
 {
 	struct drm_i915_gem_request *request;
-	u32 completed_seqno;
-
-	completed_seqno = ring->get_seqno(ring, false);
 
 	list_for_each_entry(request, &ring->request_list, list) {
-		if (i915_seqno_passed(completed_seqno, request->seqno))
+		if (i915_gem_request_completed(request, false))
 			continue;
 
 		return request;
@@ -2652,15 +2648,11 @@ void i915_gem_request_unreference_irq(struct drm_i915_gem_request *req)
 void
 i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 {
-	uint32_t seqno;
-
 	if (list_empty(&ring->request_list))
 		return;
 
 	WARN_ON(i915_verify_lists(ring->dev));
 
-	seqno = ring->get_seqno(ring, true);
-
 	/* Move any buffers on the active list that are no longer referenced
 	 * by the ringbuffer to the flushing/inactive lists as appropriate,
 	 * before we free the context associated with the requests.
@@ -2672,7 +2664,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 				      struct drm_i915_gem_object,
 				      ring_list);
 
-		if (!i915_seqno_passed(seqno, i915_gem_request_get_seqno(obj->last_read_req)))
+		if (!i915_gem_request_completed(obj->last_read_req, true))
 			break;
 
 		i915_gem_object_move_to_inactive(obj);
@@ -2687,7 +2679,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 					   struct drm_i915_gem_request,
 					   list);
 
-		if (!i915_seqno_passed(seqno, request->seqno))
+		if (!i915_gem_request_completed(request, true))
 			break;
 
 		trace_i915_gem_request_retire(request);
@@ -2714,7 +2706,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 	}
 
 	if (unlikely(ring->trace_irq_req &&
-		     i915_seqno_passed(seqno, i915_gem_request_get_seqno(ring->trace_irq_req)))) {
+		     i915_gem_request_completed(ring->trace_irq_req, true))) {
 		ring->irq_put(ring);
 		i915_gem_request_assign(&ring->trace_irq_req, NULL);
 	}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 003b268..21e1d69 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2727,8 +2727,7 @@ static bool
 ring_idle(struct intel_engine_cs *ring)
 {
 	return (list_empty(&ring->request_list) ||
-		i915_seqno_passed(ring->get_seqno(ring, false),
-				  i915_gem_request_get_seqno(ring_last_request(ring))));
+		i915_gem_request_completed(ring_last_request(ring), false));
 }
 
 static bool
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 44005a2..eca4240 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9346,8 +9346,7 @@ static int intel_postpone_flip(struct drm_i915_gem_object *obj)
 
 	ring = obj->ring;
 
-	if (i915_seqno_passed(ring->get_seqno(ring, true),
-			      i915_gem_request_get_seqno(obj->last_write_req)))
+	if (i915_gem_request_completed(obj->last_write_req, true))
 		return 0;
 
 	ret = i915_gem_check_olr(obj->last_write_req);
@@ -9365,9 +9364,6 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring)
 	struct drm_i915_private *dev_priv = to_i915(ring->dev);
 	struct intel_crtc *intel_crtc;
 	unsigned long irq_flags;
-	u32 seqno;
-
-	seqno = ring->get_seqno(ring, false);
 
 	spin_lock_irqsave(&dev_priv->mmio_flip_lock, irq_flags);
 	for_each_intel_crtc(ring->dev, intel_crtc) {
@@ -9380,7 +9376,7 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring)
 		if (ring->id != mmio_flip->ring_id)
 			continue;
 
-		if (i915_seqno_passed(seqno, i915_gem_request_get_seqno(mmio_flip->req))) {
+		if (i915_gem_request_completed(mmio_flip->req, false)) {
 			intel_do_mmio_flip(intel_crtc);
 			i915_gem_request_assign(&mmio_flip->req, NULL);
 			ring->irq_put(ring);
@@ -9451,8 +9447,7 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 
 	if (work->flip_ready_vblank == 0) {
 		if (work->flip_queued_ring) {
-			if (!i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
-				       i915_gem_request_get_seqno(work->flip_queued_req)))
+			if (!i915_gem_request_completed(work->flip_queued_req, true))
 				return false;
 
 			i915_gem_request_unreference_irq(work->flip_queued_req);
-- 
1.7.9.5

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

[PATCH 22/29] drm/i915: Remove the now redundant 'obj->ring'

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The ring member of the object structure was always updated with the
last_read_seqno member. Thus with the conversion to last_read_req, obj->ring is
now a direct copy of obj->last_read_req->ring. This makes it somewhat redundant
and potentially misleading (especially as there was no comment to explain its
purpose).

This checkin removes the redundant field. Many uses were simply testing for
non-null to see if the object is active on the GPU. Some of these have been
converted to check 'obj->active' instead. Others (where the last_read_req is
about to be used anyway) have been changed to check obj->last_read_req. The rest
simply pull the ring out from the request structure and proceed as before.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |    9 +++++----
 drivers/gpu/drm/i915/i915_drv.h         |    2 --
 drivers/gpu/drm/i915/i915_gem.c         |   32 +++++++++++++++++++------------
 drivers/gpu/drm/i915/i915_gem_context.c |    3 ++-
 drivers/gpu/drm/i915/i915_gpu_error.c   |    3 ++-
 drivers/gpu/drm/i915/intel_display.c    |   14 ++++++++------
 6 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f26c2b2..0f40e61 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -168,8 +168,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 		*t = '\0';
 		seq_printf(m, " (%s mappable)", s);
 	}
-	if (obj->ring != NULL)
-		seq_printf(m, " (%s)", obj->ring->name);
+	if (obj->last_read_req != NULL)
+		seq_printf(m, " (%s)",
+			   i915_gem_request_get_ring(obj->last_read_req)->name);
 	if (obj->frontbuffer_bits)
 		seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
 }
@@ -336,7 +337,7 @@ static int per_file_stats(int id, void *ptr, void *data)
 			if (ppgtt->file_priv != stats->file_priv)
 				continue;
 
-			if (obj->ring) /* XXX per-vma statistic */
+			if (obj->active) /* XXX per-vma statistic */
 				stats->active += obj->base.size;
 			else
 				stats->inactive += obj->base.size;
@@ -346,7 +347,7 @@ static int per_file_stats(int id, void *ptr, void *data)
 	} else {
 		if (i915_gem_obj_ggtt_bound(obj)) {
 			stats->global += obj->base.size;
-			if (obj->ring)
+			if (obj->active)
 				stats->active += obj->base.size;
 			else
 				stats->inactive += obj->base.size;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1d30dcb..41c2db3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1868,8 +1868,6 @@ struct drm_i915_gem_object {
 	void *dma_buf_vmapping;
 	int vmapping_count;
 
-	struct intel_engine_cs *ring;
-
 	/** Breadcrumb of last rendering to the buffer. */
 	struct drm_i915_gem_request *last_read_req;
 	struct drm_i915_gem_request *last_write_req;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2daafeb..ea6d679 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2183,14 +2183,18 @@ static void
 i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
 			       struct intel_engine_cs *ring)
 {
-	struct drm_i915_gem_request *req = intel_ring_get_request(ring);
+	struct drm_i915_gem_request *req;
+	struct intel_engine_cs *old_ring;
 
 	BUG_ON(ring == NULL);
-	if (obj->ring != ring && obj->last_write_req) {
+
+	req = intel_ring_get_request(ring);
+	old_ring = i915_gem_request_get_ring(obj->last_read_req);
+
+	if (old_ring != ring && obj->last_write_req) {
 		/* Keep the request relative to the current ring */
 		i915_gem_request_assign(&obj->last_write_req, req);
 	}
-	obj->ring = ring;
 
 	/* Add a reference if we're newly entering the active list. */
 	if (!obj->active) {
@@ -2229,7 +2233,6 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
 	intel_fb_obj_flush(obj, true);
 
 	list_del_init(&obj->ring_list);
-	obj->ring = NULL;
 
 	i915_gem_request_assign(&obj->last_read_req, NULL);
 	i915_gem_request_assign(&obj->last_write_req, NULL);
@@ -2246,9 +2249,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
 static void
 i915_gem_object_retire(struct drm_i915_gem_object *obj)
 {
-	struct intel_engine_cs *ring = obj->ring;
-
-	if (ring == NULL)
+	if (obj->last_read_req == NULL)
 		return;
 
 	if (i915_gem_request_completed(obj->last_read_req, true))
@@ -2786,14 +2787,17 @@ i915_gem_idle_work_handler(struct work_struct *work)
 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);
 		if (ret)
 			return ret;
 
-		i915_gem_retire_requests_ring(obj->ring);
+		i915_gem_retire_requests_ring(ring);
 	}
 
 	return 0;
@@ -2896,10 +2900,12 @@ int
 i915_gem_object_sync(struct drm_i915_gem_object *obj,
 		     struct intel_engine_cs *to)
 {
-	struct intel_engine_cs *from = obj->ring;
+	struct intel_engine_cs *from;
 	u32 seqno;
 	int ret, idx;
 
+	from = i915_gem_request_get_ring(obj->last_read_req);
+
 	if (from == NULL || to == from)
 		return 0;
 
@@ -3872,7 +3878,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 	bool was_pin_display;
 	int ret;
 
-	if (pipelined != obj->ring) {
+	if (pipelined != i915_gem_request_get_ring(obj->last_read_req)) {
 		ret = i915_gem_object_sync(obj, pipelined);
 		if (ret)
 			return ret;
@@ -4292,9 +4298,11 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 	ret = i915_gem_object_flush_active(obj);
 
 	args->busy = obj->active;
-	if (obj->ring) {
+	if (obj->last_read_req) {
+		struct intel_engine_cs *ring;
 		BUILD_BUG_ON(I915_NUM_RINGS > 16);
-		args->busy |= intel_ring_flag(obj->ring) << 16;
+		ring = i915_gem_request_get_ring(obj->last_read_req);
+		args->busy |= intel_ring_flag(ring) << 16;
 	}
 
 	drm_gem_object_unreference(&obj->base);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 7d32571..0e35aff 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -613,7 +613,8 @@ static int do_switch(struct intel_engine_cs *ring,
 		 * swapped, but there is no way to do that yet.
 		 */
 		from->legacy_hw_ctx.rcs_state->dirty = 1;
-		BUG_ON(from->legacy_hw_ctx.rcs_state->ring != ring);
+		BUG_ON(i915_gem_request_get_ring(
+			from->legacy_hw_ctx.rcs_state->last_read_req) != ring);
 
 		/* obj is kept alive until the next request by its active ref */
 		i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 10ba188..4c11e5bcf 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -683,7 +683,8 @@ static void capture_bo(struct drm_i915_error_buffer *err,
 	err->dirty = obj->dirty;
 	err->purgeable = obj->madv != I915_MADV_WILLNEED;
 	err->userptr = obj->userptr.mm != NULL;
-	err->ring = obj->ring ? obj->ring->id : -1;
+	err->ring = obj->last_read_req ?
+			i915_gem_request_get_ring(obj->last_read_req)->id : -1;
 	err->cache_level = obj->cache_level;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eca4240..eea2e92 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9304,7 +9304,7 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
 	else if (i915.enable_execlists)
 		return true;
 	else
-		return ring != obj->ring;
+		return ring != i915_gem_request_get_ring(obj->last_read_req);
 }
 
 static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
@@ -9344,7 +9344,7 @@ static int intel_postpone_flip(struct drm_i915_gem_object *obj)
 	if (!obj->last_write_req)
 		return 0;
 
-	ring = obj->ring;
+	ring = i915_gem_request_get_ring(obj->last_write_req);
 
 	if (i915_gem_request_completed(obj->last_write_req, true))
 		return 0;
@@ -9410,14 +9410,15 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
 	spin_lock_irq(&dev_priv->mmio_flip_lock);
 	i915_gem_request_assign(&intel_crtc->mmio_flip.req,
 				obj->last_write_req);
-	intel_crtc->mmio_flip.ring_id = obj->ring->id;
+	WARN_ON(ring != i915_gem_request_get_ring(intel_crtc->mmio_flip.req));
+	intel_crtc->mmio_flip.ring_id = ring->id;
 	spin_unlock_irq(&dev_priv->mmio_flip_lock);
 
 	/*
 	 * Double check to catch cases where irq fired before
 	 * mmio flip data was ready
 	 */
-	intel_notify_mmio_flip(obj->ring);
+	intel_notify_mmio_flip(ring);
 	return 0;
 }
 
@@ -9597,7 +9598,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	} else if (IS_IVYBRIDGE(dev)) {
 		ring = &dev_priv->ring[BCS];
 	} else if (INTEL_INFO(dev)->gen >= 7) {
-		ring = obj->ring;
+		ring = i915_gem_request_get_ring(obj->last_read_req);
 		if (ring == NULL || ring->id != RCS)
 			ring = &dev_priv->ring[BCS];
 	} else {
@@ -9619,7 +9620,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 		i915_gem_request_assign(&work->flip_queued_req,
 					obj->last_write_req);
-		work->flip_queued_ring = obj->ring;
+		work->flip_queued_ring =
+				i915_gem_request_get_ring(obj->last_write_req);
 	} else {
 		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring,
 						   page_flip_flags);
-- 
1.7.9.5

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

[PATCH 23/29] drm/i915: Cache request completion status

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Continuing the removal of seqno based operations - updated the request
completion query to not simply chain on to i915_seqno_passed(). Instead, it now
simply returns a pre-cached completion flag in the fast case. In the slow case
it reads the hardware seqno and, only if it has moved on since the last scan,
looks through the outstanding request list to see which requests can be marked
as completed.

The intention is that this can be optimised further by only doing the completion
scan when an interrupt is raised to say that a request has actually completed on
the hardware. Thus the call to test the completion status of an arbitrary
request simply becomes 'return req->completed'.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |   32 +++++++++++++++----------------
 drivers/gpu/drm/i915/i915_gem.c         |   21 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        |    1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |    2 ++
 drivers/gpu/drm/i915/intel_ringbuffer.h |    3 +++
 5 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 41c2db3..c77cff1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1922,6 +1922,9 @@ void i915_gem_track_fb(struct drm_i915_gem_object *old,
 struct drm_i915_gem_request {
 	struct kref ref;
 
+	/** Is this request known to be complete? */
+	bool complete;
+
 	/** On Which ring this request was generated */
 	struct intel_engine_cs *ring;
 
@@ -1955,6 +1958,8 @@ struct drm_i915_gem_request {
 };
 
 void i915_gem_request_free(struct kref *req_ref);
+void i915_gem_complete_requests_ring(struct intel_engine_cs *ring,
+				     bool lazy_coherency);
 
 static inline uint32_t
 i915_gem_request_get_seqno(struct drm_i915_gem_request *req)
@@ -1995,11 +2000,16 @@ static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
 	*pdst = src;
 }
 
-/*
- * XXX: i915_gem_request_completed should be here but currently needs the
- * definition of i915_seqno_passed() which is below. It will be moved in
- * a later patch when the call to i915_seqno_passed() is obsoleted...
- */
+static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
+					      bool lazy_coherency)
+{
+	if (req->complete)
+		return true;
+
+	i915_gem_complete_requests_ring(req->ring, lazy_coherency);
+
+	return req->complete;
+}
 
 struct drm_i915_file_private {
 	struct drm_i915_private *dev_priv;
@@ -3064,18 +3074,6 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 	}
 }
 
-static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
-					      bool lazy_coherency)
-{
-	u32 seqno;
-
-	BUG_ON(req == NULL);
-
-	seqno = req->ring->get_seqno(req->ring, lazy_coherency);
-
-	return i915_seqno_passed(seqno, req->seqno);
-}
-
 static inline void i915_trace_irq_get(struct intel_engine_cs *ring,
 				      struct drm_i915_gem_request *req)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ea6d679..035735a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2643,6 +2643,27 @@ void i915_gem_request_unreference_irq(struct drm_i915_gem_request *req)
 	spin_unlock_irqrestore(&ring->reqlist_lock, flags);
 }
 
+void i915_gem_complete_requests_ring(struct intel_engine_cs *ring,
+				     bool lazy_coherency)
+{
+	struct drm_i915_gem_request *req;
+	u32 seqno;
+
+	seqno = ring->get_seqno(ring, lazy_coherency);
+	if (seqno == ring->last_read_seqno)
+		return;
+
+	list_for_each_entry(req, &ring->request_list, list) {
+		if (req->complete)
+			continue;
+
+		if (i915_seqno_passed(seqno, req->seqno))
+			req->complete = true;
+	}
+
+	ring->last_read_seqno = seqno;
+}
+
 /**
  * This function clears the request list as sequence numbers are passed.
  */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6e6a255..2c984a4 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -808,6 +808,7 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
 
 	kref_init(&request->ref);
 	request->ring = ring;
+	request->complete = false;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 63d35a6..b5e03d9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2039,6 +2039,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
 
 	kref_init(&request->ref);
 	request->ring = ring;
+	request->complete = false;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
@@ -2131,6 +2132,7 @@ void intel_ring_init_seqno(struct intel_engine_cs *ring, u32 seqno)
 			I915_WRITE(RING_SYNC_2(ring->mmio_base), 0);
 	}
 
+	ring->last_read_seqno = 0;
 	ring->set_seqno(ring, seqno);
 	ring->hangcheck.seqno = seqno;
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index eb754cf5..718eea8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -271,6 +271,9 @@ struct  intel_engine_cs {
 	bool gpu_caches_dirty;
 	bool fbc_dirty;
 
+	/* For optimising request completion events */
+	u32 last_read_seqno;
+
 	wait_queue_head_t irq_queue;
 
 	struct intel_context *default_context;
-- 
1.7.9.5

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

[PATCH 24/29] drm/i915: Zero fill the request structure

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

There is a general theory that kzmalloc is better/safer than kmalloc, especially
for interesting data structures. This change updates the request structure
allocation to be zero filled. That also means it is no longer necessary to
explicitly clear the 'complete' field.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        |    3 +--
 drivers/gpu/drm/i915/intel_ringbuffer.c |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2c984a4..02eeff5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -802,13 +802,12 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
 	if (ring->outstanding_lazy_request)
 		return 0;
 
-	request = kmalloc(sizeof(*request), GFP_KERNEL);
+	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
 		return -ENOMEM;
 
 	kref_init(&request->ref);
 	request->ring = ring;
-	request->complete = false;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b5e03d9..04c9572 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2033,13 +2033,12 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
 	if (ring->outstanding_lazy_request)
 		return 0;
 
-	request = kmalloc(sizeof(*request), GFP_KERNEL);
+	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
 		return -ENOMEM;
 
 	kref_init(&request->ref);
 	request->ring = ring;
-	request->complete = false;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
-- 
1.7.9.5

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

[PATCH 25/29] drm/i915: Spinlock protection for request list

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The completion status for all entries in the request list is updated on demand.
This occurs whenever the code queries the completion status of a given request
and a new seqno value has popped out of the hardware. Unfortuntately, not all
such queries are done with the driver mutex lock held. Therefore there is a race
condition between the query processing and the retired request removal code
which can result in a dodgy pointer dereference.

The solution is to spinlock around those two points - the actual list entry
removal and the potentially asynchronous query. This ensures that the query will
never see a node that is in the process of being destroyed.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 035735a..c4facbd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2491,7 +2491,12 @@ static void i915_set_reset_status(struct drm_i915_private *dev_priv,
 
 static void i915_gem_free_request(struct drm_i915_gem_request *request)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&request->ring->reqlist_lock, flags);
 	list_del(&request->list);
+	spin_unlock_irqrestore(&request->ring->reqlist_lock, flags);
+
 	i915_gem_request_remove_from_client(request);
 
 	i915_gem_request_unreference(request);
@@ -2647,12 +2652,14 @@ void i915_gem_complete_requests_ring(struct intel_engine_cs *ring,
 				     bool lazy_coherency)
 {
 	struct drm_i915_gem_request *req;
+	unsigned long flags;
 	u32 seqno;
 
 	seqno = ring->get_seqno(ring, lazy_coherency);
 	if (seqno == ring->last_read_seqno)
 		return;
 
+	spin_lock_irqsave(&ring->reqlist_lock, flags);
 	list_for_each_entry(req, &ring->request_list, list) {
 		if (req->complete)
 			continue;
@@ -2660,6 +2667,7 @@ void i915_gem_complete_requests_ring(struct intel_engine_cs *ring,
 		if (i915_seqno_passed(seqno, req->seqno))
 			req->complete = true;
 	}
+	spin_unlock_irqrestore(&ring->reqlist_lock, flags);
 
 	ring->last_read_seqno = seqno;
 }
-- 
1.7.9.5

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

[PATCH 26/29] drm/i915: Add uniq id to request structure for debugging

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

For debugging purposes, it is useful to be able to uniquely identify a given
request structure as it works its way through the system. This becomes
especially tricky if the seqno value is late allocated as then the request has
nothing but its pointer to identify it for much of its life.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |    4 ++++
 drivers/gpu/drm/i915/intel_lrc.c        |    2 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c |    2 ++
 3 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c77cff1..1a3db6f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1714,6 +1714,8 @@ struct drm_i915_private {
 		void (*stop_ring)(struct intel_engine_cs *ring);
 	} gt;
 
+	uint32_t request_uniq;
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
@@ -1955,6 +1957,8 @@ struct drm_i915_gem_request {
 
 	/** deferred free list for dereferencing from IRQ context */
 	struct list_head free_list;
+
+	uint32_t uniq;
 };
 
 void i915_gem_request_free(struct kref *req_ref);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 02eeff5..edf1033 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -797,6 +797,7 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
 				      struct intel_context *ctx)
 {
 	struct drm_i915_gem_request *request;
+	struct drm_i915_private *dev_private = ring->dev->dev_private;
 	int ret;
 
 	if (ring->outstanding_lazy_request)
@@ -808,6 +809,7 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
 
 	kref_init(&request->ref);
 	request->ring = ring;
+	request->uniq = dev_private->request_uniq++;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 04c9572..7b5c8b8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2029,6 +2029,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
 {
 	int ret;
 	struct drm_i915_gem_request *request;
+	struct drm_i915_private *dev_private = ring->dev->dev_private;
 
 	if (ring->outstanding_lazy_request)
 		return 0;
@@ -2039,6 +2040,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
 
 	kref_init(&request->ref);
 	request->ring = ring;
+	request->uniq = dev_private->request_uniq++;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
-- 
1.7.9.5

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

[PATCH 27/29] drm/i915: Interrupt driven request completion

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Added a hook to the ring noftification code to process request completion. This
means that there is no longer a need to explicitly process request completions
every time a request object is tested. Instead, the test code simply becomes
'return req->completed'. Obviously, this only works if ring interrupts are
enabled, however, this is already the case for the duration of __wait_request()
which is the point where the driver really needs to know.

To prevent stale requests floating around indefinitely, the retire work handler
also now performs a completion check periodically.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |    5 -----
 drivers/gpu/drm/i915/i915_gem.c |   10 ++++++++++
 drivers/gpu/drm/i915/i915_irq.c |    2 ++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1a3db6f..ab89fd5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2007,11 +2007,6 @@ static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
 static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
 					      bool lazy_coherency)
 {
-	if (req->complete)
-		return true;
-
-	i915_gem_complete_requests_ring(req->ring, lazy_coherency);
-
 	return req->complete;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c4facbd..98052e6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1183,6 +1183,11 @@ static int __wait_request(struct drm_i915_gem_request *req,
 	if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring)))
 		return -ENODEV;
 
+	/* Completion status should be interrupt driven but it is possible
+	 * the request popped out before the interrupt was enabled. So do an
+	 * explicit check now... */
+	i915_gem_complete_requests_ring(req->ring, false);
+
 	/* Record current time in case interrupted by signal, or wedged */
 	trace_i915_gem_request_wait_begin(req);
 	before = ktime_get_raw_ns();
@@ -2405,6 +2410,10 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	list_add_tail(&request->list, &ring->request_list);
 	request->file_priv = NULL;
 
+	/* Avoid race condition where the request completes before it has
+	 * been added to the list. */
+	ring->last_read_seqno = 0;
+
 	if (file) {
 		struct drm_i915_file_private *file_priv = file->driver_priv;
 
@@ -2768,6 +2777,7 @@ i915_gem_retire_requests(struct drm_device *dev)
 	int i;
 
 	for_each_ring(ring, dev_priv, i) {
+		i915_gem_complete_requests_ring(ring, false);
 		i915_gem_retire_requests_ring(ring);
 		idle &= list_empty(&ring->request_list);
 	}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 21e1d69..1f8f708 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -979,6 +979,8 @@ static void notify_ring(struct drm_device *dev,
 
 	trace_i915_gem_request_complete(ring);
 
+	i915_gem_complete_requests_ring(ring, false);
+
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		intel_notify_mmio_flip(ring);
 
-- 
1.7.9.5

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

[PATCH 28/29] drm/i915: Remove obsolete parameter to i915_gem_request_completed()

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The request completion test no longer chains on to the request completion
processing code. Thus it no longer needs to pass a 'lazy coherency' flag through
to the seqno query call. Hence that parameter can be removed.

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |    2 +-
 drivers/gpu/drm/i915/i915_drv.h      |    3 +--
 drivers/gpu/drm/i915/i915_gem.c      |   14 +++++++-------
 drivers/gpu/drm/i915/i915_irq.c      |    2 +-
 drivers/gpu/drm/i915/intel_display.c |    6 +++---
 5 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0f40e61..dc6637d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -550,7 +550,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 					   i915_gem_request_get_seqno(work->flip_queued_req),
 					   dev_priv->next_seqno,
 					   work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
-					   i915_gem_request_completed(work->flip_queued_req, true));
+					   i915_gem_request_completed(work->flip_queued_req));
 			} else
 				seq_printf(m, "Flip not associated with any ring\n");
 			seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ab89fd5..2284ecef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2004,8 +2004,7 @@ static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
 	*pdst = src;
 }
 
-static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
-					      bool lazy_coherency)
+static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req)
 {
 	return req->complete;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 98052e6..ad0458b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1167,7 +1167,7 @@ static int __wait_request(struct drm_i915_gem_request *req,
 
 	WARN(!intel_irqs_enabled(dev_priv), "IRQs disabled");
 
-	if (i915_gem_request_completed(req, true))
+	if (i915_gem_request_completed(req))
 		return 0;
 
 	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
@@ -1208,7 +1208,7 @@ static int __wait_request(struct drm_i915_gem_request *req,
 			break;
 		}
 
-		if (i915_gem_request_completed(req, false)) {
+		if (i915_gem_request_completed(req)) {
 			ret = 0;
 			break;
 		}
@@ -2257,7 +2257,7 @@ i915_gem_object_retire(struct drm_i915_gem_object *obj)
 	if (obj->last_read_req == NULL)
 		return;
 
-	if (i915_gem_request_completed(obj->last_read_req, true))
+	if (i915_gem_request_completed(obj->last_read_req))
 		i915_gem_object_move_to_inactive(obj);
 }
 
@@ -2528,7 +2528,7 @@ i915_gem_find_active_request(struct intel_engine_cs *ring)
 	struct drm_i915_gem_request *request;
 
 	list_for_each_entry(request, &ring->request_list, list) {
-		if (i915_gem_request_completed(request, false))
+		if (i915_gem_request_completed(request))
 			continue;
 
 		return request;
@@ -2703,7 +2703,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 				      struct drm_i915_gem_object,
 				      ring_list);
 
-		if (!i915_gem_request_completed(obj->last_read_req, true))
+		if (!i915_gem_request_completed(obj->last_read_req))
 			break;
 
 		i915_gem_object_move_to_inactive(obj);
@@ -2718,7 +2718,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 					   struct drm_i915_gem_request,
 					   list);
 
-		if (!i915_gem_request_completed(request, true))
+		if (!i915_gem_request_completed(request))
 			break;
 
 		trace_i915_gem_request_retire(request);
@@ -2745,7 +2745,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 	}
 
 	if (unlikely(ring->trace_irq_req &&
-		     i915_gem_request_completed(ring->trace_irq_req, true))) {
+		     i915_gem_request_completed(ring->trace_irq_req))) {
 		ring->irq_put(ring);
 		i915_gem_request_assign(&ring->trace_irq_req, NULL);
 	}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1f8f708..33e93a5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2729,7 +2729,7 @@ static bool
 ring_idle(struct intel_engine_cs *ring)
 {
 	return (list_empty(&ring->request_list) ||
-		i915_gem_request_completed(ring_last_request(ring), false));
+		i915_gem_request_completed(ring_last_request(ring)));
 }
 
 static bool
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eea2e92..69e0d9b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9346,7 +9346,7 @@ static int intel_postpone_flip(struct drm_i915_gem_object *obj)
 
 	ring = i915_gem_request_get_ring(obj->last_write_req);
 
-	if (i915_gem_request_completed(obj->last_write_req, true))
+	if (i915_gem_request_completed(obj->last_write_req))
 		return 0;
 
 	ret = i915_gem_check_olr(obj->last_write_req);
@@ -9376,7 +9376,7 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring)
 		if (ring->id != mmio_flip->ring_id)
 			continue;
 
-		if (i915_gem_request_completed(mmio_flip->req, false)) {
+		if (i915_gem_request_completed(mmio_flip->req)) {
 			intel_do_mmio_flip(intel_crtc);
 			i915_gem_request_assign(&mmio_flip->req, NULL);
 			ring->irq_put(ring);
@@ -9448,7 +9448,7 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 
 	if (work->flip_ready_vblank == 0) {
 		if (work->flip_queued_ring) {
-			if (!i915_gem_request_completed(work->flip_queued_req, true))
+			if (!i915_gem_request_completed(work->flip_queued_req))
 				return false;
 
 			i915_gem_request_unreference_irq(work->flip_queued_req);
-- 
1.7.9.5

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

[PATCH 29/29] WIP: Defer seqno allocation until actual hardware submission time

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

*** Work in progress. Do not submit - currently broken! ***
This patch is being included in the series simply to show the intention.

The seqno value is now only used for the final test for completion of a request.
It is no longer used to track the request through the software stack. Thus it is
no longer necessary to allocate the seqno immediately with the request. Instead,
it can be done lazily and left until the request is actually sent to the
hardware. This is particular advantageous with a GPU scheduler as the requests
can then be re-ordered between their creation and their hardware submission.

The problem with lazy seqno allocation is that wrapping the seqno around zero
currently involves idling the hardware for reasons that are not entirely clear.
This is something that is not safe to do half way through ring submission. Thus
the wrapping must be done in advance even if the actual seqno assignment is not
done until i915_add_request().

Unfortunately, there is no clearly defined point at which to do the
semi-lazy-allocation or advance wrapping. The problem is that requests can be
submitted asynchronously half way through do_execbuffer() processing! The
solution is to ensure that all commands are actually submitted as requests at
the appropriate time by adding extra add_request() calls. That is future work...

For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |    2 +-
 drivers/gpu/drm/i915/i915_gem.c         |   31 ++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_lrc.c        |   10 ++++------
 drivers/gpu/drm/i915/intel_ringbuffer.c |   10 ++++------
 4 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2284ecef..fcc9a4b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2526,7 +2526,7 @@ i915_seqno_passed(uint32_t seq1, uint32_t seq2)
 	return (int32_t)(seq1 - seq2) >= 0;
 }
 
-int __must_check i915_gem_get_seqno(struct drm_device *dev, u32 *seqno);
+int __must_check i915_gem_prepare_next_seqno(struct drm_device *dev);
 int __must_check i915_gem_set_seqno(struct drm_device *dev, u32 seqno);
 int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
 int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ad0458b..da2371a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2314,12 +2314,15 @@ int i915_gem_set_seqno(struct drm_device *dev, u32 seqno)
 }
 
 int
-i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
+i915_gem_prepare_next_seqno(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/* reserve 0 for non-seqno */
 	if (dev_priv->next_seqno == 0) {
+		/* Why is the full re-initialisation required? Is it only for
+		 * hardware semaphores? If so, could skip it in the case where
+		 * semaphores are disabled? */
 		int ret = i915_gem_init_seqno(dev, 0);
 		if (ret)
 			return ret;
@@ -2327,6 +2330,24 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 		dev_priv->next_seqno = 1;
 	}
 
+	return 0;
+}
+
+static int
+i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* reserve 0 for non-seqno */
+	if (dev_priv->next_seqno == 0) {
+		/* Should never get here! Must always call 'prepare_next' in
+		 * advance. This code is called during request submission.
+		 * Trying to wrap the seqno and the implicit idle() calls that
+		 * the wrap code makes are a bad idea at this point! */
+		DRM_ERROR("Need to wrap seqno at inopportune moment!\n");
+		return -EBUSY;
+	}
+
 	*seqno = dev_priv->last_seqno = dev_priv->next_seqno++;
 	return 0;
 }
@@ -2369,6 +2390,11 @@ int __i915_add_request(struct intel_engine_cs *ring,
 			return ret;
 	}
 
+	/* Assign an identifier to track this request through the hardware: */
+	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
+	if (ret)
+		return ret;
+
 	/* 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
@@ -2673,6 +2699,9 @@ void i915_gem_complete_requests_ring(struct intel_engine_cs *ring,
 		if (req->complete)
 			continue;
 
+		if (req->seqno == 0)
+			continue;
+
 		if (i915_seqno_passed(seqno, req->seqno))
 			req->complete = true;
 	}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index edf1033..4d62978 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -803,6 +803,10 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
 	if (ring->outstanding_lazy_request)
 		return 0;
 
+	ret = i915_gem_prepare_next_seqno(ring->dev);
+	if (ret)
+		return ret;
+
 	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
 		return -ENOMEM;
@@ -811,12 +815,6 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
 	request->ring = ring;
 	request->uniq = dev_private->request_uniq++;
 
-	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
-	if (ret) {
-		kfree(request);
-		return ret;
-	}
-
 	/* Hold a reference to the context this request belongs to
 	 * (we will need it when the time comes to emit/retire the
 	 * request).
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 7b5c8b8..9ff4ea6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2034,6 +2034,10 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
 	if (ring->outstanding_lazy_request)
 		return 0;
 
+	ret = i915_gem_prepare_next_seqno(ring->dev);
+	if (ret)
+		return ret;
+
 	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
 		return -ENOMEM;
@@ -2042,12 +2046,6 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
 	request->ring = ring;
 	request->uniq = dev_private->request_uniq++;
 
-	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
-	if (ret) {
-		kfree(request);
-		return ret;
-	}
-
 	ring->outstanding_lazy_request = request;
 	return 0;
 }
-- 
1.7.9.5

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

Re: [PATCH 29/29] WIP: Defer seqno allocation until actual

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=348/348->347/348
PNV: pass/total=328/328->324/328
ILK: pass/total=329/330->329/330
IVB: pass/total=546/546->545/546
SNB: pass/total=540/541->538/541
HSW: pass/total=564/568->562/568
BDW: pass/total=435/435->435/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
BYT: Intel_gpu_tools, igt_gem_seqno_wrap, FAIL(2, M36)PASS(1, M31) -> FAIL(1, M36)PASS(2, M36)
PNV: Intel_gpu_tools, igt_drv_hangman_error-state-basic, PASS(3, M24M25) -> DMESG_WARN(1, M25)PASS(2, M25)
PNV: Intel_gpu_tools, igt_gem_concurrent_blit_cpu-bcs-gpu-read-after-write-interruptible, PASS(3, M24M25) -> DMESG_WARN(1, M25)PASS(2, M25)
PNV: Intel_gpu_tools, igt_kms_setmode_invalid-clone-exclusive-crtc, DMESG_WARN(2, M25)PASS(1, M24) -> DMESG_WARN(1, M25)PASS(2, M25)
PNV: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, DMESG_WARN(2, M25)PASS(1, M24) -> DMESG_WARN(1, M25)PASS(2, M25)
ILK: Intel_gpu_tools, igt_drv_missed_irq_hang, DMESG_FAIL(2, M37)PASS(1, M37) -> DMESG_FAIL(1, M37)PASS(2, M37)
ILK: Intel_gpu_tools, igt_gem_close_race_gem-close-race, DMESG_WARN(1, M37)PASS(2, M37) -> PASS(3, M37)
IVB: Intel_gpu_tools, igt_gem_seqno_wrap, FAIL(2, M4)PASS(1, M4) -> FAIL(1, M4)PASS(2, M4)
SNB: Intel_gpu_tools, igt_drv_missed_irq_hang, FAIL(2, M35)PASS(1, M35) -> FAIL(1, M35)PASS(2, M35)
SNB: Intel_gpu_tools, igt_pm_rpm_gem-idle, PASS(1, M35) -> NO_RESULT(1, M35)PASS(2, M35)
HSW: Intel_gpu_tools, igt_drv_missed_irq_hang, DMESG_FAIL(2, M39)PASS(1, M39) -> FAIL(1, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, DMESG_FAIL(2, M39)PASS(1, M39) -> NSPT(1, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_gem_seqno_wrap, DMESG_FAIL(2, M39)PASS(1, M39) -> FAIL(1, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-128x128-sliding, DMESG_FAIL(2, M39)DMESG_WARN(1, M39) -> DMESG_WARN(2, M39)PASS(1, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-random, DMESG_FAIL(2, M39)DMESG_WARN(1, M39) -> DMESG_WARN(2, M39)PASS(1, M39)
HSW: Intel_gpu_tools, igt_pm_rpm_gem-idle, DMESG_FAIL(2, M39)PASS(1, M39) -> DMESG_WARN(1, M39)PASS(2, M39)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/29] Replace seqno values with request structures

[John Harrison]

Oops, forgot to update the text for the zero patch. This is no longer 
'work in progress' and is now intended for proper review! The 
description should read as follows:

There is a general feeling that it is better to move away from using a 
simple
integer 'seqno' value to track batch buffer completion. Instead, the request
structure should be used. That provides for much more flexibility going
forwards. Especially which things like a GPU scheduler (which can 
re-order batch
buffers and hence seqnos after submission to the hardware), Android sync 
points
and other such features which potentially make seqno usage more and more
complex.

This patch set does the work of converting most of the driver to use request
structures in preference to seqno values. The only place left that still 
uses
seqnos is the semaphore code. It was decided to leave that alone for the 
time
being as the semaphores are hardware based and the hardware only understands
seqno values.

[Patches against drm-intel-nightly tree fetched 29/10/2014]


On 30/10/2014 18:40, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Work in progress for replacing seqno usage with requst structures.
>
> There is a general feeling that it is better to move away from using a simple
> integer 'seqno' value to track batch buffer completion. Instead, the request
> structure should be used. That provides for much more flexibility going
> forwards. Especially which things like a GPU scheduler (which can re-order batch
> buffers and hence seqnos after submission to the hardware), Android sync points
> and other such features which potentially make seqno usage more and more
> complex.
>
> The current set of patches do most of the seqno to request structure conversion.
> There are still a couple of direct seqno comparisons in the semaphore code. The
> final conversion of a seqno test into a 'completed' flag inside the request
> structure is still do to as well. Along with whatever changes are required to
> maintain such a flag.
>
> The patches are being posted now to make sure that the various people involved
> agree that it is heading in the right direction.
>
> [Patches against drm-intel-nightly tree fetched 11/09/2014]
>
> John Harrison (29):
>    drm/i915: Remove redundant parameter to i915_gem_object_wait_rendering__tail()
>    drm/i915: Ensure OLS & PLR are always in sync
>    drm/i915: Add reference count to request structure
>    drm/i915: Add helper functions to aid seqno -> request transition
>    drm/i915: Replace last_[rwf]_seqno with last_[rwf]_req
>    drm/i915: Convert i915_gem_ring_throttle to use requests
>    drm/i915: Ensure requests stick around during waits
>    drm/i915: Remove 'outstanding_lazy_seqno'
>    drm/i915: Make 'i915_gem_check_olr' actually check by request not seqno
>    drm/i915: Convert 'last_flip_req' to be a request not a seqno
>    drm/i915: Convert i915_wait_seqno to i915_wait_request
>    drm/i915: Convert __wait_seqno() to __wait_request()
>    drm/i915: Remove obsolete seqno parameter from 'i915_add_request'
>    drm/i915: Convert mmio_flip::seqno to struct request
>    drm/i915: Add IRQ friendly request deference facility
>    drm/i915: Convert 'flip_queued_seqno' into 'flip_queued_request'
>    drm/i915: Convert trace functions from seqno to request
>    drm/i915: Convert 'trace_irq' to use requests rather than seqnos
>    drm/i915: Convert 'ring_idle()' to use requests not seqnos
>    drm/i915: Connect requests to rings at creation not submission
>    drm/i915: Convert 'i915_seqno_passed' calls into 'i915_gem_request_completed'
>    drm/i915: Remove the now redundant 'obj->ring'
>    drm/i915: Cache request completion status
>    drm/i915: Zero fill the request structure
>    drm/i915: Spinlock protection for request list
>    drm/i915: Add uniq id to request structure for debugging
>    drm/i915: Interrupt driven request completion
>    drm/i915: Remove obsolete parameter to i915_gem_request_completed()
>    WIP: Defer seqno allocation until actual hardware submission time
>
>   drivers/gpu/drm/i915/i915_debugfs.c          |   20 +-
>   drivers/gpu/drm/i915/i915_drv.h              |   89 ++++++-
>   drivers/gpu/drm/i915/i915_gem.c              |  356 +++++++++++++++++---------
>   drivers/gpu/drm/i915/i915_gem_context.c      |    3 +-
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c   |   10 +-
>   drivers/gpu/drm/i915/i915_gem_gtt.h          |    4 +-
>   drivers/gpu/drm/i915/i915_gem_render_state.c |    2 +-
>   drivers/gpu/drm/i915/i915_gem_tiling.c       |    2 +-
>   drivers/gpu/drm/i915/i915_gpu_error.c        |    7 +-
>   drivers/gpu/drm/i915/i915_irq.c              |   14 +-
>   drivers/gpu/drm/i915/i915_trace.h            |   47 ++--
>   drivers/gpu/drm/i915/intel_display.c         |   53 ++--
>   drivers/gpu/drm/i915/intel_drv.h             |    4 +-
>   drivers/gpu/drm/i915/intel_lrc.c             |   59 +++--
>   drivers/gpu/drm/i915/intel_overlay.c         |   24 +-
>   drivers/gpu/drm/i915/intel_ringbuffer.c      |   82 +++---
>   drivers/gpu/drm/i915/intel_ringbuffer.h      |   25 +-
>   17 files changed, 505 insertions(+), 296 deletions(-)
>

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

Re: [PATCH 00/29] Replace seqno values with request structures

[Daniel Vetter]

On Thu, Oct 30, 2014 at 06:55:36PM +0000, John Harrison wrote:
> Oops, forgot to update the text for the zero patch. This is no longer 'work
> in progress' and is now intended for proper review! The description should
> read as follows:
> 
> There is a general feeling that it is better to move away from using a
> simple
> integer 'seqno' value to track batch buffer completion. Instead, the request
> structure should be used. That provides for much more flexibility going
> forwards. Especially which things like a GPU scheduler (which can re-order
> batch
> buffers and hence seqnos after submission to the hardware), Android sync
> points
> and other such features which potentially make seqno usage more and more
> complex.
> 
> This patch set does the work of converting most of the driver to use request
> structures in preference to seqno values. The only place left that still
> uses
> seqnos is the semaphore code. It was decided to leave that alone for the
> time
> being as the semaphores are hardware based and the hardware only understands
> seqno values.
> 
> [Patches against drm-intel-nightly tree fetched 29/10/2014]

I've done a (very) quick readthrough and looks sane - all my bikesheds
seem to have been addressed. Or I've forgotten them again which means they
can't be that important ;-)

Anyway, please find some proper reviewer for this so I can vacuum it in.

Thanks, Daniel

> 
> 
> On 30/10/2014 18:40, John.C.Harrison@Intel.com wrote:
> >From: John Harrison <John.C.Harrison@Intel.com>
> >
> >Work in progress for replacing seqno usage with requst structures.
> >
> >There is a general feeling that it is better to move away from using a simple
> >integer 'seqno' value to track batch buffer completion. Instead, the request
> >structure should be used. That provides for much more flexibility going
> >forwards. Especially which things like a GPU scheduler (which can re-order batch
> >buffers and hence seqnos after submission to the hardware), Android sync points
> >and other such features which potentially make seqno usage more and more
> >complex.
> >
> >The current set of patches do most of the seqno to request structure conversion.
> >There are still a couple of direct seqno comparisons in the semaphore code. The
> >final conversion of a seqno test into a 'completed' flag inside the request
> >structure is still do to as well. Along with whatever changes are required to
> >maintain such a flag.
> >
> >The patches are being posted now to make sure that the various people involved
> >agree that it is heading in the right direction.
> >
> >[Patches against drm-intel-nightly tree fetched 11/09/2014]
> >
> >John Harrison (29):
> >   drm/i915: Remove redundant parameter to i915_gem_object_wait_rendering__tail()
> >   drm/i915: Ensure OLS & PLR are always in sync
> >   drm/i915: Add reference count to request structure
> >   drm/i915: Add helper functions to aid seqno -> request transition
> >   drm/i915: Replace last_[rwf]_seqno with last_[rwf]_req
> >   drm/i915: Convert i915_gem_ring_throttle to use requests
> >   drm/i915: Ensure requests stick around during waits
> >   drm/i915: Remove 'outstanding_lazy_seqno'
> >   drm/i915: Make 'i915_gem_check_olr' actually check by request not seqno
> >   drm/i915: Convert 'last_flip_req' to be a request not a seqno
> >   drm/i915: Convert i915_wait_seqno to i915_wait_request
> >   drm/i915: Convert __wait_seqno() to __wait_request()
> >   drm/i915: Remove obsolete seqno parameter from 'i915_add_request'
> >   drm/i915: Convert mmio_flip::seqno to struct request
> >   drm/i915: Add IRQ friendly request deference facility
> >   drm/i915: Convert 'flip_queued_seqno' into 'flip_queued_request'
> >   drm/i915: Convert trace functions from seqno to request
> >   drm/i915: Convert 'trace_irq' to use requests rather than seqnos
> >   drm/i915: Convert 'ring_idle()' to use requests not seqnos
> >   drm/i915: Connect requests to rings at creation not submission
> >   drm/i915: Convert 'i915_seqno_passed' calls into 'i915_gem_request_completed'
> >   drm/i915: Remove the now redundant 'obj->ring'
> >   drm/i915: Cache request completion status
> >   drm/i915: Zero fill the request structure
> >   drm/i915: Spinlock protection for request list
> >   drm/i915: Add uniq id to request structure for debugging
> >   drm/i915: Interrupt driven request completion
> >   drm/i915: Remove obsolete parameter to i915_gem_request_completed()
> >   WIP: Defer seqno allocation until actual hardware submission time
> >
> >  drivers/gpu/drm/i915/i915_debugfs.c          |   20 +-
> >  drivers/gpu/drm/i915/i915_drv.h              |   89 ++++++-
> >  drivers/gpu/drm/i915/i915_gem.c              |  356 +++++++++++++++++---------
> >  drivers/gpu/drm/i915/i915_gem_context.c      |    3 +-
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c   |   10 +-
> >  drivers/gpu/drm/i915/i915_gem_gtt.h          |    4 +-
> >  drivers/gpu/drm/i915/i915_gem_render_state.c |    2 +-
> >  drivers/gpu/drm/i915/i915_gem_tiling.c       |    2 +-
> >  drivers/gpu/drm/i915/i915_gpu_error.c        |    7 +-
> >  drivers/gpu/drm/i915/i915_irq.c              |   14 +-
> >  drivers/gpu/drm/i915/i915_trace.h            |   47 ++--
> >  drivers/gpu/drm/i915/intel_display.c         |   53 ++--
> >  drivers/gpu/drm/i915/intel_drv.h             |    4 +-
> >  drivers/gpu/drm/i915/intel_lrc.c             |   59 +++--
> >  drivers/gpu/drm/i915/intel_overlay.c         |   24 +-
> >  drivers/gpu/drm/i915/intel_ringbuffer.c      |   82 +++---
> >  drivers/gpu/drm/i915/intel_ringbuffer.h      |   25 +-
> >  17 files changed, 505 insertions(+), 296 deletions(-)
> >
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx