All of lore.kernel.org
 help / color / mirror / Atom feed
* 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.