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

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

[Jamey Sharp]

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.

Signed-off-by: Jamey Sharp <jamey@minilop.net>
Cc: Theo Hill <Theo0x48@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
---
 src/uxa/intel_dri.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/uxa/intel_dri.c b/src/uxa/intel_dri.c
index ca58052..3745767 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,12 @@ 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;
 
+		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

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

[Chris Wilson]

On Mon, May 05, 2014 at 11:05:07PM -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.

The violation is intentional, as it gives us triple buffering by
default. It can be disabled. Also you could do an improved version for
recent Xorg, still using DRI2.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Jamey Sharp]

[-- Attachment #1.1: Type: text/plain, Size: 872 bytes --]

On Wed, May 07, 2014 at 04:55:18PM +0100, Chris Wilson wrote:
> On Mon, May 05, 2014 at 11:05:07PM -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.
> 
> The violation is intentional, as it gives us triple buffering by
> default. It can be disabled.

As far as I can tell, this patch has no effect on triple-buffering. I
verified that by logging new_front->handle in intel_do_pageflip: It
rotates through three different BO's on successive flips.

Jamey

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Wed, May 07, 2014 at 10:44:23PM -0700, Jamey Sharp wrote:
> On Wed, May 07, 2014 at 04:55:18PM +0100, Chris Wilson wrote:
> > On Mon, May 05, 2014 at 11:05:07PM -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.
> > 
> > The violation is intentional, as it gives us triple buffering by
> > default. It can be disabled.
> 
> As far as I can tell, this patch has no effect on triple-buffering. I
> verified that by logging new_front->handle in intel_do_pageflip: It
> rotates through three different BO's on successive flips.

The patch blocks clients in GetBuffers until the swap completes,
preventing them from rendering to the third buffer.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1244 bytes --]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Wed, May 07, 2014 at 10:44:23PM -0700, Jamey Sharp wrote:
>> On Wed, May 07, 2014 at 04:55:18PM +0100, Chris Wilson wrote:
>> > On Mon, May 05, 2014 at 11:05:07PM -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.
>> > 
>> > The violation is intentional, as it gives us triple buffering by
>> > default. It can be disabled.
>> 
>> As far as I can tell, this patch has no effect on triple-buffering. I
>> verified that by logging new_front->handle in intel_do_pageflip: It
>> rotates through three different BO's on successive flips.
>
> The patch blocks clients in GetBuffers until the swap completes,
> preventing them from rendering to the third buffer.

I don't see where the client gets blocked in a swapbuffers.  Can you
point that out?

[-- Attachment #1.2: Type: application/pgp-signature, Size: 818 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Thu, May 08, 2014 at 08:54:52AM -0700, Eric Anholt wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > On Wed, May 07, 2014 at 10:44:23PM -0700, Jamey Sharp wrote:
> >> On Wed, May 07, 2014 at 04:55:18PM +0100, Chris Wilson wrote:
> >> > On Mon, May 05, 2014 at 11:05:07PM -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.
> >> > 
> >> > The violation is intentional, as it gives us triple buffering by
> >> > default. It can be disabled.
> >> 
> >> As far as I can tell, this patch has no effect on triple-buffering. I
> >> verified that by logging new_front->handle in intel_do_pageflip: It
> >> rotates through three different BO's on successive flips.
> >
> > The patch blocks clients in GetBuffers until the swap completes,
> > preventing them from rendering to the third buffer.
> 
> I don't see where the client gets blocked in a swapbuffers.  Can you
> point that out?

DRI2ThrottleClient() called by GetBuffers and SwapBuffers will call
IgnoreClient() if swapsPending >= swap_limit and will only wake the
client on calling DRI2SwapComplete(). The premature SwapComplete is to
allow the clients next call to GetBuffers to succeed promptly returning
the third buffer prior to the page flip completing.

(gdb) bt
#0  IgnoreClient (client=client@entry=0x19de6c0) at connection.c:1156
#1  0x00000000005a1bd4 in DRI2ThrottleClient (client=client@entry=0x19de6c0, pDraw=<optimized out>) at dri2.c:713
#2  0x00000000005a4c3b in ProcDRI2GetBuffersWithFormat (client=0x19de6c0) at dri2ext.c:302
#3  ProcDRI2Dispatch (client=0x19de6c0) at dri2ext.c:608
#4  0x000000000043a70e in Dispatch () at dispatch.c:432
#5  0x000000000043f95d in dix_main (argc=6, argv=0x7fffe4302078, envp=<optimized out>) at main.c:296
#6  0x00007f14dd0c6b45 in __libc_start_main (main=0x426680 <main>, argc=6, argv=0x7fffe4302078, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7fffe4302068) at libc-start.c:287
#7  0x00000000004266b3 in _start ()

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1826 bytes --]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Thu, May 08, 2014 at 08:54:52AM -0700, Eric Anholt wrote:
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > On Wed, May 07, 2014 at 10:44:23PM -0700, Jamey Sharp wrote:
>> >> On Wed, May 07, 2014 at 04:55:18PM +0100, Chris Wilson wrote:
>> >> > On Mon, May 05, 2014 at 11:05:07PM -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.
>> >> > 
>> >> > The violation is intentional, as it gives us triple buffering by
>> >> > default. It can be disabled.
>> >> 
>> >> As far as I can tell, this patch has no effect on triple-buffering. I
>> >> verified that by logging new_front->handle in intel_do_pageflip: It
>> >> rotates through three different BO's on successive flips.
>> >
>> > The patch blocks clients in GetBuffers until the swap completes,
>> > preventing them from rendering to the third buffer.
>> 
>> I don't see where the client gets blocked in a swapbuffers.  Can you
>> point that out?
>
> DRI2ThrottleClient() called by GetBuffers and SwapBuffers will call
> IgnoreClient() if swapsPending >= swap_limit and will only wake the
> client on calling DRI2SwapComplete(). The premature SwapComplete is to
> allow the clients next call to GetBuffers to succeed promptly returning
> the third buffer prior to the page flip completing.

Oh, so swap_limit is just set wrong?

[-- Attachment #1.2: Type: application/pgp-signature, Size: 818 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Thu, May 08, 2014 at 01:55:30PM -0700, Eric Anholt wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > On Thu, May 08, 2014 at 08:54:52AM -0700, Eric Anholt wrote:
> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> 
> >> > On Wed, May 07, 2014 at 10:44:23PM -0700, Jamey Sharp wrote:
> >> >> On Wed, May 07, 2014 at 04:55:18PM +0100, Chris Wilson wrote:
> >> >> > On Mon, May 05, 2014 at 11:05:07PM -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.
> >> >> > 
> >> >> > The violation is intentional, as it gives us triple buffering by
> >> >> > default. It can be disabled.
> >> >> 
> >> >> As far as I can tell, this patch has no effect on triple-buffering. I
> >> >> verified that by logging new_front->handle in intel_do_pageflip: It
> >> >> rotates through three different BO's on successive flips.
> >> >
> >> > The patch blocks clients in GetBuffers until the swap completes,
> >> > preventing them from rendering to the third buffer.
> >> 
> >> I don't see where the client gets blocked in a swapbuffers.  Can you
> >> point that out?
> >
> > DRI2ThrottleClient() called by GetBuffers and SwapBuffers will call
> > IgnoreClient() if swapsPending >= swap_limit and will only wake the
> > client on calling DRI2SwapComplete(). The premature SwapComplete is to
> > allow the clients next call to GetBuffers to succeed promptly returning
> > the third buffer prior to the page flip completing.
> 
> Oh, so swap_limit is just set wrong?

Yes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre