[RFC] swap event handling fixes

[RFC] swap event handling fixes

[Jesse Barnes]

I obviously failed to count the swap event structure size after adding
and removing fields a few times, and didn't even account for padding.  The
end result is that clients today won't receive the sbc_lo field at all,
and so will likely stuff junk into that field on the client side (or
zero at best).

This patchset fixes up the structure definitions, bumps the proto
levels, and adds server and client handling code for it all.  It should
be forward and backward compatible, but as always review and testing
appreciated.

I think the event_type checking on the client side still needs work;
the field is split now so I need to check the right one on old servers.
I'll also add swap support for the new requests in case people ever want
to run this stuff between big and little endian machines.

Thanks,
Jesse

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] Pack swap complete bits into an XEvent

[Jesse Barnes]

The defintion of the swap complete event was wrong; XEvents are only 32
bytes long, and with padding the swap event was longer.  So use some
creative packing to get all the bits we want transmitted.  Requires a
proto version bump.
---
 configure.ac |    2 +-
 glxproto.h   |   13 +++++++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index d88e6df..a3047e4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.60])
-AC_INIT([GLProto], [1.4.12], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([GLProto], [1.4.13], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 AM_MAINTAINER_MODE
 
diff --git a/glxproto.h b/glxproto.h
index 0ff44e3..4a583c1 100644
--- a/glxproto.h
+++ b/glxproto.h
@@ -1370,18 +1370,23 @@ typedef struct {
     CARD32 unused2 B32;
 } xGLXPbufferClobberEvent;
 
+/* Note, this struct is too large for an Xevent, I fail -- jbarnes
+ * So sbc_lo won't ever be sent.  We can use a generic event though without
+ * size restrictions, thus xGLXBufferSwapComplete2.
+ */
 typedef struct {
     BYTE type;
-    BYTE pad;
+    BYTE sbc_lo0;
     CARD16 sequenceNumber B16;
-    CARD16 event_type B16;
-    CARD32 drawable;
+    CARD8 event_type;
+    CARD8 sbc_lo8;
+    CARD16 sbc_lo16 B16;
+    CARD32 drawable B32;
     CARD32 ust_hi B32;
     CARD32 ust_lo B32;
     CARD32 msc_hi B32;
     CARD32 msc_lo B32;
     CARD32 sbc_hi B32;
-    CARD32 sbc_lo B32;
 } xGLXBufferSwapComplete;
 
 /************************************************************************/
-- 
1.7.1

Re: [PATCH] Pack swap complete bits into an XEvent

[Eric Anholt]

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

On Thu, 28 Apr 2011 13:27:19 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> The defintion of the swap complete event was wrong; XEvents are only 32
> bytes long, and with padding the swap event was longer.  So use some
> creative packing to get all the bits we want transmitted.  Requires a
> proto version bump.
> ---
>  configure.ac |    2 +-
>  glxproto.h   |   13 +++++++++----
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index d88e6df..a3047e4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1,5 +1,5 @@
>  AC_PREREQ([2.60])
> -AC_INIT([GLProto], [1.4.12], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> +AC_INIT([GLProto], [1.4.13], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
>  AM_INIT_AUTOMAKE([foreign dist-bzip2])
>  AM_MAINTAINER_MODE
>  
> diff --git a/glxproto.h b/glxproto.h
> index 0ff44e3..4a583c1 100644
> --- a/glxproto.h
> +++ b/glxproto.h
> @@ -1370,18 +1370,23 @@ typedef struct {
>      CARD32 unused2 B32;
>  } xGLXPbufferClobberEvent;
>  
> +/* Note, this struct is too large for an Xevent, I fail -- jbarnes
> + * So sbc_lo won't ever be sent.  We can use a generic event though without
> + * size restrictions, thus xGLXBufferSwapComplete2.
> + */

This comment doesn't seem to match the change.

>  typedef struct {
>      BYTE type;
> -    BYTE pad;
> +    BYTE sbc_lo0;
>      CARD16 sequenceNumber B16;
> -    CARD16 event_type B16;
> -    CARD32 drawable;
> +    CARD8 event_type;
> +    CARD8 sbc_lo8;
> +    CARD16 sbc_lo16 B16;
> +    CARD32 drawable B32;
>      CARD32 ust_hi B32;
>      CARD32 ust_lo B32;
>      CARD32 msc_hi B32;
>      CARD32 msc_lo B32;
>      CARD32 sbc_hi B32;
> -    CARD32 sbc_lo B32;
>  } xGLXBufferSwapComplete;

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

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] Pack swap complete bits into an XEvent

[Jesse Barnes]

On Thu, 28 Apr 2011 14:33:58 -0700
Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> wrote:

> On Thu, 28 Apr 2011 13:27:19 -0700, Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org> wrote:
> > The defintion of the swap complete event was wrong; XEvents are only 32
> > bytes long, and with padding the swap event was longer.  So use some
> > creative packing to get all the bits we want transmitted.  Requires a
> > proto version bump.
> > ---
> >  configure.ac |    2 +-
> >  glxproto.h   |   13 +++++++++----
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index d88e6df..a3047e4 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1,5 +1,5 @@
> >  AC_PREREQ([2.60])
> > -AC_INIT([GLProto], [1.4.12], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> > +AC_INIT([GLProto], [1.4.13], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> >  AM_INIT_AUTOMAKE([foreign dist-bzip2])
> >  AM_MAINTAINER_MODE
> >  
> > diff --git a/glxproto.h b/glxproto.h
> > index 0ff44e3..4a583c1 100644
> > --- a/glxproto.h
> > +++ b/glxproto.h
> > @@ -1370,18 +1370,23 @@ typedef struct {
> >      CARD32 unused2 B32;
> >  } xGLXPbufferClobberEvent;
> >  
> > +/* Note, this struct is too large for an Xevent, I fail -- jbarnes
> > + * So sbc_lo won't ever be sent.  We can use a generic event though without
> > + * size restrictions, thus xGLXBufferSwapComplete2.
> > + */
> 
> This comment doesn't seem to match the change.

double fail. will fix.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Pack swap complete bits into an XEvent

[Keith Packard]

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

On Thu, 28 Apr 2011 13:27:19 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> The defintion of the swap complete event was wrong; XEvents are only 32
> bytes long, and with padding the swap event was longer.  So use some
> creative packing to get all the bits we want transmitted.  Requires a
> proto version bump.

If you've got a proto version on both sides, you should just stick the
low 32 bits in place of the event type and pad fields, rather than
abusing. Yes, this requires two different structures in the X server,
but you need to do that anyways -- the event type has to shrink from 16
bits to 8 bits, moving it at the same time is not a problem.

Besides, this will make the xcb case really ugly as xcb doesn't hide the
protocol event structures from the application.

-- 
keith.packard@intel.com

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

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] Fix swap complete event size

[Jesse Barnes]

XEvents are limited to 32 bytes, so use some creative stuffing to fit
all the bits we'd like to supply.
---
 configure.ac |    2 +-
 dri2proto.h  |    9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5b78d6b..9505f56 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.60])
-AC_INIT([DRI2Proto], [2.3], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([DRI2Proto], [2.4], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 AM_MAINTAINER_MODE
 
diff --git a/dri2proto.h b/dri2proto.h
index 9708a4a..e568c91 100644
--- a/dri2proto.h
+++ b/dri2proto.h
@@ -35,7 +35,7 @@
 
 #define DRI2_NAME			"DRI2"
 #define DRI2_MAJOR			1
-#define DRI2_MINOR			3
+#define DRI2_MINOR			4
 
 #define DRI2NumberErrors		0
 #define DRI2NumberEvents		2
@@ -287,16 +287,17 @@ typedef struct {
 
 typedef struct {
     CARD8 type;
-    CARD8 pad;
+    CARD8 sbc_lo0;
     CARD16 sequenceNumber B16;
-    CARD16 event_type B16;
+    CARD8 event_type;
+    CARD8 sbc_lo8;
+    CARD16 sbc_lo16 B16;
     CARD32 drawable B32;
     CARD32 ust_hi B32;
     CARD32 ust_lo B32;
     CARD32 msc_hi B32;
     CARD32 msc_lo B32;
     CARD32 sbc_hi B32;
-    CARD32 sbc_lo B32;
 } xDRI2BufferSwapComplete;
 #define sz_xDRI2BufferSwapComplete 32
 
-- 
1.7.1

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [RFC] swap event handling fixes

[Jesse Barnes]

On Thu, 28 Apr 2011 13:27:18 -0700
Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org> wrote:

> I obviously failed to count the swap event structure size after adding
> and removing fields a few times, and didn't even account for padding.  The
> end result is that clients today won't receive the sbc_lo field at all,
> and so will likely stuff junk into that field on the client side (or
> zero at best).
> 
> This patchset fixes up the structure definitions, bumps the proto
> levels, and adds server and client handling code for it all.  It should
> be forward and backward compatible, but as always review and testing
> appreciated.
> 
> I think the event_type checking on the client side still needs work;
> the field is split now so I need to check the right one on old servers.
> I'll also add swap support for the new requests in case people ever want
> to run this stuff between big and little endian machines.

Btw, I've been trying to test these fixes but I can't make the existing
code break; I'm getting a nicely incrementing sbc count using the
piglit event test (after adding some code to dump the swap event
fields), so somehow the bits are getting through.  I just don't know
how yet...

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [RFC] swap event handling fixes

[Mario Kleiner]

On Apr 29, 2011, at 11:37 PM, Jesse Barnes wrote:

> On Thu, 28 Apr 2011 13:27:18 -0700
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>
>> I obviously failed to count the swap event structure size after  
>> adding
>> and removing fields a few times, and didn't even account for  
>> padding.  The
>> end result is that clients today won't receive the sbc_lo field at  
>> all,
>> and so will likely stuff junk into that field on the client side (or
>> zero at best).
>>
>> This patchset fixes up the structure definitions, bumps the proto
>> levels, and adds server and client handling code for it all.  It  
>> should
>> be forward and backward compatible, but as always review and testing
>> appreciated.
>>
>> I think the event_type checking on the client side still needs work;
>> the field is split now so I need to check the right one on old  
>> servers.
>> I'll also add swap support for the new requests in case people  
>> ever want
>> to run this stuff between big and little endian machines.
>
> Btw, I've been trying to test these fixes but I can't make the  
> existing
> code break; I'm getting a nicely incrementing sbc count using the
> piglit event test (after adding some code to dump the swap event
> fields), so somehow the bits are getting through.  I just don't know
> how yet...
>

One thing you could try as another quick test is to use xtrace on a  
opengl app. I remember seeing weird results for the msc or ust fields  
(i believe) of decoded and printed DRI2SwapComplete events. I always  
thought part of my toolchain was just out of sync, but maybe it was  
just the result of the wrong size/padding.

-mario

> -- 
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [RFC] swap event handling fixes

[Jesse Barnes]

On Sat, 30 Apr 2011 01:10:27 +0200
Mario Kleiner <mario.kleiner@tuebingen.mpg.de> wrote:

> 
> On Apr 29, 2011, at 11:37 PM, Jesse Barnes wrote:
> 
> > On Thu, 28 Apr 2011 13:27:18 -0700
> > Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >
> >> I obviously failed to count the swap event structure size after  
> >> adding
> >> and removing fields a few times, and didn't even account for  
> >> padding.  The
> >> end result is that clients today won't receive the sbc_lo field at  
> >> all,
> >> and so will likely stuff junk into that field on the client side (or
> >> zero at best).
> >>
> >> This patchset fixes up the structure definitions, bumps the proto
> >> levels, and adds server and client handling code for it all.  It  
> >> should
> >> be forward and backward compatible, but as always review and testing
> >> appreciated.
> >>
> >> I think the event_type checking on the client side still needs work;
> >> the field is split now so I need to check the right one on old  
> >> servers.
> >> I'll also add swap support for the new requests in case people  
> >> ever want
> >> to run this stuff between big and little endian machines.
> >
> > Btw, I've been trying to test these fixes but I can't make the  
> > existing
> > code break; I'm getting a nicely incrementing sbc count using the
> > piglit event test (after adding some code to dump the swap event
> > fields), so somehow the bits are getting through.  I just don't know
> > how yet...
> >
> 
> One thing you could try as another quick test is to use xtrace on a  
> opengl app. I remember seeing weird results for the msc or ust fields  
> (i believe) of decoded and printed DRI2SwapComplete events. I always  
> thought part of my toolchain was just out of sync, but maybe it was  
> just the result of the wrong size/padding.

What I'm seeing is a result of whatever xlib sticks in that space I
think.  Now I'm seeing bogus values and am testing the fix.

However, for the GLX event, I will have to use a new generic event,
since the GLX event types are larger than 8 bits due to the way GLX
enumerates things.  For DRI2 I can re-pack the existing structure.
Will post updated patches soon.

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH] GLX/DRI2: fix swap complete handling

[Jesse Barnes]

The swap complete event wasn't being handled fully; because XEvents are
only 32 bytes long, we were getting junk for the sbc_lo value.  If the
server supports it, unpack the new structure, otherwise just return 0
for the sbc value instead of garbage.
---
 configure.ac        |    4 ++--
 src/glx/dri2.c      |    7 ++++++-
 src/glx/dri2_glx.c  |   13 +++++++++++++
 src/glx/glxclient.h |    1 +
 src/glx/glxext.c    |    8 +++++++-
 5 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3b05ca3..94fb6f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -21,8 +21,8 @@ dnl Versions for external dependencies
 LIBDRM_REQUIRED=2.4.24
 LIBDRM_RADEON_REQUIRED=2.4.24
 LIBDRM_INTEL_REQUIRED=2.4.24
-DRI2PROTO_REQUIRED=2.1
-GLPROTO_REQUIRED=1.4.11
+DRI2PROTO_REQUIRED=2.4
+GLPROTO_REQUIRED=1.4.13
 LIBDRM_XORG_REQUIRED=2.4.24
 LIBKMS_XORG_REQUIRED=1.0.0
 
diff --git a/src/glx/dri2.c b/src/glx/dri2.c
index adfd3d1..864c941 100644
--- a/src/glx/dri2.c
+++ b/src/glx/dri2.c
@@ -124,7 +124,12 @@ DRI2WireToEvent(Display *dpy, XEvent *event, xEvent *wire)
       }
       aevent->ust = ((CARD64)awire->ust_hi << 32) | awire->ust_lo;
       aevent->msc = ((CARD64)awire->msc_hi << 32) | awire->msc_lo;
-      aevent->sbc = ((CARD64)awire->sbc_hi << 32) | awire->sbc_lo;
+      if (dri2ServerSupportsSBC(dpy)) {
+	 aevent->sbc = ((CARD64)awire->sbc_hi << 32) | (awire->sbc_lo16 << 16) |
+	    (awire->sbc_lo8 << 8) | awire->sbc_lo0;
+      } else {
+	 aevent->sbc = 0;
+      }
       return True;
    }
 #endif
diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index fc0237a..be96ec6 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -791,6 +791,19 @@ static const struct glx_screen_vtable dri2_screen_vtable = {
    dri2_create_context
 };
 
+int
+dri2ServerSupportsSBC(Display *dpy)
+{
+   struct glx_display *dpyPriv = __glXInitialize(dpy);
+   struct dri2_display *pdp =
+      (struct dri2_display *) dpyPriv->dri2Display;
+
+   if (pdp->driMajor > 1 || (pdp->driMajor == 1 && pdp->driMinor > 3))
+      return 1;
+
+   return 0;
+}
+
 static struct glx_screen *
 dri2CreateScreen(int screen, struct glx_display * priv)
 {
diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
index 2b6966f..acba82c 100644
--- a/src/glx/glxclient.h
+++ b/src/glx/glxclient.h
@@ -149,6 +149,7 @@ extern __GLXDRIdisplay *driCreateDisplay(Display * dpy);
 extern __GLXDRIdisplay *dri2CreateDisplay(Display * dpy);
 extern void dri2InvalidateBuffers(Display *dpy, XID drawable);
 extern unsigned dri2GetSwapEventType(Display *dpy, XID drawable);
+extern int dri2ServerSupportsSBC(Display *dpy);
 
 
 /*
diff --git a/src/glx/glxext.c b/src/glx/glxext.c
index 278c719..7393efc 100644
--- a/src/glx/glxext.c
+++ b/src/glx/glxext.c
@@ -138,7 +138,13 @@ __glXWireToEvent(Display *dpy, XEvent *event, xEvent *wire)
       aevent->drawable = awire->drawable;
       aevent->ust = ((CARD64)awire->ust_hi << 32) | awire->ust_lo;
       aevent->msc = ((CARD64)awire->msc_hi << 32) | awire->msc_lo;
-      aevent->sbc = ((CARD64)awire->sbc_hi << 32) | awire->sbc_lo;
+      if (glx_dpy->majorVersion > 1 || (glx_dpy->majorVersion == 1 &&
+					glx_dpy->minorVersion > 4)) {
+ 	 aevent->sbc = ((CARD64)awire->sbc_hi << 32) | (awire->sbc_lo16 << 16) |
+	    (awire->sbc_lo8 << 8) | awire->sbc_lo0;
+     } else {
+	 aevent->sbc = 0;
+      }
       return True;
    }
    default:
-- 
1.7.1

[PATCH] DRI2/GLX: fix swap event handling

[Jesse Barnes]

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(-)

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
-- 
1.7.1

Re: [PATCH] DRI2/GLX: fix swap event handling

[Eric Anholt]

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

On Thu, 28 Apr 2011 13:27:22 -0700, Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.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.

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

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

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] DRI2/GLX: fix swap event handling

[Michel Dänzer]

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.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] DRI2/GLX: fix swap event handling

[Julien Cristau]

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

Re: [PATCH] DRI2/GLX: fix swap event handling

[Jesse Barnes]

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?

-- 
Jesse Barnes, Intel Open Source Technology Center