[Intel-gfx] [PATCH 1/2] drm/i915: Some cleanups for the ppgtt lifetime handling

[Chris Wilson]

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 at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  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