* [PATCH 01/12 v2] drm/i915: remove do_retire from i915_wait_request
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
@ 2012-04-26 23:02 ` Ben Widawsky
2012-04-27 14:25 ` Daniel Vetter
2012-04-26 23:02 ` [PATCH 02/12] drm/i915: remove some extra retiring Ben Widawsky
` (10 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This originates from a hack by me to quickly fix a bug in an earlier
patch where we needed control over whether or not waiting on a seqno
actually did any retire list processing. Since the two operations aren't
clearly related, we should pull the parameter out of the wait function,
and make the caller responsible for retiring if the action is desired.
The only function call site which did not get an explicit retire_request call
(on purpose) is i915_gem_inactive_shrink(). That code was already calling
retire_request a second time.
v2: don't modify any behavior excepit i915_gem_inactive_shrink(Daniel)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_dma.c | 3 ++-
drivers/gpu/drm/i915/i915_drv.h | 5 ++---
drivers/gpu/drm/i915/i915_gem.c | 33 ++++++++++------------------
drivers/gpu/drm/i915/i915_gem_evict.c | 15 ++++++++++---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ++-
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
drivers/gpu/drm/i915/intel_overlay.c | 8 +++----
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +++-
8 files changed, 37 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e2983a9..36c8d5f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2013,9 +2013,10 @@ int i915_driver_unload(struct drm_device *dev)
unregister_shrinker(&dev_priv->mm.inactive_shrinker);
mutex_lock(&dev->struct_mutex);
- ret = i915_gpu_idle(dev, true);
+ ret = i915_gpu_idle(dev);
if (ret)
DRM_ERROR("failed to idle hardware: %d\n", ret);
+ i915_gem_retire_requests(dev);
mutex_unlock(&dev->struct_mutex);
/* Cancel the retire work handler, which should be idle now. */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf2c896..34d7041 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1298,14 +1298,13 @@ int __must_check i915_gem_init_hw(struct drm_device *dev);
void i915_gem_init_swizzling(struct drm_device *dev);
void i915_gem_init_ppgtt(struct drm_device *dev);
void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
-int __must_check i915_gpu_idle(struct drm_device *dev, bool do_retire);
+int __must_check i915_gpu_idle(struct drm_device *dev);
int __must_check i915_gem_idle(struct drm_device *dev);
int __must_check i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_request *request);
int __must_check i915_wait_request(struct intel_ring_buffer *ring,
- uint32_t seqno,
- bool do_retire);
+ uint32_t seqno);
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,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0d53eac..25be0e0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1825,8 +1825,7 @@ i915_gem_retire_work_handler(struct work_struct *work)
*/
int
i915_wait_request(struct intel_ring_buffer *ring,
- uint32_t seqno,
- bool do_retire)
+ uint32_t seqno)
{
drm_i915_private_t *dev_priv = ring->dev->dev_private;
u32 ier;
@@ -1902,14 +1901,6 @@ i915_wait_request(struct intel_ring_buffer *ring,
if (atomic_read(&dev_priv->mm.wedged))
ret = -EAGAIN;
- /* Directly dispatch request retiring. While we have the work queue
- * to handle this, the waiter on a request often wants an associated
- * buffer to have made it to the inactive list, and we would need
- * a separate wait queue to handle that.
- */
- if (ret == 0 && do_retire)
- i915_gem_retire_requests_ring(ring);
-
return ret;
}
@@ -1931,10 +1922,10 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj)
* it.
*/
if (obj->active) {
- ret = i915_wait_request(obj->ring, obj->last_rendering_seqno,
- true);
+ ret = i915_wait_request(obj->ring, obj->last_rendering_seqno);
if (ret)
return ret;
+ i915_gem_retire_requests_ring(obj->ring);
}
return 0;
@@ -2117,7 +2108,7 @@ i915_gem_flush_ring(struct intel_ring_buffer *ring,
return 0;
}
-static int i915_ring_idle(struct intel_ring_buffer *ring, bool do_retire)
+static int i915_ring_idle(struct intel_ring_buffer *ring)
{
int ret;
@@ -2131,18 +2122,17 @@ static int i915_ring_idle(struct intel_ring_buffer *ring, bool do_retire)
return ret;
}
- return i915_wait_request(ring, i915_gem_next_request_seqno(ring),
- do_retire);
+ return i915_wait_request(ring, i915_gem_next_request_seqno(ring));
}
-int i915_gpu_idle(struct drm_device *dev, bool do_retire)
+int i915_gpu_idle(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
int ret, i;
/* Flush everything onto the inactive list. */
for (i = 0; i < I915_NUM_RINGS; i++) {
- ret = i915_ring_idle(&dev_priv->ring[i], do_retire);
+ ret = i915_ring_idle(&dev_priv->ring[i]);
if (ret)
return ret;
}
@@ -2331,9 +2321,7 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object *obj)
}
if (obj->last_fenced_seqno) {
- ret = i915_wait_request(obj->ring,
- obj->last_fenced_seqno,
- false);
+ ret = i915_wait_request(obj->ring, obj->last_fenced_seqno);
if (ret)
return ret;
@@ -3394,11 +3382,12 @@ i915_gem_idle(struct drm_device *dev)
return 0;
}
- ret = i915_gpu_idle(dev, true);
+ ret = i915_gpu_idle(dev);
if (ret) {
mutex_unlock(&dev->struct_mutex);
return ret;
}
+ i915_gem_retire_requests(dev);
/* Under UMS, be paranoid and evict. */
if (!drm_core_check_feature(dev, DRIVER_MODESET))
@@ -4029,7 +4018,7 @@ rescan:
* This has a dramatic impact to reduce the number of
* OOM-killer events whilst running the GPU aggressively.
*/
- if (i915_gpu_idle(dev, true) == 0)
+ if (i915_gpu_idle(dev) == 0)
goto rescan;
}
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 91ebb94..3bcf045 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -168,7 +168,7 @@ i915_gem_evict_everything(struct drm_device *dev, bool purgeable_only)
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj, *next;
bool lists_empty;
- int ret;
+ int ret,i;
lists_empty = (list_empty(&dev_priv->mm.inactive_list) &&
list_empty(&dev_priv->mm.flushing_list) &&
@@ -178,11 +178,20 @@ i915_gem_evict_everything(struct drm_device *dev, bool purgeable_only)
trace_i915_gem_evict_everything(dev, purgeable_only);
- /* Flush everything (on to the inactive lists) and evict */
- ret = i915_gpu_idle(dev, true);
+ ret = i915_gpu_idle(dev);
if (ret)
return ret;
+ /* The gpu_idle will flush everything in the write domain to the
+ * active list. Then we must move everything off the active list
+ * with retire requests.
+ */
+ for (i = 0; i < I915_NUM_RINGS; i++)
+ if (WARN_ON(!list_empty(&dev_priv->ring[i].gpu_write_list)))
+ return -EBUSY;
+
+ i915_gem_retire_requests(dev);
+
BUG_ON(!list_empty(&dev_priv->mm.flushing_list));
/* Having flushed everything, unbind() should never raise an error */
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 68ec013..cbba0aa 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1220,9 +1220,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
* so every billion or so execbuffers, we need to stall
* the GPU in order to reset the counters.
*/
- ret = i915_gpu_idle(dev, true);
+ ret = i915_gpu_idle(dev);
if (ret)
goto err;
+ i915_gem_retire_requests(dev);
BUG_ON(ring->sync_seqno[i]);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 25c8bf9..29d573c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -317,7 +317,7 @@ static bool do_idling(struct drm_i915_private *dev_priv)
if (unlikely(dev_priv->mm.gtt->do_idle_maps)) {
dev_priv->mm.interruptible = false;
- if (i915_gpu_idle(dev_priv->dev, false)) {
+ if (i915_gpu_idle(dev_priv->dev)) {
DRM_ERROR("Couldn't idle GPU\n");
/* Wait a bit, in hopes it avoids the hang */
udelay(10);
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index a0b5053..e06e46a 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -225,10 +225,10 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
}
overlay->last_flip_req = request->seqno;
overlay->flip_tail = tail;
- ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req,
- true);
+ ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
if (ret)
return ret;
+ i915_gem_retire_requests(dev);
overlay->last_flip_req = 0;
return 0;
@@ -447,10 +447,10 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
if (overlay->last_flip_req == 0)
return 0;
- ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req,
- true);
+ ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
if (ret)
return ret;
+ i915_gem_retire_requests(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 427b7c5..a7d97d1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1088,9 +1088,11 @@ static int intel_ring_wait_seqno(struct intel_ring_buffer *ring, u32 seqno)
was_interruptible = dev_priv->mm.interruptible;
dev_priv->mm.interruptible = false;
- ret = i915_wait_request(ring, seqno, true);
+ ret = i915_wait_request(ring, seqno);
dev_priv->mm.interruptible = was_interruptible;
+ if (!ret)
+ i915_gem_retire_requests_ring(ring);
return ret;
}
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 01/12 v2] drm/i915: remove do_retire from i915_wait_request
2012-04-26 23:02 ` [PATCH 01/12 v2] drm/i915: remove do_retire from i915_wait_request Ben Widawsky
@ 2012-04-27 14:25 ` Daniel Vetter
0 siblings, 0 replies; 32+ messages in thread
From: Daniel Vetter @ 2012-04-27 14:25 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, Apr 26, 2012 at 04:02:58PM -0700, Ben Widawsky wrote:
> This originates from a hack by me to quickly fix a bug in an earlier
> patch where we needed control over whether or not waiting on a seqno
> actually did any retire list processing. Since the two operations aren't
> clearly related, we should pull the parameter out of the wait function,
> and make the caller responsible for retiring if the action is desired.
>
> The only function call site which did not get an explicit retire_request call
> (on purpose) is i915_gem_inactive_shrink(). That code was already calling
> retire_request a second time.
>
> v2: don't modify any behavior excepit i915_gem_inactive_shrink(Daniel)
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 02/12] drm/i915: remove some extra retiring
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
2012-04-26 23:02 ` [PATCH 01/12 v2] drm/i915: remove do_retire from i915_wait_request Ben Widawsky
@ 2012-04-26 23:02 ` Ben Widawsky
2012-04-27 14:21 ` Daniel Vetter
2012-04-26 23:03 ` [PATCH 03/12 v2] drm/i915: move vbetool invoked ier stuff Ben Widawsky
` (9 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Not every caller of gpu_idle needs to retire requests. Try to pick only
the callers that need it. This was originally combined with the previous
patch in the first series on the mailing list.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_dma.c | 1 -
drivers/gpu/drm/i915/i915_gem.c | 1 -
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 -
drivers/gpu/drm/i915/intel_overlay.c | 2 --
4 files changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 36c8d5f..e73389d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2016,7 +2016,6 @@ int i915_driver_unload(struct drm_device *dev)
ret = i915_gpu_idle(dev);
if (ret)
DRM_ERROR("failed to idle hardware: %d\n", ret);
- i915_gem_retire_requests(dev);
mutex_unlock(&dev->struct_mutex);
/* Cancel the retire work handler, which should be idle now. */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 25be0e0..3b731ef 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3387,7 +3387,6 @@ i915_gem_idle(struct drm_device *dev)
mutex_unlock(&dev->struct_mutex);
return ret;
}
- i915_gem_retire_requests(dev);
/* Under UMS, be paranoid and evict. */
if (!drm_core_check_feature(dev, DRIVER_MODESET))
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index cbba0aa..582f6c4 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1223,7 +1223,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
ret = i915_gpu_idle(dev);
if (ret)
goto err;
- i915_gem_retire_requests(dev);
BUG_ON(ring->sync_seqno[i]);
}
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index e06e46a..07a5cad 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -228,7 +228,6 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
if (ret)
return ret;
- i915_gem_retire_requests(dev);
overlay->last_flip_req = 0;
return 0;
@@ -450,7 +449,6 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
if (ret)
return ret;
- i915_gem_retire_requests(dev);
if (overlay->flip_tail)
overlay->flip_tail(overlay);
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 02/12] drm/i915: remove some extra retiring
2012-04-26 23:02 ` [PATCH 02/12] drm/i915: remove some extra retiring Ben Widawsky
@ 2012-04-27 14:21 ` Daniel Vetter
0 siblings, 0 replies; 32+ messages in thread
From: Daniel Vetter @ 2012-04-27 14:21 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, Apr 26, 2012 at 04:02:59PM -0700, Ben Widawsky wrote:
> Not every caller of gpu_idle needs to retire requests. Try to pick only
> the callers that need it. This was originally combined with the previous
> patch in the first series on the mailing list.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 1 -
> drivers/gpu/drm/i915/i915_gem.c | 1 -
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 -
> drivers/gpu/drm/i915/intel_overlay.c | 2 --
> 4 files changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 36c8d5f..e73389d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -2016,7 +2016,6 @@ int i915_driver_unload(struct drm_device *dev)
> ret = i915_gpu_idle(dev);
> if (ret)
> DRM_ERROR("failed to idle hardware: %d\n", ret);
> - i915_gem_retire_requests(dev);
> mutex_unlock(&dev->struct_mutex);
>
> /* Cancel the retire work handler, which should be idle now. */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 25be0e0..3b731ef 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3387,7 +3387,6 @@ i915_gem_idle(struct drm_device *dev)
> mutex_unlock(&dev->struct_mutex);
> return ret;
> }
> - i915_gem_retire_requests(dev);
Imo this will still blow up resume on gen2/3 ... we may not leak any
request over a s/r cycle in general, because on resume we restart with the
seqno counting at 1. So when you then want to wait for a no longer active,
but not yet retired request after resume, that will take a while.
I also miss a bit of justification for the first hunk, but module unload
calling gpu_idle instead of gem_idle is a bit fishy in and off itself.
I think the parts below are ok, but I'm hunting an overlay regression in
dinq atm, so I'll hold of merging this.
-Daniel
> /* Under UMS, be paranoid and evict. */
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index cbba0aa..582f6c4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1223,7 +1223,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> ret = i915_gpu_idle(dev);
> if (ret)
> goto err;
> - i915_gem_retire_requests(dev);
>
> BUG_ON(ring->sync_seqno[i]);
> }
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index e06e46a..07a5cad 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -228,7 +228,6 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
> ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
> if (ret)
> return ret;
> - i915_gem_retire_requests(dev);
>
> overlay->last_flip_req = 0;
> return 0;
> @@ -450,7 +449,6 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
> ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
> if (ret)
> return ret;
> - i915_gem_retire_requests(dev);
>
> if (overlay->flip_tail)
> overlay->flip_tail(overlay);
> --
> 1.7.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 03/12 v2] drm/i915: move vbetool invoked ier stuff
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
2012-04-26 23:02 ` [PATCH 01/12 v2] drm/i915: remove do_retire from i915_wait_request Ben Widawsky
2012-04-26 23:02 ` [PATCH 02/12] drm/i915: remove some extra retiring Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-27 14:28 ` Daniel Vetter
2012-05-04 19:40 ` Jesse Barnes
2012-04-26 23:03 ` [PATCH 04/12 v2] drm/i915: kill waiting_seqno Ben Widawsky
` (8 subsequent siblings)
11 siblings, 2 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This extra bit of interrupt enabling code doesn't belong in the wait
seqno function. If anything we should pull it out to a helper so the
throttle code can also use it. The history is a bit vague, but I am
going to attempt to just dump it, unless someone can argue otherwise.
Removing this allows for a shared lock free wait seqno function. To keep
tabs on this issue though, the IER value is stored on error capture
(recommended by Chris Wilson)
v2: fixed typo EIR->IER (Ben)
Fix some white space (Ben)
Move IER capture to globally instead of per ring (Ben)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_debugfs.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 14 --------------
drivers/gpu/drm/i915/i915_irq.c | 8 ++++++++
4 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 120db46..8ed0342 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -712,6 +712,7 @@ static int i915_error_state(struct seq_file *m, void *unused)
error->time.tv_usec);
seq_printf(m, "PCI ID: 0x%04x\n", dev->pci_device);
seq_printf(m, "EIR: 0x%08x\n", error->eir);
+ seq_printf(m, "IER: 0x%08x\n", error->ier);
seq_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
for (i = 0; i < dev_priv->num_fence_regs; i++)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34d7041..d0eeb28 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -173,6 +173,7 @@ struct intel_display_error_state;
struct drm_i915_error_state {
u32 eir;
u32 pgtbl_er;
+ u32 ier;
u32 pipestat[I915_MAX_PIPES];
u32 tail[I915_NUM_RINGS];
u32 head[I915_NUM_RINGS];
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3b731ef..5938b9b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1828,7 +1828,6 @@ i915_wait_request(struct intel_ring_buffer *ring,
uint32_t seqno)
{
drm_i915_private_t *dev_priv = ring->dev->dev_private;
- u32 ier;
int ret = 0;
BUG_ON(seqno == 0);
@@ -1863,19 +1862,6 @@ i915_wait_request(struct intel_ring_buffer *ring,
}
if (!i915_seqno_passed(ring->get_seqno(ring), seqno)) {
- if (HAS_PCH_SPLIT(ring->dev))
- ier = I915_READ(DEIER) | I915_READ(GTIER);
- else if (IS_VALLEYVIEW(ring->dev))
- ier = I915_READ(GTIER) | I915_READ(VLV_IER);
- else
- ier = I915_READ(IER);
- if (!ier) {
- DRM_ERROR("something (likely vbetool) disabled "
- "interrupts, re-enabling\n");
- ring->dev->driver->irq_preinstall(ring->dev);
- ring->dev->driver->irq_postinstall(ring->dev);
- }
-
trace_i915_gem_request_wait_begin(ring, seqno);
ring->waiting_seqno = seqno;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0211263..5e9d05d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1155,6 +1155,14 @@ static void i915_capture_error_state(struct drm_device *dev)
error->eir = I915_READ(EIR);
error->pgtbl_er = I915_READ(PGTBL_ER);
+
+ if (HAS_PCH_SPLIT(dev))
+ error->ier = I915_READ(DEIER) | I915_READ(GTIER);
+ else if (IS_VALLEYVIEW(dev))
+ error->ier = I915_READ(GTIER) | I915_READ(VLV_IER);
+ else
+ error->ier = I915_READ(IER);
+
for_each_pipe(pipe)
error->pipestat[pipe] = I915_READ(PIPESTAT(pipe));
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 03/12 v2] drm/i915: move vbetool invoked ier stuff
2012-04-26 23:03 ` [PATCH 03/12 v2] drm/i915: move vbetool invoked ier stuff Ben Widawsky
@ 2012-04-27 14:28 ` Daniel Vetter
2012-05-04 19:40 ` Jesse Barnes
1 sibling, 0 replies; 32+ messages in thread
From: Daniel Vetter @ 2012-04-27 14:28 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, Apr 26, 2012 at 04:03:00PM -0700, Ben Widawsky wrote:
> This extra bit of interrupt enabling code doesn't belong in the wait
> seqno function. If anything we should pull it out to a helper so the
> throttle code can also use it. The history is a bit vague, but I am
> going to attempt to just dump it, unless someone can argue otherwise.
>
> Removing this allows for a shared lock free wait seqno function. To keep
> tabs on this issue though, the IER value is stored on error capture
> (recommended by Chris Wilson)
>
> v2: fixed typo EIR->IER (Ben)
> Fix some white space (Ben)
> Move IER capture to globally instead of per ring (Ben)
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Queued for -next with the small addition that ier is a 16bit reg on gen2,
thanks for the patch.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 03/12 v2] drm/i915: move vbetool invoked ier stuff
2012-04-26 23:03 ` [PATCH 03/12 v2] drm/i915: move vbetool invoked ier stuff Ben Widawsky
2012-04-27 14:28 ` Daniel Vetter
@ 2012-05-04 19:40 ` Jesse Barnes
1 sibling, 0 replies; 32+ messages in thread
From: Jesse Barnes @ 2012-05-04 19:40 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, 26 Apr 2012 16:03:00 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> This extra bit of interrupt enabling code doesn't belong in the wait
> seqno function. If anything we should pull it out to a helper so the
> throttle code can also use it. The history is a bit vague, but I am
> going to attempt to just dump it, unless someone can argue otherwise.
>
> Removing this allows for a shared lock free wait seqno function. To
> keep tabs on this issue though, the IER value is stored on error
> capture (recommended by Chris Wilson)
>
> v2: fixed typo EIR->IER (Ben)
> Fix some white space (Ben)
> Move IER capture to globally instead of per ring (Ben)
>
It may be safe to remove this now. Distros used to run vbetool at
random times, shutting things off and messing with hw. But these days
they use the driver with a bit less molestation, so the IER check/fix
shouldn't be needed.
Jesse
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 04/12 v2] drm/i915: kill waiting_seqno
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (2 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 03/12 v2] drm/i915: move vbetool invoked ier stuff Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-27 14:45 ` Daniel Vetter
2012-04-26 23:03 ` [PATCH 05/12 v2] drm/i915: drop polled waits from i915_wait_request Ben Widawsky
` (7 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
The waiting_seqno is not terribly useful, and as such we can remove it
so that we'll be able to extract lockless code.
v2: Keep the information for error_state (Chris)
Check if ring is initialized in hangcheck (Chris)
Capture the waiting ring (Chris)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_debugfs.c | 3 +--
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 2 --
drivers/gpu/drm/i915/i915_irq.c | 13 ++++++++-----
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8ed0342..9d4bf6b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -414,8 +414,6 @@ static void i915_ring_seqno_info(struct seq_file *m,
if (ring->get_seqno) {
seq_printf(m, "Current sequence (%s): %d\n",
ring->name, ring->get_seqno(ring));
- seq_printf(m, "Waiter sequence (%s): %d\n",
- ring->name, ring->waiting_seqno);
seq_printf(m, "IRQ sequence (%s): %d\n",
ring->name, ring->irq_seqno);
}
@@ -687,6 +685,7 @@ static void i915_ring_error_state(struct seq_file *m,
error->semaphore_mboxes[ring][1]);
}
seq_printf(m, " seqno: 0x%08x\n", error->seqno[ring]);
+ seq_printf(m, " waiting: %s\n", yesno(error->waiting[ring]));
seq_printf(m, " ring->head: 0x%08x\n", error->cpu_ring_head[ring]);
seq_printf(m, " ring->tail: 0x%08x\n", error->cpu_ring_tail[ring]);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d0eeb28..d260762 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -174,6 +174,7 @@ struct drm_i915_error_state {
u32 eir;
u32 pgtbl_er;
u32 ier;
+ bool waiting[I915_NUM_RINGS];
u32 pipestat[I915_MAX_PIPES];
u32 tail[I915_NUM_RINGS];
u32 head[I915_NUM_RINGS];
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5938b9b..c29c758 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1864,7 +1864,6 @@ i915_wait_request(struct intel_ring_buffer *ring,
if (!i915_seqno_passed(ring->get_seqno(ring), seqno)) {
trace_i915_gem_request_wait_begin(ring, seqno);
- ring->waiting_seqno = seqno;
if (ring->irq_get(ring)) {
if (dev_priv->mm.interruptible)
ret = wait_event_interruptible(ring->irq_queue,
@@ -1880,7 +1879,6 @@ i915_wait_request(struct intel_ring_buffer *ring,
seqno) ||
atomic_read(&dev_priv->mm.wedged), 3000))
ret = -EBUSY;
- ring->waiting_seqno = 0;
trace_i915_gem_request_wait_end(ring, seqno);
}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5e9d05d..98494ba 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1064,6 +1064,7 @@ static void i915_record_ring_state(struct drm_device *dev,
error->instdone[ring->id] = I915_READ(INSTDONE);
}
+ error->waiting[ring->id] = waitqueue_active(&ring->irq_queue);
error->instpm[ring->id] = I915_READ(RING_INSTPM(ring->mmio_base));
error->seqno[ring->id] = ring->get_seqno(ring);
error->acthd[ring->id] = intel_ring_get_active_head(ring);
@@ -1877,14 +1878,16 @@ ring_last_seqno(struct intel_ring_buffer *ring)
static bool i915_hangcheck_ring_idle(struct intel_ring_buffer *ring, bool *err)
{
+ /* Hanghceck may trigger before ring is initialized */
+ if (ring->obj == NULL)
+ return true;
+
if (list_empty(&ring->request_list) ||
i915_seqno_passed(ring->get_seqno(ring), ring_last_seqno(ring))) {
/* Issue a wake-up to catch stuck h/w. */
- if (ring->waiting_seqno && waitqueue_active(&ring->irq_queue)) {
- DRM_ERROR("Hangcheck timer elapsed... %s idle [waiting on %d, at %d], missed IRQ?\n",
- ring->name,
- ring->waiting_seqno,
- ring->get_seqno(ring));
+ if (waitqueue_active(&ring->irq_queue)) {
+ DRM_ERROR("Hangcheck timer elapsed... %s idle\n",
+ ring->name);
wake_up_all(&ring->irq_queue);
*err = true;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index e0b25bb..d3bf259 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -60,7 +60,6 @@ struct intel_ring_buffer {
u32 irq_enable_mask; /* bitmask to enable ring interrupt */
u32 irq_seqno; /* last seq seem at irq time */
u32 trace_irq_seqno;
- u32 waiting_seqno;
u32 sync_seqno[I915_NUM_RINGS-1];
bool __must_check (*irq_get)(struct intel_ring_buffer *ring);
void (*irq_put)(struct intel_ring_buffer *ring);
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 04/12 v2] drm/i915: kill waiting_seqno
2012-04-26 23:03 ` [PATCH 04/12 v2] drm/i915: kill waiting_seqno Ben Widawsky
@ 2012-04-27 14:45 ` Daniel Vetter
0 siblings, 0 replies; 32+ messages in thread
From: Daniel Vetter @ 2012-04-27 14:45 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, Apr 26, 2012 at 04:03:01PM -0700, Ben Widawsky wrote:
> The waiting_seqno is not terribly useful, and as such we can remove it
> so that we'll be able to extract lockless code.
>
> v2: Keep the information for error_state (Chris)
> Check if ring is initialized in hangcheck (Chris)
> Capture the waiting ring (Chris)
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Merged with the comment for the ring->obj == NULL check in hangcheck_idle
clarified, thanks for the patch.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 05/12 v2] drm/i915: drop polled waits from i915_wait_request
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (3 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 04/12 v2] drm/i915: kill waiting_seqno Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-27 14:46 ` Daniel Vetter
2012-04-26 23:03 ` [PATCH 06/12 v2] drm/i915: extract __wait_seqno " Ben Widawsky
` (6 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
The only time irq_get should fail is during unload or suspend. Both of
these points should try to quiesce the GPU before disabling interrupts
and so the atomic polling should never occur.
This was recommended by Chris Wilson as a way of reducing added
complexity to the polled wait which I introduced in an RFC patch.
09:57 < ickle_> it's only there as a fudge for waiting after irqs
after uninstalled during s&r, we aren't actually meant to hit it
09:57 < ickle_> so maybe we should just kill the code there and fix the breakage
v2: return -ENODEV instead of -EBUSY when irq_get fails
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c29c758..c5f0b03 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1864,22 +1864,19 @@ i915_wait_request(struct intel_ring_buffer *ring,
if (!i915_seqno_passed(ring->get_seqno(ring), seqno)) {
trace_i915_gem_request_wait_begin(ring, seqno);
- if (ring->irq_get(ring)) {
- if (dev_priv->mm.interruptible)
- ret = wait_event_interruptible(ring->irq_queue,
- i915_seqno_passed(ring->get_seqno(ring), seqno)
- || atomic_read(&dev_priv->mm.wedged));
- else
- wait_event(ring->irq_queue,
- i915_seqno_passed(ring->get_seqno(ring), seqno)
- || atomic_read(&dev_priv->mm.wedged));
+ if (WARN_ON(!ring->irq_get(ring)))
+ return -ENODEV;
- ring->irq_put(ring);
- } else if (wait_for_atomic(i915_seqno_passed(ring->get_seqno(ring),
- seqno) ||
- atomic_read(&dev_priv->mm.wedged), 3000))
- ret = -EBUSY;
+ if (dev_priv->mm.interruptible)
+ ret = wait_event_interruptible(ring->irq_queue,
+ i915_seqno_passed(ring->get_seqno(ring), seqno)
+ || atomic_read(&dev_priv->mm.wedged));
+ else
+ wait_event(ring->irq_queue,
+ i915_seqno_passed(ring->get_seqno(ring), seqno)
+ || atomic_read(&dev_priv->mm.wedged));
+ ring->irq_put(ring);
trace_i915_gem_request_wait_end(ring, seqno);
}
if (atomic_read(&dev_priv->mm.wedged))
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 05/12 v2] drm/i915: drop polled waits from i915_wait_request
2012-04-26 23:03 ` [PATCH 05/12 v2] drm/i915: drop polled waits from i915_wait_request Ben Widawsky
@ 2012-04-27 14:46 ` Daniel Vetter
0 siblings, 0 replies; 32+ messages in thread
From: Daniel Vetter @ 2012-04-27 14:46 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, Apr 26, 2012 at 04:03:02PM -0700, Ben Widawsky wrote:
> The only time irq_get should fail is during unload or suspend. Both of
> these points should try to quiesce the GPU before disabling interrupts
> and so the atomic polling should never occur.
>
> This was recommended by Chris Wilson as a way of reducing added
> complexity to the polled wait which I introduced in an RFC patch.
>
> 09:57 < ickle_> it's only there as a fudge for waiting after irqs
> after uninstalled during s&r, we aren't actually meant to hit it
> 09:57 < ickle_> so maybe we should just kill the code there and fix the breakage
>
> v2: return -ENODEV instead of -EBUSY when irq_get fails
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 06/12 v2] drm/i915: extract __wait_seqno from i915_wait_request
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (4 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 05/12 v2] drm/i915: drop polled waits from i915_wait_request Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-26 23:03 ` [PATCH 07/12] drm/i915: remove polled wait from throttle Ben Widawsky
` (5 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
i915_wait_request is actually a fairly large function encapsulating
quite a few different operations. Because being able to wait on seqnos
in various conditions is useful, extracting that bit of code to a helper
function seems useful
v2: pull the irq_get/put as well (Ben)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem.c | 49 +++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c5f0b03..06668eb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1819,6 +1819,36 @@ i915_gem_retire_work_handler(struct work_struct *work)
mutex_unlock(&dev->struct_mutex);
}
+static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
+ bool interruptible)
+{
+ drm_i915_private_t *dev_priv = ring->dev->dev_private;
+ int ret = 0;
+
+ if (i915_seqno_passed(ring->get_seqno(ring), seqno))
+ return 0;
+
+ trace_i915_gem_request_wait_begin(ring, seqno);
+ if (WARN_ON(!ring->irq_get(ring)))
+ return -ENODEV;
+
+#define EXIT_COND \
+ (i915_seqno_passed(ring->get_seqno(ring), seqno) || \
+ atomic_read(&dev_priv->mm.wedged))
+
+ if (interruptible)
+ ret = wait_event_interruptible(ring->irq_queue,
+ EXIT_COND);
+ else
+ wait_event(ring->irq_queue, EXIT_COND);
+
+ ring->irq_put(ring);
+ trace_i915_gem_request_wait_end(ring, seqno);
+#undef EXIT_COND
+
+ return ret;
+}
+
/**
* Waits for a sequence number to be signaled, and cleans up the
* request and object lists appropriately for that event.
@@ -1861,24 +1891,7 @@ i915_wait_request(struct intel_ring_buffer *ring,
seqno = request->seqno;
}
- if (!i915_seqno_passed(ring->get_seqno(ring), seqno)) {
- trace_i915_gem_request_wait_begin(ring, seqno);
-
- if (WARN_ON(!ring->irq_get(ring)))
- return -ENODEV;
-
- if (dev_priv->mm.interruptible)
- ret = wait_event_interruptible(ring->irq_queue,
- i915_seqno_passed(ring->get_seqno(ring), seqno)
- || atomic_read(&dev_priv->mm.wedged));
- else
- wait_event(ring->irq_queue,
- i915_seqno_passed(ring->get_seqno(ring), seqno)
- || atomic_read(&dev_priv->mm.wedged));
-
- ring->irq_put(ring);
- trace_i915_gem_request_wait_end(ring, seqno);
- }
+ ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible);
if (atomic_read(&dev_priv->mm.wedged))
ret = -EAGAIN;
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 07/12] drm/i915: remove polled wait from throttle
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (5 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 06/12 v2] drm/i915: extract __wait_seqno " Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-26 23:03 ` [PATCH 08/12 v2] drm/i915: use __wait_seqno for ring throttle Ben Widawsky
` (4 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
It's about to go away anyway. Just here to help bisection.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 06668eb..4de0515 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2996,11 +2996,8 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
if (ret == 0 && atomic_read(&dev_priv->mm.wedged))
ret = -EIO;
- } else if (wait_for_atomic(i915_seqno_passed(ring->get_seqno(ring),
- seqno) ||
- atomic_read(&dev_priv->mm.wedged), 3000)) {
+ } else
ret = -EBUSY;
- }
}
if (ret == 0)
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 08/12 v2] drm/i915: use __wait_seqno for ring throttle
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (6 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 07/12] drm/i915: remove polled wait from throttle Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-27 14:50 ` Daniel Vetter
2012-04-26 23:03 ` [PATCH 09/12] drm/i915: timeout parameter for seqno wait Ben Widawsky
` (3 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
It turns out throttle had an almost identical bit of code to do the
wait. Now we can call the new helper directly. This is just a bonus,
and not needed for the overall series.
v2: remove irq_get/put which is now in __wait_seqno (Ben)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4de0515..0ae1a73 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2981,25 +2981,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
if (seqno == 0)
return 0;
- ret = 0;
- if (!i915_seqno_passed(ring->get_seqno(ring), seqno)) {
- /* And wait for the seqno passing without holding any locks and
- * causing extra latency for others. This is safe as the irq
- * generation is designed to be run atomically and so is
- * lockless.
- */
- if (ring->irq_get(ring)) {
- ret = wait_event_interruptible(ring->irq_queue,
- i915_seqno_passed(ring->get_seqno(ring), seqno)
- || atomic_read(&dev_priv->mm.wedged));
- ring->irq_put(ring);
-
- if (ret == 0 && atomic_read(&dev_priv->mm.wedged))
- ret = -EIO;
- } else
- ret = -EBUSY;
- }
-
+ ret = __wait_seqno(ring, seqno, true);
if (ret == 0)
queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 08/12 v2] drm/i915: use __wait_seqno for ring throttle
2012-04-26 23:03 ` [PATCH 08/12 v2] drm/i915: use __wait_seqno for ring throttle Ben Widawsky
@ 2012-04-27 14:50 ` Daniel Vetter
0 siblings, 0 replies; 32+ messages in thread
From: Daniel Vetter @ 2012-04-27 14:50 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, Apr 26, 2012 at 04:03:05PM -0700, Ben Widawsky wrote:
> It turns out throttle had an almost identical bit of code to do the
> wait. Now we can call the new helper directly. This is just a bonus,
> and not needed for the overall series.
>
> v2: remove irq_get/put which is now in __wait_seqno (Ben)
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Queued for -next up to this patch, thanks for the patches.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 09/12] drm/i915: timeout parameter for seqno wait
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (7 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 08/12 v2] drm/i915: use __wait_seqno for ring throttle Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-27 14:14 ` Chris Wilson
2012-04-27 15:00 ` Daniel Vetter
2012-04-26 23:03 ` [PATCH 10/12] drm/i915: extract some common olr+wedge code Ben Widawsky
` (2 subsequent siblings)
11 siblings, 2 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Insert a wait parameter in the code so we can possibly timeout on a
seqno wait if need be. The code should be functionally the same as
before because all the callers will continue to retry if an arbitrary
timeout elapses.
We'd like to have nanosecond granularity, but the only way to do this is
with hrtimer, and that doesn't fit well with the needs of this code.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem.c | 58 +++++++++++++++++++++++++++++++--------
1 file changed, 46 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0ae1a73..f054439 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1820,10 +1820,23 @@ i915_gem_retire_work_handler(struct work_struct *work)
}
static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
- bool interruptible)
+ bool interruptible, long *usecs)
{
drm_i915_private_t *dev_priv = ring->dev->dev_private;
- int ret = 0;
+ bool wait_forever = false;
+ long timeout, end;
+
+ if (usecs == NULL || ((*usecs) < 0)) {
+ wait_forever = true;
+ timeout = msecs_to_jiffies(100);
+ } else
+ timeout = usecs_to_jiffies(*usecs);
+
+ if (i915_seqno_passed(ring->get_seqno(ring), seqno))
+ return 0;
+
+ if (WARN_ON(!ring->irq_get(ring)))
+ return -ENODEV;
if (i915_seqno_passed(ring->get_seqno(ring), seqno))
return 0;
@@ -1836,17 +1849,40 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
(i915_seqno_passed(ring->get_seqno(ring), seqno) || \
atomic_read(&dev_priv->mm.wedged))
+ trace_i915_gem_request_wait_begin(ring, seqno);
+again:
if (interruptible)
- ret = wait_event_interruptible(ring->irq_queue,
- EXIT_COND);
+ end = wait_event_interruptible_timeout(ring->irq_queue,
+ EXIT_COND,
+ timeout);
else
- wait_event(ring->irq_queue, EXIT_COND);
+ end = wait_event_timeout(ring->irq_queue, EXIT_COND, timeout);
+#undef EXIT_COND
+
+ if (atomic_read(&dev_priv->mm.wedged))
+ end = -EAGAIN;
+
+ if (end == 0 && wait_forever)
+ goto again;
- ring->irq_put(ring);
trace_i915_gem_request_wait_end(ring, seqno);
-#undef EXIT_COND
+ ring->irq_put(ring);
- return ret;
+ if (!wait_forever) {
+ BUG_ON(end == 0);
+ *usecs = jiffies_to_usecs(timeout - end);
+ }
+
+ switch (end) {
+ case -EAGAIN: /* Wedged */
+ case -ERESTARTSYS: /* Signal */
+ return (int)end;
+ case 0: /* Tiemout */
+ return -ETIME;
+ default: /* Completed */
+ WARN_ON(end < 0); /* We're not aware of other errors */
+ return 0;
+ }
}
/**
@@ -1891,9 +1927,7 @@ i915_wait_request(struct intel_ring_buffer *ring,
seqno = request->seqno;
}
- ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible);
- if (atomic_read(&dev_priv->mm.wedged))
- ret = -EAGAIN;
+ ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible, NULL);
return ret;
}
@@ -2981,7 +3015,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
if (seqno == 0)
return 0;
- ret = __wait_seqno(ring, seqno, true);
+ ret = __wait_seqno(ring, seqno, true, NULL);
if (ret == 0)
queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 09/12] drm/i915: timeout parameter for seqno wait
2012-04-26 23:03 ` [PATCH 09/12] drm/i915: timeout parameter for seqno wait Ben Widawsky
@ 2012-04-27 14:14 ` Chris Wilson
2012-04-28 19:34 ` Ben Widawsky
2012-04-27 15:00 ` Daniel Vetter
1 sibling, 1 reply; 32+ messages in thread
From: Chris Wilson @ 2012-04-27 14:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Thu, 26 Apr 2012 16:03:06 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> - bool interruptible)
> + bool interruptible, long *usecs)
> {
> drm_i915_private_t *dev_priv = ring->dev->dev_private;
> - int ret = 0;
> + bool wait_forever = false;
> + long timeout, end;
> +
> + if (usecs == NULL || ((*usecs) < 0)) {
> + wait_forever = true;
> + timeout = msecs_to_jiffies(100);
> + } else
> + timeout = usecs_to_jiffies(*usecs);
> +
> + if (i915_seqno_passed(ring->get_seqno(ring), seqno))
> + return 0;
> +
> + if (WARN_ON(!ring->irq_get(ring)))
> + return -ENODEV;
Rebase error, duplicated ring->irq_get/ring->get_seqno.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 09/12] drm/i915: timeout parameter for seqno wait
2012-04-27 14:14 ` Chris Wilson
@ 2012-04-28 19:34 ` Ben Widawsky
0 siblings, 0 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-28 19:34 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, 27 Apr 2012 15:14:10 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, 26 Apr 2012 16:03:06 -0700, Ben Widawsky <ben@bwidawsk.net>
> wrote:
> > static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> > - bool interruptible)
> > + bool interruptible, long *usecs)
> > {
> > drm_i915_private_t *dev_priv = ring->dev->dev_private;
> > - int ret = 0;
> > + bool wait_forever = false;
> > + long timeout, end;
> > +
> > + if (usecs == NULL || ((*usecs) < 0)) {
> > + wait_forever = true;
> > + timeout = msecs_to_jiffies(100);
> > + } else
> > + timeout = usecs_to_jiffies(*usecs);
> > +
> > + if (i915_seqno_passed(ring->get_seqno(ring), seqno))
> > + return 0;
> > +
> > + if (WARN_ON(!ring->irq_get(ring)))
> > + return -ENODEV;
>
> Rebase error, duplicated ring->irq_get/ring->get_seqno.
> -Chris
>
Got it, thanks.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 09/12] drm/i915: timeout parameter for seqno wait
2012-04-26 23:03 ` [PATCH 09/12] drm/i915: timeout parameter for seqno wait Ben Widawsky
2012-04-27 14:14 ` Chris Wilson
@ 2012-04-27 15:00 ` Daniel Vetter
2012-04-28 4:28 ` Ben Widawsky
1 sibling, 1 reply; 32+ messages in thread
From: Daniel Vetter @ 2012-04-27 15:00 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, Apr 26, 2012 at 04:03:06PM -0700, Ben Widawsky wrote:
> Insert a wait parameter in the code so we can possibly timeout on a
> seqno wait if need be. The code should be functionally the same as
> before because all the callers will continue to retry if an arbitrary
> timeout elapses.
>
> We'd like to have nanosecond granularity, but the only way to do this is
> with hrtimer, and that doesn't fit well with the needs of this code.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
I have to admit, I'm a bit unhappy with this swiss-army-tool wait_seqno
and what it looks like. What about copy&pasting __wait_seqno_timeout,
which is always interruptible?
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 58 +++++++++++++++++++++++++++++++--------
> 1 file changed, 46 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0ae1a73..f054439 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1820,10 +1820,23 @@ i915_gem_retire_work_handler(struct work_struct *work)
> }
>
> static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> - bool interruptible)
> + bool interruptible, long *usecs)
> {
> drm_i915_private_t *dev_priv = ring->dev->dev_private;
> - int ret = 0;
> + bool wait_forever = false;
> + long timeout, end;
> +
> + if (usecs == NULL || ((*usecs) < 0)) {
> + wait_forever = true;
> + timeout = msecs_to_jiffies(100);
> + } else
> + timeout = usecs_to_jiffies(*usecs);
> +
> + if (i915_seqno_passed(ring->get_seqno(ring), seqno))
> + return 0;
> +
> + if (WARN_ON(!ring->irq_get(ring)))
> + return -ENODEV;
>
> if (i915_seqno_passed(ring->get_seqno(ring), seqno))
> return 0;
> @@ -1836,17 +1849,40 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> (i915_seqno_passed(ring->get_seqno(ring), seqno) || \
> atomic_read(&dev_priv->mm.wedged))
>
> + trace_i915_gem_request_wait_begin(ring, seqno);
> +again:
> if (interruptible)
> - ret = wait_event_interruptible(ring->irq_queue,
> - EXIT_COND);
> + end = wait_event_interruptible_timeout(ring->irq_queue,
> + EXIT_COND,
> + timeout);
> else
> - wait_event(ring->irq_queue, EXIT_COND);
> + end = wait_event_timeout(ring->irq_queue, EXIT_COND, timeout);
> +#undef EXIT_COND
> +
> + if (atomic_read(&dev_priv->mm.wedged))
> + end = -EAGAIN;
> +
> + if (end == 0 && wait_forever)
> + goto again;
>
> - ring->irq_put(ring);
> trace_i915_gem_request_wait_end(ring, seqno);
> -#undef EXIT_COND
> + ring->irq_put(ring);
>
> - return ret;
> + if (!wait_forever) {
> + BUG_ON(end == 0);
> + *usecs = jiffies_to_usecs(timeout - end);
> + }
> +
> + switch (end) {
> + case -EAGAIN: /* Wedged */
> + case -ERESTARTSYS: /* Signal */
> + return (int)end;
> + case 0: /* Tiemout */
> + return -ETIME;
> + default: /* Completed */
> + WARN_ON(end < 0); /* We're not aware of other errors */
> + return 0;
> + }
> }
>
> /**
> @@ -1891,9 +1927,7 @@ i915_wait_request(struct intel_ring_buffer *ring,
> seqno = request->seqno;
> }
>
> - ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible);
> - if (atomic_read(&dev_priv->mm.wedged))
> - ret = -EAGAIN;
> + ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible, NULL);
>
> return ret;
> }
> @@ -2981,7 +3015,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
> if (seqno == 0)
> return 0;
>
> - ret = __wait_seqno(ring, seqno, true);
> + ret = __wait_seqno(ring, seqno, true, NULL);
> if (ret == 0)
> queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
>
> --
> 1.7.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 09/12] drm/i915: timeout parameter for seqno wait
2012-04-27 15:00 ` Daniel Vetter
@ 2012-04-28 4:28 ` Ben Widawsky
0 siblings, 0 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-28 4:28 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Fri, 27 Apr 2012 17:00:20 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Apr 26, 2012 at 04:03:06PM -0700, Ben Widawsky wrote:
> > Insert a wait parameter in the code so we can possibly timeout on a
> > seqno wait if need be. The code should be functionally the same as
> > before because all the callers will continue to retry if an
> > arbitrary timeout elapses.
> >
> > We'd like to have nanosecond granularity, but the only way to do
> > this is with hrtimer, and that doesn't fit well with the needs of
> > this code.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>
> I have to admit, I'm a bit unhappy with this swiss-army-tool
> wait_seqno and what it looks like. What about copy&pasting
> __wait_seqno_timeout, which is always interruptible?
> -Daniel
I'm going to put the onus on you for this bikeshed to test it, and see
how you like it. I have tried have the other function, and I felt this
one looked better. Though I assume I have to fix the rebase error that
Chris pointed out anyway. So I'll do a v3 with the separate function
*after* you confirm you really want it. Otherwise, you'll just get the
rebase fail fix.
Ben
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 10/12] drm/i915: extract some common olr+wedge code
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (8 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 09/12] drm/i915: timeout parameter for seqno wait Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-27 8:46 ` Chris Wilson
2012-04-26 23:03 ` [PATCH 11/12 v2] drm/i915: wait render timeout ioctl Ben Widawsky
2012-04-26 23:03 ` [PATCH 12/12] drm/i915: s/i915_wait_reqest/i915_wait_seqno/g Ben Widawsky
11 siblings, 1 reply; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Refactor.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem.c | 122 +++++++++++++++++++++------------------
1 file changed, 65 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f054439..63eda79 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1819,6 +1819,59 @@ i915_gem_retire_work_handler(struct work_struct *work)
mutex_unlock(&dev->struct_mutex);
}
+
+static int
+i915_gem_check_wedge(struct drm_i915_private *dev_priv)
+{
+ BUG_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+
+ if (atomic_read(&dev_priv->mm.wedged)) {
+ struct completion *x = &dev_priv->error_completion;
+ bool recovery_complete;
+ unsigned long flags;
+
+ /* Give the error handler a chance to run. */
+ spin_lock_irqsave(&x->wait.lock, flags);
+ recovery_complete = x->done > 0;
+ spin_unlock_irqrestore(&x->wait.lock, flags);
+
+ return recovery_complete ? -EIO : -EAGAIN;
+ }
+
+ return 0;
+}
+
+/*
+ * Compare seqno against outstanding lazy request. Emit a request if they are
+ * equal. Seqno is updated with the new value if a request was emitted.
+ */
+static int
+i915_gem_check_olr(struct intel_ring_buffer *ring, u32 *seqno)
+{
+ int ret = 0;
+
+ BUG_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+
+ if (*seqno == ring->outstanding_lazy_request) {
+ struct drm_i915_gem_request *request;
+
+ request = kzalloc(sizeof(*request), GFP_KERNEL);
+ if (request == NULL)
+ return -ENOMEM;
+
+ ret = i915_add_request(ring, NULL, request);
+ if (ret) {
+ kfree(request);
+ return ret;
+ }
+
+ *seqno = request->seqno;
+ }
+
+ return ret;
+}
+
+
static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
bool interruptible, long *usecs)
{
@@ -1898,34 +1951,13 @@ i915_wait_request(struct intel_ring_buffer *ring,
BUG_ON(seqno == 0);
- if (atomic_read(&dev_priv->mm.wedged)) {
- struct completion *x = &dev_priv->error_completion;
- bool recovery_complete;
- unsigned long flags;
-
- /* Give the error handler a chance to run. */
- spin_lock_irqsave(&x->wait.lock, flags);
- recovery_complete = x->done > 0;
- spin_unlock_irqrestore(&x->wait.lock, flags);
-
- return recovery_complete ? -EIO : -EAGAIN;
- }
-
- if (seqno == ring->outstanding_lazy_request) {
- struct drm_i915_gem_request *request;
-
- request = kzalloc(sizeof(*request), GFP_KERNEL);
- if (request == NULL)
- return -ENOMEM;
-
- ret = i915_add_request(ring, NULL, request);
- if (ret) {
- kfree(request);
- return ret;
- }
+ ret = i915_gem_check_wedge(dev_priv);
+ if (ret)
+ return ret;
- seqno = request->seqno;
- }
+ ret = i915_gem_check_olr(ring, &seqno);
+ if (ret)
+ return ret;
ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible, NULL);
@@ -1991,22 +2023,9 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
if (seqno <= from->sync_seqno[idx])
return 0;
- if (seqno == from->outstanding_lazy_request) {
- struct drm_i915_gem_request *request;
-
- request = kzalloc(sizeof(*request), GFP_KERNEL);
- if (request == NULL)
- return -ENOMEM;
-
- ret = i915_add_request(from, NULL, request);
- if (ret) {
- kfree(request);
- return ret;
- }
-
- seqno = request->seqno;
- }
-
+ ret = i915_gem_check_olr(obj->ring, &seqno);
+ if (ret)
+ return ret;
ret = to->sync_to(to, from, seqno);
if (!ret)
@@ -3194,20 +3213,9 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (obj->base.write_domain & I915_GEM_GPU_DOMAINS) {
ret = i915_gem_flush_ring(obj->ring,
0, obj->base.write_domain);
- } else if (obj->ring->outstanding_lazy_request ==
- obj->last_rendering_seqno) {
- struct drm_i915_gem_request *request;
-
- /* This ring is not being cleared by active usage,
- * so emit a request to do so.
- */
- request = kzalloc(sizeof(*request), GFP_KERNEL);
- if (request) {
- ret = i915_add_request(obj->ring, NULL, request);
- if (ret)
- kfree(request);
- } else
- ret = -ENOMEM;
+ } else {
+ u32 seqno = obj->last_rendering_seqno;
+ ret = i915_gem_check_olr(obj->ring, &seqno);
}
/* Update the active list for the hardware's current position.
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 10/12] drm/i915: extract some common olr+wedge code
2012-04-26 23:03 ` [PATCH 10/12] drm/i915: extract some common olr+wedge code Ben Widawsky
@ 2012-04-27 8:46 ` Chris Wilson
2012-04-28 22:06 ` Ben Widawsky
0 siblings, 1 reply; 32+ messages in thread
From: Chris Wilson @ 2012-04-27 8:46 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Thu, 26 Apr 2012 16:03:07 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> +/*
> + * Compare seqno against outstanding lazy request. Emit a request if they are
> + * equal. Seqno is updated with the new value if a request was emitted.
> + */
> +static int
> +i915_gem_check_olr(struct intel_ring_buffer *ring, u32 *seqno)
> +{
> + int ret = 0;
> +
> + BUG_ON(!mutex_is_locked(&ring->dev->struct_mutex));
> +
> + if (*seqno == ring->outstanding_lazy_request) {
> + struct drm_i915_gem_request *request;
> +
> + request = kzalloc(sizeof(*request), GFP_KERNEL);
> + if (request == NULL)
> + return -ENOMEM;
> +
> + ret = i915_add_request(ring, NULL, request);
> + if (ret) {
> + kfree(request);
> + return ret;
> + }
> +
> + *seqno = request->seqno;
I'd love for this to be BUG_ON(seqno != request->seqno) and so drop the
out parameter, as it would tidy all the callers up.
> + }
> +
> + return ret;
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 10/12] drm/i915: extract some common olr+wedge code
2012-04-27 8:46 ` Chris Wilson
@ 2012-04-28 22:06 ` Ben Widawsky
0 siblings, 0 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-28 22:06 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, 27 Apr 2012 09:46:31 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, 26 Apr 2012 16:03:07 -0700, Ben Widawsky <ben@bwidawsk.net>
> wrote:
> > +/*
> > + * Compare seqno against outstanding lazy request. Emit a request
> > if they are
> > + * equal. Seqno is updated with the new value if a request was
> > emitted.
> > + */
> > +static int
> > +i915_gem_check_olr(struct intel_ring_buffer *ring, u32 *seqno)
> > +{
> > + int ret = 0;
> > +
> > + BUG_ON(!mutex_is_locked(&ring->dev->struct_mutex));
> > +
> > + if (*seqno == ring->outstanding_lazy_request) {
> > + struct drm_i915_gem_request *request;
> > +
> > + request = kzalloc(sizeof(*request), GFP_KERNEL);
> > + if (request == NULL)
> > + return -ENOMEM;
> > +
> > + ret = i915_add_request(ring, NULL, request);
> > + if (ret) {
> > + kfree(request);
> > + return ret;
> > + }
> > +
> > + *seqno = request->seqno;
> I'd love for this to be BUG_ON(seqno != request->seqno) and so drop
> the out parameter, as it would tidy all the callers up.
> > + }
> > +
> > + return ret;
> -Chris
>
Got it, thanks.
I hesitated to do this initially because of the logic in
i915_gem_object_sync. In that function, the updated seqno is used by the
ring sync code, and without the outparam, there was no way to not modify
the behavior. However after inspecting the code a bit deeper, I
understand why it's not a functional change, and the BUG_ON asserts
that.
The one bit which is still not clear to me, even after IRC, is why we
need to add the olr request at this point at all. Even if the mbox update
is delayed for a bit, I don't understand why things won't work
correctly; though I've confirmed I do get a hangcheck if I remove the
bit of code.
Comments?
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 11/12 v2] drm/i915: wait render timeout ioctl
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (9 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 10/12] drm/i915: extract some common olr+wedge code Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
2012-04-27 8:44 ` Chris Wilson
2012-04-27 15:24 ` Chris Wilson
2012-04-26 23:03 ` [PATCH 12/12] drm/i915: s/i915_wait_reqest/i915_wait_seqno/g Ben Widawsky
11 siblings, 2 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This helps implement glClientWaitSync.
Finally we can use the new timed seqno waiting function to allow
userspace to wait on a request with a timeout. This implements that
interface.
The new ioctl is very straight forward, there is a flags field which I
envision may be useful for various flush permutations of the command.
On 32bit systems, the timeout is restricted to 2^32 - 1 microseconds.
v2: ETIME/ERESTARTSYS instead of changing to EBUSY, and EGAIN (Chris)
Flush the object from the gpu write domain (Chris + Daniel)
Fix leaked refcount in good case (Chris)
Naturally align ioctl struct (Chris)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_dma.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem.c | 55 +++++++++++++++++++++++++++++++++++++++
include/drm/i915_drm.h | 8 ++++++
4 files changed, 66 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e73389d..ed0ccb7 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2189,6 +2189,7 @@ struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, intel_sprite_get_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_UNLOCKED),
};
int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d260762..d415f0f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1221,6 +1221,8 @@ int i915_gem_get_tiling(struct drm_device *dev, void *data,
struct drm_file *file_priv);
int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
+int i915_gem_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
void i915_gem_load(struct drm_device *dev);
int i915_gem_init_object(struct drm_gem_object *obj);
int __must_check i915_gem_flush_ring(struct intel_ring_buffer *ring,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 63eda79..0dee6bc 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1991,6 +1991,61 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj)
return 0;
}
+int
+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 intel_ring_buffer *ring = NULL;
+ long timeout;
+ u32 seqno = 0;
+ int ret = 0;
+
+ /* For now timeout is microseconds only */
+ timeout = (long)DIV_ROUND_UP_ULL(args->timeout_ns, NSEC_PER_USEC);
+
+ ret = i915_mutex_lock_interruptible(dev);
+ if (ret)
+ return ret;
+
+ obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->bo_handle));
+ if (&obj->base == NULL) {
+ mutex_unlock(&dev->struct_mutex);
+ return -ENOENT;
+ }
+
+ /* Need to make sure the object is flushed first. This non-obvious
+ * flush is required to enforce that (active && !olr) == no wait
+ * necessary.
+ */
+ ret = i915_gem_object_flush_gpu_write_domain(obj);
+ if (ret)
+ goto out;
+
+ if (obj->active) {
+ seqno = obj->last_rendering_seqno;
+ ring = obj->ring;
+ }
+
+ if (seqno == 0)
+ goto out;
+
+ ret = i915_gem_check_olr(ring, &seqno);
+ if (ret)
+ goto out;
+
+ mutex_unlock(&dev->struct_mutex);
+ ret = __wait_seqno(ring, seqno, true, &timeout);
+ drm_gem_object_unreference_unlocked(&obj->base);
+ args->timeout_ns = timeout * NSEC_PER_USEC;
+ return ret;
+
+out:
+ drm_gem_object_unreference(&obj->base);
+ mutex_unlock(&dev->struct_mutex);
+ return ret;
+}
+
/**
* i915_gem_object_sync - sync an object to a ring.
*
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index f3f8224..8f818f8 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -200,6 +200,7 @@ typedef struct _drm_i915_sarea {
#define DRM_I915_GEM_EXECBUFFER2 0x29
#define DRM_I915_GET_SPRITE_COLORKEY 0x2a
#define DRM_I915_SET_SPRITE_COLORKEY 0x2b
+#define DRM_I915_GEM_WAIT 0x2c
#define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
#define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
@@ -243,6 +244,7 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_OVERLAY_ATTRS DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs)
#define DRM_IOCTL_I915_SET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
#define DRM_IOCTL_I915_GET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
+#define DRM_IOCTL_I915_GEM_WAIT DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_WAIT, struct drm_i915_gem_wait)
/* Allow drivers to submit batchbuffers directly to hardware, relying
* on the security mechanisms provided by hardware.
@@ -886,4 +888,10 @@ struct drm_intel_sprite_colorkey {
__u32 flags;
};
+struct drm_i915_gem_wait {
+ __u32 bo_handle;
+ __u32 flags;
+ __u64 timeout_ns;
+};
+
#endif /* _I915_DRM_H_ */
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 11/12 v2] drm/i915: wait render timeout ioctl
2012-04-26 23:03 ` [PATCH 11/12 v2] drm/i915: wait render timeout ioctl Ben Widawsky
@ 2012-04-27 8:44 ` Chris Wilson
2012-04-28 22:18 ` Ben Widawsky
2012-04-27 15:24 ` Chris Wilson
1 sibling, 1 reply; 32+ messages in thread
From: Chris Wilson @ 2012-04-27 8:44 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Thu, 26 Apr 2012 16:03:08 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> + mutex_unlock(&dev->struct_mutex);
> + ret = __wait_seqno(ring, seqno, true, &timeout);
> + drm_gem_object_unreference_unlocked(&obj->base);
Once we have the seqno to wait on, we can drop the reference to the
object. The reference to the ring will be persist whilst the device is
open. Just saves doing an unsightly unlock/lock dance in the unref.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 11/12 v2] drm/i915: wait render timeout ioctl
2012-04-27 8:44 ` Chris Wilson
@ 2012-04-28 22:18 ` Ben Widawsky
0 siblings, 0 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-28 22:18 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, 27 Apr 2012 09:44:55 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, 26 Apr 2012 16:03:08 -0700, Ben Widawsky <ben@bwidawsk.net>
> wrote:
> > + mutex_unlock(&dev->struct_mutex);
> > + ret = __wait_seqno(ring, seqno, true, &timeout);
> > + drm_gem_object_unreference_unlocked(&obj->base);
>
> Once we have the seqno to wait on, we can drop the reference to the
> object. The reference to the ring will be persist whilst the device is
> open. Just saves doing an unsightly unlock/lock dance in the unref.
> -Chris
>
Got it, thanks.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 11/12 v2] drm/i915: wait render timeout ioctl
2012-04-26 23:03 ` [PATCH 11/12 v2] drm/i915: wait render timeout ioctl Ben Widawsky
2012-04-27 8:44 ` Chris Wilson
@ 2012-04-27 15:24 ` Chris Wilson
2012-04-29 2:11 ` Ben Widawsky
1 sibling, 1 reply; 32+ messages in thread
From: Chris Wilson @ 2012-04-27 15:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Thu, 26 Apr 2012 16:03:08 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> This helps implement glClientWaitSync.
>
> Finally we can use the new timed seqno waiting function to allow
> userspace to wait on a request with a timeout. This implements that
> interface.
>
> The new ioctl is very straight forward, there is a flags field which I
> envision may be useful for various flush permutations of the command.
What are the semantics of the ioctl? A simple use case would help
specify the interface here.
In particular, I can't tell whether the return value (timeout_ns) is
meant to be the time elapsed or the time remaining. What value is
returned in the timeout if we are interrupted before the wait completes?
Would
end = gettimeofday() + timeout;
do {
ret = i915_gem_wait(handle, 0, &timeout);
} while (ret == -1 && errno == EINTR);
assert(gettimeofday() <= end);
wait forever, or until the original timeout expires?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 11/12 v2] drm/i915: wait render timeout ioctl
2012-04-27 15:24 ` Chris Wilson
@ 2012-04-29 2:11 ` Ben Widawsky
2012-04-29 2:33 ` Ben Widawsky
2012-04-29 9:27 ` Chris Wilson
0 siblings, 2 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-29 2:11 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, 27 Apr 2012 16:24:24 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, 26 Apr 2012 16:03:08 -0700, Ben Widawsky <ben@bwidawsk.net>
> wrote:
> > This helps implement glClientWaitSync.
> >
> > Finally we can use the new timed seqno waiting function to allow
> > userspace to wait on a request with a timeout. This implements that
> > interface.
> >
> > The new ioctl is very straight forward, there is a flags field
> > which I envision may be useful for various flush permutations of
> > the command.
>
> What are the semantics of the ioctl? A simple use case would help
> specify the interface here.
The first time I sent out the series, I included the libdrm and igt
test. Is this what you're looking for, or something in the commit
message?
>
> In particular, I can't tell whether the return value (timeout_ns) is
> meant to be the time elapsed or the time remaining. What value is
> returned in the timeout if we are interrupted before the wait
> completes? Would
> end = gettimeofday() + timeout;
> do {
> ret = i915_gem_wait(handle, 0, &timeout);
> } while (ret == -1 && errno == EINTR);
> assert(gettimeofday() <= end);
> wait forever, or until the original timeout expires?
> -Chris
>
The return value should give the remaining time left if the return value
was not 0, though I venture to guess you're Socratically trying to tell
me something, so I'll go through this again and find the bug.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 11/12 v2] drm/i915: wait render timeout ioctl
2012-04-29 2:11 ` Ben Widawsky
@ 2012-04-29 2:33 ` Ben Widawsky
2012-04-29 9:27 ` Chris Wilson
1 sibling, 0 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-29 2:33 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Sat, 28 Apr 2012 19:11:34 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> On Fri, 27 Apr 2012 16:24:24 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> > On Thu, 26 Apr 2012 16:03:08 -0700, Ben Widawsky <ben@bwidawsk.net>
> > wrote:
> > > This helps implement glClientWaitSync.
> > >
> > > Finally we can use the new timed seqno waiting function to allow
> > > userspace to wait on a request with a timeout. This implements
> > > that interface.
> > >
> > > The new ioctl is very straight forward, there is a flags field
> > > which I envision may be useful for various flush permutations of
> > > the command.
> >
> > What are the semantics of the ioctl? A simple use case would help
> > specify the interface here.
>
> The first time I sent out the series, I included the libdrm and igt
> test. Is this what you're looking for, or something in the commit
> message?
>
> >
> > In particular, I can't tell whether the return value (timeout_ns) is
> > meant to be the time elapsed or the time remaining. What value is
> > returned in the timeout if we are interrupted before the wait
> > completes? Would
> > end = gettimeofday() + timeout;
> > do {
> > ret = i915_gem_wait(handle, 0, &timeout);
> > } while (ret == -1 && errno == EINTR);
> > assert(gettimeofday() <= end);
> > wait forever, or until the original timeout expires?
> > -Chris
> >
>
> The return value should give the remaining time left if the return
> value was not 0, though I venture to guess you're Socratically trying
> to tell me something, so I'll go through this again and find the bug.
Ah, I think I see the problem...
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 11/12 v2] drm/i915: wait render timeout ioctl
2012-04-29 2:11 ` Ben Widawsky
2012-04-29 2:33 ` Ben Widawsky
@ 2012-04-29 9:27 ` Chris Wilson
1 sibling, 0 replies; 32+ messages in thread
From: Chris Wilson @ 2012-04-29 9:27 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Sat, 28 Apr 2012 19:11:34 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> On Fri, 27 Apr 2012 16:24:24 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> > On Thu, 26 Apr 2012 16:03:08 -0700, Ben Widawsky <ben@bwidawsk.net>
> > wrote:
> > > This helps implement glClientWaitSync.
> > >
> > > Finally we can use the new timed seqno waiting function to allow
> > > userspace to wait on a request with a timeout. This implements that
> > > interface.
> > >
> > > The new ioctl is very straight forward, there is a flags field
> > > which I envision may be useful for various flush permutations of
> > > the command.
> >
> > What are the semantics of the ioctl? A simple use case would help
> > specify the interface here.
>
> The first time I sent out the series, I included the libdrm and igt
> test. Is this what you're looking for, or something in the commit
> message?
Whilst we will be bound by the API as used forever and a day, I found I
could not review the code against the interface for any aberrant
behaviour...
I'd at least like to some justification for the form of the interface in
the changelog and some indication of expected behaviour. That block
should be good enough also to include next to the ioctl so that we have
some ideas of what userspace is expecting when reviewing the code later.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 12/12] drm/i915: s/i915_wait_reqest/i915_wait_seqno/g
2012-04-26 23:02 [PATCH 00/12 v2] wait for BO with timeout Ben Widawsky
` (10 preceding siblings ...)
2012-04-26 23:03 ` [PATCH 11/12 v2] drm/i915: wait render timeout ioctl Ben Widawsky
@ 2012-04-26 23:03 ` Ben Widawsky
11 siblings, 0 replies; 32+ messages in thread
From: Ben Widawsky @ 2012-04-26 23:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Wait request is poorly named IMO. After working with these functions for
some time, I feel it's much clearer to name the functions more
appropriately.
Of course we must update the callers to use the new name as well.
This leaves room within our namespace for a *real* wait request function
at some point.
Note to maintainer: this patch is optional.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/i915_gem.c | 9 ++++-----
drivers/gpu/drm/i915/intel_overlay.c | 4 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
4 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d415f0f..f961cc8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1307,8 +1307,8 @@ int __must_check i915_gem_idle(struct drm_device *dev);
int __must_check i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_request *request);
-int __must_check i915_wait_request(struct intel_ring_buffer *ring,
- uint32_t seqno);
+int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
+ uint32_t seqno);
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,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0dee6bc..3ae21d4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1943,8 +1943,7 @@ again:
* request and object lists appropriately for that event.
*/
int
-i915_wait_request(struct intel_ring_buffer *ring,
- uint32_t seqno)
+i915_wait_seqno(struct intel_ring_buffer *ring, uint32_t seqno)
{
drm_i915_private_t *dev_priv = ring->dev->dev_private;
int ret = 0;
@@ -1982,7 +1981,7 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj)
* it.
*/
if (obj->active) {
- ret = i915_wait_request(obj->ring, obj->last_rendering_seqno);
+ ret = i915_wait_seqno(obj->ring, obj->last_rendering_seqno);
if (ret)
return ret;
i915_gem_retire_requests_ring(obj->ring);
@@ -2224,7 +2223,7 @@ static int i915_ring_idle(struct intel_ring_buffer *ring)
return ret;
}
- return i915_wait_request(ring, i915_gem_next_request_seqno(ring));
+ return i915_wait_seqno(ring, i915_gem_next_request_seqno(ring));
}
int i915_gpu_idle(struct drm_device *dev)
@@ -2423,7 +2422,7 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object *obj)
}
if (obj->last_fenced_seqno) {
- ret = i915_wait_request(obj->ring, obj->last_fenced_seqno);
+ ret = i915_wait_seqno(obj->ring, obj->last_fenced_seqno);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 07a5cad..7a05a34 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -225,7 +225,7 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
}
overlay->last_flip_req = request->seqno;
overlay->flip_tail = tail;
- ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
+ ret = i915_wait_seqno(LP_RING(dev_priv), overlay->last_flip_req);
if (ret)
return ret;
@@ -446,7 +446,7 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
if (overlay->last_flip_req == 0)
return 0;
- ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
+ ret = i915_wait_seqno(LP_RING(dev_priv), overlay->last_flip_req);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a7d97d1..9bc9e12 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1088,7 +1088,7 @@ static int intel_ring_wait_seqno(struct intel_ring_buffer *ring, u32 seqno)
was_interruptible = dev_priv->mm.interruptible;
dev_priv->mm.interruptible = false;
- ret = i915_wait_request(ring, seqno);
+ ret = i915_wait_seqno(ring, seqno);
dev_priv->mm.interruptible = was_interruptible;
if (!ret)
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread