diff mbox

DRI2/GLX: fix swap event handling

Message ID 1304022442-8070-5-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes April 28, 2011, 8:27 p.m. UTC 
Use the new swap event structure packing and send it to the client if
possible.  This means tracking client version information when clients
connect.  If they don't support the new packing, they'll get the old
bits and fill junk into their sbc values when they receive the event.
If they do support the new packing, send off the right data.
---
 configure.ac                |    4 +-
 glx/glxdri2.c               |   28 +++++++++++++++++++-
 hw/xfree86/dri2/dri2.c      |   57 ++++++++++++++++++++++++++++++++++++++++++-
 hw/xfree86/dri2/dri2ext.c   |   14 ++++++++++-
 include/protocol-versions.h |    2 +-
 5 files changed, 98 insertions(+), 7 deletions(-)

Comments

Eric Anholt April 28, 2011, 9:37 p.m. UTC | #1 
On Thu, 28 Apr 2011 13:27:22 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Use the new swap event structure packing and send it to the client if
> possible.  This means tracking client version information when clients
> connect.  If they don't support the new packing, they'll get the old
> bits and fill junk into their sbc values when they receive the event.
> If they do support the new packing, send off the right data.

> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -192,8 +192,17 @@ __glXdriSwapEvent(ClientPtr client, void *data, int type, CARD64 ust,
>      wire.ust_lo = ust & 0xffffffff;
>      wire.msc_hi = msc >> 32;
>      wire.msc_lo = msc & 0xffffffff;
> -    wire.sbc_hi = sbc >> 32;
> -    wire.sbc_lo = sbc & 0xffffffff;
> +    wire.sbc_hi = 0;

was that supposed to be wire.sbc_lo and not whacking wire.sbc_hi?

At first I was confused by this whole thing -- why not rearrange the
structure a bit if we're messing things up?  Then I realized that this
let the server emit the mostly-the-same event structure and only steal
the other half of event_type for clients that understand the new 8-bit
event_type protocol.  Seems like a reasonable approach.
Michel Dänzer April 29, 2011, 6:52 a.m. UTC | #2 
On Don, 2011-04-28 at 13:27 -0700, Jesse Barnes wrote: 
> @@ -1114,7 +1169,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
>  	ds->ScheduleSwap = info->ScheduleSwap;
>  	ds->ScheduleWaitMSC = info->ScheduleWaitMSC;
>  	ds->GetMSC = info->GetMSC;
> -	cur_minor = 3;
> +	cur_minor = 4;
>      } else {
>  	cur_minor = 1;
>      }

This bugfix should probably be separate.
Julien Cristau April 29, 2011, 7:20 a.m. UTC | #3 
On Thu, Apr 28, 2011 at 13:27:22 -0700, Jesse Barnes wrote:

> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index d979717..654b4ae 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -192,8 +192,17 @@ __glXdriSwapEvent(ClientPtr client, void *data, int type, CARD64 ust,
>      wire.ust_lo = ust & 0xffffffff;
>      wire.msc_hi = msc >> 32;
>      wire.msc_lo = msc & 0xffffffff;
> -    wire.sbc_hi = sbc >> 32;
> -    wire.sbc_lo = sbc & 0xffffffff;
> +    wire.sbc_hi = 0;
> +
> +    if (DRI2ClientSupportsSBC(client)) {
> +	wire.sbc_lo0 = sbc & 0xff;
> +	wire.sbc_lo8 = (sbc >> 8) & 0xff;
> +	wire.sbc_lo16 = (sbc >> 16) & 0xff;

This one should be wire.sbc_lo16 = (sbc >> 16) & 0xffff;

> +    } else {
> +	wire.sbc_lo0 = 0;
> +	wire.sbc_lo8 = 0;
> +	wire.sbc_lo16 = 0;
> +    }
>  
>      WriteEventsToClient(client, 1, (xEvent *) &wire);
>  }

Also I think big endian clients (when built against the old protocol
header) will expect the low byte of event_type where you now have
sbc_lo8, so you would need to swap them.  Same for the dri2 event.

[...]
> diff --git a/include/protocol-versions.h b/include/protocol-versions.h
> index 8692ded..8fde917 100644
> --- a/include/protocol-versions.h
> +++ b/include/protocol-versions.h
> @@ -57,7 +57,7 @@
>  
>  /* GLX */
>  #define SERVER_GLX_MAJOR_VERSION		1
> -#define SERVER_GLX_MINOR_VERSION		4
> +#define SERVER_GLX_MINOR_VERSION		5
>  
Do we get to bump the GLX protocol version?  Somehow I thought that was
khronos business.

>  /* Xinerama */
>  #define SERVER_PANORAMIX_MAJOR_VERSION          1

Cheers,
Julien
Jesse Barnes April 29, 2011, 3:24 p.m. UTC | #4 
On Fri, 29 Apr 2011 09:20:31 +0200
Julien Cristau <jcristau@debian.org> wrote:

> On Thu, Apr 28, 2011 at 13:27:22 -0700, Jesse Barnes wrote:
> 
> > diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> > index d979717..654b4ae 100644
> > --- a/glx/glxdri2.c
> > +++ b/glx/glxdri2.c
> > @@ -192,8 +192,17 @@ __glXdriSwapEvent(ClientPtr client, void *data, int type, CARD64 ust,
> >      wire.ust_lo = ust & 0xffffffff;
> >      wire.msc_hi = msc >> 32;
> >      wire.msc_lo = msc & 0xffffffff;
> > -    wire.sbc_hi = sbc >> 32;
> > -    wire.sbc_lo = sbc & 0xffffffff;
> > +    wire.sbc_hi = 0;
> > +
> > +    if (DRI2ClientSupportsSBC(client)) {
> > +	wire.sbc_lo0 = sbc & 0xff;
> > +	wire.sbc_lo8 = (sbc >> 8) & 0xff;
> > +	wire.sbc_lo16 = (sbc >> 16) & 0xff;
> 
> This one should be wire.sbc_lo16 = (sbc >> 16) & 0xffff;

Ah thanks, good catch. In this case, too much typing and not enough
cult & paste. :)

> 
> > +    } else {
> > +	wire.sbc_lo0 = 0;
> > +	wire.sbc_lo8 = 0;
> > +	wire.sbc_lo16 = 0;
> > +    }
> >  
> >      WriteEventsToClient(client, 1, (xEvent *) &wire);
> >  }
> 
> Also I think big endian clients (when built against the old protocol
> header) will expect the low byte of event_type where you now have
> sbc_lo8, so you would need to swap them.  Same for the dri2 event.

Right.  I should probably add swapping for the whole thing for swapped
clients as well.

> 
> [...]
> > diff --git a/include/protocol-versions.h b/include/protocol-versions.h
> > index 8692ded..8fde917 100644
> > --- a/include/protocol-versions.h
> > +++ b/include/protocol-versions.h
> > @@ -57,7 +57,7 @@
> >  
> >  /* GLX */
> >  #define SERVER_GLX_MAJOR_VERSION		1
> > -#define SERVER_GLX_MINOR_VERSION		4
> > +#define SERVER_GLX_MINOR_VERSION		5
> >  
> Do we get to bump the GLX protocol version?  Somehow I thought that was
> khronos business.

Maybe it is, but unless we're going to ignore this problem for indirect
glx clients, we need a way of knowing which type of event structure to
expect.  Is there another version I could use instead?
diff mbox

Patch

diff --git a/configure.ac b/configure.ac
index 6eb780c..87194a5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -771,11 +771,11 @@  RECORDPROTO="recordproto >= 1.13.99.1"
 SCRNSAVERPROTO="scrnsaverproto >= 1.1"
 RESOURCEPROTO="resourceproto"
 DRIPROTO="xf86driproto >= 2.1.0"
-DRI2PROTO="dri2proto >= 2.3"
+DRI2PROTO="dri2proto >= 2.4"
 XINERAMAPROTO="xineramaproto"
 BIGFONTPROTO="xf86bigfontproto >= 1.2.0"
 DGAPROTO="xf86dgaproto >= 2.0.99.1"
-GLPROTO="glproto >= 1.4.10"
+GLPROTO="glproto >= 1.4.13"
 DMXPROTO="dmxproto >= 2.2.99.1"
 VIDMODEPROTO="xf86vidmodeproto >= 2.2.99.1"
 WINDOWSWMPROTO="windowswmproto"
diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index d979717..654b4ae 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -192,8 +192,17 @@  __glXdriSwapEvent(ClientPtr client, void *data, int type, CARD64 ust,
     wire.ust_lo = ust & 0xffffffff;
     wire.msc_hi = msc >> 32;
     wire.msc_lo = msc & 0xffffffff;
-    wire.sbc_hi = sbc >> 32;
-    wire.sbc_lo = sbc & 0xffffffff;
+    wire.sbc_hi = 0;
+
+    if (DRI2ClientSupportsSBC(client)) {
+	wire.sbc_lo0 = sbc & 0xff;
+	wire.sbc_lo8 = (sbc >> 8) & 0xff;
+	wire.sbc_lo16 = (sbc >> 16) & 0xff;
+    } else {
+	wire.sbc_lo0 = 0;
+	wire.sbc_lo8 = 0;
+	wire.sbc_lo16 = 0;
+    }
 
     WriteEventsToClient(client, 1, (xEvent *) &wire);
 }
@@ -228,6 +237,18 @@  __glXDRIdrawableSwapBuffers(ClientPtr client, __GLXdrawable *drawable)
     return TRUE;
 }
 
+static void
+__glXDRIclientGone(CallbackListPtr	*list,
+		   pointer		closure,
+		   pointer		data)
+{
+    NewClientInfoRec	*clientinfo = (NewClientInfoRec *) data;
+    ClientPtr		pClient = clientinfo->client;
+
+    if (pClient->clientState == ClientStateGone)
+	DRI2ClientGone(client);
+}
+
 static int
 __glXDRIdrawableSwapInterval(__GLXdrawable *drawable, int interval)
 {
@@ -769,6 +790,9 @@  __glXDRIscreenProbe(ScreenPtr pScreen)
     screen->leaveVT = pScrn->LeaveVT;
     pScrn->LeaveVT = glxDRILeaveVT;
 
+    if (!AddCallback (&ClientStateCallback, __glXDRIclientGone, 0))
+	return;
+
     LogMessage(X_INFO,
 	       "AIGLX: Loaded and initialized %s\n", driverName);
 
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 5c42a51..9b5eab2 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -60,6 +60,9 @@  static DevPrivateKeyRec dri2WindowPrivateKeyRec;
 static DevPrivateKeyRec dri2PixmapPrivateKeyRec;
 #define dri2PixmapPrivateKey (&dri2PixmapPrivateKeyRec)
 
+static DevPrivateKeyRec dri2ClientPrivateKeyRec;
+#define dri2ClientPrivateKey (&dri2ClientPrivateKeyRec)
+
 static RESTYPE       dri2DrawableRes;
 
 typedef struct _DRI2Screen *DRI2ScreenPtr;
@@ -107,6 +110,11 @@  typedef struct _DRI2Screen {
     ConfigNotifyProcPtr		 ConfigNotify;
 } DRI2ScreenRec;
 
+typedef struct _DRI2Client {
+    CARD32 major;
+    CARD32 minor;
+} DRI2ClientRec, *DRI2ClientPtr;
+
 static DRI2ScreenPtr
 DRI2GetScreen(ScreenPtr pScreen)
 {
@@ -131,6 +139,12 @@  DRI2GetDrawable(DrawablePtr pDraw)
     }
 }
 
+static DRI2ClientPtr
+DRI2GetClient(ClientPtr client)
+{
+    return dixLookupPrivate(&client->devPrivates, dri2ClientPrivateKey);
+}
+
 static unsigned long
 DRI2DrawableSerial(DrawablePtr pDraw)
 {
@@ -190,6 +204,44 @@  DRI2AllocateDrawable(DrawablePtr pDraw)
     return pPriv;
 }
 
+void
+DRI2InitClient(ClientPtr client, CARD32 major, CARD32 minor)
+{
+    DRI2ClientPtr pPriv;
+
+    pPriv = malloc(sizeof *pPriv);
+    if (!pPriv)
+	return;
+
+    pPriv->major = major;
+    pPriv->minor = minor;
+
+    dixSetPrivate(&client->devPrivates, dri2ClientPrivateKey, pPriv);
+}
+
+void
+DRI2ClientGone(ClientPtr client)
+{
+    DRI2ClientPtr pPriv = DRI2GetClient(client);
+
+    if (pPriv)
+	free(pPriv);
+}
+
+Bool
+DRI2ClientSupportsSBC(ClientPtr client)
+{
+    DRI2ClientPtr pPriv = DRI2GetClient(client);
+
+    if (!pPriv)
+	return FALSE;
+
+    if (pPriv->major > 1 || (pPriv->major == 1 && pPriv->minor > 3))
+	return TRUE;
+
+    return FALSE;
+}
+
 typedef struct DRI2DrawableRefRec {
     XID		  id;
     XID		  dri2_id;
@@ -1097,6 +1149,9 @@  DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
     if (!dixRegisterPrivateKey(&dri2PixmapPrivateKeyRec, PRIVATE_PIXMAP, 0))
 	return FALSE;
 
+    if (!dixRegisterPrivateKey(&dri2ClientPrivateKeyRec, PRIVATE_CLIENT, 0))
+	return FALSE;
+
     ds = calloc(1, sizeof *ds);
     if (!ds)
 	return FALSE;
@@ -1114,7 +1169,7 @@  DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
 	ds->ScheduleSwap = info->ScheduleSwap;
 	ds->ScheduleWaitMSC = info->ScheduleWaitMSC;
 	ds->GetMSC = info->GetMSC;
-	cur_minor = 3;
+	cur_minor = 4;
     } else {
 	cur_minor = 1;
     }
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 4e48e65..b634bf9 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -77,6 +77,9 @@  ProcDRI2QueryVersion(ClientPtr client)
 	swaps(&stuff->length, n);
 
     REQUEST_SIZE_MATCH(xDRI2QueryVersionReq);
+
+    DRI2InitClient(client, stuff->majorVersion, stuff->minorVersion);
+
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -370,7 +373,16 @@  DRI2SwapEvent(ClientPtr client, void *data, int type, CARD64 ust, CARD64 msc,
     event.msc_hi = (CARD64)msc >> 32;
     event.msc_lo = msc & 0xffffffff;
     event.sbc_hi = (CARD64)sbc >> 32;
-    event.sbc_lo = sbc & 0xffffffff;
+
+    if (DRI2ClientSupportsSBC(client)) {
+	event.sbc_lo0 = sbc & 0xff;
+	event.sbc_lo8 = (sbc >> 8) & 0xff;
+	event.sbc_lo16 = (sbc >> 16) & 0xffff;
+    } else {
+	event.sbc_lo0 = 0;
+	event.sbc_lo8 = 0;
+	event.sbc_lo16 = 0;
+    }
 
     WriteEventsToClient(client, 1, (xEvent *)&event);
 }
diff --git a/include/protocol-versions.h b/include/protocol-versions.h
index 8692ded..8fde917 100644
--- a/include/protocol-versions.h
+++ b/include/protocol-versions.h
@@ -57,7 +57,7 @@ 
 
 /* GLX */
 #define SERVER_GLX_MAJOR_VERSION		1
-#define SERVER_GLX_MINOR_VERSION		4
+#define SERVER_GLX_MINOR_VERSION		5
 
 /* Xinerama */
 #define SERVER_PANORAMIX_MAJOR_VERSION          1