drm/i915: Some cleanups for the ppgtt lifetime handling

So when reviewing Michel's patch I've noticed a few things and cleaned
them up:
- The early checks in ppgtt_release are now redundant: The inactive
  list should always be empty now, so we can ditch these checks. Even
  for the aliasing ppgtt (though that's a different confusion) since
  we tear that down after all the objects are gone.
- The ppgtt handling functions are splattered all over. Consolidate
  them in i915_gem_gtt.c, give them OCD prefixes and add wrappers for
  get/put.
- There was a bit a confusion in ppgtt_release about whether it cares
  about the active or inactive list. It should care about them both,
  so augment the WARNINGs to check for both.

There's still create_vm_for_ctx left to do, put that is blocked on the
removal of ppgtt->ctx. Once that's done we can rename it to
i915_ppgtt_create and move it to its siblings for handling ppgtts.

v2: Move the ppgtt checks into the inline get/put functions as
suggested by Chris.

v3: Inline the now redundant ppgtt local variable.

Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 38b1849a450a27e4b90ebdb33dea8327f1e6ad07..101fc637eb4677f082311b608957572230c989c9 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2549,7 +2549,6 @@ void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj);
 #define ctx_to_ppgtt(ctx) container_of((ctx)->vm, struct i915_hw_ppgtt, base)
 #define vm_to_ppgtt(vm) container_of(vm, struct i915_hw_ppgtt, base)
 int __must_check i915_gem_context_init(struct drm_device *dev);
-void ppgtt_release(struct kref *kref);
 void i915_gem_context_fini(struct drm_device *dev);
 void i915_gem_context_reset(struct drm_device *dev);
 int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 63aee412b2582c16eba93b56b439d6e0fccbd922..8061d45eaa8017f44b58b59be5ab637554d5226a 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4477,7 +4477,6 @@ struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
 void i915_gem_vma_destroy(struct i915_vma *vma)
 {
        struct i915_address_space *vm = NULL;
-       struct i915_hw_ppgtt *ppgtt = NULL;
        WARN_ON(vma->node.allocated);
 
        /* Keep the vma as a placeholder in the execbuffer reservation lists */
@@ -4485,10 +4484,8 @@ void i915_gem_vma_destroy(struct i915_vma *vma)
                return;
 
        vm = vma->vm;
-       ppgtt = vm_to_ppgtt(vm);
 
-       if (ppgtt)
-               kref_put(&ppgtt->ref, ppgtt_release);
+       i915_ppgtt_put(vm_to_ppgtt(vm));
 
        list_del(&vma->vma_link);
 

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a509a4bca99165cfca5f975a04ac173625052329..dc43d0263a01e625e80893b3584c56ae247c7bdf 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -96,33 +96,6 @@
 #define GEN6_CONTEXT_ALIGN (64<<10)
 #define GEN7_CONTEXT_ALIGN 4096
 
-static void do_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt)
-{
-       struct drm_device *dev = ppgtt->base.dev;
-       struct drm_i915_private *dev_priv = dev->dev_private;
-       struct i915_address_space *vm = &ppgtt->base;
-
-       if (ppgtt == dev_priv->mm.aliasing_ppgtt ||
-           (list_empty(&vm->active_list) && list_empty(&vm->inactive_list))) {
-               ppgtt->base.cleanup(&ppgtt->base);
-               return;
-       }
-
-       /* vmas should already be unbound */
-       WARN_ON(!list_empty(&vm->active_list));
-
-       ppgtt->base.cleanup(&ppgtt->base);
-}
-
-void ppgtt_release(struct kref *kref)
-{
-       struct i915_hw_ppgtt *ppgtt =
-               container_of(kref, struct i915_hw_ppgtt, ref);
-
-       do_ppgtt_cleanup(ppgtt);
-       kfree(ppgtt);
-}
-
 static size_t get_context_alignment(struct drm_device *dev)
 {
        if (IS_GEN6(dev))
@@ -174,8 +147,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
                        ppgtt = ctx_to_ppgtt(ctx);
        }
 
-       if (ppgtt)
-               kref_put(&ppgtt->ref, ppgtt_release);
+       i915_ppgtt_put(ppgtt);
        if (ctx->legacy_hw_ctx.rcs_state)
                drm_gem_object_unreference(&ctx->legacy_hw_ctx.rcs_state->base);
        list_del(&ctx->link);
@@ -222,7 +194,7 @@ create_vm_for_ctx(struct drm_device *dev, struct intel_context *ctx)
        if (!ppgtt)
                return ERR_PTR(-ENOMEM);
 
-       ret = i915_gem_init_ppgtt(dev, ppgtt);
+       ret = i915_ppgtt_init(dev, ppgtt);
        if (ret) {
                kfree(ppgtt);
                return ERR_PTR(ret);
@@ -234,7 +206,7 @@ create_vm_for_ctx(struct drm_device *dev, struct intel_context *ctx)
 
 static struct intel_context *
 __create_hw_context(struct drm_device *dev,
-                 struct drm_i915_file_private *file_priv)
+                   struct drm_i915_file_private *file_priv)
 {
        struct drm_i915_private *dev_priv = dev->dev_private;
        struct intel_context *ctx;
@@ -342,7 +314,7 @@ i915_gem_create_context(struct drm_device *dev,
                /* For platforms which only have aliasing PPGTT, we fake the
                 * address space and refcounting. */
                ctx->vm = &dev_priv->mm.aliasing_ppgtt->base;
-               kref_get(&dev_priv->mm.aliasing_ppgtt->ref);
+               i915_ppgtt_get(dev_priv->mm.aliasing_ppgtt);
        } else
                ctx->vm = &dev_priv->gtt.base;
 

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 76dd3b42848313b06d3308fca29d1eb2ee0ea9be..6ffa12b72538fbf5238655af21fdc9abd13c7c45 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1180,7 +1180,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
        return 0;
 }
 
-int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
+int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 {
        struct drm_i915_private *dev_priv = dev->dev_private;
        int ret = 0;
@@ -1211,6 +1211,19 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
        return ret;
 }
 
+void  i915_ppgtt_release(struct kref *kref)
+{
+       struct i915_hw_ppgtt *ppgtt =
+               container_of(kref, struct i915_hw_ppgtt, ref);
+
+       /* vmas should already be unbound */
+       WARN_ON(!list_empty(&ppgtt->base.active_list));
+       WARN_ON(!list_empty(&ppgtt->base.inactive_list));
+
+       ppgtt->base.cleanup(&ppgtt->base);
+       kfree(ppgtt);
+}
+
 static void
 ppgtt_bind_vma(struct i915_vma *vma,
               enum i915_cache_level cache_level,
@@ -2148,15 +2161,12 @@ i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
                                  struct i915_address_space *vm)
 {
        struct i915_vma *vma;
-       struct i915_hw_ppgtt *ppgtt = NULL;
 
        vma = i915_gem_obj_to_vma(obj, vm);
        if (!vma)
                vma = __i915_gem_vma_create(obj, vm);
 
-       ppgtt = vm_to_ppgtt(vm);
-       if (ppgtt)
-               kref_get(&ppgtt->ref);
+       i915_ppgtt_get(vm_to_ppgtt(vm));
 
        return vma;
 }

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 666c938a51e37329cb0a2e1bd25a6c21b7b6d38e..c6beb528f955c775106f8abf0a4add4beaab0117 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -272,7 +272,19 @@ void i915_gem_init_global_gtt(struct drm_device *dev);
 void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
                               unsigned long mappable_end, unsigned long end);
 
-int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt);
+
+int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt);
+void i915_ppgtt_release(struct kref *kref);
+static inline void i915_ppgtt_get(struct i915_hw_ppgtt *ppgtt)
+{
+       if (ppgtt)
+               kref_get(&ppgtt->ref);
+}
+static inline void i915_ppgtt_put(struct i915_hw_ppgtt *ppgtt)
+{
+       if (ppgtt)
+               kref_put(&ppgtt->ref, i915_ppgtt_release);
+}
 
 void i915_check_and_clear_faults(struct drm_device *dev);
 void i915_gem_suspend_gtt_mappings(struct drm_device *dev);