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@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@collabora.co.uk

lgtm

Reviewed-by: Ville Syrjälä ville.syrjala@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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel