From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mario Kleiner Subject: Re: [PATCH] drm/i915: Fix pre-CTG vblank counter Date: Thu, 26 Sep 2013 19:32:58 +0200 Message-ID: <52446FCA.2030406@gmail.com> References: <1380128126-4457-1-git-send-email-ville.syrjala@linux.intel.com> <20130925205536.GQ12663@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f48.google.com (mail-ee0-f48.google.com [74.125.83.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 7C664E6402 for ; Thu, 26 Sep 2013 10:32:58 -0700 (PDT) Received: by mail-ee0-f48.google.com with SMTP id l10so703349eei.21 for ; Thu, 26 Sep 2013 10:32:57 -0700 (PDT) In-Reply-To: <20130925205536.GQ12663@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, Mario Kleiner List-Id: intel-gfx@lists.freedesktop.org On 25.09.13 22:55, Daniel Vetter wrote: > On Wed, Sep 25, 2013 at 07:55:26PM +0300, ville.syrjala@linux.intel.com w= rote: >> From: Ville Syrj=E4l=E4 >> >> The old style frame counter increments at the start of active video. >> However for i915_get_vblank_counter() we want a counter that increments >> at the start of vblank. >> >> Fortunately the low frame counter register also contains the pixel >> counter for the current frame. We can can compare that against the >> vblank start pixel count to determine if we need to increment the >> frame counter by 1 to get the correct answer. >> >> Also reorganize the function pointer assignments in intel_irq_init() a >> bit to avoid confusing people. >> >> Cc: Mario Kleiner >> Signed-off-by: Ville Syrj=E4l=E4 >> --- >> >> Just another small vblank related gem I forgot to polish up and send >> out until Imre started asking me questions about the vblank counter >> functions. > > Hm, I've thought the magic fixup code does take care of that for us? But I > agree that we should do this explicitly ... > > This could explain some of the strange vblank timestamp off failures QA > has reported (if there's too much delay and the fixup doesn't fire any > more), care to attach this patch to the relevant bug reports? Searching > for ts jitter + pre-gen5 should be good enough. > -Daniel > The vblank off code has one known race in that if = dev->driver->get_vblank_counter does not increment its counter at start = of vblank, it can have off-by-one vblank. There's a comment in that = function about that, e.g., http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_irq.c#L102 The magic code there only fixes races between the vblank off code and = the vblank irq handler. Normally that race would not really affect = applications as long as the vblank_off_delay is as large (5 secs) as it = is now. But in QA testing, if you exercise that code not the way a = normal app would do it, i'd expect you to see that error about 4% of the = time. The only fix is to fixup the vblank counter in the kms driver, so this = patch looks like a perfect solution to that. You can add my reviewed-by = if you wish. -mario