| Message ID | 1400687598-17477-1-git-send-email-jamey@minilop.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 21, 2014 at 08:53:18AM -0700, Jamey Sharp wrote: > 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. Why not just change the default to double buffering? Or fix it correctly? -Chris
On Wed, May 21, 2014 at 05:24:18PM +0100, Chris Wilson wrote: > On Wed, May 21, 2014 at 08:53:18AM -0700, Jamey Sharp wrote: > > 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. > > Why not just change the default to double buffering? Or fix it > correctly? This patch *is* fixing it correctly. I don't understand the buffer management well enough to implement triple buffering in your driver though, sorry; all I know is that what's there now isn't it. Jamey
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;
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(-)