[PATCH] dri3: Support GLX_INTEL_swap_event

[PATCH] dri3: Support GLX_INTEL_swap_event

[Keith Packard]

The easy part is turning on the extension, now that the X server has a patch
to send the events.

The only trick was making sure the Present extension reliably provided the
right 'sbc' count back in the event, and that's done by making sure the sbc
count is always the same as the sequence number that we send in the
PresentPixmap requests, and that's done by using the same variable for both
roles.

Signed-off-by: Keith Packard <keithp@keithp.com>
---

This passes the piglet glx-swap-event test.

 src/glx/dri3_glx.c  | 27 ++++++---------------------
 src/glx/dri3_priv.h |  3 +--
 2 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
index 669f0bb..a7bf318 100644
--- a/src/glx/dri3_glx.c
+++ b/src/glx/dri3_glx.c
@@ -423,7 +423,7 @@ dri3_wait_for_msc(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t divisor,
 
    *ust = priv->ust;
    *msc = priv->msc;
-   *sbc = priv->sbc;
+   *sbc = priv->present_count;
 
    return 1;
 }
@@ -450,7 +450,7 @@ dri3_wait_for_sbc(__GLXDRIdrawable *pdraw, int64_t target_sbc, int64_t *ust,
 {
    struct dri3_drawable *priv = (struct dri3_drawable *) pdraw;
 
-   while (priv->sbc < target_sbc) {
+   while (priv->present_count < target_sbc) {
       sleep(1);
    }
    return dri3_wait_for_msc(pdraw, 0, 0, 0, ust, msc, sbc);
@@ -1282,7 +1282,8 @@ dri3_swap_buffers(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t divisor,
       /* Compute when we want the frame shown by taking the last known successful
        * MSC and adding in a swap interval for each outstanding swap request
        */
-      ++priv->present_request_serial;
+      ++priv->present_count;
+      priv->present_request_serial = (uint32_t) priv->present_count;
       if (target_msc == 0)
          target_msc = priv->msc + priv->swap_interval * (priv->present_request_serial - priv->present_event_serial);
 
@@ -1302,7 +1303,7 @@ dri3_swap_buffers(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t divisor,
                          target_msc,
                          divisor,
                          remainder, 0, NULL);
-      ret = ++priv->sbc;
+      ret = priv->present_count;
 
       /* If there's a fake front, then copy the source back buffer
        * to the fake front to keep it up to date. This needs
@@ -1494,23 +1495,7 @@ dri3_bind_extensions(struct dri3_screen *psc, struct glx_display * priv,
    __glXEnableDirectExtension(&psc->base, "GLX_SGI_swap_control");
    __glXEnableDirectExtension(&psc->base, "GLX_MESA_swap_control");
    __glXEnableDirectExtension(&psc->base, "GLX_SGI_make_current_read");
-
-   /*
-    * GLX_INTEL_swap_event is broken on the server side, where it's
-    * currently unconditionally enabled. This completely breaks
-    * systems running on drivers which don't support that extension.
-    * There's no way to test for its presence on this side, so instead
-    * of disabling it unconditionally, just disable it for drivers
-    * which are known to not support it, or for DDX drivers supporting
-    * only an older (pre-ScheduleSwap) version of DRI2.
-    *
-    * This is a hack which is required until:
-    * http://lists.x.org/archives/xorg-devel/2013-February/035449.html
-    * is merged and updated xserver makes it's way into distros:
-    */
-//   if (pdp->swapAvailable && strcmp(driverName, "vmwgfx") != 0) {
-//      __glXEnableDirectExtension(&psc->base, "GLX_INTEL_swap_event");
-//   }
+   __glXEnableDirectExtension(&psc->base, "GLX_INTEL_swap_event");
 
    mask = psc->image_driver->getAPIMask(psc->driScreen);
 
diff --git a/src/glx/dri3_priv.h b/src/glx/dri3_priv.h
index 05f66cf..cdf986d 100644
--- a/src/glx/dri3_priv.h
+++ b/src/glx/dri3_priv.h
@@ -183,11 +183,10 @@ struct dri3_drawable {
    uint8_t have_fake_front;
    uint8_t is_pixmap;
 
+   uint64_t present_count;
    uint32_t present_request_serial;
    uint32_t present_event_serial;
 
-   uint64_t sbc;
-
    uint64_t ust, msc;
 
    /* For WaitMSC */
-- 
1.8.4.2

Re: [PATCH] dri3: Support GLX_INTEL_swap_event

[Eric Anholt]

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

Keith Packard <keithp@keithp.com> writes:

> The easy part is turning on the extension, now that the X server has a patch
> to send the events.
>
> The only trick was making sure the Present extension reliably provided the
> right 'sbc' count back in the event, and that's done by making sure the sbc
> count is always the same as the sequence number that we send in the
> PresentPixmap requests, and that's done by using the same variable for both
> roles.

I'd prefer to see sbc stay with its current name, since that's its name
in the specs we're trying to implement (GLX_OML_sync_control,
GLX_INTEL_swap_event).  If you drop the rename from the patch,

Reviewed-by: Eric Anholt <eric@anholt.net>

I think our handling of SBC for glXWaitForSbcOML() is wrong.  From the
OML_sync_control spec:

    The SBC value is incremented by the graphics driver at the completion
    of each buffer swap (e.g., the pixel copy has been completed or the
    hardware register that swaps memory banks has been written). For pixel
    formats that do not contain a back buffer, the SBC will always be
    returned as 0.

I read that as "SBC is incremented when the PresentComplete comes in"
not "SBC is incremented when we generate the Present request".
Otherwise glXWaitForSbcOML doesn't make much sense. (in the "e.g." I'm
assuming they're talking a hardware register for pageflipping that
immediately starts scanning out the new stuff, not our fancy new
automatically double buffered ones that you have to push hard on to get
an immediate pageflip mode)

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

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

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] dri3: Support GLX_INTEL_swap_event

[Keith Packard]

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

Eric Anholt <eric@anholt.net> writes:

> I'd prefer to see sbc stay with its current name, since that's its name
> in the specs we're trying to implement (GLX_OML_sync_control,
> GLX_INTEL_swap_event).  If you drop the rename from the patch,
>
> Reviewed-by: Eric Anholt <eric@anholt.net>

Sounds good.

> I read that as "SBC is incremented when the PresentComplete comes in"
> not "SBC is incremented when we generate the Present request".
> Otherwise glXWaitForSbcOML doesn't make much sense. (in the "e.g." I'm
> assuming they're talking a hardware register for pageflipping that
> immediately starts scanning out the new stuff, not our fancy new
> automatically double buffered ones that you have to push hard on to get
> an immediate pageflip mode)

Oh, that's almost sensible. And nicely eliminates the silly sleep(1)
loop. New patches coming shortly.

-- 
keith.packard@intel.com

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

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

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev