Re: [Intel-gfx] [PATCH] drm/i915: Pin relocations for the duration of constructing the execbuffer

2013-11-28 Thread Barbalho, Rafael 
 On Tue, Nov 26, 2013 at 11:23:15AM +, Chris Wilson wrote:
  As the execbuffer dispatch grows ever more complex and involves
  multiple stages of moving objects into the aperture, we need to take
  greater care that we do not evict our execbuffer objects prior to
  dispatch. This is relatively simple as we can just keep the objects
  pinned for not just the relocation but until we are finished.
 
 One such example is the possibility of the context switch causing an eviction
 or hitting the shrinker in order to fit its object into the aperture.
 
 Link: http://lists.freedesktop.org/archives/intel-gfx/2013-
 November/036166.html
 Reported-by: Siluvery, Arun arun.siluv...@intel.com
 

After a backport to the 3.10 tree and running the soak test for 25 hours:
Tested-by: Rafael Barbalho rafael.barba...@intel.com

  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Ben Widawsky benjamin.widaw...@intel.com
  Cc: Daniel Vetter dan...@ffwll.ch
  Cc: sta...@vger.kernel.org
 
 --
 Chris Wilson, Intel Open Source Technology Centre
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Pin relocations for the duration of constructing the execbuffer

2013-11-27 Thread Daniel Vetter 
On Tue, Nov 26, 2013 at 11:23:50PM -0800, Ben Widawsky wrote:
 On Tue, Nov 26, 2013 at 11:23:15AM +, Chris Wilson wrote:
  As the execbuffer dispatch grows ever more complex and involves multiple
  stages of moving objects into the aperture, we need to take greater care
  that we do not evict our execbuffer objects prior to dispatch. This is
  relatively simple as we can just keep the objects pinned for not just
  the relocation but until we are finished.
  
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Ben Widawsky benjamin.widaw...@intel.com
  Cc: Daniel Vetter dan...@ffwll.ch
  Cc: sta...@vger.kernel.org
 
 To be honest, I don't quite see the actual issue, but the problem
 description, and solution make sense to me. I've also been running with
 the patch quite a bit on HSW.
 
 Acked-by: Ben Widawsky b...@bwidawsk.net
 Tested-by: Ben Widawsky b...@bwidawsk.net

Picked up for -fixes, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Pin relocations for the duration of constructing the execbuffer

2013-11-26 Thread Chris Wilson 
As the execbuffer dispatch grows ever more complex and involves multiple
stages of moving objects into the aperture, we need to take greater care
that we do not evict our execbuffer objects prior to dispatch. This is
relatively simple as we can just keep the objects pinned for not just
the relocation but until we are finished.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky benjamin.widaw...@intel.com
Cc: Daniel Vetter dan...@ffwll.ch
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 60 --
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 885d595e0e02..b7e787fb4649 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -33,6 +33,9 @@
 #include intel_drv.h
 #include linux/dma_remapping.h
 
+#define  __EXEC_OBJECT_HAS_PIN (131)
+#define  __EXEC_OBJECT_HAS_FENCE (130)
+
 struct eb_vmas {
struct list_head vmas;
int and;
@@ -187,7 +190,28 @@ static struct i915_vma *eb_get_vma(struct eb_vmas *eb, 
unsigned long handle)
}
 }
 
-static void eb_destroy(struct eb_vmas *eb) {
+static void
+i915_gem_execbuffer_unreserve_vma(struct i915_vma *vma)
+{
+   struct drm_i915_gem_exec_object2 *entry;
+   struct drm_i915_gem_object *obj = vma-obj;
+
+   if (!drm_mm_node_allocated(vma-node))
+   return;
+
+   entry = vma-exec_entry;
+
+   if (entry-flags  __EXEC_OBJECT_HAS_FENCE)
+   i915_gem_object_unpin_fence(obj);
+
+   if (entry-flags  __EXEC_OBJECT_HAS_PIN)
+   i915_gem_object_unpin(obj);
+
+   entry-flags = ~(__EXEC_OBJECT_HAS_FENCE | __EXEC_OBJECT_HAS_PIN);
+}
+
+static void eb_destroy(struct eb_vmas *eb)
+{
while (!list_empty(eb-vmas)) {
struct i915_vma *vma;
 
@@ -195,6 +219,7 @@ static void eb_destroy(struct eb_vmas *eb) {
   struct i915_vma,
   exec_list);
list_del_init(vma-exec_list);
+   i915_gem_execbuffer_unreserve_vma(vma);
drm_gem_object_unreference(vma-obj-base);
}
kfree(eb);
@@ -478,9 +503,6 @@ i915_gem_execbuffer_relocate(struct eb_vmas *eb,
return ret;
 }
 
-#define  __EXEC_OBJECT_HAS_PIN (131)
-#define  __EXEC_OBJECT_HAS_FENCE (130)
-
 static int
 need_reloc_mappable(struct i915_vma *vma)
 {
@@ -552,26 +574,6 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
return 0;
 }
 
-static void
-i915_gem_execbuffer_unreserve_vma(struct i915_vma *vma)
-{
-   struct drm_i915_gem_exec_object2 *entry;
-   struct drm_i915_gem_object *obj = vma-obj;
-
-   if (!drm_mm_node_allocated(vma-node))
-   return;
-
-   entry = vma-exec_entry;
-
-   if (entry-flags  __EXEC_OBJECT_HAS_FENCE)
-   i915_gem_object_unpin_fence(obj);
-
-   if (entry-flags  __EXEC_OBJECT_HAS_PIN)
-   i915_gem_object_unpin(obj);
-
-   entry-flags = ~(__EXEC_OBJECT_HAS_FENCE | __EXEC_OBJECT_HAS_PIN);
-}
-
 static int
 i915_gem_execbuffer_reserve(struct intel_ring_buffer *ring,
struct list_head *vmas,
@@ -670,13 +672,14 @@ i915_gem_execbuffer_reserve(struct intel_ring_buffer 
*ring,
goto err;
}
 
-err:   /* Decrement pin count for bound objects */
-   list_for_each_entry(vma, vmas, exec_list)
-   i915_gem_execbuffer_unreserve_vma(vma);
-
+err:
if (ret != -ENOSPC || retry++)
return ret;
 
+   /* Decrement pin count for bound objects */
+   list_for_each_entry(vma, vmas, exec_list)
+   i915_gem_execbuffer_unreserve_vma(vma);
+
ret = i915_gem_evict_vm(vm, true);
if (ret)
return ret;
@@ -708,6 +711,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
while (!list_empty(eb-vmas)) {
vma = list_first_entry(eb-vmas, struct i915_vma, exec_list);
list_del_init(vma-exec_list);
+   i915_gem_execbuffer_unreserve_vma(vma);
drm_gem_object_unreference(vma-obj-base);
}
 
-- 
1.8.4.4

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

Re: [Intel-gfx] [PATCH] drm/i915: Pin relocations for the duration of constructing the execbuffer

2013-11-26 Thread Chris Wilson 
On Tue, Nov 26, 2013 at 11:23:15AM +, Chris Wilson wrote:
 As the execbuffer dispatch grows ever more complex and involves multiple
 stages of moving objects into the aperture, we need to take greater care
 that we do not evict our execbuffer objects prior to dispatch. This is
 relatively simple as we can just keep the objects pinned for not just
 the relocation but until we are finished.

One such example is the possibility of the context switch causing an
eviction or hitting the shrinker in order to fit its object into the
aperture.

Link: http://lists.freedesktop.org/archives/intel-gfx/2013-November/036166.html
Reported-by: Siluvery, Arun arun.siluv...@intel.com

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ben Widawsky benjamin.widaw...@intel.com
 Cc: Daniel Vetter dan...@ffwll.ch
 Cc: sta...@vger.kernel.org

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Pin relocations for the duration of constructing the execbuffer

2013-11-26 Thread Ben Widawsky 
On Tue, Nov 26, 2013 at 11:23:15AM +, Chris Wilson wrote:
 As the execbuffer dispatch grows ever more complex and involves multiple
 stages of moving objects into the aperture, we need to take greater care
 that we do not evict our execbuffer objects prior to dispatch. This is
 relatively simple as we can just keep the objects pinned for not just
 the relocation but until we are finished.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ben Widawsky benjamin.widaw...@intel.com
 Cc: Daniel Vetter dan...@ffwll.ch
 Cc: sta...@vger.kernel.org

To be honest, I don't quite see the actual issue, but the problem
description, and solution make sense to me. I've also been running with
the patch quite a bit on HSW.

Acked-by: Ben Widawsky b...@bwidawsk.net
Tested-by: Ben Widawsky b...@bwidawsk.net

 ---
  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 60 
 --
  1 file changed, 32 insertions(+), 28 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
 b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 index 885d595e0e02..b7e787fb4649 100644
 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 @@ -33,6 +33,9 @@
  #include intel_drv.h
  #include linux/dma_remapping.h
  
 +#define  __EXEC_OBJECT_HAS_PIN (131)
 +#define  __EXEC_OBJECT_HAS_FENCE (130)
 +
  struct eb_vmas {
   struct list_head vmas;
   int and;
 @@ -187,7 +190,28 @@ static struct i915_vma *eb_get_vma(struct eb_vmas *eb, 
 unsigned long handle)
   }
  }
  
 -static void eb_destroy(struct eb_vmas *eb) {
 +static void
 +i915_gem_execbuffer_unreserve_vma(struct i915_vma *vma)
 +{
 + struct drm_i915_gem_exec_object2 *entry;
 + struct drm_i915_gem_object *obj = vma-obj;
 +
 + if (!drm_mm_node_allocated(vma-node))
 + return;
 +
 + entry = vma-exec_entry;
 +
 + if (entry-flags  __EXEC_OBJECT_HAS_FENCE)
 + i915_gem_object_unpin_fence(obj);
 +
 + if (entry-flags  __EXEC_OBJECT_HAS_PIN)
 + i915_gem_object_unpin(obj);
 +
 + entry-flags = ~(__EXEC_OBJECT_HAS_FENCE | __EXEC_OBJECT_HAS_PIN);
 +}
 +
 +static void eb_destroy(struct eb_vmas *eb)
 +{
   while (!list_empty(eb-vmas)) {
   struct i915_vma *vma;
  
 @@ -195,6 +219,7 @@ static void eb_destroy(struct eb_vmas *eb) {
  struct i915_vma,
  exec_list);
   list_del_init(vma-exec_list);
 + i915_gem_execbuffer_unreserve_vma(vma);
   drm_gem_object_unreference(vma-obj-base);
   }
   kfree(eb);
 @@ -478,9 +503,6 @@ i915_gem_execbuffer_relocate(struct eb_vmas *eb,
   return ret;
  }
  
 -#define  __EXEC_OBJECT_HAS_PIN (131)
 -#define  __EXEC_OBJECT_HAS_FENCE (130)
 -
  static int
  need_reloc_mappable(struct i915_vma *vma)
  {
 @@ -552,26 +574,6 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
   return 0;
  }
  
 -static void
 -i915_gem_execbuffer_unreserve_vma(struct i915_vma *vma)
 -{
 - struct drm_i915_gem_exec_object2 *entry;
 - struct drm_i915_gem_object *obj = vma-obj;
 -
 - if (!drm_mm_node_allocated(vma-node))
 - return;
 -
 - entry = vma-exec_entry;
 -
 - if (entry-flags  __EXEC_OBJECT_HAS_FENCE)
 - i915_gem_object_unpin_fence(obj);
 -
 - if (entry-flags  __EXEC_OBJECT_HAS_PIN)
 - i915_gem_object_unpin(obj);
 -
 - entry-flags = ~(__EXEC_OBJECT_HAS_FENCE | __EXEC_OBJECT_HAS_PIN);
 -}
 -
  static int
  i915_gem_execbuffer_reserve(struct intel_ring_buffer *ring,
   struct list_head *vmas,
 @@ -670,13 +672,14 @@ i915_gem_execbuffer_reserve(struct intel_ring_buffer 
 *ring,
   goto err;
   }
  
 -err: /* Decrement pin count for bound objects */
 - list_for_each_entry(vma, vmas, exec_list)
 - i915_gem_execbuffer_unreserve_vma(vma);
 -
 +err:
   if (ret != -ENOSPC || retry++)
   return ret;
  
 + /* Decrement pin count for bound objects */
 + list_for_each_entry(vma, vmas, exec_list)
 + i915_gem_execbuffer_unreserve_vma(vma);
 +
   ret = i915_gem_evict_vm(vm, true);
   if (ret)
   return ret;
 @@ -708,6 +711,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
   while (!list_empty(eb-vmas)) {
   vma = list_first_entry(eb-vmas, struct i915_vma, exec_list);
   list_del_init(vma-exec_list);
 + i915_gem_execbuffer_unreserve_vma(vma);
   drm_gem_object_unreference(vma-obj-base);
   }
  
 -- 
 1.8.4.4
 

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx