Re: [Intel-gfx] [PATCH 1/2] drm/i915: Improve fallback ring waiting

2014-05-07 Thread Volkin, Bradley D 
On Mon, May 05, 2014 at 01:07:32AM -0700, Chris Wilson wrote:
 A few improvements to the fallback method for waiting upon ring space:
 
 1. Fix the start/end wait tracepoints to always be paired.
 2. Increase responsiveness of checking
 3. Mark the process as waiting upon io
 4. Check for signal interruptions
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

These seem like reasonable improvements.
Reviewed-by: Brad Volkin bradley.d.vol...@intel.com

 ---
  drivers/gpu/drm/i915/intel_ringbuffer.c | 24 +---
  1 file changed, 17 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index e0c7bf27eafd..d6b7b884adf9 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -1546,7 +1546,6 @@ static int ring_wait_for_space(struct intel_ring_buffer 
 *ring, int n)
   /* force the tail write in case we have been skipping them */
   __intel_ring_advance(ring);
  
 - trace_i915_ring_wait_begin(ring);
   /* With GEM the hangcheck timer should kick us out of the loop,
* leaving it early runs the risk of corrupting GEM state (due
* to running on almost untested codepaths). But on resume
 @@ -1554,12 +1553,13 @@ static int ring_wait_for_space(struct 
 intel_ring_buffer *ring, int n)
* case by choosing an insanely large timeout. */
   end = jiffies + 60 * HZ;
  
 + trace_i915_ring_wait_begin(ring);
   do {
   ring-head = I915_READ_HEAD(ring);
   ring-space = ring_space(ring);
   if (ring-space = n) {
 - trace_i915_ring_wait_end(ring);
 - return 0;
 + ret = 0;
 + break;
   }
  
   if (!drm_core_check_feature(dev, DRIVER_MODESET) 
 @@ -1569,15 +1569,25 @@ static int ring_wait_for_space(struct 
 intel_ring_buffer *ring, int n)
   master_priv-sarea_priv-perf_boxes |= 
 I915_BOX_WAIT;
   }
  
 - msleep(1);
 + io_schedule_timeout(1);
 +
 + if (dev_priv-mm.interruptible  signal_pending(current)) {
 + ret = -ERESTARTSYS;
 + break;
 + }
  
   ret = i915_gem_check_wedge(dev_priv-gpu_error,
  dev_priv-mm.interruptible);
   if (ret)
 - return ret;
 - } while (!time_after(jiffies, end));
 + break;
 +
 + if (time_after(jiffies, end)) {
 + ret = -EBUSY;
 + break;
 + }
 + } while (1);
   trace_i915_ring_wait_end(ring);
 - return -EBUSY;
 + return ret;
  }
  
  static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring)
 -- 
 2.0.0.rc0
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Improve fallback ring waiting

2014-05-07 Thread Daniel Vetter 
On Mon, May 05, 2014 at 09:07:32AM +0100, Chris Wilson wrote:
 A few improvements to the fallback method for waiting upon ring space:
 
 1. Fix the start/end wait tracepoints to always be paired.
 2. Increase responsiveness of checking
 3. Mark the process as waiting upon io
 4. Check for signal interruptions
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/intel_ringbuffer.c | 24 +---
  1 file changed, 17 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index e0c7bf27eafd..d6b7b884adf9 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -1546,7 +1546,6 @@ static int ring_wait_for_space(struct intel_ring_buffer 
 *ring, int n)
   /* force the tail write in case we have been skipping them */
   __intel_ring_advance(ring);
  
 - trace_i915_ring_wait_begin(ring);
   /* With GEM the hangcheck timer should kick us out of the loop,
* leaving it early runs the risk of corrupting GEM state (due
* to running on almost untested codepaths). But on resume
 @@ -1554,12 +1553,13 @@ static int ring_wait_for_space(struct 
 intel_ring_buffer *ring, int n)
* case by choosing an insanely large timeout. */
   end = jiffies + 60 * HZ;
  
 + trace_i915_ring_wait_begin(ring);
   do {
   ring-head = I915_READ_HEAD(ring);
   ring-space = ring_space(ring);
   if (ring-space = n) {
 - trace_i915_ring_wait_end(ring);
 - return 0;
 + ret = 0;
 + break;
   }
  
   if (!drm_core_check_feature(dev, DRIVER_MODESET) 
 @@ -1569,15 +1569,25 @@ static int ring_wait_for_space(struct 
 intel_ring_buffer *ring, int n)
   master_priv-sarea_priv-perf_boxes |= 
 I915_BOX_WAIT;
   }
  
 - msleep(1);
 + io_schedule_timeout(1);

This isn't exported so doesn't compile too well. I've undone this change
for now. Otherwise both patches merged.
-Daniel

 +
 + if (dev_priv-mm.interruptible  signal_pending(current)) {
 + ret = -ERESTARTSYS;
 + break;
 + }
  
   ret = i915_gem_check_wedge(dev_priv-gpu_error,
  dev_priv-mm.interruptible);
   if (ret)
 - return ret;
 - } while (!time_after(jiffies, end));
 + break;
 +
 + if (time_after(jiffies, end)) {
 + ret = -EBUSY;
 + break;
 + }
 + } while (1);
   trace_i915_ring_wait_end(ring);
 - return -EBUSY;
 + return ret;
  }
  
  static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring)
 -- 
 2.0.0.rc0
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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