diff mbox

[3/3] drm/i915: Limit request busywaiting

Message ID 1447840568-20167-4-git-send-email-chris@chris-wilson.co.uk
State New, archived
Headers show

Commit Message

Chris Wilson Nov. 18, 2015, 9:56 a.m. UTC
If we suppose that on average it takes 1ms to do a fullscreen paint
(which is fairly typical across generations, as the GPUs get faster the
displays get larger), then the likelihood of a synchronous wait on the last
request completing within 2 microseconds is miniscule. On the other
hand, not everything attempts a fullscreen repaint (or is ratelimited by
such) and for those a short busywait is much faster than setting up the
interrupt driven waiter. The challenge is identifying such tasks. Here
we opt to never spin for an unlocked wait (such as from a set-domain or
wait ioctl) and instead only spin in circumstances where we are holding
the lock and have either already completed one unlocked wait or we are
in a situation where we are being forced to drain old requests (which we
know must be close to completion).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_gem.c         | 12 +++++++-----
 drivers/gpu/drm/i915/intel_ringbuffer.c |  2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

Comments

Daniel Vetter Nov. 19, 2015, 3:22 p.m. UTC | #1
On Wed, Nov 18, 2015 at 09:56:08AM +0000, Chris Wilson wrote:
> If we suppose that on average it takes 1ms to do a fullscreen paint
> (which is fairly typical across generations, as the GPUs get faster the
> displays get larger), then the likelihood of a synchronous wait on the last
> request completing within 2 microseconds is miniscule. On the other
> hand, not everything attempts a fullscreen repaint (or is ratelimited by
> such) and for those a short busywait is much faster than setting up the
> interrupt driven waiter. The challenge is identifying such tasks. Here
> we opt to never spin for an unlocked wait (such as from a set-domain or
> wait ioctl) and instead only spin in circumstances where we are holding
> the lock and have either already completed one unlocked wait or we are
> in a situation where we are being forced to drain old requests (which we
> know must be close to completion).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Do you have some perf datat for this?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/i915_gem.c         | 12 +++++++-----
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  2 +-
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f47d701f2ddb..dc8f2a87dac0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2979,6 +2979,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>  			s64 *timeout,
>  			struct intel_rps_client *rps);
>  #define I915_WAIT_INTERRUPTIBLE 0x1
> +#define I915_WAIT_MAY_SPIN 0x2
>  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 d4d5c6e8c02f..cfa101fad479 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1273,9 +1273,11 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>  	before = ktime_get_raw_ns();
>  
>  	/* Optimistic spin for the next jiffie before touching IRQs */
> -	ret = __i915_spin_request(req, state);
> -	if (ret == 0)
> -		goto out;
> +	if (flags & I915_WAIT_MAY_SPIN) {
> +		ret = __i915_spin_request(req, state);
> +		if (ret == 0)
> +			goto out;
> +	}
>  
>  	if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring))) {
>  		ret = -ENODEV;
> @@ -1467,7 +1469,7 @@ i915_wait_request(struct drm_i915_gem_request *req)
>  	if (ret)
>  		return ret;
>  
> -	flags = 0;
> +	flags = I915_WAIT_MAY_SPIN;
>  	if (dev_priv->mm.interruptible)
>  		flags |= I915_WAIT_INTERRUPTIBLE;
>  
> @@ -4107,7 +4109,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
>  
>  	ret = __i915_wait_request(target,
>  				  reset_counter,
> -				  I915_WAIT_INTERRUPTIBLE,
> +				  I915_WAIT_INTERRUPTIBLE | I915_WAIT_MAY_SPIN,
>  				  NULL, NULL);
>  	if (ret == 0)
>  		queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 664ce0b20b23..d3142af0f347 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2241,7 +2241,7 @@ int intel_ring_idle(struct intel_engine_cs *ring)
>  			struct drm_i915_gem_request,
>  			list);
>  
> -	flags = 0;
> +	flags = I915_WAIT_MAY_SPIN;
>  	if (to_i915(ring->dev)->mm.interruptible)
>  		flags |= I915_WAIT_INTERRUPTIBLE;
>  
> -- 
> 2.6.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f47d701f2ddb..dc8f2a87dac0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2979,6 +2979,7 @@  int __i915_wait_request(struct drm_i915_gem_request *req,
 			s64 *timeout,
 			struct intel_rps_client *rps);
 #define I915_WAIT_INTERRUPTIBLE 0x1
+#define I915_WAIT_MAY_SPIN 0x2
 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 d4d5c6e8c02f..cfa101fad479 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1273,9 +1273,11 @@  int __i915_wait_request(struct drm_i915_gem_request *req,
 	before = ktime_get_raw_ns();
 
 	/* Optimistic spin for the next jiffie before touching IRQs */
-	ret = __i915_spin_request(req, state);
-	if (ret == 0)
-		goto out;
+	if (flags & I915_WAIT_MAY_SPIN) {
+		ret = __i915_spin_request(req, state);
+		if (ret == 0)
+			goto out;
+	}
 
 	if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring))) {
 		ret = -ENODEV;
@@ -1467,7 +1469,7 @@  i915_wait_request(struct drm_i915_gem_request *req)
 	if (ret)
 		return ret;
 
-	flags = 0;
+	flags = I915_WAIT_MAY_SPIN;
 	if (dev_priv->mm.interruptible)
 		flags |= I915_WAIT_INTERRUPTIBLE;
 
@@ -4107,7 +4109,7 @@  i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 
 	ret = __i915_wait_request(target,
 				  reset_counter,
-				  I915_WAIT_INTERRUPTIBLE,
+				  I915_WAIT_INTERRUPTIBLE | I915_WAIT_MAY_SPIN,
 				  NULL, NULL);
 	if (ret == 0)
 		queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 664ce0b20b23..d3142af0f347 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2241,7 +2241,7 @@  int intel_ring_idle(struct intel_engine_cs *ring)
 			struct drm_i915_gem_request,
 			list);
 
-	flags = 0;
+	flags = I915_WAIT_MAY_SPIN;
 	if (to_i915(ring->dev)->mm.interruptible)
 		flags |= I915_WAIT_INTERRUPTIBLE;