| Message ID | 1410361397-20744-1-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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> > --- > 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
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
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);