Re: [Intel-gfx] [PATCH 4/5] drm/i915: wait render timeout ioctl

2012-05-02 Thread Daniel Vetter
On Mon, Apr 30, 2012 at 06:41:08PM -0700, Ben Widawsky wrote:
 This helps implement GL_ARB_sync put stops short of allowing full blow
 sync objects.
 
 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.
 
 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)
 
 v3: Drop lock after getting seqno to avoid ugly dance (Chris)
 
 v4: check for 0 timeout after olr check to allow polling (Chris)
 
 Signed-off-by: Ben Widawsky b...@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 |   63 
 +++
  include/drm/i915_drm.h  |8 +
  4 files changed, 74 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 17081da..97a8f02 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -2186,6 +2186,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 ca13098..dd8b33f 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1220,6 +1220,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 8f19cc1..cf3e506 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -1997,6 +1997,69 @@ 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;
 + struct timespec timeout;
 + u32 seqno = 0;
 + int ret = 0;
 +
 + timeout = ns_to_timespec(args-timeout_ns);
 +
 + 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;
 +
 + /* Do this after OLR check to make sure we make forward progress polling
 +  * on this IOCTL with a 0 timeout (like busy ioctl)
 +  */
 + if (!args-timeout_ns) {
 + ret = -EBUSY;

This return value is a bit inconsitency with the case where we have
timeout_ns  0 but time out. I think we should return the same -ETIME in
both cases.
-Daniel
 + goto out;
 + }
 +
 + drm_gem_object_unreference(obj-base);
 + mutex_unlock(dev-struct_mutex);
 +
 + ret = __wait_seqno(ring, seqno, true, timeout);
 + args-timeout_ns = timespec_to_ns(timeout);
 + 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
 --- 

Re: [Intel-gfx] [PATCH 4/5] drm/i915: wait render timeout ioctl

2012-05-01 Thread Ben Widawsky
On Tue, 01 May 2012 10:19:53 -0700
Eric Anholt e...@anholt.net wrote:

 On Mon, 30 Apr 2012 18:41:08 -0700, Ben Widawsky b...@bwidawsk.net wrote:
  This helps implement GL_ARB_sync put stops short of allowing full blow
   but?   blown?
 
  sync objects.
  
  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.
 
 It looks like this actually lets them wait on a BO with a timeout.  I
 like it as an interface, but that's not what I expected from the commit
 message.  Waiting on a request sounds like passing a seqno back out to
 userland that they could wait on.

Yeah. Chris asked me to update the comments actually, and I forgot. I
will resubmit this patch with better comments in the not too distant
future.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 4/5] drm/i915: wait render timeout ioctl

2012-04-30 Thread Ben Widawsky
This helps implement GL_ARB_sync put stops short of allowing full blow
sync objects.

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.

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)

v3: Drop lock after getting seqno to avoid ugly dance (Chris)

v4: check for 0 timeout after olr check to allow polling (Chris)

Signed-off-by: Ben Widawsky b...@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 |   63 +++
 include/drm/i915_drm.h  |8 +
 4 files changed, 74 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 17081da..97a8f02 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2186,6 +2186,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 ca13098..dd8b33f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1220,6 +1220,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 8f19cc1..cf3e506 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1997,6 +1997,69 @@ 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;
+   struct timespec timeout;
+   u32 seqno = 0;
+   int ret = 0;
+
+   timeout = ns_to_timespec(args-timeout_ns);
+
+   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;
+
+   /* Do this after OLR check to make sure we make forward progress polling
+* on this IOCTL with a 0 timeout (like busy ioctl)
+*/
+   if (!args-timeout_ns) {
+   ret = -EBUSY;
+   goto out;
+   }
+
+   drm_gem_object_unreference(obj-base);
+   mutex_unlock(dev-struct_mutex);
+
+   ret = __wait_seqno(ring, seqno, true, timeout);
+   args-timeout_ns = timespec_to_ns(timeout);
+   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