* [PATCH 2/2 v4] intel: wait render timeout implementation [not found] <1339026077-27937-1-git-send-email-ben@bwidawsk.net> @ 2012-06-06 23:42 ` Ben Widawsky 2012-06-07 7:10 ` Daniel Vetter 0 siblings, 1 reply; 3+ messages in thread From: Ben Widawsky @ 2012-06-06 23:42 UTC (permalink / raw) To: intel-gfx, dri-devel; +Cc: Daniel Vetter, mesa-dev, Ben Widawsky int drm_intel_gem_bo_wait(drm_intel_bo *bo, uint64_t timeout_ns) This should bump the libdrm version. We're waiting for context support so we can do both features in one bump. v2: don't return remaining timeout amount use get param and fallback for older kernels v3: only doing getparam at init prototypes now have a signed input value v4: update comments fall back to correct polling behavior with new userspace and old kernel Cc: Eric Anholt <eric@anholt.net> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- intel/intel_bufmgr.h | 1 + intel/intel_bufmgr_gem.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h index c197abc..fa6c4dd 100644 --- a/intel/intel_bufmgr.h +++ b/intel/intel_bufmgr.h @@ -184,6 +184,7 @@ int drm_intel_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, int crtc_id); int drm_intel_get_aperture_sizes(int fd, size_t *mappable, size_t *total); int drm_intel_bufmgr_gem_get_devid(drm_intel_bufmgr *bufmgr); +int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns); /* drm_intel_bufmgr_fake.c */ drm_intel_bufmgr *drm_intel_bufmgr_fake_init(int fd, diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index b776d2f..f8e3706 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -119,6 +119,7 @@ typedef struct _drm_intel_bufmgr_gem { unsigned int has_blt : 1; unsigned int has_relaxed_fencing : 1; unsigned int has_llc : 1; + unsigned int has_wait_timeout : 1; unsigned int bo_reuse : 1; unsigned int no_exec : 1; bool fenced_relocs; @@ -1479,6 +1480,52 @@ drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo) } /** + * Waits on a BO for the given amount of time. + * + * @bo: buffer object to wait for + * @timeout_ns: amount of time to wait in nanoseconds. + * If value is less than 0, an infinite wait will occur. + * + * Returns 0 if the wait was successful ie. the last batch referencing the + * object has completed within the allotted time. Otherwise some negative return + * value describes the error. + * + * Similar to drm_intel_gem_bo_wait_rendering except a timeout parameter allows + * the operation to give up after a certain amount of time. Another subtle + * difference is the internal locking semantics are different (this variant does + * not hold the lock for the duration of the wait). This makes the wait subject + * to a larger userspace race window. + * + * The implementation shall wait until the object is no longer actively + * referenced within a batch buffer at the time of the call. The wait will + * not guarantee that the buffer is re-issued via another thread, or an flinked + * handle. Userspace must make sure this race does not occur if such precision + * is important. + */ +int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns) +{ + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; + struct drm_i915_gem_wait wait; + + if (!bufmgr_gem->has_wait_timeout) { + DBG("%s:%d: Timed wait is not supported. Falling back to " + "infinite wait\n", __FILE__, __LINE__); + if (timeout_ns) { + drm_intel_gem_bo_wait_rendering(bo); + return 0; + } else { + return drm_intel_gem_bo_busy(bo); + } + } + + wait.bo_handle = bo_gem->gem_handle; + wait.timeout_ns = timeout_ns; + wait.flags = 0; + return drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_WAIT, &wait); +} + +/** * Sets the object to the GTT read and possibly write domain, used by the X * 2D driver in the absence of kernel support to do drm_intel_gem_bo_map_gtt(). * @@ -2898,6 +2945,10 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size) ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); bufmgr_gem->has_relaxed_fencing = ret == 0; + gp.param = I915_PARAM_HAS_WAIT_TIMEOUT; + ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); + bufmgr_gem->has_wait_timeout = ret == 0; + gp.param = I915_PARAM_HAS_LLC; ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); if (ret != 0) { -- 1.7.10.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2 v4] intel: wait render timeout implementation 2012-06-06 23:42 ` [PATCH 2/2 v4] intel: wait render timeout implementation Ben Widawsky @ 2012-06-07 7:10 ` Daniel Vetter 2012-06-07 14:56 ` Ben Widawsky 0 siblings, 1 reply; 3+ messages in thread From: Daniel Vetter @ 2012-06-07 7:10 UTC (permalink / raw) To: Ben Widawsky; +Cc: Daniel Vetter, mesa-dev, intel-gfx, dri-devel On Wed, Jun 06, 2012 at 04:42:11PM -0700, Ben Widawsky wrote: > int drm_intel_gem_bo_wait(drm_intel_bo *bo, uint64_t timeout_ns) > > This should bump the libdrm version. We're waiting for context support > so we can do both features in one bump. > > v2: don't return remaining timeout amount > use get param and fallback for older kernels > > v3: only doing getparam at init > prototypes now have a signed input value > > v4: update comments > fall back to correct polling behavior with new userspace and old kernel > > Cc: Eric Anholt <eric@anholt.net> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > intel/intel_bufmgr.h | 1 + > intel/intel_bufmgr_gem.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > > diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h > index c197abc..fa6c4dd 100644 > --- a/intel/intel_bufmgr.h > +++ b/intel/intel_bufmgr.h > @@ -184,6 +184,7 @@ int drm_intel_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, int crtc_id); > > int drm_intel_get_aperture_sizes(int fd, size_t *mappable, size_t *total); > int drm_intel_bufmgr_gem_get_devid(drm_intel_bufmgr *bufmgr); > +int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns); > > /* drm_intel_bufmgr_fake.c */ > drm_intel_bufmgr *drm_intel_bufmgr_fake_init(int fd, > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index b776d2f..f8e3706 100644 > --- a/intel/intel_bufmgr_gem.c > +++ b/intel/intel_bufmgr_gem.c > @@ -119,6 +119,7 @@ typedef struct _drm_intel_bufmgr_gem { > unsigned int has_blt : 1; > unsigned int has_relaxed_fencing : 1; > unsigned int has_llc : 1; > + unsigned int has_wait_timeout : 1; > unsigned int bo_reuse : 1; > unsigned int no_exec : 1; > bool fenced_relocs; > @@ -1479,6 +1480,52 @@ drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo) > } > > /** > + * Waits on a BO for the given amount of time. > + * > + * @bo: buffer object to wait for > + * @timeout_ns: amount of time to wait in nanoseconds. > + * If value is less than 0, an infinite wait will occur. > + * > + * Returns 0 if the wait was successful ie. the last batch referencing the > + * object has completed within the allotted time. Otherwise some negative return > + * value describes the error. I'm not too sure about that being correct yet. Iirc drmIoctl simply returns -1 on error, with the (positive error number stored in errno), and the busy function returns 1 when the thing is busy. I think we should map both properly to the same -ETIME. -Daniel > + * > + * Similar to drm_intel_gem_bo_wait_rendering except a timeout parameter allows > + * the operation to give up after a certain amount of time. Another subtle > + * difference is the internal locking semantics are different (this variant does > + * not hold the lock for the duration of the wait). This makes the wait subject > + * to a larger userspace race window. > + * > + * The implementation shall wait until the object is no longer actively > + * referenced within a batch buffer at the time of the call. The wait will > + * not guarantee that the buffer is re-issued via another thread, or an flinked > + * handle. Userspace must make sure this race does not occur if such precision > + * is important. > + */ > +int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns) > +{ > + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; > + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; > + struct drm_i915_gem_wait wait; > + > + if (!bufmgr_gem->has_wait_timeout) { > + DBG("%s:%d: Timed wait is not supported. Falling back to " > + "infinite wait\n", __FILE__, __LINE__); > + if (timeout_ns) { > + drm_intel_gem_bo_wait_rendering(bo); > + return 0; > + } else { > + return drm_intel_gem_bo_busy(bo); > + } > + } > + > + wait.bo_handle = bo_gem->gem_handle; > + wait.timeout_ns = timeout_ns; > + wait.flags = 0; > + return drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_WAIT, &wait); > +} > + > +/** > * Sets the object to the GTT read and possibly write domain, used by the X > * 2D driver in the absence of kernel support to do drm_intel_gem_bo_map_gtt(). > * > @@ -2898,6 +2945,10 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size) > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); > bufmgr_gem->has_relaxed_fencing = ret == 0; > > + gp.param = I915_PARAM_HAS_WAIT_TIMEOUT; > + ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); > + bufmgr_gem->has_wait_timeout = ret == 0; > + > gp.param = I915_PARAM_HAS_LLC; > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); > if (ret != 0) { > -- > 1.7.10.3 > -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2 v4] intel: wait render timeout implementation 2012-06-07 7:10 ` Daniel Vetter @ 2012-06-07 14:56 ` Ben Widawsky 0 siblings, 0 replies; 3+ messages in thread From: Ben Widawsky @ 2012-06-07 14:56 UTC (permalink / raw) To: Daniel Vetter; +Cc: Vetter, intel-gfx, dri-devel, Daniel, mesa-dev On Thu, 7 Jun 2012 09:10:08 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Jun 06, 2012 at 04:42:11PM -0700, Ben Widawsky wrote: > > int drm_intel_gem_bo_wait(drm_intel_bo *bo, uint64_t timeout_ns) > > > > This should bump the libdrm version. We're waiting for context support > > so we can do both features in one bump. > > > > v2: don't return remaining timeout amount > > use get param and fallback for older kernels > > > > v3: only doing getparam at init > > prototypes now have a signed input value > > > > v4: update comments > > fall back to correct polling behavior with new userspace and old kernel > > > > Cc: Eric Anholt <eric@anholt.net> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > intel/intel_bufmgr.h | 1 + > > intel/intel_bufmgr_gem.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 52 insertions(+) > > > > diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h > > index c197abc..fa6c4dd 100644 > > --- a/intel/intel_bufmgr.h > > +++ b/intel/intel_bufmgr.h > > @@ -184,6 +184,7 @@ int drm_intel_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, int crtc_id); > > > > int drm_intel_get_aperture_sizes(int fd, size_t *mappable, size_t *total); > > int drm_intel_bufmgr_gem_get_devid(drm_intel_bufmgr *bufmgr); > > +int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns); > > > > /* drm_intel_bufmgr_fake.c */ > > drm_intel_bufmgr *drm_intel_bufmgr_fake_init(int fd, > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > > index b776d2f..f8e3706 100644 > > --- a/intel/intel_bufmgr_gem.c > > +++ b/intel/intel_bufmgr_gem.c > > @@ -119,6 +119,7 @@ typedef struct _drm_intel_bufmgr_gem { > > unsigned int has_blt : 1; > > unsigned int has_relaxed_fencing : 1; > > unsigned int has_llc : 1; > > + unsigned int has_wait_timeout : 1; > > unsigned int bo_reuse : 1; > > unsigned int no_exec : 1; > > bool fenced_relocs; > > @@ -1479,6 +1480,52 @@ drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo) > > } > > > > /** > > + * Waits on a BO for the given amount of time. > > + * > > + * @bo: buffer object to wait for > > + * @timeout_ns: amount of time to wait in nanoseconds. > > + * If value is less than 0, an infinite wait will occur. > > + * > > + * Returns 0 if the wait was successful ie. the last batch referencing the > > + * object has completed within the allotted time. Otherwise some negative return > > + * value describes the error. > > I'm not too sure about that being correct yet. Iirc drmIoctl simply > returns -1 on error, with the (positive error number stored in errno), > and the busy function returns 1 when the thing is busy. I think we should > map both properly to the same -ETIME. > -Daniel > This was supposed to be solved by patch 1/2 where I changed the behavior of drmIoctl. It seems Dave didn't like the idea so much. Unless you want to take up arms with me, there will be a v5 of this patch. > > + * > > + * Similar to drm_intel_gem_bo_wait_rendering except a timeout parameter allows > > + * the operation to give up after a certain amount of time. Another subtle > > + * difference is the internal locking semantics are different (this variant does > > + * not hold the lock for the duration of the wait). This makes the wait subject > > + * to a larger userspace race window. > > + * > > + * The implementation shall wait until the object is no longer actively > > + * referenced within a batch buffer at the time of the call. The wait will > > + * not guarantee that the buffer is re-issued via another thread, or an flinked > > + * handle. Userspace must make sure this race does not occur if such precision > > + * is important. > > + */ > > +int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns) > > +{ > > + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; > > + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; > > + struct drm_i915_gem_wait wait; > > + > > + if (!bufmgr_gem->has_wait_timeout) { > > + DBG("%s:%d: Timed wait is not supported. Falling back to " > > + "infinite wait\n", __FILE__, __LINE__); > > + if (timeout_ns) { > > + drm_intel_gem_bo_wait_rendering(bo); > > + return 0; > > + } else { > > + return drm_intel_gem_bo_busy(bo); > > + } > > + } > > + > > + wait.bo_handle = bo_gem->gem_handle; > > + wait.timeout_ns = timeout_ns; > > + wait.flags = 0; > > + return drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_WAIT, &wait); > > +} > > + > > +/** > > * Sets the object to the GTT read and possibly write domain, used by the X > > * 2D driver in the absence of kernel support to do drm_intel_gem_bo_map_gtt(). > > * > > @@ -2898,6 +2945,10 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size) > > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); > > bufmgr_gem->has_relaxed_fencing = ret == 0; > > > > + gp.param = I915_PARAM_HAS_WAIT_TIMEOUT; > > + ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); > > + bufmgr_gem->has_wait_timeout = ret == 0; > > + > > gp.param = I915_PARAM_HAS_LLC; > > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp); > > if (ret != 0) { > > -- > > 1.7.10.3 > > > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-06-07 14:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1339026077-27937-1-git-send-email-ben@bwidawsk.net>
2012-06-06 23:42 ` [PATCH 2/2 v4] intel: wait render timeout implementation Ben Widawsky
2012-06-07 7:10 ` Daniel Vetter
2012-06-07 14:56 ` Ben Widawsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox