Re: [Intel-gfx] [PATCH 4/5] drm/i915: wait render timeout ioctl
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
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
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