Re: [Intel-gfx] [PATCH] drm/i915: report all active objects as busy

2010-08-06 Thread Eric Anholt
On Wed, 4 Aug 2010 21:11:13 +0200, Daniel Vetter dan...@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.vet...@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.vet...@ffwll.ch

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



pgp3tSNXUNoSd.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: report all active objects as busy

2010-08-04 Thread Chris Wilson
Incorporates a similar patch by Daniel Vetter, the alteration being to
report the current busy state after retiring.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc : Daniel Vetter daniel.vet...@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


Re: [Intel-gfx] [PATCH] drm/i915: report all active objects as busy

2010-08-04 Thread Daniel Vetter
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.vet...@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.vet...@ffwll.ch
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: report all active objects as busy v2

2010-04-23 Thread Daniel Vetter
Reporting objects that are not currently used by the gpu but are
dirty and not yet flushed creates havoc to libdrm's bo reuse:
The usually immediatly following gtt_map afterwards will stall
until the gpu has executed the writeback.

v2: Fix refcount leak and missing unlock in the error path. Spotted
by Chris Wilson.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_gem.c |   30 +-
 1 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 023f4db..eedc456 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4379,6 +4379,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_busy *args = data;
struct drm_gem_object *obj;
struct drm_i915_gem_object *obj_priv;
+   int ret = 0;
 
obj = drm_gem_object_lookup(dev, file_priv, args-handle);
if (obj == NULL) {
@@ -4396,18 +4397,29 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
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.
-*/
-   args-busy = obj_priv-active  obj_priv-last_rendering_seqno != 0;
+
+   /* 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;
+
+   /* 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) {
+   uint32_t seqno;
+
+   i915_gem_flush(dev, 0, obj-write_domain);
+   seqno = i915_add_request(dev, file_priv, obj-write_domain);
+   if (seqno == 0)
+   ret = -ENOMEM;
+   }
 
drm_gem_object_unreference(obj);
mutex_unlock(dev-struct_mutex);
-   return 0;
+   return ret;
 }
 
 int
-- 
1.6.6.1

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