[PATCH v2] drm/i915: pin sprite fb only if it changed

[Daniel Vetter]

On Wed, Sep 10, 2014 at 07:26:53PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 10, 2014 at 12:03:17PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> > 
> > Optimize code avoiding helding dev mutex if old fb and current fb
> > are the same.
> > 
> > v2: take Ville's comments
> > 	- move comment along with the pin_and_fence call
> > 	- check for error before calling i915_gem_track_fb
> > 	- move old_obj != obj to an upper if condition
> > 
> > Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> 
> lgtm
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 34 +++++++++++++++++++---------------
> >  1 file changed, 19 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index a4306cf..90bb45f 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1038,21 +1038,24 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> >  	primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
> >  	WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);
> >  
> > -	mutex_lock(&dev->struct_mutex);
> > -
> > -	/* Note that this will apply the VT-d workaround for scanouts,
> > -	 * which is more restrictive than required for sprites. (The
> > -	 * primary plane requires 256KiB alignment with 64 PTE padding,
> > -	 * the sprite planes only require 128KiB alignment and 32 PTE padding.
> > -	 */
> > -	ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
> >  
> > -	i915_gem_track_fb(old_obj, obj,
> > -			  INTEL_FRONTBUFFER_SPRITE(pipe));
> > -	mutex_unlock(&dev->struct_mutex);
> > +	if (old_obj != obj) {
> > +		mutex_lock(&dev->struct_mutex);
> >  
> > -	if (ret)
> > -		return ret;
> > +		/* Note that this will apply the VT-d workaround for scanouts,
> > +		 * which is more restrictive than required for sprites. (The
> > +		 * primary plane requires 256KiB alignment with 64 PTE padding,
> > +		 * the sprite planes only require 128KiB alignment and 32 PTE
> > +		 * padding.
> > +		 */
> > +		ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
> > +		if (ret == 0)
> > +			i915_gem_track_fb(old_obj, obj,
> > +					  INTEL_FRONTBUFFER_SPRITE(pipe));
> > +		mutex_unlock(&dev->struct_mutex);
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	intel_plane->crtc_x = state->orig_dst.x1;
> >  	intel_plane->crtc_y = state->orig_dst.y1;
> > @@ -1099,14 +1102,15 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> >  	}
> >  
> >  	/* Unpin old obj after new one is active to avoid ugliness */
> > -	if (old_obj) {
> > +	if (old_obj && old_obj != obj) {
> > +
> >  		/*
> >  		 * It's fairly common to simply update the position of
> >  		 * an existing object.  In that case, we don't need to
> >  		 * wait for vblank to avoid ugliness, we only need to
> >  		 * do the pin & ref bookkeeping.
> >  		 */
> > -		if (old_obj != obj && intel_crtc->active)
> > +		if (intel_crtc->active)
> >  			intel_wait_for_vblank(dev, intel_crtc->pipe);
> >  
> >  		mutex_lock(&dev->struct_mutex);
> > -- 
> > 1.9.3
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch