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

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

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.

> +/**
> + * drm_arm_vblank_event - arm vblanke event after pageflip
> + * @crtc: the source CRTC of the vblank event

Ye olde vblanke event? ;) (typo, same in the previous comment)

The function name in this comment doesn't match the name of the function
it describes.

-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel