[Mesa-dev] [PATCH 11/19] glx: Replace DRI2WaitForSBC custom protocol with XCB.

[Chad Versace]

I would have used the checked variant of the xcb call, xcb_dri2_wait_xbc, and
then returned 1 if an error had occured. Why did you choose to use the unchecked
variant?

I'm aware that the old DRI2WaitSBC also always blindly returned success, but is
there justification for the new code to also do that?

On 09/25/2012 07:50 PM, Eric Anholt wrote:
> ---
>  src/glx/dri2.c     |   41 -----------------------------------------
>  src/glx/dri2.h     |    4 ----
>  src/glx/dri2_glx.c |   43 ++++++++++++++++++++++++++++++++++---------
>  3 files changed, 34 insertions(+), 54 deletions(-)
> 
> diff --git a/src/glx/dri2.c b/src/glx/dri2.c
> index e17da6f..21b9c21 100644
> --- a/src/glx/dri2.c
> +++ b/src/glx/dri2.c
> @@ -664,47 +664,6 @@ Bool DRI2WaitMSC(Display *dpy, XID drawable, CARD64 target_msc, CARD64 divisor,
>  }
>  #endif
>  
> -#ifdef X_DRI2WaitSBC
> -static void
> -load_sbc_req(xDRI2WaitSBCReq *req, CARD64 target)
> -{
> -    req->target_sbc_hi = target >> 32;
> -    req->target_sbc_lo = target & 0xffffffff;
> -}
> -
> -Bool DRI2WaitSBC(Display *dpy, XID drawable, CARD64 target_sbc, CARD64 *ust,
> -		 CARD64 *msc, CARD64 *sbc)
> -{
> -    XExtDisplayInfo *info = DRI2FindDisplay(dpy);
> -    xDRI2WaitSBCReq *req;
> -    xDRI2MSCReply rep;
> -
> -    XextCheckExtension (dpy, info, dri2ExtensionName, False);
> -
> -    LockDisplay(dpy);
> -    GetReq(DRI2WaitSBC, req);
> -    req->reqType = info->codes->major_opcode;
> -    req->dri2ReqType = X_DRI2WaitSBC;
> -    req->drawable = drawable;
> -    load_sbc_req(req, target_sbc);
> -
> -    if (!_XReply(dpy, (xReply *)&rep, 0, xFalse)) {
> -	UnlockDisplay(dpy);
> -	SyncHandle();
> -	return False;
> -    }
> -
> -    *ust = ((CARD64)rep.ust_hi << 32) | rep.ust_lo;
> -    *msc = ((CARD64)rep.msc_hi << 32) | rep.msc_lo;
> -    *sbc = ((CARD64)rep.sbc_hi << 32) | rep.sbc_lo;
> -
> -    UnlockDisplay(dpy);
> -    SyncHandle();
> -
> -    return True;
> -}
> -#endif
> -
>  #ifdef X_DRI2SwapInterval
>  void DRI2SwapInterval(Display *dpy, XID drawable, int interval)
>  {
> diff --git a/src/glx/dri2.h b/src/glx/dri2.h
> index 114e9f8..bfd9910 100644
> --- a/src/glx/dri2.h
> +++ b/src/glx/dri2.h
> @@ -96,10 +96,6 @@ extern Bool
>  DRI2WaitMSC(Display *dpy, XID drawable, CARD64 target_msc, CARD64 divisor,
>  	    CARD64 remainder, CARD64 *ust, CARD64 *msc, CARD64 *sbc);
>  
> -extern Bool
> -DRI2WaitSBC(Display *dpy, XID drawable, CARD64 target_sbc, CARD64 *ust,
> -	    CARD64 *msc, CARD64 *sbc);
> -
>  extern void
>  DRI2SwapInterval(Display *dpy, XID drawable, int interval);
>  
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index ff5373c..d691577 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -34,6 +34,9 @@
>  
>  #include <X11/Xlib.h>
>  #include <X11/extensions/Xfixes.h>
> +#include <X11/Xlib-xcb.h>
> +#include <xcb/xcb.h>
> +#include <xcb/dri2.h>
>  #include "glapi.h"
>  #include "glxclient.h"
>  #include <X11/extensions/dri2proto.h>
> @@ -118,6 +121,22 @@ struct dri2_drawable
>  
>  static const struct glx_context_vtable dri2_context_vtable;
>  
> +/* For XCB's handling of ust/msc/sbc counters, we have to hand it the high and
> + * low halves separately.  This helps you split them.
> + */
> +static void
> +split_counter(uint64_t counter, uint32_t *hi, uint32_t *lo)
> +{
> +   *hi = (counter >> 32);
> +   *lo = counter & 0xffffffff;
> +}
> +
> +static uint64_t
> +merge_counter(uint32_t hi, uint32_t lo)
> +{
> +   return ((uint64_t)hi << 32) | lo;
> +}
> +
>  static void
>  dri2_destroy_context(struct glx_context *context)
>  {
> @@ -456,16 +475,22 @@ static int
>  dri2WaitForSBC(__GLXDRIdrawable *pdraw, int64_t target_sbc, int64_t *ust,
>  	       int64_t *msc, int64_t *sbc)
>  {
> -   CARD64 dri2_ust, dri2_msc, dri2_sbc;
> -   int ret;
> +   xcb_connection_t *c = XGetXCBConnection(pdraw->psc->dpy);
> +   xcb_dri2_wait_sbc_cookie_t wait_sbc_cookie;
> +   xcb_dri2_wait_sbc_reply_t *wait_sbc_reply;
> +   uint32_t target_sbc_hi, target_sbc_lo;
>  
> -   ret = DRI2WaitSBC(pdraw->psc->dpy, pdraw->xDrawable,
> -		     target_sbc, &dri2_ust, &dri2_msc, &dri2_sbc);
> -   *ust = dri2_ust;
> -   *msc = dri2_msc;
> -   *sbc = dri2_sbc;
> +   split_counter(target_sbc, &target_sbc_hi, &target_sbc_lo);
>  
> -   return ret;
> +   wait_sbc_cookie = xcb_dri2_wait_sbc_unchecked(c, pdraw->xDrawable,
> +                                                 target_sbc_hi, target_sbc_lo);
> +   wait_sbc_reply = xcb_dri2_wait_sbc_reply(c, wait_sbc_cookie, NULL);
> +   *ust = merge_counter(wait_sbc_reply->ust_hi, wait_sbc_reply->ust_lo);
> +   *msc = merge_counter(wait_sbc_reply->msc_hi, wait_sbc_reply->msc_lo);
> +   *sbc = merge_counter(wait_sbc_reply->sbc_hi, wait_sbc_reply->sbc_lo);
> +   free(wait_sbc_reply);
> +
> +   return 0;
>  }
>  
>  #endif /* X_DRI2WaitMSC */
> @@ -1119,8 +1144,8 @@ dri2CreateScreen(int screen, struct glx_display * priv)
>  #endif
>  #ifdef X_DRI2WaitMSC
>        psp->waitForMSC = dri2WaitForMSC;
> -      psp->waitForSBC = dri2WaitForSBC;
>  #endif
> +      psp->waitForSBC = dri2WaitForSBC;
>  #ifdef X_DRI2SwapInterval
>        psp->setSwapInterval = dri2SetSwapInterval;
>        psp->getSwapInterval = dri2GetSwapInterval;
>