Re: [PATCH] drm/i915: restore ggtt double-bind avoidance

[Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>]

On Wed, May 06, 2015 at 05:18:01PM +0200, Daniel Vetter wrote:
> This was accidentally lost in
> 
> commit 75d04a3773ecee617847de963ae4195d6aa74c28
> Author: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Date:   Tue Apr 28 17:56:17 2015 +0300
> 
>     drm/i915/gtt: Allocate va range only if vma is not bound
> 
> While at it implement an improved version suggested by Chris which
> avoids the double-bind irrespective of what type of bind is done
> first.
> 
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8fee6789fae2..11a3b511ae64 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1928,6 +1928,9 @@ static int ggtt_bind_vma(struct i915_vma *vma,
>  					    cache_level, pte_flags);
>  	}
>  
> +	if (!dev_priv->mm.aliasing_ppgtt)
> +		vma->bound |= GLOBAL_BIND | LOCAL_BIND;
> +
>  	return 0;
>  }

Meh, it wasn't quite what I had in mind. If we do it there, we may as
well go whole hog and do the vma->bound manipulation for both ggtt and
ppgtt in the bind_vma()

Something like:

ickle@crystalwell:/usr/src/linux$ vim drivers/gpu/drm/i915/i915_gem_gtt.c 
ickle@crystalwell:/usr/src/linux$ git diff
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem
index 698a30b..59114af 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -148,7 +148,7 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int
 
 static int ppgtt_bind_vma(struct i915_vma *vma,
                          enum i915_cache_level cache_level,
-                         u32 unused)
+                         u32 bind_flags)
 {
        u32 pte_flags = 0;
 
@@ -159,6 +159,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
        vma->vm->insert_entries(vma->vm, vma->obj->pages, vma->node.start,
                                cache_level, pte_flags);
 
+       vma->bound |= bind_flags;
        return 0;
 }
 
@@ -1956,6 +1957,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
                vma->vm->insert_entries(vma->vm, pages,
:
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 698a30b..59114af 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -148,7 +148,7 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
 
 static int ppgtt_bind_vma(struct i915_vma *vma,
                          enum i915_cache_level cache_level,
-                         u32 unused)
+                         u32 bind_flags)
 {
        u32 pte_flags = 0;
 
@@ -159,6 +159,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
        vma->vm->insert_entries(vma->vm, vma->obj->pages, vma->node.start,
                                cache_level, pte_flags);
 
+       vma->bound |= bind_flags;
        return 0;
 }
 
@@ -1956,6 +1957,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
                vma->vm->insert_entries(vma->vm, pages,
                                        vma->node.start,
                                        cache_level, pte_flags);
+               flags |= GLOBAL_BIND;
        }
 
        if (dev_priv->mm.aliasing_ppgtt && flags & LOCAL_BIND) {
@@ -1965,6 +1967,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
                                            cache_level, pte_flags);
        }
 
+       vma->bound |= flags;
        return 0;
 }
 
@@ -2945,10 +2948,5 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
        }
 
        trace_i915_vma_bind(vma, bind_flags);
-       ret = vma->vm->bind_vma(vma, cache_level, bind_flags);
-       if (ret)
-               return ret;
-
-       vma->bound |= bind_flags;
-       return 0;
+       return vma->vm->bind_vma(vma, cache_level, bind_flags);
 }


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