* [PATCH v5] intel: wait render timeout implementation
@ 2012-06-07 16:12 Ben Widawsky
2012-06-07 16:12 ` [RFC] [PATCH] i965: better ClientWaitSync Ben Widawsky
2012-06-08 18:19 ` [PATCH v5] intel: wait render timeout implementation Daniel Vetter
0 siblings, 2 replies; 5+ messages in thread
From: Ben Widawsky @ 2012-06-07 16:12 UTC (permalink / raw)
To: DRI devel, Intel GFX; +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
v5: since the drmIoctl patch was not well received, return appropriate
values in this function instead. As Daniel pointed out, the polling
case (timeout == 0) should also return -ETIME.
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 | 57 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 58 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..e90f8bd 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,58 @@ 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. Of particular interest is -ETIME when the wait has
+ * failed to yield the desired result.
+ *
+ * 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;
+ int ret;
+
+ 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) ? -ETIME : 0;
+ }
+ }
+
+ wait.bo_handle = bo_gem->gem_handle;
+ wait.timeout_ns = timeout_ns;
+ wait.flags = 0;
+ ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
+ if (ret == -1)
+ return -errno;
+
+ return ret;
+}
+
+/**
* 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 +2951,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] 5+ messages in thread* [RFC] [PATCH] i965: better ClientWaitSync
2012-06-07 16:12 [PATCH v5] intel: wait render timeout implementation Ben Widawsky
@ 2012-06-07 16:12 ` Ben Widawsky
2012-06-07 19:08 ` Daniel Vetter
2012-06-08 18:19 ` [PATCH v5] intel: wait render timeout implementation Daniel Vetter
1 sibling, 1 reply; 5+ messages in thread
From: Ben Widawsky @ 2012-06-07 16:12 UTC (permalink / raw)
To: Mesa dev; +Cc: Ben Widawsky, Intel GFX
Use the new libdrm functionality to actually do timed waits on the sync
object.
This patch is missing the configure.ac update to check for the correct
libdrm supporting this function. As of now, libdrm has not yet received
the version bump. That's mostly why this patch is "RFC"
Since intel_client_wait_sync function previously had no way of reporting
back the errors/timeouts, and I do not have a good test case anyway,
I've decided to not worry about this also.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
src/mesa/drivers/dri/intel/intel_syncobj.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/intel/intel_syncobj.c b/src/mesa/drivers/dri/intel/intel_syncobj.c
index b303ea8..4dd8b7b 100644
--- a/src/mesa/drivers/dri/intel/intel_syncobj.c
+++ b/src/mesa/drivers/dri/intel/intel_syncobj.c
@@ -93,7 +93,7 @@ static void intel_client_wait_sync(struct gl_context *ctx, struct gl_sync_object
struct intel_sync_object *sync = (struct intel_sync_object *)s;
if (sync->bo) {
- drm_intel_bo_wait_rendering(sync->bo);
+ drm_intel_gem_bo_wait(sync->bo, timeout);
s->StatusFlag = 1;
drm_intel_bo_unreference(sync->bo);
sync->bo = NULL;
--
1.7.10.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [RFC] [PATCH] i965: better ClientWaitSync
2012-06-07 16:12 ` [RFC] [PATCH] i965: better ClientWaitSync Ben Widawsky
@ 2012-06-07 19:08 ` Daniel Vetter
2012-06-07 20:23 ` Ben Widawsky
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2012-06-07 19:08 UTC (permalink / raw)
To: Ben Widawsky; +Cc: Mesa dev, Intel GFX
On Thu, Jun 07, 2012 at 09:12:42AM -0700, Ben Widawsky wrote:
> Use the new libdrm functionality to actually do timed waits on the sync
> object.
>
> This patch is missing the configure.ac update to check for the correct
> libdrm supporting this function. As of now, libdrm has not yet received
> the version bump. That's mostly why this patch is "RFC"
>
> Since intel_client_wait_sync function previously had no way of reporting
> back the errors/timeouts, and I do not have a good test case anyway,
> I've decided to not worry about this also.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> src/mesa/drivers/dri/intel/intel_syncobj.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_syncobj.c b/src/mesa/drivers/dri/intel/intel_syncobj.c
> index b303ea8..4dd8b7b 100644
> --- a/src/mesa/drivers/dri/intel/intel_syncobj.c
> +++ b/src/mesa/drivers/dri/intel/intel_syncobj.c
> @@ -93,7 +93,7 @@ static void intel_client_wait_sync(struct gl_context *ctx, struct gl_sync_object
> struct intel_sync_object *sync = (struct intel_sync_object *)s;
>
> if (sync->bo) {
> - drm_intel_bo_wait_rendering(sync->bo);
> + drm_intel_gem_bo_wait(sync->bo, timeout);
If I've read the core mesa code correctly, StatusFlag = 1 means that the
fence has actually signalled. I guess you need to add a check for the
return value and do the below 3 lines only when the wait hasn't timed out
(or failed for another stupid reason).
-Daniel
> s->StatusFlag = 1;
> drm_intel_bo_unreference(sync->bo);
> sync->bo = NULL;
> --
> 1.7.10.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC] [PATCH] i965: better ClientWaitSync
2012-06-07 19:08 ` Daniel Vetter
@ 2012-06-07 20:23 ` Ben Widawsky
0 siblings, 0 replies; 5+ messages in thread
From: Ben Widawsky @ 2012-06-07 20:23 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Mesa dev, Intel GFX
On Thu, 7 Jun 2012 21:08:49 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Jun 07, 2012 at 09:12:42AM -0700, Ben Widawsky wrote:
> > Use the new libdrm functionality to actually do timed waits on the sync
> > object.
> >
> > This patch is missing the configure.ac update to check for the correct
> > libdrm supporting this function. As of now, libdrm has not yet received
> > the version bump. That's mostly why this patch is "RFC"
> >
> > Since intel_client_wait_sync function previously had no way of reporting
> > back the errors/timeouts, and I do not have a good test case anyway,
> > I've decided to not worry about this also.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > src/mesa/drivers/dri/intel/intel_syncobj.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/intel/intel_syncobj.c b/src/mesa/drivers/dri/intel/intel_syncobj.c
> > index b303ea8..4dd8b7b 100644
> > --- a/src/mesa/drivers/dri/intel/intel_syncobj.c
> > +++ b/src/mesa/drivers/dri/intel/intel_syncobj.c
> > @@ -93,7 +93,7 @@ static void intel_client_wait_sync(struct gl_context *ctx, struct gl_sync_object
> > struct intel_sync_object *sync = (struct intel_sync_object *)s;
> >
> > if (sync->bo) {
> > - drm_intel_bo_wait_rendering(sync->bo);
> > + drm_intel_gem_bo_wait(sync->bo, timeout);
>
> If I've read the core mesa code correctly, StatusFlag = 1 means that the
> fence has actually signalled. I guess you need to add a check for the
> return value and do the below 3 lines only when the wait hasn't timed out
> (or failed for another stupid reason).
> -Daniel
It would have been wise for me to look at that :-). I was sort of not
planning to ever push this anyway, I'd like to get some real test cases
first. The patch was mostly just to make the intel mesa devs aware of
the existence.
>
> > s->StatusFlag = 1;
> > drm_intel_bo_unreference(sync->bo);
> > sync->bo = NULL;
> > --
> > 1.7.10.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5] intel: wait render timeout implementation
2012-06-07 16:12 [PATCH v5] intel: wait render timeout implementation Ben Widawsky
2012-06-07 16:12 ` [RFC] [PATCH] i965: better ClientWaitSync Ben Widawsky
@ 2012-06-08 18:19 ` Daniel Vetter
1 sibling, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2012-06-08 18:19 UTC (permalink / raw)
To: Ben Widawsky; +Cc: Daniel Vetter, Mesa dev, Intel GFX, DRI devel
On Thu, Jun 07, 2012 at 09:12:41AM -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
>
> v5: since the drmIoctl patch was not well received, return appropriate
> values in this function instead. As Daniel pointed out, the polling
> case (timeout == 0) should also return -ETIME.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> intel/intel_bufmgr.h | 1 +
> intel/intel_bufmgr_gem.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 58 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..e90f8bd 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,58 @@ 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. Of particular interest is -ETIME when the wait has
> + * failed to yield the desired result.
> + *
> + * 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;
> + int ret;
> +
> + 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) ? -ETIME : 0;
> + }
> + }
> +
> + wait.bo_handle = bo_gem->gem_handle;
> + wait.timeout_ns = timeout_ns;
> + wait.flags = 0;
> + ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
> + if (ret == -1)
> + return -errno;
> +
> + return ret;
> +}
> +
> +/**
> * 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 +2951,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] 5+ messages in thread
end of thread, other threads:[~2012-06-08 18:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-07 16:12 [PATCH v5] intel: wait render timeout implementation Ben Widawsky
2012-06-07 16:12 ` [RFC] [PATCH] i965: better ClientWaitSync Ben Widawsky
2012-06-07 19:08 ` Daniel Vetter
2012-06-07 20:23 ` Ben Widawsky
2012-06-08 18:19 ` [PATCH v5] intel: wait render timeout implementation Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox