From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 14/19] drm: Don't update vblank timestamp when the counter didn't change Date: Wed, 6 Aug 2014 15:09:50 +0200 Message-ID: <20140806130950.GI8727@phenom.ffwll.local> References: <1407325803-6944-1-git-send-email-ville.syrjala@linux.intel.com> <1407325803-6944-15-git-send-email-ville.syrjala@linux.intel.com> <20140806125614.GG8727@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20140806125614.GG8727@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Wed, Aug 06, 2014 at 02:56:14PM +0200, Daniel Vetter wrote: > On Wed, Aug 06, 2014 at 02:49:57PM +0300, ville.syrjala@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > If we already have a timestamp for the current vblank counter, don't > > update it with a new timestmap. Small errors can creep in between two > > timestamp queries for the same vblank count, which could be confusing to > > userspace when it queries the timestamp for the same vblank sequence > > number twice. > > = > > This problem gets exposed when the vblank disable timer is not used > > (or is set to expire quickly) and thus we can get multiple vblank > > disable<->enable transition during the same frame which would all > > attempt to update the timestamp with the latest estimate. > > = > > Testcase: igt/kms_flip/flip-vs-expired-vblank > > Signed-off-by: Ville Syrj=E4l=E4 > = > Reviewed-by: Daniel Vetter On second consideration this seems to just paper over drivers that enable vblanks a few too many times. Or a bug in the vblank_get handling in drm_irq.c. If it's just that I think we should just tighten up the checks to catch that and drop this patch here. -Daniel > > --- > > drivers/gpu/drm/drm_irq.c | 3 +++ > > 1 file changed, 3 insertions(+) > > = > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > > index af33df1..0523f5b 100644 > > --- a/drivers/gpu/drm/drm_irq.c > > +++ b/drivers/gpu/drm/drm_irq.c > > @@ -106,6 +106,9 @@ static void drm_update_vblank_count(struct drm_devi= ce *dev, int crtc) > > DRM_DEBUG("enabling vblank interrupts on crtc %d, missed %d\n", > > crtc, diff); > > = > > + if (diff =3D=3D 0) > > + return; > > + > > /* Reinitialize corresponding vblank timestamp if high-precision query > > * available. Skip this step if query unsupported or failed. Will > > * reinitialize delayed at next vblank interrupt in that case. > > -- = > > 1.8.5.5 > > = > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > = > -- = > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch