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

[Mario Kleiner <mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 11/10/2015 05:00 PM, Thierry Reding wrote:
> On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote:
>> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> 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.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
>> Cc: Thierry Reding <treding@nvidia.com>
>> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
>> Cc: Ben Skeggs <bskeggs@redhat.com>
>> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
>>
>> v2 (mario): Integrate my own review comments into Daniels patch.
>>     - Fix function prototypes in drmP.h
>>     - Add missing vblank_put() for pageflip completion without
>>       pageflip event.
>>     - Initialize sequence number for queued pageflip event to avoidng
>>       trouble in drm_handle_vblank_events().
>>     - Remove dead code and spelling fix.
>>
>> v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice.
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> (v1) Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> (v2/v3) Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>>
>> Cc: stable@vger.kernel.org # v4.3
>> ---
>>   drivers/gpu/drm/drm_irq.c                 | 54 ++++++++++++++++++++++++++++++-
>>   drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++++++-----
>>   include/drm/drmP.h                        |  4 +++
>>   3 files changed, 68 insertions(+), 9 deletions(-)
>
> This looks good to me. Let me clean this up a little and submit it to
> Dave.
>
> Thierry
>

Btw., if somebody has a functional old card for testing this, it should 
be easy to verify if it works on pre-nv50. If it would not work it would 
deliver the pageflip event 1 frame delayed, so at least on standard 
nouveau + default DRI2 + default double-buffering the rate for a tight 
loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, 
quite noticeable. Afaik we also have Piglit tests for OML_sync_control 
which would likely fail if this would be broken.

Oh and if someone has tips on how to resurrect an old nv-40 PC (booted 
with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind 
hearing them. It would be nice to still be able to use that card for 
testing.

thanks,
-mario
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau