Re: [PATCH] drm/nouveau: Fix pre-nv50 pageflip events

From: Daniel Vetter <daniel@ffwll.ch>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	Ben Skeggs <bskeggs@redhat.com>,
	Nouveau Dev <nouveau@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Thierry Reding <treding@nvidia.com>
Subject: Re: [PATCH] drm/nouveau: Fix pre-nv50 pageflip events
Date: Mon, 16 Nov 2015 16:11:00 +0100	[thread overview]
Message-ID: <20151116151100.GU16848@phenom.ffwll.local> (raw)
In-Reply-To: <5637147C.6010802@daenzer.net>

On Mon, Nov 02, 2015 at 04:45:00PM +0900, Michel Dänzer wrote:
> On 31.10.2015 06:55, Daniel Vetter wrote:
> > Apparently pre-nv50 pageflip events happen before the actual vblank
> > period. Therefore that functionality got semi-disabled in
> > 
> > commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
> > Author: Mario Kleiner <mario.kleiner.de@gmail.com>
> > Date:   Tue May 13 00:42:08 2014 +0200
> > 
> >     drm/nouveau/kms/nv04-nv40: fix pageflip events via special case.
> > 
> > Unfortunately that hack got uprooted in
> > 
> > commit cc1ef118fc099295ae6aabbacc8af94d8d8885eb
> > Author: Thierry Reding <treding@nvidia.com>
> > Date:   Wed Aug 12 17:00:31 2015 +0200
> > 
> >     drm/irq: Make pipe unsigned and name consistent
> > 
> > Trigering a warning when trying to sample the vblank timestamp for a
> > non-existing pipe. There's a few ways to fix this:
> > 
> > - Open-code the old behaviour, which just enshrines this slight
> >   breakage of the userspace ABI.
> > 
> > - Revert Mario's commit and again inflict broken timestamps, again not
> >   pretty.
> > 
> > - Fix this for real by delaying the pageflip TS until the next vblank
> >   interrupt, thereby making it accurate.
> > 
> > This patch implements the third option. Since having a page flip
> > interrupt that happens when the pageflip gets armed and not when it
> > completes in the next vblank seems to be fairly common (older i915 hw
> > works very similarly) create a new helper to arm vblank events for
> > such drivers.
> 
> What happens when the page flip interrupt arrives during a vertical
> blank period?  Presumably the userspace event will be deferred until the
> next vertical blank period, but the flip might already take effect in
> the current one.

Hm yeah there's a tiny race if your update handler for the pageflip can
race with your vblank handler. That's impossible here since it's all done
from the same hw irq hanlder, and since that is single-threaded there
shouldn't be a problem, as long as vblank handling are pageflip are
ordered correctly.

Might be worth a note in the kerneldoc though that this function isn't
perfectly foolproof.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel