[PATCH v2] UXA: Wait until a pageflip actually completes to report it.

* [PATCH v2] UXA: Wait until a pageflip actually completes to report it.
@ 2014-05-21 15:53 Jamey Sharp
  2014-05-21 16:24 ` Chris Wilson
  0 siblings, 1 reply; 3+ messages in thread
From: Jamey Sharp @ 2014-05-21 15:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: Theo Hill

UXA was reporting page-flip completion as soon as the flip was scheduled
with the kernel, instead of waiting for the kernel to indicate that the
flip had actually completed.

Moving the DRI2SwapComplete call to the right place fixes all of our
Piglit tests for OML_sync_control when run on xf86-video-intel/UXA,
aside from a bit of difficult-to-reproduce flakiness when using a
divisor > 1.

This also eliminates a compile-time and run-time warning when built
against an xserver with "Warn on DRI2SwapComplete with constant UST/MSC"
applied.

v2: The drawable may have disappeared by the time the flip completes.
Don't try to report swap completion in that case.

Signed-off-by: Jamey Sharp <jamey@minilop.net>
Cc: Theo Hill <Theo0x48@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---

I can experimentally confirm Chris' claim that this patch causes
SwapBuffers to block once one swap is already outstanding, giving
double-buffering behavior rather than the desired triple-buffering.

However, it only has an effect for full-screen windows, and only when
not running under a compositor.

- If the window is not full-screen, UXA is already only double-buffered.
- If full-screen, UXA is usually triple-buffered, but not reliably.
- If run under a compositor, either the compositor crashes during my
  test, or it still appears to be triple-buffered even with this patch.

If you want triple-buffering, NAK'ing this patch is clearly not the way
to get it, since the driver already doesn't do it reliably.

Please merge this patch, which fixes two spec violations that make
OML_sync_control unusable; and if you're concerned about uncomposited
triple-buffering in UXA, please find a less awful way to get it.

Thanks,
Jamey

 src/uxa/intel_dri.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/uxa/intel_dri.c b/src/uxa/intel_dri.c
index ca58052..d4a242e 100644
--- a/src/uxa/intel_dri.c
+++ b/src/uxa/intel_dri.c
@@ -932,10 +932,6 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
 
 	/* Then flip DRI2 pointers and update the screen pixmap */
 	I830DRI2ExchangeBuffers(intel, info->front, info->back);
-	DRI2SwapComplete(info->client, draw, 0, 0, 0,
-			 DRI2_EXCHANGE_COMPLETE,
-			 info->event_complete,
-			 info->event_data);
 	return TRUE;
 }
 
@@ -1090,6 +1086,14 @@ void I830DRI2FlipEventHandler(unsigned int frame, unsigned int tv_sec,
 		assert(intel->pending_flip[flip_info->pipe] == flip_info);
 		intel->pending_flip[flip_info->pipe] = NULL;
 
+		/* Assuming the drawable's still around, complete the swap. */
+		if (drawable)
+			DRI2SwapComplete(flip_info->client, drawable,
+					 frame, tv_sec, tv_usec,
+					 DRI2_EXCHANGE_COMPLETE,
+					 flip_info->event_complete,
+					 flip_info->event_data);
+
 		chain = flip_info->chain;
 		if (chain) {
 			DrawablePtr chain_drawable = NULL;
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread