[Intel-gfx] [PATCH] drm/i915: Track pinned vma in intel_plane_state

[Chris Wilson]

On Wed, Jan 04, 2017 at 05:06:46PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 04, 2017 at 02:31:26PM +0100, Maarten Lankhorst wrote:
> > From: Chris Wilson <chris at chris-wilson.co.uk>
> > 
> > With atomic plane states we are able to track an allocation right from
> > preparation, during use and through to the final free after being
> > swapped out for a new plane. We can couple the VMA we pin for the
> > framebuffer (and its rotation) to this lifetime and avoid all the clumsy
> > lookups in between.
> > 
> > v2: Remove residual vma on plane cleanup (Chris)
> > v3: Add a description for the vma destruction in
> >     intel_plane_destroy_state (Maarten)
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h           |  16 +---
> >  drivers/gpu/drm/i915/intel_atomic_plane.c |  20 +++++
> >  drivers/gpu/drm/i915/intel_display.c      | 129 +++++++++++-------------------
> >  drivers/gpu/drm/i915/intel_drv.h          |   9 ++-
> >  drivers/gpu/drm/i915/intel_fbc.c          |  52 +++++-------
> >  drivers/gpu/drm/i915/intel_fbdev.c        |   4 +-
> >  drivers/gpu/drm/i915/intel_sprite.c       |   8 +-
> >  7 files changed, 103 insertions(+), 135 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 22d3f610212c..5369f5f9ce3a 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1079,6 +1079,8 @@ struct intel_fbc {
> >  	struct work_struct underrun_work;
> >  
> >  	struct intel_fbc_state_cache {
> > +		struct i915_vma *vma;
> > +
> >  		struct {
> >  			unsigned int mode_flags;
> >  			uint32_t hsw_bdw_pixel_rate;
> > @@ -1092,15 +1094,14 @@ struct intel_fbc {
> >  		} plane;
> >  
> >  		struct {
> > -			u64 ilk_ggtt_offset;
> >  			const struct drm_format_info *format;
> >  			unsigned int stride;
> > -			int fence_reg;
> > -			unsigned int tiling_mode;
> >  		} fb;
> >  	} state_cache;
> >  
> >  	struct intel_fbc_reg_params {
> > +		struct i915_vma *vma;
> > +
> >  		struct {
> >  			enum pipe pipe;
> >  			enum plane plane;
> > @@ -1108,10 +1109,8 @@ struct intel_fbc {
> >  		} crtc;
> >  
> >  		struct {
> > -			u64 ggtt_offset;
> >  			const struct drm_format_info *format;
> >  			unsigned int stride;
> > -			int fence_reg;
> >  		} fb;
> >  
> >  		int cfb_size;
> > @@ -3406,13 +3405,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj,
> >  	return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view);
> >  }
> >  
> > -static inline unsigned long
> > -i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o,
> > -			    const struct i915_ggtt_view *view)
> > -{
> > -	return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view));
> > -}
> > -
> >  /* i915_gem_fence_reg.c */
> >  int __must_check i915_vma_get_fence(struct i915_vma *vma);
> >  int __must_check i915_vma_put_fence(struct i915_vma *vma);
> > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > index 4612ffd555a7..41fd94e62d3c 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > @@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane)
> >  
> >  	__drm_atomic_helper_plane_duplicate_state(plane, state);
> >  
> > +	intel_state->vma = NULL;
> 
> Shouldn't we be doing vma_get() instead?

I went with NULL (dropping the copy) for simplicity. Before the plane
can be used it must be prepared, so vma will always be set before
commiting, and using NULL avoided having the reference counting dance.
It also allowed detection of when we didn't prepare the plane as
required.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre