On Tue, Sep 09, 2014 at 05:49:46PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
>
> Optimize code avoiding helding dev mutex if old fb and current fb
> are the same.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_sprite.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index a4306cf..a301838 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1038,21 +1038,23 @@ 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.
> */
The comment should probably remain just before the
intel_pin_and_fence_fb_obj() call.
> - ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
> + if (old_obj != obj) {
> + mutex_lock(&dev->struct_mutex);
> + 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);
> + i915_gem_track_fb(old_obj, obj,
> + INTEL_FRONTBUFFER_SPRITE(pipe));
I don't think we should be calling this when things failed. This bug was
already present in the code, but might as well fix it while you're at
it.
> + mutex_unlock(&dev->struct_mutex);
>
> - if (ret)
> - return ret;
> + if (ret)
> + return ret;
> + }
>
> intel_plane->crtc_x = state->orig_dst.x1;
> intel_plane->crtc_y = state->orig_dst.y1;
> @@ -1098,15 +1100,18 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> intel_post_enable_primary(crtc);
> }
>
> + if (!old_obj)
> + return 0;
> +
> /* Unpin old obj after new one is active to avoid ugliness */
> - if (old_obj) {
> + if (old_obj != obj) {
I'd prefer
if (old_obj && old_obj != obj) {
in this case.
> /*
> * 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
