Re: [Intel-gfx] [PATCH 17/17] drm/i915: Move i915_get_ggtt_vma_pages into ggtt_bind_vma

[Daniel Vetter]

On Tue, Apr 21, 2015 at 04:36:55PM +0300, Mika Kuoppala wrote:
 Daniel Vetter daniel.vet...@ffwll.ch writes:
 
  We have this neat abstraction between ppgtt and ggtt for (un)bind_vma
  and didn't end up using it really. What a shame, so fix this and make
  the -bind_vma hook a bit more useful.
 
  Signed-off-by: Daniel Vetter daniel.vet...@intel.com
 
 This one I had problems with applying. But it looks simple enough.
 
 Patches 13-17,
 Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com
 
 With 12/17 I don't yet understand all the implications so I left
 that out.

Thanks for the review, remaining patches except patch 12 pulled in.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 17/17] drm/i915: Move i915_get_ggtt_vma_pages into ggtt_bind_vma

[Daniel Vetter]

We have this neat abstraction between ppgtt and ggtt for (un)bind_vma
and didn't end up using it really. What a shame, so fix this and make
the -bind_vma hook a bit more useful.

Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 41 ++---
 drivers/gpu/drm/i915/i915_gem_gtt.h |  6 +++---
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 458819b99a0b..763c17dd2118 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -92,6 +92,9 @@
  *
  */
 
+static int
+i915_get_ggtt_vma_pages(struct i915_vma *vma);
+
 const struct i915_ggtt_view i915_ggtt_view_normal;
 const struct i915_ggtt_view i915_ggtt_view_rotated = {
 .type = I915_GGTT_VIEW_ROTATED
@@ -143,9 +146,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, 
int enable_ppgtt)
return has_aliasing_ppgtt ? 1 : 0;
 }
 
-static void ppgtt_bind_vma(struct i915_vma *vma,
-  enum i915_cache_level cache_level,
-  u32 unused)
+static int ppgtt_bind_vma(struct i915_vma *vma,
+ enum i915_cache_level cache_level,
+ u32 unused)
 {
u32 pte_flags = 0;
 
@@ -155,6 +158,8 @@ static void ppgtt_bind_vma(struct i915_vma *vma,
 
vma-vm-insert_entries(vma-vm, vma-obj-pages, vma-node.start,
cache_level, pte_flags);
+
+   return 0;
 }
 
 static void ppgtt_unbind_vma(struct i915_vma *vma)
@@ -1898,22 +1903,26 @@ static void i915_ggtt_clear_range(struct 
i915_address_space *vm,
intel_gtt_clear_range(first_entry, num_entries);
 }
 
-static void ggtt_bind_vma(struct i915_vma *vma,
- enum i915_cache_level cache_level,
- u32 flags)
+static int ggtt_bind_vma(struct i915_vma *vma,
+enum i915_cache_level cache_level,
+u32 flags)
 {
struct drm_device *dev = vma-vm-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_i915_gem_object *obj = vma-obj;
struct sg_table *pages = obj-pages;
u32 pte_flags = 0;
+   int ret;
+
+   ret = i915_get_ggtt_vma_pages(vma);
+   if (ret)
+   return ret;
+   pages = vma-ggtt_view.pages;
 
/* Currently applicable only to VLV */
if (obj-gt_ro)
pte_flags |= PTE_READ_ONLY;
 
-   if (i915_is_ggtt(vma-vm))
-   pages = vma-ggtt_view.pages;
 
if (!dev_priv-mm.aliasing_ppgtt || flags  GLOBAL_BIND) {
vma-vm-insert_entries(vma-vm, pages,
@@ -1929,6 +1938,8 @@ static void ggtt_bind_vma(struct i915_vma *vma,
vma-node.start,
cache_level, pte_flags);
}
+
+   return 0;
 }
 
 static void ggtt_unbind_vma(struct i915_vma *vma)
@@ -2749,7 +2760,7 @@ err_st_alloc:
return ERR_PTR(ret);
 }
 
-static inline int
+static int
 i915_get_ggtt_vma_pages(struct i915_vma *vma)
 {
int ret = 0;
@@ -2793,8 +2804,8 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma)
 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
  u32 flags)
 {
+   int ret = 0;
u32 bind_flags = 0;
-   int ret;
 
if (vma-vm-allocate_va_range) {
trace_i915_va_alloc(vma-vm, vma-node.start,
@@ -2808,12 +2819,6 @@ int i915_vma_bind(struct i915_vma *vma, enum 
i915_cache_level cache_level,
return ret;
}
 
-   if (i915_is_ggtt(vma-vm)) {
-   ret = i915_get_ggtt_vma_pages(vma);
-   if (ret)
-   return 0;
-   }
-
if (flags  PIN_GLOBAL)
bind_flags |= GLOBAL_BIND;
if (flags  PIN_EXECBUF)
@@ -2825,7 +2830,9 @@ int i915_vma_bind(struct i915_vma *vma, enum 
i915_cache_level cache_level,
bind_flags = ~vma-bound;
 
if (bind_flags)
-   vma-vm-bind_vma(vma, cache_level, bind_flags);
+   ret = vma-vm-bind_vma(vma, cache_level, bind_flags);
+   if (ret)
+   return ret;
 
vma-bound |= bind_flags;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index fb0a04aa5363..4e6cac575cd8 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -278,9 +278,9 @@ struct i915_address_space {
 * setting the valid PTE entries to a reserved scratch page. */
void (*unbind_vma)(struct i915_vma *vma);
/* Map an object into an address space with the given cache flags. */
-   void (*bind_vma)(struct i915_vma *vma,
-enum i915_cache_level cache_level,
-u32 flags);
+   int (*bind_vma)(struct i915_vma