Nouveau - Feb 2012 - [Patches][nouveau/ddx]: Improvements to bufferswap implementation and timestamping

Hi,

here a set of patches against the nouveau-ddx. This is an extended and
revised set, based on Francisco Jerez feedback from autumn last year.

[1/9] Makes pageflipping work again on X-Server 1.12rc. It apparently stopped
working somewhere around Xorg 1.11+.

[2/9] Implements handling of pageflip completion events from the kernel.
Francisco Jerez argument against including it was that the x-server didn't
have a swaplimit api, so this couldn't be applied at the same time as the
pseudo triple-buffering hack which is in place in the current ddx.
Now we have the swaplimit api in 1.12, so this problem should be solved.

[3/9] Makes use of the swaplimit api. A new xorg.conf option
"SwapLimit"
allows to select a swaplimit of 1 or 2 for double-buffering or
triple-buffering. It defaults to 2 for Xorg 1.12+ and 1 for older servers.
This way, on 1.12+ nouveau retains the kind of triple buffering behaviour it
currently has, but swap completion timestamping (OML_sync_control,
INTEL_swap_events, and client swap throttling) works conforming to
the specs. On older servers it removes triple-buffering but makes
nouveau conform to the specs. This is important for apps that need
precise and reliable swap scheduling and timestamping.

[4/9] A bug fix against corrupted desktop when switching from redirected
to non-redirected fullscreen windows under a desktop compositor.
Fixes FDO bug #35452.

[5/9] Some fixes to swap scheduling, revised according to Francisco's
review.

[6/9] Fixes swap throttling for non-fullscreen windows under a desktop
compositor. Split into a separate patch according to Francisco's
feedback.

[7/9] An attempt to provide more sane swap completion events for non-
fullscreen windows. This is a bit of cheating, as it delivers the
events for the earliest point in time one would expect the swap
to complete, assuming only a lightly loaded gpu. Real completion
could be later. I think this is an "improvement" because the current
implementation delivers swap complete events with timestamps and
counts that signal swap completion before the client even requested
the swap.

[8/9] This one adds Francisco's original triple-buffering hack back
for Xorg 1.11 and earlier servers if the xorg.conf option requests
a swaplimit > 1. Users can choose between "triple-buffering" but
broken timestamping or double-buffering with sane timestamping.

[9/9] Fixes a corner case which could cause the ddx to segfault
with its current triple-buffering implementation.

I've tested these on single-display and dual-display setups,
with/without compositor, with/without redirection and checked
the robustness and precision of the timestamps with special
measurement equipment, on XOrg 1.12-rc2 and 1.10. They work
pretty well for me and finally make nouveau very useable for
the kind of scientific applications that require precise
swap scheduling and timestamping, so i'd love to see them
reviewed and hopefully included into the ddx soon.

A couple of things are a bit of hacks:

[3/9] I think the setup of a default swap limit would be
better done in the x-server itself instead of the ddx. The
setup code is a bit awkward, hijacking the ddx->CreateBuffer
function to apply the swaplimit. A DRI2GetSwapLimit() function
is also missing from the server, which could help in the
future to track swaplimit changes by other clients than the ddx
itself. Unfortunately it is too late for the 1.12 release to do
this.

[8/9] Don't know if this is still wanted/needed or not?

[9/9] It fixes the problem and doesn't affect performance, but is
somewhat of a hack. I don't know how to do better, maybe somebody
else has a better solution?

Thanks,
-mario

can_exchange() fails on at least Xorg 1.12+. This fixes
it in the same way it was fixed in the ati & intel ddx.

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 src/nouveau_dri2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index 3aa5ec5..5b62425 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -160,7 +160,7 @@ can_exchange(DrawablePtr draw, PixmapPtr dst_pix, PixmapPtr
src_pix)
 	return ((DRI2CanFlip(draw) && pNv->has_pageflip)) &&
 		dst_pix->drawable.width == src_pix->drawable.width &&
 		dst_pix->drawable.height == src_pix->drawable.height &&
-		dst_pix->drawable.depth == src_pix->drawable.depth &&
+		dst_pix->drawable.bitsPerPixel == src_pix->drawable.bitsPerPixel
&&
 		dst_pix->devKind == src_pix->devKind;
 }
 
-- 
1.7.5.4

Requests pageflip completion events from the kernel.
Implements pageflip completion handler to finalize
and timestamp swaps.

Completion handler includes a consistency check, and
disambiguation if multiple crtc's are involved in a
pageflip (e.g., clone mode, extendend desktop). Only
the timestamp of the crtc whose vblank event initially
triggered the swap is used, but handler waits for flip
completion on all involved crtc's before completing the
swap and releasing the old framebuffer.

This code is almost identical to the code used in the
ati/radeon ddx and intel ddx.

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 src/drmmode_display.c |  107 +++++++++++++++++++++++++++++++++++++++++++++++--
 src/nouveau_dri2.c    |   89 ++++++++++++++++++++++++++++++++++++++--
 src/nv_proto.h        |    5 ++-
 3 files changed, 191 insertions(+), 10 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 75ef6dd..9e15c29 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -83,6 +83,21 @@ typedef struct {
     drmmode_prop_ptr props;
 } drmmode_output_private_rec, *drmmode_output_private_ptr;
 
+typedef struct {
+    drmmode_ptr drmmode;
+    unsigned old_fb_id;
+    int flip_count;
+    void *event_data;
+    unsigned int fe_frame;
+    unsigned int fe_tv_sec;
+    unsigned int fe_tv_usec;
+} drmmode_flipdata_rec, *drmmode_flipdata_ptr;
+
+typedef struct {
+    drmmode_flipdata_ptr flipdata;
+    Bool dispatch_me;
+} drmmode_flipevtcarrier_rec, *drmmode_flipevtcarrier_ptr;
+
 static void drmmode_output_dpms(xf86OutputPtr output, int mode);
 
 static drmmode_ptr
@@ -1245,13 +1260,17 @@ drmmode_cursor_init(ScreenPtr pScreen)
 }
 
 Bool
-drmmode_page_flip(DrawablePtr draw, PixmapPtr back, void *priv)
+drmmode_page_flip(DrawablePtr draw, PixmapPtr back, void *priv,
+		  unsigned int ref_crtc_hw_id)
 {
 	ScrnInfoPtr scrn = xf86Screens[draw->pScreen->myNum];
 	xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
 	drmmode_crtc_private_ptr crtc = config->crtc[0]->driver_private;
 	drmmode_ptr mode = crtc->drmmode;
 	int ret, i, old_fb_id;
+	int emitted = 0;
+	drmmode_flipdata_ptr flipdata;
+	drmmode_flipevtcarrier_ptr flipcarrier;
 
 	old_fb_id = mode->fb_id;
 	ret = drmModeAddFB(mode->fd, scrn->virtualX, scrn->virtualY,
@@ -1264,24 +1283,64 @@ drmmode_page_flip(DrawablePtr draw, PixmapPtr back, void
*priv)
 		return FALSE;
 	}
 
+	flipdata = calloc(1, sizeof(drmmode_flipdata_rec));
+	if (!flipdata) {
+		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+		"flip queue: data alloc failed.\n");
+		goto error_undo;
+	}
+
+	flipdata->event_data = priv;
+	flipdata->drmmode = mode;
+
 	for (i = 0; i < config->num_crtc; i++) {
 		crtc = config->crtc[i]->driver_private;
 
 		if (!config->crtc[i]->enabled)
 			continue;
 
+		flipdata->flip_count++;
+
+		flipcarrier = calloc(1, sizeof(drmmode_flipevtcarrier_rec));
+		if (!flipcarrier) {
+			xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+				   "flip queue: carrier alloc failed.\n");
+			if (emitted == 0)
+				free(flipdata);
+			goto error_undo;
+		}
+
+		/* Only the reference crtc will finally deliver its page flip
+		 * completion event. All other crtc's events will be discarded.
+		 */
+		flipcarrier->dispatch_me = ((1 << i) == ref_crtc_hw_id);
+		flipcarrier->flipdata = flipdata;
+
 		ret = drmModePageFlip(mode->fd, crtc->mode_crtc->crtc_id,
-				      mode->fb_id, 0, priv);
+				      mode->fb_id, DRM_MODE_PAGE_FLIP_EVENT,
+				      flipcarrier);
 		if (ret) {
 			xf86DrvMsg(scrn->scrnIndex, X_WARNING,
 				   "flip queue failed: %s\n", strerror(errno));
-			return FALSE;
+
+			free(flipcarrier);
+			if (emitted == 0)
+				free(flipdata);
+			goto error_undo;
 		}
+
+		emitted++;
 	}
 
-	drmModeRmFB(mode->fd, old_fb_id);
+	/* Will release old fb after all crtc's completed flip. */
+	flipdata->old_fb_id = old_fb_id;
 
 	return TRUE;
+
+error_undo:
+	drmModeRmFB(mode->fd, mode->fb_id);
+	mode->fb_id = old_fb_id;
+	return FALSE;
 }
 
 #ifdef HAVE_LIBUDEV
@@ -1347,6 +1406,42 @@ drmmode_uevent_fini(ScrnInfoPtr scrn)
 }
 
 static void
+drmmode_flip_handler(int fd, unsigned int frame, unsigned int tv_sec,
+		     unsigned int tv_usec, void *event_data)
+{
+	drmmode_flipevtcarrier_ptr flipcarrier = event_data;
+	drmmode_flipdata_ptr flipdata = flipcarrier->flipdata;
+	drmmode_ptr drmmode = flipdata->drmmode;
+
+	/* Is this the event whose info shall be delivered to higher level? */
+	if (flipcarrier->dispatch_me) {
+		/* Yes: Cache msc, ust for later delivery. */
+		flipdata->fe_frame = frame;
+		flipdata->fe_tv_sec = tv_sec;
+		flipdata->fe_tv_usec = tv_usec;
+	}
+	free(flipcarrier);
+
+	/* Last crtc completed flip? */
+	flipdata->flip_count--;
+	if (flipdata->flip_count > 0)
+		return;
+
+	/* Release framebuffer */
+	drmModeRmFB(drmmode->fd, flipdata->old_fb_id);
+
+	if (flipdata->event_data == NULL) {
+		free(flipdata);
+		return;
+	}
+
+	/* Deliver cached msc, ust from reference crtc to flip event handler */
+	nouveau_dri2_flip_event_handler(flipdata->fe_frame, flipdata->fe_tv_sec,
+					flipdata->fe_tv_usec, flipdata->event_data);
+	free(flipdata);
+}
+
+static void
 drmmode_wakeup_handler(pointer data, int err, pointer p)
 {
 	ScrnInfoPtr scrn = data;
@@ -1376,6 +1471,10 @@ drmmode_screen_init(ScreenPtr pScreen)
 	/* Plug in a vblank event handler */
 	drmmode->event_context.version = DRM_EVENT_CONTEXT_VERSION;
 	drmmode->event_context.vblank_handler = nouveau_dri2_vblank_handler;
+
+	/* Plug in a pageflip completion event handler */
+	drmmode->event_context.page_flip_handler = drmmode_flip_handler;
+
 	AddGeneralSocket(drmmode->fd);
 
 	/* Register a wakeup handler to get informed on DRM events */
diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index 5b62425..acef08a 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -140,6 +140,7 @@ struct nouveau_dri2_vblank_state {
 	DRI2BufferPtr src;
 	DRI2SwapEventPtr func;
 	void *data;
+	unsigned int frame;
 };
 
 static Bool
@@ -225,6 +226,18 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 	REGION_INIT(0, &reg, (&(BoxRec){ 0, 0, draw->width, draw->height
}), 0);
 	REGION_TRANSLATE(0, &reg, draw->x, draw->y);
 
+	/* Main crtc for this drawable shall finally deliver pageflip event. */
+	unsigned int ref_crtc_hw_id = nv_window_belongs_to_crtc(scrn, draw->x,
+								draw->y,
+								draw->width,
+								draw->height);
+
+	/* Whenever first crtc is involved, choose it as reference, as
+	 * its vblank event triggered this swap.
+	 */
+	if (ref_crtc_hw_id & 1)
+		ref_crtc_hw_id = 1;
+
 	/* Throttle on the previous frame before swapping */
 	nouveau_bo_map(dst_bo, NOUVEAU_BO_RD);
 	nouveau_bo_unmap(dst_bo);
@@ -249,7 +262,7 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 
 		if (DRI2CanFlip(draw)) {
 			type = DRI2_FLIP_COMPLETE;
-			ret = drmmode_page_flip(draw, src_pix, s);
+			ret = drmmode_page_flip(draw, src_pix, s, ref_crtc_hw_id);
 			if (!ret)
 				goto out;
 		}
@@ -258,6 +271,10 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 		SWAP(nouveau_pixmap(dst_pix)->bo, nouveau_pixmap(src_pix)->bo);
 
 		DamageRegionProcessPending(draw);
+
+		/* If it is a page flip, finish it in the flip event handler. */
+		if (type == DRI2_FLIP_COMPLETE)
+			return;
 	} else {
 		type = DRI2_BLIT_COMPLETE;
 
@@ -292,7 +309,7 @@ nouveau_dri2_schedule_swap(ClientPtr client, DrawablePtr
draw,
 			   DRI2SwapEventPtr func, void *data)
 {
 	struct nouveau_dri2_vblank_state *s;
-	CARD64 current_msc;
+	CARD64 current_msc, expect_msc;
 	int ret;
 
 	/* Initialize a swap structure */
@@ -301,7 +318,7 @@ nouveau_dri2_schedule_swap(ClientPtr client, DrawablePtr
draw,
 		return FALSE;
 
 	*s = (struct nouveau_dri2_vblank_state)
-		{ SWAP, client, draw->id, dst, src, func, data };
+		{ SWAP, client, draw->id, dst, src, func, data, 0 };
 
 	if (can_sync_to_vblank(draw)) {
 		/* Get current sequence */
@@ -319,10 +336,10 @@ nouveau_dri2_schedule_swap(ClientPtr client, DrawablePtr
draw,
 		ret = nouveau_wait_vblank(draw, DRM_VBLANK_ABSOLUTE |
 					  DRM_VBLANK_EVENT,
 					  max(current_msc, *target_msc - 1),
-					  NULL, NULL, s);
+					  &expect_msc, NULL, s);
 		if (ret)
 			goto fail;
-
+		s->frame = (unsigned int) expect_msc & 0xffffffff;
 	} else {
 		/* We can't/don't want to sync to vblank, just swap. */
 		nouveau_dri2_finish_swap(draw, 0, 0, 0, s);
@@ -426,6 +443,68 @@ nouveau_dri2_vblank_handler(int fd, unsigned int frame,
 	}
 }
 
+void
+nouveau_dri2_flip_event_handler(unsigned int frame, unsigned int tv_sec,
+				unsigned int tv_usec, void *event_data)
+{
+	struct nouveau_dri2_vblank_state *flip = event_data;
+	DrawablePtr draw;
+	ScreenPtr screen;
+	ScrnInfoPtr scrn;
+	int status;
+	PixmapPtr pixmap;
+
+	status = dixLookupDrawable(&draw, flip->draw, serverClient,
+				   M_ANY, DixWriteAccess);
+	if (status != Success) {
+		free(flip);
+		return;
+	}
+
+	screen = draw->pScreen;
+	scrn = xf86Screens[screen->myNum];
+
+	pixmap = screen->GetScreenPixmap(screen);
+	xf86DrvMsgVerb(scrn->scrnIndex, X_INFO, 4,
+		       "%s: flipevent : width %d x height %d : msc %d : ust =
%d.%06d\n",
+		       __func__, pixmap->drawable.width, pixmap->drawable.height,
+		       frame, tv_sec, tv_usec);
+
+	/* We assume our flips arrive in order, so we don't check the frame */
+	switch (flip->action) {
+	case SWAP:
+		/* Check for too small vblank count of pageflip completion,
+		 * taking wraparound into account. This usually means some
+		 * defective kms pageflip completion, causing wrong (msc, ust)
+		 * return values and possible visual corruption.
+		 * Skip test for frame == 0, as this is a valid constant value
+		 * reported by all Linux kernels at least up to Linux 3.0.
+		 */
+		if ((frame != 0) &&
+		    (frame < flip->frame) && (flip->frame - frame < 5)) {
+			xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+				   "%s: Pageflip has impossible msc %d < target_msc %d\n",
+				   __func__, frame, flip->frame);
+			/* All-Zero values signal failure of (msc, ust)
+			 * timestamping to client.
+			 */
+			frame = tv_sec = tv_usec = 0;
+		}
+
+		DRI2SwapComplete(flip->client, draw, frame, tv_sec, tv_usec,
+				 DRI2_FLIP_COMPLETE, flip->func,
+				 flip->data);
+		break;
+	default:
+		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+			   "%s: unknown vblank event received\n", __func__);
+		/* Unknown type */
+		break;
+	}
+
+	free(flip);
+}
+
 Bool
 nouveau_dri2_init(ScreenPtr pScreen)
 {
diff --git a/src/nv_proto.h b/src/nv_proto.h
index 0b8e513..8bf2fc1 100644
--- a/src/nv_proto.h
+++ b/src/nv_proto.h
@@ -7,7 +7,8 @@ void drmmode_adjust_frame(ScrnInfoPtr pScrn, int x, int y, int
flags);
 void drmmode_remove_fb(ScrnInfoPtr pScrn);
 Bool drmmode_cursor_init(ScreenPtr pScreen);
 void drmmode_fbcon_copy(ScreenPtr pScreen);
-Bool drmmode_page_flip(DrawablePtr draw, PixmapPtr back, void *priv);
+Bool drmmode_page_flip(DrawablePtr draw, PixmapPtr back, void *priv,
+		       unsigned int ref_crtc_hw_id);
 void drmmode_screen_init(ScreenPtr pScreen);
 void drmmode_screen_fini(ScreenPtr pScreen);
 
@@ -26,6 +27,8 @@ Bool nouveau_allocate_surface(ScrnInfoPtr scrn, int width, int
height,
 void nouveau_dri2_vblank_handler(int fd, unsigned int frame,
 				 unsigned int tv_sec, unsigned int tv_usec,
 				 void *event_data);
+void nouveau_dri2_flip_event_handler(unsigned int frame, unsigned int tv_sec,
+				     unsigned int tv_usec, void *event_data);
 Bool nouveau_dri2_init(ScreenPtr pScreen);
 void nouveau_dri2_fini(ScreenPtr pScreen);
 
-- 
1.7.5.4

Uses the new DRI2SwapLimit() API of X-Server 1.12+
to allow to change the maximum number of pending
swaps on a drawable before the OpenGL client is
throttled by the server.

The new optional xorg.conf parameter "SwapLimit"
allows to select a new swap limit >= 1. The default
swap limit is 2 for triple-buffering on XOrg 1.12+,
1 for double-buffering on older servers, as we can't
change the swap limit there.

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 man/nouveau.man    |   11 +++++++++++
 src/nouveau_dri2.c |   29 +++++++++++++++++++++++++++--
 src/nv_const.h     |    2 ++
 src/nv_driver.c    |   34 ++++++++++++++++++++++++++++++++++
 src/nv_type.h      |    3 +++
 5 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/man/nouveau.man b/man/nouveau.man
index dd9d938..59f6c1a 100644
--- a/man/nouveau.man
+++ b/man/nouveau.man
@@ -93,6 +93,17 @@ will assign xrandr outputs LVDS and VGA-0 to this instance of
the driver.
 .TP
 .BI "Option \*qPageFlip\*q \*q" boolean \*q
 Enable DRI2 page flipping. Default: on.
+.TP
+.BI "Option \*qSwapLimit\*q \*q" integer \*q
+Set maximum allowed number of pending OpenGL double-buffer swaps for
+a drawable before a client is blocked.
+.br
+A value of 1 corresponds to double-buffering. A value of 2 corresponds
+to triple-buffering. Higher values may allow higher framerate, but also
+increase lag for interactive applications, e.g., games. Nouveau currently
+supports a maximum value of 2 on XOrg 1.12+ and a maximum of 1 on older
servers.
+.br
+Default: 2 for XOrg 1.12+, 1 for older servers.
 .SH "SEE ALSO"
 __xservername__(__appmansuffix__), __xconfigfile__(__filemansuffix__),
Xserver(__appmansuffix__), X(__miscmansuffix__)
 .SH AUTHORS
diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index acef08a..2908e56 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -42,6 +42,11 @@ nouveau_dri2_create_buffer(DrawablePtr pDraw, unsigned int
attachment,
 		} else {
 			WindowPtr pwin = (WindowPtr)pDraw;
 			ppix = pScreen->GetWindowPixmap(pwin);
+
+#if DRI2INFOREC_VERSION >= 6
+			/* Set initial swap limit on drawable. */
+			DRI2SwapLimit(pDraw, pNv->swap_limit);
+#endif
 		}
 
 		ppix->refcnt++;
@@ -208,6 +213,20 @@ nouveau_wait_vblank(DrawablePtr draw, int type, CARD64 msc,
 	return 0;
 }
 
+#if DRI2INFOREC_VERSION >= 6
+static Bool
+nouveau_dri2_swap_limit_validate(DrawablePtr draw, int swap_limit)
+{
+	ScrnInfoPtr scrn = xf86Screens[draw->pScreen->myNum];
+	NVPtr pNv = NVPTR(scrn);
+
+	if ((swap_limit < 1 ) || (swap_limit > pNv->max_swap_limit))
+		return FALSE;
+
+	return TRUE;
+}
+#endif
+
 static void
 nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int frame,
 			 unsigned int tv_sec, unsigned int tv_usec,
@@ -293,8 +312,10 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 	 * not, to prevent it from blocking the client on the next
 	 * GetBuffers request (and let the client do triple-buffering).
 	 *
-	 * XXX - The DRI2SwapLimit() API will allow us to move this to
-	 *	 the flip handler with no FPS hit.
+	 * XXX - The DRI2SwapLimit() API allowed us to move this to
+	 *	 the flip handler with no FPS hit for page flipped swaps.
+	 *       It is still needed for copy swaps as we lack a method
+	 *       to detect true swap completion for DRI2_BLIT_COMPLETE.
 	 */
 	DRI2SwapComplete(s->client, draw, frame, tv_sec, tv_usec,
 			 type, s->func, s->data);
@@ -534,6 +555,10 @@ nouveau_dri2_init(ScreenPtr pScreen)
 	dri2.ScheduleWaitMSC = nouveau_dri2_schedule_wait;
 	dri2.GetMSC = nouveau_dri2_get_msc;
 
+#if DRI2INFOREC_VERSION >= 6
+	dri2.SwapLimitValidate = nouveau_dri2_swap_limit_validate;
+#endif
+
 	return DRI2ScreenInit(pScreen, &dri2);
 }
 
diff --git a/src/nv_const.h b/src/nv_const.h
index a27a951..5c232d4 100644
--- a/src/nv_const.h
+++ b/src/nv_const.h
@@ -15,6 +15,7 @@ typedef enum {
     OPTION_GLX_VBLANK,
     OPTION_ZAPHOD_HEADS,
     OPTION_PAGE_FLIP,
+    OPTION_SWAP_LIMIT,
 } NVOpts;
 
 
@@ -28,6 +29,7 @@ static const OptionInfoRec NVOptions[] = {
     { OPTION_GLX_VBLANK,	"GLXVBlank",	OPTV_BOOLEAN,	{0}, FALSE },
     { OPTION_ZAPHOD_HEADS,	"ZaphodHeads",	OPTV_STRING,	{0}, FALSE },
     { OPTION_PAGE_FLIP,		"PageFlip",	OPTV_BOOLEAN,	{0}, FALSE },
+    { OPTION_SWAP_LIMIT,	"SwapLimit",	OPTV_INTEGER,	{0}, FALSE },
     { -1,                       NULL,           OPTV_NONE,      {0}, FALSE }
 };
 
diff --git a/src/nv_driver.c b/src/nv_driver.c
index 87ef2c4..5def531 100644
--- a/src/nv_driver.c
+++ b/src/nv_driver.c
@@ -31,6 +31,9 @@
 #include "xf86drm.h"
 #include "xf86drmMode.h"
 #include "nouveau_drm.h"
+#ifdef DRI2
+#include "dri2.h"
+#endif
 
 /*
  * Forward definitions for the functions that make up the driver.
@@ -847,6 +850,37 @@ NVPreInit(ScrnInfoPtr pScrn, int flags)
 		(((pScrn->mask.blue >> pScrn->offset.blue) - 1) <<
pScrn->offset.blue);
 	}
 
+	/* Limit to max 2 pending swaps - we can't handle more than
triple-buffering: */
+	pNv->max_swap_limit = 2;
+
+	if(xf86GetOptValInteger(pNv->Options, OPTION_SWAP_LIMIT,
&(pNv->swap_limit))) {
+		if (pNv->swap_limit < 1)
+			pNv->swap_limit = 1;
+
+		if (pNv->swap_limit > pNv->max_swap_limit)
+			pNv->swap_limit = pNv->max_swap_limit;
+
+		reason = "";
+		from = X_CONFIG;
+
+		if (DRI2INFOREC_VERSION < 6) {
+			/* No swap limit api in server. Stick to server default of 1. */
+			pNv->swap_limit = 1;
+			from = X_DEFAULT;
+			reason = ": This X-Server only supports a swap limit of 1.";
+		}
+	} else {
+		/* Driver default: Double buffering on old servers, triple-buffering
+		 * on Xorg 1.12+.
+		 */
+		pNv->swap_limit = (DRI2INFOREC_VERSION < 6) ? 1 : 2;
+		reason = "";
+		from = X_DEFAULT;
+	}
+
+	xf86DrvMsg(pScrn->scrnIndex, from, "Swap limit set to %d [Max allowed
%d]%s\n",
+		   pNv->swap_limit, pNv->max_swap_limit, reason);
+
 	ret = drmmode_pre_init(pScrn, nouveau_device(pNv->dev)->fd,
 			       pScrn->bitsPerPixel >> 3);
 	if (ret == FALSE)
diff --git a/src/nv_type.h b/src/nv_type.h
index 4204556..73328fc 100644
--- a/src/nv_type.h
+++ b/src/nv_type.h
@@ -53,6 +53,9 @@ typedef struct _NVRec {
     Bool		tiled_scanout;
     Bool		glx_vblank;
     Bool		has_pageflip;
+    int 		swap_limit;
+    int 		max_swap_limit;
+
     ScreenBlockHandlerProcPtr BlockHandler;
     CreateScreenResourcesProcPtr CreateScreenResources;
     CloseScreenProcPtr  CloseScreen;
-- 
1.7.5.4

Buffer exchange assumes that the front buffer pixmap and name
information is accurate. That may not be the case eg. if the window
has been (un)redirected since the buffer was created.

This is a translation to nouveau of a fix that was originally developed
by Ville Syrjala <syrjala at sci.fi> for the ati/radeon ddx to fix the
same bug there.

See thread at:

http://lists.x.org/archives/xorg-devel/2011-May/021908.html

Fixes FDO bug #35452.

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 src/nouveau_dri2.c |   45 ++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index 2908e56..8608678 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -149,6 +149,35 @@ struct nouveau_dri2_vblank_state {
 };
 
 static Bool
+update_front(DrawablePtr draw, DRI2BufferPtr front)
+{
+	int r;
+	PixmapPtr pixmap;
+	struct nouveau_dri2_buffer *nvbuf = nouveau_dri2_buffer(front);
+
+	if (draw->type == DRAWABLE_PIXMAP)
+		pixmap = (PixmapPtr)draw;
+	else
+		pixmap = (*draw->pScreen->GetWindowPixmap)((WindowPtr)draw);
+
+	pixmap->refcnt++;
+
+	exaMoveInPixmap(pixmap);
+	r = nouveau_bo_handle_get(nouveau_pixmap_bo(pixmap), &front->name);
+	if (r) {
+		(*draw->pScreen->DestroyPixmap)(pixmap);
+		return FALSE;
+	}
+
+	(*draw->pScreen->DestroyPixmap)(nvbuf->ppix);
+	front->pitch = pixmap->devKind;
+	front->cpp = pixmap->drawable.bitsPerPixel / 8;
+	nvbuf->ppix = pixmap;
+
+	return TRUE;
+}
+
+static Bool
 can_exchange(DrawablePtr draw, PixmapPtr dst_pix, PixmapPtr src_pix)
 {
 	ScrnInfoPtr scrn = xf86Screens[draw->pScreen->myNum];
@@ -234,13 +263,14 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 {
 	ScrnInfoPtr scrn = xf86Screens[draw->pScreen->myNum];
 	NVPtr pNv = NVPTR(scrn);
-	PixmapPtr dst_pix = nouveau_dri2_buffer(s->dst)->ppix;
+	PixmapPtr dst_pix;
 	PixmapPtr src_pix = nouveau_dri2_buffer(s->src)->ppix;
-	struct nouveau_bo *dst_bo = nouveau_pixmap_bo(dst_pix);
+	struct nouveau_bo *dst_bo;
 	struct nouveau_bo *src_bo = nouveau_pixmap_bo(src_pix);
 	struct nouveau_channel *chan = pNv->chan;
 	RegionRec reg;
 	int type, ret;
+	Bool front_updated;
 
 	REGION_INIT(0, &reg, (&(BoxRec){ 0, 0, draw->width, draw->height
}), 0);
 	REGION_TRANSLATE(0, &reg, draw->x, draw->y);
@@ -257,6 +287,15 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 	if (ref_crtc_hw_id & 1)
 		ref_crtc_hw_id = 1;
 
+	/* Update frontbuffer pixmap and name: Could have changed due to
+	 * window (un)redirection as part of compositing.
+	 */
+	front_updated = update_front(draw, s->dst);
+
+	/* Assign frontbuffer pixmap, after update in update_front() */
+	dst_pix = nouveau_dri2_buffer(s->dst)->ppix;
+	dst_bo = nouveau_pixmap_bo(dst_pix);
+
 	/* Throttle on the previous frame before swapping */
 	nouveau_bo_map(dst_bo, NOUVEAU_BO_RD);
 	nouveau_bo_unmap(dst_bo);
@@ -275,7 +314,7 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 		FIRE_RING(chan);
 	}
 
-	if (can_exchange(draw, dst_pix, src_pix)) {
+	if (front_updated && can_exchange(draw, dst_pix, src_pix)) {
 		type = DRI2_EXCHANGE_COMPLETE;
 		DamageRegionAppend(draw, &reg);
 
-- 
1.7.5.4

Fix some small off-by-one errors and a mismatch
between 32 bit kernel interfaces for vblank count
and 64 bit dri2 interfaces for target_msc et al.

Return corrected target_msc to swap scheduling in
x-server.

A revised version of the patch discussed here:
http://lists.freedesktop.org/archives/nouveau/2011-September/009143.html

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 src/nouveau_dri2.c |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index 8608678..719b3bb 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -387,11 +387,22 @@ nouveau_dri2_schedule_swap(ClientPtr client, DrawablePtr
draw,
 		if (ret)
 			goto fail;
 
+		/* Truncate to match kernel interfaces; means occasional overflow
+		 * misses, but that's generally not a big deal.
+		 */
+		*target_msc &= 0xffffffff;
+		divisor &= 0xffffffff;
+		remainder &= 0xffffffff;
+
 		/* Calculate a swap target if we don't have one */
 		if (current_msc >= *target_msc && divisor)
 			*target_msc = current_msc + divisor
 				- (current_msc - remainder) % divisor;
 
+		/* Avoid underflow of unsigned value below */
+		if (*target_msc == 0)
+			*target_msc = 1;
+
 		/* Request a vblank event one frame before the target */
 		ret = nouveau_wait_vblank(draw, DRM_VBLANK_ABSOLUTE |
 					  DRM_VBLANK_EVENT,
@@ -399,7 +410,8 @@ nouveau_dri2_schedule_swap(ClientPtr client, DrawablePtr
draw,
 					  &expect_msc, NULL, s);
 		if (ret)
 			goto fail;
-		s->frame = (unsigned int) expect_msc & 0xffffffff;
+		s->frame = 1 + ((unsigned int) expect_msc & 0xffffffff);
+		*target_msc = 1 + expect_msc;
 	} else {
 		/* We can't/don't want to sync to vblank, just swap. */
 		nouveau_dri2_finish_swap(draw, 0, 0, 0, s);
@@ -420,6 +432,13 @@ nouveau_dri2_schedule_wait(ClientPtr client, DrawablePtr
draw,
 	CARD64 current_msc;
 	int ret;
 
+	/* Truncate to match kernel interfaces; means occasional overflow
+	 * misses, but that's generally not a big deal.
+	 */
+	target_msc &= 0xffffffff;
+	divisor &= 0xffffffff;
+	remainder &= 0xffffffff;
+
 	if (!can_sync_to_vblank(draw)) {
 		DRI2WaitMSCComplete(client, draw, target_msc, 0, 0);
 		return TRUE;
@@ -439,7 +458,7 @@ nouveau_dri2_schedule_wait(ClientPtr client, DrawablePtr
draw,
 		goto fail;
 
 	/* Calculate a wait target if we don't have one */
-	if (current_msc > target_msc && divisor)
+	if (current_msc >= target_msc && divisor)
 		target_msc = current_msc + divisor
 			- (current_msc - remainder) % divisor;
 
-- 
1.7.5.4

Make sure that swaps for redirected windows under a
compositor are still scheduled via vblank events, to
avoid effects like 2900 fps swaps under a compositor.

See discussion with Francisco Jerez at:

http://lists.freedesktop.org/archives/nouveau/2011-September/009278.html
http://lists.freedesktop.org/archives/nouveau/2011-September/009292.html

This is part I of the agreed upon band-aid, in a separate patch.

It allows to use vblank related functions on redirected
windows and thereby fixes functions from sgi_sync_control
and oml_sync_control extension, e.g., glXWaitForMscOML(),
glXGetSyncValuesOML(), glXWaitVideoSyncSGI, ...

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 src/nouveau_dri2.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index 719b3bb..6a0800c 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -204,10 +204,8 @@ can_sync_to_vblank(DrawablePtr draw)
 {
 	ScrnInfoPtr scrn = xf86Screens[draw->pScreen->myNum];
 	NVPtr pNv = NVPTR(scrn);
-	PixmapPtr pix = NVGetDrawablePixmap(draw);
 
 	return pNv->glx_vblank &&
-		nouveau_exa_pixmap_is_onscreen(pix) &&
 		nv_window_belongs_to_crtc(scrn, draw->x, draw->y,
 					  draw->width, draw->height);
 }
-- 
1.7.5.4

This part implements proper throttling for clients. For
vblank synchronized blits, it defers DRI2SwapComplete()
until 1 vblank after the framebuffer blit is submitted to
the gpu.

Rationale:

For unredirected windows, this is the earliest time the
"blit swap" can complete, as blits are submitted one vblank
before the target vblank and synchronized with vblank in the
gpu. This makes swap completion timestamps at least reasonable.

For redirected windows, the compositor will probably pick
up the "blit swapped" frontbuffer pixmap of the window quickly,
but defer its own recomposition to the next vblank, at least
if sync to vblank for the compositor is on.

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 src/nouveau_dri2.c |   32 +++++++++++++++++++++++++++++---
 1 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index 6a0800c..fdc5148 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -135,6 +135,7 @@ nouveau_dri2_copy_region(DrawablePtr pDraw, RegionPtr
pRegion,
 struct nouveau_dri2_vblank_state {
 	enum {
 		SWAP,
+		BLIT,
 		WAIT
 	} action;
 
@@ -342,6 +343,22 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 
 		REGION_TRANSLATE(0, &reg, -draw->x, -draw->y);
 		nouveau_dri2_copy_region(draw, &reg, s->dst, s->src);
+
+		if (can_sync_to_vblank(draw)) {
+			/* Request a vblank event one vblank from now, the most
+			 * likely (optimistic?) time a direct framebuffer blit
+			 * will complete or a desktop compositor will update its
+			 * screen. This defers DRI2SwapComplete() to the earliest
+			 * likely time of real swap completion.
+			 */
+			s->action = BLIT;
+			ret = nouveau_wait_vblank(draw, DRM_VBLANK_EVENT |
+						  DRM_VBLANK_RELATIVE, 1,
+						  NULL, NULL, s);
+			/* Done, if success. Otherwise use fallback below. */
+			if (!ret)
+				return;
+		}
 	}
 
 	/*
@@ -351,8 +368,9 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 	 *
 	 * XXX - The DRI2SwapLimit() API allowed us to move this to
 	 *	 the flip handler with no FPS hit for page flipped swaps.
-	 *       It is still needed for copy swaps as we lack a method
-	 *       to detect true swap completion for DRI2_BLIT_COMPLETE.
+	 *       It is still needed as a fallback for some copy swaps as
+	 *       we lack a method to detect true swap completion for
+	 *       DRI2_BLIT_COMPLETE.
 	 */
 	DRI2SwapComplete(s->client, draw, frame, tv_sec, tv_usec,
 			 type, s->func, s->data);
@@ -505,8 +523,10 @@ nouveau_dri2_vblank_handler(int fd, unsigned int frame,
 
 	ret = dixLookupDrawable(&draw, s->draw, serverClient,
 				M_ANY, DixWriteAccess);
-	if (ret)
+	if (ret) {
+		free(s);
 		return;
+	}
 
 	switch (s->action) {
 	case SWAP:
@@ -517,6 +537,12 @@ nouveau_dri2_vblank_handler(int fd, unsigned int frame,
 		DRI2WaitMSCComplete(s->client, draw, frame, tv_sec, tv_usec);
 		free(s);
 		break;
+
+	case BLIT:
+		DRI2SwapComplete(s->client, draw, frame, tv_sec, tv_usec,
+				 DRI2_BLIT_COMPLETE, s->func, s->data);
+		free(s);
+		break;
 	}
 }
 
-- 
1.7.5.4

X-Servers before 1.12.0 don't have the DRI2SwapLimit()
API. On these, default to a swaplimit of 1 - double-buffering.

This patch implements support for swap limit of 2,
triple-buffering, on old x-servers via Francisco Jerez
previous hack:

Return DRI2SwapComplete() before the swap has completed,
so clients don't get blocked on the pending swap. This
allows for a "triple-buffering look-alike" behaviour, but
breaks the swap scheduling and timestamping defined
in the OML_sync_control spec, so applications which
rely on conformant behaviour will break with a swap
limit of 2 on pre 1.12.0 x-servers.

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 man/nouveau.man    |    6 +++++-
 src/nouveau_dri2.c |   32 +++++++++++++++++++++++++++++---
 src/nv_driver.c    |   11 ++++++-----
 3 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/man/nouveau.man b/man/nouveau.man
index 59f6c1a..7c72907 100644
--- a/man/nouveau.man
+++ b/man/nouveau.man
@@ -101,7 +101,11 @@ a drawable before a client is blocked.
 A value of 1 corresponds to double-buffering. A value of 2 corresponds
 to triple-buffering. Higher values may allow higher framerate, but also
 increase lag for interactive applications, e.g., games. Nouveau currently
-supports a maximum value of 2 on XOrg 1.12+ and a maximum of 1 on older
servers.
+reliably supports a maximum value of 2 on XOrg 1.12+. A maximum setting of 2
+on older x-servers is allowed, but it will break conformance with the
+OpenML OML_sync_control specification and will cause failure of software
+that relies on correct presentation timing behaviour as defined in that
+specification.
 .br
 Default: 2 for XOrg 1.12+, 1 for older servers.
 .SH "SEE ALSO"
diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index fdc5148..f0c7fec 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -255,6 +255,18 @@ nouveau_dri2_swap_limit_validate(DrawablePtr draw, int
swap_limit)
 }
 #endif
 
+/* Shall we intentionally violate the OML_sync_control spec to
+ * get some sort of triple-buffering behaviour on a pre 1.12.0
+ * x-server?
+ */
+static Bool violate_oml(DrawablePtr draw)
+{
+	ScrnInfoPtr scrn = xf86Screens[draw->pScreen->myNum];
+	NVPtr pNv = NVPTR(scrn);
+
+	return (DRI2INFOREC_VERSION < 6) && (pNv->swap_limit > 1);
+}
+
 static void
 nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int frame,
 			 unsigned int tv_sec, unsigned int tv_usec,
@@ -319,7 +331,9 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 
 		if (DRI2CanFlip(draw)) {
 			type = DRI2_FLIP_COMPLETE;
-			ret = drmmode_page_flip(draw, src_pix, s, ref_crtc_hw_id);
+			ret = drmmode_page_flip(draw, src_pix,
+						violate_oml(draw) ? NULL : s,
+						ref_crtc_hw_id);
 			if (!ret)
 				goto out;
 		}
@@ -330,7 +344,7 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 		DamageRegionProcessPending(draw);
 
 		/* If it is a page flip, finish it in the flip event handler. */
-		if (type == DRI2_FLIP_COMPLETE)
+		if ((type == DRI2_FLIP_COMPLETE) && !violate_oml(draw))
 			return;
 	} else {
 		type = DRI2_BLIT_COMPLETE;
@@ -344,7 +358,7 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 		REGION_TRANSLATE(0, &reg, -draw->x, -draw->y);
 		nouveau_dri2_copy_region(draw, &reg, s->dst, s->src);
 
-		if (can_sync_to_vblank(draw)) {
+		if (can_sync_to_vblank(draw) && !violate_oml(draw)) {
 			/* Request a vblank event one vblank from now, the most
 			 * likely (optimistic?) time a direct framebuffer blit
 			 * will complete or a desktop compositor will update its
@@ -361,6 +375,14 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 		}
 	}
 
+	/* Special triple-buffering hack for old pre 1.12.0 x-servers used? */
+	if (violate_oml(draw)) {
+		/* Signal to client that swap completion timestamps and counts
+		 * are invalid - they violate the specification.
+		 */
+		frame = tv_sec = tv_usec = 0;
+	}
+
 	/*
 	 * Tell the X server buffers are already swapped even if they're
 	 * not, to prevent it from blocking the client on the next
@@ -371,6 +393,10 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int
frame,
 	 *       It is still needed as a fallback for some copy swaps as
 	 *       we lack a method to detect true swap completion for
 	 *       DRI2_BLIT_COMPLETE.
+	 *
+	 *       It is also used if triple-buffering is requested on
+	 *       old x-servers which don't support the DRI2SwapLimit()
+	 *       function.
 	 */
 	DRI2SwapComplete(s->client, draw, frame, tv_sec, tv_usec,
 			 type, s->func, s->data);
diff --git a/src/nv_driver.c b/src/nv_driver.c
index 5def531..7512464 100644
--- a/src/nv_driver.c
+++ b/src/nv_driver.c
@@ -863,11 +863,12 @@ NVPreInit(ScrnInfoPtr pScrn, int flags)
 		reason = "";
 		from = X_CONFIG;
 
-		if (DRI2INFOREC_VERSION < 6) {
-			/* No swap limit api in server. Stick to server default of 1. */
-			pNv->swap_limit = 1;
-			from = X_DEFAULT;
-			reason = ": This X-Server only supports a swap limit of 1.";
+		if ((DRI2INFOREC_VERSION < 6) && (pNv->swap_limit > 1)) {
+			/* No swap limit api in server. A value > 1 requires use
+			 * of problematic hacks.
+			 */
+			from = X_WARNING;
+			reason = ": Caution: Use of this swap limit > 1 violates
OML_sync_control spec on this X-Server!\n";
 		}
 	} else {
 		/* Driver default: Double buffering on old servers, triple-buffering
-- 
1.7.5.4

If a swaplimit > 1 is set on a server which
supports the swaplimit api (XOrg 1.12.0+),
the following can happen:

1. Client calls glXSwapBuffersMscOML() with a
   swap target > 1 vblank in the future, or a
   client calls glXSwapbuffers() while the swap
   interval is set to > 1 (unusual but possible).

2. nouveau_dri2_finish_swap() is therefore called
   only at the target vblank, instead of immediately.

3. Because of the deferred execution of
   nouveu_dri2_finish_swap(), the OpenGL client
   can call x-servers DRI2GetBuffersWithFormat()
   before nouveau_dri2_finish_swap() executes and
   it deletes pixmaps that would be needed by
   nouveau_dri2_finish_swap() --> Segfault --> Crash.

Prevent this: When a swap is scheduled into the
future, we temporarily reduce the swaplimit to 1
until nouveau_dri2_finish_swap() is done, then
restore it to its original value. This throttles
the client inside the server in DRI2ThrottleClient()
before it can call the evil DRI2GetbuffersWithFormat().

The client will still be released one video refresh
interval before swap completion, so there is still
some potential win.

This doesn't affect the common case of swapping at
the next vblank, where this throttling is not needed
or done.

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 src/nouveau_dri2.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index f0c7fec..7878a5a 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -445,6 +445,26 @@ nouveau_dri2_schedule_swap(ClientPtr client, DrawablePtr
draw,
 		if (*target_msc == 0)
 			*target_msc = 1;
 
+#if DRI2INFOREC_VERSION >= 6
+		/* Is this a swap in the future, ie. the vblank event will
+		 * not be immediately dispatched, but only at a future vblank?
+		 * If so, we need to temporarily lower the swaplimit to 1, so
+		 * that DRI2GetBuffersWithFormat() requests from the client get
+		 * deferred in the x-server until the vblank event has been
+		 * dispatched to us and nouveau_dri2_finish_swap() is done. If
+		 * we wouldn't do this, DRI2GetBuffersWithFormat() would operate
+		 * on wrong (pre-swap) buffers, and cause a segfault later on in
+		 * nouveau_dri2_finish_swap(). Our vblank event handler restores
+		 * the old swaplimit immediately after nouveau_dri2_finish_swap()
+		 * is done, so we still get 1 video refresh cycle worth of
+		 * triple-buffering. For a swap at next vblank, dispatch of the
+		 * vblank event happens immediately, so there isn't any need
+		 * for this lowered swaplimit.
+		 */
+		if (current_msc < *target_msc - 1)
+			DRI2SwapLimit(draw, 1);
+#endif
+
 		/* Request a vblank event one frame before the target */
 		ret = nouveau_wait_vblank(draw, DRM_VBLANK_ABSOLUTE |
 					  DRM_VBLANK_EVENT,
@@ -557,6 +577,12 @@ nouveau_dri2_vblank_handler(int fd, unsigned int frame,
 	switch (s->action) {
 	case SWAP:
 		nouveau_dri2_finish_swap(draw, frame, tv_sec, tv_usec, s);
+#if DRI2INFOREC_VERSION >= 6
+		/* Restore real swap limit on drawable, now that it is safe. */
+		ScrnInfoPtr scrn = xf86Screens[draw->pScreen->myNum];
+		DRI2SwapLimit(draw, NVPTR(scrn)->swap_limit);
+#endif
+
 		break;
 
 	case WAIT:
-- 
1.7.5.4

On Don, 2012-02-16 at 00:45 +0100, Mario Kleiner wrote: 
> If a swaplimit > 1 is set on a server which
> supports the swaplimit api (XOrg 1.12.0+),
> the following can happen:
> 
> 1. Client calls glXSwapBuffersMscOML() with a
>    swap target > 1 vblank in the future, or a
>    client calls glXSwapbuffers() while the swap
>    interval is set to > 1 (unusual but possible).
> 
> 2. nouveau_dri2_finish_swap() is therefore called
>    only at the target vblank, instead of immediately.
> 
> 3. Because of the deferred execution of
>    nouveu_dri2_finish_swap(), the OpenGL client
>    can call x-servers DRI2GetBuffersWithFormat()
>    before nouveau_dri2_finish_swap() executes and
>    it deletes pixmaps that would be needed by
>    nouveau_dri2_finish_swap() --> Segfault --> Crash.
Pixmaps are reference counted, so it should be possible to fix this via
proper reference counting.


-- 
Earthling Michel D?nzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer

On Thu, 2012-02-16 at 00:45 +0100, Mario Kleiner wrote:
> Hi,
Hey Mario,

What's your plan with this patchset?  Do you intend on taking Michel's
comments into account?

CC'ing Francisco as he had some comments on IRC.  I'd like to get this
all sorted and merged so I can rebase the ported-to-new-libdrm ddx on
top of it all and get that ready.

Ben.
> 
> here a set of patches against the nouveau-ddx. This is an extended and
> revised set, based on Francisco Jerez feedback from autumn last year.
> 
> [1/9] Makes pageflipping work again on X-Server 1.12rc. It apparently
stopped
> working somewhere around Xorg 1.11+.
> 
> [2/9] Implements handling of pageflip completion events from the kernel.
> Francisco Jerez argument against including it was that the x-server
didn't
> have a swaplimit api, so this couldn't be applied at the same time as
the
> pseudo triple-buffering hack which is in place in the current ddx.
> Now we have the swaplimit api in 1.12, so this problem should be solved.
> 
> [3/9] Makes use of the swaplimit api. A new xorg.conf option
"SwapLimit"
> allows to select a swaplimit of 1 or 2 for double-buffering or
> triple-buffering. It defaults to 2 for Xorg 1.12+ and 1 for older servers.
> This way, on 1.12+ nouveau retains the kind of triple buffering behaviour
it
> currently has, but swap completion timestamping (OML_sync_control,
> INTEL_swap_events, and client swap throttling) works conforming to
> the specs. On older servers it removes triple-buffering but makes
> nouveau conform to the specs. This is important for apps that need
> precise and reliable swap scheduling and timestamping.
> 
> [4/9] A bug fix against corrupted desktop when switching from redirected
> to non-redirected fullscreen windows under a desktop compositor.
> Fixes FDO bug #35452.
> 
> [5/9] Some fixes to swap scheduling, revised according to Francisco's
> review.
> 
> [6/9] Fixes swap throttling for non-fullscreen windows under a desktop
> compositor. Split into a separate patch according to Francisco's
> feedback.
> 
> [7/9] An attempt to provide more sane swap completion events for non-
> fullscreen windows. This is a bit of cheating, as it delivers the
> events for the earliest point in time one would expect the swap
> to complete, assuming only a lightly loaded gpu. Real completion
> could be later. I think this is an "improvement" because the
current
> implementation delivers swap complete events with timestamps and
> counts that signal swap completion before the client even requested
> the swap.
> 
> [8/9] This one adds Francisco's original triple-buffering hack back
> for Xorg 1.11 and earlier servers if the xorg.conf option requests
> a swaplimit > 1. Users can choose between "triple-buffering"
but
> broken timestamping or double-buffering with sane timestamping.
> 
> [9/9] Fixes a corner case which could cause the ddx to segfault
> with its current triple-buffering implementation.
> 
> I've tested these on single-display and dual-display setups,
> with/without compositor, with/without redirection and checked
> the robustness and precision of the timestamps with special
> measurement equipment, on XOrg 1.12-rc2 and 1.10. They work
> pretty well for me and finally make nouveau very useable for
> the kind of scientific applications that require precise
> swap scheduling and timestamping, so i'd love to see them
> reviewed and hopefully included into the ddx soon.
> 
> A couple of things are a bit of hacks:
> 
> [3/9] I think the setup of a default swap limit would be
> better done in the x-server itself instead of the ddx. The
> setup code is a bit awkward, hijacking the ddx->CreateBuffer
> function to apply the swaplimit. A DRI2GetSwapLimit() function
> is also missing from the server, which could help in the
> future to track swaplimit changes by other clients than the ddx
> itself. Unfortunately it is too late for the 1.12 release to do
> this.
> 
> [8/9] Don't know if this is still wanted/needed or not?
> 
> [9/9] It fixes the problem and doesn't affect performance, but is
> somewhat of a hack. I don't know how to do better, maybe somebody
> else has a better solution?
> 
> Thanks,
> -mario
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau