* Fw: [Intel-gfx] [PATCH] intel: add a timed wait function
@ 2012-05-27 20:16 Ben Widawsky
2012-05-30 17:41 ` Eric Anholt
0 siblings, 1 reply; 7+ messages in thread
From: Ben Widawsky @ 2012-05-27 20:16 UTC (permalink / raw)
To: dri-devel, mesa-dev
The kernel patches have now landed in Daniel's -next-queued tree.
Begin forwarded message:
Date: Fri, 11 May 2012 13:54:11 -0700
From: Ben Widawsky <ben@bwidawsk.net>
To: intel-gfx@lists.freedesktop.org
Cc: Ben Widawsky <ben@bwidawsk.net>
Subject: [Intel-gfx] [PATCH] intel: add a timed wait function
drm_intel_gem_bo_wait(bo, &timeout in ns)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
include/drm/i915_drm.h | 10 ++++++++++
intel/intel_bufmgr.h | 1 +
intel/intel_bufmgr_gem.c | 26 ++++++++++++++++++++++++++
3 files changed, 37 insertions(+)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index af3ce17..5dc5f12 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -191,6 +191,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 0x2c
#define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE +
DRM_I915_INIT, drm_i915_init_t) #define
DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE +
DRM_I915_FLUSH) @@ -234,6 +235,7 @@ typedef struct _drm_i915_sarea
{ #define DRM_IOCTL_I915_OVERLAY_ATTRS DRM_IOWR(DRM_COMMAND_BASE
+ DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs) #define
DRM_IOCTL_I915_SET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey) #define
DRM_IOCTL_I915_GET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
+#define DRM_IOCTL_I915_GEM_WAIT
DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_WAIT, struct
drm_i915_gem_wait) /* Allow drivers to submit batchbuffers directly to
hardware, relying
* on the security mechanisms provided by hardware.
@@ -876,4 +878,12 @@ struct drm_intel_sprite_colorkey {
__u32 flags;
};
+struct drm_i915_gem_wait {
+ /** Handle of BO we shall wait on */
+ __u32 bo_handle;
+ __u32 flags;
+ /** Number of nanoseconds to wait, Returns time remaining. */
+ __u64 timeout_ns;
+};
+
#endif /* _I915_DRM_H_ */
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
index c197abc..d78884e 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, uint64_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..695a449 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -1478,6 +1478,32 @@ drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo)
drm_intel_gem_bo_start_gtt_access(bo, 1);
}
+int drm_intel_gem_bo_wait(drm_intel_bo *bo, uint64_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 (!timeout_ns)
+ return -EINVAL;
+
+ 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)
+ return ret;
+
+ if (wait.timeout_ns == 0) {
+ DBG("Wait timed out on buffer %d\n",
bo_gem->gem_handle);
+ *timeout_ns = 0;
+ } else
+ *timeout_ns = wait.timeout_ns;
+
+ 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().
--
1.7.10.1
_______________________________________________
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 related [flat|nested] 7+ messages in thread
* Re: Fw: [Intel-gfx] [PATCH] intel: add a timed wait function
2012-05-27 20:16 Fw: [Intel-gfx] [PATCH] intel: add a timed wait function Ben Widawsky
@ 2012-05-30 17:41 ` Eric Anholt
2012-05-30 18:13 ` Ben Widawsky
2012-05-30 19:07 ` Fw: " Daniel Vetter
0 siblings, 2 replies; 7+ messages in thread
From: Eric Anholt @ 2012-05-30 17:41 UTC (permalink / raw)
To: Ben Widawsky, dri-devel, mesa-dev
[-- Attachment #1.1: Type: text/plain, Size: 1594 bytes --]
On Sun, 27 May 2012 13:16:54 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index b776d2f..695a449 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -1478,6 +1478,32 @@ drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo)
> drm_intel_gem_bo_start_gtt_access(bo, 1);
> }
>
> +int drm_intel_gem_bo_wait(drm_intel_bo *bo, uint64_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 (!timeout_ns)
> + return -EINVAL;
At least for the GL case, timeout of 0 ns wants to turn into
GL_TIMEOUT_EXPIRED or GL_ALREADY_SIGNALED. -EINVAL doesn't sound like
translating into either of those -- are you thinking that GL will
special case 0 ns to not call this function?
> +
> + 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)
> + return ret;
> +
> + if (wait.timeout_ns == 0) {
> + DBG("Wait timed out on buffer %d\n",
> bo_gem->gem_handle);
> + *timeout_ns = 0;
> + } else
> + *timeout_ns = wait.timeout_ns;
> +
> + return ret;
> +}
Do we see any consumers wanting the unslept time? GL doesn't care, and
not passing a pointer would be more convenient for the caller.
I guess GL_ALREADY_SIGNALED handling will be done using a check for
bo_busy() before calling this.
[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] intel: add a timed wait function
2012-05-30 17:41 ` Eric Anholt
@ 2012-05-30 18:13 ` Ben Widawsky
2012-05-30 19:07 ` Fw: " Daniel Vetter
1 sibling, 0 replies; 7+ messages in thread
From: Ben Widawsky @ 2012-05-30 18:13 UTC (permalink / raw)
To: Eric Anholt; +Cc: mesa-dev, dri-devel
On Wed, 30 May 2012 10:41:20 -0700
Eric Anholt <eric@anholt.net> wrote:
> On Sun, 27 May 2012 13:16:54 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > index b776d2f..695a449 100644
> > --- a/intel/intel_bufmgr_gem.c
> > +++ b/intel/intel_bufmgr_gem.c
> > @@ -1478,6 +1478,32 @@ drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo)
> > drm_intel_gem_bo_start_gtt_access(bo, 1);
> > }
> >
> > +int drm_intel_gem_bo_wait(drm_intel_bo *bo, uint64_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 (!timeout_ns)
> > + return -EINVAL;
>
> At least for the GL case, timeout of 0 ns wants to turn into
> GL_TIMEOUT_EXPIRED or GL_ALREADY_SIGNALED. -EINVAL doesn't sound like
> translating into either of those -- are you thinking that GL will
> special case 0 ns to not call this function?
Well, it's timeout of NULL, not 0. 0 should do what you want. I can turn
NULL into 0 just as easily, if you want?
>
> > +
> > + 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)
> > + return ret;
> > +
> > + if (wait.timeout_ns == 0) {
> > + DBG("Wait timed out on buffer %d\n",
> > bo_gem->gem_handle);
> > + *timeout_ns = 0;
> > + } else
> > + *timeout_ns = wait.timeout_ns;
> > +
> > + return ret;
> > +}
>
> Do we see any consumers wanting the unslept time? GL doesn't care, and
> not passing a pointer would be more convenient for the caller.
That is how I originally had it, but Daniel Vetter requested otherwise.
I don't care either way. This interacts with your earlier comment as
well.
>
> I guess GL_ALREADY_SIGNALED handling will be done using a check for
> bo_busy() before calling this.
It shouldn't have to.
I think the outcome is either, drop the return time, or convert NULLs to
0, and everything should be fine, right?
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Fw: [Intel-gfx] [PATCH] intel: add a timed wait function
2012-05-30 17:41 ` Eric Anholt
2012-05-30 18:13 ` Ben Widawsky
@ 2012-05-30 19:07 ` Daniel Vetter
2012-05-30 20:32 ` Ben Widawsky
2012-05-30 22:35 ` Fw: " Eric Anholt
1 sibling, 2 replies; 7+ messages in thread
From: Daniel Vetter @ 2012-05-30 19:07 UTC (permalink / raw)
To: Eric Anholt; +Cc: mesa-dev, Ben Widawsky, dri-devel
On Wed, May 30, 2012 at 7:41 PM, Eric Anholt <eric@anholt.net> wrote:
> I guess GL_ALREADY_SIGNALED handling will be done using a check for
> bo_busy() before calling this.
I've just read through the mesa code and gl_already_signalled seems to
be handled already by core mesa code in _mesa_ClientWaitSync (if the
driver sets syncObject->Status correctly). So I guess the current
kernel code should work as-is and only the libdrm interface needs some
colour adjustments around the timeout parameter.
-Daniel
--
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] intel: add a timed wait function
2012-05-30 19:07 ` Fw: " Daniel Vetter
@ 2012-05-30 20:32 ` Ben Widawsky
2012-05-30 22:35 ` Fw: " Eric Anholt
1 sibling, 0 replies; 7+ messages in thread
From: Ben Widawsky @ 2012-05-30 20:32 UTC (permalink / raw)
To: Daniel Vetter; +Cc: dri-devel, mesa-dev
On Wed, 30 May 2012 21:07:57 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, May 30, 2012 at 7:41 PM, Eric Anholt <eric@anholt.net> wrote:
> > I guess GL_ALREADY_SIGNALED handling will be done using a check for
> > bo_busy() before calling this.
>
> I've just read through the mesa code and gl_already_signalled seems to
> be handled already by core mesa code in _mesa_ClientWaitSync (if the
> driver sets syncObject->Status correctly). So I guess the current
> kernel code should work as-is and only the libdrm interface needs some
> colour adjustments around the timeout parameter.
> -Daniel
What have we all agreed on for the color?
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Fw: [Intel-gfx] [PATCH] intel: add a timed wait function
2012-05-30 19:07 ` Fw: " Daniel Vetter
2012-05-30 20:32 ` Ben Widawsky
@ 2012-05-30 22:35 ` Eric Anholt
2012-05-31 7:05 ` Daniel Vetter
1 sibling, 1 reply; 7+ messages in thread
From: Eric Anholt @ 2012-05-30 22:35 UTC (permalink / raw)
To: Daniel Vetter; +Cc: mesa-dev, Ben Widawsky, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 943 bytes --]
On Wed, 30 May 2012 21:07:57 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, May 30, 2012 at 7:41 PM, Eric Anholt <eric@anholt.net> wrote:
> > I guess GL_ALREADY_SIGNALED handling will be done using a check for
> > bo_busy() before calling this.
>
> I've just read through the mesa code and gl_already_signalled seems to
> be handled already by core mesa code in _mesa_ClientWaitSync (if the
> driver sets syncObject->Status correctly). So I guess the current
> kernel code should work as-is and only the libdrm interface needs some
> colour adjustments around the timeout parameter.
Yeah, matches what I found.
Did you want pointer for timeout in the userspace api? I don't feel
strongly about it, I just didn't see a use. The equivalent API I could
think of was select(), where apparently linux returns time unwaited,
while "everyone else" doesn't. I don't see a strong recommendation
either way from that.
[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Fw: [Intel-gfx] [PATCH] intel: add a timed wait function
2012-05-30 22:35 ` Fw: " Eric Anholt
@ 2012-05-31 7:05 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2012-05-31 7:05 UTC (permalink / raw)
To: Eric Anholt; +Cc: mesa-dev, Ben Widawsky, dri-devel
On Thu, May 31, 2012 at 12:35 AM, Eric Anholt <eric@anholt.net> wrote:
>
> Did you want pointer for timeout in the userspace api? I don't feel
> strongly about it, I just didn't see a use. The equivalent API I could
> think of was select(), where apparently linux returns time unwaited,
> while "everyone else" doesn't. I don't see a strong recommendation
> either way from that.
I wanted the kernel to return the unwaited time (or at least a upper
bound to it, with coarse clocks that's the best we can do) so that
ioctl restarting after a signal does the right thing. Exposing that
any further than libdrm doesn't make much sense imo.
-Daniel
--
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-05-31 7:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-27 20:16 Fw: [Intel-gfx] [PATCH] intel: add a timed wait function Ben Widawsky
2012-05-30 17:41 ` Eric Anholt
2012-05-30 18:13 ` Ben Widawsky
2012-05-30 19:07 ` Fw: " Daniel Vetter
2012-05-30 20:32 ` Ben Widawsky
2012-05-30 22:35 ` Fw: " Eric Anholt
2012-05-31 7:05 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.