Re: [Intel-gfx] [PATCH v2] drm/i915: Do not use ggtt_view with (aliasing) PPGTT

2015-03-16 Thread Joonas Lahtinen 
On to, 2015-03-12 at 12:21 +, Tvrtko Ursulin wrote:
 Hi,
 
 Much more likeable in general, some comments inline:
 
 On 03/12/2015 11:10 AM, Joonas Lahtinen wrote:
  GGTT views are only applicable when dealing with GGTT. Change the code to
  reject ggtt_view where it should not be used and require it when it should
  be.
 
  v2:
  - Dropped _ppgtt_ infixes, allow both types to be passed
  - Disregard other but normal views when no view is specified
  - More checks that valid parameters are passed
  - More readable error checking
 
  Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
  ---
drivers/gpu/drm/i915/i915_drv.h | 102 +-
drivers/gpu/drm/i915/i915_gem.c | 169 
  +++-
drivers/gpu/drm/i915/i915_gem_gtt.c |  67 ++
3 files changed, 219 insertions(+), 119 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index a80b15f..8b7d4f9 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -2620,20 +2620,16 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
#define PIN_GLOBAL 0x4
#define PIN_OFFSET_BIAS 0x8
#define PIN_OFFSET_MASK (~4095)
  -int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj,
  - struct i915_address_space *vm,
  - uint32_t alignment,
  - uint64_t flags,
  - const struct i915_ggtt_view *view);
  -static inline
  -int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj,
  -struct i915_address_space *vm,
  -uint32_t alignment,
  -uint64_t flags)
  -{
  -   return i915_gem_object_pin_view(obj, vm, alignment, flags,
  -   i915_ggtt_view_normal);
  -}
  +int __must_check
  +i915_gem_object_pin(struct drm_i915_gem_object *obj,
  +   struct i915_address_space *vm,
  +   uint32_t alignment,
  +   uint64_t flags);
  +int __must_check
  +i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
  +const struct i915_ggtt_view *view,
  +uint32_t alignment,
  +uint64_t flags);
 
int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
u32 flags);
  @@ -2797,60 +2793,48 @@ struct dma_buf *i915_gem_prime_export(struct 
  drm_device *dev,
 
void i915_gem_restore_fences(struct drm_device *dev);
 
  -unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o,
  -  struct i915_address_space *vm,
  -  enum i915_ggtt_view_type view);
  -static inline
  -unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o,
  - struct i915_address_space *vm)
  +static inline bool i915_is_ggtt(struct i915_address_space *vm);
  +
  +unsigned long
  +i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
  + enum i915_ggtt_view_type view);
  +unsigned long
  +i915_gem_obj_offset(struct drm_i915_gem_object *o,
  +   struct i915_address_space *vm);
  +static inline unsigned long
  +i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
{
  -   return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL);
  +   return i915_gem_obj_ggtt_offset_view(o, I915_GGTT_VIEW_NORMAL);
}
  +
bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o);
  -bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o,
  -struct i915_address_space *vm,
  -enum i915_ggtt_view_type view);
  -static inline
  +bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,
  + enum i915_ggtt_view_type view);
bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
  -   struct i915_address_space *vm)
  -{
  -   return i915_gem_obj_bound_view(o, vm, I915_GGTT_VIEW_NORMAL);
  -}
  +   struct i915_address_space *vm);
 
unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
  struct i915_address_space *vm);
  -struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj,
  - struct i915_address_space *vm,
  - const struct i915_ggtt_view *view);
  -static inline
  -struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
  -struct i915_address_space *vm)
  -{
  -   return i915_gem_obj_to_vma_view(obj, vm, i915_ggtt_view_normal);
  -}
  -
struct i915_vma *
  -i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj,
  -  struct

Re: [Intel-gfx] [PATCH v2] drm/i915: Do not use ggtt_view with (aliasing) PPGTT

2015-03-12 Thread Tvrtko Ursulin 


Hi,

Much more likeable in general, some comments inline:

On 03/12/2015 11:10 AM, Joonas Lahtinen wrote:

GGTT views are only applicable when dealing with GGTT. Change the code to
reject ggtt_view where it should not be used and require it when it should
be.

v2:
- Dropped _ppgtt_ infixes, allow both types to be passed
- Disregard other but normal views when no view is specified
- More checks that valid parameters are passed
- More readable error checking

Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
---
  drivers/gpu/drm/i915/i915_drv.h | 102 +-
  drivers/gpu/drm/i915/i915_gem.c | 169 +++-
  drivers/gpu/drm/i915/i915_gem_gtt.c |  67 ++
  3 files changed, 219 insertions(+), 119 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a80b15f..8b7d4f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2620,20 +2620,16 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
  #define PIN_GLOBAL 0x4
  #define PIN_OFFSET_BIAS 0x8
  #define PIN_OFFSET_MASK (~4095)
-int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm,
- uint32_t alignment,
- uint64_t flags,
- const struct i915_ggtt_view *view);
-static inline
-int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm,
-uint32_t alignment,
-uint64_t flags)
-{
-   return i915_gem_object_pin_view(obj, vm, alignment, flags,
-   i915_ggtt_view_normal);
-}
+int __must_check
+i915_gem_object_pin(struct drm_i915_gem_object *obj,
+   struct i915_address_space *vm,
+   uint32_t alignment,
+   uint64_t flags);
+int __must_check
+i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
+const struct i915_ggtt_view *view,
+uint32_t alignment,
+uint64_t flags);

  int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
  u32 flags);
@@ -2797,60 +2793,48 @@ struct dma_buf *i915_gem_prime_export(struct drm_device 
*dev,

  void i915_gem_restore_fences(struct drm_device *dev);

-unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o,
-  struct i915_address_space *vm,
-  enum i915_ggtt_view_type view);
-static inline
-unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o,
- struct i915_address_space *vm)
+static inline bool i915_is_ggtt(struct i915_address_space *vm);
+
+unsigned long
+i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
+ enum i915_ggtt_view_type view);
+unsigned long
+i915_gem_obj_offset(struct drm_i915_gem_object *o,
+   struct i915_address_space *vm);
+static inline unsigned long
+i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
  {
-   return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL);
+   return i915_gem_obj_ggtt_offset_view(o, I915_GGTT_VIEW_NORMAL);
  }
+
  bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o);
-bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o,
-struct i915_address_space *vm,
-enum i915_ggtt_view_type view);
-static inline
+bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,
+ enum i915_ggtt_view_type view);
  bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
-   struct i915_address_space *vm)
-{
-   return i915_gem_obj_bound_view(o, vm, I915_GGTT_VIEW_NORMAL);
-}
+   struct i915_address_space *vm);

  unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
struct i915_address_space *vm);
-struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm,
- const struct i915_ggtt_view *view);
-static inline
-struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm)
-{
-   return i915_gem_obj_to_vma_view(obj, vm, i915_ggtt_view_normal);
-}
-
  struct i915_vma *
-i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj,
-  struct i915_address_space *vm,
-  const struct i915_ggtt_view *view);
+i915_gem_obj_to_vma(struct