[PATCH 0/6] glamor: synchronize Xv display to vblank

[Adam Jackson]

On Wed, 2014-08-06 at 22:21 -0700, Keith Packard wrote:
> This series creates a new API within the Present code that provides
> vblank-synchronized callbacks using the Present driver
> interfaces and then uses that new interface to delay Xv put image
> until vblank occurrs.
> 
> This synchronization is only done when painting directly to the
> screen, so a composited environment won't see additional delay, but
> when the window goes full-screen and the compositing manager
> un-redirects it, you'll continue to get synchronized video output.
> 
> There are a couple of DebugPresent cleanups:
> 
>  [PATCH 1/6] present: Move DebugPresent macro to present_priv.h
>  [PATCH 2/6] present: Fix int types for DebugPresent arguments

Reviewed-by: Adam Jackson <ajax at redhat.com>

> And two patches to expose the Present API to drivers:
> 
>  [PATCH 3/6] present: Provide stub for present_event_abandon
>  [PATCH 4/6] present: Create an internal server API to perform

Likewise.  Though it'd be nice if the init/fini model in 4 was made more
obvious (as per Mark Marshall's comments), see below...

> One patch that implements the new Present vblank API:
> 
>  [PATCH 5/6] xfree86: Expose present API to drivers via sdksyms

R-b.

> And one little patch that uses the new API in glamor's Xv code:
> 
>  [PATCH 6/6] glamor: Use Present to delay Xv painting to vblank

Not sure I'm okay with this?  I think you're calling
present_vblank_window_queue as if present_vblank_screen_init had been
done, but it doesn't need to have been (the patch doesn't show it added
anywhere, I assume you hid that in the DDX driver).  If we take this
functionality, is there a good reason for a driver to refuse it?  If so
then you need to insulate against present_queue never being initialized;
if not, why not just run vblank setup from present_screen_init (and tear
it down from present_close_screen, addressing Mark's comment).

- ajax