From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2] drm/i915: pin sprite fb only if it changed Date: Wed, 10 Sep 2014 18:56:37 +0200 Message-ID: <20140910165637.GW15520@phenom.ffwll.local> References: <1410361397-20744-1-git-send-email-gustavo@padovan.org> <20140910162653.GV4193@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-we0-f174.google.com (mail-we0-f174.google.com [74.125.82.174]) by gabe.freedesktop.org (Postfix) with ESMTP id 9C1AD89CCB for ; Wed, 10 Sep 2014 09:56:14 -0700 (PDT) Received: by mail-we0-f174.google.com with SMTP id t60so4511497wes.33 for ; Wed, 10 Sep 2014 09:56:13 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140910162653.GV4193@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org, Gustavo Padovan , dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Sep 10, 2014 at 07:26:53PM +0300, Ville Syrj=E4l=E4 wrote: > On Wed, Sep 10, 2014 at 12:03:17PM -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan > > = > > 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 !=3D obj to an upper if condition > > = > > Signed-off-by: Gustavo Padovan > = > lgtm > = > Reviewed-by: Ville Syrj=E4l=E4 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 *pla= ne, > > primary_enabled =3D !drm_rect_equals(dst, clip) || colorkey_enabled(i= ntel_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 =3D 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 !=3D 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 =3D intel_pin_and_fence_fb_obj(dev, obj, NULL); > > + if (ret =3D=3D 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 =3D state->orig_dst.x1; > > intel_plane->crtc_y =3D state->orig_dst.y1; > > @@ -1099,14 +1102,15 @@ intel_commit_sprite_plane(struct drm_plane *pla= ne, > > } > > = > > /* Unpin old obj after new one is active to avoid ugliness */ > > - if (old_obj) { > > + if (old_obj && old_obj !=3D 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 !=3D 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=E4l=E4 > Intel OTC > _______________________________________________ > dri-devel mailing list > dri-devel@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