From patchwork Thu Dec 4 23:56:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Anholt X-Patchwork-Id: 5440891 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 3AE979F30B for ; Thu, 4 Dec 2014 23:56:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3686220266 for ; Thu, 4 Dec 2014 23:56:13 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 2C05F200ED for ; Thu, 4 Dec 2014 23:56:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8B8AC6E271; Thu, 4 Dec 2014 15:56:11 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from annarchy.freedesktop.org (annarchy.freedesktop.org [131.252.210.176]) by gabe.freedesktop.org (Postfix) with ESMTP id 15D456E220; Thu, 4 Dec 2014 15:56:10 -0800 (PST) Received: from eliezer.anholt.net (annarchy.freedesktop.org [127.0.0.1]) by annarchy.freedesktop.org (Postfix) with ESMTP id 033EE180C5; Thu, 4 Dec 2014 15:56:10 -0800 (PST) Received: by eliezer.anholt.net (Postfix, from userid 1000) id 70685F02ADF; Thu, 4 Dec 2014 15:56:08 -0800 (PST) From: Eric Anholt To: Mario Kleiner , keithp@keithp.com, axel.davy@ens.fr, mario.kleiner.de@gmail.com, jamey@minilop.net, chris@chris-wilson.co.uk, skeggsb@gmail.com In-Reply-To: <1417547329-9352-4-git-send-email-mario.kleiner.de@gmail.com> References: <1417547329-9352-1-git-send-email-mario.kleiner.de@gmail.com> <1417547329-9352-4-git-send-email-mario.kleiner.de@gmail.com> User-Agent: Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Thu, 04 Dec 2014 15:56:08 -0800 Message-ID: <871tofkm07.fsf@eliezer.anholt.net> MIME-Version: 1.0 Cc: maarten.lankhorst@canonical.com, intel-gfx@lists.freedesktop.org, xorg-devel@lists.x.org, Theo0x48@gmail.com, jcristau@debian.org Subject: Re: [Intel-gfx] [PATCH 2/2] present: Fix use of vsynced pageflips and honor PresentOptionAsync. (v3) X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Mario Kleiner writes: > Pageflips for Pixmap presents were not synchronized to vblank on > drivers with support for PresentCapabilityAsync, due to some > missing init for vblank->sync_flips. The PresentOptionAsync > flag was completely ignored for pageflipped presents. > > Vsynced flips only worked by accident on the intel-ddx, as that > driver doesn't have PresentCapabilityAsync support. > > On nouveau-ddx, which supports PresentCapabilityAsync, this > always caused non-vsynced pageflips with pretty ugly tearing. > > This patch fixes the problem, as tested on top of XOrg 1.16.2 > on nouveau and intel. > > Please also apply to XOrg 1.17 and XOrg 1.16.2 stable. > > Applying on top of XOrg 1.16.2 may require cherry-picking > commit 2051514652481a83bd7cf22e57cb0fcd40333f33 > which trivially fixes lack of support for protocol option > PresentOptionCopy - get two bug fixes for the price of one! > > Signed-off-by: Mario Kleiner > --- > present/present.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/present/present.c b/present/present.c > index e5d3fd5..be1c9f1 100644 > --- a/present/present.c > +++ b/present/present.c > @@ -834,7 +834,7 @@ present_pixmap(WindowPtr window, > vblank->notifies = notifies; > vblank->num_notifies = num_notifies; > > - if (!screen_priv->info || !(screen_priv->info->capabilities & PresentCapabilityAsync)) > + if (!(options & PresentOptionAsync)) > vblank->sync_flip = TRUE; I think I'd like to see a hunk like this in with this patch, so that each driver doesn't need to have the cap check: Seem reasonable? If you wanted to squash this in, then this is: Reviewed-by: Eric Anholt (So's patch 1/2, regardless). diff --git a/present/present.c b/present/present.c index a9f2214..ed0d734 100644 --- a/present/present.c +++ b/present/present.c @@ -838,6 +838,9 @@ present_pixmap(WindowPtr window, vblank->sync_flip = TRUE; if (!(options & PresentOptionCopy) && + !((options & PresentOptionAsync) && + (!screen_priv->info || + !(screen_priv->info->capabilities & PresentCapabilityAsync))) && pixmap != NULL && present_check_flip (target_crtc, window, pixmap, vblank->sync_flip, valid, x_off, y_off)) {