Re: [Intel-gfx] gitlab.fd.o financial situation and impact on services

[Kristian Høgsberg]

On Thu, Feb 27, 2020 at 7:38 PM Dave Airlie  wrote:
>
> On Fri, 28 Feb 2020 at 07:27, Daniel Vetter  wrote:
> >
> > Hi all,
> >
> > You might have read the short take in the X.org board meeting minutes
> > already, here's the long version.
> >
> > The good news: gitlab.fd.o has become very popular with our
> > communities, and is used extensively. This especially includes all the
> > CI integration. Modern development process and tooling, yay!
> >
> > The bad news: The cost in growth has also been tremendous, and it's
> > breaking our bank account. With reasonable estimates for continued
> > growth we're expecting hosting expenses totalling 75k USD this year,
> > and 90k USD next year. With the current sponsors we've set up we can't
> > sustain that. We estimate that hosting expenses for gitlab.fd.o
> > without any of the CI features enabled would total 30k USD, which is
> > within X.org's ability to support through various sponsorships, mostly
> > through XDC.
> >
> > Note that X.org does no longer sponsor any CI runners themselves,
> > we've stopped that. The huge additional expenses are all just in
> > storing and serving build artifacts and images to outside CI runners
> > sponsored by various companies. A related topic is that with the
> > growth in fd.o it's becoming infeasible to maintain it all on
> > volunteer admin time. X.org is therefore also looking for admin
> > sponsorship, at least medium term.
> >
> > Assuming that we want cash flow reserves for one year of gitlab.fd.o
> > (without CI support) and a trimmed XDC and assuming no sponsor payment
> > meanwhile, we'd have to cut CI services somewhere between May and June
> > this year. The board is of course working on acquiring sponsors, but
> > filling a shortfall of this magnitude is neither easy nor quick work,
> > and we therefore decided to give an early warning as soon as possible.
> > Any help in finding sponsors for fd.o is very much appreciated.
>
> a) Ouch.
>
> b) we probably need to take a large step back here.

If we're taking a step back here, I also want to recognize what a
tremendous success this has been so far and thank everybody involved
for building something so useful. Between gitlab and the CI, our
workflow has improved and code quality has gone up.  I don't have
anything useful to add to the technical discussion, except that that
it seems pretty standard engineering practice to build a system,
observe it and identify and eliminate bottlenecks. Planning never
hurts, of course, but I don't think anybody could have realistically
modeled and projected the cost of this infrastructure as it's grown
organically and fast.

Kristian

> Look at this from a sponsor POV, why would I give X.org/fd.o
> sponsorship money that they are just giving straight to google to pay
> for hosting credits? Google are profiting in some minor way from these
> hosting credits being bought by us, and I assume we aren't getting any
> sort of discounts here. Having google sponsor the credits costs google
> substantially less than having any other company give us money to do
> it.
>
> If our current CI architecture is going to burn this amount of money a
> year and we hadn't worked this out in advance of deploying it then I
> suggest the system should be taken offline until we work out what a
> sustainable system would look like within the budget we have, whether
> that be never transferring containers and build artifacts from the
> google network, just having local runner/build combos etc.
>
> Dave.
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/2] Introduce keyboard grabbing protocol for Xwayland

[Kristian Høgsberg]

On Fri, Apr 21, 2017 at 5:50 AM Olivier Fourdan  wrote:

>
> Hi Jonas,
>
> > > [...]
> > > For that last point, I'd rather use:
> > >
> > > * does not guarantee that events sent to this client are
> continuous,
> > >   a compositor may change and reroute keyboard events while the
> grab
> > >   is nominally active.
> > >
> > > > hmm, and thinking about the last point: do we need to specify what
> the
> > > > focus
> > > > behaviour is if a grab is active? I'm assuming 'no' because there is
> no
> > > > notification whatsoever whether it ever activates anyway, but...
> > >
> > > No, indeed, that's precisely the main difference with the shortcuts
> > > inhibitor logic for the wayland native apps.
> > >
> > > Here, keyboards events are routed to the surface regardless of the
> focus,
> > > just like an X11 grab.
> >
> > Should this part really be respected though? In what circumstances does
> > it make sense to route input events to Xwayland when a Wayland client is
> > the one focused?
>
> Yes, that's precisely the whole point of this protocol (and grabs in
> general) :)
>
> Take the attached sample code for example, it maps a single override
> redirect window (one that no X11 window manager would focus, because it's
> an O-R) and issues an active keyboard grab to get all keyboard events.
>

What is the use case that this is trying to support though? Which Xwayland
apps are you trying to support with this?


> While that works fine in X11 native, that won't work in Wayland/Xwayland
> without this protocol and the ability to route keyboard events even when
> the surface is not focused.
>
> > > > Last question: how will you deal with XGrabKeyboard() requests on
> already
> > > > grabbed keyboards? I can send that request twice with different grab
> > > > windows
> > > > and it'll change the grab over (iirc). Xwayland will destroy the
> current
> > > > grab and request a new one?
> > >
> > > Yeap, good point, "XGrabKeyboard overrides any active keyboard grab by
> the
> > > client" so Xwayland needs to destroy any current grab before setting a
> new
> > > one in this case.
> > >
> >
> > FWIW, this will create a minor race, where any key presses between the
> > .destroy and .grab requests will be ungrabbed.
>
> Yeah, you're right, but I reckon it's acceptable.
>
> Cheers,
> Olivier
>
> ___
> wayland-devel mailing list
> wayland-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] xwayland: Use drm buffers for cursors if available

[Kristian Høgsberg]

On Wed, Feb 10, 2016 at 7:39 AM, Emil Velikov  wrote:
> Hi all,
>
> Just a small note:
>
> On 5 February 2016 at 08:42, Pekka Paalanen  wrote:
>
>> umm, do we really want to add even more uses of the Mesa-private wl_drm
>> protocol outside of Mesa? Though it seems that ship sailed a long time
>> ago.
>>
>> Should we just bite the bullet, and have Mesa install drm.xml for
>> public consumption? Or move drm.xml to wayland-protocols.git?
>>
>> I thought all that was a Bad Idea(tm).
>>
> If we consider that distributing the protocol for public consumption
> is OK, please use wayland-protocols.git. Keeping it in mesa will lead
> to nasty circular dependencies.

I think we should. It's pretty much the equivalent of dri2proto and
it's useful for projects such as libva as well.

> Thanks
> Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] xwayland: Group multiple cursor buffers per shm pool

[Kristian Høgsberg]

On Tue, Dec 1, 2015 at 7:48 AM, Rui Matos  wrote:
> Each shm pool implies a file descriptor which means that currently, we
> can quickly exhaust the available FDs since we create a shm pool per
> cursor buffer. Instead, this patch creates shm pools big enough to
> contain multiple cursor buffers to avoid hitting the FD limit on
> reasonable workloads.
>
> On my testing, most cursors require 2304 bytes so I made the pools be
> 256 kB meaning that each can hold around 100 buffers.
>
> Signed-off-by: Rui Matos 
> ---
>
> This fixes bug https://bugzilla.gnome.org/show_bug.cgi?id=758255 . The
> use case reported there, which is running two instances of
> ImageMagick's display tool, makes xwayland realize exactly 1024
> cursors which breaks due to FD exhaustion before this patch.
>
> Note that there's no smart allocation algorithm here, it just
> allocates more pools as needed and only frees them after all cursors
> inside a pool are freed which, I'm afraid, will lead to terrible
> fragmentation over time.
>
> Our shm pixmap usage suffers from the same problem of using one pool
> per pixmap which seems problematic too but since most setups should be
> able to use drm for pixmaps it's not as pressing a problem to fix. At
> first I attempted to use drm buffers for cursors too but failed to get
> it working since I got lost finding a way to copy the cursor data into
> the buffers. I'm happy to do that instead if someone points me to how
> to do it.

Nice work. I think it would be about the same amount of work to do
this for shm pixmaps instead of cursors. That way, an application that
creates a lot of small pixmaps on shm-Xwayland (non-glamor) will also
not crash the server either.

Kristian

>  hw/xwayland/xwayland-cursor.c | 68 +++---
>  hw/xwayland/xwayland-shm.c| 76 
> +++
>  hw/xwayland/xwayland.h|  6 
>  3 files changed, 138 insertions(+), 12 deletions(-)
>
> diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
> index 76729db..e5b88a5 100644
> --- a/hw/xwayland/xwayland-cursor.c
> +++ b/hw/xwayland/xwayland-cursor.c
> @@ -28,7 +28,17 @@
>
>  #include 
>
> +#define SHM_POOL_SIZE (1 << 18) /* 256 kB */
> +
> +struct xwl_cursor {
> +struct xorg_list link;
> +struct xwl_shm_pool *pool;
> +struct wl_buffer *buffer;
> +char *data;
> +};
> +
>  static DevPrivateKeyRec xwl_cursor_private_key;
> +static struct xorg_list xwl_cursor_list;
>
>  static void
>  expand_source_and_mask(CursorPtr cursor, CARD32 *data)
> @@ -63,23 +73,55 @@ expand_source_and_mask(CursorPtr cursor, CARD32 *data)
>  static Bool
>  xwl_realize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
>  {
> -PixmapPtr pixmap;
> +struct xwl_cursor *xwl_cursor;
> +struct xwl_shm_pool *pool = NULL;
> +
> +xwl_cursor = malloc(sizeof *xwl_cursor);
> +if (!xwl_cursor)
> +return FALSE;
> +
> +if (!xorg_list_is_empty(_cursor_list)) {
> +pool = (xorg_list_first_entry(_cursor_list, struct xwl_cursor, 
> link))->pool;
> +xwl_cursor->buffer = xwl_shm_pool_allocate(pool, cursor->bits->width,
> +   cursor->bits->height, 
> _cursor->data);
> +if (!xwl_cursor->buffer)
> +pool = NULL;
> +}
>
> -pixmap = xwl_shm_create_pixmap(screen, cursor->bits->width,
> -   cursor->bits->height, 32, 0);
> -dixSetPrivate(>devPrivates, _cursor_private_key, pixmap);
> +if (!pool) {
> +pool = xwl_shm_pool_create(xwl_screen_get(screen)->shm, 
> SHM_POOL_SIZE);
> +if (!pool)
> +goto err_free;
> +xwl_cursor->buffer = xwl_shm_pool_allocate(pool, cursor->bits->width,
> +   cursor->bits->height, 
> _cursor->data);
> +if (!xwl_cursor->buffer)
> +goto err_free;
> +}
> +
> +xwl_cursor->pool = pool;
> +
> +xorg_list_add(_cursor->link, _cursor_list);
> +dixSetPrivate(>devPrivates, _cursor_private_key, xwl_cursor);
>
>  return TRUE;
> +
> + err_free:
> +free(xwl_cursor);
> +return FALSE;
>  }
>
>  static Bool
>  xwl_unrealize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
>  {
> -PixmapPtr pixmap;
> +struct xwl_cursor *xwl_cursor;
>
> -pixmap = dixGetPrivate(>devPrivates, _cursor_private_key);
> +xwl_cursor = dixGetPrivate(>devPrivates, 
> _cursor_private_key);
> +wl_buffer_destroy(xwl_cursor->buffer);
> +xwl_shm_pool_unref(xwl_cursor->pool);
> +xorg_list_del(_cursor->link);
> +free(xwl_cursor);
>
> -return xwl_shm_destroy_pixmap(pixmap);
> +return TRUE;
>  }
>
>  static void
> @@ -102,7 +144,7 @@ static const struct wl_callback_listener frame_listener = 
> {
>  void
>  xwl_seat_set_cursor(struct xwl_seat *xwl_seat)
>  {
> -PixmapPtr pixmap;
> +struct xwl_cursor 

[PATCH] xwayland: Add glamor and DRI3 support

[Kristian Høgsberg]

Reviewed-by: Axel Davy axel.d...@ens.fr
Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---

This patch comes in after the 1.16 feature freeze, but we had to wait for
mesa 10.2 to come out.  The mesa release brings the new gbm api that we
use to implement dri3 and glamor support in Xwayland.

As can be seen from the diffstat below, the only change outside hw/xwayland
is the configure.ac change to look for a new enough gbm and to look up the
wayland-scanner binary.

Kristian

 configure.ac  |   6 +-
 hw/xwayland/Makefile.am   |  31 ++-
 hw/xwayland/drm.xml   | 182 ++
 hw/xwayland/xwayland-glamor.c | 570 ++
 hw/xwayland/xwayland.c|  38 ++-
 hw/xwayland/xwayland.h|  17 ++
 6 files changed, 837 insertions(+), 7 deletions(-)
 create mode 100644 hw/xwayland/drm.xml
 create mode 100644 hw/xwayland/xwayland-glamor.c

diff --git a/configure.ac b/configure.ac
index 0a6e772..2056323 100644
--- a/configure.ac
+++ b/configure.ac
@@ -810,7 +810,7 @@ LIBDMX=dmx = 1.0.99.1
 LIBDRI=dri = 7.8.0
 LIBDRM=libdrm = 2.3.0
 LIBEGL=egl
-LIBGBM=gbm = 9
+LIBGBM=gbm = 10.2.0
 LIBGL=gl = 7.1.0
 LIBXEXT=xext = 1.0.99.4
 LIBXFONT=xfont = 1.4.2
@@ -2459,6 +2459,10 @@ if test x$XWAYLAND = xyes; then
XWAYLAND_SYS_LIBS=$XWAYLANDMODULES_LIBS $GLX_SYS_LIBS
AC_SUBST([XWAYLAND_LIBS])
AC_SUBST([XWAYLAND_SYS_LIBS])
+
+   WAYLAND_PREFIX=`$PKG_CONFIG --variable=prefix wayland-client`
+   AC_PATH_PROG([WAYLAND_SCANNER], [wayland-scanner],,
+[${WAYLAND_PREFIX}/bin$PATH_SEPARATOR$PATH])
 fi
 
 
diff --git a/hw/xwayland/Makefile.am b/hw/xwayland/Makefile.am
index 36e6127..dc16b8b 100644
--- a/hw/xwayland/Makefile.am
+++ b/hw/xwayland/Makefile.am
@@ -1,10 +1,13 @@
 bin_PROGRAMS = Xwayland
 
 Xwayland_CFLAGS =  \
+   -I$(top_srcdir)/glamor  \
-I$(top_srcdir)/dri3\
-DHAVE_DIX_CONFIG_H \
$(XWAYLANDMODULES_CFLAGS)   \
-   $(DIX_CFLAGS)
+   $(DIX_CFLAGS)   \
+   $(GLAMOR_CFLAGS)\
+   $(GBM_CFLAGS)
 
 Xwayland_SOURCES = \
xwayland.c  \
@@ -19,6 +22,7 @@ Xwayland_SOURCES =\
$(top_srcdir)/mi/miinitext.c
 
 Xwayland_LDADD =   \
+   $(glamor_lib)   \
$(XWAYLAND_LIBS)\
$(XWAYLAND_SYS_LIBS)\
$(XSERVER_SYS_LIBS)
@@ -26,5 +30,30 @@ Xwayland_DEPENDENCIES = $(XWAYLAND_LIBS)
 Xwayland_LDFLAGS = $(LD_EXPORT_SYMBOLS_FLAG)
 
 
+if GLAMOR_EGL
+Xwayland_SOURCES += xwayland-glamor.c
+
+nodist_Xwayland_SOURCES =  \
+   drm-client-protocol.h   \
+   drm-protocol.c
+
+CLEANFILES = $(nodist_Xwayland_SOURCES)
+
+EXTRA_DIST = drm.xml
+
+xwayland-glamor.c : $(nodist_Xwayland_SOURCES)
+
+glamor_lib = $(top_builddir)/glamor/libglamor.la
+
+Xwayland_LDADD += $(GLAMOR_LIBS) $(GBM_LIBS) -lEGL -lGL
+endif
+
+
 relink:
$(AM_V_at)rm -f Xwayland$(EXEEXT)  $(MAKE) Xwayland$(EXEEXT)
+
+%-protocol.c : %.xml
+   $(AM_V_GEN)$(WAYLAND_SCANNER) code  $  $@
+
+%-client-protocol.h : %.xml
+   $(AM_V_GEN)$(WAYLAND_SCANNER) client-header  $  $@
diff --git a/hw/xwayland/drm.xml b/hw/xwayland/drm.xml
new file mode 100644
index 000..8a3ad69
--- /dev/null
+++ b/hw/xwayland/drm.xml
@@ -0,0 +1,182 @@
+?xml version=1.0 encoding=UTF-8?
+protocol name=drm
+
+  copyright
+Copyright © 2008-2011 Kristian Høgsberg
+Copyright © 2010-2011 Intel Corporation
+
+Permission to use, copy, modify, distribute, and sell this
+software and its documentation for any purpose is hereby granted
+without fee, provided that\n the above copyright notice appear in
+all copies and that both that copyright notice and this permission
+notice appear in supporting documentation, and that the name of
+the copyright holders not be used in advertising or publicity
+pertaining to distribution of the software without specific,
+written prior permission.  The copyright holders make no
+representations about the suitability of this software for any
+purpose.  It is provided as is without express or implied
+warranty.
+
+THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
+AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
+ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
+THIS SOFTWARE.
+  /copyright
+
+  !-- drm support. This object is created

[PULL] Xwayland fixes

[Kristian Høgsberg]

The following changes since commit c7011249d2abe6cc7af82ee4b79d8f6873444707:

  xkb: Verify reads of compiled keymap header and TOC (2014-04-18
16:30:18 -0700)

are available in the git repository at:

  ssh://people.freedesktop.org/~krh/xserver xwayland-for-keithp

for you to fetch changes up to 66b602474047c499b8a888267a489790fc9f9d85:

  xwayland: Remove left-over ErrorF logging (2014-04-21 11:25:12 -0700)


Kristian Høgsberg (4):
  xwayland: Build without GLX extension
  xwayland: Build without xshmfence
  configure.ac: Remove check for WAYLAND_SCANNER_RULES
  xwayland: Remove left-over ErrorF logging

 configure.ac   |  1 -
 hw/xwayland/xwayland.c | 10 +++---
 2 files changed, 7 insertions(+), 4 deletions(-)
___
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

[PATCH 3/3] configure.ac: Remove check for WAYLAND_SCANNER_RULES

[Kristian Høgsberg]

This makes configure fail if the wayland autoconf macros aren't found.
We don't need the scanner for shm-only xwayland so just drop this line for
now.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 configure.ac | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 70eceab..cad4ceb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2458,7 +2458,6 @@ if test x$XWAYLAND = xyes; then
XWAYLAND_SYS_LIBS=$XWAYLANDMODULES_LIBS $GLX_SYS_LIBS
AC_SUBST([XWAYLAND_LIBS])
AC_SUBST([XWAYLAND_SYS_LIBS])
-   WAYLAND_SCANNER_RULES(['$(top_srcdir)/hw/xwayland'])
 fi
 
 
-- 
1.9.0

___
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

[PATCH 1/3] xwayland: Build without GLX extension

[Kristian Høgsberg]

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 hw/xwayland/xwayland.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index c2c6481..5cecefd 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -616,8 +616,10 @@ xwl_log_handler(const char *format, va_list args)
 FatalError(%s, msg);
 }
 
-static const ExtensionModule glx_extension[] = {
+static const ExtensionModule xwayland_extensions[] = {
+#ifdef GLXEXT
 { GlxExtensionInit, GLX, noGlxExtension },
+#endif
 };
 
 void
@@ -639,7 +641,8 @@ InitOutput(ScreenInfo * screen_info, int argc, char **argv)
 screen_info-bitmapBitOrder = BITMAP_BIT_ORDER;
 screen_info-numPixmapFormats = ARRAY_SIZE(depths);
 
-LoadExtensionList(glx_extension, ARRAY_SIZE(glx_extension), FALSE);
+LoadExtensionList(xwayland_extensions,
+  ARRAY_SIZE(xwayland_extensions), FALSE);
 
 /* Cast away warning from missing printf annotation for
  * wl_log_func_t.  Wayland 1.5 will have the annotation, so we can
-- 
1.9.0

___
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

[PATCH 2/3] xwayland: Build without xshmfence

[Kristian Høgsberg]

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 hw/xwayland/xwayland.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 5cecefd..844745a 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -573,8 +573,10 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 
 fbPictureInit(pScreen, 0, 0);
 
+#ifdef HAVE_XSHMFENCE
 if (!miSyncShmScreenInit(pScreen))
 return FALSE;
+#endif
 
 xwl_screen-wayland_fd = wl_display_get_fd(xwl_screen-display);
 AddGeneralSocket(xwl_screen-wayland_fd);
-- 
1.9.0

___
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: Xwayland DDX: build breaks in configuration

[Kristian Høgsberg]

On Sun, Apr 6, 2014 at 8:05 AM, Gaetan Nadon mems...@videotron.ca wrote:
 On 14-04-06 06:02 AM, Yaakov (Cygwin/X) wrote:

 Any idea how to fix this? I would prefer that the RC I'm still trying to
 construct will build for most people...


 Adding a copy of wayland-scanner.m4 to m4/ should work.

 This would work if this macro does not itself introduces more dependencies.
 The drawback is dual maintenance, as it must be updated when the original
 one is changed. In addition, a user could end up with both out-of-sync
 macros being on the search path, not really aware of which one will be
 picked-up.

 Another approach is to adopt the common idiom for auto detected features.
 If the macro is missing, for whatever reason, disable wayland building. It
 will require some m4 coding.

 Something like this:

We only need the wayland scanner for when we add the glamor + dri3
support, so we can just drop the WAYLAND_SCANNER_RULES line from
configure for now.

When I send the patch for glamor + dri3 support, I'll change this to
do what we do for mesa: just look for the scanner binary and then add
the four lines of Makefile.am to generate the files.  I've given up on
trying to share the Makefile.am logic for this, it's more work than
just copy-and-pasting this:

%-protocol.c : %.xml
$(AM_V_GEN)$(WAYLAND_SCANNER) code  $  $@

%-client-protocol.h : %.xml
$(AM_V_GEN)$(WAYLAND_SCANNER) client-header  $  $@

Anyway, patch is on the list to remove the configure.ac macro along
with patches to build without GLX and xshmfence.

Kristian

 diff --git a/configure.ac b/configure.ac
 index 70eceab..b13f180 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -2442,6 +2442,8 @@ AM_CONDITIONAL(XFAKESERVER, [test x$KDRIVE = xyes 
 test x$XFAKE = xyes])
  dnl Xwayland DDX

  PKG_CHECK_MODULES(XWAYLANDMODULES, [wayland-client libdrm epoxy],
 [have_xwayland=yes], [have_xwayland=no])
 +m4_ifdef([WAYLAND_SCANNER_RULES], [have_xwayland=yes], [have_xwayland=no])
 +
  AC_MSG_CHECKING([whether to build Xwayland DDX])
  if test x$XWAYLAND = xauto; then
 XWAYLAND=$have_xwayland
 @@ -2458,7 +2460,8 @@ if test x$XWAYLAND = xyes; then
  XWAYLAND_SYS_LIBS=$XWAYLANDMODULES_LIBS $GLX_SYS_LIBS
  AC_SUBST([XWAYLAND_LIBS])
  AC_SUBST([XWAYLAND_SYS_LIBS])
 -WAYLAND_SCANNER_RULES(['$(top_srcdir)/hw/xwayland'])
 +m4_ifdef([WAYLAND_SCANNER_RULES],
 WAYLAND_SCANNER_RULES(['$(top_srcdir)/hw/xwayland']))
 +
  fi

 Not tested with xwayland installed, but no more build breaks without
 xwayland. Worth a try.

 ___
 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
___
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: [PATCH 2/3] xwayland: Build without xshmfence

[Kristian Høgsberg]

On Tue, Apr 8, 2014 at 10:13 AM, Daniel Stone dan...@fooishbar.org wrote:
 Hi,

 On 8 April 2014 17:24, Kristian Høgsberg k...@bitplanet.net wrote:

 diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
 index 5cecefd..844745a 100644
 --- a/hw/xwayland/xwayland.c
 +++ b/hw/xwayland/xwayland.c
 @@ -573,8 +573,10 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char
 **argv)

  fbPictureInit(pScreen, 0, 0);

 +#ifdef HAVE_XSHMFENCE
  if (!miSyncShmScreenInit(pScreen))
  return FALSE;
 +#endif

  xwl_screen-wayland_fd = wl_display_get_fd(xwl_screen-display);
  AddGeneralSocket(xwl_screen-wayland_fd);


 Doesn't this also need the syncshm.h include guarded?

No, the header is just the protoype for miSyncShmScreenInit().

Kristian

 The other two, though:
 Reviewed-by: Daniel Stone dan...@fooishbar.org

 Cheers,
 Daniel
___
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: Xwayland series v2

[Kristian Høgsberg]

On Thu, Apr 3, 2014 at 3:38 PM, Keith Packard kei...@keithp.com wrote:
 Kristian Høgsberg k...@bitplanet.net writes:

 Here's an updated version of the Xwayland series.

 Thanks to Kristian for getting this cleaned up today and ready for the
 merge.
3c34dd3..b4d0bec  master - master

Thanks Keith.  As a heads up, we decided to pull out the glamor
support for now as it depends on unreleased gbm changes in mesa
master.  The commit to add glamor support on top is here:

  http://cgit.freedesktop.org/~krh/xserver/log/?h=xwayland-with-glamor

If we can get mesa released before X goes into code freeze (beginning
of June) we'll try to merge the glamor patch in time for 1.16.  With
all infrastructure patches already merged, the glamor patch only
affects the Xwayland DDX.

Kristian


 --
 keith.pack...@intel.com
___
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

[PATCH 2/7] composite: Add exception mechanism for implicit redirection policy

[Kristian Høgsberg]

Normally composite will decide to add implicit redirection when a
window with an alternate visual is a parent of a window with a regular
visual or vice versa.  This uses extra pixmap memory and incurs an extra
copy.  This exception mechanism provides a way for a DDX to override this
if the DDX knows that its acceleration architecture will render correctly.

The relevant case is that of an RGB window reparented into a ARGB parent
frame window.  If the DDX knows that the acceleration architecture in use
will pad the alpha channel to opaque when rendering to the RGB window,
the implicit redirection can be avoided.

This patch adds a new composite function:

  CompositeRegisterImplicitRedirectionException()

which lets a DDX register a pair of parent and child window visuals, that
will not be implicitly redirected even if the normal policy would have
made that choice.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
Reviewed-by: Keith Packard kei...@keithp.com
---
 composite/compinit.c | 24 
 composite/compint.h  |  7 +++
 composite/compositeext.h |  4 
 composite/compwindow.c   | 18 ++
 4 files changed, 53 insertions(+)

diff --git a/composite/compinit.c b/composite/compinit.c
index 1db9e0b..48e938f 100644
--- a/composite/compinit.c
+++ b/composite/compinit.c
@@ -229,6 +229,28 @@ CompositeRegisterAlternateVisuals(ScreenPtr pScreen, 
VisualID * vids,
 return compRegisterAlternateVisuals(cs, vids, nVisuals);
 }
 
+Bool
+CompositeRegisterImplicitRedirectionException(ScreenPtr pScreen,
+  VisualID parentVisual,
+  VisualID winVisual)
+{
+CompScreenPtr cs = GetCompScreen(pScreen);
+CompImplicitRedirectException *p;
+
+p = realloc(cs-implicitRedirectExceptions,
+sizeof(p[0]) * (cs-numImplicitRedirectExceptions + 1));
+if (p == NULL)
+return FALSE;
+
+p[cs-numImplicitRedirectExceptions].parentVisual = parentVisual;
+p[cs-numImplicitRedirectExceptions].winVisual = winVisual;
+
+cs-implicitRedirectExceptions = p;
+cs-numImplicitRedirectExceptions++;
+
+return TRUE;
+}
+
 typedef struct _alternateVisual {
 int depth;
 CARD32 format;
@@ -349,6 +371,8 @@ compScreenInit(ScreenPtr pScreen)
 
 cs-numAlternateVisuals = 0;
 cs-alternateVisuals = NULL;
+cs-numImplicitRedirectExceptions = 0;
+cs-implicitRedirectExceptions = NULL;
 
 if (!compAddAlternateVisuals(pScreen, cs)) {
 free(cs);
diff --git a/composite/compint.h b/composite/compint.h
index 12dc8b3..56b76c5 100644
--- a/composite/compint.h
+++ b/composite/compint.h
@@ -119,6 +119,11 @@ typedef struct _CompOverlayClientRec {
 XID resource;
 } CompOverlayClientRec;
 
+typedef struct _CompImplicitRedirectException {
+XID parentVisual;
+XID winVisual;
+} CompImplicitRedirectException;
+
 typedef struct _CompScreen {
 PositionWindowProcPtr PositionWindow;
 CopyWindowProcPtr CopyWindow;
@@ -155,6 +160,8 @@ typedef struct _CompScreen {
 CloseScreenProcPtr CloseScreen;
 int numAlternateVisuals;
 VisualID *alternateVisuals;
+int numImplicitRedirectExceptions;
+CompImplicitRedirectException *implicitRedirectExceptions;
 
 WindowPtr pOverlayWin;
 Window overlayWid;
diff --git a/composite/compositeext.h b/composite/compositeext.h
index 0b148f0..b96cb1d 100644
--- a/composite/compositeext.h
+++ b/composite/compositeext.h
@@ -35,6 +35,10 @@ extern _X_EXPORT Bool 
CompositeRegisterAlternateVisuals(ScreenPtr pScreen,
 VisualID * vids,
 int nVisuals);
 
+extern _X_EXPORT Bool CompositeRegisterImplicitRedirectionException(ScreenPtr 
pScreen,
+VisualID 
parentVisual,
+VisualID 
winVisual);
+
 extern _X_EXPORT RESTYPE CompositeClientWindowType;
 
 #endif  /* _COMPOSITEEXT_H_ */
diff --git a/composite/compwindow.c b/composite/compwindow.c
index 6c24349..8824294 100644
--- a/composite/compwindow.c
+++ b/composite/compwindow.c
@@ -336,6 +336,21 @@ compIsAlternateVisual(ScreenPtr pScreen, XID visual)
 }
 
 static Bool
+compIsImplicitRedirectException(ScreenPtr pScreen,
+XID parentVisual, XID winVisual)
+{
+CompScreenPtr cs = GetCompScreen(pScreen);
+int i;
+
+for (i = 0; i  cs-numImplicitRedirectExceptions; i++)
+if (cs-implicitRedirectExceptions[i].parentVisual == parentVisual 
+cs-implicitRedirectExceptions[i].winVisual == winVisual)
+return TRUE;
+
+return FALSE;
+}
+
+static Bool
 compImplicitRedirect(WindowPtr pWin, WindowPtr pParent)
 {
 if (pParent) {
@@ -343,6 +358,9 @@ compImplicitRedirect(WindowPtr pWin, WindowPtr pParent)
 XID

[PATCH 1/7] events: Make XYToWindow a screen function pointer

[Kristian Høgsberg]

This allows DDXen to override the window picking to account for
native windows not seen by the X server.  The bulk of the picking logic
is exposed as a new helper function, miSpriteTrace().  This function
completes the sprite trace filled out by the caller, and can be set up
to start the search from a given toplevel window.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 dix/events.c | 99 +---
 dix/touch.c  | 10 --
 include/cursor.h |  2 ++
 include/input.h  |  1 -
 include/scrnintstr.h |  4 +++
 mi/mi.h  |  3 ++
 mi/miscrinit.c   | 99 
 7 files changed, 124 insertions(+), 94 deletions(-)

diff --git a/dix/events.c b/dix/events.c
index f05dada..601c1a4 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -565,7 +565,7 @@ XineramaConstrainCursor(DeviceIntPtr pDev)
 (*pScreen-ConstrainCursor) (pDev, pScreen, newBox);
 }
 
-static Bool
+Bool
 XineramaSetWindowPntrs(DeviceIntPtr pDev, WindowPtr pWin)
 {
 SpritePtr pSprite = pDev-spriteInfo-sprite;
@@ -1302,6 +1302,7 @@ static void
 ComputeFreezes(void)
 {
 DeviceIntPtr replayDev = syncEvents.replayDev;
+ScreenPtr pScreen;
 WindowPtr w;
 GrabPtr grab;
 DeviceIntPtr dev;
@@ -1318,8 +1319,9 @@ ComputeFreezes(void)
 
 syncEvents.replayDev = (DeviceIntPtr) NULL;
 
-w = XYToWindow(replayDev-spriteInfo-sprite,
-   event-root_x, event-root_y);
+pScreen = RootWindow(replayDev-spriteInfo-sprite)-drawable.pScreen;
+w = (*pScreen-XYToWindow) (replayDev, replayDev-spriteInfo-sprite,
+event-root_x, event-root_y);
 if (!CheckDeviceGrabs(replayDev, event, syncEvents.replayWin)) {
 if (IsTouchEvent((InternalEvent *) event)) {
 TouchPointInfoPtr ti =
@@ -2835,92 +2837,6 @@ DeliverEvents(WindowPtr pWin, xEvent *xE, int count, 
WindowPtr otherParent)
 return deliveries;
 }
 
-static Bool
-PointInBorderSize(WindowPtr pWin, int x, int y)
-{
-BoxRec box;
-
-if (RegionContainsPoint(pWin-borderSize, x, y, box))
-return TRUE;
-
-#ifdef PANORAMIX
-if (!noPanoramiXExtension 
-XineramaSetWindowPntrs(inputInfo.pointer, pWin)) {
-SpritePtr pSprite = inputInfo.pointer-spriteInfo-sprite;
-int i;
-
-FOR_NSCREENS_FORWARD_SKIP(i) {
-if (RegionContainsPoint(pSprite-windows[i]-borderSize,
-x + screenInfo.screens[0]-x -
-screenInfo.screens[i]-x,
-y + screenInfo.screens[0]-y -
-screenInfo.screens[i]-y, box))
-return TRUE;
-}
-}
-#endif
-return FALSE;
-}
-
-/**
- * Traversed from the root window to the window at the position x/y. While
- * traversing, it sets up the traversal history in the spriteTrace array.
- * After completing, the spriteTrace history is set in the following way:
- *   spriteTrace[0] ... root window
- *   spriteTrace[1] ... top level window that encloses x/y
- *   ...
- *   spriteTrace[spriteTraceGood - 1] ... window at x/y
- *
- * @returns the window at the given coordinates.
- */
-WindowPtr
-XYToWindow(SpritePtr pSprite, int x, int y)
-{
-WindowPtr pWin;
-BoxRec box;
-
-pSprite-spriteTraceGood = 1;   /* root window still there */
-pWin = RootWindow(pSprite)-firstChild;
-while (pWin) {
-if ((pWin-mapped) 
-(x = pWin-drawable.x - wBorderWidth(pWin)) 
-(x  pWin-drawable.x + (int) pWin-drawable.width +
- wBorderWidth(pWin)) 
-(y = pWin-drawable.y - wBorderWidth(pWin)) 
-(y  pWin-drawable.y + (int) pWin-drawable.height +
- wBorderWidth(pWin))
-/* When a window is shaped, a further check
- * is made to see if the point is inside
- * borderSize
- */
- (!wBoundingShape(pWin) || PointInBorderSize(pWin, x, y))
- (!wInputShape(pWin) ||
-RegionContainsPoint(wInputShape(pWin),
-x - pWin-drawable.x,
-y - pWin-drawable.y, box))
-#ifdef ROOTLESS
-/* In rootless mode windows may be offscreen, even when
- * they're in X's stack. (E.g. if the native window system
- * implements some form of virtual desktop system).
- */
- !pWin-rootlessUnhittable
-#endif
-) {
-if (pSprite-spriteTraceGood = pSprite-spriteTraceSize) {
-pSprite-spriteTraceSize += 10;
-pSprite-spriteTrace = realloc(pSprite-spriteTrace,
-   pSprite-spriteTraceSize *
-   sizeof(WindowPtr));
-}
-pSprite-spriteTrace

[PATCH 3/7] dri3: Allow asynchronous implementation for dri3_open

[Kristian Høgsberg]

By passing the client pointer to the dri3_open implementation, we allow
the clients to implement the open callback asynchronously.  If the
client ignore count is positive after returning from dri3_open, we
assume that authentication is in progress and doesn't send the reply.
The code to send the reply is moved into a helper function, which the
implementation can call upon receiving its authenticaion reply.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 dri3/dri3.h |  6 +-
 dri3/dri3_request.c | 38 --
 dri3/dri3_screen.c  |  2 +-
 glamor/glamor_egl.c |  3 ++-
 4 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/dri3/dri3.h b/dri3/dri3.h
index 7c0c330..1c04cbd 100644
--- a/dri3/dri3.h
+++ b/dri3/dri3.h
@@ -32,7 +32,8 @@
 
 #define DRI3_SCREEN_INFO_VERSION0
 
-typedef int (*dri3_open_proc)(ScreenPtr screen,
+typedef int (*dri3_open_proc)(ClientPtr client,
+  ScreenPtr screen,
   RRProviderPtr provider,
   int *fd);
 
@@ -60,6 +61,9 @@ typedef struct dri3_screen_info {
 extern _X_EXPORT Bool
 dri3_screen_init(ScreenPtr screen, dri3_screen_info_ptr info);
 
+extern _X_EXPORT int
+dri3_send_open_reply(ClientPtr client, int fd);
+
 #endif
 
 #endif /* _DRI3_H_ */
diff --git a/dri3/dri3_request.c b/dri3/dri3_request.c
index 31dce83..fe45620 100644
--- a/dri3/dri3_request.c
+++ b/dri3/dri3_request.c
@@ -55,16 +55,35 @@ proc_dri3_query_version(ClientPtr client)
 return Success;
 }
 
-static int
-proc_dri3_open(ClientPtr client)
+int
+dri3_send_open_reply(ClientPtr client, int fd)
 {
-REQUEST(xDRI3OpenReq);
 xDRI3OpenReply rep = {
 .type = X_Reply,
 .nfd = 1,
 .sequenceNumber = client-sequence,
 .length = 0,
 };
+
+if (client-swapped) {
+swaps(rep.sequenceNumber);
+swapl(rep.length);
+}
+
+if (WriteFdToClient(client, fd, TRUE)  0) {
+close(fd);
+return BadAlloc;
+}
+
+WriteToClient(client, sizeof (rep), rep);
+
+return Success;
+}
+
+static int
+proc_dri3_open(ClientPtr client)
+{
+REQUEST(xDRI3OpenReq);
 RRProviderPtr provider;
 DrawablePtr drawable;
 ScreenPtr screen;
@@ -92,17 +111,8 @@ proc_dri3_open(ClientPtr client)
 if (status != Success)
 return status;
 
-if (client-swapped) {
-swaps(rep.sequenceNumber);
-swapl(rep.length);
-}
-
-if (WriteFdToClient(client, fd, TRUE)  0) {
-close(fd);
-return BadAlloc;
-}
-
-WriteToClient(client, sizeof (rep), rep);
+if (client-ignoreCount == 0)
+return dri3_send_open_reply(client, fd);
 
 return Success;
 }
diff --git a/dri3/dri3_screen.c b/dri3/dri3_screen.c
index c880296..bbf1d05 100644
--- a/dri3/dri3_screen.c
+++ b/dri3/dri3_screen.c
@@ -40,7 +40,7 @@ dri3_open(ClientPtr client, ScreenPtr screen, RRProviderPtr 
provider, int *fd)
 if (!info || !info-open)
 return BadMatch;
 
-rc = (*info-open) (screen, provider, fd);
+rc = (*info-open) (client, screen, provider, fd);
 if (rc != Success)
 return rc;
 
diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c
index 8123421..e2b6a92 100644
--- a/glamor/glamor_egl.c
+++ b/glamor/glamor_egl.c
@@ -608,7 +608,8 @@ glamor_egl_close_screen(ScreenPtr screen)
 }
 
 static int
-glamor_dri3_open(ScreenPtr screen,
+glamor_dri3_open(ClientPtr client,
+ ScreenPtr screen,
  RRProviderPtr provider,
  int *fdp)
 {
-- 
1.9.0

___
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

Xwayland series v2

[Kristian Høgsberg]

Hi,

Here's an updated version of the Xwayland series.  This takes into account
the patches from last series that are already upstream and adds support for
glamor acceleration, DRI3+present and render nodes (when available).
To that end, there are a few more patches outside Xwayland in this series,
notably a few small glamor fixes, a modification to the DRI3 API to
allow asynchronous dri3_open and a mechanism to disable implicit
redirection for certain parent/child visual combinations.

The series is rebased on 9d20d18fb and has been updated to not link in
fbcmap_mi.c as per Jon TURNEYs change.

Thanks to Axel Davy for all the help in testing and debugging this.

Kristian

___
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

[PATCH 5/7] glamor: Add new GLAMOR_CREATE_PIXMAP_NO_TEXTURE pixmap create flag

[Kristian Høgsberg]

This flag lets a DDX allocate a glamor pixmap without allocating the
texture that backs it.  The DDX can then allocate the texture itself
and then set it later.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 glamor/glamor.c | 10 +-
 glamor/glamor.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 3094432..8d24531 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -182,7 +182,15 @@ glamor_create_pixmap(ScreenPtr screen, int w, int h, int 
depth,
 pitch = (((w * pixmap-drawable.bitsPerPixel + 7) / 8) + 3)  ~3;
 screen-ModifyPixmapHeader(pixmap, w, h, 0, 0, pitch, NULL);
 
-if (type == GLAMOR_MEMORY_MAP || usage == GLAMOR_CREATE_NO_LARGE ||
+if (usage == GLAMOR_CREATE_PIXMAP_NO_TEXTURE) {
+pixmap_priv-type = GLAMOR_TEXTURE_ONLY;
+pixmap_priv-base.box.x1 = 0;
+pixmap_priv-base.box.y1 = 0;
+pixmap_priv-base.box.x2 = w;
+pixmap_priv-base.box.y2 = h;
+return pixmap;
+}
+else if (type == GLAMOR_MEMORY_MAP || usage == GLAMOR_CREATE_NO_LARGE ||
 glamor_check_fbo_size(glamor_priv, w, h))
 {
 pixmap_priv-type = type;
diff --git a/glamor/glamor.h b/glamor/glamor.h
index 84b6736..913bdce 100644
--- a/glamor/glamor.h
+++ b/glamor/glamor.h
@@ -149,6 +149,7 @@ extern _X_EXPORT PixmapPtr glamor_create_pixmap(ScreenPtr 
screen, int w, int h,
 #define GLAMOR_CREATE_FBO_NO_FBO0x103
 #define GLAMOR_CREATE_PIXMAP_MAP0x104
 #define GLAMOR_CREATE_NO_LARGE  0x105
+#define GLAMOR_CREATE_PIXMAP_NO_TEXTURE 0x106
 
 /* @glamor_egl_exchange_buffers: Exchange the underlying buffers(KHR 
image,fbo).
  *
-- 
1.9.0

___
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

[PATCH 4/7] glamor: Move glamor_egl_screen_init() prototype to glamor.h

[Kristian Høgsberg]

A DDX that implements the glamor EGL functions need to pull in this
prototype but shouldn't need to pull in glamor_priv.h

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 glamor/glamor.h   | 2 ++
 glamor/glamor_egl_stubs.c | 4 +++-
 glamor/glamor_priv.h  | 3 ---
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/glamor/glamor.h b/glamor/glamor.h
index 11ec493..84b6736 100644
--- a/glamor/glamor.h
+++ b/glamor/glamor.h
@@ -310,6 +310,8 @@ extern _X_EXPORT Bool
 
 #endif
 
+extern _X_EXPORT void glamor_egl_screen_init(ScreenPtr screen,
+ struct glamor_context 
*glamor_ctx);
 extern _X_EXPORT void glamor_egl_destroy_textured_pixmap(PixmapPtr pixmap);
 
 extern _X_EXPORT int glamor_create_gc(GCPtr gc);
diff --git a/glamor/glamor_egl_stubs.c b/glamor/glamor_egl_stubs.c
index 4fd9e80..028d1cc 100644
--- a/glamor/glamor_egl_stubs.c
+++ b/glamor/glamor_egl_stubs.c
@@ -26,7 +26,9 @@
  * Stubbed out glamor_egl.c functions for servers other than Xorg.
  */
 
-#include glamor_priv.h
+#include dix-config.h
+
+#include glamor.h
 
 void
 glamor_egl_screen_init(ScreenPtr screen, struct glamor_context *glamor_ctx)
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 4c305ab..6aa2f74 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -992,9 +992,6 @@ void glamor_composite_rectangles(CARD8 op,
  xRenderColor *color,
  int num_rects, xRectangle *rects);
 
-extern _X_EXPORT void glamor_egl_screen_init(ScreenPtr screen,
- struct glamor_context 
*glamor_ctx);
-
 /* glamor_xv */
 typedef struct {
 uint32_t transform_index;
-- 
1.9.0

___
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

[PATCH 6/7] glamor: Expose glamor_destroy_pixmap()

[Kristian Høgsberg]

When we create a glamor pixmap by calling glamor_create_pixmap()
directly, we need to call glamor_destroy_pixmap() to destroy it.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 glamor/glamor.h  | 1 +
 glamor/glamor_priv.h | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/glamor/glamor.h b/glamor/glamor.h
index 913bdce..27c9cb3 100644
--- a/glamor/glamor.h
+++ b/glamor/glamor.h
@@ -143,6 +143,7 @@ extern _X_EXPORT void glamor_block_handler(ScreenPtr 
screen);
 
 extern _X_EXPORT PixmapPtr glamor_create_pixmap(ScreenPtr screen, int w, int h,
 int depth, unsigned int usage);
+extern _X_EXPORT Bool glamor_destroy_pixmap(PixmapPtr pixmap);
 
 #define GLAMOR_CREATE_PIXMAP_CPU0x100
 #define GLAMOR_CREATE_PIXMAP_FIXUP  0x101
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 6aa2f74..e8925a6 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -584,8 +584,6 @@ extern int glamor_debug_level;
 /* glamor.c */
 PixmapPtr glamor_get_drawable_pixmap(DrawablePtr drawable);
 
-Bool glamor_destroy_pixmap(PixmapPtr pixmap);
-
 glamor_pixmap_fbo *glamor_pixmap_detach_fbo(glamor_pixmap_private *
 pixmap_priv);
 void glamor_pixmap_attach_fbo(PixmapPtr pixmap, glamor_pixmap_fbo *fbo);
-- 
1.9.0

___
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: [PATCH 6/6] Xwayland DDX

[Kristian Høgsberg]

On Tue, Apr 01, 2014 at 12:10:09AM -0700, Keith Packard wrote:
 Kristian Høgsberg k...@bitplanet.net writes:
 
  +static WindowPtr
  +xwl_xy_to_window(DeviceIntPtr master, SpritePtr sprite, int x, int y)
 
 With my previous adjustment to leave the existing public XYToWindow API
 unchanged, xwl_xy_to_window will need to figure out which pointer device
 the sprite is associated with. Here's a patch which should obviously
 be correct; it should be easy to reduce this to a single pass over the
 input devices though.

Indeed, I'll send out a v2 of the Wayland DDX patch.

Kristian

 diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
 index 452789f..99ae5bd 100644
 --- a/hw/xwayland/xwayland-input.c
 +++ b/hw/xwayland/xwayland-input.c
 @@ -599,10 +599,15 @@ DDXRingBell(int volume, int pitch, int duration)
  }
  
  static WindowPtr
 -xwl_xy_to_window(DeviceIntPtr master, SpritePtr sprite, int x, int y)
 +xwl_xy_to_window(ScreenPtr screen, SpritePtr sprite, int x, int y)
  {
  struct xwl_seat *xwl_seat = NULL;
  DeviceIntPtr device;
 +DeviceIntPtr master;
 +
 +for (master = inputInfo.devices; master; master = master-next)
 +if (master-spriteInfo  master-spriteInfo-sprite == sprite)
 +break;
  
  for (device = inputInfo.devices; device; device = device-next)
  if (GetMaster(device, MASTER_ATTACHED) == master) {
 
 -- 
 keith.pack...@intel.com


___
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

[PATCH] dri3: Allow asynchronous implementation for dri3_open

[Kristian Høgsberg]

By passing the client pointer to the dri3_open implementation, we allow
the clients to implement the open callback asynchronously.  If the
client ignore count is positive after returning from dri3_open, we
assume that authentication is in progress and doesn't send the reply.
The code to send the reply is moved into a helper function, which the
implementation can call upon receiving its authenticaion reply.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 dri3/dri3.h |  6 +-
 dri3/dri3_request.c | 38 --
 dri3/dri3_screen.c  |  2 +-
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/dri3/dri3.h b/dri3/dri3.h
index 7c0c330..1c04cbd 100644
--- a/dri3/dri3.h
+++ b/dri3/dri3.h
@@ -32,7 +32,8 @@
 
 #define DRI3_SCREEN_INFO_VERSION0
 
-typedef int (*dri3_open_proc)(ScreenPtr screen,
+typedef int (*dri3_open_proc)(ClientPtr client,
+  ScreenPtr screen,
   RRProviderPtr provider,
   int *fd);
 
@@ -60,6 +61,9 @@ typedef struct dri3_screen_info {
 extern _X_EXPORT Bool
 dri3_screen_init(ScreenPtr screen, dri3_screen_info_ptr info);
 
+extern _X_EXPORT int
+dri3_send_open_reply(ClientPtr client, int fd);
+
 #endif
 
 #endif /* _DRI3_H_ */
diff --git a/dri3/dri3_request.c b/dri3/dri3_request.c
index 31dce83..fe45620 100644
--- a/dri3/dri3_request.c
+++ b/dri3/dri3_request.c
@@ -55,16 +55,35 @@ proc_dri3_query_version(ClientPtr client)
 return Success;
 }
 
-static int
-proc_dri3_open(ClientPtr client)
+int
+dri3_send_open_reply(ClientPtr client, int fd)
 {
-REQUEST(xDRI3OpenReq);
 xDRI3OpenReply rep = {
 .type = X_Reply,
 .nfd = 1,
 .sequenceNumber = client-sequence,
 .length = 0,
 };
+
+if (client-swapped) {
+swaps(rep.sequenceNumber);
+swapl(rep.length);
+}
+
+if (WriteFdToClient(client, fd, TRUE)  0) {
+close(fd);
+return BadAlloc;
+}
+
+WriteToClient(client, sizeof (rep), rep);
+
+return Success;
+}
+
+static int
+proc_dri3_open(ClientPtr client)
+{
+REQUEST(xDRI3OpenReq);
 RRProviderPtr provider;
 DrawablePtr drawable;
 ScreenPtr screen;
@@ -92,17 +111,8 @@ proc_dri3_open(ClientPtr client)
 if (status != Success)
 return status;
 
-if (client-swapped) {
-swaps(rep.sequenceNumber);
-swapl(rep.length);
-}
-
-if (WriteFdToClient(client, fd, TRUE)  0) {
-close(fd);
-return BadAlloc;
-}
-
-WriteToClient(client, sizeof (rep), rep);
+if (client-ignoreCount == 0)
+return dri3_send_open_reply(client, fd);
 
 return Success;
 }
diff --git a/dri3/dri3_screen.c b/dri3/dri3_screen.c
index c880296..bbf1d05 100644
--- a/dri3/dri3_screen.c
+++ b/dri3/dri3_screen.c
@@ -40,7 +40,7 @@ dri3_open(ClientPtr client, ScreenPtr screen, RRProviderPtr 
provider, int *fd)
 if (!info || !info-open)
 return BadMatch;
 
-rc = (*info-open) (screen, provider, fd);
+rc = (*info-open) (client, screen, provider, fd);
 if (rc != Success)
 return rc;
 
-- 
1.9.0

___
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: [PATCH] mi: Hush extension initialization (#75870)

[Kristian Høgsberg]

On Thu, Mar 27, 2014 at 8:48 AM, Adam Jackson a...@redhat.com wrote:
 Printing these as ErrorF is fairly obnoxious, since it means the
 non-hardware servers now spew stuff to the console for entirely routine
 events.  And actually, printing these at all is fairly obnoxious, since
 a) we're printing a line for every extension, whether it's enabled or
 not, and b) we're not actually initializing the extension at this point.

I can get behind that.

Reviewed-by: Kristian Høgsberg k...@bitplanet.net

 Signed-off-by: Adam Jackson a...@redhat.com
 ---
  mi/miinitext.c | 8 
  1 file changed, 8 deletions(-)

 diff --git a/mi/miinitext.c b/mi/miinitext.c
 index 5b45ab4..1d90516 100644
 --- a/mi/miinitext.c
 +++ b/mi/miinitext.c
 @@ -365,7 +365,6 @@ void
  LoadExtensionList(const ExtensionModule ext[], int size, Bool builtin)
  {
  ExtensionModule *newext;
 -const char *msg;
  int i;

  /* Make sure built-in extensions get added to the list before those
 @@ -375,14 +374,7 @@ LoadExtensionList(const ExtensionModule ext[], int size, 
 Bool builtin)
  if (!(newext = NewExtensionModuleList(size)))
  return;

 -if (builtin)
 -msg = Initializing built-in;
 -else
 -msg = Loading;
 -
  for (i = 0; i  size; i++, newext++) {
 -ErrorF(%s extension %s\n, msg, ext[i].name);
 -
  newext-name = ext[i].name;
  newext-initFunc = ext[i].initFunc;
  newext-disablePtr = ext[i].disablePtr;
 --
 1.8.5.3

 ___
 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
___
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: [PATCH libxtrans v2] configure: Also add -D_DEFAULT_SOURCE to .pc cflags to shut up glibc warnings

[Kristian Høgsberg]

On Tue, Mar 25, 2014 at 7:16 AM, Hans de Goede hdego...@redhat.com wrote:
 The latest glibc considers _BSD_SOURCE deprecated, leading to the following
 warning being issued for pretty much every C-file in the xserver:

 In file included from /usr/include/stdint.h:25:0,
  from 
 /usr/lib/gcc/x86_64-redhat-linux/4.8.2/include/stdint.h:9,
  from ../include/misc.h:81,
  from miexpose.c:82:
 /usr/include/features.h:145:3: warning: #warning _BSD_SOURCE and 
 _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE [-Wcpp]
  # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE
^

 I've discussed this with the glibc developers and the prefered way of fixing
 this is by also defining _DEFAULT_SOURCE which is the new way of stating
 _BSD_SOURCE / _SVID_SOURCE .

 Signed-off-by: Hans de Goede hdego...@redhat.com

Tested-by: Kristian Høgsberg k...@bitplanet.net
Reviewed-by: Kristian Høgsberg k...@bitplanet.net

This makes the warnings go away for me and the patch does the right
thing according to features.h:

/* _BSD_SOURCE and _SVID_SOURCE are deprecated aliases for
   _DEFAULT_SOURCE.  If _DEFAULT_SOURCE is present we do not
   issue a warning; the expectation is that the source is being
   transitioned to use the new macro.  */
#if (defined _BSD_SOURCE || defined _SVID_SOURCE) \
 !defined _DEFAULT_SOURCE
# warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE
# undef  _DEFAULT_SOURCE
# define _DEFAULT_SOURCE1
#endif

It's worth mentioning that there are development versions of glibc
that doesn't have the  !defined _DEFAULT_SOURCE part in the condition
and will warn even with this patch in Xtrans.  That's what I had when
I first tested the patch - Fedora RPM 0:2.19.90-3.fc21.  In that case,
get a more recent glibc development snapshot.  Upgrading to
2.19.90-7.fc21 fixed it for me.

Kristian

 ---
  xtrans.pc.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/xtrans.pc.in b/xtrans.pc.in
 index 90d19b1..b8d135b 100644
 --- a/xtrans.pc.in
 +++ b/xtrans.pc.in
 @@ -6,4 +6,4 @@ includedir=@includedir@
  Name: XTrans
  Description: Abstract network code for X
  Version: @PACKAGE_VERSION@
 -Cflags: -I${includedir} -D_BSD_SOURCE @fchown_define@ @sticky_bit_define@
 +Cflags: -I${includedir} -D_DEFAULT_SOURCE -D_BSD_SOURCE @fchown_define@ 
 @sticky_bit_define@
 --
 1.9.0

 ___
 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
___
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: [PATCH] Handle -displayfd and an explicit display number sensibly

[Kristian Høgsberg]

On Tue, Mar 25, 2014 at 9:39 AM, Jon TURNEY jon.tur...@dronecode.org.uk wrote:
 On 24/03/2014 19:56, Kristian Høgsberg wrote:
 On Wed, Mar 19, 2014 at 7:26 AM, Jon TURNEY wrote:
 Handle -displayfd and an explicit display number sensibly, e.g. use the
 explicitly specified display number, and write it to the displayfd

 I  don't think the two options were meant to be used together, but I
 can see how that would be useful.  Non-signal based ready notification
 while letting the parent process pick the display is useful for
 Xwayland as well.

 Well, in the alternative, the X server could exit with an error if -displayfd
 and an explicit display number are used, but this seems more useful.

 The parent process creates the fd in our process environment and could
 pick 0 as the display fd.  Let's initialize to -1 which is never a
 valid fd.

 A very good point.  Amended patch to account for this attached.


Reviewed-by: Kristian Høgsberg k...@bitplanet.net


___
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

[PATCH 2/6] os: Always compile ListenOnOpenFD() and export it

[Kristian Høgsberg]

This function was written to allow the X server to inherit the listen
socket from launchd on OS X.  The code is not specific to OS X though
and will be useful for on-demand launched Xwayland servers.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 include/os.h| 4 +---
 os/connection.c | 7 ++-
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/include/os.h b/include/os.h
index e5f86d6..90229e6 100644
--- a/include/os.h
+++ b/include/os.h
@@ -166,9 +166,7 @@ extern _X_EXPORT void MakeClientGrabImpervious(ClientPtr 
/*client */ );
 
 extern _X_EXPORT void MakeClientGrabPervious(ClientPtr /*client */ );
 
-#ifdef XQUARTZ
-extern void ListenOnOpenFD(int /* fd */ , int /* noxauth */ );
-#endif
+extern _X_EXPORT void ListenOnOpenFD(int /* fd */ , int /* noxauth */ );
 
 extern _X_EXPORT CARD32 GetTimeInMillis(void);
 extern _X_EXPORT CARD64 GetTimeInMicros(void);
diff --git a/os/connection.c b/os/connection.c
index ddf4f0a..400c542 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -1253,8 +1253,7 @@ MakeClientGrabPervious(ClientPtr client)
 }
 }
 
-#ifdef XQUARTZ
-/* Add a fd (from launchd) to our listeners */
+/* Add a fd (from launchd or similar) to our listeners */
 void
 ListenOnOpenFD(int fd, int noxauth)
 {
@@ -1276,7 +1275,7 @@ ListenOnOpenFD(int fd, int noxauth)
  */
 ciptr = _XSERVTransReopenCOTSServer(5, fd, port);
 if (ciptr == NULL) {
-ErrorF(Got NULL while trying to Reopen launchd port.\n);
+ErrorF(Got NULL while trying to Reopen listen port.\n);
 return;
 }
 
@@ -1308,5 +1307,3 @@ ListenOnOpenFD(int fd, int noxauth)
 XdmcpReset();
 #endif
 }
-
-#endif
-- 
1.9.0

___
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

[PATCH 4/6] os: Add AddClientOnOpenFD() to create a new client for an file descriptor

[Kristian Høgsberg]

When the Xwayland server is socket-activated, we need to connect and
initialize the window manager before the activating client gets to
proceed with connecting.  We do this by passing a socket file
descriptor for the window manager connection to the Xwayland server,
which then uses this new function to set it up as an X client.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 include/os.h|  2 ++
 os/connection.c | 27 +++
 2 files changed, 29 insertions(+)

diff --git a/include/os.h b/include/os.h
index 90229e6..d26e399 100644
--- a/include/os.h
+++ b/include/os.h
@@ -168,6 +168,8 @@ extern _X_EXPORT void MakeClientGrabPervious(ClientPtr 
/*client */ );
 
 extern _X_EXPORT void ListenOnOpenFD(int /* fd */ , int /* noxauth */ );
 
+extern _X_EXPORT Bool AddClientOnOpenFD(int /* fd */ );
+
 extern _X_EXPORT CARD32 GetTimeInMillis(void);
 extern _X_EXPORT CARD64 GetTimeInMicros(void);
 
diff --git a/os/connection.c b/os/connection.c
index b50f9e9..b3640b8 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -1312,3 +1312,30 @@ ListenOnOpenFD(int fd, int noxauth)
 XdmcpReset();
 #endif
 }
+
+/* based on TRANS(SocketUNIXAccept) (XtransConnInfo ciptr, int *status) */
+Bool
+AddClientOnOpenFD(int fd)
+{
+XtransConnInfo ciptr;
+CARD32 connect_time;
+char port[20];
+
+snprintf(port, sizeof(port), :%d, atoi(display));
+ciptr = _XSERVTransReopenCOTSServer(5, fd, port);
+if (ciptr == NULL)
+return FALSE;
+
+_XSERVTransSetOption(ciptr, TRANS_NONBLOCKING, 1);
+ciptr-flags |= TRANS_NOXAUTH;
+
+connect_time = GetTimeInMillis();
+
+if (!AllocNewConnection(ciptr, fd, connect_time)) {
+ErrorConnMax(ciptr);
+_XSERVTransClose(ciptr);
+return FALSE;
+}
+
+return TRUE;
+}
-- 
1.9.0

___
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

[PATCH 3/6] os: Add a mechanism to prevent creating any listen sockets

[Kristian Høgsberg]

A socket-activated server will receive its listening sockets from the
parent process and should not create its own sockets.  This patch
introduces a NoListen flag that can be set by a DDX to prevent
the server from creating the sockets.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 include/opaque.h | 1 +
 os/connection.c  | 9 +++--
 os/utils.c   | 4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/opaque.h b/include/opaque.h
index 73d40c3..7ec1d85 100644
--- a/include/opaque.h
+++ b/include/opaque.h
@@ -74,5 +74,6 @@ extern _X_EXPORT Bool whiteRoot;
 extern _X_EXPORT Bool bgNoneRoot;
 
 extern _X_EXPORT Bool CoreDump;
+extern _X_EXPORT Bool NoListenAll;
 
 #endif  /* OPAQUE_H */
diff --git a/os/connection.c b/os/connection.c
index 400c542..b50f9e9 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -138,6 +138,7 @@ fd_set OutputPending;   /* clients with reply/event 
data ready to go */
 int MaxClients = 0;
 Bool NewOutputPending;  /* not yet attempted to write some new output 
*/
 Bool AnyClientsWriteBlocked;/* true if some client blocked on write */
+Bool NoListenAll;   /* Don't establish any listening sockets */
 
 static Bool RunFromSmartParent; /* send SIGUSR1 to parent process */
 Bool RunFromSigStopParent;  /* send SIGSTOP to our own process; Upstart (or
@@ -406,7 +407,10 @@ CreateWellKnownSockets(void)
 /* display is initialized to 0 by main(). It is then set to the display
  * number if specified on the command line, or to NULL when the -displayfd
  * option is used. */
-if (display) {
+if (NoListenAll) {
+ListenTransCount = 0;
+}
+else if (display) {
 if (TryCreateSocket(atoi(display), partial) 
 ListenTransCount = 1)
 if (!PartialNetwork  partial)
@@ -440,9 +444,10 @@ CreateWellKnownSockets(void)
 DefineSelf (fd);
 }
 
-if (!XFD_ANYSET(WellKnownConnections))
+if (!XFD_ANYSET(WellKnownConnections)  !NoListenAll)
 FatalError
 (Cannot establish any listening sockets - Make sure an X server 
isn't already running);
+
 #if !defined(WIN32)
 OsSignal(SIGPIPE, SIG_IGN);
 OsSignal(SIGHUP, AutoResetServer);
diff --git a/os/utils.c b/os/utils.c
index 497779b..c513968 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -270,7 +270,7 @@ LockServer(void)
 int len;
 char port[20];
 
-if (nolock)
+if (nolock || NoListenAll)
 return;
 /*
  * Path names
@@ -390,7 +390,7 @@ LockServer(void)
 void
 UnlockServer(void)
 {
-if (nolock)
+if (nolock || NoListenAll)
 return;
 
 if (!StillLocking) {
-- 
1.9.0

___
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

[PATCH 1/6] .gitignore: Add new autotools file 'test-driver'

[Kristian Høgsberg]

Automake 1.12 introduces a new parallel test framework that uses a shell
script helper and generates *.log and *.trs files.  Add to .gitignore.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
Reviewed-by: Gaetan Nadon mems...@videotron.ca
---
 .gitignore  | 1 +
 test/.gitignore | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/.gitignore b/.gitignore
index 94a12fd..dc56b46 100644
--- a/.gitignore
+++ b/.gitignore
@@ -41,6 +41,7 @@ mkinstalldirs
 py-compile
 stamp-h?
 symlink-tree
+test-driver
 texinfo.tex
 ylwrap
 
diff --git a/test/.gitignore b/test/.gitignore
index acbda7a..f8653e3 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -10,3 +10,5 @@ xfree86
 xkb
 xtest
 signal-logging
+*.log
+*.trs
-- 
1.9.0

___
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: [PATCH] Handle -displayfd and an explicit display number sensibly

[Kristian Høgsberg]

On Wed, Mar 19, 2014 at 7:26 AM, Jon TURNEY jon.tur...@dronecode.org.uk wrote:
 Handle -displayfd and an explicit display number sensibly, e.g. use the
 explicitly specified display number, and write it to the displayfd

I  don't think the two options were meant to be used together, but I
can see how that would be useful.  Non-signal based ready notification
while letting the parent process pick the display is useful for
Xwayland as well.

 Signed-off-by: Jon TURNEY jon.tur...@dronecode.org.uk
 ---
  dix/globals.c|  3 ++-
  include/opaque.h |  1 +
  os/connection.c  | 11 +--
  os/utils.c   |  2 +-
  4 files changed, 9 insertions(+), 8 deletions(-)

 diff --git a/dix/globals.c b/dix/globals.c
 index 9738e9c..5a21f07 100644
 --- a/dix/globals.c
 +++ b/dix/globals.c
 @@ -127,7 +127,8 @@ int defaultColorVisualClass = -1;
  int monitorResolution = 0;

  const char *display;
 -int displayfd;
 +int displayfd = 0;

The parent process creates the fd in our process environment and could
pick 0 as the display fd.  Let's initialize to -1 which is never a
valid fd.

 +Bool explicit_display = FALSE;
  char *ConnectionInfo;

  CARD32 TimeOutValue = DEFAULT_TIMEOUT * MILLI_PER_SECOND;
 diff --git a/include/opaque.h b/include/opaque.h
 index 73d40c3..0d917b0 100644
 --- a/include/opaque.h
 +++ b/include/opaque.h
 @@ -51,6 +51,7 @@ extern _X_EXPORT int defaultScreenSaverBlanking;
  extern _X_EXPORT int defaultScreenSaverAllowExposures;
  extern _X_EXPORT const char *display;
  extern _X_EXPORT int displayfd;
 +extern _X_EXPORT Bool explicit_display;

  extern _X_EXPORT int defaultBackingStore;
  extern _X_EXPORT Bool disableBackingStore;
 diff --git a/os/connection.c b/os/connection.c
 index ddf4f0a..23a4f6a 100644
 --- a/os/connection.c
 +++ b/os/connection.c
 @@ -351,8 +351,8 @@ void
  NotifyParentProcess(void)
  {
  #if !defined(WIN32)
 -if (dynamic_display[0]) {
 -write(displayfd, dynamic_display, strlen(dynamic_display));
 +if (displayfd) {
 +write(displayfd, display, strlen(display));
  write(displayfd, \n, 1);
  close(displayfd);
  }
 @@ -404,15 +404,14 @@ CreateWellKnownSockets(void)
  FD_ZERO(WellKnownConnections);

  /* display is initialized to 0 by main(). It is then set to the display
 - * number if specified on the command line, or to NULL when the 
 -displayfd
 - * option is used. */
 -if (display) {
 + * number if specified on the command line. */
 +if ((displayfd == 0) || explicit_display) {
  if (TryCreateSocket(atoi(display), partial) 
  ListenTransCount = 1)
  if (!PartialNetwork  partial)
  FatalError (Failed to establish all listening sockets);
  }
 -else { /* -displayfd */
 +else { /* -displayfd and no explicit display number */
  Bool found = 0;
  for (i = 0; i  65535 - X_TCP_PORT; i++) {
  if (TryCreateSocket(i, partial)  !partial) {
 diff --git a/os/utils.c b/os/utils.c
 index 497779b..8ea6e7a 100644
 --- a/os/utils.c
 +++ b/os/utils.c
 @@ -666,6 +666,7 @@ ProcessCommandLine(int argc, char *argv[])
  else if (argv[i][0] == ':') {
  /* initialize display */
  display = argv[i];
 +explicit_display = TRUE;
  display++;
  if (!VerifyDisplayName(display)) {
  ErrorF(Bad display name: %s\n, display);
 @@ -736,7 +737,6 @@ ProcessCommandLine(int argc, char *argv[])
  else if (strcmp(argv[i], -displayfd) == 0) {
  if (++i  argc) {
  displayfd = atoi(argv[i]);
 -display = NULL;
  #ifdef LOCK_SERVER
  nolock = TRUE;
  #endif
 --
 1.8.3.4

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

[PATCH 2/2] test: Don't add TEST_LDADD to list test

[Kristian Høgsberg]

The list test case is always enabled, even if Xorg is disabled.
TEST_LDADD pulls in Xorg files which breaks linking when Xorg is disabled.
The list test doesn't need any libraries, so just remove list_LDADD.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
Cc: Peter Hutterer peter.hutte...@who-t.net
---
 test/Makefile.am | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/Makefile.am b/test/Makefile.am
index f8aa659..88fb6aa 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -30,7 +30,6 @@ endif
 xkb_LDADD=$(TEST_LDADD)
 input_LDADD=$(TEST_LDADD)
 xtest_LDADD=$(TEST_LDADD)
-list_LDADD=$(TEST_LDADD)
 misc_LDADD=$(TEST_LDADD)
 fixes_LDADD=$(TEST_LDADD)
 xfree86_LDADD=$(TEST_LDADD)
-- 
1.9.0

___
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

[PATCH 1/2] .gitignore: Add new autotools file 'test-driver'

[Kristian Høgsberg]

Automake 1.12 introduces a new parallel test framework that uses a shell
script helper and generates *.log and *.trs files.  Add to .gitignore.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
Cc: Gaetan Nadon mems...@videotron.ca
---
 .gitignore  | 1 +
 test/.gitignore | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/.gitignore b/.gitignore
index 94a12fd..dc56b46 100644
--- a/.gitignore
+++ b/.gitignore
@@ -41,6 +41,7 @@ mkinstalldirs
 py-compile
 stamp-h?
 symlink-tree
+test-driver
 texinfo.tex
 ylwrap
 
diff --git a/test/.gitignore b/test/.gitignore
index acbda7a..f8653e3 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -10,3 +10,5 @@ xfree86
 xkb
 xtest
 signal-logging
+*.log
+*.trs
-- 
1.9.0

___
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

[PATCH v2 2/3] os: Add a mechanism to prevent creating any listen sockets

[Kristian Høgsberg]

A socket-activated server will receive its listening sockets from the
parent process and should not create its own sockets.  This patch
introduces a NoListen flag that can be set by a DDX to prevent
the server from creating the sockets.  When NoListen is enabled, we
also disable the server lock checking, since the parent process is
responsible for checking the lock before picking the display name and
creating the sockets.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---

The three patches in this series were split out from one big patch and I
lost the nolock detail when I split it.  This v2 of the patch adds it back.

Kristian

 include/opaque.h | 1 +
 os/connection.c  | 9 +++--
 os/utils.c   | 4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/opaque.h b/include/opaque.h
index 73d40c3..7ec1d85 100644
--- a/include/opaque.h
+++ b/include/opaque.h
@@ -74,5 +74,6 @@ extern _X_EXPORT Bool whiteRoot;
 extern _X_EXPORT Bool bgNoneRoot;
 
 extern _X_EXPORT Bool CoreDump;
+extern _X_EXPORT Bool NoListenAll;
 
 #endif  /* OPAQUE_H */
diff --git a/os/connection.c b/os/connection.c
index 400c542..b50f9e9 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -138,6 +138,7 @@ fd_set OutputPending;   /* clients with reply/event 
data ready to go */
 int MaxClients = 0;
 Bool NewOutputPending;  /* not yet attempted to write some new output 
*/
 Bool AnyClientsWriteBlocked;/* true if some client blocked on write */
+Bool NoListenAll;   /* Don't establish any listening sockets */
 
 static Bool RunFromSmartParent; /* send SIGUSR1 to parent process */
 Bool RunFromSigStopParent;  /* send SIGSTOP to our own process; Upstart (or
@@ -406,7 +407,10 @@ CreateWellKnownSockets(void)
 /* display is initialized to 0 by main(). It is then set to the display
  * number if specified on the command line, or to NULL when the -displayfd
  * option is used. */
-if (display) {
+if (NoListenAll) {
+ListenTransCount = 0;
+}
+else if (display) {
 if (TryCreateSocket(atoi(display), partial) 
 ListenTransCount = 1)
 if (!PartialNetwork  partial)
@@ -440,9 +444,10 @@ CreateWellKnownSockets(void)
 DefineSelf (fd);
 }
 
-if (!XFD_ANYSET(WellKnownConnections))
+if (!XFD_ANYSET(WellKnownConnections)  !NoListenAll)
 FatalError
 (Cannot establish any listening sockets - Make sure an X server 
isn't already running);
+
 #if !defined(WIN32)
 OsSignal(SIGPIPE, SIG_IGN);
 OsSignal(SIGHUP, AutoResetServer);
diff --git a/os/utils.c b/os/utils.c
index 497779b..c513968 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -270,7 +270,7 @@ LockServer(void)
 int len;
 char port[20];
 
-if (nolock)
+if (nolock || NoListenAll)
 return;
 /*
  * Path names
@@ -390,7 +390,7 @@ LockServer(void)
 void
 UnlockServer(void)
 {
-if (nolock)
+if (nolock || NoListenAll)
 return;
 
 if (!StillLocking) {
-- 
1.9.0

___
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: [PATCH 3/3] os: Add AddClientOnOpenFD() to create a new client for an file descriptor

[Kristian Høgsberg]

On Wed, Mar 19, 2014 at 4:29 PM, Peter Hutterer
peter.hutte...@who-t.net wrote:
 On Tue, Mar 18, 2014 at 10:06:00PM -0700, Kristian Høgsberg wrote:
 When the Xwayland server is socket-activated, we need to connect and
 initialize the window manager before the activating client gets to
 proceed with connecting.  We do this by passing a socket file
 descriptor for the window manager connection to the Xwayland server,
 which then uses this new function to set it up as an X client.

 Signed-off-by: Kristian Høgsberg k...@bitplanet.net
 ---
  include/os.h|  2 ++
  os/connection.c | 20 
  2 files changed, 22 insertions(+)

 diff --git a/include/os.h b/include/os.h
 index 90229e6..5be3bcc 100644
 --- a/include/os.h
 +++ b/include/os.h
 @@ -168,6 +168,8 @@ extern _X_EXPORT void MakeClientGrabPervious(ClientPtr 
 /*client */ );

  extern _X_EXPORT void ListenOnOpenFD(int /* fd */ , int /* noxauth */ );

 +extern _X_EXPORT void AddClientOnOpenFD(int /* fd */ );
 +
  extern _X_EXPORT CARD32 GetTimeInMillis(void);
  extern _X_EXPORT CARD64 GetTimeInMicros(void);

 diff --git a/os/connection.c b/os/connection.c
 index b50f9e9..f1e7a25 100644
 --- a/os/connection.c
 +++ b/os/connection.c
 @@ -1312,3 +1312,23 @@ ListenOnOpenFD(int fd, int noxauth)
  XdmcpReset();
  #endif
  }
 +
 +/* based on TRANS(SocketUNIXAccept) (XtransConnInfo ciptr, int *status) */
 +void
 +AddClientOnOpenFD(int fd)
 +{
 +XtransConnInfo ciptr;
 +CARD32 connect_time;
 +
 +ciptr = _XSERVTransReopenCOTSServer(5, fd, @anonymous);
 +
 +_XSERVTransSetOption(ciptr, TRANS_NONBLOCKING, 1);
 +ciptr-flags |= TRANS_NOXAUTH;
 +
 +connect_time = GetTimeInMillis();
 +
 +if (!AllocNewConnection(ciptr, fd, connect_time)) {
 +ErrorF(stderr, failed to create client for wayland server\n);

 ErrorF doesn't take a stream, so you can drop the first argument.

It certainly doesn't... I used fprintf for some reason in the patch I
split into these three patches, that's where stderr came from.

 Plus, a
 more generic error message may be good, it's a bit confusing to see wayland
 here when the actual function seems generic enough.

Yup, true.  I'll send out a new patch that returns TRUE/FALSE for
success/failure and log a FatalError in Xwayland if it fails.

Thanks for reviewing,
Kristian

 Cheers,
Peter

 +return;
 +}
 +}
 --
 1.9.0

 ___
 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

___
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

[PATCH v2 3/3] os: Add AddClientOnOpenFD() to create a new client for an file descriptor

[Kristian Høgsberg]

When the Xwayland server is socket-activated, we need to connect and
initialize the window manager before the activating client gets to
proceed with connecting.  We do this by passing a socket file
descriptor for the window manager connection to the Xwayland server,
which then uses this new function to set it up as an X client.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---

Here's v2 of this patch, which fixes the ErrorF(stderr, ...) Peter
pointed out.  AddClientOnOpenFD() now returns TRUE on success and
FALSE on error.  It's up to the caller to log an error message and
either recover or shut down.  Additionally, we now use :%d for the
port argument to Xtrans as everywhere else and clean up the XtransConnInfo
properly if we fail to add the new connection.

Kristian

 include/os.h|  2 ++
 os/connection.c | 27 +++
 2 files changed, 29 insertions(+)

diff --git a/include/os.h b/include/os.h
index 90229e6..d26e399 100644
--- a/include/os.h
+++ b/include/os.h
@@ -168,6 +168,8 @@ extern _X_EXPORT void MakeClientGrabPervious(ClientPtr 
/*client */ );
 
 extern _X_EXPORT void ListenOnOpenFD(int /* fd */ , int /* noxauth */ );
 
+extern _X_EXPORT Bool AddClientOnOpenFD(int /* fd */ );
+
 extern _X_EXPORT CARD32 GetTimeInMillis(void);
 extern _X_EXPORT CARD64 GetTimeInMicros(void);
 
diff --git a/os/connection.c b/os/connection.c
index b50f9e9..b3640b8 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -1312,3 +1312,30 @@ ListenOnOpenFD(int fd, int noxauth)
 XdmcpReset();
 #endif
 }
+
+/* based on TRANS(SocketUNIXAccept) (XtransConnInfo ciptr, int *status) */
+Bool
+AddClientOnOpenFD(int fd)
+{
+XtransConnInfo ciptr;
+CARD32 connect_time;
+char port[20];
+
+snprintf(port, sizeof(port), :%d, atoi(display));
+ciptr = _XSERVTransReopenCOTSServer(5, fd, port);
+if (ciptr == NULL)
+return FALSE;
+
+_XSERVTransSetOption(ciptr, TRANS_NONBLOCKING, 1);
+ciptr-flags |= TRANS_NOXAUTH;
+
+connect_time = GetTimeInMillis();
+
+if (!AllocNewConnection(ciptr, fd, connect_time)) {
+ErrorConnMax(ciptr);
+_XSERVTransClose(ciptr);
+return FALSE;
+}
+
+return TRUE;
+}
-- 
1.9.0

___
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: [PULL] xkb keymap loading

[Kristian Høgsberg]

On Tue, Mar 18, 2014 at 2:54 PM, Peter Hutterer
peter.hutte...@who-t.net wrote:
 The following changes since commit 795066477ee81b5b82e490eac8bed6b656d01f17:

   config: search for PnPID on all parents (#75513) (2014-03-12 07:43:16 +1000)

 are available in the git repository at:

   git://people.freedesktop.org/~whot/xserver for-keith

 for you to fetch changes up to 505eec9f38e8b167ce7ad51f72cca3ef33d0ceb3:

   xkb: add XkbLoadKeymapFromString (2014-03-17 15:10:51 +1000)

I just rebased Xwayland on top of the rewritten patches and now
something is broken.  Xwayland doesn't get the keymap from the Wayland
server and falls back to us.  Let's not pull this for now.

Kristian

 
 Kristian Høgsberg (3):
   xkb: factor out xkb loading to LoadXkm
   xkb: add KeymapOrDefault
   xkb: add XkbLoadKeymapFromString

 Peter Hutterer (2):
   xkb: constify XkbDDXOpenConfigFile
   xkb: add a callback to xkbcomp

 Rui Matos (2):
   xkb: Factor out a function to copy a keymap's controls onto another
   xkb: Repurpose XkbCopyDeviceKeymap to apply a given keymap to a device

  Xi/exevents.c|   2 +-
  include/input.h  |   6 ++
  include/xkbsrv.h |  11 ++-
  xkb/ddxLoad.c| 241 
 ++-
  xkb/xkb.c|  16 +---
  xkb/xkbInit.c|  42 --
  xkb/xkbUtils.c   |  37 +++--
  7 files changed, 269 insertions(+), 86 deletions(-)

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

[PATCH 1/3] os: Always compile ListenOnOpenFD() and export it

[Kristian Høgsberg]

This function was written to allow the X server to inherit the listen
socket from launchd on OS X.  The code is not specific to OS X though
and will be useful for on-demand launched Xwayland servers.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
 include/os.h| 4 +---
 os/connection.c | 7 ++-
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/include/os.h b/include/os.h
index e5f86d6..90229e6 100644
--- a/include/os.h
+++ b/include/os.h
@@ -166,9 +166,7 @@ extern _X_EXPORT void MakeClientGrabImpervious(ClientPtr 
/*client */ );
 
 extern _X_EXPORT void MakeClientGrabPervious(ClientPtr /*client */ );
 
-#ifdef XQUARTZ
-extern void ListenOnOpenFD(int /* fd */ , int /* noxauth */ );
-#endif
+extern _X_EXPORT void ListenOnOpenFD(int /* fd */ , int /* noxauth */ );
 
 extern _X_EXPORT CARD32 GetTimeInMillis(void);
 extern _X_EXPORT CARD64 GetTimeInMicros(void);
diff --git a/os/connection.c b/os/connection.c
index ddf4f0a..400c542 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -1253,8 +1253,7 @@ MakeClientGrabPervious(ClientPtr client)
 }
 }
 
-#ifdef XQUARTZ
-/* Add a fd (from launchd) to our listeners */
+/* Add a fd (from launchd or similar) to our listeners */
 void
 ListenOnOpenFD(int fd, int noxauth)
 {
@@ -1276,7 +1275,7 @@ ListenOnOpenFD(int fd, int noxauth)
  */
 ciptr = _XSERVTransReopenCOTSServer(5, fd, port);
 if (ciptr == NULL) {
-ErrorF(Got NULL while trying to Reopen launchd port.\n);
+ErrorF(Got NULL while trying to Reopen listen port.\n);
 return;
 }
 
@@ -1308,5 +1307,3 @@ ListenOnOpenFD(int fd, int noxauth)
 XdmcpReset();
 #endif
 }
-
-#endif
-- 
1.9.0

___
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

[PATCH] .gitignore: Add new autotools file 'test-driver'

[Kristian Høgsberg]

Recent automake introduces a new shell script helper for the automake
test framework.  Add it to .gitignore.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---

Gaetan, not sure what the convention is for .gitignore.  You added the
Do not edit comment, but I don't see any other mechanism for updating
the autotools list.

Kristian

 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 94a12fd..dc56b46 100644
--- a/.gitignore
+++ b/.gitignore
@@ -41,6 +41,7 @@ mkinstalldirs
 py-compile
 stamp-h?
 symlink-tree
+test-driver
 texinfo.tex
 ylwrap
 
-- 
1.9.0

___
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: [PATCH v2 0/5] xwayland: Refactor bits of XKB functionality

[Kristian Høgsberg]

On Wed, Mar 12, 2014 at 7:56 PM, Peter Hutterer
peter.hutte...@who-t.net wrote:
 Refactored series. I really disliked the XkbCompContext patches, something
 just didn't sit right with it. I wish we could use fmemopen() but short of
 that or a bigger refactoring this should do. Replaces the xkbcomp-specific
 context with a callback. After all, what we need is a stream to write to
 xkbcomp, the rest of the context was superfluous anyway. This patchset does
 that - sets everything up, then calls the callback.

 The rest is mostly identical, except that I split two features out (and
 replaced all the tabs with spaces...)

That's fine, I think I was trying to avoid a callback, but I don't
think there was a good reasons for that.  This new approach looks good
too, for the new patches,

Reviewed-by: Kristian Høgsberg k...@bitplanet.net

 Cheers,
   Peter



___
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

xwayland: Refactor bits of XKB functionality

[Kristian Høgsberg]

Hi list,

Here's few xkb related patches that refactor and expose xkb functionality.
There should be no change in behavior in these patches, the idea is to
expose new entry points into existing code.

This is in preparation for merging xwayland, where the X server has to
parse and apply xkb maps coming from the wayland server.  In this case we
need the ability to parse the keymap from a string and apply a keymap to
a device.

Kristian

___
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

[PATCH 5/5] xkb: Factor out a function to copy a keymap's controls unto another

[Kristian Høgsberg]

From: Rui Matos tiagoma...@gmail.com

Reviewed-by: Kristian Høgsberg k...@bitplanet.net
---
 include/xkbsrv.h |  3 +++
 xkb/xkb.c| 14 +-
 xkb/xkbUtils.c   | 23 +++
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/include/xkbsrv.h b/include/xkbsrv.h
index 5d8e409..49e236d 100644
--- a/include/xkbsrv.h
+++ b/include/xkbsrv.h
@@ -837,6 +837,9 @@ extern void XkbFakeDeviceButton(DeviceIntPtr /* dev */ ,
 int /* press */ ,
 int /* button */ );
 
+extern _X_EXPORT void XkbCopyControls(XkbDescPtr /* dst */ ,
+  XkbDescPtr /* src */ );
+
 #include xkbfile.h
 #include xkbrules.h
 
diff --git a/xkb/xkb.c b/xkb/xkb.c
index 6196a17..dc570f0 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -5950,25 +5950,13 @@ ProcXkbGetKbdByName(ClientPtr client)
 if (rep.loaded) {
 XkbDescPtr old_xkb;
 xkbNewKeyboardNotify nkn;
-int i, nG, nTG;
 
 old_xkb = xkb;
 xkb = new;
 dev-key-xkbInfo-desc = xkb;
 new = old_xkb;  /* so it'll get freed automatically */
 
-*xkb-ctrls = *old_xkb-ctrls;
-for (nG = nTG = 0, i = xkb-min_key_code; i = xkb-max_key_code; i++) 
{
-nG = XkbKeyNumGroups(xkb, i);
-if (nG = XkbNumKbdGroups) {
-nTG = XkbNumKbdGroups;
-break;
-}
-if (nG  nTG) {
-nTG = nG;
-}
-}
-xkb-ctrls-num_groups = nTG;
+XkbCopyControls(xkb, old_xkb);
 
 nkn.deviceID = nkn.oldDeviceID = dev-id;
 nkn.minKeyCode = new-min_key_code;
diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
index 1f8a839..6cf6e79 100644
--- a/xkb/xkbUtils.c
+++ b/xkb/xkbUtils.c
@@ -2090,3 +2090,26 @@ XkbMergeLockedPtrBtns(DeviceIntPtr master)
 xkbi-lockedPtrButtons |= d-key-xkbInfo-lockedPtrButtons;
 }
 }
+
+void
+XkbCopyControls(XkbDescPtr dst, XkbDescPtr src)
+{
+int i, nG, nTG;
+
+if (!dst || !src)
+return;
+
+*dst-ctrls = *src-ctrls;
+
+for (nG = nTG = 0, i = dst-min_key_code; i = dst-max_key_code; i++) {
+nG = XkbKeyNumGroups(dst, i);
+if (nG = XkbNumKbdGroups) {
+nTG = XkbNumKbdGroups;
+break;
+}
+if (nG  nTG) {
+nTG = nG;
+}
+}
+dst-ctrls-num_groups = nTG;
+}
-- 
1.9.0

___
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

[PATCH 3/5] xkb: Add XkbCompileKeymapFromString()

[Kristian Høgsberg]

This new function compiles a keymap from an in-memory string.  We use it
to add a new keyooard device init function,
InitKeyboardDeviceStructFromString(), which inits a keyboard device with
a keymap specified as a string instead of a rmlvo set.

Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 include/input.h  |   6 +++
 include/xkbsrv.h |   4 ++
 xkb/ddxLoad.c| 129 ++-
 xkb/xkbInit.c|  44 +++
 4 files changed, 144 insertions(+), 39 deletions(-)

diff --git a/include/input.h b/include/input.h
index 93c4510..36463f2 100644
--- a/include/input.h
+++ b/include/input.h
@@ -385,6 +385,12 @@ extern _X_EXPORT Bool 
InitKeyboardDeviceStruct(DeviceIntPtr /*device */ ,
KbdCtrlProcPtr /*controlProc */
);
 
+extern _X_EXPORT Bool InitKeyboardDeviceStructFromString(DeviceIntPtr dev,
+const char *keymap,
+int keymap_length,
+BellProcPtr bell_func,
+KbdCtrlProcPtr 
ctrl_func);
+
 extern int ApplyPointerMapping(DeviceIntPtr /* pDev */ ,
CARD8 * /* map */ ,
int /* len */ ,
diff --git a/include/xkbsrv.h b/include/xkbsrv.h
index e799799..253612e 100644
--- a/include/xkbsrv.h
+++ b/include/xkbsrv.h
@@ -869,4 +869,8 @@ extern _X_EXPORT XkbDescPtr XkbCompileKeymap(DeviceIntPtr 
/* dev */ ,
  XkbRMLVOSet *  /* rmlvo */
 );
 
+extern _X_EXPORT XkbDescPtr XkbCompileKeymapFromString(DeviceIntPtr dev,
+  const char *keymap,
+  int keymap_length);
+
 #endif  /* _XKBSRV_H_ */
diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
index d266b36..d4281a0 100644
--- a/xkb/ddxLoad.c
+++ b/xkb/ddxLoad.c
@@ -262,6 +262,35 @@ XkbDDXOpenConfigFile(char *mapName, char *fileNameRtrn, 
int fileNameRtrnLen)
 return file;
 }
 
+static unsigned
+LoadXKM(unsigned want, unsigned need, XkbCompContextPtr ctx, XkbDescPtr 
*xkbRtrn)
+{
+FILE *file;
+char fileName[PATH_MAX];
+unsigned missing;
+
+file = XkbDDXOpenConfigFile(ctx-keymap, fileName, PATH_MAX);
+if (file == NULL) {
+LogMessage(X_ERROR, Couldn't open compiled keymap file %s\n,
+   fileName);
+return 0;
+}
+missing = XkmReadFile(file, need, want, xkbRtrn);
+if (*xkbRtrn == NULL) {
+LogMessage(X_ERROR, Error loading keymap %s\n, fileName);
+fclose(file);
+(void) unlink(fileName);
+return 0;
+}
+else {
+DebugF(Loaded XKB keymap %s, defined=0x%x\n, fileName,
+   (*xkbRtrn)-defined);
+}
+fclose(file);
+(void) unlink(fileName);
+return (need | want)  (~missing);
+}
+
 unsigned
 XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
 XkbComponentNamesPtr names,
@@ -270,9 +299,6 @@ XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
 XkbDescPtr *xkbRtrn, char *nameRtrn, int nameRtrnLen)
 {
 XkbDescPtr xkb;
-FILE *file;
-char fileName[PATH_MAX];
-unsigned missing;
 XkbCompContextRec ctx;
 
 *xkbRtrn = NULL;
@@ -292,26 +318,30 @@ XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
 LogMessage(X_ERROR, XKB: Couldn't compile keymap\n);
 return 0;
 }
-file = XkbDDXOpenConfigFile(ctx.keymap, fileName, PATH_MAX);
-if (file == NULL) {
-LogMessage(X_ERROR, Couldn't open compiled keymap file %s\n,
-   fileName);
-return 0;
-}
-missing = XkmReadFile(file, need, want, xkbRtrn);
-if (*xkbRtrn == NULL) {
-LogMessage(X_ERROR, Error loading keymap %s\n, fileName);
-fclose(file);
-(void) unlink(fileName);
+
+return LoadXKM(want, need, ctx, xkbRtrn);
+}
+
+static unsigned
+XkbDDXLoadKeymapFromString(DeviceIntPtr keybd,
+  const char *keymap, int keymap_length,
+  unsigned want,
+  unsigned need,
+  XkbDescPtr *xkbRtrn)
+{
+XkbCompContextRec ctx;
+
+*xkbRtrn = NULL;
+
+if (StartXkbComp(ctx))
+   fwrite(keymap, keymap_length, 1, ctx.out);
+
+if (!FinishXkbComp(ctx)) {
+LogMessage(X_ERROR, XKB: Couldn't compile keymap\n);
 return 0;
 }
-else {
-DebugF(Loaded XKB keymap %s, defined=0x%x\n, fileName,
-   (*xkbRtrn)-defined);
-}
-fclose(file);
-(void) unlink(fileName);
-return (need | want)  (~missing);
+
+return LoadXKM(want, need, ctx, xkbRtrn);
 }
 
 Bool
@@ -407,6 +437,29 @@ XkbCompileKeymapForDevice(DeviceIntPtr dev, XkbRMLVOSet * 
rmlvo, 

[PATCH 1/5] xkb: Add struct XkbCompContext

[Kristian Høgsberg]

This commit adds a struct that contains most of the context for starting,
running and cleaning up after xkbcomp.

Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 xkb/ddxLoad.c | 76 +--
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
index f458e3b..cf9faa4 100644
--- a/xkb/ddxLoad.c
+++ b/xkb/ddxLoad.c
@@ -90,14 +90,21 @@ OutputDirectory(char *outdir, size_t size)
 }
 }
 
+typedef struct XkbCompContext {
+char keymap[PATH_MAX];
+FILE *out;
+char *buf;
+char tmpname[PATH_MAX];
+const char *xkmfile;
+} XkbCompContextRec, *XkbCompContextPtr;
+
 static Bool
 XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
XkbComponentNamesPtr names,
unsigned want,
-   unsigned need, char *nameRtrn, int nameRtrnLen)
+   unsigned need, XkbCompContextPtr ctx)
 {
-FILE *out;
-char *buf = NULL, keymap[PATH_MAX], xkm_output_dir[PATH_MAX];
+char xkm_output_dir[PATH_MAX];
 
 const char *emptystring = ;
 char *xkbbasedirflag = NULL;
@@ -105,22 +112,19 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 const char *xkbbindirsep = emptystring;
 
 #ifdef WIN32
-/* WIN32 has no popen. The input must be stored in a file which is
-   used as input for xkbcomp. xkbcomp does not read from stdin. */
-char tmpname[PATH_MAX];
-const char *xkmfile = tmpname;
+ctx-xkmfile = ctx-tmpname;
 #else
-const char *xkmfile = -;
+ctx-xkmfile = -;
 #endif
 
-snprintf(keymap, sizeof(keymap), server-%s, display);
+snprintf(ctx-keymap, sizeof(ctx-keymap), server-%s, display);
 
 OutputDirectory(xkm_output_dir, sizeof(xkm_output_dir));
 
 #ifdef WIN32
-strcpy(tmpname, Win32TempDir());
-strcat(tmpname, \\xkb_XX);
-(void) mktemp(tmpname);
+strcpy(ctx-tmpname, Win32TempDir());
+strcat(ctx-tmpname, \\xkb_XX);
+(void) mktemp(ctx-tmpname);
 #endif
 
 if (XkbBaseDirectory != NULL) {
@@ -139,73 +143,69 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 }
 }
 
-if (asprintf(buf,
+if (asprintf(ctx-buf,
  \%s%sxkbcomp\ -w %d %s -xkm \%s\ 
  -em1 %s -emp %s -eml %s \%s%s.xkm\,
  xkbbindir, xkbbindirsep,
  ((xkbDebugFlags  2) ? 1 :
   ((xkbDebugFlags  10) ? 10 : (int) xkbDebugFlags)),
- xkbbasedirflag ? xkbbasedirflag : , xkmfile,
+ xkbbasedirflag ? xkbbasedirflag : , ctx-xkmfile,
  PRE_ERROR_MSG, ERROR_PREFIX, POST_ERROR_MSG1,
- xkm_output_dir, keymap) == -1)
-buf = NULL;
+ xkm_output_dir, ctx-keymap) == -1)
+ctx-buf = NULL;
 
 free(xkbbasedirflag);
 
-if (!buf) {
+if (!ctx-buf) {
 LogMessage(X_ERROR,
XKB: Could not invoke xkbcomp: not enough memory\n);
 return FALSE;
 }
 
 #ifndef WIN32
-out = Popen(buf, w);
+ctx-out = Popen(ctx-buf, w);
 #else
-out = fopen(tmpname, w);
+ctx-out = fopen(ctx-tmpname, w);
 #endif
 
-if (out != NULL) {
+if (ctx-out != NULL) {
 #ifdef DEBUG
 if (xkbDebugFlags) {
 ErrorF([xkb] XkbDDXCompileKeymapByNames compiling keymap:\n);
 XkbWriteXKBKeymapForNames(stderr, names, xkb, want, need);
 }
 #endif
-XkbWriteXKBKeymapForNames(out, names, xkb, want, need);
+XkbWriteXKBKeymapForNames(ctx-out, names, xkb, want, need);
 #ifndef WIN32
-if (Pclose(out) == 0)
+if (Pclose(ctx-out) == 0)
 #else
-if (fclose(out) == 0  System(buf) = 0)
+if (fclose(ctx-out) == 0  System(ctx-buf) = 0)
 #endif
 {
 if (xkbDebugFlags)
-DebugF([xkb] xkb executes: %s\n, buf);
-if (nameRtrn) {
-strlcpy(nameRtrn, keymap, nameRtrnLen);
-}
-free(buf);
+DebugF([xkb] xkb executes: %s\n, ctx-buf);
+free(ctx-buf);
 #ifdef WIN32
-unlink(tmpname);
+unlink(ctx-tmpname);
 #endif
 return TRUE;
 }
 else
-LogMessage(X_ERROR, Error compiling keymap (%s)\n, keymap);
+LogMessage(X_ERROR, Error compiling keymap (%s)\n, ctx-keymap);
 #ifdef WIN32
 /* remove the temporary file */
-unlink(tmpname);
+unlink(ctx-tmpname);
 #endif
 }
 else {
 #ifndef WIN32
 LogMessage(X_ERROR, XKB: Could not invoke xkbcomp\n);
 #else
-LogMessage(X_ERROR, Could not open file %s\n, tmpname);
+LogMessage(X_ERROR, Could not open file %s\n, ctx-tmpname);
 #endif
 }
-if (nameRtrn)
-nameRtrn[0] = '\0';
-free(buf);
+ctx-keymap[0] = '\0';
+free(ctx-buf);
 return FALSE;
 }
 
@@ -256,6 +256,7 @@ XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
 FILE *file;
 char fileName[PATH_MAX];
 

[PATCH 2/5] xkb: Split out code to start and finish xkbcomp

[Kristian Høgsberg]

Using the context struct from previous commit, we can now split out
code to start xkbcomp and to finish and clean up after it.

Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 xkb/ddxLoad.c | 39 ---
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
index cf9faa4..d266b36 100644
--- a/xkb/ddxLoad.c
+++ b/xkb/ddxLoad.c
@@ -99,10 +99,7 @@ typedef struct XkbCompContext {
 } XkbCompContextRec, *XkbCompContextPtr;
 
 static Bool
-XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
-   XkbComponentNamesPtr names,
-   unsigned want,
-   unsigned need, XkbCompContextPtr ctx)
+StartXkbComp(XkbCompContextPtr ctx)
 {
 char xkm_output_dir[PATH_MAX];
 
@@ -168,14 +165,15 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 ctx-out = fopen(ctx-tmpname, w);
 #endif
 
+return ctx-out != NULL;
+}
+
+static Bool
+FinishXkbComp(XkbCompContextPtr ctx)
+{
+if (!ctx-buf)
+   return FALSE;
 if (ctx-out != NULL) {
-#ifdef DEBUG
-if (xkbDebugFlags) {
-ErrorF([xkb] XkbDDXCompileKeymapByNames compiling keymap:\n);
-XkbWriteXKBKeymapForNames(stderr, names, xkb, want, need);
-}
-#endif
-XkbWriteXKBKeymapForNames(ctx-out, names, xkb, want, need);
 #ifndef WIN32
 if (Pclose(ctx-out) == 0)
 #else
@@ -209,6 +207,25 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 return FALSE;
 }
 
+static Bool
+XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
+   XkbComponentNamesPtr names,
+   unsigned want,
+   unsigned need, XkbCompContextPtr ctx)
+{
+if (StartXkbComp(ctx)) {
+#ifdef DEBUG
+if (xkbDebugFlags) {
+ErrorF([xkb] XkbDDXCompileKeymapByNames compiling keymap:\n);
+XkbWriteXKBKeymapForNames(stderr, names, xkb, want, need);
+}
+#endif
+XkbWriteXKBKeymapForNames(ctx-out, names, xkb, want, need);
+}
+
+return FinishXkbComp(ctx);
+}
+
 static FILE *
 XkbDDXOpenConfigFile(char *mapName, char *fileNameRtrn, int fileNameRtrnLen)
 {
-- 
1.9.0

___
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

[PATCH 4/5] xkb: Repurpose XkbCopyDeviceKeymap to apply a given keymap to a device

[Kristian Høgsberg]

From: Rui Matos tiagoma...@gmail.com

This will also make it useful for cases when we have a new keymap to
apply to a device but don't have a source device.

Reviewed-by: Kristian Høgsberg k...@bitplanet.net
---
 Xi/exevents.c|  2 +-
 include/xkbsrv.h |  4 ++--
 xkb/xkb.c|  2 +-
 xkb/xkbUtils.c   | 14 +++---
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Xi/exevents.c b/Xi/exevents.c
index e9f670e..9c207eb 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -230,7 +230,7 @@ CopyKeyClass(DeviceIntPtr device, DeviceIntPtr master)
 
 mk-sourceid = device-id;
 
-if (!XkbCopyDeviceKeymap(master, device))
+if (!XkbDeviceApplyKeymap(master, device-key-xkbInfo-desc))
 FatalError(Couldn't pivot keymap from device to core!\n);
 }
 
diff --git a/include/xkbsrv.h b/include/xkbsrv.h
index 253612e..5d8e409 100644
--- a/include/xkbsrv.h
+++ b/include/xkbsrv.h
@@ -820,8 +820,8 @@ extern _X_EXPORT void XkbSendNewKeyboardNotify(DeviceIntPtr 
/* kbd */ ,
 extern Bool XkbCopyKeymap(XkbDescPtr /* dst */ ,
   XkbDescPtr /* src */ );
 
-extern _X_EXPORT Bool XkbCopyDeviceKeymap(DeviceIntPtr /* dst */ ,
-  DeviceIntPtr /* src */ );
+extern _X_EXPORT Bool XkbDeviceApplyKeymap(DeviceIntPtr /* dst */ ,
+   XkbDescPtr /* src */ );
 
 extern void XkbFilterEvents(ClientPtr /* pClient */ ,
 int /* nEvents */ ,
diff --git a/xkb/xkb.c b/xkb/xkb.c
index 31bb8d3..6196a17 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -5991,7 +5991,7 @@ ProcXkbGetKbdByName(ClientPtr client)
 continue;
 
 if (tmpd != dev)
-XkbCopyDeviceKeymap(tmpd, dev);
+XkbDeviceApplyKeymap(tmpd, xkb);
 
 if (tmpd-kbdfeed  tmpd-kbdfeed-xkb_sli) {
 old_sli = tmpd-kbdfeed-xkb_sli;
diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
index 6c6af60..1f8a839 100644
--- a/xkb/xkbUtils.c
+++ b/xkb/xkbUtils.c
@@ -1999,28 +1999,28 @@ XkbCopyKeymap(XkbDescPtr dst, XkbDescPtr src)
 }
 
 Bool
-XkbCopyDeviceKeymap(DeviceIntPtr dst, DeviceIntPtr src)
+XkbDeviceApplyKeymap(DeviceIntPtr dst, XkbDescPtr desc)
 {
 xkbNewKeyboardNotify nkn;
 Bool ret;
 
-if (!dst-key || !src-key)
+if (!dst-key || !desc)
 return FALSE;
 
 memset(nkn, 0, sizeof(xkbNewKeyboardNotify));
 nkn.oldMinKeyCode = dst-key-xkbInfo-desc-min_key_code;
 nkn.oldMaxKeyCode = dst-key-xkbInfo-desc-max_key_code;
 nkn.deviceID = dst-id;
-nkn.oldDeviceID = dst-id;  /* maybe src-id? */
-nkn.minKeyCode = src-key-xkbInfo-desc-min_key_code;
-nkn.maxKeyCode = src-key-xkbInfo-desc-max_key_code;
+nkn.oldDeviceID = dst-id;
+nkn.minKeyCode = desc-min_key_code;
+nkn.maxKeyCode = desc-max_key_code;
 nkn.requestMajor = XkbReqCode;
 nkn.requestMinor = X_kbSetMap;  /* Near enough's good enough. */
 nkn.changed = XkbNKN_KeycodesMask;
-if (src-key-xkbInfo-desc-geom)
+if (desc-geom)
 nkn.changed |= XkbNKN_GeometryMask;
 
-ret = XkbCopyKeymap(dst-key-xkbInfo-desc, src-key-xkbInfo-desc);
+ret = XkbCopyKeymap(dst-key-xkbInfo-desc, desc);
 if (ret)
 XkbSendNewKeyboardNotify(dst, nkn);
 
-- 
1.9.0

___
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: [PATCH 0/3] libxtrans + xserver: Add support for systemd socket activation (v5)

[Kristian Høgsberg]

On Tue, Dec 3, 2013 at 1:48 AM, Hans de Goede hdego...@redhat.com wrote:
 Hi,


 On 12/02/2013 09:54 PM, Kristian Høgsberg wrote:

 On Mon, Dec 2, 2013 at 11:44 AM, Hans de Goede hdego...@redhat.com
 wrote:

 Hi All,


 Hi,

 I didn't follow all this in detail, but I'm not sure that we need
 Xtrans patches at all.  The server already has ListenOnOpenFD() in
 os/connection.c.  It was added for launchd under OSX, but we use it
 for xwayland socket activation too.


 Looking at how this is used, it is used for fds which are received through
 messages send over another fd while the Xserver is running.

 Where as the systemd sockets are passed in as already open fds on startup
 (just like stdout  friends are already open on startup).

I know, but I'm not sure why that matters?  The server receives an fd
that's one end of a socket, whether it comes from the environment or
through another socket doesn't seem relevant.

 Note that libxtrans has code for OSX for getting sockets passed at startup
 to, see this bit in XTrans.c:

But why? What's the point?  I'm trying to help you by making your
patch set smaller here...

Kristian

 #ifdef XQUARTZ_EXPORTS_LAUNCHD_FD
 fprintf(stderr, Launchd socket fd: %d\n, xquartz_launchd_fd);
 if(xquartz_launchd_fd != -1) {
 if((ciptr = TRANS(ReopenCOTSServer(TRANS_SOCKET_LOCAL_INDEX,
xquartz_launchd_fd,
 getenv(DISPLAY
 fprintf(stderr,Got NULL while trying to Reopen launchd
 port\n);
 else
 temp_ciptrs[(*count_ret)++] = ciptr;
 }
 #endif

 Regards,

 Hans
___
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: [PATCH 0/3] libxtrans + xserver: Add support for systemd socket activation (v5)

[Kristian Høgsberg]

On Mon, Dec 2, 2013 at 11:44 AM, Hans de Goede hdego...@redhat.com wrote:
 Hi All,

Hi,

I didn't follow all this in detail, but I'm not sure that we need
Xtrans patches at all.  The server already has ListenOnOpenFD() in
os/connection.c.  It was added for launchd under OSX, but we use it
for xwayland socket activation too.

Kristian

 Here is v5 (I'm starting the numbering where Łukasz stopped) of the systemd
 socket activation patch-set. This version addresses all review comments
 made in response to my previous posting of this set.

 Testing can still be done like this:

 1) Create a file named /etc/systemd/system/xorg.socket with the following
 contents:

 [Unit]
 Description=Xorg DISPLAY=:1.0 socket

 [Socket]
 ListenStream=/tmp/.X11-unix/X1

 [Install]
 WantedBy=sockets.target

 2) Create a file named /etc/systemd/system/xorg.service with the following
 contents:

 [Unit]
 Description=Xorg DISPLAY=:1.0 Service

 [Service]
 ExecStart=/usr/bin/Xorg :1

 [Install]
 Also=xorg.socke

 3) In a terminal / on the console do:
 export DISPLAY=:1
 metacity
 xterm

 Regards,

 Hans
 ___
 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
___
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: [PATCH 08/10] Add support for MIT-SHM AttachFd request

[Kristian Høgsberg]

On Fri, Nov 1, 2013 at 12:37 AM, Keith Packard kei...@keithp.com wrote:
 Kristian Høgsberg k...@bitplanet.net writes:

 On Thu, Oct 31, 2013 at 3:43 PM, Keith Packard kei...@keithp.com wrote:
 This passes a file descriptor from the client to the server, which is
 then mmap'd

 A problem we recently hit in wayland, which also affects this
 extension is that a client can set up shared memory like this and the
 truncate the tmp file to 0.  When the server then tries to access the
 mapped memory it dies with SIGBUS.  We're planning on handling this
 case by installing a SIGBUS handler that flags the error, maps
 /dev/zero over the faulting mmap area and then lets the access
 continue.  We'll wrap access the the map with call to begin/end access
 functions and in the end_access function we check the flag to see if
 the access cause a fault and kill the client in that case.

 Thanks; I'll have to think about how to handle this in the X server
 case.

Just so we're clear, what I'm saying above is that this request can be
trivially exploited by any client to crash the X server with SIGBUS.

Kristian
___
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: [PATCH 08/10] Add support for MIT-SHM AttachFd request

[Kristian Høgsberg]

On Thu, Oct 31, 2013 at 3:43 PM, Keith Packard kei...@keithp.com wrote:
 This passes a file descriptor from the client to the server, which is
 then mmap'd

A problem we recently hit in wayland, which also affects this
extension is that a client can set up shared memory like this and the
truncate the tmp file to 0.  When the server then tries to access the
mapped memory it dies with SIGBUS.  We're planning on handling this
case by installing a SIGBUS handler that flags the error, maps
/dev/zero over the faulting mmap area and then lets the access
continue.  We'll wrap access the the map with call to begin/end access
functions and in the end_access function we check the flag to see if
the access cause a fault and kill the client in that case.

Kristian

 Signed-off-by: Keith Packard kei...@keithp.com
 ---
  Xext/shm.c| 159 
 --
  Xext/shmint.h |   1 +
  include/os.h  |   2 +
  os/io.c   |   8 +++
  4 files changed, 167 insertions(+), 3 deletions(-)

 diff --git a/Xext/shm.c b/Xext/shm.c
 index f6995cc..1a70260 100644
 --- a/Xext/shm.c
 +++ b/Xext/shm.c
 @@ -53,6 +53,7 @@ in this Software without prior written authorization from 
 The Open Group.
  #include xace.h
  #include X11/extensions/shmproto.h
  #include X11/Xfuncproto.h
 +#include sys/mman.h
  #include protocol-versions.h

  /* Needed for Solaris cross-zone shared memory extension */
 @@ -382,8 +383,10 @@ ProcShmAttach(ClientPtr client)
  client-errorValue = stuff-readOnly;
  return BadValue;
  }
 -for (shmdesc = Shmsegs;
 - shmdesc  (shmdesc-shmid != stuff-shmid); shmdesc = 
 shmdesc-next);
 +for (shmdesc = Shmsegs; shmdesc; shmdesc = shmdesc-next) {
 +if (!shmdesc-is_fd  shmdesc-shmid == stuff-shmid)
 +break;
 +}
  if (shmdesc) {
  if (!stuff-readOnly  !shmdesc-writable)
  return BadAccess;
 @@ -393,6 +396,7 @@ ProcShmAttach(ClientPtr client)
  shmdesc = malloc(sizeof(ShmDescRec));
  if (!shmdesc)
  return BadAlloc;
 +shmdesc-is_fd = FALSE;
  shmdesc-addr = shmat(stuff-shmid, 0,
stuff-readOnly ? SHM_RDONLY : 0);
  if ((shmdesc-addr == ((char *) -1)) || SHMSTAT(stuff-shmid, buf)) 
 {
 @@ -431,7 +435,10 @@ ShmDetachSegment(pointer value, /* must conform to 
 DeleteType */

  if (--shmdesc-refcnt)
  return TRUE;
 -shmdt(shmdesc-addr);
 +if (shmdesc-is_fd)
 +munmap(shmdesc-addr, shmdesc-size);
 +else
 +shmdt(shmdesc-addr);
  for (prev = Shmsegs; *prev != shmdesc; prev = (*prev)-next);
  *prev = shmdesc-next;
  free(shmdesc);
 @@ -1088,6 +1095,122 @@ ProcShmCreatePixmap(ClientPtr client)
  }

  static int
 +ProcShmAttachFd(ClientPtr client)
 +{
 +int fd;
 +ShmDescPtr shmdesc;
 +REQUEST(xShmAttachFdReq);
 +struct stat statb;
 +
 +SetReqFds(client, 1);
 +REQUEST_SIZE_MATCH(xShmAttachFdReq);
 +LEGAL_NEW_RESOURCE(stuff-shmseg, client);
 +if ((stuff-readOnly != xTrue)  (stuff-readOnly != xFalse)) {
 +client-errorValue = stuff-readOnly;
 +return BadValue;
 +}
 +fd = ReadFdFromClient(client);
 +if (fd  0)
 +return BadMatch;
 +
 +if (fstat(fd, statb)  0 || statb.st_size == 0) {
 +close(fd);
 +return BadMatch;
 +}
 +
 +shmdesc = malloc(sizeof(ShmDescRec));
 +if (!shmdesc) {
 +close(fd);
 +return BadAlloc;
 +}
 +shmdesc-is_fd = TRUE;
 +shmdesc-addr = mmap(NULL, statb.st_size,
 + stuff-readOnly ? PROT_READ : PROT_READ|PROT_WRITE,
 + MAP_SHARED,
 + fd, 0);
 +
 +close(fd);
 +if ((shmdesc-addr == ((char *) -1))) {
 +free(shmdesc);
 +return BadAccess;
 +}
 +
 +shmdesc-refcnt = 1;
 +shmdesc-writable = !stuff-readOnly;
 +shmdesc-size = statb.st_size;
 +shmdesc-next = Shmsegs;
 +Shmsegs = shmdesc;
 +
 +if (!AddResource(stuff-shmseg, ShmSegType, (pointer) shmdesc))
 +return BadAlloc;
 +return Success;
 +}
 +
 +static int
 +ProcShmCreateSegment(ClientPtr client)
 +{
 +int fd;
 +ShmDescPtr shmdesc;
 +REQUEST(xShmCreateSegmentReq);
 +xShmCreateSegmentReply rep = {
 +.type = X_Reply,
 +.nfd = 1,
 +.sequenceNumber = client-sequence,
 +.length = 0,
 +};
 +char template[] = /tmp/shm-XX;
 +
 +REQUEST_SIZE_MATCH(xShmCreateSegmentReq);
 +if ((stuff-readOnly != xTrue)  (stuff-readOnly != xFalse)) {
 +client-errorValue = stuff-readOnly;
 +return BadValue;
 +}
 +fd = mkstemp(template);
 +if (fd  0)
 +return BadAlloc;
 +unlink(template);
 +if (ftruncate(fd, stuff-size)  0) {
 +close(fd);
 +return BadAlloc;
 +}
 +shmdesc = malloc(sizeof(ShmDescRec));
 +if (!shmdesc) {
 +close(fd);
 +return 

[PATCH 1/4] xkb: Add struct XkbCompContext

[Kristian Høgsberg]

This commit adds a struct that contains most of the context for starting,
running and cleaning up after xkbcomp.

Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 xkb/ddxLoad.c | 76 +--
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
index cb2dfc3..c717bd5 100644
--- a/xkb/ddxLoad.c
+++ b/xkb/ddxLoad.c
@@ -165,14 +165,21 @@ OutputDirectory(char *outdir, size_t size)
 }
 }
 
+typedef struct XkbCompContext {
+char keymap[PATH_MAX];
+FILE *out;
+char *buf;
+char tmpname[PATH_MAX];
+const char *xkmfile;
+} XkbCompContextRec, *XkbCompContextPtr;
+
 static Bool
 XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
XkbComponentNamesPtr names,
unsigned want,
-   unsigned need, char *nameRtrn, int nameRtrnLen)
+   unsigned need, XkbCompContextPtr ctx)
 {
-FILE *out;
-char *buf = NULL, keymap[PATH_MAX], xkm_output_dir[PATH_MAX];
+char xkm_output_dir[PATH_MAX];
 
 const char *emptystring = ;
 char *xkbbasedirflag = NULL;
@@ -180,22 +187,19 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 const char *xkbbindirsep = emptystring;
 
 #ifdef WIN32
-/* WIN32 has no popen. The input must be stored in a file which is
-   used as input for xkbcomp. xkbcomp does not read from stdin. */
-char tmpname[PATH_MAX];
-const char *xkmfile = tmpname;
+ctx-xkmfile = ctx-tmpname;
 #else
-const char *xkmfile = -;
+ctx-xkmfile = -;
 #endif
 
-snprintf(keymap, sizeof(keymap), server-%s, display);
+snprintf(ctx-keymap, sizeof(ctx-keymap), server-%s, display);
 
 OutputDirectory(xkm_output_dir, sizeof(xkm_output_dir));
 
 #ifdef WIN32
-strcpy(tmpname, Win32TempDir());
-strcat(tmpname, \\xkb_XX);
-(void) mktemp(tmpname);
+strcpy(ctx-tmpname, Win32TempDir());
+strcat(ctx-tmpname, \\xkb_XX);
+(void) mktemp(ctx-tmpname);
 #endif
 
 if (XkbBaseDirectory != NULL) {
@@ -214,73 +218,69 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 }
 }
 
-if (asprintf(buf,
+if (asprintf(ctx-buf,
  \%s%sxkbcomp\ -w %d %s -xkm \%s\ 
  -em1 %s -emp %s -eml %s \%s%s.xkm\,
  xkbbindir, xkbbindirsep,
  ((xkbDebugFlags  2) ? 1 :
   ((xkbDebugFlags  10) ? 10 : (int) xkbDebugFlags)),
- xkbbasedirflag ? xkbbasedirflag : , xkmfile,
+ xkbbasedirflag ? xkbbasedirflag : , ctx-xkmfile,
  PRE_ERROR_MSG, ERROR_PREFIX, POST_ERROR_MSG1,
- xkm_output_dir, keymap) == -1)
-buf = NULL;
+ xkm_output_dir, ctx-keymap) == -1)
+ctx-buf = NULL;
 
 free(xkbbasedirflag);
 
-if (!buf) {
+if (!ctx-buf) {
 LogMessage(X_ERROR,
XKB: Could not invoke xkbcomp: not enough memory\n);
 return FALSE;
 }
 
 #ifndef WIN32
-out = Popen(buf, w);
+ctx-out = Popen(ctx-buf, w);
 #else
-out = fopen(tmpname, w);
+ctx-out = fopen(ctx-tmpname, w);
 #endif
 
-if (out != NULL) {
+if (ctx-out != NULL) {
 #ifdef DEBUG
 if (xkbDebugFlags) {
 ErrorF([xkb] XkbDDXCompileKeymapByNames compiling keymap:\n);
 XkbWriteXKBKeymapForNames(stderr, names, xkb, want, need);
 }
 #endif
-XkbWriteXKBKeymapForNames(out, names, xkb, want, need);
+XkbWriteXKBKeymapForNames(ctx-out, names, xkb, want, need);
 #ifndef WIN32
-if (Pclose(out) == 0)
+if (Pclose(ctx-out) == 0)
 #else
-if (fclose(out) == 0  System(buf) = 0)
+if (fclose(ctx-out) == 0  System(ctx-buf) = 0)
 #endif
 {
 if (xkbDebugFlags)
-DebugF([xkb] xkb executes: %s\n, buf);
-if (nameRtrn) {
-strlcpy(nameRtrn, keymap, nameRtrnLen);
-}
-free(buf);
+DebugF([xkb] xkb executes: %s\n, ctx-buf);
+free(ctx-buf);
 #ifdef WIN32
-unlink(tmpname);
+unlink(ctx-tmpname);
 #endif
 return TRUE;
 }
 else
-LogMessage(X_ERROR, Error compiling keymap (%s)\n, keymap);
+LogMessage(X_ERROR, Error compiling keymap (%s)\n, ctx-keymap);
 #ifdef WIN32
 /* remove the temporary file */
-unlink(tmpname);
+unlink(ctx-tmpname);
 #endif
 }
 else {
 #ifndef WIN32
 LogMessage(X_ERROR, XKB: Could not invoke xkbcomp\n);
 #else
-LogMessage(X_ERROR, Could not open file %s\n, tmpname);
+LogMessage(X_ERROR, Could not open file %s\n, ctx-tmpname);
 #endif
 }
-if (nameRtrn)
-nameRtrn[0] = '\0';
-free(buf);
+ctx-keymap[0] = '\0';
+free(ctx-buf);
 return FALSE;
 }
 
@@ -331,6 +331,7 @@ XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
 FILE *file;
 char fileName[PATH_MAX];
 

[PATCH 2/4] xkb: Split out code to start and finish xkbcomp

[Kristian Høgsberg]

Using the context struct from previous commit, we can now split out
code to start xkbcomp and to finish and clean up after it.

Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 xkb/ddxLoad.c | 39 ---
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
index c717bd5..5defaf1 100644
--- a/xkb/ddxLoad.c
+++ b/xkb/ddxLoad.c
@@ -174,10 +174,7 @@ typedef struct XkbCompContext {
 } XkbCompContextRec, *XkbCompContextPtr;
 
 static Bool
-XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
-   XkbComponentNamesPtr names,
-   unsigned want,
-   unsigned need, XkbCompContextPtr ctx)
+StartXkbComp(XkbCompContextPtr ctx)
 {
 char xkm_output_dir[PATH_MAX];
 
@@ -243,14 +240,15 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 ctx-out = fopen(ctx-tmpname, w);
 #endif
 
+return ctx-out != NULL;
+}
+
+static Bool
+FinishXkbComp(XkbCompContextPtr ctx)
+{
+if (!ctx-buf)
+   return FALSE;
 if (ctx-out != NULL) {
-#ifdef DEBUG
-if (xkbDebugFlags) {
-ErrorF([xkb] XkbDDXCompileKeymapByNames compiling keymap:\n);
-XkbWriteXKBKeymapForNames(stderr, names, xkb, want, need);
-}
-#endif
-XkbWriteXKBKeymapForNames(ctx-out, names, xkb, want, need);
 #ifndef WIN32
 if (Pclose(ctx-out) == 0)
 #else
@@ -284,6 +282,25 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 return FALSE;
 }
 
+static Bool
+XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
+   XkbComponentNamesPtr names,
+   unsigned want,
+   unsigned need, XkbCompContextPtr ctx)
+{
+if (StartXkbComp(ctx)) {
+#ifdef DEBUG
+if (xkbDebugFlags) {
+ErrorF([xkb] XkbDDXCompileKeymapByNames compiling keymap:\n);
+XkbWriteXKBKeymapForNames(stderr, names, xkb, want, need);
+}
+#endif
+XkbWriteXKBKeymapForNames(ctx-out, names, xkb, want, need);
+}
+
+return FinishXkbComp(ctx);
+}
+
 static FILE *
 XkbDDXOpenConfigFile(char *mapName, char *fileNameRtrn, int fileNameRtrnLen)
 {
-- 
1.8.1.4

___
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

[PATCH 3/4] xkb: Add XkbCompileKeymapFromString()

[Kristian Høgsberg]

This new function compiles a keymap from an in-memory string.  We use it
to add a new keyooard device init function,
InitKeyboardDeviceStructFromString(), which inits a keyboard device with
a keymap specified as a string instead of a rmlvo set.

Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 include/input.h  |   6 +++
 include/xkbsrv.h |   4 ++
 xkb/ddxLoad.c| 129 ++-
 xkb/xkbInit.c|  44 +++
 4 files changed, 144 insertions(+), 39 deletions(-)

diff --git a/include/input.h b/include/input.h
index 991d648..4bd34d4 100644
--- a/include/input.h
+++ b/include/input.h
@@ -382,6 +382,12 @@ extern _X_EXPORT Bool 
InitKeyboardDeviceStruct(DeviceIntPtr /*device */ ,
KbdCtrlProcPtr /*controlProc */
);
 
+extern _X_EXPORT Bool InitKeyboardDeviceStructFromString(DeviceIntPtr dev,
+const char *keymap,
+int keymap_length,
+BellProcPtr bell_func,
+KbdCtrlProcPtr 
ctrl_func);
+
 extern int ApplyPointerMapping(DeviceIntPtr /* pDev */ ,
CARD8 * /* map */ ,
int /* len */ ,
diff --git a/include/xkbsrv.h b/include/xkbsrv.h
index 3b72885..fd9a1b8 100644
--- a/include/xkbsrv.h
+++ b/include/xkbsrv.h
@@ -872,4 +872,8 @@ extern _X_EXPORT XkbDescPtr XkbCompileKeymap(DeviceIntPtr 
/* dev */ ,
  XkbRMLVOSet *  /* rmlvo */
 );
 
+extern _X_EXPORT XkbDescPtr XkbCompileKeymapFromString(DeviceIntPtr dev,
+  const char *keymap,
+  int keymap_length);
+
 #endif  /* _XKBSRV_H_ */
diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
index 5defaf1..404ba3f 100644
--- a/xkb/ddxLoad.c
+++ b/xkb/ddxLoad.c
@@ -337,6 +337,35 @@ XkbDDXOpenConfigFile(char *mapName, char *fileNameRtrn, 
int fileNameRtrnLen)
 return file;
 }
 
+static unsigned
+LoadXKM(unsigned want, unsigned need, XkbCompContextPtr ctx, XkbDescPtr 
*xkbRtrn)
+{
+FILE *file;
+char fileName[PATH_MAX];
+unsigned missing;
+
+file = XkbDDXOpenConfigFile(ctx-keymap, fileName, PATH_MAX);
+if (file == NULL) {
+LogMessage(X_ERROR, Couldn't open compiled keymap file %s\n,
+   fileName);
+return 0;
+}
+missing = XkmReadFile(file, need, want, xkbRtrn);
+if (*xkbRtrn == NULL) {
+LogMessage(X_ERROR, Error loading keymap %s\n, fileName);
+fclose(file);
+(void) unlink(fileName);
+return 0;
+}
+else {
+DebugF(Loaded XKB keymap %s, defined=0x%x\n, fileName,
+   (*xkbRtrn)-defined);
+}
+fclose(file);
+(void) unlink(fileName);
+return (need | want)  (~missing);
+}
+
 unsigned
 XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
 XkbComponentNamesPtr names,
@@ -345,9 +374,6 @@ XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
 XkbDescPtr *xkbRtrn, char *nameRtrn, int nameRtrnLen)
 {
 XkbDescPtr xkb;
-FILE *file;
-char fileName[PATH_MAX];
-unsigned missing;
 XkbCompContextRec ctx;
 
 *xkbRtrn = NULL;
@@ -367,26 +393,30 @@ XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
 LogMessage(X_ERROR, XKB: Couldn't compile keymap\n);
 return 0;
 }
-file = XkbDDXOpenConfigFile(ctx.keymap, fileName, PATH_MAX);
-if (file == NULL) {
-LogMessage(X_ERROR, Couldn't open compiled keymap file %s\n,
-   fileName);
-return 0;
-}
-missing = XkmReadFile(file, need, want, xkbRtrn);
-if (*xkbRtrn == NULL) {
-LogMessage(X_ERROR, Error loading keymap %s\n, fileName);
-fclose(file);
-(void) unlink(fileName);
+
+return LoadXKM(want, need, ctx, xkbRtrn);
+}
+
+static unsigned
+XkbDDXLoadKeymapFromString(DeviceIntPtr keybd,
+  const char *keymap, int keymap_length,
+  unsigned want,
+  unsigned need,
+  XkbDescPtr *xkbRtrn)
+{
+XkbCompContextRec ctx;
+
+*xkbRtrn = NULL;
+
+if (StartXkbComp(ctx))
+   fwrite(keymap, keymap_length, 1, ctx.out);
+
+if (!FinishXkbComp(ctx)) {
+LogMessage(X_ERROR, XKB: Couldn't compile keymap\n);
 return 0;
 }
-else {
-DebugF(Loaded XKB keymap %s, defined=0x%x\n, fileName,
-   (*xkbRtrn)-defined);
-}
-fclose(file);
-(void) unlink(fileName);
-return (need | want)  (~missing);
+
+return LoadXKM(want, need, ctx, xkbRtrn);
 }
 
 Bool
@@ -482,6 +512,29 @@ XkbCompileKeymapForDevice(DeviceIntPtr dev, XkbRMLVOSet * 
rmlvo, 

[FYI PATCH 4/4] xwayland: Use InitKeyboardDeviceStructFromString

[Kristian Høgsberg]

We can now use this new entry point from xwayland to initialize the
keyboard map to what the wayland server sends.

Reviewed-by: Daniel Stone dan...@fooishbar.org
---
 hw/xfree86/xwayland/xwayland-input.c   | 55 --
 hw/xfree86/xwayland/xwayland-private.h |  5 
 2 files changed, 45 insertions(+), 15 deletions(-)

This one isn't for xserver master.  I'll add it to the xwayland
branch, but it's here to show the motivation for the three previous
patches.

Kristian

diff --git a/hw/xfree86/xwayland/xwayland-input.c 
b/hw/xfree86/xwayland/xwayland-input.c
index 8f52c3e..b6f3331 100644
--- a/hw/xfree86/xwayland/xwayland-input.c
+++ b/hw/xfree86/xwayland/xwayland-input.c
@@ -51,6 +51,7 @@
 #include windowstr.h
 #include xf86Priv.h
 #include mipointrst.h
+#include sys/mman.h
 
 #include xwayland.h
 #include xwayland-private.h
@@ -137,22 +138,17 @@ xwl_keyboard_control(DeviceIntPtr device, KeybdCtrl *ctrl)
 static int
 xwl_keyboard_proc(DeviceIntPtr device, int what)
 {
-XkbRMLVOSet rmlvo;
+InputInfoPtr pInfo = device-public.devicePrivate;
+struct xwl_seat *xwl_seat = pInfo-private;
+int len;
 
 switch (what) {
 case DEVICE_INIT:
device-public.on = FALSE;
-
-/* FIXME: Get the keymap from wl_keyboard::keymap events, which
- *requires more X server API to set a keymap from a string
- *rather than RMLVO. */
-rmlvo.rules = evdev;
-rmlvo.model = evdev;
-rmlvo.layout = us;
-rmlvo.variant = NULL;
-rmlvo.options = NULL;
-
-if (!InitKeyboardDeviceStruct(device, rmlvo, NULL, 
xwl_keyboard_control))
+   len = strnlen(xwl_seat-keymap, xwl_seat-keymap_size);
+if (!InitKeyboardDeviceStructFromString(device, xwl_seat-keymap,
+   len,
+   NULL, xwl_keyboard_control))
 return BadValue;
 
 return Success;
@@ -441,7 +437,12 @@ static void
 keyboard_handle_keymap(void *data, struct wl_keyboard *keyboard,
   uint32_t format, int fd, uint32_t size)
 {
-/* FIXME: Handle keymap */
+struct xwl_seat *xwl_seat = data;
+
+xwl_seat-keymap_size = size;
+xwl_seat-keymap = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
+if (xwl_seat-keymap == MAP_FAILED)
+   ;   /* wah wah */
 
 close(fd);
 }
@@ -492,13 +493,30 @@ static const struct wl_keyboard_listener 
keyboard_listener = {
 };
 
 static void
+add_devices(void *data, struct wl_callback *callback, uint32_t time)
+{
+struct xwl_seat *xwl_seat = data;
+
+wl_callback_destroy(callback);
+
+if (xwl_seat-wl_pointer)
+   xwl_seat-pointer = device_added(xwl_seat, xwayland-pointer);
+if (xwl_seat-wl_keyboard)
+   xwl_seat-keyboard = device_added(xwl_seat, xwayland-keyboard);
+}
+
+static const struct wl_callback_listener add_devices_listener = {
+   add_devices
+};
+
+static void
 seat_handle_capabilities(void *data, struct wl_seat *seat,
 enum wl_seat_capability caps)
 {
struct xwl_seat *xwl_seat = data;
+   struct wl_callback *callback;
 
if (caps  WL_SEAT_CAPABILITY_POINTER) {
-   xwl_seat-pointer = device_added(xwl_seat, xwayland-pointer);
xwl_seat-wl_pointer = wl_seat_get_pointer(seat);
wl_pointer_add_listener(xwl_seat-wl_pointer,
pointer_listener, xwl_seat);
@@ -506,12 +524,19 @@ seat_handle_capabilities(void *data, struct wl_seat *seat,
}
 
if (caps  WL_SEAT_CAPABILITY_KEYBOARD) {
-   xwl_seat-keyboard = device_added(xwl_seat, xwayland-keyboard);
xwl_seat-wl_keyboard = wl_seat_get_keyboard(seat);
wl_keyboard_add_listener(xwl_seat-wl_keyboard,
 keyboard_listener, xwl_seat);
+
}
 /* FIXME: Touch ... */
+
+   /* Add devices after we've received keymaps. */
+   if (caps) {
+   callback = wl_display_sync(xwl_seat-xwl_screen-display);
+   wl_callback_add_listener(callback,
+add_devices_listener, xwl_seat);
+   }
 }
 
 static const struct wl_seat_listener seat_listener = {
diff --git a/hw/xfree86/xwayland/xwayland-private.h 
b/hw/xfree86/xwayland/xwayland-private.h
index 5b602d7..3bdd1b9 100644
--- a/hw/xfree86/xwayland/xwayland-private.h
+++ b/hw/xfree86/xwayland/xwayland-private.h
@@ -99,8 +99,13 @@ struct xwl_seat {
 uint32_tpointer_enter_serial;
 struct xorg_listlink;
 CursorPtrx_cursor;
+
+size_t  keymap_size;
+char   *keymap;
+
 };
 
+
 struct xwl_screen *xwl_screen_get(ScreenPtr screen);
 
 void xwayland_screen_preinit_output(struct xwl_screen *xwl_screen, ScrnInfoPtr 
scrninfo);
-- 
1.8.1.4

___
xorg-devel@lists.x.org: 

Re: Initial DRI3000 protocol specs available

[Kristian Høgsberg]

On Fri, Mar 8, 2013 at 3:59 AM, James Jones jajo...@nvidia.com wrote:
 On 03/07/2013 05:17 PM, Keith Packard wrote:

 * PGP Signed by an unknown key

 James Jones jajo...@nvidia.com writes:

 There didn't seem to be much interest outside of NVIDIA, so
 besides fence sync, the ideas are tabled internally ATM.


 This shouldn't surprise you though -- no-one else needs this kind of
 synchronization, so it's really hard for anyone to evaluate it. And,
 DRI2 offers 'sufficient' support for the various GL sync extensions.

 I was referring to the multi-buffer/tear-free presentation part, not the
 synchronization parts.  I still rather surprised everyone thinks implicit
 synchronization is a good idea though.  I don't think we're the only ones
 that have loosely defined command buffer processing in HW anymore.  Meh.

It's not that other hw don't have that (or even other drivers for your
hw, ie nouveau).  Serializing through the kernel execution manager
lets the kernel know the the expected order of rendering.  If
rendering in hw queue A depends on a result from a hw queue B (B
renders to buffer, A textures from same buffer),  the kernel can
insert synchronization primitives to ensure that the A queue doesn't
proceed before the B queue signals the fence.  If A and B queues don't
have any inter-dependencies, no synchronization is necessary and they
can run in parallel or out of order.

Kristian
___
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: [PATCH] os: use libunwind to generate backtraces

[Kristian Høgsberg]

On Sat, Feb 16, 2013 at 10:45 AM, Marcin Slusarz
marcin.slus...@gmail.com wrote:
 Libunwind generates backtraces much more reliably than glibc's backtrace.

Wow, didn't know about libunwind, it looks amazing.  Do you mind if
port this and use in weston?

Kristian

 Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
 ---
  configure.ac|  7 +
  include/dix-config.h.in |  3 +++
  os/Makefile.am  |  5 
  os/backtrace.c  | 70 
 +
  4 files changed, 85 insertions(+)

 diff --git a/configure.ac b/configure.ac
 index 9415a54..4a292da 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -309,6 +309,13 @@ AC_CHECK_HEADER([execinfo.h],[
  ])]
  )

 +PKG_CHECK_MODULES(LIBUNWIND, libunwind, [HAVE_LIBUNWIND=yes], 
 [HAVE_LIBUNWIND=no])
 +if test x$HAVE_LIBUNWIND = xyes; then
 +   AC_DEFINE(HAVE_LIBUNWIND, 1, [Have libunwind support])
 +fi
 +AM_CONDITIONAL(HAVE_LIBUNWIND, [test x$HAVE_LIBUNWIND = xyes])
 +
 +
  dnl 
 ---
  dnl Bus options and CPU capabilities.  Replaces logic in
  dnl hw/xfree86/os-support/bus/Makefile.am, among others.
 diff --git a/include/dix-config.h.in b/include/dix-config.h.in
 index 578f249..5102263 100644
 --- a/include/dix-config.h.in
 +++ b/include/dix-config.h.in
 @@ -60,6 +60,9 @@
  /* Has backtrace support */
  #undef HAVE_BACKTRACE

 +/* Has libunwind support */
 +#undef HAVE_LIBUNWIND
 +
  /* Define to 1 if you have the byteswap.h header file. */
  #undef HAVE_BYTESWAP_H

 diff --git a/os/Makefile.am b/os/Makefile.am
 index 8891485..364b6da 100644
 --- a/os/Makefile.am
 +++ b/os/Makefile.am
 @@ -34,6 +34,11 @@ if XDMCP
  libos_la_SOURCES += $(XDMCP_SRCS)
  endif

 +if HAVE_LIBUNWIND
 +AM_CFLAGS += $(LIBUNWIND_CFLAGS)
 +libos_la_LIBADD += $(LIBUNWIND_LIBS)
 +endif
 +
  EXTRA_DIST = $(SECURERPC_SRCS) $(XDMCP_SRCS)

  if SPECIAL_DTRACE_OBJECTS
 diff --git a/os/backtrace.c b/os/backtrace.c
 index daac60c..02aeb03 100644
 --- a/os/backtrace.c
 +++ b/os/backtrace.c
 @@ -30,6 +30,75 @@
  #include errno.h
  #include string.h

 +#ifdef HAVE_LIBUNWIND
 +
 +#define UNW_LOCAL_ONLY
 +#include libunwind.h
 +
 +#ifndef _GNU_SOURCE
 +#define _GNU_SOURCE
 +#endif
 +#include dlfcn.h
 +
 +void
 +xorg_backtrace(void)
 +{
 +unw_cursor_t cursor;
 +unw_context_t context;
 +unw_word_t off;
 +unw_proc_info_t pip;
 +int ret, i = 0;
 +char procname[256];
 +const char *filename;
 +Dl_info dlinfo;
 +
 +pip.unwind_info = NULL;
 +ret = unw_getcontext(context);
 +if (ret) {
 +ErrorFSigSafe(unw_getcontext: %d\n, ret);
 +return;
 +}
 +
 +ret = unw_init_local(cursor, context);
 +if (ret) {
 +ErrorFSigSafe(unw_init_local: %d\n, ret);
 +return;
 +}
 +
 +ErrorFSigSafe(\n);
 +ErrorFSigSafe(Backtrace:\n);
 +ret = unw_step(cursor);
 +while (ret  0) {
 +ret = unw_get_proc_info(cursor, pip);
 +if (ret) {
 +ErrorFSigSafe(unw_get_proc_info: %d\n, ret);
 +break;
 +}
 +
 +ret = unw_get_proc_name(cursor, procname, 256, off);
 +if (ret  ret != -UNW_ENOMEM) {
 +if (ret != -UNW_EUNSPEC)
 +ErrorFSigSafe(unw_get_proc_name: %d\n, ret);
 +procname[0] = '?';
 +procname[1] = 0;
 +}
 +
 +if (dladdr((void *)(pip.start_ip + off), dlinfo)  
 dlinfo.dli_fname 
 +*dlinfo.dli_fname)
 +filename = dlinfo.dli_fname;
 +else
 +filename = ?;
 +
 +ErrorFSigSafe(%u: %s (%s%s+0x%x) [%p]\n, i++, filename, procname,
 +ret == -UNW_ENOMEM ? ... : , (int)off, (void *)(pip.start_ip 
 + off));
 +
 +ret = unw_step(cursor);
 +if (ret  0)
 +ErrorFSigSafe(unw_step: %d\n, ret);
 +}
 +ErrorFSigSafe(\n);
 +}
 +#else
  #ifdef HAVE_BACKTRACE
  #ifndef _GNU_SOURCE
  #define _GNU_SOURCE
 @@ -246,3 +315,4 @@ xorg_backtrace(void)

  #endif
  #endif
 +#endif
 --
 1.8.1

 ___
 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
___
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: [PATCH xserver 0/2] glx/dri2: Fix bug #50019.

[Kristian Høgsberg]

On Thu, Jul 12, 2012 at 7:16 AM, Michel Dänzer mic...@daenzer.net wrote:
 As suggested by Kristian Høgsberg, explicitly call FreeResource for the DRI2
 drawable reference.

 [PATCH 1/2] dri2: Add DRI2CreateDrawable2.
 [PATCH 2/2] glx: Free DRI2 drawable reference to destroyed GLX

Thanks, Michel.  For the series:

Reviewed-by: Kristian Høgsberg k...@bitplanet.net
___
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: [PATCH] dri2proto: add prime protocol support. (v2.1)

[Kristian Høgsberg]

On Fri, Jul 6, 2012 at 6:27 AM, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com

 So we reserve bits 16-19 for offload device ids, this means we can
 have 6 offload devices, which is plenty for now, and we can bump
 this further later without fear.

 v2: I suck at maths, that is all. Fixed up the maths to match reality.
 v2.1: fix typo

Looks fine to me.  Is there a version bump that goes along with this
so drivers can know they can ask for prime devices or should they just
try and expect a NULL reply if it's not available?

Reviewed-by: Kristian Høgsberg k...@bitplanet.net

 Signed-off-by: Dave Airlie airl...@redhat.com
 ---
  dri2tokens.h |5 +
  1 file changed, 5 insertions(+)

 diff --git a/dri2tokens.h b/dri2tokens.h
 index 16c9008..bdca866 100644
 --- a/dri2tokens.h
 +++ b/dri2tokens.h
 @@ -45,6 +45,11 @@
  #define DRI2BufferDepthStencil 9
  #define DRI2BufferHiz  10

 +/* keep bits 16 and above for prime IDs */
 +#define DRI2DriverPrimeMask 7 /* 0 - 7 - allows for 6 devices*/
 +#define DRI2DriverPrimeShift   16
 +#define DRI2DriverPrimeId(x) (((x)  DRI2DriverPrimeShift)  
 (DRI2DriverPrimeMask))
 +
  #define DRI2DriverDRI  0
  #define DRI2DriverVDPAU1

 --
 1.7.10.2

 ___
 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
___
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: [PATCH] glx: Pass GLX drawable ID to DRI2CreateDrawable().

[Kristian Høgsberg]

On Wed, Jul 4, 2012 at 5:03 AM, Michel Dänzer mic...@daenzer.net wrote:
 On Fre, 2012-06-29 at 19:09 +0200, Michel Dänzer wrote:
 On Fre, 2012-06-29 at 12:58 -0400, Kristian Høgsberg wrote:
  On Fri, Jun 29, 2012 at 12:30 PM, Michel Dänzer mic...@daenzer.net wrote:
   On Fre, 2012-06-29 at 12:20 -0400, Kristian Høgsberg wrote:
   On Thu, Jun 28, 2012 at 7:39 AM, Michel Dänzer mic...@daenzer.net 
   wrote:
From: Michel Dänzer michel.daen...@amd.com
   
Otherwise the DRI2Drawable may retain references to the destroyed
__GLXDRIdrawable, leading to use after free in 
__glXDRIinvalidateBuffers().
  
   That looks wrong to me.  DRI2 isn't concerned with GLX drawables, only
   X drawables.  If you're destroying the GLX drawable and want to not
   get invalidate callbacks, you need to destroy the DRI2DrawableRef that
   DRI2CreateDrawable creates.
  
   Which is what this patch does? :) (By means of
   glxcmds.c:DoDestroyDrawable - FreeResource - DRI2DrawableGone, where
   the ID matches ref-id, so it calls
   FreeResourceByType(ref-dri2_id, ...) as well)
  
   Can you explain why the non-GLX drawable ID needs to be passed to
   DRI2CreateDrawable?
 
  The DRI2Drawable is created for the X drawable, not the GLX drawable.
  When the X drawable goes away the DRI2 drawable needs to go away.

 And it still does. When the X drawable goes away, so does the GLX
 drawable (via a similar Resource trick), so the above sequence comes
 into play.


  It works the way it does, since a pixmap can have multiple XIDs and for
  each XID, mutliple clients could call DRI2CreateClient.  We need to
  keep the DRI2 drawable alive for each reference for each XID.  DRI2
  automatically reclaims the DRI2Drawable when the underlying X drawable
  is destroyed, but that will break if you pass in the GLX drawable XID.

 I don't understand how that will break given the above. Can you
 elaborate?

 I can implement the more complicated fix you suggested, but I'd like to
 understand why it's necessary. It should be helpful if you could
 describe a specific scenario that would break with the simple fix.

I'm not saying it wont work, just that I don't want to revisit all the
different destruction paths that are possible and see if what you
propose works.  This code is a nightmare, but I think that with the
way the DRI2 ref-counting and destruction code works now, it's been
pretty stable.  If you want to use it in a different way, go for it,
to me it just seems much easier and safer to just use the code the way
it was supposed to work instead of saving a bit of typing and then
have to think through all the glx/dri2 destruction paths again.

Kristian
___
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: [PATCH] dri2proto: add prime protocol support. (v2.1)

[Kristian Høgsberg]

On Fri, Jul 6, 2012 at 9:41 AM, Dave Airlie airl...@gmail.com wrote:
 On Fri, Jul 6, 2012 at 2:32 PM, Kristian Høgsberg k...@bitplanet.net wrote:
 On Fri, Jul 6, 2012 at 6:27 AM, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com

 So we reserve bits 16-19 for offload device ids, this means we can
 have 6 offload devices, which is plenty for now, and we can bump
 this further later without fear.

 v2: I suck at maths, that is all. Fixed up the maths to match reality.
 v2.1: fix typo

 Looks fine to me.  Is there a version bump that goes along with this
 so drivers can know they can ask for prime devices or should they just
 try and expect a NULL reply if it's not available?

 Drivers don't ever know about prime, its all hidden in the
 server/libGL/dri2 layers.

Ah, I meant client side / aiglx there, ie how does the caller know
that there are prime device types available.  But just asking and
getting NULL sounds fine.

Kristian
___
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: [PATCH] glx: Pass GLX drawable ID to DRI2CreateDrawable().

[Kristian Høgsberg]

On Fri, Jun 29, 2012 at 12:30 PM, Michel Dänzer mic...@daenzer.net wrote:
 [ Did you intentionally not Cc the xorg-devel list? ]

No that was an accident... not sure why I have reply all as default...

 On Fre, 2012-06-29 at 12:20 -0400, Kristian Høgsberg wrote:
 On Thu, Jun 28, 2012 at 7:39 AM, Michel Dänzer mic...@daenzer.net wrote:
  From: Michel Dänzer michel.daen...@amd.com
 
  Otherwise the DRI2Drawable may retain references to the destroyed
  __GLXDRIdrawable, leading to use after free in __glXDRIinvalidateBuffers().

 That looks wrong to me.  DRI2 isn't concerned with GLX drawables, only
 X drawables.  If you're destroying the GLX drawable and want to not
 get invalidate callbacks, you need to destroy the DRI2DrawableRef that
 DRI2CreateDrawable creates.

 Which is what this patch does? :) (By means of
 glxcmds.c:DoDestroyDrawable - FreeResource - DRI2DrawableGone, where
 the ID matches ref-id, so it calls
 FreeResourceByType(ref-dri2_id, ...) as well)

 Can you explain why the non-GLX drawable ID needs to be passed to
 DRI2CreateDrawable?

The DRI2Drawable is created for the X drawable, not the GLX drawable.
When the X drawable goes away the DRI2 drawable needs to go away.  It
works the way it does, since a pixmap can have multiple XIDs and for
each XID, mutliple clients could call DRI2CreateClient.  We need to
keep the DRI2 drawable alive for each reference for each XID.  DRI2
automatically reclaims the DRI2Drawable when the underlying X drawable
is destroyed, but that will break if you pass in the GLX drawable XID.
 What's missing now is a way to destroy the DRI2DrawableRef when you
destroy the GLX drawable.  I'm suggesting we add that by returning the
dri2_id so glx can FreeResource that.

Maybe it could work the way you describe, but that's not how it was
designed to work and I can't immediately say that it won't break some
subtle way.

Kristian



 --
 Earthling Michel Dänzer           |                   http://www.amd.com
 Libre software enthusiast         |          Debian, X and DRI developer
___
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: [PATCH] dri2: Pass a ScreenPtr through to the driver's AuthMagic function. (v3)

[Kristian Høgsberg]

On Tue, Jun 19, 2012 at 11:53 PM, Christopher James Halse Rogers
christopher.halse.rog...@canonical.com wrote:
 xwayland drivers need access to their screen private data to authenticate.
 Now that drivers no longer have direct access to the global screen arrays,
 this needs to be passed in as function context.

 v2: Don't break ABI
 v3: Paint the bikeshed blue; drop fd from AuthMagic2ProcPtr prototype

Looks good,

Reviewed-by: Kristian Høgsberg k...@bitplanet.net

 Signed-off-by: Christopher James Halse Rogers 
 christopher.halse.rog...@canonical.com
 ---
  hw/xfree86/dri2/dri2.c |   35 ---
  hw/xfree86/dri2/dri2.h |    9 -
  2 files changed, 36 insertions(+), 8 deletions(-)

 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index babf32f..d0f1789 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -104,7 +104,8 @@ typedef struct _DRI2Screen {
     DRI2ScheduleSwapProcPtr ScheduleSwap;
     DRI2GetMSCProcPtr GetMSC;
     DRI2ScheduleWaitMSCProcPtr ScheduleWaitMSC;
 -    DRI2AuthMagicProcPtr AuthMagic;
 +    DRI2AuthMagic2ProcPtr AuthMagic;
 +    DRI2AuthMagicProcPtr LegacyAuthMagic;
     DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify;
     DRI2SwapLimitValidateProcPtr SwapLimitValidate;
     DRI2GetParamProcPtr GetParam;
 @@ -1110,12 +,22 @@ DRI2Connect(ScreenPtr pScreen, unsigned int 
 driverType, int *fd,
     return TRUE;
  }

 +static Bool
 +DRI2AuthMagic (ScreenPtr pScreen, uint32_t magic)
 +{
 +    DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
 +    if (ds == NULL || (*ds-LegacyAuthMagic) (ds-fd, magic))
 +        return FALSE;
 +
 +    return TRUE;
 +}
 +
  Bool
  DRI2Authenticate(ScreenPtr pScreen, uint32_t magic)
  {
     DRI2ScreenPtr ds = DRI2GetScreen(pScreen);

 -    if (ds == NULL || (*ds-AuthMagic) (ds-fd, magic))
 +    if (ds == NULL || (*ds-AuthMagic) (pScreen, magic))
         return FALSE;

     return TRUE;
 @@ -1202,8 +1213,11 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
         cur_minor = 1;
     }

 +    if (info-version = 8) {
 +        ds-AuthMagic = info-AuthMagic2;
 +    }
     if (info-version = 5) {
 -        ds-AuthMagic = info-AuthMagic;
 +        ds-LegacyAuthMagic = info-AuthMagic;
     }

     if (info-version = 6) {
 @@ -1218,14 +1232,21 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)

     /*
      * if the driver doesn't provide an AuthMagic function or the info struct
 -     * version is too low, it relies on the old method (using libdrm) or fail
 +     * version is too low, call through LegacyAuthMagic
      */
 -    if (!ds-AuthMagic)
 +    if (!ds-AuthMagic) {
 +        ds-AuthMagic = DRI2AuthMagic;
 +        /*
 +         * If the driver doesn't provide an AuthMagic function
 +         * it relies on the old method (using libdrm) or fails
 +         */
 +        if (!ds-LegacyAuthMagic)
  #ifdef WITH_LIBDRM
 -        ds-AuthMagic = drmAuthMagic;
 +            ds-LegacyAuthMagic = drmAuthMagic;
  #else
 -        goto err_out;
 +            goto err_out;
  #endif
 +    }

     /* Initialize minor if needed and set to minimum provied by DDX */
     if (!dri2_minor || dri2_minor  cur_minor)
 diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
 index f849be6..4fd0fbc 100644
 --- a/hw/xfree86/dri2/dri2.h
 +++ b/hw/xfree86/dri2/dri2.h
 @@ -64,6 +64,7 @@ typedef void (*DRI2CopyRegionProcPtr) (DrawablePtr pDraw,
                                        DRI2BufferPtr pSrcBuffer);
  typedef void (*DRI2WaitProcPtr) (WindowPtr pWin, unsigned int sequence);
  typedef int (*DRI2AuthMagicProcPtr) (int fd, uint32_t magic);
 +typedef int (*DRI2AuthMagic2ProcPtr) (ScreenPtr pScreen, uint32_t magic);

  /**
  * Schedule a buffer swap
 @@ -192,7 +193,7 @@ typedef int (*DRI2GetParamProcPtr) (ClientPtr client,
  /**
  * Version of the DRI2InfoRec structure defined in this header
  */
 -#define DRI2INFOREC_VERSION 7
 +#define DRI2INFOREC_VERSION 8

  typedef struct {
     unsigned int version;       /** Version of this struct */
 @@ -229,6 +230,12 @@ typedef struct {
     /* added in version 7 */

     DRI2GetParamProcPtr GetParam;
 +
 +    /* added in version 8 */
 +    /* AuthMagic callback which passes extra context */
 +    /* If this is NULL the AuthMagic callback is used */
 +    /* If this is non-NULL the AuthMagic callback is ignored */
 +    DRI2AuthMagic2ProcPtr AuthMagic2;
  } DRI2InfoRec, *DRI2InfoPtr;

  extern _X_EXPORT int DRI2EventBase;
 --
 1.7.10.4

 ___
 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
___
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: [PATCH] dri2: Pass a ScreenPtr through to the driver's AuthMagic function.

[Kristian Høgsberg]

On Mon, Jun 18, 2012 at 12:58 PM, Keith Packard kei...@keithp.com wrote:
 Christopher James Halse Rogers christopher.halse.rog...@canonical.com
 writes:

 +typedef int (*DRI2AuthMagic2ProcPtr) (ScreenPtr pScreen, int fd, uint32_t 
 magic);

 Bikeshed -- seems like the 'fd' parameter is not needed in this API?
 I'll note that in the implementation of the wrapper, you pull it from
 the screen private instead of using the provided parameter...

Oh, that is a good point, I like that bikeshed color better.

Kristian
___
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: [PATCH] dri2: Pass a ScreenPtr through to the driver's AuthMagic function.

[Kristian Høgsberg]

On Sun, Jun 17, 2012 at 9:52 PM, Christopher James Halse Rogers
christopher.halse.rog...@canonical.com wrote:
 xwayland drivers need access to their screen private data to authenticate.
 Now that drivers no longer have direct access to the global screen arrays,
 this needs to be passed in as function context.

 v2: Don't break ABI

Yeah, looks good to me now.

Reviewed-by: Kristian Høgsberg k...@bitplanet.net

 Signed-off-by: Christopher James Halse Rogers 
 christopher.halse.rog...@canonical.com
 ---
  hw/xfree86/dri2/dri2.c |   35 ---
  hw/xfree86/dri2/dri2.h |    9 -
  2 files changed, 36 insertions(+), 8 deletions(-)

 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index babf32f..412feb3 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -104,7 +104,8 @@ typedef struct _DRI2Screen {
     DRI2ScheduleSwapProcPtr ScheduleSwap;
     DRI2GetMSCProcPtr GetMSC;
     DRI2ScheduleWaitMSCProcPtr ScheduleWaitMSC;
 -    DRI2AuthMagicProcPtr AuthMagic;
 +    DRI2AuthMagic2ProcPtr AuthMagic;
 +    DRI2AuthMagicProcPtr LegacyAuthMagic;
     DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify;
     DRI2SwapLimitValidateProcPtr SwapLimitValidate;
     DRI2GetParamProcPtr GetParam;
 @@ -1110,12 +,22 @@ DRI2Connect(ScreenPtr pScreen, unsigned int 
 driverType, int *fd,
     return TRUE;
  }

 +static Bool
 +DRI2AuthMagic (ScreenPtr pScreen, int fd, uint32_t magic)
 +{
 +    DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
 +    if (ds == NULL || (*ds-LegacyAuthMagic) (ds-fd, magic))
 +        return FALSE;
 +
 +    return TRUE;
 +}
 +
  Bool
  DRI2Authenticate(ScreenPtr pScreen, uint32_t magic)
  {
     DRI2ScreenPtr ds = DRI2GetScreen(pScreen);

 -    if (ds == NULL || (*ds-AuthMagic) (ds-fd, magic))
 +    if (ds == NULL || (*ds-AuthMagic) (pScreen, ds-fd, magic))
         return FALSE;

     return TRUE;
 @@ -1202,8 +1213,11 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
         cur_minor = 1;
     }

 +    if (info-version = 8) {
 +        ds-AuthMagic = info-AuthMagic2;
 +    }
     if (info-version = 5) {
 -        ds-AuthMagic = info-AuthMagic;
 +        ds-LegacyAuthMagic = info-AuthMagic;
     }

     if (info-version = 6) {
 @@ -1218,14 +1232,21 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)

     /*
      * if the driver doesn't provide an AuthMagic function or the info struct
 -     * version is too low, it relies on the old method (using libdrm) or fail
 +     * version is too low, call through LegacyAuthMagic
      */
 -    if (!ds-AuthMagic)
 +    if (!ds-AuthMagic) {
 +        ds-AuthMagic = DRI2AuthMagic;
 +        /*
 +         * If the driver doesn't provide an AuthMagic function
 +         * it relies on the old method (using libdrm) or fails
 +         */
 +        if (!ds-LegacyAuthMagic)
  #ifdef WITH_LIBDRM
 -        ds-AuthMagic = drmAuthMagic;
 +            ds-LegacyAuthMagic = drmAuthMagic;
  #else
 -        goto err_out;
 +            goto err_out;
  #endif
 +    }

     /* Initialize minor if needed and set to minimum provied by DDX */
     if (!dri2_minor || dri2_minor  cur_minor)
 diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
 index f849be6..004d286 100644
 --- a/hw/xfree86/dri2/dri2.h
 +++ b/hw/xfree86/dri2/dri2.h
 @@ -64,6 +64,7 @@ typedef void (*DRI2CopyRegionProcPtr) (DrawablePtr pDraw,
                                        DRI2BufferPtr pSrcBuffer);
  typedef void (*DRI2WaitProcPtr) (WindowPtr pWin, unsigned int sequence);
  typedef int (*DRI2AuthMagicProcPtr) (int fd, uint32_t magic);
 +typedef int (*DRI2AuthMagic2ProcPtr) (ScreenPtr pScreen, int fd, uint32_t 
 magic);

  /**
  * Schedule a buffer swap
 @@ -192,7 +193,7 @@ typedef int (*DRI2GetParamProcPtr) (ClientPtr client,
  /**
  * Version of the DRI2InfoRec structure defined in this header
  */
 -#define DRI2INFOREC_VERSION 7
 +#define DRI2INFOREC_VERSION 8

  typedef struct {
     unsigned int version;       /** Version of this struct */
 @@ -229,6 +230,12 @@ typedef struct {
     /* added in version 7 */

     DRI2GetParamProcPtr GetParam;
 +
 +    /* added in version 8 */
 +    /* AuthMagic callback which passes extra context */
 +    /* If this is NULL the AuthMagic callback is used */
 +    /* If this is non-NULL the AuthMagic callback is ignored */
 +    DRI2AuthMagic2ProcPtr AuthMagic2;
  } DRI2InfoRec, *DRI2InfoPtr;

  extern _X_EXPORT int DRI2EventBase;
 --
 1.7.10.4

 ___
 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
___
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: [PATCH] dri2: Pass a ScreenPtr through to the driver's AuthMagic

[Kristian Høgsberg]

On Fri, Jun 15, 2012 at 07:01:34PM +1000, Christopher James Halse Rogers wrote:
 xwayland drivers need access to their screen private data to authenticate.
 Now that drivers no longer have direct access to the global screen arrays,
 this needs to be passed in as function context. The way it was working
 was ugly, anyway.

Yes, sure, but it also didn't need to extend the DRI2 API.  Now that
Dave's gone and locked up the screen arrays, we have a good excuse to
go clean that up.

 Signed-off-by: Christopher James Halse Rogers 
 christopher.halse.rog...@canonical.com
 ---
 
 This came up when I got around to fixing the nouveau xwayland patch review
 comments; with things no longer meant to access the global xf86Screens array
 the authmagic callback needed some other way to access driver private data.
 
 Nouveau patch using this follows; I'll send it upstream once this gets
 applied somewhere.
 
 CCd to xorg-devel mainly for sanity review, but could be applied; as Airlied
 has pointed out, there's no particular reason for the xwayland branch to
 diverge on core infrastructure.

Yeah, this is good to go upstream.  Just add a new function pointer to
the struct and a new typedef though, no need to play games with
casting the pointers:

  typedef int (*DRI2AuthMagicWithScreenProcPtr) (ScreenPtr pScreen, int fd, 
uint32_t magic);

or something.  Aside from that and the small nit-pick below, it looks
good.

Kristian

 
  hw/xfree86/dri2/dri2.c |   40 ++--
  hw/xfree86/dri2/dri2.h |2 +-
  2 files changed, 35 insertions(+), 7 deletions(-)
 
 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index babf32f..0e79875 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -89,6 +89,8 @@ typedef struct _DRI2Drawable {
  Bool needInvalidate;
  } DRI2DrawableRec, *DRI2DrawablePtr;
  
 +typedef int (*DRI2LegacyAuthMagicProcPtr) (int fd, uint32_t magic);
 +
  typedef struct _DRI2Screen {
  ScreenPtr screen;
  int refcnt;
 @@ -105,6 +107,7 @@ typedef struct _DRI2Screen {
  DRI2GetMSCProcPtr GetMSC;
  DRI2ScheduleWaitMSCProcPtr ScheduleWaitMSC;
  DRI2AuthMagicProcPtr AuthMagic;
 +DRI2LegacyAuthMagicProcPtr LegacyAuthMagic;
  DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify;
  DRI2SwapLimitValidateProcPtr SwapLimitValidate;
  DRI2GetParamProcPtr GetParam;
 @@ -1110,12 +1113,22 @@ DRI2Connect(ScreenPtr pScreen, unsigned int 
 driverType, int *fd,
  return TRUE;
  }
  
 +static Bool
 +DRI2AuthMagic (ScreenPtr pScreen, int fd, uint32_t magic)
 +{
 +DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
 +if (ds == NULL || (*ds-LegacyAuthMagic) (ds-fd, magic))
 +return FALSE;

Don't need to check ds == NULL here, we're only called when ds is non-NULL.

 +return TRUE;
 +}
 +
  Bool
  DRI2Authenticate(ScreenPtr pScreen, uint32_t magic)
  {
  DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
  
 -if (ds == NULL || (*ds-AuthMagic) (ds-fd, magic))
 +if (ds == NULL || (*ds-AuthMagic) (pScreen, ds-fd, magic))
  return FALSE;
  
  return TRUE;
 @@ -1202,9 +1215,17 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
  cur_minor = 1;
  }
  
 -if (info-version = 5) {
 +if (info-version = 7) {
  ds-AuthMagic = info-AuthMagic;
  }
 +else if (info-version = 5) {
 +/*
 + * This cast is safe; if the driver has provided a V5 or V6 InfoRec
 + * then AuthMagic is of type DRI2LegacyAuthMagicProcPtr, and the C
 + * standard guarantees that we can typecast it back and call it.
 + */
 +ds-LegacyAuthMagic = (DRI2LegacyAuthMagicProcPtr)info-AuthMagic;
 +}
  
  if (info-version = 6) {
  ds-ReuseBufferNotify = info-ReuseBufferNotify;
 @@ -1218,14 +1239,21 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
  
  /*
   * if the driver doesn't provide an AuthMagic function or the info struct
 - * version is too low, it relies on the old method (using libdrm) or fail
 + * version is too low, call through LegacyAuthMagic
   */
 -if (!ds-AuthMagic)
 +if (!ds-AuthMagic) {
 +ds-AuthMagic = DRI2AuthMagic;
 +/*
 + * If the driver doesn't provide an AuthMagic function
 + * it relies on the old method (using libdrm) or fails
 + */
 +if (!ds-LegacyAuthMagic)
  #ifdef WITH_LIBDRM
 -ds-AuthMagic = drmAuthMagic;
 +ds-LegacyAuthMagic = drmAuthMagic;
  #else
 -goto err_out;
 +goto err_out;
  #endif
 +}
  
  /* Initialize minor if needed and set to minimum provied by DDX */
  if (!dri2_minor || dri2_minor  cur_minor)
 diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
 index f849be6..90886a9 100644
 --- a/hw/xfree86/dri2/dri2.h
 +++ b/hw/xfree86/dri2/dri2.h
 @@ -63,7 +63,7 @@ typedef void (*DRI2CopyRegionProcPtr) (DrawablePtr pDraw,
 DRI2BufferPtr pDestBuffer,
 

Re: [PATCH] dri2: Pass a ScreenPtr through to the driver's AuthMagic

[Kristian Høgsberg]

On Fri, Jun 15, 2012 at 12:35:00PM +0200, Michel Dänzer wrote:
 On Fre, 2012-06-15 at 19:01 +1000, Christopher James Halse Rogers
 wrote: 
  xwayland drivers need access to their screen private data to authenticate.
  Now that drivers no longer have direct access to the global screen arrays,
  this needs to be passed in as function context. The way it was working
  was ugly, anyway.
  
  Signed-off-by: Christopher James Halse Rogers 
  christopher.halse.rog...@canonical.com
  ---
  
  This came up when I got around to fixing the nouveau xwayland patch review
  comments; with things no longer meant to access the global xf86Screens array
  the authmagic callback needed some other way to access driver private data.
  
  Nouveau patch using this follows; I'll send it upstream once this gets
  applied somewhere.
  
  CCd to xorg-devel mainly for sanity review, but could be applied; as Airlied
  has pointed out, there's no particular reason for the xwayland branch to
  diverge on core infrastructure.
 
 [...]
 
  @@ -1202,9 +1215,17 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
   cur_minor = 1;
   }
   
  -if (info-version = 5) {
  +if (info-version = 7) {
   ds-AuthMagic = info-AuthMagic;
   }
  +else if (info-version = 5) {
  +/*
  + * This cast is safe; if the driver has provided a V5 or V6 InfoRec
  + * then AuthMagic is of type DRI2LegacyAuthMagicProcPtr, and the C
  + * standard guarantees that we can typecast it back and call it.
  + */
  +ds-LegacyAuthMagic = (DRI2LegacyAuthMagicProcPtr)info-AuthMagic;
  +}
 
 This is nifty, but it's an ABI break, as servers without this change
 will treat the version 7 AuthMagic field incorrectly. Wouldn't it be
 easier to add a second AuthMagic field at the end of version 7?

Drivers can use DRI2Version to see if the xserver is recent enough to
understand this trick, but I agree just adding a field to the structs
is simpler.

Kristian
___
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: [PATCH 02/02] dri2: Add DRI2GetParam request

[Kristian Høgsberg]

On Thu, May 10, 2012 at 3:04 AM, Chad Versace
chad.vers...@linux.intel.com wrote:
 Bump dri2proto dependency to 2.7.
 Bump DRI2INFOREC_VERSION to 7.

 This new protocol request effectively allows clients to perform feature
 detection on the DDX. The request was added in DRI2 protocol 1.4.

 If I had DRI2GetParam in June 2011, when I was implementing support in the
 Intel DDX and Mesa for new hardware that required a new DRI2 attachment
 format, then I could have avoided a week of pain caused by the necessity
 of writing a horrid feature detection hack [1] in Mesa. In the future, when
 the work begins to add MSAA support to the Intel DDX, having a clean way
 to do feature detection will allow us to avoid revisiting and expanding
 that hack.

Yeah, that looks fine, should make things simpler than trying to infer
from version numbers.  If we expect to add many parameters or query
them often (per frame), we should think about how to query a batch of
parameters at a time to avoid multiple roundtrips.  EGL uses xcb, so
it's not a big deal there, and for GLX, the protocol code is in mesa,
so we could just pipeline the request/reply code by hand if we have
to.

Also, if I was painting this bikeshed I'd just drop the namespace arg,
and have a CARD32 be the param and put driver params above 0x1000 or
whatever.  CARD64 for the value makes sense, at least some of the
crazy swap parameters are 64 bit.

Kristian

 [1] mesa, commit aea2236a, function intel_verify_dri2_has_hiz

 CC: Keith Packard kei...@keithp.com
 CC: Kristian Høgsberg k...@bitplanet.net
 CC: Ian Romanick i...@freedesktop.org
 CC: Eric Anholt e...@anholt.net
 CC: Ville Syrjälä syrj...@sci.fi
 CC: Michel Dänzer daen...@vmware.com
 CC: Jesse Barnes jbar...@virtuousgeek.org
 CC: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Chad Versace chad.vers...@linux.intel.com
 ---
  configure.ac              |    2 +-
  hw/xfree86/dri2/dri2.c    |   30 ++
  hw/xfree86/dri2/dri2.h    |   34 +-
  hw/xfree86/dri2/dri2ext.c |   36 
  4 files changed, 100 insertions(+), 2 deletions(-)

 diff --git a/configure.ac b/configure.ac
 index 6a41ea8..a2e22f7 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -773,7 +773,7 @@ RECORDPROTO=recordproto = 1.13.99.1
  SCRNSAVERPROTO=scrnsaverproto = 1.1
  RESOURCEPROTO=resourceproto = 1.2.0
  DRIPROTO=xf86driproto = 2.1.0
 -DRI2PROTO=dri2proto = 2.6
 +DRI2PROTO=dri2proto = 2.7
  XINERAMAPROTO=xineramaproto
  BIGFONTPROTO=xf86bigfontproto = 1.2.0
  DGAPROTO=xf86dgaproto = 2.0.99.1
 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index 591ff3a..d40cea2 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -107,6 +107,7 @@ typedef struct _DRI2Screen {
     DRI2AuthMagicProcPtr AuthMagic;
     DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify;
     DRI2SwapLimitValidateProcPtr SwapLimitValidate;
 +    DRI2GetParamProcPtr GetParam;

     HandleExposuresProcPtr HandleExposures;

 @@ -1210,6 +1211,11 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
         ds-SwapLimitValidate = info-SwapLimitValidate;
     }

 +    if (info-version = 7) {
 +        ds-GetParam = info-GetParam;
 +        cur_minor = 4;
 +    }
 +
     /*
      * if the driver doesn't provide an AuthMagic function or the info struct
      * version is too low, it relies on the old method (using libdrm) or fail
 @@ -1332,3 +1338,27 @@ DRI2Version(int *major, int *minor)
     if (minor != NULL)
         *minor = DRI2VersRec.minorversion;
  }
 +
 +int
 +DRI2GetParam(ClientPtr client,
 +             DrawablePtr drawable,
 +             CARD32 namespace_,
 +             CARD64 param,
 +             BOOL *is_param_recognized,
 +             CARD64 *value)
 +{
 +    DRI2ScreenPtr ds = DRI2GetScreen(drawable-pScreen);
 +
 +    switch (namespace_) {
 +    case DRI2ParamNamespaceServer:
 +        /* The server currently recognizes no parameters. */
 +        *is_param_recognized = FALSE;
 +        return Success;
 +    case DRI2ParamNamespaceDriver:
 +        return ds-GetParam(client, drawable, namespace_, param,
 +                            is_param_recognized, value);
 +    default:
 +        client-errorValue = namespace_;
 +        return BadValue;
 +    }
 +}
 diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
 index 00b3668..49489c7 100644
 --- a/hw/xfree86/dri2/dri2.h
 +++ b/hw/xfree86/dri2/dri2.h
 @@ -176,9 +176,30 @@ typedef Bool (*DRI2SwapLimitValidateProcPtr) 
 (DrawablePtr pDraw,
                                               int swap_limit);

  /**
 + * \brief Get the value of a parameter.
 + *
 + * The parameter's \a value is looked up in the \a namespace_ on the screen
 + * associated with \a pDrawable.
 + *
 + * Parameter names in the server  namespace are invariant with
 + * respect to the loaded driver. Parameter names in the driver
 + * namespace are specific to the loaded driver.
 + *
 + * \param namespace_ is one

Re: [PULL libxkbcommon] Some more fixes and minor enhancements

[Kristian Høgsberg]

Hi Ran,

Let me just point you to this branch as well:

  http://cgit.freedesktop.org/~krh/libxkbcommon/log/?h=keysyms

I've been talking with Daniel about this in IRC, but I thought you
might want to take a look too.  With those patches the API is
completely self-contained.  We still need xproto, kbproto and
Xfuncproto at compile time, but none of that leaks through the
exported API.

Krisitian

On Wed, May 9, 2012 at 11:13 AM, Ran Benita ran...@gmail.com wrote:
 On Tue, May 08, 2012 at 05:35:23PM +0100, Daniel Stone wrote:
 Hi Ran,
 Sorry for the delay, have been sidetracked by core Wayland stuff for a bit.

 Hi!

 Thanks a bunch for all your last changes too, I've merged everything
 except the DFLT_XKB_CONFIG_ROOT change and the Unicode tests.  Again
 for the Unicode tests I want to use UTF-8 rather than UCS-4 and will
 get to this really quite soon now I promise (finally have some time to
 poke back at xkbcommon).

 Yes, but I think having UCS-4 makes sense as well, by which I mean that
 are applications (mine..) which have good reasons to use it. Since the
 existing implemantations (the libX11 one and the Markus Kuhn one)
 convert to uint32_t anyway, why not expose it in addition to UTF-8? So
 when you implement it please have a UCS-4 one :)

 As for the DFLT_XKB_CONFIG_ROOT change, it's ugly as sin but
 unfortunately it's the only way for it to work properly, since just
 substituting from within configure doesn't always perform enough
 expansion.  On some versions you'll be left with the string being
 ${datarootdir}/share/X11/xkb or similar.  The 'fix' there is to just
 do more rounds of expansion, but the upstream-recommended method is
 actually to do it as we're doing it now, believe it or not.

 autotools makes me sad sometimes.

 Didn't know that; good that you caught it.

 As for the rest -

 On 11 April 2012 19:58, Ran Benita ran...@gmail.com wrote:
  On Mon, Apr 02, 2012 at 12:04:25PM +0100, Daniel Stone wrote:
  Right, so allow it to call through arbitrary function pointers? I was
  considering doing that, but wasn't particularly feeling like dealing
  with the intracacies of varargs again that day.
 
  Varags aren't so bad. I had some fun abusing them here:
  https://github.com/bluetech/libxkbcommon/blob/tests2/test/keyseq.c
  That said, you can look at the libabc example code, if you think that's
  a better approach.

 Yeah, the tests look really good and we should definitely be doing
 that, I'm going to merge in a really minimal dataset too so we're not
 relying on an installed dataset for all our tests.  At that point we
 can go all-out and throw in a lot more tests for esoteric stuff like
 locking modifiers.

 I don't know if I'll have too much time to poke at the log stuff
 anytime soon, but it definitely sounds good.

  I'd like to be able to support full dumps (e.g. generated from xkbcomp
  -xkb :0 foo.txt) being loaded in to xkbcommon without triggering
  parsing errors because it doesn't have natural support for floats.
  That was the best way I found to do that; is there something I'm
  missing?
 
  Hmm, if I get it right, floats etc, can only appear inside xkb_geometry
  sections (which we treat as files). So the scanner should be able to lex
  them, but the parser can just skip the section and not have to know
  anything inside it. I'm not sure yacc does that off hand, I'll try it.
  Of course, it may be worth keeping for completeness or whatever.

 Hmm.  If it works then it'd definitely be a net win. :)

 Of course I didn't take a compiler class yet and so I hadn't realized
 yacc does bottom-up parsing, making this impossible... (at least in a
 straightforward way)

  Another question than :) We currently support three types of aggregate
  files, which are xkb_keymap, xkb_layout and xkb_semantics. The keymap
  can include all components (i.e. keycodes, symbols...), and the other
  two only one or two (mandatory / optional - see XKBcommonint.h
  #defines). Looking in /usr/share/X11/xkb, the layout type is completely
  unused, and there are a couple semantics files (which seem unused). So,
  is it worth keeping those? I don't think they serve any useful purpose?
  (unless someone tries to load them, but I don't think anyone has them
  lying around).

 No, we can jettison the other two.  In theory we need them for
 compatibility, but in reality they don't exist and I don't care.

 I have done this now. I've kept backward compatibility by just treating
 them as xkb_keymap in the parser. See the branch below.

  And lastly (I promise :), I'm thinking about a situation where there are
  several users of the library (say xserver, wayland/weston, kmscon,
  toolkits) and they all load a keymap from an rmlvo. Currently it seems
  like the end user would have to configure her preferred rmlvo separately
  for each one, which can get annoying (already..). Since you mentioned
  loading files from ~/.xkb/, would it make sense to support some simple
  config file for that? That way if the end 

Re: [PATCH] dri2proto: Fix documented opcodes

[Kristian Høgsberg]

On Wed, May 2, 2012 at 3:03 PM, Chad Versace
chad.vers...@linux.intel.com wrote:
 Fix the documented opcodes in dri2proto.txt to be consistent with the
 actual opcode values in dri2proto.h and in xcb/proto:src/dri2.xml. (It
 looks like the opcodes were incorrect due to copy-paste errors).

Looks correct to me.

Kristian

 CC: Kristian Høgsberg k...@bitplanet.net
 Signed-off-by: Chad Versace chad.vers...@linux.intel.com
 ---
  dri2proto.txt |   18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)

 diff --git a/dri2proto.txt b/dri2proto.txt
 index df763c7..7bde067 100644
 --- a/dri2proto.txt
 +++ b/dri2proto.txt
 @@ -658,7 +658,7 @@ A.2 Protocol Requests
  ┌───
     DRI2GetBuffers
        1       CARD8                   major opcode
 -       1       3                       DRI2 opcode
 +       1       5                       DRI2 opcode
        2       3                       length
        4       DRAWABLE                drawable
        4       n                       number of attachments
 @@ -678,7 +678,7 @@ A.2 Protocol Requests
  ┌───
     DRI2CopyRegion
        1       CARD8                   major opcode
 -       1       4                       DRI2 opcode
 +       1       6                       DRI2 opcode
        2       3                       length
        4       DRAWABLE                drawable
        4       REGION                  region
 @@ -695,7 +695,7 @@ A.2 Protocol Requests
  ┌───
     DRI2GetBuffersWithFormat
        1       CARD8                   major opcode
 -       1       3                       DRI2 opcode
 +       1       7                       DRI2 opcode
        2       3                       length
        4       DRAWABLE                drawable
        4       n                       number of attachments
 @@ -715,7 +715,7 @@ A.2 Protocol Requests
  ┌───
     DRI2SwapBuffers
        1       CARD8                   major opcode
 -       1       7                       DRI2 opcode
 +       1       8                       DRI2 opcode
        2       8                       length
        4       DRAWABLE                drawable
       ▶
 @@ -736,7 +736,7 @@ A.2 Protocol Requests
  ┌───
     DRI2SwapBuffers
        1       CARD8                   major opcode
 -       1       7                       DRI2 opcode
 +       1       8                       DRI2 opcode
        2       8                       length
        4       DRAWABLE                drawable
        4       CARD32                  target_msc_hi
 @@ -758,7 +758,7 @@ A.2 Protocol Requests
  ┌───
     DRI2GetMSC
        1       CARD8                   major opcode
 -       1       7                       DRI2 opcode
 +       1       9                       DRI2 opcode
        2       8                       length
        4       DRAWABLE                drawable
       ▶
 @@ -777,7 +777,7 @@ A.2 Protocol Requests
  ┌───
     DRI2WaitMSC
        1       CARD8                   major opcode
 -       1       7                       DRI2 opcode
 +       1       10                      DRI2 opcode
        2       8                       length
        4       DRAWABLE                drawable
        4       CARD32                  target_msc_hi
 @@ -802,7 +802,7 @@ A.2 Protocol Requests
  ┌───
     DRI2WaitSBC
        1       CARD8                   major opcode
 -       1       7                       DRI2 opcode
 +       1       11                      DRI2 opcode
        2       8                       length
        4       DRAWABLE                drawable
        4       CARD32                  swap_hi
 @@ -823,7 +823,7 @@ A.2 Protocol Requests
  ┌───
     DRI2SwapInterval
        1       CARD8                   major opcode
 -       1       7                       DRI2 opcode
 +       1       12                      DRI2 opcode
        2       8                       length
        4       DRAWABLE                drawable
        4       CARD32                  interval
 --
 1.7.10

___
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: [Pull Request] Glamor: A 2D rendering acceleration implementation based on OpenGL

[Kristian Høgsberg]

2011/9/28 zhigang gong zhigang.g...@gmail.com:
 2011/9/28 Michel Dänzer mic...@daenzer.net:
 On Die, 2011-09-27 at 21:50 +0800, zhigang gong wrote:

 3. Only support Intel platform currently.
 As glamor depends on KMS + MESA/EGL + GBM, currently only
 intel gfx device get supported.

 A Gallium based EGL driver (e.g. r3/600g, nouveau or llvmpipe) can
 support all of the above, any particular reason why it wouldn't work
 with that?

 You are talking about gallium driver, right? As I met some problem with
 intel's gallium driver, it just simply failed at egl initialization stage. And
 I was told that don't use gallium driver at that time. So you can see at
 my first email, I said please disable gallium when building mesa. That's
 my working environment.

Yes, the intel gallium drivers aren't well supported, but the other
gallium drivers support gbm and EGL/GLES2 just fine.  We use that
combination for Wayland.  If you're just using gbm, kms and EGL/GLES2,
it doesn't really matter whether your driver uses gallium or not, it's
really just an implementation detail.  Similar to how it doesn't
matter to X clients whether the X server uses the EXA, UXA or XAA
acceleration architecture.

Kristian
___
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: [Mesa-dev] threads and X.org/AIGLX drivers

[Kristian Høgsberg]

On Wed, May 11, 2011 at 2:53 AM, Dave Airlie airl...@gmail.com wrote:
 Hey,

 So we got a bug reported against F15 where we were getting an illegal
 input event type 0, after passing it around the RH X team I eventually
 came to look at it.

 The problem appears to be that we are using llvmpipe as our swrast
 renderer and on systems that fallback to that we end up with threads
 inside the X server, that we didn't spawn. It appears SIGIO gets
 delivered to one of these threads while the main thread keeps on
 trucking and fail ensues.

 So I'm crossposting this as there are two fixes we can do that aren't
 exclusive but could act as belt and braces on Linux at least (other
 OSes get the belt only).

 Fix 1: use F_SETOWN_EX, thank to DrNick and AaronP on irc for digging
 up this nugget, From  Linux  2.6.32 onwards, use F_SETOWN_EX to
 target SIGIO
                and SIGURG signals at a particular thread.. So on
 Linux we would use that instead of F_SETOWN to make sure SIGIO only
 goes to the main X process/thread. Though I'm not sure how to get the
 defintion for gettid which we need, though I'll look into this a bit
 more.

 Fix 2: block all signals in the gallium thread spawning code, this
 involves using pthread_sigmask around the pthread_create function.
 Calling it once with a full set of signals to block, and get a copy of
 the current signals, then calling it again to put back the current
 signals. I think this might be a good plan for some other things, as I
 can't see a good reason for a DRI driver to ever get SIGIO, esp as it
 may interfere with the parent app.

 I've built a test package with 2 in it for Fedora and am awaiting
 feedback from the tester, but just though I'd raise the subject here
 for some discussion/feedback.

A different solution could be to use the input thread idea and stop
taking SIGIO.

Kristian
___
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: [Mesa-dev] auto generated glx code in X server

[Kristian Høgsberg]

On Tue, Mar 15, 2011 at 12:12 AM, Keith Packard kei...@keithp.com wrote:
 On Tue, 15 Mar 2011 13:52:40 +1000, Dave Airlie airl...@gmail.com wrote:

 a) undo ajax's cleanup, fix generator scripts to work again (not sure
 how possible that is
 since Olv's mapi changes), import latest GLX into server on a
 semi-regular basis.

 The X server generator seems like it'll bit-rot if not used for every
 build...


 b) move all the xml files into glproto or somewhere like that, have
 Mesa generator scripts
 do mesa code and X.org generator script generate X.org code, set
 glapi.c/h glthread.c/h
 on their own paths for ever more in each project.

 I'd be up for plan b if it seems workable.

Yeah, me too, sounds a lot more robust and will allow simpler and
faster dispatching for AIGLX (fwiw).

Kristian
___
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

[PATCH] Replace malloc with calloc to initialize the buffers[] as NULL in do_get_buffers function

[Kristian Høgsberg]

From: Justin Dou justin@intel.com

The calling for allocate_or_reuse_buffer may fail due to some reason, e.g. out 
of memory.
If the buffers[] were not initialized to be NULL, the following err_out may try 
to access an illegal memory, which will cause X crash afterward.

Reviewed-by: Kristian Høgsberg k...@bitplanet.net
Signed-off-by: Justin Dou justin@intel.com
---
 hw/xfree86/dri2/dri2.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 34f735f..5d31e77 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -403,7 +403,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
 (pDraw-height == pPriv-height)
 (pPriv-serialNumber == DRI2DrawableSerial(pDraw));
 
-buffers = malloc((count + 1) * sizeof(buffers[0]));
+buffers = calloc((count + 1), sizeof(buffers[0]));
 
 for (i = 0; i  count; i++) {
const unsigned attachment = *(attachments++);
-- 
1.7.2.2

___
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: [PATCH 0/5] glx input sanitizing

[Kristian Høgsberg]

On Mon, Jan 10, 2011 at 6:41 AM, Julien Cristau jcris...@debian.org wrote:
 On Mon, Jan  3, 2011 at 21:08:05 +0100, Julien Cristau wrote:

 Most of this series has been sitting in bug#28823 for a while.  It adds
 some missing checks for client-provided data in the glx code, and fixes
 a bug in the swapped-client path (patch 4/5).

 Anyone?

They all look good to me, please add RB: krh to the series.

Kristian
___
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

[PATCH] glx: Refcnt the GLXDrawable to avoid use after free with multiple FreeResource

[Kristian Høgsberg]

From: Chris Wilson ch...@chris-wilson.co.uk

Although there may be more than one resource handles pointing to the
Drawable, we only want to destroy it once and only reference the
resource which may have just been deleted on the first instance.

v2: Apply fixes and combine with another bug fix from Michel Dänzer,
https://bugs.freedesktop.org/show_bug.cgi?id=28181
v3: Just use the refcnt and don't try to free other resources in the
DrawableGone callback.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net

Can we just do this instead?  Didn't test the patch, but this should do
the same and actually simplifies the code.

Kristian

---
 glx/glxcmds.c |   13 +
 glx/glxdrawable.h |2 ++
 glx/glxext.c  |   13 +++--
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 8d13c15..1e8044b 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -530,6 +530,7 @@ __glXGetDrawable(__GLXcontext *glxc, GLXDrawable drawId, 
ClientPtr client,
*error = BadAlloc;
return NULL;
 }
+pGlxDraw-refcnt++;
 
 return pGlxDraw;
 }
@@ -1102,6 +1103,7 @@ __glXDrawableInit(__GLXdrawable *drawable,
 drawable-drawId = drawId;
 drawable-config = config;
 drawable-eventMask = 0;
+drawable-refcnt = 0;
 
 return GL_TRUE;
 }
@@ -1131,14 +1133,17 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen 
*pGlxScreen,
pGlxDraw-destroy (pGlxDraw);
return BadAlloc;
 }
+pGlxDraw-refcnt++;
 
 /* Add the glx drawable under the XID of the underlying X drawable
  * too.  That way we'll get a callback in DrawableGone and can
  * clean up properly when the drawable is destroyed. */
-if (drawableId != glxDrawableId 
-   !AddResource(pDraw-id, __glXDrawableRes, pGlxDraw)) {
-   pGlxDraw-destroy (pGlxDraw);
-   return BadAlloc;
+if (drawableId != glxDrawableId) {
+   if (!AddResource(pDraw-id, __glXDrawableRes, pGlxDraw)) {
+   pGlxDraw-destroy (pGlxDraw);
+   return BadAlloc;
+   }
+   pGlxDraw-refcnt++;
 }
 
 return Success;
diff --git a/glx/glxdrawable.h b/glx/glxdrawable.h
index 2a365c5..e3d4bb5 100644
--- a/glx/glxdrawable.h
+++ b/glx/glxdrawable.h
@@ -51,6 +51,8 @@ struct __GLXdrawable {
 void  (*waitX)(__GLXdrawable *);
 void  (*waitGL)(__GLXdrawable *);
 
+int refcnt; /* number of resources handles referencing this */
+
 DrawablePtr pDraw;
 XID drawId;
 
diff --git a/glx/glxext.c b/glx/glxext.c
index f5ebe4f..499567a 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -126,16 +126,9 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
 {
 __GLXcontext *c, *next;
 
-/* If this drawable was created using glx 1.3 drawable
- * constructors, we added it as a glx drawable resource under both
- * its glx drawable ID and it X drawable ID.  Remove the other
- * resource now so we don't a callback for freed memory. */
-if (glxPriv-drawId != glxPriv-pDraw-id) {
-   if (xid == glxPriv-drawId)
-   FreeResourceByType(glxPriv-pDraw-id, __glXDrawableRes, TRUE);
-   else
-   FreeResourceByType(glxPriv-drawId, __glXDrawableRes, TRUE);
-}
+glxPriv-refcnt--;
+if (glxPriv-refcnt  0)
+   return True;
 
 for (c = glxAllContexts; c; c = next) {
next = c-next;
-- 
1.7.3.2

___
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: where is KMS API docs?

[Kristian Høgsberg]

On Tue, Dec 14, 2010 at 9:59 AM, Jeremy C. Reed r...@reedmedia.net wrote:
 Where is the documentation for the KMS API?

 Any man pages?

 If not written, please point me to any specifications or source used for
 learning the API.

There is no documentation, you can browse the API here:

  http://cgit.freedesktop.org/mesa/drm/tree/xf86drmMode.h

and there's a simple example of how to use it here:

  http://cgit.freedesktop.org/mesa/drm/tree/tests/modetest/modetest.c

Kristian
___
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: Fence Sync patches

[Kristian Høgsberg]

On Tue, Dec 7, 2010 at 7:54 PM, James Jones jajo...@nvidia.com wrote:
 On Sunday 05 December 2010 20:31:24 Owen Taylor wrote:
...
 But I can't say that I'm at all happy the idea that we'll have two sets of
 drivers, one where flushing rendering enables an implicit fence for
 subsequent rendering from that buffer, and one where it doesn't. If the
 implicit fence approach is actually less efficient... work being
 pointlessly done, then I say we should just kill it everywhere and move to
 a consistently looser model.

 Agreed.  I can't force other drivers to change, but I don't think buffer
 accesses should be implicitly fenced by drivers.  Two coordinated threads, for
 instance, should be able to simultaneously render to two regions of the same
 drawable/buffer.  OpenGL, and I believe X, allow for this.

But do you think the implicit fencing in inherently less efficient
than explicit fencing?  I think the flip side to Owens suggestion is
that if explicit fencing doesn't allow a performance improvement, then
we can't justify pushing this bookkeeping to the clients and making
correct damage/GL interaction require it.

I have a personal preference on explicit vs implicit fencing as well,
and I'm certainly willing to put that aside if there's even a
theoretic benefit to explicit fencing.  And just to be clear, I don't
see implicit fencing affecting write/write contention as in your
example above, only the case where you're reading from a buffer that's
currently being written to in a different hw context.

Is there a way explicit fencing makes this use case faster or does it
just add extra latency?

Kristian
___
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: [PATCH libxkbcommon 0/6] Review libxkbcommon configuration Part 2

[Kristian Høgsberg]

On Tue, Dec 7, 2010 at 4:52 PM, Daniel Stone dan...@fooishbar.org wrote:
 On Tue, Dec 07, 2010 at 04:41:21PM -0500, Gaetan Nadon wrote:
 The README file could use a paragraph to describe the role of this lib
 in a follow-on patch.

 The lex and yacc test comes from the few apps that have it.

 Acked-by: Daniel Stone dan...@fooishbar.org

 Please feel free to push these patches to the repo as well.

Yup, it all looks fine.  For the series:

Reviewed-by: Kristian Høgsberg k...@bitplanet.net
___
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 XI 2.1 - inputproto] Various fixes in response to Peter Hutterer's review

[Kristian Høgsberg]

On Thu, Dec 2, 2010 at 10:46 AM, Daniel Stone dan...@fooishbar.org wrote:
...
 OK, so would something like this suit you:
  * at selection time, clients can choose whether or not to receive
    not-for-you events
  * at any point during a touch stream, the current owner of a touch can
    tell the server that it should not send any more events for that
    touch stream to non-owning clients
  * at any point during a touch stream, non-owning clients can opt out
    of receiving further events from that stream

Are these not-for-you-events touches outside any of the clients
windows?  Are we allowing to let the client select for all touch
events, anywhere they occur?  That sounds like a bad idea from a
security / client isolation point of view.  If on the other hand,
that's not the case and clients only receive events that land inside
one of their windows and only when they select for it, I really don't
think we need the roundtrips to stop event delivery.We're not
waking up a ton of clients, we're waking up the one client that's
actually being touched.

Either way, the requests to stop delivery to yourself or other clients
don't make sense to me.  If we're concerned about waking sleeping
clients unecessary, we need to not wake them at all.  Waking up a
bunch of clients so they can tell us that they don't want to be woken
up is broken.  Especially if the premise is that they need to inspect
the event stream a bit to decided whether or not they need the events.
 In that case, the clients wake up do some amount of work, and then
opt of the stream eventually, but the benefit is negligible, since
most gestures are pretty short lived anyway, and the big cost is
waking the client in the first place.

  * if a client which has not selected for not-for-you events becomes
    the owner of a touch stream, an event is sent to that client with
    the entire current state of the touch (i.e. dev-touch-last*)

How would that happen?

Kristian
___
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: [PATCH] glx: Remove swap barrier and hyperpipe support

[Kristian Høgsberg]

On Fri, Nov 19, 2010 at 1:52 PM, Adam Jackson a...@redhat.com wrote:
 Never implemented in any open source driver.  The implementation
 assumed explicit DDX driver knowledge of how the client-side driver
 worked, since at the time the server's GL renderer was not a DRI driver.
 But now, it is, so any implementation of these should be done with
 additional DRI driver API, like the swap control extension.

Yeah, kill it.

Signed-off-by: Kristian Høgsberg k...@bitplanet.net

 Signed-off-by: Adam Jackson a...@redhat.com
 ---
  glx/Makefile.am          |    1 -
  glx/g_disptab.h          |   52 --
  glx/glxcmds.c            |  233 
 --
  glx/glxcmdsswap.c        |    1 -
  glx/glxdri.c             |    1 -
  glx/glxdri2.c            |    1 -
  glx/glxdriswrast.c       |    1 -
  glx/glxext.c             |   19 +
  glx/glxscreens.c         |   27 --
  glx/glxscreens.h         |   18 
  glx/indirect_table.c     |    1 -
  glx/xfont.c              |    1 -
  hw/xquartz/GL/indirect.c |    2 -
  hw/xwin/glx/indirect.c   |    2 -
  14 files changed, 1 insertions(+), 359 deletions(-)
  delete mode 100644 glx/g_disptab.h

 diff --git a/glx/Makefile.am b/glx/Makefile.am
 index 9d9fa3c..d708872 100644
 --- a/glx/Makefile.am
 +++ b/glx/Makefile.am
 @@ -68,7 +68,6 @@ libglx_la_SOURCES = \
        indirect_program.c \
        indirect_table.h \
        indirect_texture_compression.c \
 -        g_disptab.h \
         glxbyteorder.h \
         glxcmds.c \
         glxcmdsswap.c \
 diff --git a/glx/g_disptab.h b/glx/g_disptab.h
 deleted file mode 100644
 index 9b4308b..000
 --- a/glx/g_disptab.h
 +++ /dev/null
 @@ -1,52 +0,0 @@
 -/* DO NOT EDIT - THIS FILE IS AUTOMATICALLY GENERATED */
 -#ifdef HAVE_DIX_CONFIG_H
 -#include dix-config.h
 -#endif
 -
 -#ifndef _GLX_g_disptab_h_
 -#define _GLX_g_disptab_h_
 -/*
 - * SGI FREE SOFTWARE LICENSE B (Version 2.0, Sept. 18, 2008)
 - * Copyright (C) 1991-2000 Silicon Graphics, Inc. All Rights Reserved.
 - *
 - * Permission is hereby granted, free of charge, to any person obtaining a
 - * copy of this software and associated documentation files (the Software),
 - * to deal in the Software without restriction, including without limitation
 - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 - * and/or sell copies of the Software, and to permit persons to whom the
 - * Software is furnished to do so, subject to the following conditions:
 - *
 - * The above copyright notice including the dates of first publication and
 - * either this permission notice or a reference to
 - * http://oss.sgi.com/projects/FreeB/
 - * shall be included in all copies or substantial portions of the Software.
 - *
 - * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS
 - * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
 MERCHANTABILITY,
 - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 - * SILICON GRAPHICS, INC. BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER 
 LIABILITY,
 - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF
 - * OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 - * SOFTWARE.
 - *
 - * Except as contained in this notice, the name of Silicon Graphics, Inc.
 - * shall not be used in advertising or otherwise to promote the sale, use or
 - * other dealings in this Software without prior written authorization from
 - * Silicon Graphics, Inc.
 - */
 -
 -extern int __glXDisp_BindSwapBarrierSGIX(__GLXclientState *cl, GLbyte *pc);
 -extern int __glXDisp_QueryMaxSwapBarriersSGIX(__GLXclientState *cl, GLbyte 
 *pc);
 -extern int __glXDisp_QueryHyperpipeNetworkSGIX(__GLXclientState *cl, GLbyte 
 *pc);
 -extern int __glXDisp_DestroyHyperpipeConfigSGIX (__GLXclientState *cl, 
 GLbyte *pc);
 -extern int __glXDisp_QueryHyperpipeConfigSGIX(__GLXclientState *cl, GLbyte 
 *pc);
 -extern int __glXDisp_HyperpipeConfigSGIX(__GLXclientState *cl, GLbyte *pc);
 -
 -extern int __glXDispSwap_BindSwapBarrierSGIX(__GLXclientState *cl, GLbyte 
 *pc);
 -extern int __glXDispSwap_QueryMaxSwapBarriersSGIX(__GLXclientState *cl, 
 GLbyte *pc);
 -extern int __glXDispSwap_QueryHyperpipeNetworkSGIX(__GLXclientState *cl, 
 GLbyte *pc);
 -extern int __glXDispSwap_DestroyHyperpipeConfigSGIX (__GLXclientState *cl, 
 GLbyte *pc);
 -extern int __glXDispSwap_QueryHyperpipeConfigSGIX(__GLXclientState *cl, 
 GLbyte *pc);
 -extern int __glXDispSwap_HyperpipeConfigSGIX(__GLXclientState *cl, GLbyte 
 *pc);
 -
 -#endif /* _GLX_g_disptab_h_ */
 diff --git a/glx/glxcmds.c b/glx/glxcmds.c
 index 8d13c15..de9c3f0 100644
 --- a/glx/glxcmds.c
 +++ b/glx/glxcmds.c
 @@ -38,7 +38,6 @@
  #include glxserver.h
  #include GL/glxtokens.h
  #include unpack.h
 -#include g_disptab.h
  #include pixmapstr.h
  #include windowstr.h
  #include glxutil.h
 @@ -2061,238 +2060,6 @@ int __glXDisp_RenderLarge(__GLXclientState *cl, 
 GLbyte *pc)
     }
  }

 -extern RESTYPE

Re: The future of Xephyr and Kdriver

[Kristian Høgsberg]

On Mon, Nov 8, 2010 at 11:01 AM, Jamey Sharp ja...@minilop.net wrote:
 On Mon, Nov 8, 2010 at 1:45 AM, Feng, Haitao haitao.f...@intel.com wrote:
 Have you noticed Kristian's Wayland project? In an email discussion,
 Kristian mentioned the Xorg on Xorg idea and discussed its design and
 implementation. Quoted from the email:

 I imagine a new module called 'hosted' that lets the DDX drivers detect
 that they're running under an existing display server (Xorg or wayland)
 and then use that instead of KMS for modesetting and allocating front
 buffers.  Ideally this can be done in a way so that the DDX driver
 doesn't need to know what the host display server is, but just
 interfaces with the 'hosted' module.  Then there will be a little bit of
 code that each DDX driver needs to add that talks to the 'hosted' module
 to get the DRM fd, to set the front buffer and to post damage.

 That's fascinating. I haven't heard Kristian mention the 'hosted'
 module idea before.

 That approach should get great performance for nested displays, and
 sounds fairly easy to implement. It only works if both servers are
 running on the same machine though. Obviously that's an important use
 case, and covers what you need for Meego. It doesn't address several
 other use cases we care about, where there are one or more backend X
 servers on different machines.

 Our project will go ahead on a pure X basis. If somebody builds this
 'hosted' module, maybe at some point in the future we could find a way
 for the two to cooperate. Something like, if the backend server has
 working DRI2 then try loading the hosted and hardware drivers
 automatically?

I already wrote it:

  http://cgit.freedesktop.org/~krh/xserver/log/?h=hosted

Right now that only supports Xorg on wayland, but I've started
separating out the backend, to let it run on either wayland or Xorg.

Kristian
___
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: [PATCH] DRI2: Free DRI2 drawable references in DRI2DestroyDrawable

[Kristian Høgsberg]

On Mon, Oct 25, 2010 at 9:55 AM, Pauli Nieminen
ext-pauli.niemi...@nokia.com wrote:
 If client calls DRI2CreateDrawable multiple times for same drawable
 DRI2 creates multiple references. Multiple references cause DRI2 send
 multiple invalidate events for same client.

Yup, there is a leak, but I think we should implement
DRI2DestroyDrawable by looking up the first DRI2DrawableRef where with
a dri2_id matching the client id and then call FreeResource(id, FALSE)
on that.

Kristian

 Problem is easy to spot from xtrace. For example following filtered
 snippet from problematic client:

 000::0b85: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
 attachments={attachment=BackLeft(0x0001)};
 000::0b89: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
 target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
 remainder_lo=0
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8e: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
 attachments={attachment=BackLeft(0x0001)};
 000::0b8f: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
 target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
 remainder_lo=0
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
 000::0bc4: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
 attachments={attachment=BackLeft(0x0001)};
 000::0bc5: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
 target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
 remainder_lo=0

 Problem is triggered because client side EGL implementation is using
 DRI2 directly.

 DRI2 code in server doesn't handle DRI2DestroyDrawable that leaks
 references. If client recreates rendering target EGL destroys and
 creates DRI2 drawable. In that case DRI2DestroyDrawable leaks DRI2
 drawable references.

 To fix this memory leak/performance problem server has to free the
 reference when client requests for it.

 If client calls DRI2CreateDrawable twice for same drawable same
 reference is reused and reference count is incremented.

 Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
 CC: Kristian Høgsberg k...@bitplanet.net

 ---
  hw/xfree86/dri2/dri2.c    |  100 +++-
  hw/xfree86/dri2/dri2.h    |    4 +-
  hw/xfree86/dri2/dri2ext.c |    2 +-
  3 files changed, 83 insertions(+), 23 deletions(-)

 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index 34f735f..103db1a 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -193,6 +193,7 @@ DRI2AllocateDrawable(DrawablePtr pDraw)
  typedef struct DRI2DrawableRefRec {
     XID                  id;
     XID                  dri2_id;
 +    int                  refcnt;
     DRI2InvalidateProcPtr      invalidate;
     void        *priv;
     struct list          link;
 @@ -229,6 +230,7 @@ DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID 
 dri2_id,

     ref-id = id;
     ref-dri2_id = dri2_id;
 +    ref-refcnt = 1;
     ref-invalidate = invalidate;
     ref-priv = priv;
     list_add(ref-link, pPriv-reference_list);
 @@ -236,11 +238,24 @@ DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID 
 dri2_id,
     return Success;
  }

 +static DRI2DrawableRefPtr
 +DRI2FindClientDrawableRef(ClientPtr client

Re: [PATCH 00/15] xserver: cleanup and standardize startup options

[Kristian Høgsberg]

On Thu, Oct 28, 2010 at 10:08 AM, Alan Coopersmith
alan.coopersm...@oracle.com wrote:
 Tiago Vignatti wrote:
 Hi fellows,

 Our culture is turning -option the default syntax for arguments to be 
 passed
 to the server. This patch set tries to standardize the rest of arguments that
 are not on such shape yet, removing some superfluous ones and also doing some
 other janitor work around.

 I tried to be very conservative in this patchset, avoiding to do major
 changes on behaviour.

 Very conservative is not changing any existing working configs into
 things that generate errors and fail to start the X server because
 their options are now invalid.   This series doesn't seem to qualify
 as very conservative.

I agree, this is not something we can change.

Kristian
___
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: Use libxkbcommon in Xorg

[Kristian Høgsberg]

2010/10/26 Jon TURNEY jon.tur...@dronecode.org.uk:


 On 22/10/2010 03:26, Kristian Høgsberg wrote:

 Now, it's all looking pretty promising, but there are a few open
 issues I'd like to throw to the list.  First of all, there's the issue
 of how this intersects/dovetails/conflicts with xkb2.  Part of the
 original plan for libxkbcommon as part of xkb2 was to also increase
 the size of certain datatypes in xkb: bump keycodes and virtual mods
 to 32 bits.  We can't do that with the existing xkb protocol and will
 require xkb2 protocol with new requests to serialize the xkb
 descriptions and we need to rethink some of the ways we represent
 certain state (per keycode repeat info makes a pretty big bit vector
 for 32 bit keycodes).  I don't think that's realistic for a 1.10
 xserver release, and I've changed the libxkbcommon structs back to be
 compatible with the current xkb protocol and implementation.  We can
 extend libxkbcommon to support bigger keycodes and vmods in a later
 release.  It's not going to be as pretty as if we did it all in the
 first release, but I think there's too much potential here to block it
 on xkb2.

 I can't really speak to the technical issues, but fwiw, I'd also rather not
 see this get blocked on xkb2, as forking xkbcomp on startup is particularly
 slow on cygwin due to the expensive fork emulation, and also unfortunately
 rather brittle.

Indeed, it should be an even bigger win on windows.

 A couple of minor build fix patches to follow.

Thanks, applied, though I didn't see the 2/2 patch for Xorg - stuck in
moderation somewhere?

Kristian
___
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

RFC: Use libxkbcommon in Xorg

[Kristian Høgsberg]

Hello,

At XDS I volunteered to make X use libxkbcommon and I've spent a few
days this week trying to make that happen.  libxkbcommon is a new
library started by Dan Nicholson which pulls in bits and pieces from
various XKB components, and primarily it provides a library interface
to the xkb compiler.  The idea is then to use libxkbcommon in Xorg to
compile the XKB keymaps instead of writing out a temp file and then
forking to run the compiler, all which is very messy.

We put the libxkbcommon library up here:

  http://cgit.freedesktop.org/xorg/lib/libxkbcommon

Since Dans and Daniels initial work on libxkbcommon I've added a few
changes myself towards making the library more self contained and the
public API as small as possible.  To that end I've copied over structs
from kbproto so that libxkbcommon becomes the new owner of those
structs and removed dependencies on libxkbfile.  At this point
libxkbcommon only depends on xproto (for keycodes and a few other
defines) and kbproto.  I added a flex generated lexer instead of the
handcoded one in xkbcomp - sadly this didn't give much of a
performance boost.  It did make it easy to make the compiler read from
a string, which avoids messing with pipes/temp files.

The Xorg side of the work is over here:

  http://cgit.freedesktop.org/~krh/xserver/log/?h=xkbcommon

and the diffstat for the changes look pretty good:

  [...@hinata xserver]$ git diff --shortstat origin/master HEAD
   66 files changed, 1554 insertions(+), 5845 deletions(-)

especially considering that among the deleted code is the fork of
xkbcomp and code to create and read tmp files and the
/usr/share/X11/xkb cached XKM file.  As for caching the compiled
keymap, I'm not really sure that we need to do that.  I tried timing
500 compilations (including forking the small test binary) of a map
and it comes in at around 10ms.  If anybody wants to try that, use the
rulescomp binary in the test/ directory of libxkbcommon but don't run
it with the libtool wrapper, that's going to account for 50% of the
time... even better write a dedicated test program that calls the
compiler entry point in a loop and profile that.

Now, it's all looking pretty promising, but there are a few open
issues I'd like to throw to the list.  First of all, there's the issue
of how this intersects/dovetails/conflicts with xkb2.  Part of the
original plan for libxkbcommon as part of xkb2 was to also increase
the size of certain datatypes in xkb: bump keycodes and virtual mods
to 32 bits.  We can't do that with the existing xkb protocol and will
require xkb2 protocol with new requests to serialize the xkb
descriptions and we need to rethink some of the ways we represent
certain state (per keycode repeat info makes a pretty big bit vector
for 32 bit keycodes).  I don't think that's realistic for a 1.10
xserver release, and I've changed the libxkbcommon structs back to be
compatible with the current xkb protocol and implementation.  We can
extend libxkbcommon to support bigger keycodes and vmods in a later
release.  It's not going to be as pretty as if we did it all in the
first release, but I think there's too much potential here to block it
on xkb2.

The remaining issues are smaller and easier to sort out;

1) the libxkbcommon API still needs review cleaning up from an
namespace point of view.

2) I'm considering whether to include a copy of the Xorg keysym under
a XKB_KEY_* namespace to break the dependency on xproto?

3) Right now libxkbcommon is essentially just an xkb keymap parser
library, should we include the xkb state machine logic too (basically
the action processing from xkbAction.c and the keysym lookup logic
from libX11)? Any user of libxkbcommon (xcb, wayland, clutter with
evdev input on kms etc) will have to implement this logic...

4) One of the big chunks of code left in the xkb modules deals with
writing out an xkb keymap based on a struct xkb_desc and a few changes
only to then parse it back into a new struct xkb_desc
(XkbWriteXKBKeymapForNames followed by xkb_compile_keymap_from_string
in XkbDDXLoadKeymapByNames).  If we moved this to libxkbcommon, we
could avoid the dump+parse step and just create the struct xkb_desc
directly... not sure if it's worth it.

and I'm sure there's more.  Like, exposing an API that's 90% hairy
structs is recipe for disaster, but it's kinda late to change that.
Anyway, looking forward to some feedback.

Kristian
___
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: [PATCH] Revert Set DamageSetReportAfterOp to true for the damage extension (#30260)

[Kristian Høgsberg]

On Sun, Oct 17, 2010 at 12:58 PM, Aaron Plattner aplatt...@nvidia.com wrote:
 This change does not solve the problem it claims to solve because it
 makes two assumptions about the driver that are not always true:

 1. that it sends rendering requests to the GPU immediately, which can
   kill performance on some GPUs, and

No.  We batch up rendering commands in a userspace command buffer and
the X server batches up outgoing events in a per client protocol
buffer.  We flush the command batch to the kernel just before the
server flushes its protocol buffers, using the flush callback chain.
This way we make sure that when we send out damage events, the render
has been submitted already, and still (typically) batch up all
rendering between one set of wake and block.

The reason for this patch is that when the client protocol buffer
overflows, the X server ends up flushing the damage events queued so
far *plus* the one it was trying to add to the protocol buffer (that's
how it works, it uses writev to send the protocol buffer *and* the
event that overflowed the buffer).  If we're using pre-op damage
reporting, that means that the rendering command corresponding to that
last damage event has not yet been added to the batch buffer and the
client will receive the damage event before we queue up the rendering.

Switching to post-op reporting fixes this race, and from a client
point of view, all we ever did was post-op reporting.  Pre-op
reporting just can't work when the damage listener is in a different
process - the X server would have to send the event immediately and
wait for the client to acknowledge it before it could submit the
rendering.  So in principle there should be no sematic difference from
the client point of view.  Unfortunately, there seems to be a problem
with the post-op reporting - it's not a code path that we've really
used before.

 2. that the GPU processes all requests from multiple clients in order,
   which is false for some GPUs.

Yes, that's an assumption I make.  I understand that it's not a fix
for hardware with multiple queues, but it allows single queue hardware
to work correctly.  The back story is that we used to have a roundtrip
in the glXBindTexImage path, which as a side effect made the intel
driver flush its rendering batch buffers.  When I eliminated the round
trip, we ended up with damage events and rendering commands no longer
in sync.  The post-op patch plus an intel driver patch to submit the
rendering from the flush callback chain (instead of just block
handler) fixed that regression.

So I just fixed a regression.  I'm not claiming to solve anything for
other drivers.

 All a Damage event tells you is that your next X request will be
 procesed after the damage has landed, not that the damage has already
 landed or even that the rendering commands that cause the damage have
 been submitted to the GPU.

 In addition to the above, this commit also broke the Compiz
 Wallpaper plugin.

 This reverts commit 8d7b7a0d71e0b89321b3341b781bc8845386def6.

Sure, we can revert it.  I never saw the corner case it fixes trigger,
we usually never actually fill up a protocol buffer with damage
events.  The only case I saw that would flush a protocol buffer was
sending out xkb keymaps.

Acked-by: Kristian Høgsberg k...@bitplanet.net

 ---
 Fixing this correctly requires spec changes, which is the purpose of James
 Jones's damage sync stuff.  If you're reading this, you should go review
 that now.  :)

  damageext/damageext.c |    1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

 diff --git a/damageext/damageext.c b/damageext/damageext.c
 index b4bb478..f5265dd 100644
 --- a/damageext/damageext.c
 +++ b/damageext/damageext.c
 @@ -217,7 +217,6 @@ ProcDamageCreate (ClientPtr client)
     if (!AddResource (stuff-damage, DamageExtType, (pointer) pDamageExt))
        return BadAlloc;

 -    DamageSetReportAfterOp (pDamageExt-pDamage, TRUE);
     DamageRegister (pDamageExt-pDrawable, pDamageExt-pDamage);

     if (pDrawable-type == DRAWABLE_WINDOW)
 --
 1.7.0.4

 ___
 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

___
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: [PATCH] Revert Set DamageSetReportAfterOp to true for the damage extension (#30260)

[Kristian Høgsberg]

On Wed, Oct 20, 2010 at 7:45 PM, Keith Packard kei...@keithp.com wrote:
 On Sun, 17 Oct 2010 09:58:50 -0700, Aaron Plattner aplatt...@nvidia.com 
 wrote:

 In addition to the above, this commit also broke the Compiz
 Wallpaper plugin.

 Ok, as usual, Dave Airlie is right here -- independent of whether this
 patch is a good idea (and I think it is), it should be reverted until it
 doesn't cause a regression.

Yup, and that's why I acked it in the first place.  It doesn't reduce
GPU/CPU parallelism and it's not new semantics for the damage
extension.  If the post-op reporting worked correctly, the only
difference would be in the case where a damage event overflows the
clients protocol buffer.  And in that case, the only difference would
be that the driver gets to see the rendering request before X flushes
the buffer.  There's really nothing to discuss.

Kristian
___
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: [PATCH v2] glx: Fix use after free in DrawableGone

[Kristian Høgsberg]

2010/9/23 Kristian Høgsberg k...@bitplanet.net:
 2010/9/23 Jeremy Huddleston jerem...@apple.com:
 That seems off to me.  This is doing more than changing the c-next 
 dereference.  You're now freeing it where you weren't before.

 Previously, you freed it inside:
 if (c-isCurrent  (c-drawPriv == glxPriv || c-readPriv == glxPriv))
    if(!c-idExists)

 Now, you free it inside:
 if (!c-idExists  !c-isCurrent)

 So you're missing the check for (c-drawPriv == glxPriv || c-readPriv == 
 glxPriv) ... is that intentional and we had a leak before, or are we now 
 over-releasing?

 All the contexts on the context list have either idExists or isCurrent
 or both true, otherwise we would have already destroyed them.  If a
 context that was current is destroyed, we set idExists to NULL (it no
 longer has an XID) but keep it around while it's still current.  If
 the context is not current, we just destroy it right away and take it
 out of the list.

 The loop above, iterates through the list of all contexts to see if
 there is a context that has been destroyed but is still current with
 the drawable that we're getting the DrawableGone() callback for.  We
 treat that as an unbind, which triggers the context destruction.  As
 Jon mentions, that may not be the right thing to do, but in the scope
 of this patch, we're just delaying the destroy until we're done
 touching the context.

Jeremy, does the above explanation satisfy your concerns?  Keith, do
you want to pick this up for master?

 On Sep 23, 2010, at 06:04, Kristian Høgsberg wrote:

 Signed-off-by: Kristian Høgsberg k...@bitplanet.net
 ---

 Chris Wilson points out that we were still accessing c-next after free.
 Here's an updated version that fixes that.

 Kristian

 glx/glxext.c |   11 +--
 1 files changed, 5 insertions(+), 6 deletions(-)

 diff --git a/glx/glxext.c b/glx/glxext.c
 index e203156..f5ebe4f 100644
 --- a/glx/glxext.c
 +++ b/glx/glxext.c
 @@ -124,7 +124,7 @@ static int glxBlockClients;
 */
 static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
 {
 -    __GLXcontext *c;
 +    __GLXcontext *c, *next;

     /* If this drawable was created using glx 1.3 drawable
      * constructors, we added it as a glx drawable resource under both
 @@ -137,7 +137,8 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID 
 xid)
           FreeResourceByType(glxPriv-drawId, __glXDrawableRes, TRUE);
     }

 -    for (c = glxAllContexts; c; c = c-next) {
 +    for (c = glxAllContexts; c; c = next) {
 +     next = c-next;
       if (c-isCurrent  (c-drawPriv == glxPriv || c-readPriv == 
 glxPriv)) {
           int i;

 @@ -160,15 +161,13 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID 
 xid)
                   }
               }
           }
 -
 -         if (!c-idExists) {
 -             __glXFreeContext(c);
 -         }
       }
       if (c-drawPriv == glxPriv)
           c-drawPriv = NULL;
       if (c-readPriv == glxPriv)
           c-readPriv = NULL;
 +     if (!c-idExists  !c-isCurrent)
 +         __glXFreeContext(c);
     }

     glxPriv-destroy(glxPriv);
 --
 1.7.3

 ___
 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



___
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

[PATCH] glx: Fix use after free in DrawableGone

[Kristian Høgsberg]

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---

 ==2989== Invalid write of size 4
 ==2989==at 0x48CE6E5: DrawableGone (glxext.c:169)
 ==2989==by 0x809F401: FreeResource (resource.c:601)
 ==2989==by 0x80845CE: ProcDestroyWindow (dispatch.c:733)
 ==2989==by 0x8087D76: Dispatch (dispatch.c:432)
 ==2989==by 0x8066439: main (main.c:291)
 ==2989==  Address 0x55a9c1c is 76 bytes inside a block of size 88 free'd
 ==2989==at 0x4023B6A: free (vg_replace_malloc.c:366)
 ==2989==by 0x48D9DD8: __glXDRIcontextDestroy (glxdri2.c:250)
 ==2989==by 0x48CE1A0: __glXFreeContext (glxext.c:222)
 ==2989==by 0x48CE786: DrawableGone (glxext.c:165)
 ==2989==by 0x809F401: FreeResource (resource.c:601)
 ==2989==by 0x80845CE: ProcDestroyWindow (dispatch.c:733)
 ==2989==by 0x8087D76: Dispatch (dispatch.c:432)
 ==2989==by 0x8066439: main (main.c:291)

 Kristian, I know how much you enjoy fixing these... You probably already
 have a patch. ;-)

I think we're looking at this use after free problem.  We may free the
context in the big if-statement above and the go and touch its drawPriv
and readPriv fields afterwards.

Kristian

 glx/glxext.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/glx/glxext.c b/glx/glxext.c
index e203156..0b04c37 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -160,15 +160,13 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
}
}
}
-
-   if (!c-idExists) {
-   __glXFreeContext(c);
-   }
}
if (c-drawPriv == glxPriv)
c-drawPriv = NULL;
if (c-readPriv == glxPriv)
c-readPriv = NULL;
+   if (!c-idExists  !c-isCurrent)
+   __glXFreeContext(c);
 }
 
 glxPriv-destroy(glxPriv);
-- 
1.7.3

___
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: [PATCH v2] glx: Fix use after free in DrawableGone

[Kristian Høgsberg]

2010/9/23 Jeremy Huddleston jerem...@apple.com:
 That seems off to me.  This is doing more than changing the c-next 
 dereference.  You're now freeing it where you weren't before.

 Previously, you freed it inside:
 if (c-isCurrent  (c-drawPriv == glxPriv || c-readPriv == glxPriv))
    if(!c-idExists)

 Now, you free it inside:
 if (!c-idExists  !c-isCurrent)

 So you're missing the check for (c-drawPriv == glxPriv || c-readPriv == 
 glxPriv) ... is that intentional and we had a leak before, or are we now 
 over-releasing?

All the contexts on the context list have either idExists or isCurrent
or both true, otherwise we would have already destroyed them.  If a
context that was current is destroyed, we set idExists to NULL (it no
longer has an XID) but keep it around while it's still current.  If
the context is not current, we just destroy it right away and take it
out of the list.

The loop above, iterates through the list of all contexts to see if
there is a context that has been destroyed but is still current with
the drawable that we're getting the DrawableGone() callback for.  We
treat that as an unbind, which triggers the context destruction.  As
Jon mentions, that may not be the right thing to do, but in the scope
of this patch, we're just delaying the destroy until we're done
touching the context.

Kristian

 On Sep 23, 2010, at 06:04, Kristian Høgsberg wrote:

 Signed-off-by: Kristian Høgsberg k...@bitplanet.net
 ---

 Chris Wilson points out that we were still accessing c-next after free.
 Here's an updated version that fixes that.

 Kristian

 glx/glxext.c |   11 +--
 1 files changed, 5 insertions(+), 6 deletions(-)

 diff --git a/glx/glxext.c b/glx/glxext.c
 index e203156..f5ebe4f 100644
 --- a/glx/glxext.c
 +++ b/glx/glxext.c
 @@ -124,7 +124,7 @@ static int glxBlockClients;
 */
 static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
 {
 -    __GLXcontext *c;
 +    __GLXcontext *c, *next;

     /* If this drawable was created using glx 1.3 drawable
      * constructors, we added it as a glx drawable resource under both
 @@ -137,7 +137,8 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
           FreeResourceByType(glxPriv-drawId, __glXDrawableRes, TRUE);
     }

 -    for (c = glxAllContexts; c; c = c-next) {
 +    for (c = glxAllContexts; c; c = next) {
 +     next = c-next;
       if (c-isCurrent  (c-drawPriv == glxPriv || c-readPriv == 
 glxPriv)) {
           int i;

 @@ -160,15 +161,13 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID 
 xid)
                   }
               }
           }
 -
 -         if (!c-idExists) {
 -             __glXFreeContext(c);
 -         }
       }
       if (c-drawPriv == glxPriv)
           c-drawPriv = NULL;
       if (c-readPriv == glxPriv)
           c-readPriv = NULL;
 +     if (!c-idExists  !c-isCurrent)
 +         __glXFreeContext(c);
     }

     glxPriv-destroy(glxPriv);
 --
 1.7.3

 ___
 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


___
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: [PATCH xserver] config: remove --with-dri-driver-path option, use dri.pc #29740

[Kristian Høgsberg]

2010/8/24 Gaetan Nadon mems...@videotron.ca:
 On Tue, 2010-08-24 at 12:56 +0200, Michel Dänzer wrote:

 Could just use dri.pc for the default value of the option, rather than
 removing it altogether?

 Yes. I think the author of the bug report wishes to also remove the option
 as it is no longer useful.

 This is an opportunity for me to get the real story being this type of
 option. There are many in the server but also in other modules. It looks to
 me that in the past there were no pkg-config infrastructure (or it was not
 used or not mature) so there had to be an option for every directory a
 module needed to know.

 Once a directory is published through pkg-config, there is no longer a need
 to ask the user for its value. I suspect it is even wrong to allow the user
 to specify one as there cannot be 2 directories for the same stuff.

I added the option in c3342c8000f6d2bfb61e2cf95e028d11b59698fa in
2004.  At the time we didn't have dri.pc.  I don't see a reason to
keep it now.

Kristian
___
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

[PULL] Flush fixes

[Kristian Høgsberg]

The following changes since commit 7e0575baf14ec4a89492fd2780f9ab5b9244afbd:

  ddc: Fix memory leak in GetEDID_DDC1 (2010-08-01 22:48:21 -0400)

are available in the git repository at:
  git://anongit.freedesktop.org/~krh/xserver flush-callback

Kristian Høgsberg (2):
  Always call the flush callback chain when we flush client buffers
  Set DamageSetReportAfterOp to true for the damage extension

 damageext/damageext.c |1 +
 os/connection.c   |3 +++
 os/io.c   |4 
 3 files changed, 8 insertions(+), 0 deletions(-)
___
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: [Intel-gfx] xf86-video-intel: configure.ac

[Kristian Høgsberg]

On Wed, Jul 28, 2010 at 4:42 AM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Tue, 27 Jul 2010 09:34:01 -0700, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
 and glproto is missing a dep on GL:

 arjan In file included from i810.h:60:0,
 arjan                  from i810_accel.c:41:
 arjan /usr/include/GL/glxint.h:36:19: fatal error: GL/gl.h: No such file 
 or directory

 From memory, I think this is just a bit of idiocacy in the driver
 inappropriately using a GL type, i.e. for a variable unconnected with
 any of the GL interfaces.

There was a few gratuitous uses of GLuint in the core driver that I
just removed.  Then the legacy i810 driver still had the code to
create and set GLX visuals, which hasn't been used for years now (we
get the configs from the dri driver even for dri1 since 2007).I
removed that chunk of code, which means that the legacy driver wont
work with X server before 2007, but the purpose of the legacy code is
to run the old driver on new servers anyway (right?).

Kristian
___
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: [PATCH 14/15] xkb: Use memcpy for copy

[Kristian Høgsberg]

On Wed, Jul 28, 2010 at 7:26 AM, Daniel Stone dan...@fooishbar.org wrote:
 On Wed, Jul 28, 2010 at 02:11:05PM +0300, Pauli Nieminen wrote:
   On Tue, Jul 27, 2010 at 03:09:55PM +0300, Pauli Nieminen wrote:
Source and destination have well defined size so use memcpy instead of
strncpy. strncpy tryes to add NULL to end of destination but it is not
                      ^ tries
possible if source doesn't have NULL.

  
   my strncpy man page claims that if there is no null byte among the 
   first n
   bytes of src, the string placed in dest will not be null terminated. 
   That's
   counter to what the message above says.

 static analyzer complains about possible missing NULL termination for string,

 Also code is already mainly using memcpy for same copy in other places. That
 why I tough change to memcpy would make sense. I should have added that to
 commit message.

 It doesn't need to be NULL-terminated; all the key names are always 4
 bytes long (cf. XkbKeyNameLength), so to use it as a string, you copy it
 out to a five-byte array and then NULL-terminate it.

Or you can use %.4s if you're just printf-ing it.

Kristian
___
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: [PATCH] DRI2: use ConfigNotify to re-allocate buffers if root window changes

[Kristian Høgsberg]

On Thu, Jul 8, 2010 at 3:30 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 On Thu, 08 Jul 2010 12:26:22 -0700
 Keith Packard kei...@keithp.com wrote:

 On Thu, 8 Jul 2010 11:39:01 -0700, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:

  If the root window changes size, we need to re-allocate any DRI2
  buffers that share the same pixmap so that they'll pick up the new root
  pixmap (if the underlying driver ended up creating a new one).

 Why is this only relevant at the screen level? What happens if a
 redirected window is resized?

 I think the existing code handles that already.

It's the GL child window of redirected pixmap we just talked about in
IRC (shame on me for doing code review over IRC).  If the GL window is
a child window of a redirect window that gets resized, we end up in
the same situation: the parent window gets a new window pixmap, but
the dri2 drawable is the same size.  It only causes trouble when we're
swapping the window pixmap, that is, page flipping or replacing the
backing pixmap for a redirected window.  As long as we go through the
DRI2CopyRegion path, it doesn't matter that the window pixmap is
replaced, the auxillary buffers for the DRI2 window can  stay the same
just fine.

So the current code handles it as it is, but only because we don't
swap window pixmaps for redirected windows.  If we want to handle
that, we need to check if the window in ConfigureNotify has a DRI2
subwindow and do stuff if so, which doesn't sound like a good
approach.  Maybe we're better off hooking pScreen-SetWindowPixmap and
marking the buffers as invalid there.

Kristian
___
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: [PATCH 1/2] list.h: Fix list_for_each_entry_safe()

[Kristian Høgsberg]

On Fri, Jun 18, 2010 at 5:43 PM, Dan Nicholson dbn.li...@gmail.com wrote:
 On Fri, Jun 18, 2010 at 2:34 PM, Keith Packard kei...@keithp.com wrote:
 On Mon, 14 Jun 2010 09:25:22 -0400, Kristian Høgsberg k...@bitplanet.net 
 wrote:

 -      pos = next, next = __container_of(next-member.next, next, member))
 +      pos = tmp, tmp = __container_of(pos-member.next, tmp, member))

 Any reason this uses a ',' instead of a ';'?

 Because it's the initializer in a for loop. If there was a semicolon,
 it would become the test argument.

Yup, we're doing to assignments in the initializer, separated by the
comma operator.

Kristian
___
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

[PATCH 1/2] list.h: Fix list_for_each_entry_safe()

[Kristian Høgsberg]

Can't use next as a macro argument since we're accessing the .next field
of struct list.
---
 include/list.h |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/list.h b/include/list.h
index 89dc29d..4ce20a8 100644
--- a/include/list.h
+++ b/include/list.h
@@ -94,10 +94,10 @@ list_is_empty(struct list *head)
 pos-member != (head);\
 pos = __container_of(pos-member.next, pos, member))
 
-#define list_for_each_entry_safe(pos, next, head, member)  \
+#define list_for_each_entry_safe(pos, tmp, head, member)   \
 for (pos = __container_of((head)-next, pos, member),  \
-next = __container_of(pos-member.next, pos, member);  \
+tmp = __container_of(pos-member.next, pos, member);   \
 pos-member != (head);\
-pos = next, next = __container_of(next-member.next, next, member))
+pos = tmp, tmp = __container_of(pos-member.next, tmp, member))
 
 #endif
-- 
1.7.1

___
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

[PATCH 2/2] xfixes: Use list.h macros in tracking hide count

[Kristian Høgsberg]

---

This is not a patch to be applied.  Recently, the utility of list.h was
questioned and as I came a across the cursor hide count code today I
figured it would make a good example of how list.h can make the code
significantly simpler.  In particular, it's easy to prepend and iterate
though a singly linked list, but anything else is going to be buggy or slow
or best-case just 20 lines of code to delete an element that you shouldn't
have to write and debug again (cf deleteCursorHideCount() below).

Kristian

 xfixes/cursor.c |   47 +--
 1 files changed, 13 insertions(+), 34 deletions(-)

diff --git a/xfixes/cursor.c b/xfixes/cursor.c
index d3a207d..c8d03c7 100644
--- a/xfixes/cursor.c
+++ b/xfixes/cursor.c
@@ -53,6 +53,7 @@
 #include inputstr.h
 #include windowstr.h
 #include xace.h
+#include list.h
 
 static RESTYPE CursorClientType;
 static RESTYPE CursorHideCountType;
@@ -100,7 +101,7 @@ static CursorEventPtr   cursorEvents;
 typedef struct _CursorHideCountRec *CursorHideCountPtr;
 
 typedef struct _CursorHideCountRec {
-CursorHideCountPtr   pNext;  
+struct list  link;
 ClientPtrpClient;
 ScreenPtrpScreen;
 int  hideCount;
@@ -114,7 +115,8 @@ typedef struct _CursorHideCountRec {
 typedef struct _CursorScreen {
 DisplayCursorProcPtr   DisplayCursor;
 CloseScreenProcPtr CloseScreen;
-CursorHideCountPtr  pCursorHideCounts;
+struct list CursorHideCounts;
+
 } CursorScreenRec, *CursorScreenPtr;
 
 #define GetCursorScreen(s) 
((CursorScreenPtr)dixLookupPrivate((s)-devPrivates, CursorScreenPrivateKey))
@@ -146,7 +148,7 @@ CursorDisplayCursor (DeviceIntPtr pDev,
 if (ConnectionInfo)
CursorVisible = EnableCursor;
 
-if (cs-pCursorHideCounts != NULL || !CursorVisible) {
+if (!list_is_empty(cs-CursorHideCounts) || !CursorVisible) {
ret = (*pScreen-DisplayCursor) (pDev, pScreen, NullCursor);
 } else {
ret = (*pScreen-DisplayCursor) (pDev, pScreen, pCursor);
@@ -779,7 +781,7 @@ findCursorHideCount (ClientPtr pClient, ScreenPtr pScreen)
 CursorScreenPtrcs = GetCursorScreen(pScreen);
 CursorHideCountPtr pChc;
 
-for (pChc = cs-pCursorHideCounts; pChc != NULL; pChc = pChc-pNext) {
+list_for_each_entry(pChc, cs-CursorHideCounts, link) {
if (pChc-pClient == pClient) {
return pChc;
}
@@ -802,8 +804,7 @@ createCursorHideCount (ClientPtr pClient, ScreenPtr pScreen)
 pChc-pScreen = pScreen;
 pChc-hideCount = 1;
 pChc-resource = FakeClientID(pClient-index);
-pChc-pNext = cs-pCursorHideCounts;
-cs-pCursorHideCounts = pChc;
+list_add(pChc-link, cs-CursorHideCounts);
 
 /* 
  * Create a resource for this element so it can be deleted
@@ -822,27 +823,10 @@ createCursorHideCount (ClientPtr pClient, ScreenPtr 
pScreen)
  * Delete the given hide-counts list element from its screen list.
  */
 static void
-deleteCursorHideCount (CursorHideCountPtr pChcToDel, ScreenPtr pScreen)
+deleteCursorHideCount (CursorHideCountPtr pChcToDel)
 {
-CursorScreenPtrcs = GetCursorScreen(pScreen);
-CursorHideCountPtr pChc, pNext;
-CursorHideCountPtr pChcLast = NULL;
-
-pChc = cs-pCursorHideCounts;
-while (pChc != NULL) {
-   pNext = pChc-pNext;
-   if (pChc == pChcToDel) {
-   free(pChc);
-   if (pChcLast == NULL) {
-   cs-pCursorHideCounts = pNext;
-   } else {
-   pChcLast-pNext = pNext;
-   }
-   return;
-   }
-   pChcLast = pChc;
-   pChc = pNext;
-}
+list_del(pChcToDel-link);
+free(pChcToDel);
 }
 
 /* 
@@ -854,13 +838,8 @@ deleteCursorHideCountsForScreen (ScreenPtr pScreen)
 CursorScreenPtrcs = GetCursorScreen(pScreen);
 CursorHideCountPtr pChc, pTmp;
 
-pChc = cs-pCursorHideCounts;
-while (pChc != NULL) {
-   pTmp = pChc-pNext;
+list_for_each_entry_safe(pChc, pTmp, cs-CursorHideCounts, link)
FreeResource(pChc-resource, 0);
-   pChc = pTmp;
-}
-cs-pCursorHideCounts = NULL;   
 }
 
 int 
@@ -1002,7 +981,7 @@ CursorFreeHideCount (pointer data, XID id)
 ScreenPtr pScreen = pChc-pScreen;
 DeviceIntPtr dev;
 
-deleteCursorHideCount(pChc, pChc-pScreen);
+deleteCursorHideCount(pChc);
 for (dev = inputInfo.devices; dev; dev = dev-next)
 {
 if (IsMaster(dev)  IsPointerDevice(dev))
@@ -1047,7 +1026,7 @@ XFixesCursorInit (void)
return FALSE;
Wrap (cs, pScreen, CloseScreen, CursorCloseScreen);
Wrap (cs, pScreen, DisplayCursor, CursorDisplayCursor);
-   cs-pCursorHideCounts = NULL;
+   list_init(cs-CursorHideCounts);
SetCursorScreen (pScreen, cs);
 }
 CursorClientType = CreateNewResourceType(CursorFreeClient,
-- 
1.7.1

___
xorg-devel@lists.x.org: X.Org 

Re: [PATCH 1/2] list.h: Fix list_for_each_entry_safe()

[Kristian Høgsberg]

On Mon, Jun 14, 2010 at 10:52 AM, Tiago Vignatti
tiago.vigna...@nokia.com wrote:
 On Mon, Jun 14, 2010 at 03:25:22PM +0200, ext Kristian H�gsberg wrote:
 Can't use next as a macro argument since we're accessing the .next field
 of struct list.
 ---
  include/list.h |    6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/include/list.h b/include/list.h
 index 89dc29d..4ce20a8 100644
 --- a/include/list.h
 +++ b/include/list.h
 @@ -94,10 +94,10 @@ list_is_empty(struct list *head)
        pos-member != (head);                                        \
        pos = __container_of(pos-member.next, pos, member))

 -#define list_for_each_entry_safe(pos, next, head, member)            \
 +#define list_for_each_entry_safe(pos, tmp, head, member)             \
      for (pos = __container_of((head)-next, pos, member),            \
 -      next = __container_of(pos-member.next, pos, member);          \
 +      tmp = __container_of(pos-member.next, pos, member);           \
        pos-member != (head);                                        \
 -      pos = next, next = __container_of(next-member.next, next, member))
 +      pos = tmp, tmp = __container_of(pos-member.next, tmp, member))

  #endif

 Kristian, doesn't make sense then to use inline function here instead keep
 juggling with the names?

We can't do that for this macro.  If you look closer, you'll see that
it expands to a for statement without the body.  You use it like this:

list_for_each_entry_safe(...) {
/* do something with each entry */
}

Kristian
___
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: [PATCH 2/2] xfixes: Use list.h macros in tracking hide count

[Kristian Høgsberg]

On Mon, Jun 14, 2010 at 10:56 AM, Tiago Vignatti
tiago.vigna...@nokia.com wrote:
 On Mon, Jun 14, 2010 at 03:25:23PM +0200, ext Kristian H�gsberg wrote:

 This is not a patch to be applied.

 why not?! :)

Oh, I mentioned this in my first attempt to send out these patches
(which git send-mail threw away when it couldn't find an smtp server).
 I don't think it makes sense to go through the server and replace
existing linked list code.  That code is already written and tested,
and replacing it can only break stuff.  The patch was just meant to
illustrate how list.h easily pays for itself once you need to do
something more advanced than prepending or iterating through the list.
 You know, just in case Keith got tempted to rewrite more list.h users
to open coded lists.

Kristian
___
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: [PATCH 2/2] xfixes: Use list.h macros in tracking hide count

[Kristian Høgsberg]

2010/6/14 Vignatti Tiago (Nokia-D/Helsinki) tiago.vigna...@nokia.com:
 On Mon, Jun 14, 2010 at 05:24:16PM +0200, ext Kristian Høgsberg wrote:

  I don't think it makes sense to go through the server and replace
 existing linked list code.  That code is already written and tested,
 and replacing it can only break stuff.

 I think open coded list implementation everywhere is more error prone than
 just have one single set of basic macros. Isn't it?

All the open coded lists in ther server are there now, and they're
working.  If it's not broken, don't mess with it.  If you come across
an error in an open coded list implementation, that would be a good
reason to port it to list.h.  But otherwise, you're just almost
certainly going to break working code.

Kristian
___
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: performance of pci_device_get_{vendor, device}_name() in X server startup

[Kristian Høgsberg]

On Wed, Jun 9, 2010 at 7:23 AM, Daniel Stone dan...@fooishbar.org wrote:
 On Tue, Jun 08, 2010 at 09:40:55PM -0400, Matt Turner wrote:
 On Tue, Jun 8, 2010 at 9:35 PM, Richard Barnette
 jrbarne...@chromium.org wrote:
  Still, cost/benefit matters here:  Essentially, the justification
  for all this work is a debug feature (being able to print the information
  in the log when things go wrong), not a performance enhancement.
  I'm not yet persuaded that that feature is worth the identified effort.

 I'd still like to hear some opinions from people who do serious
 xserver work, but from my perspective there's nothing wrong with only
 executing this code if -verbose is used. The output of `lspci -vv` is
 already a nearly required piece of any bug report, so I don't think
 we're losing anything here.

 Indeed.  We already get a more accurate/useful device/vendor identifier
 string from the driver, and we don't need to know/care about non-GPU
 devices.

 I can see how it would be useful in verbose/error cases, but eh.

Agree, we should be able to just get rid of it in all cases and
require the driver to log the chipset name if that something the
driver authors want to see in the log.

Kristian
___
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