Attachment 56750 Details for Bug 45492 – Record tail at each request and use to estimate ring->head.

[patch] Record tail at each request and use to estimate ring->head.

0002-drm-i915-Record-the-tail-at-each-request-and-use-it-.patch (text/plain), 7.45 KB, created by Chris Wilson on 2012-02-08 06:26:32 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2012-02-08 06:26:32 UTC

Size: 7.45 KB

patch

obsolete

>From 8282b495f95937ea273a4d7b9bc8aa4405edd4b4 Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>Date: Wed, 1 Feb 2012 21:21:06 +0000
>Subject: [PATCH 2/2] drm/i915: Record the tail at each request and use it to
> estimate the head
>
>By recording the location of every request in the ringbuffer, we know
>that in order to retire the request the GPU must have finished reading
>it and so the GPU head is now beyond the tail of the request. We can
>therefore provide a conservative estimate of where the GPU is reading
>from in order to avoid having to read back the ring buffer registers
>when polling for space upon starting a new write into the ringbuffer.
>
>A secondary effect is that this allows us to convert
>intel_ring_buffer_wait() to use i915_wait_request() and so consolidate
>upon the single function to handle the complicated task of waiting upon
>the GPU. A necessary precaution is that we need to make that wait
>uninterruptible to match the existing conditions as all the callers of
>intel_ring_begin() have not been audited to handle ERESTARTSYS
>correctly.
>
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> drivers/gpu/drm/i915/i915_drv.h         |    5 ++
> drivers/gpu/drm/i915/i915_gem.c         |    9 +++-
> drivers/gpu/drm/i915/intel_ringbuffer.c |   83 +++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_ringbuffer.h |   15 ++++++
> 4 files changed, 111 insertions(+), 1 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 4ee0793..e8e1b85 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -945,6 +945,9 @@ struct drm_i915_gem_request {
> 	/** GEM sequence number associated with this request. */
> 	uint32_t seqno;
> 
>+	/** Postion in the ringbuffer of the end of the request */
>+	u32 tail;
>+
> 	/** Time at which this request was emitted, in jiffies. */
> 	unsigned long emitted_jiffies;
> 
>@@ -1234,6 +1237,8 @@ i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj)
> }
> 
> void i915_gem_retire_requests(struct drm_device *dev);
>+void i915_gem_retire_requests_ring(struct intel_ring_buffer *ring);
>+
> void i915_gem_reset(struct drm_device *dev);
> void i915_gem_clflush_object(struct drm_i915_gem_object *obj);
> int __must_check i915_gem_object_set_domain(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 51390b0..4de9e4e 100644
>--- a/drivers/gpu/drm/i915/i915_gem.c
>+++ b/drivers/gpu/drm/i915/i915_gem.c
>@@ -1798,6 +1798,7 @@ i915_add_request(struct intel_ring_buffer *ring,
> 
> 	request->seqno = seqno;
> 	request->ring = ring;
>+	request->tail = intel_ring_get_tail(ring);
> 	request->emitted_jiffies = jiffies;
> 	was_empty = list_empty(&ring->request_list);
> 	list_add_tail(&request->list, &ring->request_list);
>@@ -1934,7 +1935,7 @@ void i915_gem_reset(struct drm_device *dev)
> /**
>  * This function clears the request list as sequence numbers are passed.
>  */
>-static void
>+void
> i915_gem_retire_requests_ring(struct intel_ring_buffer *ring)
> {
> 	uint32_t seqno;
>@@ -1962,6 +1963,12 @@ i915_gem_retire_requests_ring(struct intel_ring_buffer *ring)
> 			break;
> 
> 		trace_i915_gem_request_retire(ring, request->seqno);
>+		/* We know the GPU must have read the request to have
>+		 * sent us the seqno + interrupt, so use the position
>+		 * of tail of the request to update the last known position
>+		 * of the GPU head.
>+		 */
>+		ring->last_retired_head = request->tail;
> 
> 		list_del(&request->list);
> 		i915_gem_request_remove_from_client(request);
>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>index 620b2ed..2bb5226 100644
>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>@@ -533,6 +533,7 @@ pc_render_add_request(struct intel_ring_buffer *ring,
> 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
> 	scratch_addr += 128;
> 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
>+
> 	intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4) | PIPE_CONTROL_QW_WRITE |
> 			PIPE_CONTROL_WRITE_FLUSH |
> 			PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE |
>@@ -1054,11 +1055,93 @@ static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring)
> 	return 0;
> }
> 
>+static int intel_ring_wait_seqno(struct intel_ring_buffer *ring, u32 seqno)
>+{
>+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
>+	bool was_interruptible;
>+	int ret;
>+
>+	/* XXX As we have not yet audited all the paths to check that
>+	 * they are ready for ERESTARTSYS from intel_ring_begin, do not
>+	 * allow us to be interruptible by a signal.
>+	 */
>+	was_interruptible = dev_priv->mm.interruptible;
>+	dev_priv->mm.interruptible = false;
>+
>+	ret = i915_wait_request(ring, seqno);
>+
>+	dev_priv->mm.interruptible = was_interruptible;
>+
>+	return ret;
>+}
>+
>+static int intel_ring_wait_request(struct intel_ring_buffer *ring, int n)
>+{
>+	struct drm_i915_gem_request *request;
>+	u32 seqno = 0;
>+	int ret;
>+
>+	i915_gem_retire_requests_ring(ring);
>+
>+	if (ring->last_retired_head != -1) {
>+		ring->head = ring->last_retired_head;
>+		ring->last_retired_head = -1;
>+		ring->space = ring_space(ring);
>+		if (ring->space >= n)
>+			return 0;
>+	}
>+
>+	list_for_each_entry(request, &ring->request_list, list) {
>+		int space;
>+
>+		if (request->tail == -1)
>+			continue;
>+
>+		space = request->tail - (ring->tail + 8);
>+		if (space < 0)
>+			space += ring->size;
>+		if (space >= n) {
>+			seqno = request->seqno;
>+			break;
>+		}
>+
>+		/* Consume this request in case we need more space than
>+		 * is available and so need to prevent a race between
>+		 * updating last_retired_head and direct reads of
>+		 * I915_RING_HEAD.
>+		 */
>+		request->tail = -1;
>+	}
>+
>+	if (seqno == 0)
>+		return -ENOSPC;
>+
>+	ret = intel_ring_wait_seqno(ring, seqno);
>+	if (ret)
>+		return ret;
>+
>+	if (WARN_ON(ring->last_retired_head == -1))
>+		return -ENOSPC;
>+
>+	ring->head = ring->last_retired_head;
>+	ring->last_retired_head = -1;
>+	ring->space = ring_space(ring);
>+	if (WARN_ON(ring->space < n))
>+		return -ENOSPC;
>+
>+	return 0;
>+}
>+
> int intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n)
> {
> 	struct drm_device *dev = ring->dev;
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	unsigned long end;
>+	int ret;
>+
>+	ret = intel_ring_wait_request(ring, n);
>+	if (ret != -ENOSPC)
>+		return ret;
> 
> 	trace_i915_ring_wait_begin(ring);
> 	if (drm_core_check_feature(dev, DRIVER_GEM))
>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
>index 4c5338c..855233b 100644
>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>@@ -46,6 +46,16 @@ struct  intel_ring_buffer {
> 	int		effective_size;
> 	struct intel_hw_status_page status_page;
> 
>+	/** We track the position of the requests in the ring buffer, and
>+	 * when each is retired we increment last_retired_head as the GPU
>+	 * must have finished processing the request and so we know we
>+	 * can advance the ringbuffer up to that position.
>+	 *
>+	 * last_retired_head is set to -1 after the value is consumed so
>+	 * we can detect new retirements.
>+	 */
>+	u32		last_retired_head;
>+
> 	spinlock_t	irq_lock;
> 	u32		irq_refcount;
> 	u32		irq_mask;
>@@ -191,6 +201,11 @@ int intel_init_blt_ring_buffer(struct drm_device *dev);
> u32 intel_ring_get_active_head(struct intel_ring_buffer *ring);
> void intel_ring_setup_status_page(struct intel_ring_buffer *ring);
> 
>+static inline u32 intel_ring_get_tail(struct intel_ring_buffer *ring)
>+{
>+	return ring->tail;
>+}
>+
> static inline void i915_trace_irq_get(struct intel_ring_buffer *ring, u32 seqno)
> {
> 	if (ring->trace_irq_seqno == 0 && ring->irq_get(ring))
>-- 
>1.7.9
>

Actions: View | Splinter Review

Attachments on bug 45492: 56439 | 56445 | 56453 | 56463 | 56479 | 56480 | 56481 | 56522 | 56749 | 56750 | 56765

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.