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 <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
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 <ben@xxxxxxxxxxxx>
Tested-by: Ben Widawsky <ben@xxxxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html