[Intel-gfx] [PATCH v2] drm/i915: pin sprite fb only if it changed
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
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
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