On Wed, Jul 30, 2014 at 12:44:05PM +0200, Daniel Vetter wrote: > 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. > > Cc: Michel Thierry <michel.thierry@xxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/i915_gem.c | 2 +- > drivers/gpu/drm/i915/i915_gem_context.c | 35 ++++----------------------------- > drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ++++++++++++++-- > drivers/gpu/drm/i915/i915_gem_gtt.h | 12 ++++++++++- > 5 files changed, 31 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 5fe3b1dfc34c..a89eb87e4af6 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2497,7 +2497,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 25a32b9c9b4b..1a46be5d2979 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4511,7 +4511,7 @@ void i915_gem_vma_destroy(struct i915_vma *vma) > ppgtt = vm_to_ppgtt(vm); > > if (ppgtt) > - kref_put(&ppgtt->ref, ppgtt_release); > + i915_ppgtt_put(ppgtt); Move the if(ppgtt) into i915_ppgtt_put(). And trust in the gcc DCE. For get, do if (ppgtt)...; return ppgtt. And trust gcc. Doing that can make callers much neater. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx