All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: report all active objects as busy
@ 2010-08-04 14:36 Chris Wilson
  2010-08-04 18:57 ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2010-08-04 14:36 UTC (permalink / raw)
  To: intel-gfx

Incorporates a similar patch by Daniel Vetter, the alteration being to
report the current busy state after retiring.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc : Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c |   40 +++++++++++++++++++++++++-------------
 1 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f599d77..909e727 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4163,22 +4163,34 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 	}
 
 	mutex_lock(&dev->struct_mutex);
-	/* Update the active list for the hardware's current position.
-	 * Otherwise this only updates on a delayed timer or when irqs are
-	 * actually unmasked, and our working set ends up being larger than
-	 * required.
-	 */
-	i915_gem_retire_requests(dev);
 
-	obj_priv = to_intel_bo(obj);
-	/* Don't count being on the flushing list against the object being
-	 * done.  Otherwise, a buffer left on the flushing list but not getting
-	 * flushed (because nobody's flushing that domain) won't ever return
-	 * unbusy and get reused by libdrm's bo cache.  The other expected
-	 * consumer of this interface, OpenGL's occlusion queries, also specs
-	 * that the objects get unbusy "eventually" without any interference.
+	/* Count all active objects as busy, even if they are currently not used
+	 * by the gpu. Users of this interface expect objects to eventually
+	 * become non-busy without any further actions, therefore emit any
+	 * necessary flushes here.
 	 */
-	args->busy = obj_priv->active && obj_priv->last_rendering_seqno != 0;
+	obj_priv = to_intel_bo(obj);
+	args->busy = obj_priv->active;
+	if (args->busy) {
+		/* Unconditionally flush objects, even when the gpu still uses this
+		 * object. Userspace calling this function indicates that it wants to
+		 * use this buffer rather sooner than later, so issuing the required
+		 * flush earlier is beneficial.
+		 */
+		if (obj->write_domain) {
+			i915_gem_flush(dev, 0, obj->write_domain);
+			(void)i915_add_request(dev, file_priv, obj->write_domain, obj_priv->ring);
+		}
+
+		/* Update the active list for the hardware's current position.
+		 * Otherwise this only updates on a delayed timer or when irqs
+		 * are actually unmasked, and our working set ends up being
+		 * larger than required.
+		 */
+		i915_gem_retire_requests_ring(dev, obj_priv->ring);
+
+		args->busy = obj_priv->active;
+	}
 
 	drm_gem_object_unreference(obj);
 	mutex_unlock(&dev->struct_mutex);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915: report all active objects as busy
  2010-08-04 14:36 [PATCH] drm/i915: report all active objects as busy Chris Wilson
@ 2010-08-04 18:57 ` Daniel Vetter
  2010-08-04 19:11   ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2010-08-04 18:57 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Aug 04, 2010 at 03:36:30PM +0100, Chris Wilson wrote:
> Incorporates a similar patch by Daniel Vetter, the alteration being to
> report the current busy state after retiring.
Woot, nice idea to exactly preserve the semantics of the old
implementation.

/me bangs the head against the wall for not coming up with this myself

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc : Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem.c |   40 +++++++++++++++++++++++++-------------
>  1 files changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index f599d77..909e727 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4163,22 +4163,34 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  	}
>  
>  	mutex_lock(&dev->struct_mutex);
> -	/* Update the active list for the hardware's current position.
> -	 * Otherwise this only updates on a delayed timer or when irqs are
> -	 * actually unmasked, and our working set ends up being larger than
> -	 * required.
> -	 */
> -	i915_gem_retire_requests(dev);
>  
> -	obj_priv = to_intel_bo(obj);
> -	/* Don't count being on the flushing list against the object being
> -	 * done.  Otherwise, a buffer left on the flushing list but not getting
> -	 * flushed (because nobody's flushing that domain) won't ever return
> -	 * unbusy and get reused by libdrm's bo cache.  The other expected
> -	 * consumer of this interface, OpenGL's occlusion queries, also specs
> -	 * that the objects get unbusy "eventually" without any interference.
> +	/* Count all active objects as busy, even if they are currently not used
> +	 * by the gpu. Users of this interface expect objects to eventually
> +	 * become non-busy without any further actions, therefore emit any
> +	 * necessary flushes here.
>  	 */
> -	args->busy = obj_priv->active && obj_priv->last_rendering_seqno != 0;
> +	obj_priv = to_intel_bo(obj);
> +	args->busy = obj_priv->active;
> +	if (args->busy) {
> +		/* Unconditionally flush objects, even when the gpu still uses this
> +		 * object. Userspace calling this function indicates that it wants to
> +		 * use this buffer rather sooner than later, so issuing the required
> +		 * flush earlier is beneficial.
> +		 */
> +		if (obj->write_domain) {
> +			i915_gem_flush(dev, 0, obj->write_domain);
> +			(void)i915_add_request(dev, file_priv, obj->write_domain, obj_priv->ring);
> +		}
> +
> +		/* Update the active list for the hardware's current position.
> +		 * Otherwise this only updates on a delayed timer or when irqs
> +		 * are actually unmasked, and our working set ends up being
> +		 * larger than required.
> +		 */
> +		i915_gem_retire_requests_ring(dev, obj_priv->ring);
> +
> +		args->busy = obj_priv->active;
> +	}
>  
>  	drm_gem_object_unreference(obj);
>  	mutex_unlock(&dev->struct_mutex);
> -- 
> 1.7.1
> 
> _______________________________________________
> 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] 4+ messages in thread

* Re: [PATCH] drm/i915: report all active objects as busy
  2010-08-04 18:57 ` Daniel Vetter
@ 2010-08-04 19:11   ` Daniel Vetter
  2010-08-06 21:45     ` Eric Anholt
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2010-08-04 19:11 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Aug 04, 2010 at 08:57:26PM +0200, Daniel Vetter wrote:
> On Wed, Aug 04, 2010 at 03:36:30PM +0100, Chris Wilson wrote:
> > Incorporates a similar patch by Daniel Vetter, the alteration being to
> > report the current busy state after retiring.
> Woot, nice idea to exactly preserve the semantics of the old
> implementation.
> 
> /me bangs the head against the wall for not coming up with this myself
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Chris thought that given how much his patch looks like mine, a s-o-b is
more appropriated:

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915: report all active objects as busy
  2010-08-04 19:11   ` Daniel Vetter
@ 2010-08-06 21:45     ` Eric Anholt
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Anholt @ 2010-08-06 21:45 UTC (permalink / raw)
  To: Daniel Vetter, Chris Wilson; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 852 bytes --]

On Wed, 4 Aug 2010 21:11:13 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Aug 04, 2010 at 08:57:26PM +0200, Daniel Vetter wrote:
> > On Wed, Aug 04, 2010 at 03:36:30PM +0100, Chris Wilson wrote:
> > > Incorporates a similar patch by Daniel Vetter, the alteration being to
> > > report the current busy state after retiring.
> > Woot, nice idea to exactly preserve the semantics of the old
> > implementation.
> > 
> > /me bangs the head against the wall for not coming up with this myself
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Chris thought that given how much his patch looks like mine, a s-o-b is
> more appropriated:
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Very nice, I suspect this will resolve the issues I think I was seeing
with the last version.  Applied to for-linus.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-08-06 21:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-04 14:36 [PATCH] drm/i915: report all active objects as busy Chris Wilson
2010-08-04 18:57 ` Daniel Vetter
2010-08-04 19:11   ` Daniel Vetter
2010-08-06 21:45     ` Eric Anholt

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.