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

2014-09-10 Thread Gustavo Padovan 
From: Gustavo Padovan gustavo.pado...@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.pado...@collabora.co.uk
---
 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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: pin sprite fb only if it changed

2014-09-10 Thread Ville Syrjälä 
On Wed, Sep 10, 2014 at 12:03:17PM -0300, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@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.pado...@collabora.co.uk

lgtm

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

 ---
  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-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: pin sprite fb only if it changed

2014-09-10 Thread 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.pado...@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.pado...@collabora.co.uk
 
 lgtm
 
 Reviewed-by: Ville Syrjälä ville.syrj...@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-de...@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/dri-devel
 
 -- 
 Ville Syrjälä
 Intel OTC
 ___
 dri-devel mailing list
 dri-de...@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx