On Tue, Nov 26, 2013 at 11:23:15AM +0000, 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 (1<<31)
> +#define  __EXEC_OBJECT_HAS_FENCE (1<<30)
> +
>  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 (1<<31)
> -#define  __EXEC_OBJECT_HAS_FENCE (1<<30)
> -
>  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

Reply via email to