[PATCH xorg-gtest 5/9] Split usage help text into a helper function

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/xorg-gtest_main.cpp |   25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/xorg-gtest_main.cpp b/src/xorg-gtest_main.cpp
index bbf232b..08927a0 100644
--- a/src/xorg-gtest_main.cpp
+++ b/src/xorg-gtest_main.cpp
@@ -105,6 +105,18 @@ static void setup_signal_handlers() {
  signals[i]  \n;
 }
 
+static int usage(int exitcode) {
+  std::cout  \nAdditional options:\n;
+  std::cout  --no-dummy-server: Use the currently running X server 
+   for testing\n;
+  std::cout  --xorg-conf: Path to xorg dummy configuration file\n;
+  std::cout  --server: Path to X server executable\n;
+  std::cout  --xorg-display: xorg dummy display port\n;
+  std::cout  --xorg-logfile: xorg logfile filename. See -logfile in 
\man Xorg\.\n
+   Its default value is 
DEFAULT_XORG_LOGFILE.\n;
+  return exitcode;
+}
+
 int main(int argc, char *argv[]) {
   std::string xorg_conf_path;
   std::string xorg_log_file_path;
@@ -151,17 +163,8 @@ int main(int argc, char *argv[]) {
 }
   }
 
-  if (help) {
-std::cout  \nAdditional options:\n;
-std::cout  --no-dummy-server: Use the currently running X server 
-for testing\n;
-std::cout  --xorg-conf: Path to xorg dummy configuration file\n;
-std::cout  --server: Path to X server executable\n;
-std::cout  --xorg-display: xorg dummy display port\n;
-std::cout  --xorg-logfile: xorg logfile filename. See -logfile in 
\man Xorg\.\n
- Its default value is 
DEFAULT_XORG_LOGFILE.\n;
-exit(-1);
-  }
+  if (help)
+return usage(-1);
 
   if (!no_dummy_server) {
 environment = new xorg::testing::Environment;
-- 
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


[PATCH xorg-gtest 4/9] Fix up a couple of doxygen links

2012-07-03 Thread Peter Hutterer
Introduced in e1c010f23272e61c28c73aa603b477ba6fbae875

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 examples/xorg-gtest-example.cpp  |2 +-
 include/xorg/gtest/evemu/xorg-gtest_device.h |2 +-
 include/xorg/gtest/xorg-gtest_environment.h  |2 +-
 include/xorg/gtest/xorg-gtest_process.h  |2 +-
 include/xorg/gtest/xorg-gtest_test.h |2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/examples/xorg-gtest-example.cpp b/examples/xorg-gtest-example.cpp
index 6a2f662..32d575d 100644
--- a/examples/xorg-gtest-example.cpp
+++ b/examples/xorg-gtest-example.cpp
@@ -3,7 +3,7 @@
 using namespace xorg::testing;
 
 /**
- * @example xorg-gtest.cpp
+ * @example xorg-gtest-example.cpp
  *
  * This is an example for using the fixture
  * xorg::testing::Test for your own tests. Please
diff --git a/include/xorg/gtest/evemu/xorg-gtest_device.h 
b/include/xorg/gtest/evemu/xorg-gtest_device.h
index 4e456ce..5f8faa2 100644
--- a/include/xorg/gtest/evemu/xorg-gtest_device.h
+++ b/include/xorg/gtest/evemu/xorg-gtest_device.h
@@ -42,7 +42,7 @@ namespace testing {
 namespace evemu {
 
 /**
- * @class Device device.h xorg/gtest/evemu/device.h
+ * @class Device xorg-gtest_device.h xorg/gtest/evemu/xorg-gtest_device.h
  *
  * uTouch-Evemu input device for replaying events through the Linux uinput
  * evdev subsystem.
diff --git a/include/xorg/gtest/xorg-gtest_environment.h 
b/include/xorg/gtest/xorg-gtest_environment.h
index e113cd7..723223f 100644
--- a/include/xorg/gtest/xorg-gtest_environment.h
+++ b/include/xorg/gtest/xorg-gtest_environment.h
@@ -46,7 +46,7 @@ namespace testing {
  */
 
 /**
- * @class Environment environment.h xorg/gtest/environment.h
+ * @class Environment xorg-gtest_environment.h environment.h 
xorg/gtest/xorg-gtest_environment.h
  *
  * Global Google %Test environment providing a dummy X server.
  *
diff --git a/include/xorg/gtest/xorg-gtest_process.h 
b/include/xorg/gtest/xorg-gtest_process.h
index d7e1143..476c0ad 100644
--- a/include/xorg/gtest/xorg-gtest_process.h
+++ b/include/xorg/gtest/xorg-gtest_process.h
@@ -37,7 +37,7 @@ namespace xorg {
 namespace testing {
 
 /**
- * @class Process test.h xorg/gtest/process.h
+ * @class Process xorg-gtest_process.h xorg/gtest/xorg-gtest_process.h
  *
  * Class that abstracts child process creation and termination.
  *
diff --git a/include/xorg/gtest/xorg-gtest_test.h 
b/include/xorg/gtest/xorg-gtest_test.h
index 093890b..a3cd3b9 100644
--- a/include/xorg/gtest/xorg-gtest_test.h
+++ b/include/xorg/gtest/xorg-gtest_test.h
@@ -37,7 +37,7 @@ namespace xorg {
 namespace testing {
 
 /**
- * @class Test test.h xorg/gtest/test.h
+ * @class Test xorg-gtest_test.h xorg/gtest/xorg-gtest_test.h
  *
  * Google %Test fixture providing an Xlib connection to an X11 server.
  *
-- 
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


[PATCH xorg-gtest 7/9] Add some comments to the whacky library building

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 examples/Makefile.am |3 +++
 1 file changed, 3 insertions(+)

diff --git a/examples/Makefile.am b/examples/Makefile.am
index b9b914b..4a3b633 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -29,10 +29,12 @@ check_LIBRARIES = libgtest.a libxorg-gtest.a 
libxorg-gtest_main.a
 AM_CPPFLAGS = $(GTEST_CPPFLAGS)
 AM_CXXFLAGS = $(XSERVER_CFLAGS) $(BASE_CXXFLAGS)
 
+# build googletest as static lib
 nodist_libgtest_a_SOURCES = $(GTEST_SOURCE)/src/gtest-all.cc
 libgtest_a_CPPFLAGS = $(AM_CPPFLAGS) -w
 libgtest_a_CXXFLAGS = $(GTEST_CXXFLAGS) $(AM_CXXFLAGS)
 
+# build xorg-gtest as static lib
 libxorg_gtest_a_SOURCES = $(top_srcdir)/src/xorg-gtest-all.cpp
 libxorg_gtest_a_CPPFLAGS = \
$(AM_CPPFLAGS) \
@@ -41,6 +43,7 @@ libxorg_gtest_a_CPPFLAGS = \
-DDUMMY_CONF_PATH=\$(top_srcdir)/data/xorg/gtest/dummy.conf\
 libxorg_gtest_a_CXXFLAGS = $(GTEST_CXXFLAGS) $(AM_CXXFLAGS)
 
+# build xorg-gtest's main as separate lib for those that need it
 libxorg_gtest_main_a_SOURCES = $(top_srcdir)/src/xorg-gtest_main.cpp
 libxorg_gtest_main_a_CPPFLAGS = \
$(AM_CPPFLAGS) \
-- 
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


[PATCH xorg-gtest 2/9] doc: shut up doxygen

2012-07-03 Thread Peter Hutterer
Errors/warnings on building documentation are enough, we don't need to see
the whole spew

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 doc/Doxyfile |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/Doxyfile b/doc/Doxyfile
index 488b15c..cf58184 100644
--- a/doc/Doxyfile
+++ b/doc/Doxyfile
@@ -559,7 +559,7 @@ LAYOUT_FILE=
 # The QUIET tag can be used to turn on/off the messages that are generated 
 # by doxygen. Possible values are YES and NO. If left blank NO is used.
 
-QUIET  = NO
+QUIET  = YES
 
 # The WARNINGS tag can be used to turn on/off the warning messages that are 
 # generated by doxygen. Possible values are YES and NO. If left blank 
-- 
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


[PATCH xorg-gtest 8/9] Rename XSERVER_LIBS to X11_LIBS

2012-07-03 Thread Peter Hutterer
XSERVER implies this has to do with the server, but these are the client
libs.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 configure.ac |2 +-
 examples/Makefile.am |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3dea940..6e9e458 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,7 +25,7 @@ XORG_DEFAULT_OPTIONS
 XORG_ENABLE_INTEGRATION_TESTS([yes])
 XORG_WITH_DOXYGEN
 
-PKG_CHECK_MODULES(XSERVER, x11)
+PKG_CHECK_MODULES(X11, x11)
  
 # Check for Google Test
 CHECK_GTEST
diff --git a/examples/Makefile.am b/examples/Makefile.am
index 4a3b633..49c133f 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -27,7 +27,7 @@
 check_LIBRARIES = libgtest.a libxorg-gtest.a libxorg-gtest_main.a
 
 AM_CPPFLAGS = $(GTEST_CPPFLAGS)
-AM_CXXFLAGS = $(XSERVER_CFLAGS) $(BASE_CXXFLAGS)
+AM_CXXFLAGS = $(X11_CFLAGS) $(BASE_CXXFLAGS)
 
 # build googletest as static lib
 nodist_libgtest_a_SOURCES = $(GTEST_SOURCE)/src/gtest-all.cc
@@ -66,5 +66,5 @@ xorg_gtest_example_LDADD = \
libxorg-gtest.a \
libxorg-gtest_main.a \
-lpthread \
-   $(XSERVER_LIBS) \
+   $(X11_LIBS) \
$(EVEMU_LIBS)
-- 
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


[PATCH xorg-gtest 9/9] Add note to Makefile.am that new sources must be added to xorg-gtest-all.cpp

2012-07-03 Thread Peter Hutterer
Hopefully saves the next poor sod weird linker errors.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/Makefile.am |1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Makefile.am b/src/Makefile.am
index 148f1f2..0b93df0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -23,6 +23,7 @@
 # SOFTWARE.
 #
 
+# NOTE: source files added here must be included in xorg-gtest-all.cpp
 libxorg_gtest_sources = \
environment.cpp \
device.cpp \
-- 
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


Re: [PATCH 17/19 v2] Set padding bytes to 0 in WriteToClient

2012-07-03 Thread Keith Packard
Alan Coopersmith alan.coopersm...@oracle.com writes:

 Clear them out when needed instead of leaving whatever values were
 present in previously sent messages.

 Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com
 ---

 v2: Simplified as suggested by Keith to just use memset instead of a 
 Duff-device style switch statement, since most calls won't need this.

Looks good.

Reviewed-by: Keith Packard kei...@keithp.com

Any response to my complaint about mixing initializers and struct value
assignment styles? I'm afraid I've scared you off...

-- 
keith.pack...@intel.com


pgpbUplhrN2vx.pgp
Description: PGP signature
___
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 0/5] unified sigio handling

2012-07-03 Thread Peter Hutterer

I left the first patch as-is because it's easy to cherry-pick to 1.12 this
way. I also left the xf86BlockSIGIO() calls in though I don't think outside
of the ddx actually uses them. We can probably drop them, either this cycle
or the next.

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


[PATCH 1/5] xfree86: always enable SIGIO on OsVendorInit (#50957)

2012-07-03 Thread Peter Hutterer
Drivers call xf86InstallSIGIOHandler() for their fd on DEVICE_ON. That
function does not actually enable the signal if it was blocked to begin
with. As a result, if one vt-switches away from the server (SIGIO is
blocked) and then triggers a server regeneration, the signal remains
blocked and input devices are dead.

Avoid this by always unblocking SIGIO when we start the server.

X.Org Bug 50957 http://bugs.freedesktop.org/show_bug.cgi?id=50957

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 hw/xfree86/common/xf86Init.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index ca6efd4..526b95d 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -964,6 +964,7 @@ OsVendorInit(void)
 }
 #endif
 #endif
+xf86UnblockSIGIO(0);
 
 beenHere = TRUE;
 }
-- 
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


[PATCH 2/5] os: add OsBlockSIGIO and OsReleaseSIGIO

2012-07-03 Thread Peter Hutterer
Let the dix be in charge of changing the sigprocmask so we only have one
entity that changes it.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/os.h |6 +++
 os/utils.c   |   54 +--
 test/Makefile.am |3 +-
 test/os.c|  130 ++
 4 files changed, 189 insertions(+), 4 deletions(-)
 create mode 100644 test/os.c

diff --git a/include/os.h b/include/os.h
index 276eb52..8540b82 100644
--- a/include/os.h
+++ b/include/os.h
@@ -333,6 +333,12 @@ OsBlockSignals(void);
 extern _X_EXPORT void
 OsReleaseSignals(void);
 
+extern _X_EXPORT int
+OsBlockSIGIO(void);
+
+extern _X_EXPORT void
+OsReleaseSIGIO(void);
+
 extern _X_EXPORT void
 OsAbort(void)
 _X_NORETURN;
diff --git a/os/utils.c b/os/utils.c
index 3a1ef93..55f58b9 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -1165,15 +1165,15 @@ OsBlockSignals(void)
 if (BlockedSignalCount++ == 0) {
 sigset_t set;
 
+#ifdef SIGIO
+OsBlockSIGIO();
+#endif
 sigemptyset(set);
 sigaddset(set, SIGALRM);
 sigaddset(set, SIGVTALRM);
 #ifdef SIGWINCH
 sigaddset(set, SIGWINCH);
 #endif
-#ifdef SIGIO
-sigaddset(set, SIGIO);
-#endif
 sigaddset(set, SIGTSTP);
 sigaddset(set, SIGTTIN);
 sigaddset(set, SIGTTOU);
@@ -1183,12 +1183,60 @@ OsBlockSignals(void)
 #endif
 }
 
+#ifdef SIG_BLOCK
+static sig_atomic_t sigio_blocked;
+#endif
+
+/**
+ * returns zero if this call caused SIGIO to be blocked now, non-zero if it
+ * was already blocked by a previous call to this function.
+ */
+int
+OsBlockSIGIO(void)
+{
+#ifdef SIGIO
+#ifdef SIG_BLOCK
+if (sigio_blocked++ == 0) {
+sigset_t set, old;
+int ret;
+
+sigemptyset(set);
+sigaddset(set, SIGIO);
+sigprocmask(SIG_BLOCK, set, old);
+ret = sigismember(old, SIGIO);
+return ret;
+} else
+return 1;
+#endif
+#endif
+}
+
+void
+OsReleaseSIGIO(void)
+{
+#ifdef SIGIO
+#ifdef SIG_BLOCK
+if (--sigio_blocked == 0) {
+sigset_t set;
+
+sigemptyset(set);
+sigaddset(set, SIGIO);
+sigprocmask(SIG_UNBLOCK, set, NULL);
+} else if (sigio_blocked  0) {
+BUG_WARN(sigio_blocked  0);
+sigio_blocked = 0;
+}
+#endif
+#endif
+}
+
 void
 OsReleaseSignals(void)
 {
 #ifdef SIG_BLOCK
 if (--BlockedSignalCount == 0) {
 sigprocmask(SIG_SETMASK, PreviousSignalMask, 0);
+OsReleaseSIGIO();
 }
 #endif
 }
diff --git a/test/Makefile.am b/test/Makefile.am
index b2a53aa..436351c 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -5,7 +5,7 @@ if XORG
 # Tests that require at least some DDX functions in order to fully link
 # For now, requires xf86 ddx, could be adjusted to use another
 SUBDIRS += xi2
-noinst_PROGRAMS += xkb input xtest misc fixes xfree86  hashtabletest
+noinst_PROGRAMS += xkb input xtest misc fixes xfree86  hashtabletest os
 endif
 check_LTLIBRARIES = libxservertest.la
 
@@ -37,6 +37,7 @@ fixes_LDADD=$(TEST_LDADD)
 xfree86_LDADD=$(TEST_LDADD)
 touch_LDADD=$(TEST_LDADD)
 hashtabletest_LDADD=$(TEST_LDADD) $(top_srcdir)/Xext/hashtable.c
+os_LDADD=$(TEST_LDADD)
 
 libxservertest_la_LIBADD = $(XSERVER_LIBS)
 if XORG
diff --git a/test/os.c b/test/os.c
new file mode 100644
index 000..1460a34
--- /dev/null
+++ b/test/os.c
@@ -0,0 +1,130 @@
+/**
+ * Copyright © 2012 Red Hat, Inc.
+ *
+ *  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 and this permission notice (including the next
+ *  paragraph) 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
+ *  THE AUTHORS OR COPYRIGHT HOLDERS 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.
+ */
+
+#ifdef HAVE_DIX_CONFIG_H
+#include dix-config.h
+#endif
+
+#include signal.h
+#include os.h
+
+static int
+sig_is_blocked(int sig)
+{
+sigset_t current;
+
+sigemptyset(current);
+assert(sigprocmask(SIG_BLOCK, NULL, current) == 0);
+return sigismember(current, sig);
+}
+
+static void block_sigio_test(void)
+{
+#ifdef SIG_BLOCK
+sigset_t current;
+
+sigemptyset(current);
+

[PATCH 3/5] xfree86: use OsBlockSIGIO from the ddx

2012-07-03 Thread Peter Hutterer
We can ignore the wasset argument now since the DIX will keep proper
refcounting.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 hw/xfree86/os-support/shared/sigio.c |   17 ++---
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/hw/xfree86/os-support/shared/sigio.c 
b/hw/xfree86/os-support/shared/sigio.c
index 12ae8a4..e9c1c0e 100644
--- a/hw/xfree86/os-support/shared/sigio.c
+++ b/hw/xfree86/os-support/shared/sigio.c
@@ -259,26 +259,13 @@ xf86RemoveSIGIOHandler(int fd)
 int
 xf86BlockSIGIO(void)
 {
-sigset_t set, old;
-int ret;
-
-sigemptyset(set);
-sigaddset(set, SIGIO);
-sigprocmask(SIG_BLOCK, set, old);
-ret = sigismember(old, SIGIO);
-return ret;
+return OsBlockSIGIO();
 }
 
 void
 xf86UnblockSIGIO(int wasset)
 {
-sigset_t set;
-
-if (!wasset) {
-sigemptyset(set);
-sigaddset(set, SIGIO);
-sigprocmask(SIG_UNBLOCK, set, NULL);
-}
+OsReleaseSIGIO();
 }
 
 void
-- 
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


[PATCH 4/5] xfree86: drop ddx-specific SIGIO blocking

2012-07-03 Thread Peter Hutterer
The hooks are left for this cycle, we can drop it next cycle once the
drivers that need it (e.g. wacom) have been updated.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 hw/xfree86/common/xf86Cursor.c   |   12 +---
 hw/xfree86/common/xf86Events.c   |   14 +++---
 hw/xfree86/common/xf86Init.c |   10 +-
 hw/xfree86/common/xf86PM.c   |   12 +---
 hw/xfree86/os-support/shared/sigio.c |7 +++
 5 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/hw/xfree86/common/xf86Cursor.c b/hw/xfree86/common/xf86Cursor.c
index c01cfd1..65a9e82 100644
--- a/hw/xfree86/common/xf86Cursor.c
+++ b/hw/xfree86/common/xf86Cursor.c
@@ -199,7 +199,7 @@ xf86SwitchMode(ScreenPtr pScreen, DisplayModePtr mode)
 ScrnInfoPtr pScr = xf86ScreenToScrn(pScreen);
 ScreenPtr pCursorScreen;
 Bool Switched;
-int px, py, was_blocked;
+int px, py;
 DeviceIntPtr dev, it;
 
 if (!pScr-vtSema || !mode || !pScr-SwitchMode)
@@ -228,7 +228,7 @@ xf86SwitchMode(ScreenPtr pScreen, DisplayModePtr mode)
 if (pScreen == pCursorScreen)
 miPointerGetPosition(dev, px, py);
 
-was_blocked = xf86BlockSIGIO();
+OsBlockSIGIO();
 Switched = (*pScr-SwitchMode) (pScr, mode);
 if (Switched) {
 pScr-currentMode = mode;
@@ -267,7 +267,7 @@ xf86SwitchMode(ScreenPtr pScreen, DisplayModePtr mode)
 pScr-frameY1 = pScr-virtualY - 1;
 }
 }
-xf86UnblockSIGIO(was_blocked);
+OsReleaseSIGIO();
 
 if (pScr-AdjustFrame)
 (*pScr-AdjustFrame) (pScr, pScr-frameX0, pScr-frameY0);
@@ -469,13 +469,11 @@ xf86CrossScreen(ScreenPtr pScreen, Bool entering)
 static void
 xf86WarpCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y)
 {
-int sigstate;
-
-sigstate = xf86BlockSIGIO();
+OsBlockSIGIO();
 miPointerWarpCursor(pDev, pScreen, x, y);
 
 xf86Info.currentScreen = pScreen;
-xf86UnblockSIGIO(sigstate);
+OsReleaseSIGIO();
 }
 
 void *
diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c
index 4fcad40..47429ec 100644
--- a/hw/xfree86/common/xf86Events.c
+++ b/hw/xfree86/common/xf86Events.c
@@ -254,7 +254,7 @@ xf86Wakeup(pointer blockData, int err, pointer pReadmask)
 while (pInfo) {
 if (pInfo-read_input  pInfo-fd = 0 
 (FD_ISSET(pInfo-fd, devicesWithInput) != 0)) {
-int sigstate = xf86BlockSIGIO();
+OsBlockSIGIO();
 
 /*
  * Remove the descriptior from the set because more than 
one
@@ -263,7 +263,7 @@ xf86Wakeup(pointer blockData, int err, pointer pReadmask)
 FD_CLR(pInfo-fd, devicesWithInput);
 
 pInfo-read_input(pInfo);
-xf86UnblockSIGIO(sigstate);
+OsReleaseSIGIO();
 }
 pInfo = pInfo-next;
 }
@@ -397,9 +397,9 @@ xf86ReleaseKeys(DeviceIntPtr pDev)
 for (i = keyc-xkbInfo-desc-min_key_code;
  i  keyc-xkbInfo-desc-max_key_code; i++) {
 if (key_is_down(pDev, i, KEY_POSTED)) {
-sigstate = xf86BlockSIGIO();
+OsBlockSIGIO();
 QueueKeyboardEvents(pDev, KeyRelease, i, NULL);
-xf86UnblockSIGIO(sigstate);
+OsReleaseSIGIO();
 }
 }
 }
@@ -457,7 +457,7 @@ xf86VTSwitch(void)
 }
 }
 
-prevSIGIO = xf86BlockSIGIO();
+OsBlockSIGIO();
 for (i = 0; i  xf86NumScreens; i++)
 xf86Screens[i]-LeaveVT(xf86Screens[i]);
 
@@ -492,7 +492,7 @@ xf86VTSwitch(void)
 for (ih = InputHandlers; ih; ih = ih-next)
 xf86EnableInputHandler(ih);
 
-xf86UnblockSIGIO(prevSIGIO);
+OsReleaseSIGIO();
 
 }
 else {
@@ -549,7 +549,7 @@ xf86VTSwitch(void)
 for (ih = InputHandlers; ih; ih = ih-next)
 xf86EnableInputHandler(ih);
 
-xf86UnblockSIGIO(prevSIGIO);
+OsReleaseSIGIO();
 }
 }
 
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 526b95d..0f8f810 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -394,7 +394,7 @@ InstallSignalHandlers(void)
 void
 InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
 {
-int i, j, k, scr_index, was_blocked = 0;
+int i, j, k, scr_index;
 char **modulelist;
 pointer *optionlist;
 Pix24Flags screenpix24, pix24;
@@ -806,7 +806,7 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
 ioctl(xf86Info.consoleFd, VT_RELDISP, VT_ACKACQ);
 #endif
 xf86AccessEnter();
-was_blocked = xf86BlockSIGIO();
+OsBlockSIGIO();
 }
 }
 
@@ -879,7 +879,7 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
 }
 
 xf86VGAarbiterWrapFunctions();
-xf86UnblockSIGIO(was_blocked);
+OsReleaseSIGIO();
 
 

Re: [PATCH xorg-gtest 1/9] Add a few linebreaks into the standard error notice

2012-07-03 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net

These patches all seem innocuous enough;

For the series, Reviewed-by: Keith Packard kei...@keithp.com


-- 
keith.pack...@intel.com


pgpPT88BkAcHC.pgp
Description: PGP signature
___
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/5] xfree86: use OsBlockSIGIO from the ddx

2012-07-03 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

 We can ignore the wasset argument now since the DIX will keep proper
 refcounting.

Ok, so I was thinking that we should change the API here, but really
what we should do is plan to change users to use
OsBlockSIGIO/OsReleaseSIGIO directly and plan on deprecating these
interfaces after 1.13 ships. Seems reasonable?

-- 
keith.pack...@intel.com


pgp2W8umNGAdv.pgp
Description: PGP signature
___
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] Fix alphamap interactions with wfb

2012-07-03 Thread Arcady Goldmints-Orlov
Set a destructor function on pixman images and call fbFinishAccess()
from there, rather than directly from free_pixman_pict(). This ensures
that fbFinishAccess() gets called even if pixman still has a reference
to the image after free_pixman_pict(), as is the case for alphamaps.

Reviewed-by: Aaron Plattner aplatt...@nvidia.com
Signed-off-by: Arcady Goldmints-Orlov arca...@nvidia.com
---
 fb/fbpict.c |   14 --
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/fb/fbpict.c b/fb/fbpict.c
index 097a1a6..672b46f 100644
--- a/fb/fbpict.c
+++ b/fb/fbpict.c
@@ -207,6 +207,12 @@ static pixman_image_t *image_from_pict_internal(PicturePtr 
pict, Bool has_clip,
 int *xoff, int *yoff,
 Bool is_alpha_map);
 
+static void image_destroy(pixman_image_t *image, void *data)
+{
+DrawablePtr pDrawable = data;
+fbFinishAccess(pDrawable);
+}
+
 static void
 set_image_properties(pixman_image_t * image, PicturePtr pict, Bool has_clip,
  int *xoff, int *yoff, Bool is_alpha_map)
@@ -291,6 +297,10 @@ set_image_properties(pixman_image_t * image, PicturePtr 
pict, Bool has_clip,
 break;
 }
 
+if (pict-pDrawable)
+pixman_image_set_destroy_function(image, image_destroy,
+  pict-pDrawable);
+
 pixman_image_set_filter(image, filter,
 (pixman_fixed_t *) pict-filter_params,
 pict-filter_nparams);
@@ -343,8 +353,8 @@ image_from_pict(PicturePtr pict, Bool has_clip, int *xoff, 
int *yoff)
 void
 free_pixman_pict(PicturePtr pict, pixman_image_t * image)
 {
-if (image  pixman_image_unref(image)  pict-pDrawable)
-fbFinishAccess(pict-pDrawable);
+if (image)
+pixman_image_unref(image);
 }
 
 Bool
-- 
1.7.2.3
nvpublic

___
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/5] os: add OsBlockSIGIO and OsReleaseSIGIO

2012-07-03 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

  void
  OsReleaseSignals(void)
  {
  #ifdef SIG_BLOCK
  if (--BlockedSignalCount == 0) {
  sigprocmask(SIG_SETMASK, PreviousSignalMask, 0);
 +OsReleaseSIGIO();
  }
  #endif

Should you use SIG_UNBLOCK instead of SIG_SETMASK? It took me several
minutes to figure out that SIG_SETMASK will work because
PreviousSignalMask *always* contains SIGIO as it is set after
the call to OsBlockSIGIO in OsBlockSignals. Either that, or a comment
explaining why this works so future-me doesn't get confused again...

-- 
keith.pack...@intel.com


pgpy54Y9H0LrV.pgp
Description: PGP signature
___
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] unified sigio handling

2012-07-03 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

 I left the first patch as-is because it's easy to cherry-pick to 1.12 this
 way. I also left the xf86BlockSIGIO() calls in though I don't think outside
 of the ddx actually uses them. We can probably drop them, either this cycle
 or the next.

I like the whole series, except where OsReleaseSignals confused me.

For the series:

Reviewed-by: Keith Packard kei...@keithp.com

-- 
keith.pack...@intel.com


pgpeKJ1KdYdhJ.pgp
Description: PGP signature
___
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 xorg-gtest 00/16] adding a bunch of functionality

2012-07-03 Thread Peter Hutterer

This branch is available on 
git://people.freedesktop.org/~whot/xorg-gtest :devel

The main features here are adding a new XServer class that controls most of
the server-related bits. The current API is maintained, the bits just move
into different classes so existing tests should continue to work (the
xserver tests are happy).

What this new class gives us though is a simple way of starting a server
from within a test, and hooking the test up to that server.

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


[PATCH xorg-gtest 01/16] Add a new class representing the X server

2012-07-03 Thread Peter Hutterer
This class is a Process, so it's a drop-in replacement for the current
Environment, but it provides a few methods to talk to the server being
started.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 configure.ac|2 +-
 include/Makefile.am |1 +
 include/xorg/gtest/xorg-gtest-xserver.h |  113 ++
 include/xorg/gtest/xorg-gtest.h |1 +
 src/Makefile.am |1 +
 src/environment.cpp |   23 ++--
 src/xorg-gtest-all.cpp  |1 +
 src/xserver.cpp |  192 +++
 8 files changed, 323 insertions(+), 11 deletions(-)
 create mode 100644 include/xorg/gtest/xorg-gtest-xserver.h
 create mode 100644 src/xserver.cpp

diff --git a/configure.ac b/configure.ac
index 6e9e458..e46cc0b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,7 +25,7 @@ XORG_DEFAULT_OPTIONS
 XORG_ENABLE_INTEGRATION_TESTS([yes])
 XORG_WITH_DOXYGEN
 
-PKG_CHECK_MODULES(X11, x11)
+PKG_CHECK_MODULES(X11, x11 xi)
  
 # Check for Google Test
 CHECK_GTEST
diff --git a/include/Makefile.am b/include/Makefile.am
index 9ff5f2a..4a6ce03 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -34,6 +34,7 @@ nobase_include_HEADERS = \
xorg/gtest/xorg-gtest-environment.h \
xorg/gtest/xorg-gtest-process.h \
xorg/gtest/xorg-gtest-test.h \
+   xorg/gtest/xorg-gtest-xserver.h \
xorg/gtest/evemu/xorg-gtest-device.h \
xorg/gtest/xorg-gtest.h \
$(compat_headers)
diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
new file mode 100644
index 000..78f72ce
--- /dev/null
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -0,0 +1,113 @@
+/***
+ *
+ * X testing environment - Google Test helper class to communicate with the
+ * server
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * 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 and this permission notice (including the next
+ * paragraph) 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 THE
+ * AUTHORS OR COPYRIGHT HOLDERS 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.
+ *
+ 
**/
+
+
+#ifndef XORG_GTEST_XSERVER_H
+#define XORG_GTEST_XSERVER_H
+
+#include gtest/gtest.h
+#include xorg/gtest/xorg-gtest.h
+#include X11/Xlib.h
+
+namespace xorg {
+namespace testing {
+
+/**
+ * @class XServer xorg-gtest_xserver.h xorg/gtest/xorg-gtest_xserver.h
+ *
+ * Miscellaneous interfaces to communicate with the X server.
+ */
+class XServer : public xorg::testing::Process {
+  public:
+XServer();
+
+/**
+ * @param [in] display_number The display number the server runs on
+ */
+void SetDisplayNumber(unsigned int display_number);
+
+/**
+ * Get the display string that may be used for XOpenDisplay to this
+ * server. This string is effectively :display_number.
+ *
+ * @return The display string used for XOpenDisplay() to this server.
+ */
+const char* GetDisplayString(void);
+
+/**
+ * Wait for a specific device to be added to the server.
+ *
+ * @param [in] display The X display connection
+ * @param [in] nameThe name of the device to wait for
+ * @param [in] timeout The timeout in milliseconds
+ *
+ * @return Whether the device was added
+ */
+static bool WaitForDevice(::Display *display, const std::string name, 
time_t timeout = 1000);
+
+/**
+ * Wait for an event on the X connection.
+ *
+ * @param [in] display The X display connection
+ * @param [in] timeout The timeout in milliseconds
+ *
+ * @return Whether an event is available
+ */
+static bool WaitForEvent(::Display *display, time_t timeout = 1000);
+
+/**
+ * Wait for an event of a specific type on the X connection.
+ *
+ * All events preceding the matching event are discarded. If no event was 
found
+ * before the timeout 

[PATCH xorg-gtest 03/16] xserver: store config, logfile, binary paths in the XServer object

2012-07-03 Thread Peter Hutterer
And initialise with the default settings

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |   12 
 src/environment.cpp |4 +++-
 src/xserver.cpp |   26 +-
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index c62970d..c5cb32d 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -55,6 +55,18 @@ class XServer : public xorg::testing::Process {
  * @param [in] display_number The display number the server runs on
  */
 void SetDisplayNumber(unsigned int display_number);
+/**
+ * @param [in] path_to_logfile The path to the log file
+ */
+void SetLogfilePath(std::string path_to_logfile);
+/**
+ * @param [in] path_to_conf The path to the xorg.conf file
+ */
+void SetConfigPath(std::string path_to_conf);
+/**
+ * @param [in] path_to_server The path to the binary
+ */
+void SetServerPath(std::string path_to_server);
 
 /**
  * Get the display string that may be used for XOpenDisplay to this
diff --git a/src/environment.cpp b/src/environment.cpp
index 5e7a156..2b3abca 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -141,13 +141,15 @@ void xorg::testing::Environment::SetUp() {
 throw std::runtime_error(message);
   }
 
+  d_-server.SetDisplayNumber(d_-display);
+  d_-server.SetLogfilePath(d_-path_to_log_file);
+  d_-server.SetConfigPath(d_-path_to_conf);
   d_-server.Start(d_-path_to_server, d_-path_to_server.c_str(),
 display_string,
 -logverbose, 10,
 -logfile, d_-path_to_log_file.c_str(),
 -config, d_-path_to_conf.c_str(),
 NULL);
-  d_-server.SetDisplayNumber(d_-display);
   d_-server.WaitForConnections();
 
   Process::SetEnv(DISPLAY, display_string, true);
diff --git a/src/xserver.cpp b/src/xserver.cpp
index ba847f1..7f0483b 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -27,6 +27,7 @@
  
**/
 
 #include xorg/gtest/xorg-gtest_xserver.h
+#include defines.h
 
 #include sys/types.h
 #include sys/wait.h
@@ -45,8 +46,19 @@
 #include X11/extensions/XInput2.h
 
 struct xorg::testing::XServer::Private {
+  Private()
+  : display_number(DEFAULT_DISPLAY),
+display_string(),
+path_to_logfile(DEFAULT_XORG_LOGFILE),
+path_to_conf(DUMMY_CONF_PATH),
+path_to_server(DEFAULT_XORG_SERVER) {
+  }
+
   unsigned int display_number;
   std::string display_string;
+  std::string path_to_logfile;
+  std::string path_to_conf;
+  std::string path_to_server;
 };
 
 xorg::testing::XServer::XServer() : d_(new Private) {
@@ -65,6 +77,18 @@ const char* xorg::testing::XServer::GetDisplayString(void) {
   return d_-display_string.c_str();
 }
 
+void xorg::testing::XServer::SetLogfilePath(std::string path_to_logfile) {
+d_-path_to_logfile = path_to_logfile;
+}
+
+void xorg::testing::XServer::SetConfigPath(std::string path_to_conf) {
+d_-path_to_conf = path_to_conf;
+}
+
+void xorg::testing::XServer::SetServerPath(std::string path_to_server) {
+d_-path_to_server = path_to_server;
+}
+
 bool xorg::testing::XServer::WaitForEvent(::Display *display, time_t timeout)
 {
 fd_set fds;
@@ -210,7 +234,7 @@ void xorg::testing::XServer::WaitForConnections(void) {
   message += . Ensure that the \dummy\ video driver is installed.\n
  If the X.org server is older than 1.12, 
  tests will need to be run as root.\nCheck ;
-  //message += d_-path_to_log_file;
+  message += d_-path_to_logfile;
   message +=  for any errors;
   throw std::runtime_error(message);
 } else if (pid == 0) {
-- 
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


[PATCH xorg-gtest 02/16] xserver: add WaitForConnections()

2012-07-03 Thread Peter Hutterer
Moved from Environment to XServer class

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |5 +
 src/environment.cpp |   34 +-
 src/xserver.cpp |   35 +++
 3 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 78f72ce..c62970d 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -47,6 +47,11 @@ class XServer : public xorg::testing::Process {
 XServer();
 
 /**
+ * Waits until this server is ready to take connections.
+ */
+void WaitForConnections(void);
+
+/**
  * @param [in] display_number The display number the server runs on
  */
 void SetDisplayNumber(unsigned int display_number);
diff --git a/src/environment.cpp b/src/environment.cpp
index 43e1e70..5e7a156 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -147,42 +147,10 @@ void xorg::testing::Environment::SetUp() {
 -logfile, d_-path_to_log_file.c_str(),
 -config, d_-path_to_conf.c_str(),
 NULL);
-
   d_-server.SetDisplayNumber(d_-display);
+  d_-server.WaitForConnections();
 
   Process::SetEnv(DISPLAY, display_string, true);
-
-  for (int i = 0; i  10; ++i) {
-test_display = XOpenDisplay(NULL);
-
-if (test_display) {
-  XCloseDisplay(test_display);
-  return;
-}
-
-int status;
-int pid = waitpid(d_-server.Pid(), status, WNOHANG);
-if (pid == d_-server.Pid()) {
-  std::string message;
-  message += X server failed to start on display ;
-  message += display_string;
-  message += . Ensure that the \dummy\ video driver is installed.\n
- If the X.org server is older than 1.12, 
- tests will need to be run as root.\nCheck ;
-  message += d_-path_to_log_file;
-  message +=  for any errors;
-  throw std::runtime_error(message);
-} else if (pid == 0) {
-  sleep(1); /* Give the dummy X server some time to start */
-} else if (pid == -1) {
-  throw std::runtime_error(Could not get status of dummy X server 
-   process);
-} else {
-  throw std::runtime_error(Invalid child PID returned by 
Process::Wait());
-}
-  }
-
-  throw std::runtime_error(Unable to open connection to dummy X server);
 }
 
 void xorg::testing::Environment::TearDown() {
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 28130d7..ba847f1 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -41,6 +41,7 @@
 #include stdexcept
 #include vector
 
+#include X11/Xlib.h
 #include X11/extensions/XInput2.h
 
 struct xorg::testing::XServer::Private {
@@ -190,3 +191,37 @@ bool xorg::testing::XServer::WaitForDevice(::Display 
*display, const std::string
 
 return false;
 }
+
+void xorg::testing::XServer::WaitForConnections(void) {
+  for (int i = 0; i  10; ++i) {
+Display *test_display = XOpenDisplay(GetDisplayString());
+
+if (test_display) {
+  XCloseDisplay(test_display);
+  return;
+}
+
+int status;
+int pid = waitpid(Pid(), status, WNOHANG);
+if (pid == Pid()) {
+  std::string message;
+  message += X server failed to start on display ;
+  message +=  GetDisplayString();
+  message += . Ensure that the \dummy\ video driver is installed.\n
+ If the X.org server is older than 1.12, 
+ tests will need to be run as root.\nCheck ;
+  //message += d_-path_to_log_file;
+  message +=  for any errors;
+  throw std::runtime_error(message);
+} else if (pid == 0) {
+  sleep(1); /* Give the dummy X server some time to start */
+} else if (pid == -1) {
+  throw std::runtime_error(Could not get status of dummy X server 
+   process);
+} else {
+  throw std::runtime_error(Invalid child PID returned by 
Process::Wait());
+}
+  }
+
+  throw std::runtime_error(Unable to open connection to dummy X server);
+}
-- 
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


[PATCH xorg-gtest 04/16] xserver: move starting the process into the XServer object

2012-07-03 Thread Peter Hutterer
The API currently means we lose the ability to pass arbitrary options (aside
from display/logfile/conf path), but this should be of minor impact only.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |6 ++
 src/environment.cpp |7 +--
 src/xserver.cpp |9 +
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index c5cb32d..5c5ce99 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -47,6 +47,12 @@ class XServer : public xorg::testing::Process {
 XServer();
 
 /**
+ * Start a new server
+ * @param [in] program Path to the XServer binary
+ */
+void Start(std::string program);
+
+/**
  * Waits until this server is ready to take connections.
  */
 void WaitForConnections(void);
diff --git a/src/environment.cpp b/src/environment.cpp
index 2b3abca..7ed23b3 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -144,12 +144,7 @@ void xorg::testing::Environment::SetUp() {
   d_-server.SetDisplayNumber(d_-display);
   d_-server.SetLogfilePath(d_-path_to_log_file);
   d_-server.SetConfigPath(d_-path_to_conf);
-  d_-server.Start(d_-path_to_server, d_-path_to_server.c_str(),
-display_string,
--logverbose, 10,
--logfile, d_-path_to_log_file.c_str(),
--config, d_-path_to_conf.c_str(),
-NULL);
+  d_-server.Start(d_-path_to_server);
   d_-server.WaitForConnections();
 
   Process::SetEnv(DISPLAY, display_string, true);
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 7f0483b..38394f3 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -249,3 +249,12 @@ void xorg::testing::XServer::WaitForConnections(void) {
 
   throw std::runtime_error(Unable to open connection to dummy X server);
 }
+
+void xorg::testing::XServer::Start(std::string program) {
+  Process::Start(program, program.c_str(),
+ GetDisplayString(),
+ -logverbose, 10,
+ -logfile, d_-path_to_logfile.c_str(),
+ -config, d_-path_to_conf.c_str(),
+ NULL);
+}
-- 
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


[PATCH xorg-gtest 05/16] xserver: move testing startup to the XServer object

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |2 ++
 src/environment.cpp |   35 ---
 src/xserver.cpp |   40 +++
 3 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 5c5ce99..52a2fd0 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -129,6 +129,8 @@ class XServer : public xorg::testing::Process {
 XServer(const XServer);
 XServer operator=(const XServer);
 
+void TestStartup(void);
+
 };
 } // namespace testing
 } // namespace xorg
diff --git a/src/environment.cpp b/src/environment.cpp
index 7ed23b3..69972a4 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -106,41 +106,6 @@ void xorg::testing::Environment::SetUp() {
   static char display_string[6];
   snprintf(display_string, 6, :%d, d_-display);
 
-  Display* test_display = XOpenDisplay(display_string);
-  if (test_display) {
-XCloseDisplay(test_display);
-std::string message;
-message += A server is already running on ;
-message += display_string;
-message += .;
-throw std::runtime_error(message);
-  }
-
-  /* The Xorg server won't start unless the log file and the old log file are
-   * writable. */
-  std::ofstream log_test;
-  log_test.open(d_-path_to_log_file.c_str(), std::ofstream::out);
-  log_test.close();
-  if (log_test.fail()) {
-std::string message;
-message += X.org server log file ;
-message += d_-path_to_log_file;
-message +=  is not writable.;
-throw std::runtime_error(message);
-  }
-
-  std::string old_log_file = d_-path_to_log_file.c_str();
-  old_log_file += .old;
-  log_test.open(old_log_file.c_str(), std::ofstream::out);
-  log_test.close();
-  if (log_test.fail()) {
-std::string message;
-message += X.org old server log file ;
-message += old_log_file;
-message +=  is not writable.;
-throw std::runtime_error(message);
-  }
-
   d_-server.SetDisplayNumber(d_-display);
   d_-server.SetLogfilePath(d_-path_to_log_file);
   d_-server.SetConfigPath(d_-path_to_conf);
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 38394f3..1a46dbb 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -41,6 +41,7 @@
 #include cstring
 #include stdexcept
 #include vector
+#include fstream
 
 #include X11/Xlib.h
 #include X11/extensions/XInput2.h
@@ -250,7 +251,46 @@ void xorg::testing::XServer::WaitForConnections(void) {
   throw std::runtime_error(Unable to open connection to dummy X server);
 }
 
+void xorg::testing::XServer::TestStartup(void) {
+  Display* test_display = XOpenDisplay(GetDisplayString());
+  if (test_display) {
+XCloseDisplay(test_display);
+std::string message;
+message += A server is already running on ;
+message += GetDisplayString();
+message += .;
+throw std::runtime_error(message);
+  }
+
+  /* The Xorg server won't start unless the log file and the old log file are
+   * writable. */
+  std::ofstream log_test;
+  log_test.open(d_-path_to_logfile.c_str(), std::ofstream::out);
+  log_test.close();
+  if (log_test.fail()) {
+std::string message;
+message += X.org server log file ;
+message += d_-path_to_logfile;
+message +=  is not writable.;
+throw std::runtime_error(message);
+  }
+
+  std::string old_log_file = d_-path_to_logfile.c_str();
+  old_log_file += .old;
+  log_test.open(old_log_file.c_str(), std::ofstream::out);
+  log_test.close();
+  if (log_test.fail()) {
+std::string message;
+message += X.org old server log file ;
+message += old_log_file;
+message +=  is not writable.;
+throw std::runtime_error(message);
+  }
+
+}
+
 void xorg::testing::XServer::Start(std::string program) {
+  TestStartup();
   Process::Start(program, program.c_str(),
  GetDisplayString(),
  -logverbose, 10,
-- 
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


[PATCH xorg-gtest 07/16] xserver: add Start() for default binary path

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |4 
 src/xserver.cpp |4 
 2 files changed, 8 insertions(+)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 821b01f..1cd 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -47,6 +47,10 @@ class XServer : public xorg::testing::Process {
 XServer();
 
 /**
+ * Start a new server, using the default binary
+ */
+void Start(void);
+/**
  * Start a new server
  * @param [in] program Path to the XServer binary
  */
diff --git a/src/xserver.cpp b/src/xserver.cpp
index bd1e2f9..160eadc 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -289,6 +289,10 @@ void xorg::testing::XServer::TestStartup(void) {
 
 }
 
+void xorg::testing::XServer::Start(void) {
+  Start(d_-path_to_server);
+}
+
 void xorg::testing::XServer::Start(std::string program) {
   TestStartup();
   Process::Start(program, program.c_str(),
-- 
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


[PATCH xorg-gtest 06/16] xserver: move Terminate and Kill handling here

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |   13 
 src/environment.cpp |   31 +++-
 src/xserver.cpp |   34 +++
 3 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 52a2fd0..821b01f 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -53,6 +53,19 @@ class XServer : public xorg::testing::Process {
 void Start(std::string program);
 
 /**
+ * Terminates this server process. Will signal the server to terminate
+ * multiple times before giving up.
+ *
+ * @return false if the server did not terminate, true otherwise
+ */
+bool Terminate(void);
+
+/**
+ * Kills the server. With a vengeance.
+ */
+bool Kill(void);
+
+/**
  * Waits until this server is ready to take connections.
  */
 void WaitForConnections(void);
diff --git a/src/environment.cpp b/src/environment.cpp
index 69972a4..b041236 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -116,35 +116,10 @@ void xorg::testing::Environment::SetUp() {
 }
 
 void xorg::testing::Environment::TearDown() {
-  if (d_-server.Terminate()) {
-for (int i = 0; i  10; i++) {
-  int status;
-  int pid = waitpid(d_-server.Pid(), status, WNOHANG);
-
-  if (pid == d_-server.Pid())
-return;
-
-  sleep(1); /* Give the dummy X server more time to shut down */
-}
-  }
-
-  Kill();
+  if (!d_-server.Terminate())
+Kill();
 }
 
 void xorg::testing::Environment::Kill() {
-  if (!d_-server.Kill())
-std::cerr  Warning: Failed to kill dummy Xorg server: 
-   std::strerror(errno)  \n;
-
-  for (int i = 0; i  10; i++) {
-int status;
-int pid = waitpid(d_-server.Pid(), status, WNOHANG);
-
-if (pid == d_-server.Pid())
-  return;
-
-  sleep(1); /* Give the dummy X server more time to shut down */
-  }
-
-  std::cerr  Warning: Dummy X server did not shut down\n;
+  d_-server.Kill();
 }
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 1a46dbb..bd1e2f9 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -298,3 +298,37 @@ void xorg::testing::XServer::Start(std::string program) {
  -config, d_-path_to_conf.c_str(),
  NULL);
 }
+
+bool xorg::testing::XServer::Terminate(void) {
+  if (Process::Terminate()) {
+for (int i = 0; i  10; i++) {
+  int status;
+  int pid = waitpid(Pid(), status, WNOHANG);
+
+  if (pid == Pid())
+return true;
+
+  sleep(1); /* Give the dummy X server more time to shut down */
+}
+  }
+  return false;
+}
+
+bool xorg::testing::XServer::Kill(void) {
+  if (!Process::Kill())
+std::cerr  Warning: Failed to kill dummy Xorg server: 
+   std::strerror(errno)  \n;
+
+  for (int i = 0; i  10; i++) {
+int status;
+int pid = waitpid(Pid(), status, WNOHANG);
+
+if (pid == Pid())
+  return true;
+
+  sleep(1); /* Give the dummy X server more time to shut down */
+  }
+
+  std::cerr  Warning: Dummy X server did not shut down\n;
+  return false;
+}
-- 
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


[PATCH xorg-gtest 08/16] xserver: return used display number from Start()

2012-07-03 Thread Peter Hutterer
Doesn't do much yet, but once we support -displayfd we can hook it up here.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |7 +--
 src/xserver.cpp |9 ++---
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 1cd..5cab01b 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -48,13 +48,16 @@ class XServer : public xorg::testing::Process {
 
 /**
  * Start a new server, using the default binary
+ *
+ * @return The display number this server is running on
  */
-void Start(void);
+unsigned int Start(void);
 /**
  * Start a new server
  * @param [in] program Path to the XServer binary
+ * @return The display number this server is running on
  */
-void Start(std::string program);
+unsigned int Start(std::string program);
 
 /**
  * Terminates this server process. Will signal the server to terminate
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 160eadc..f5fd245 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -289,11 +289,11 @@ void xorg::testing::XServer::TestStartup(void) {
 
 }
 
-void xorg::testing::XServer::Start(void) {
-  Start(d_-path_to_server);
+unsigned int xorg::testing::XServer::Start(void) {
+  return Start(d_-path_to_server);
 }
 
-void xorg::testing::XServer::Start(std::string program) {
+unsigned int xorg::testing::XServer::Start(std::string program) {
   TestStartup();
   Process::Start(program, program.c_str(),
  GetDisplayString(),
@@ -301,6 +301,9 @@ void xorg::testing::XServer::Start(std::string program) {
  -logfile, d_-path_to_logfile.c_str(),
  -config, d_-path_to_conf.c_str(),
  NULL);
+
+  /* FIXME: use -displayfd here once the released servers support it */
+  return d_-display_number;
 }
 
 bool xorg::testing::XServer::Terminate(void) {
-- 
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


[PATCH xorg-gtest 09/16] environment: remove default settings

2012-07-03 Thread Peter Hutterer
Keep those in the server only, not the environment. And only override the
build-in ones when they've been set by main.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/environment.cpp |   22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/environment.cpp b/src/environment.cpp
index b041236..01b2148 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -28,7 +28,6 @@
 #include xorg/gtest/xorg-gtest-environment.h
 #include xorg/gtest/xorg-gtest-process.h
 #include xorg/gtest/xorg-gtest-xserver.h
-#include defines.h
 
 #include sys/types.h
 #include unistd.h
@@ -44,11 +43,8 @@
 #include X11/Xlib.h
 
 struct xorg::testing::Environment::Private {
-  Private()
-  : path_to_conf(DUMMY_CONF_PATH), path_to_log_file(DEFAULT_XORG_LOGFILE),
-path_to_server(DEFAULT_XORG_SERVER), display(DEFAULT_DISPLAY) {
+  Private() : display(-1) {
   }
-
   std::string path_to_conf;
   std::string path_to_log_file;
   std::string path_to_server;
@@ -103,15 +99,23 @@ int xorg::testing::Environment::display() const
 }
 
 void xorg::testing::Environment::SetUp() {
+  unsigned int display_used;
   static char display_string[6];
   snprintf(display_string, 6, :%d, d_-display);
 
-  d_-server.SetDisplayNumber(d_-display);
-  d_-server.SetLogfilePath(d_-path_to_log_file);
-  d_-server.SetConfigPath(d_-path_to_conf);
-  d_-server.Start(d_-path_to_server);
+  if (d_-display = 0)
+d_-server.SetDisplayNumber(d_-display);
+  if (d_-path_to_log_file.length())
+d_-server.SetLogfilePath(d_-path_to_log_file);
+  if (d_-path_to_conf.length())
+d_-server.SetConfigPath(d_-path_to_conf);
+  if (d_-path_to_server.length())
+display_used = d_-server.Start(d_-path_to_server);
+  else
+display_used = d_-server.Start();
   d_-server.WaitForConnections();
 
+  snprintf(display_string, 6, :%d, display_used);
   Process::SetEnv(DISPLAY, display_string, true);
 }
 
-- 
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


[PATCH xorg-gtest 10/16] xserver: replace separate paths with an option/value map

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |   18 
 src/xserver.cpp |   35 ---
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 5cab01b..707888e 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -82,10 +82,16 @@ class XServer : public xorg::testing::Process {
  */
 void SetDisplayNumber(unsigned int display_number);
 /**
+ * Set the logfile path for the server. This call is equivalent to
+ * XServer::SetOption(-logfile, path_to_logfile)
+ *
  * @param [in] path_to_logfile The path to the log file
  */
 void SetLogfilePath(std::string path_to_logfile);
 /**
+ * Set the logfile path for the server. This call is equivalent to
+ * XServer::SetOption(-config, path_to_conf)
+ *
  * @param [in] path_to_conf The path to the xorg.conf file
  */
 void SetConfigPath(std::string path_to_conf);
@@ -103,6 +109,18 @@ class XServer : public xorg::testing::Process {
 const char* GetDisplayString(void);
 
 /**
+ * Set startup options for the server. This is a more generic call
+ * to XServer::SetLogfilePath and XServer::SetConfigPath.
+ *
+ * For arguments that do not take/need a value, use the empty string as
+ * value.
+ *
+ * @param [in] key Commandline option
+ * @param [in] value Option value (if any)
+ */
+void SetOption(std::string key, std::string value);
+
+/**
  * Wait for a specific device to be added to the server.
  *
  * @param [in] display The X display connection
diff --git a/src/xserver.cpp b/src/xserver.cpp
index f5fd245..765feb1 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -42,6 +42,7 @@
 #include stdexcept
 #include vector
 #include fstream
+#include map
 
 #include X11/Xlib.h
 #include X11/extensions/XInput2.h
@@ -50,16 +51,22 @@ struct xorg::testing::XServer::Private {
   Private()
   : display_number(DEFAULT_DISPLAY),
 display_string(),
-path_to_logfile(DEFAULT_XORG_LOGFILE),
-path_to_conf(DUMMY_CONF_PATH),
 path_to_server(DEFAULT_XORG_SERVER) {
+
+std::string key, value;
+key = std::string(-config);
+value = std::string(DUMMY_CONF_PATH);
+options[key] = value;
+
+key = std::string(-logfile);
+value = std::string(DEFAULT_XORG_LOGFILE);
+options[key] = value;
   }
 
   unsigned int display_number;
   std::string display_string;
-  std::string path_to_logfile;
-  std::string path_to_conf;
   std::string path_to_server;
+  std::mapstd::string, std::string options;
 };
 
 xorg::testing::XServer::XServer() : d_(new Private) {
@@ -79,11 +86,11 @@ const char* xorg::testing::XServer::GetDisplayString(void) {
 }
 
 void xorg::testing::XServer::SetLogfilePath(std::string path_to_logfile) {
-d_-path_to_logfile = path_to_logfile;
+  SetOption(-logfile, path_to_logfile);
 }
 
 void xorg::testing::XServer::SetConfigPath(std::string path_to_conf) {
-d_-path_to_conf = path_to_conf;
+  SetOption(-config, path_to_conf);
 }
 
 void xorg::testing::XServer::SetServerPath(std::string path_to_server) {
@@ -235,7 +242,7 @@ void xorg::testing::XServer::WaitForConnections(void) {
   message += . Ensure that the \dummy\ video driver is installed.\n
  If the X.org server is older than 1.12, 
  tests will need to be run as root.\nCheck ;
-  message += d_-path_to_logfile;
+  message += d_-options[-logfile];
   message +=  for any errors;
   throw std::runtime_error(message);
 } else if (pid == 0) {
@@ -265,17 +272,17 @@ void xorg::testing::XServer::TestStartup(void) {
   /* The Xorg server won't start unless the log file and the old log file are
* writable. */
   std::ofstream log_test;
-  log_test.open(d_-path_to_logfile.c_str(), std::ofstream::out);
+  log_test.open(d_-options[-logfile].c_str(), std::ofstream::out);
   log_test.close();
   if (log_test.fail()) {
 std::string message;
 message += X.org server log file ;
-message += d_-path_to_logfile;
+message += d_-options[-logfile];
 message +=  is not writable.;
 throw std::runtime_error(message);
   }
 
-  std::string old_log_file = d_-path_to_logfile.c_str();
+  std::string old_log_file = d_-options[-logfile].c_str();
   old_log_file += .old;
   log_test.open(old_log_file.c_str(), std::ofstream::out);
   log_test.close();
@@ -298,8 +305,8 @@ unsigned int xorg::testing::XServer::Start(std::string 
program) {
   Process::Start(program, program.c_str(),
  GetDisplayString(),
  -logverbose, 10,
- -logfile, d_-path_to_logfile.c_str(),
- -config, d_-path_to_conf.c_str(),
+ -logfile, d_-options[-logfile].c_str(),
+ 

[PATCH xorg-gtest 11/16] environment: use SetOption, instead of separate calls

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/environment.cpp |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/environment.cpp b/src/environment.cpp
index 01b2148..f28bec4 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -106,9 +106,9 @@ void xorg::testing::Environment::SetUp() {
   if (d_-display = 0)
 d_-server.SetDisplayNumber(d_-display);
   if (d_-path_to_log_file.length())
-d_-server.SetLogfilePath(d_-path_to_log_file);
+d_-server.SetOption(-logfile, d_-path_to_log_file);
   if (d_-path_to_conf.length())
-d_-server.SetConfigPath(d_-path_to_conf);
+d_-server.SetOption(-config, d_-path_to_log_file);
   if (d_-path_to_server.length())
 display_used = d_-server.Start(d_-path_to_server);
   else
-- 
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


[PATCH xorg-gtest 12/16] process: add Start(program, vectorchar*)

2012-07-03 Thread Peter Hutterer
Same thing as the va_list but takes a std::vector of arguments instead.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-process.h |   16 
 src/process.cpp |   20 +---
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-process.h 
b/include/xorg/gtest/xorg-gtest-process.h
index 47aefcf..c41cd9e 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -93,6 +93,22 @@ class Process {
   /**
* Starts a program as a child process.
*
+   * See 'man execvp' for further information on the elements in
+   * the vector.
+   *
+   * @param program The program to start.
+   * @param args Vector of arguments passed to the program.
+   *
+   * @throws std::runtime_error on failure.
+   *
+   * @post If successful: Child process forked and program started.
+   * @post If successful: Subsequent calls to Pid() return child process pid.
+   */
+  void Start(const std::string program, std::vectorchar* args);
+
+  /**
+   * Starts a program as a child process.
+   *
* See 'man execvp' for further information on the variadic argument list.
*
* @param program The program to start.
diff --git a/src/process.cpp b/src/process.cpp
index e79b694..4c4a738 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -48,7 +48,7 @@ xorg::testing::Process::Process() : d_(new Private) {
   d_-pid = -1;
 }
 
-void xorg::testing::Process::Start(const std::string program, va_list args) {
+void xorg::testing::Process::Start(const std::string program, 
std::vectorchar* argv) {
   if (d_-pid != -1)
 throw std::runtime_error(Attempting to start an already started process);
 
@@ -61,18 +61,24 @@ void xorg::testing::Process::Start(const std::string 
program, va_list args) {
 close(1);
 close(2);
 
-std::vectorchar* argv;
-
-do
-  argv.push_back(va_arg(args, char*));
-while (argv.back());
-
+if (argv.back())
+  argv.push_back(NULL);
 execvp(program.c_str(), argv[0]);
 
 throw std::runtime_error(Failed to start process);
   }
 }
 
+void xorg::testing::Process::Start(const std::string program, va_list args) {
+  std::vectorchar* argv;
+
+  do {
+argv.push_back(va_arg(args, char*));
+  } while (argv.back());
+
+  Start(program, argv);
+}
+
 void xorg::testing::Process::Start(const std::string program, ...) {
   va_list list;
   va_start(list, program);
-- 
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


[PATCH xorg-gtest 14/16] xserver: update documentation

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-xserver.h |   21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 707888e..6eb2241 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -40,7 +40,26 @@ namespace testing {
 /**
  * @class XServer xorg-gtest_xserver.h xorg/gtest/xorg-gtest_xserver.h
  *
- * Miscellaneous interfaces to communicate with the X server.
+ * Class representing the X server process.
+ *
+ * @code
+ * XServer server;
+ * server.SetOption(-logfile, /tmp/Xserver.log);
+ *
+ * try {
+ *   server.Start();
+ * } catch (const std::runtime_error e) {
+ *   std::cerr  Problem starting the X server:   e.what()  std::endl;
+ * }
+ *
+ * ...
+ *
+ * if (!server.Terminate()) {
+ *   std::cerr  Problem terminating server ... killing now ...  
std::endl;
+ *   if (!server.Kill())
+ * std::cerr  Problem killing server  std::endl;
+ * }
+ * @endcode
  */
 class XServer : public xorg::testing::Process {
   public:
-- 
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


[PATCH xorg-gtest 13/16] xserver: use new Process::Start(vector) call to populate argument list

2012-07-03 Thread Peter Hutterer
There's probably some better way to do this than strdup()ing everything,
send me a patch if you know how.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/xserver.cpp |   24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/xserver.cpp b/src/xserver.cpp
index 765feb1..cc78d59 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -301,13 +301,25 @@ unsigned int xorg::testing::XServer::Start(void) {
 }
 
 unsigned int xorg::testing::XServer::Start(std::string program) {
+  std::vectorchar* args;
+  std::mapstd::string, std::string::iterator it;
+
   TestStartup();
-  Process::Start(program, program.c_str(),
- GetDisplayString(),
- -logverbose, 10,
- -logfile, d_-options[-logfile].c_str(),
- -config, d_-options[-config].c_str(),
- NULL);
+
+  args.push_back(strdup(program.c_str()));
+  args.push_back(strdup(GetDisplayString()));
+
+  for (it = d_-options.begin(); it != d_-options.end(); it++) {
+args.push_back(strdup(it-first.c_str()));
+if (it-second.length())
+  args.push_back(strdup(it-second.c_str()));
+  }
+
+  Process::Start(program, args);
+
+  std::vectorchar*::iterator vit;
+  for (vit = args.begin(); vit != args.end(); vit++)
+free(*vit);
 
   /* FIXME: use -displayfd here once the released servers support it */
   return d_-display_number;
-- 
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


[PATCH xorg-gtest 15/16] test: add SetDisplayString()

2012-07-03 Thread Peter Hutterer
Don't rely on the environment alone, take the display string if it's been
set.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/xorg-gtest-test.h |7 +++
 src/test.cpp |8 +++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/xorg/gtest/xorg-gtest-test.h 
b/include/xorg/gtest/xorg-gtest-test.h
index 3b78a17..3338c80 100644
--- a/include/xorg/gtest/xorg-gtest-test.h
+++ b/include/xorg/gtest/xorg-gtest-test.h
@@ -88,6 +88,13 @@ class Test : public ::testing::Test {
*/
   ::Display* Display() const;
 
+  /**
+   * Set the display string used for XOpenDisplay()
+   *
+   * @param display The string representing the display connection, or NULL
+   */
+  void SetDisplayString(const char *display);
+
   /** @cond Implementation */
   struct Private;
   std::auto_ptrPrivate d_;
diff --git a/src/test.cpp b/src/test.cpp
index e3e65e4..499915b 100644
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -33,16 +33,18 @@
 
 struct xorg::testing::Test::Private {
   ::Display* display;
+  const char *display_string;
 };
 
 xorg::testing::Test::Test() : d_(new Private) {
   d_-display = NULL;
+  d_-display_string = NULL;
 }
 
 xorg::testing::Test::~Test() {}
 
 void xorg::testing::Test::SetUp() {
-  d_-display = XOpenDisplay(NULL);
+  d_-display = XOpenDisplay(d_-display_string);
   if (!d_-display)
 throw std::runtime_error(Failed to open connection to display);
 }
@@ -56,3 +58,7 @@ void xorg::testing::Test::TearDown() {
 ::Display* xorg::testing::Test::Display() const {
   return d_-display;
 }
+
+void xorg::testing::Test::SetDisplayString(const char *display) {
+  d_-display_string = display;
+}
-- 
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


[PATCH xorg-gtest 16/16] Add Device::GetDeviceNode() to return device node path from an evemu device

2012-07-03 Thread Peter Hutterer
evemu doesn't export this information and even evemu-device just trawls
through the file system to print this info. So do the same here, noting the
time before evemu_create() and the ctime of the new device file. If the
latter is later than the former and the device names match, we can assume
this is our device.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 include/xorg/gtest/evemu/xorg-gtest-device.h |   13 ++
 src/device.cpp   |   65 +-
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/include/xorg/gtest/evemu/xorg-gtest-device.h 
b/include/xorg/gtest/evemu/xorg-gtest-device.h
index 5f8faa2..359e65b 100644
--- a/include/xorg/gtest/evemu/xorg-gtest-device.h
+++ b/include/xorg/gtest/evemu/xorg-gtest-device.h
@@ -75,6 +75,17 @@ class Device {
*/
   void Play(const std::string path) const;
 
+  /**
+   * Return the /dev/input/eventX device node for this device.
+   *
+   * Note that evemu doesn't know the device node, so we traverse the file
+   * system looking for it. There is a tiny chance of the device node being
+   * wrong.
+   *
+   * @return The string representing the device node
+   */
+  const char* GetDeviceNode(void);
+
  private:
   struct Private;
   std::auto_ptrPrivate d_;
@@ -82,6 +93,8 @@ class Device {
   /* Disable copy constructor  assignment operator */
   Device(const Device);
   Device operator=(const Device);
+
+  void GuessDeviceNode(time_t ctime);
 };
 
 } // namespace evemu
diff --git a/src/device.cpp b/src/device.cpp
index 226d4e0..555e3e0 100644
--- a/src/device.cpp
+++ b/src/device.cpp
@@ -28,18 +28,74 @@
 #include xorg/gtest/evemu/xorg-gtest-device.h
 
 #include fcntl.h
+#include dirent.h
 
 #include stdexcept
 
 #include gtest/gtest.h
 
+#define SYS_INPUT_DIR /sys/class/input
+#define DEV_INPUT_DIR /dev/input/
+
 struct xorg::testing::evemu::Device::Private {
-  Private() : fd(-1), device(NULL) {}
+  Private() : fd(-1), device(NULL), device_node() {}
 
   int fd;
   struct evemu_device* device;
+  std::string device_node;
 };
 
+static int _event_device_compare(const struct dirent **a,
+ const struct dirent **b) {
+  int na, nb;
+
+  sscanf((*a)-d_name, event%d, na);
+  sscanf((*b)-d_name, event%d, nb);
+
+  return (na  nb) ? 1 : (na  nb) ? -1 : 0;
+
+}
+
+static int _event_device_filter(const struct dirent *d) {
+  return (strncmp(event, d-d_name, sizeof(event) - 1) == 0);
+}
+
+void xorg::testing::evemu::Device::GuessDeviceNode(time_t ctime) {
+  struct dirent **event_devices;
+  int n_event_devices;
+
+  n_event_devices = scandir(SYS_INPUT_DIR, event_devices,
+_event_device_filter, _event_device_compare);
+
+  if (n_event_devices  0) {
+std::cerr  Failed to guess device node.  std::endl;
+return;
+  }
+
+  bool found = false;
+  for (int i = 0; i  n_event_devices  !found; i++) {
+std::stringstream s;
+s  DEV_INPUT_DIR  event_devices[i]-d_name;
+
+int fd = open(s.str().c_str(), O_RDONLY);
+char device_name[256];
+
+ioctl(fd, EVIOCGNAME(sizeof(device_name)), device_name);
+if (strcmp(device_name, evemu_get_name(d_-device)) == 0) {
+  struct stat buf;
+  if (fstat(fd, buf) == 0) {
+if (buf.st_ctime = ctime) {
+  d_-device_node = s.str();
+  found = true;
+}
+  }
+}
+close(fd);
+free(event_devices[i]);
+  }
+  free(event_devices);
+}
+
 xorg::testing::evemu::Device::Device(const std::string path)
 : d_(new Private) {
   static const char UINPUT_NODE[] = /dev/uinput;
@@ -68,11 +124,14 @@ xorg::testing::evemu::Device::Device(const std::string 
path)
 throw std::runtime_error(Failed to open uinput node);
   }
 
+  time_t ctime = time(NULL);
   if (evemu_create(d_-device, d_-fd)  0) {
 close(d_-fd);
 evemu_delete(d_-device);
 throw std::runtime_error(Failed to create evemu device);
   }
+
+  GuessDeviceNode(ctime);
 }
 
 void xorg::testing::evemu::Device::Play(const std::string path) const {
@@ -88,6 +147,10 @@ void xorg::testing::evemu::Device::Play(const std::string 
path) const {
   fclose(file);
 }
 
+const char* xorg::testing::evemu::Device::GetDeviceNode(void) {
+  return d_-device_node.length()  0 ? d_-device_node.c_str() : NULL;
+}
+
 xorg::testing::evemu::Device::~Device() {
   close(d_-fd);
   evemu_delete(d_-device);
-- 
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


[PATCH 00/10] integration testing for driver behaviour

2012-07-03 Thread Peter Hutterer

This series builds on top of chase' integration test branch, the whole lot
is available for, ahaha, testing from 
git://people.freedesktop.org/~whot/xserver :driver-integration-testing

It requires the xorg-gtest patches available from
git://people.freedesktop.org/~whot/xorg-gtest :devel

The first two patches are build fixes, including the requested target
make integration-check

The actual test are a quite simple ones that test otherwise rarely tested
behaviours. Although the build system is whacky at best, writing tests is
simple and this type of testing beats unit tests for server behaviour.

It does require the server and drivers to be installed correctly though,
without merging all drivers to test into the server we can't easily get
around this.

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


[PATCH 02/10] test: add integration-check target

2012-07-03 Thread Peter Hutterer
integration tests require a bit more effort than the other tests, so make it
run on a separate target.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 Makefile.am  |3 +++
 test/Makefile.am |2 ++
 test/integration/Makefile.am |9 ++---
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index cea140b..e1d5a86 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -95,3 +95,6 @@ DIST_SUBDIRS = \
 # gross hack
 relink: all
$(AM_V_at)$(MAKE) -C hw relink
+
+integration-check: all
+   $(AM_V_AT)$(MAKE) -C test integration-check
diff --git a/test/Makefile.am b/test/Makefile.am
index aacd0c7..a2d1cb9 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -118,3 +118,5 @@ endif
 
 EXTRA_DIST = ddxstubs.c
 
+integration-check: all
+   $(AM_V_AT)$(MAKE) -C integration integration-check
diff --git a/test/integration/Makefile.am b/test/integration/Makefile.am
index ff9789e..6109e23 100644
--- a/test/integration/Makefile.am
+++ b/test/integration/Makefile.am
@@ -1,15 +1,18 @@
-TESTS =
+integration_tests =
 
 if ENABLE_XORG_GTEST_TESTS
 include $(top_srcdir)/test/integration/Makefile-xorg-gtest.am
 noinst_LIBRARIES = $(XORG_GTEST_BUILD_LIBS)
 
 if ENABLE_XORG_GTEST_INPUT_TESTS
-TESTS += gtest-tests
+integration_tests += gtest-tests
 endif
 endif
 
-noinst_PROGRAMS = $(TESTS)
+noinst_PROGRAMS = $(integration_tests)
+
+integration-check: $(integration_tests)
+   $(AM_V_AT)for test in $(integration_tests); do $(builddir)/$$test; done
 
 AM_CPPFLAGS = \
-DDEFAULT_XORG_SERVER=\$(abs_top_builddir)/hw/xfree86/Xorg\ \
-- 
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


[PATCH 01/10] test: enable integration test directory only if building integration tests

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/Makefile.am |   10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/test/Makefile.am b/test/Makefile.am
index f58faed..aacd0c7 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -1,10 +1,11 @@
+SUBDIRS =
 if ENABLE_UNIT_TESTS
-SUBDIRS= .
+SUBDIRS += .
 noinst_PROGRAMS = list string touch
 if XORG
 # Tests that require at least some DDX functions in order to fully link
 # For now, requires xf86 ddx, could be adjusted to use another
-SUBDIRS += xi2 integration
+SUBDIRS += xi2
 noinst_PROGRAMS += xkb input xtest misc fixes xfree86  hashtabletest
 endif
 check_LTLIBRARIES = libxservertest.la
@@ -110,5 +111,10 @@ endif
 libxservertest_la_DEPENDENCIES = $(libxservertest_la_LIBADD)
 endif
 
+if ENABLE_INTEGRATION_TESTS
+SUBDIRS += integration
+endif
+
+
 EXTRA_DIST = ddxstubs.c
 
-- 
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


[PATCH 04/10] test/integration: split input driver test from xi2 test

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/integration/Makefile.am |   11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/test/integration/Makefile.am b/test/integration/Makefile.am
index 4b02463..eefcbe5 100644
--- a/test/integration/Makefile.am
+++ b/test/integration/Makefile.am
@@ -5,7 +5,7 @@ include $(top_srcdir)/test/integration/Makefile-xorg-gtest.am
 noinst_LIBRARIES = $(XORG_GTEST_BUILD_LIBS)
 
 if ENABLE_XORG_GTEST_INPUT_TESTS
-integration_tests += gtest-tests
+integration_tests += xi2-gtest input-driver-gtest
 endif
 endif
 
@@ -22,10 +22,15 @@ AM_CPPFLAGS = \
 
 AM_CXXFLAGS = $(GTEST_CXXFLAGS) $(XORG_GTEST_CXXFLAGS)
 
-gtest_tests_SOURCES = xi2.cpp input_drivers.cpp
-gtest_tests_LDADD = \
+GTEST_LDADDS = \
$(TEST_XINPUT_LIBS) \
$(TEST_X11_LIBS) \
$(XORG_GTEST_LIBS) \
$(EVEMU_LIBS) \
$(XORG_GTEST_MAIN_LIBS)
+
+xi2_gtest_SOURCES = xi2.cpp
+xi2_gtest_LDADD = $(GTEST_LDADDS)
+
+input_driver_gtest_SOURCES = input_drivers.cpp
+input_driver_gtest_LDADD = $(GTEST_LDADDS)
-- 
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


[PATCH 03/10] test/integration: add basic checks for default core/xtest devices

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/integration/Makefile.am   |2 +-
 test/integration/input_drivers.cpp |  128 
 2 files changed, 129 insertions(+), 1 deletion(-)
 create mode 100644 test/integration/input_drivers.cpp

diff --git a/test/integration/Makefile.am b/test/integration/Makefile.am
index 6109e23..4b02463 100644
--- a/test/integration/Makefile.am
+++ b/test/integration/Makefile.am
@@ -22,7 +22,7 @@ AM_CPPFLAGS = \
 
 AM_CXXFLAGS = $(GTEST_CXXFLAGS) $(XORG_GTEST_CXXFLAGS)
 
-gtest_tests_SOURCES = xi2.cpp
+gtest_tests_SOURCES = xi2.cpp input_drivers.cpp
 gtest_tests_LDADD = \
$(TEST_XINPUT_LIBS) \
$(TEST_X11_LIBS) \
diff --git a/test/integration/input_drivers.cpp 
b/test/integration/input_drivers.cpp
new file mode 100644
index 000..5b3060b
--- /dev/null
+++ b/test/integration/input_drivers.cpp
@@ -0,0 +1,128 @@
+#include stdexcept
+
+#include xorg/gtest/xorg-gtest.h
+
+#include X11/extensions/XInput.h
+#include X11/extensions/XInput2.h
+
+/**
+ * A test fixture for testing some input drivers
+ *
+ */
+class InputDriverTest : public xorg::testing::Test {
+protected:
+virtual void SetUp()
+{
+ASSERT_NO_FATAL_FAILURE(xorg::testing::Test::SetUp());
+
+int event_start;
+int error_start;
+
+ASSERT_TRUE(XQueryExtension(Display(), XInputExtension, xi2_opcode_,
+event_start, error_start));
+
+int major = 2;
+int minor = 0;
+
+ASSERT_EQ(Success, XIQueryVersion(Display(), major, minor));
+}
+
+int xi2_opcode_;
+};
+
+/* check default master devices */
+TEST_F(InputDriverTest, CoreDevices)
+{
+int ndevices;
+XIDeviceInfo *info;
+
+info = XIQueryDevice(Display(), XIAllMasterDevices, ndevices);
+ASSERT_EQ(ndevices, 2);
+ASSERT_EQ(strcmp(info[0].name, Virtual core pointer), 0);
+ASSERT_EQ(info[0].deviceid, 2);
+ASSERT_EQ(strcmp(info[1].name, Virtual core keyboard), 0);
+ASSERT_EQ(info[1].deviceid, 3);
+
+XIFreeDeviceInfo(info);
+}
+
+TEST_F(InputDriverTest, XTestDevices)
+{
+int ndevices;
+XIDeviceInfo *info;
+
+info = XIQueryDevice(Display(), XIAllDevices, ndevices);
+ASSERT_GT(ndevices, 4);
+ASSERT_EQ(strcmp(info[2].name, Virtual core XTEST pointer), 0);
+ASSERT_EQ(info[2].deviceid, 4);
+ASSERT_EQ(strcmp(info[3].name, Virtual core XTEST keyboard), 0);
+ASSERT_EQ(info[3].deviceid, 5);
+
+XIFreeDeviceInfo(info);
+}
+
+TEST_F(InputDriverTest, NewMaster)
+{
+int ndevices;
+XIDeviceInfo *info;
+
+info = XIQueryDevice(Display(), XIAllMasterDevices, ndevices);
+XIFreeDeviceInfo(info);
+
+ASSERT_EQ(ndevices, 2);
+
+/* add a new master */
+XIAnyHierarchyChangeInfo *any;
+XIAddMasterInfo add;
+add.type = XIAddMaster;
+add.name = strdup(test);
+add.send_core = 1;
+add.enable = 1;
+
+any = reinterpret_castXIAnyHierarchyChangeInfo* (add);
+ASSERT_EQ(Success, XIChangeHierarchy(Display(), any, 1));
+free(add.name);
+
+int new_pointer_id;
+info = XIQueryDevice(Display(), XIAllMasterDevices, ndevices);
+ASSERT_EQ(ndevices, 4);
+ASSERT_EQ(strcmp(info[2].name, test pointer), 0);
+ASSERT_EQ(strcmp(info[3].name, test keyboard), 0);
+new_pointer_id = info[2].deviceid;
+XIFreeDeviceInfo(info);
+
+/* check for xtest devices */
+bool xtest_ptr_found = false,
+ xtest_kbd_found = false;
+info = XIQueryDevice(Display(), XIAllDevices, ndevices);
+while (ndevices--  (!xtest_kbd_found || !xtest_ptr_found)) {
+if (strcmp(info[ndevices].name, test XTEST pointer) == 0)
+xtest_ptr_found = true;
+else if (strcmp(info[ndevices].name, test XTEST keyboard) == 0)
+xtest_kbd_found = true;
+}
+ASSERT_TRUE(xtest_ptr_found);
+ASSERT_TRUE(xtest_kbd_found);
+XIFreeDeviceInfo(info);
+
+/* remove master again */
+XIRemoveMasterInfo remove;
+remove.type = XIRemoveMaster;
+remove.deviceid = new_pointer_id;
+remove.return_mode = XIFloating;
+any = reinterpret_castXIAnyHierarchyChangeInfo* (remove);
+ASSERT_EQ(Success, XIChangeHierarchy(Display(), any, 1));
+
+info = XIQueryDevice(Display(), XIAllMasterDevices, ndevices);
+ASSERT_EQ(ndevices, 2);
+XIFreeDeviceInfo(info);
+
+/* XTEST devices disappear with master */
+info = XIQueryDevice(Display(), XIAllDevices, ndevices);
+while (ndevices--) {
+ASSERT_NE(strcmp(info[ndevices].name, test XTEST pointer), 0);
+ASSERT_NE(strcmp(info[ndevices].name, test XTEST keyboard), 0);
+}
+XIFreeDeviceInfo(info);
+}
+
-- 
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


[PATCH 05/10] test/integration: use new static calls for WaitFor*

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/integration/xi2.cpp |  172 ++
 1 file changed, 5 insertions(+), 167 deletions(-)

diff --git a/test/integration/xi2.cpp b/test/integration/xi2.cpp
index 1cbbe05..5dcf831 100644
--- a/test/integration/xi2.cpp
+++ b/test/integration/xi2.cpp
@@ -5,168 +5,6 @@
 #include X11/extensions/XInput.h
 #include X11/extensions/XInput2.h
 
-namespace {
-
-/**
- * Wait for an event on the X connection.
- *
- * @param [in] display The X display connection
- * @param [in] timeout The timeout in milliseconds
- *
- * @return Whether an event is available
- */
-bool WaitForEvent(::Display *display, time_t timeout = 1000)
-{
-fd_set fds;
-FD_ZERO(fds);
-
-int display_fd = ConnectionNumber(display);
-
-XSync(display, False);
-
-if (XPending(display))
-return true;
-else {
-FD_SET(display_fd, fds);
-
-struct timeval timeval = {
-static_casttime_t(timeout / 1000),
-static_casttime_t(timeout % 1000),
-};
-
-int ret;
-if (timeout)
-ret = select(display_fd + 1, fds, NULL, NULL, timeval);
-else
-ret = select(display_fd + 1, fds, NULL, NULL, NULL);
-
-if (ret  0)
-throw std::runtime_error(Failed to select on X fd);
-
-if (ret == 0)
-return false;
-
-return XPending(display);
-}
-}
-
-/**
- * Wait for an event of a specific type on the X connection.
- *
- * All events preceding the matching event are discarded. If no event was found
- * before the timeout expires, all events in the queue will have been 
discarded.
- *
- * @param [in] display   The X display connection
- * @param [in] type  The X core protocol event type
- * @param [in] extension The X extension opcode of a generic event, or -1 for
- *   any generic event
- * @param [in] evtypeThe X extension event type of a generic event, or -1
- *   for any event of the given extension
- * @param [in] timeout   The timeout in milliseconds
- *
- * @return Whether an event is available
- */
-bool WaitForEventOfType(::Display *display, int type, int extension, int 
evtype,
-time_t timeout = 1000)
-{
-while (WaitForEvent(display)) {
-XEvent event;
-if (!XPeekEvent(display, event))
-throw std::runtime_error(Failed to peek X event);
-
-if (event.type != type) {
-if (XNextEvent(display, event) != Success)
-throw std::runtime_error(Failed to remove X event);
-continue;
-}
-
-if (event.type != GenericEvent || extension == -1)
-return true;
-
-XGenericEvent *generic_event = 
reinterpret_castXGenericEvent*(event);
-
-if (generic_event-extension != extension) {
-if (XNextEvent(display, event) != Success)
-throw std::runtime_error(Failed to remove X event);
-continue;
-}
-
-if (evtype == -1 || generic_event-evtype == evtype)
-return true;
-
-if (XNextEvent(display, event) != Success)
-throw std::runtime_error(Failed to remove X event);
-}
-}
-
-/**
- * Wait for a specific device to be added to the server.
- *
- * @param [in] display The X display connection
- * @param [in] nameThe name of the device to wait for
- * @param [in] timeout The timeout in milliseconds
- *
- * @return Whether the device was added
- */
-bool WaitForDevice(::Display *display, const std::string name,
-   time_t timeout = 1000)
-{
-int opcode;
-int event_start;
-int error_start;
-
-if (!XQueryExtension(display, XInputExtension, opcode, event_start,
- error_start))
-throw std::runtime_error(Failed to query XInput extension);
-
-while (WaitForEventOfType(display, GenericEvent, opcode,
-  XI_HierarchyChanged)) {
-XEvent event;
-if (XNextEvent(display, event) != Success)
-throw std::runtime_error(Failed to get X event);
-
-XGenericEventCookie *xcookie =
-reinterpret_castXGenericEventCookie*(event.xcookie);
-if (!XGetEventData(display, xcookie))
-throw std::runtime_error(Failed to get X event data);
-
-XIHierarchyEvent *hierarchy_event =
-reinterpret_castXIHierarchyEvent*(xcookie-data);
-
-if (!(hierarchy_event-flags  XIDeviceEnabled)) {
-XFreeEventData(display, xcookie);
-continue;
-}
-
-bool device_found = false;
-for (int i = 0; i  hierarchy_event-num_info; i++) {
-if (!(hierarchy_event-info[i].flags  XIDeviceEnabled))
-continue;
-
-int num_devices;
-XIDeviceInfo *device_info =
-XIQueryDevice(display, hierarchy_event-info[i].deviceid,
-   

[PATCH 06/10] test/integration: decouple input_drivers test from standard mainloop

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/integration/input_drivers.cpp |   19 +++
 1 file changed, 19 insertions(+)

diff --git a/test/integration/input_drivers.cpp 
b/test/integration/input_drivers.cpp
index 5b3060b..bd0777f 100644
--- a/test/integration/input_drivers.cpp
+++ b/test/integration/input_drivers.cpp
@@ -10,9 +10,15 @@
  *
  */
 class InputDriverTest : public xorg::testing::Test {
+private:
+xorg::testing::XServer server;
 protected:
 virtual void SetUp()
 {
+server.Start();
+server.WaitForConnections();
+xorg::testing::Test::SetDisplayString(server.GetDisplayString());
+
 ASSERT_NO_FATAL_FAILURE(xorg::testing::Test::SetUp());
 
 int event_start;
@@ -27,6 +33,13 @@ protected:
 ASSERT_EQ(Success, XIQueryVersion(Display(), major, minor));
 }
 
+virtual void TearDown()
+{
+if (server.Pid() != -1)
+if (!server.Terminate())
+server.Kill();
+}
+
 int xi2_opcode_;
 };
 
@@ -126,3 +139,9 @@ TEST_F(InputDriverTest, NewMaster)
 XIFreeDeviceInfo(info);
 }
 
+int main(int argc, char **argv) {
+
+testing::InitGoogleTest(argc, argv);
+
+return RUN_ALL_TESTS();
+}
-- 
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


[PATCH 07/10] test/integration: test for default evdev behaviour

2012-07-03 Thread Peter Hutterer
Default behaviour on linux is new devices are assigned the evdev driver and
show up with the same name as the uinput device. Test for that.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/integration/input_drivers.cpp |   65 
 1 file changed, 65 insertions(+)

diff --git a/test/integration/input_drivers.cpp 
b/test/integration/input_drivers.cpp
index bd0777f..54a01a8 100644
--- a/test/integration/input_drivers.cpp
+++ b/test/integration/input_drivers.cpp
@@ -139,6 +139,71 @@ TEST_F(InputDriverTest, NewMaster)
 XIFreeDeviceInfo(info);
 }
 
+/* default setup on linux, we have hotplugging on and the device appears
+ * with its name */
+#ifdef __linux__
+TEST_F(InputDriverTest, Evdev)
+{
+XIEventMask mask;
+mask.deviceid = XIAllDevices;
+mask.mask_len = XIMaskLen(XI_TouchEnd);
+mask.mask = reinterpret_castunsigned char*(calloc(mask.mask_len, 1));
+XISetMask(mask.mask, XI_HierarchyChanged);
+
+ASSERT_EQ(Success,
+  XISelectEvents(Display(), DefaultRootWindow(Display()), mask,
+ 1));
+
+free(mask.mask);
+
+XFlush(Display());
+std::auto_ptrxorg::testing::evemu::Device device;
+try {
+  device = std::auto_ptrxorg::testing::evemu::Device(
+  new xorg::testing::evemu::Device(
+  TEST_ROOT_DIR recordings/ntrig_dell_xt2/device.prop));
+} catch (std::runtime_error error) {
+  std::cerr  Failed to create evemu device, skipping test.\n;
+  return;
+}
+
+ASSERT_TRUE(xorg::testing::XServer::WaitForDevice(Display(),
+  N-Trig MultiTouch (Virtual Test Device)));
+
+
+Atom evdev_prop = XInternAtom(Display(),
+  Evdev Middle Button Emulation, true);
+ASSERT_NE(evdev_prop, None);
+
+XIDeviceInfo *info;
+int ndevices;
+info = XIQueryDevice(Display(), XIAllDevices, ndevices);
+while(ndevices--) {
+if (strcmp(info[ndevices].name,
+   N-Trig MultiTouch (Virtual Test Device)) == 0)
+break;
+}
+ASSERT_GT(ndevices, 0);
+
+Atom *props;
+int nprops;
+
+props = XIListProperties(Display(),
+ info[ndevices].deviceid,
+ nprops);
+ASSERT_GT(nprops, 0);
+
+while(nprops--)
+if (props[nprops] == evdev_prop)
+break;
+ASSERT_GT(nprops, 0);
+
+XIFreeDeviceInfo(info);
+XFree(props);
+
+}
+#endif
+
 int main(int argc, char **argv) {
 
 testing::InitGoogleTest(argc, argv);
-- 
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


[PATCH 08/10] test/integration: add behaviour tests for AutoAddDevices off

2012-07-03 Thread Peter Hutterer
Check if the default mouse/keyboard sections are added if AutoAddDevices is
disabled.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/integration/Makefile.am   |5 +-
 test/integration/xorg-conf-input-tests.cpp |  111 
 2 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 test/integration/xorg-conf-input-tests.cpp

diff --git a/test/integration/Makefile.am b/test/integration/Makefile.am
index eefcbe5..b646e54 100644
--- a/test/integration/Makefile.am
+++ b/test/integration/Makefile.am
@@ -5,7 +5,7 @@ include $(top_srcdir)/test/integration/Makefile-xorg-gtest.am
 noinst_LIBRARIES = $(XORG_GTEST_BUILD_LIBS)
 
 if ENABLE_XORG_GTEST_INPUT_TESTS
-integration_tests += xi2-gtest input-driver-gtest
+integration_tests += xi2-gtest input-driver-gtest xorg-conf-input-gtest
 endif
 endif
 
@@ -34,3 +34,6 @@ xi2_gtest_LDADD = $(GTEST_LDADDS)
 
 input_driver_gtest_SOURCES = input_drivers.cpp
 input_driver_gtest_LDADD = $(GTEST_LDADDS)
+
+xorg_conf_input_gtest_SOURCES = xorg-conf-input-tests.cpp
+xorg_conf_input_gtest_LDADD = $(GTEST_LDADDS)
diff --git a/test/integration/xorg-conf-input-tests.cpp 
b/test/integration/xorg-conf-input-tests.cpp
new file mode 100644
index 000..1a9871e
--- /dev/null
+++ b/test/integration/xorg-conf-input-tests.cpp
@@ -0,0 +1,111 @@
+#include stdexcept
+#include fstream
+
+#include xorg/gtest/xorg-gtest.h
+
+#include X11/extensions/XInput.h
+#include X11/extensions/XInput2.h
+
+/**
+ * A test fixture for testing some input drivers
+ *
+ */
+class XOrgConfigInputTest : public xorg::testing::Test {
+protected:
+void WriteConfig(const char *param) {
+std::stringstream s;
+s  /tmp/  param  .conf;
+config_file = s.str();
+
+std::ofstream conffile(config_file.c_str());
+conffile.exceptions(std::ofstream::failbit | std::ofstream::badbit);
+
+std::string driver(param);
+
+conffile  
+Section \ServerLayout\
+Identifier \Dummy layout\
+Screen 0 \Dummy screen\ 0 0
+Option \AutoAddDevices\ \off\
+EndSection
+
+Section \Screen\
+Identifier \Dummy screen\
+Device \Dummy video device\
+EndSection
+
+Section \Device\
+Identifier \Dummy video device\
+Driver \dummy\
+EndSection;
+server.SetOption(-config, config_file);
+}
+
+void StartServer() {
+server.Start();
+server.WaitForConnections();
+xorg::testing::Test::SetDisplayString(server.GetDisplayString());
+
+ASSERT_NO_FATAL_FAILURE(xorg::testing::Test::SetUp());
+
+int event_start;
+int error_start;
+
+ASSERT_TRUE(XQueryExtension(Display(), XInputExtension, xi2_opcode_,
+event_start, error_start));
+
+int major = 2;
+int minor = 0;
+
+ASSERT_EQ(Success, XIQueryVersion(Display(), major, minor));
+}
+
+virtual void SetUp()
+{
+WriteConfig(default-input-drivers);
+StartServer();
+}
+
+virtual void TearDown()
+{
+if (server.Pid() != -1)
+if (!server.Terminate())
+server.Kill();
+
+if (config_file.size())
+unlink(config_file.c_str());
+}
+
+int xi2_opcode_;
+std::string config_file;
+xorg::testing::XServer server;
+};
+
+/**
+ * Test the behaviour of the server if AutoAddDevices if off and no input
+ * devices are present - server should create default core devices.
+ */
+TEST_F(XOrgConfigInputTest, DefaultDevices)
+{
+const char *param;
+int ndevices;
+XIDeviceInfo *info;
+
+info = XIQueryDevice(Display(), XIAllDevices, ndevices);
+/* VCP, VCK, 2 test devices, forced mouse/keyboard */
+ASSERT_EQ(ndevices, 6)  This test may fail if you do not have 
+the mouse/keyboard drivers installed;
+
+bool ptr_found = false, kbd_found = false;
+while(ndevices--  (!ptr_found || !kbd_found)) {
+if (strcmp(info[ndevices].name, default pointer) == 0)
+ptr_found = true;
+else if (strcmp(info[ndevices].name, default keyboard) == 0)
+kbd_found = true;
+}
+
+ASSERT_EQ(ptr_found, true);
+ASSERT_EQ(kbd_found, true);
+
+XIFreeDeviceInfo(info);
+}
-- 
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


[PATCH 09/10] test/integration: Add default mouse/keyboard driver behaviour

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/integration/xorg-conf-input-tests.cpp |   71 
 1 file changed, 71 insertions(+)

diff --git a/test/integration/xorg-conf-input-tests.cpp 
b/test/integration/xorg-conf-input-tests.cpp
index 1a9871e..76512ca 100644
--- a/test/integration/xorg-conf-input-tests.cpp
+++ b/test/integration/xorg-conf-input-tests.cpp
@@ -109,3 +109,74 @@ TEST_F(XOrgConfigInputTest, DefaultDevices)
 
 XIFreeDeviceInfo(info);
 }
+
+
+class XOrgConfigInputDriverTest : public XOrgConfigInputTest,
+  public ::testing::WithParamInterfaceconst 
char* {
+void AddDriverSection(const char *param) {
+std::ofstream conffile(config_file.c_str(), std::ios_base::app);
+conffile.exceptions(std::ofstream::failbit | std::ofstream::badbit);
+
+std::string driver(param);
+
+conffile  
+Section \InputDevice\
+Identifier \--device--\
+Driver \  driver  \
+EndSection;
+server.SetOption(-config, config_file);
+}
+
+virtual void SetUp()
+{
+const char *param = GetParam();
+
+WriteConfig(param);
+AddDriverSection(param);
+StartServer();
+}
+};
+
+/**
+ * AutoAddDevices is off, InputDevice sections are present but unreferenced,
+ * with those drivers that are elevated to core device status if found.
+ */
+TEST_P(XOrgConfigInputDriverTest, DriverDevice)
+{
+const char *param;
+int ndevices;
+XIDeviceInfo *info;
+
+param = GetParam();
+info = XIQueryDevice(Display(), XIAllDevices, ndevices);
+/* VCP, VCK, 2 test devices, forced mouse/keyboard */
+ASSERT_EQ(ndevices, 6)  Drivers required for this test: mouse, 
+   keyboard,   param;
+
+bool found = false;
+while(ndevices--) {
+if (strcmp(info[ndevices].name, --device--) == 0) {
+ASSERT_EQ(found, false)  Duplicate device  std::endl;
+found = true;
+}
+}
+
+ASSERT_EQ(found, true);
+
+XIFreeDeviceInfo(info);
+}
+
+int main(int argc, char **argv) {
+
+testing::InitGoogleTest(argc, argv);
+
+return RUN_ALL_TESTS();
+}
+
+/* mouse, keyboard, void, evdev, synaptics and vmmouse have special status,
+   they are picked as core devices if AAD is off and any section is found
+   in the xorg.conf.
+   mouse, keyboard and void auto-pick their device
+ */
+INSTANTIATE_TEST_CASE_P(, XOrgConfigInputDriverTest,
+::testing::Values(mouse, keyboard, void));
-- 
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


[PATCH 10/10] test/integration: add tests for evdev/synaptics default behaviour

2012-07-03 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 .../SynPS2_Synaptics_TouchPad/device.prop  |   33 
 test/integration/xorg-conf-input-tests.cpp |   82 ++--
 2 files changed, 107 insertions(+), 8 deletions(-)
 create mode 100644 
test/integration/recordings/SynPS2_Synaptics_TouchPad/device.prop

diff --git a/test/integration/recordings/SynPS2_Synaptics_TouchPad/device.prop 
b/test/integration/recordings/SynPS2_Synaptics_TouchPad/device.prop
new file mode 100644
index 000..3f4982b
--- /dev/null
+++ b/test/integration/recordings/SynPS2_Synaptics_TouchPad/device.prop
@@ -0,0 +1,33 @@
+N: SynPS/2 Synaptics TouchPad
+I: 0011 0002 0007 01b1
+P: 05 00 00 00 00 00 00 00
+B: 00 0b 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 01 00 00 00 00 00
+B: 01 20 e5 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 01 00 00 00 00 00 00 00 00
+B: 02 00 00 00 00 00 00 00 00
+B: 03 03 00 00 11 00 80 60 06
+B: 04 00 00 00 00 00 00 00 00
+B: 05 00 00 00 00 00 00 00 00
+B: 11 00 00 00 00 00 00 00 00
+B: 12 00 00 00 00 00 00 00 00
+B: 15 00 00 00 00 00 00 00 00
+B: 15 00 00 00 00 00 00 00 00
+A: 00 1472 5472 8 0
+A: 01 1408 4448 8 0
+A: 18 0 255 0 0
+A: 1c 0 15 0 0
+A: 2f 0 1 0 0
+A: 35 1472 5472 8 0
+A: 36 1408 4448 8 0
+A: 39 0 65535 0 0
+A: 3a 0 255 0 0
diff --git a/test/integration/xorg-conf-input-tests.cpp 
b/test/integration/xorg-conf-input-tests.cpp
index 76512ca..401a981 100644
--- a/test/integration/xorg-conf-input-tests.cpp
+++ b/test/integration/xorg-conf-input-tests.cpp
@@ -113,7 +113,8 @@ TEST_F(XOrgConfigInputTest, DefaultDevices)
 
 class XOrgConfigInputDriverTest : public XOrgConfigInputTest,
   public ::testing::WithParamInterfaceconst 
char* {
-void AddDriverSection(const char *param) {
+public:
+void AddDriverSection(const char *param, const char *options = ) {
 std::ofstream conffile(config_file.c_str(), std::ios_base::app);
 conffile.exceptions(std::ofstream::failbit | std::ofstream::badbit);
 
@@ -123,6 +124,7 @@ class XOrgConfigInputDriverTest : public 
XOrgConfigInputTest,
 Section \InputDevice\
 Identifier \--device--\
 Driver \  driver  \
+  options 
 EndSection;
 server.SetOption(-config, config_file);
 }
@@ -166,13 +168,6 @@ TEST_P(XOrgConfigInputDriverTest, DriverDevice)
 XIFreeDeviceInfo(info);
 }
 
-int main(int argc, char **argv) {
-
-testing::InitGoogleTest(argc, argv);
-
-return RUN_ALL_TESTS();
-}
-
 /* mouse, keyboard, void, evdev, synaptics and vmmouse have special status,
they are picked as core devices if AAD is off and any section is found
in the xorg.conf.
@@ -180,3 +175,74 @@ int main(int argc, char **argv) {
  */
 INSTANTIATE_TEST_CASE_P(, XOrgConfigInputDriverTest,
 ::testing::Values(mouse, keyboard, void));
+
+
+#ifdef __linux__ /* we need evemu for these */
+class XOrgConfigInputDriverTestDevice : public XOrgConfigInputDriverTest {
+private:
+std::auto_ptrxorg::testing::evemu::Device device;
+
+virtual void SetUp()
+{
+const char *param = GetParam();
+
+try {
+device = std::auto_ptrxorg::testing::evemu::Device(
+new xorg::testing::evemu::Device(TEST_ROOT_DIR 
recordings/SynPS2_Synaptics_TouchPad/device.prop)
+);
+} catch (std::runtime_error error) {
+std::cerr  Failed to create evemu device, skipping test.\n;
+return;
+}
+
+std::stringstream s;
+s  Option \Device\ \  device-GetDeviceNode()  \;
+std::string device_option = s.str();
+
+WriteConfig(param);
+AddDriverSection(param, device_option.c_str());
+StartServer();
+}
+};
+
+/**
+ * AutoAddDevices is off, InputDevice sections are present but unreferenced,
+ * with those drivers that are elevated to core device status if found.
+ */
+TEST_P(XOrgConfigInputDriverTestDevice, DriverDevice)
+{
+const char *param;
+int ndevices;
+XIDeviceInfo *info;
+
+param = GetParam();
+info = XIQueryDevice(Display(), XIAllDevices, ndevices);
+/* VCP, VCK, 2 test devices, forced mouse/keyboard */
+ASSERT_EQ(ndevices, 6)  Drivers required for this test: 
+   keyboard,   param;
+
+bool found = false;
+while(ndevices--) {
+if (strcmp(info[ndevices].name, --device--) == 0) {
+ASSERT_EQ(found, false)  Duplicate device  std::endl;
+found = true;
+}
+}
+
+ASSERT_EQ(found, true);
+
+XIFreeDeviceInfo(info);
+}
+
+
+INSTANTIATE_TEST_CASE_P(, XOrgConfigInputDriverTestDevice,
+::testing::Values(evdev, 

Re: [PATCH 15/36] xf86dga: handle DGAAvailable for gpu screens.

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 11:52 PM, Keith Packard kei...@keithp.com wrote:
 Adam Jackson a...@nwnk.net writes:

 On 7/2/12 6:13 AM, Dave Airlie wrote:

 I think anywhere you see a screen number on the wire the safe thing to
 do is index protocolscreens not gpuscreens.

 DGAAvailable is also called from things like xf86SetScrnInfoModes, which
 seem likely to be called with non-protocol screens though.

Yeah its because of this, its called from a lot of places that aren't
obviously protocol decoded.

Dave.
___
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 04/36] dix: introduce gpu screens. (v3)

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 9:07 PM, Adam Jackson a...@nwnk.net wrote:
 On 7/2/12 6:12 AM, Dave Airlie wrote:

 +void
 +RemoveGPUScreen(ScreenPtr pScreen)
 +{
 +int idx, j;
 +if (!pScreen-isGPU)
 +return;
 +
 +idx = pScreen-myNum - GPU_SCREEN_OFFSET;
 +for (j = idx; j  screenInfo.numGPUScreens - 1; j++) {
 +screenInfo.gpuscreens[j] = screenInfo.gpuscreens[j + 1];
 +}
 +screenInfo.numGPUScreens--;
 +
 +free(pScreen);
 +
 +}


 I don't see this getting called from the xf86 DDX, at least not in 6/36.

It doesn't get used until hotplug time.


 Even if it were, it's not symmetric with how xf86's remove path works.
 There you change -scrnIndex to match the current slot in the array, here
 you're not doing that with -myNum.  That seems like an error.

Oh good point, will revise to be saner.

Dave.
___
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 12/36] dix: add unattached list for attaching screens to initially.

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 11:45 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:11 PM, Keith Packard kei...@keithp.com wrote:

 Well an unbound gpu you probably don't know its a slave, its just
 unbound.

 It's about to become a slave though

It may not be in the future though, for xinerama emulation it might
become another master.

That policy decision is up to the client.

Dave.
___
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/36] xfree86: add platform bus hotplug support (v2)

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 10:31 PM, Keith Packard kei...@keithp.com wrote:

 Dave Airlie airl...@gmail.com writes:
 No they can all do it, just haven't the code to do the picking yet of
 a driver, and since
 USB is really the only thing I can hotplug so far.

 It seems like more driver ABI changes would be required to let them know
 about the new device then?

No the drivers will just have to be able to accept the udev entry
point with the GPU screen flag.

no further changes needed, granted I've no idea what drivers other
than modesetting will ever do proper hotplug,
I suppose I could get a real PCIE dock at some point and hotplug
radeon or nouveau, but there is a lot of work
kernel side before we could even start on the userspace drivers.

with dual-gpu machines even if we power off the discrete we never
unplug it, its all abstracted in the kernel.

Dave.
___
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] dix: introduce gpu screens. (v4)

2012-07-03 Thread Dave Airlie
From: Dave Airlie airl...@redhat.com

This patch introduces gpu screens into screenInfo. It adds interfaces
for adding and removing gpu screens, along with adding private fixup,
block handler support, and scratch pixmap init.

GPU screens have a myNum that is offset by GPU_SCREEN_OFFSET (256),
this is used for logging etc.

RemoveGPUScreen isn't used until xfree86: add platform bus hotplug support.

v2: no glyph pictures for GPU screens for now.
v3: introduce MAXGPUSCREENS, fix return value check
v4: fixup myNum when renumbering screens (ajax)

Signed-off-by: Dave Airlie airl...@redhat.com
---
 dix/dispatch.c   |   76 --
 dix/dixutils.c   |6 
 dix/main.c   |   15 ++
 dix/privates.c   |5 
 include/misc.h   |4 +++
 include/screenint.h  |9 ++
 include/scrnintstr.h |4 +++
 render/glyph.c   |2 ++
 8 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/dix/dispatch.c b/dix/dispatch.c
index 34d82f4..12e3b59 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3724,7 +3724,7 @@ with its screen number, a pointer to its ScreenRec, argc, 
and argv.
 
 */
 
-static int init_screen(ScreenPtr pScreen, int i)
+static int init_screen(ScreenPtr pScreen, int i, Bool gpu)
 {
 int scanlinepad, format, depth, bitsPerPixel, j, k;
 
@@ -3732,6 +3732,10 @@ static int init_screen(ScreenPtr pScreen, int i)
 return -1;
 }
 pScreen-myNum = i;
+if (gpu) {
+pScreen-myNum += GPU_SCREEN_OFFSET;
+pScreen-isGPU = TRUE;
+}
 pScreen-totalPixmapSize = 0;   /* computed in 
CreateScratchPixmapForScreen */
 pScreen-ClipNotify = 0;/* for R4 ddx compatibility */
 pScreen-CreateScreenResources = 0;
@@ -3788,7 +3792,7 @@ AddScreen(Bool (*pfnInit) (ScreenPtr /*pScreen */ ,
 if (!pScreen)
 return -1;
 
-ret = init_screen(pScreen, i);
+ret = init_screen(pScreen, i, FALSE);
 if (ret != 0) {
 free(pScreen);
 return ret;
@@ -3817,3 +3821,71 @@ AddScreen(Bool (*pfnInit) (ScreenPtr /*pScreen */ ,
 
 return i;
 }
+
+int
+AddGPUScreen(Bool (*pfnInit) (ScreenPtr /*pScreen */ ,
+  int /*argc */ ,
+  char **  /*argv */
+  ),
+ int argc, char **argv)
+{
+int i;
+ScreenPtr pScreen;
+Bool ret;
+
+i = screenInfo.numGPUScreens;
+if (i == MAXGPUSCREENS)
+return -1;
+
+pScreen = (ScreenPtr) calloc(1, sizeof(ScreenRec));
+if (!pScreen)
+return -1;
+
+ret = init_screen(pScreen, i, TRUE);
+if (ret != 0) {
+free(pScreen);
+return ret;
+}
+
+/* This is where screen specific stuff gets initialized.  Load the
+   screen structure, call the hardware, whatever.
+   This is also where the default colormap should be allocated and
+   also pixel values for blackPixel, whitePixel, and the cursor
+   Note that InitScreen is NOT allowed to modify argc, argv, or
+   any of the strings pointed to by argv.  They may be passed to
+   multiple screens.
+ */
+screenInfo.gpuscreens[i] = pScreen;
+screenInfo.numGPUScreens++;
+if (!(*pfnInit) (pScreen, argc, argv)) {
+dixFreePrivates(pScreen-devPrivates, PRIVATE_SCREEN);
+free(pScreen);
+screenInfo.numGPUScreens--;
+return -1;
+}
+
+update_desktop_dimensions();
+
+dixRegisterScreenPrivateKey(cursorScreenDevPriv, pScreen, PRIVATE_CURSOR,
+0);
+
+return i;
+}
+
+void
+RemoveGPUScreen(ScreenPtr pScreen)
+{
+int idx, j;
+if (!pScreen-isGPU)
+return;
+
+idx = pScreen-myNum - GPU_SCREEN_OFFSET;
+for (j = idx; j  screenInfo.numGPUScreens - 1; j++) {
+screenInfo.gpuscreens[j] = screenInfo.gpuscreens[j + 1];
+screenInfo.gpuscreens[j]-myNum = j + GPU_SCREEN_OFFSET;
+}
+screenInfo.numGPUScreens--;
+
+free(pScreen);
+
+}
diff --git a/dix/dixutils.c b/dix/dixutils.c
index b249a81..3f24629 100644
--- a/dix/dixutils.c
+++ b/dix/dixutils.c
@@ -386,6 +386,9 @@ BlockHandler(pointer pTimeout, pointer pReadmask)
 for (i = 0; i  screenInfo.numScreens; i++)
 (*screenInfo.screens[i]-BlockHandler) (screenInfo.screens[i],
 pTimeout, pReadmask);
+for (i = 0; i  screenInfo.numGPUScreens; i++)
+(*screenInfo.gpuscreens[i]-BlockHandler) (screenInfo.gpuscreens[i],
+   pTimeout, pReadmask);
 for (i = 0; i  numHandlers; i++)
 if (!handlers[i].deleted)
 (*handlers[i].BlockHandler) (handlers[i].blockData,
@@ -422,6 +425,9 @@ WakeupHandler(int result, pointer pReadmask)
 for (i = 0; i  screenInfo.numScreens; i++)
 (*screenInfo.screens[i]-WakeupHandler) (screenInfo.screens[i],
  result, pReadmask);
+

Re: [PATCH 15/36] xf86dga: handle DGAAvailable for gpu screens.

2012-07-03 Thread Dave Airlie
On Tue, Jul 3, 2012 at 8:51 AM, Dave Airlie airl...@gmail.com wrote:
 On Mon, Jul 2, 2012 at 11:52 PM, Keith Packard kei...@keithp.com wrote:
 Adam Jackson a...@nwnk.net writes:

 On 7/2/12 6:13 AM, Dave Airlie wrote:

 I think anywhere you see a screen number on the wire the safe thing to
 do is index protocolscreens not gpuscreens.

 DGAAvailable is also called from things like xf86SetScrnInfoModes, which
 seem likely to be called with non-protocol screens though.

 Yeah its because of this, its called from a lot of places that aren't
 obviously protocol decoded.

Okay I decided I liked what ajax said anyways, so I've split the api into
one that gets called from protocol decode and one that gets called internally
with a pScreen.

I'll send out a new version.

Dave.

 Dave.
___
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] xf86dga: handle DGAAvailable for gpu screens. (v2)

2012-07-03 Thread Dave Airlie
From: Dave Airlie airl...@redhat.com

v2: Split out DGAAvailable into two interfaces, one for calls from protocol
decoding and one for internal usage, after discussion with ajax and keithp.

Signed-off-by: Dave Airlie airl...@redhat.com
---
 hw/xfree86/common/xf86DGA.c |   15 ---
 hw/xfree86/dixmods/extmod/dgaproc.h |1 +
 hw/xfree86/modes/xf86DiDGA.c|2 +-
 hw/xfree86/vgahw/vgaCmap.c  |2 +-
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/xfree86/common/xf86DGA.c b/hw/xfree86/common/xf86DGA.c
index 6416372..61612c3 100644
--- a/hw/xfree86/common/xf86DGA.c
+++ b/hw/xfree86/common/xf86DGA.c
@@ -521,18 +521,27 @@ DGAChangePixmapMode(int index, int *x, int *y, int mode)
 }
 
 Bool
-DGAAvailable(int index)
+DGAScreenAvailable(ScreenPtr pScreen)
 {
 if (!DGAScreenKeyRegistered)
 return FALSE;
 
-if (DGA_GET_SCREEN_PRIV(screenInfo.screens[index]))
+if (DGA_GET_SCREEN_PRIV(pScreen))
 return TRUE;
-
 return FALSE;
 }
 
 Bool
+DGAAvailable(int index)
+{
+ScreenPtr pScreen;
+
+assert(index  MAXSCREENS);
+pScreen = screenInfo.screens[index];
+return DGAScreenAvailable(pScreen);
+}
+
+Bool
 DGAActive(int index)
 {
 DGAScreenPtr pScreenPriv;
diff --git a/hw/xfree86/dixmods/extmod/dgaproc.h 
b/hw/xfree86/dixmods/extmod/dgaproc.h
index b4e0ddf..2c2fae0 100644
--- a/hw/xfree86/dixmods/extmod/dgaproc.h
+++ b/hw/xfree86/dixmods/extmod/dgaproc.h
@@ -64,6 +64,7 @@ extern _X_EXPORT void
  DGASelectInput(int Index, ClientPtr client, long mask);
 
 extern _X_EXPORT Bool DGAAvailable(int Index);
+extern _X_EXPORT Bool DGAScreenAvailable(ScreenPtr pScreen);
 extern _X_EXPORT Bool DGAActive(int Index);
 extern _X_EXPORT void DGAShutdown(void);
 extern _X_EXPORT void DGAInstallCmap(ColormapPtr cmap);
diff --git a/hw/xfree86/modes/xf86DiDGA.c b/hw/xfree86/modes/xf86DiDGA.c
index bb954ac..3f1a330 100644
--- a/hw/xfree86/modes/xf86DiDGA.c
+++ b/hw/xfree86/modes/xf86DiDGA.c
@@ -178,7 +178,7 @@ _xf86_di_dga_reinit_internal(ScreenPtr pScreen)
 ScrnInfoPtr scrn = xf86ScreenToScrn(pScreen);
 xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
 
-if (!DGAAvailable(pScreen-myNum))
+if (!DGAScreenAvailable(pScreen))
 return TRUE;
 
 if (!xf86_dga_get_modes(pScreen))
diff --git a/hw/xfree86/vgahw/vgaCmap.c b/hw/xfree86/vgahw/vgaCmap.c
index a1aa405..e7a0d02 100644
--- a/hw/xfree86/vgahw/vgaCmap.c
+++ b/hw/xfree86/vgahw/vgaCmap.c
@@ -97,7 +97,7 @@ xColorItem *pdefs;
 }
 
 writeColormap = scrninfp-vtSema;
-if (DGAAvailable(scrnIndex)) {
+if (DGAScreenAvailable(pmap-pScreen)) {
 writeColormap = writeColormap ||
 (DGAGetDirectMode(scrnIndex) 
  !(DGAGetFlags(scrnIndex)  XF86DGADirectColormap)) ||
-- 
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


Re: [PATCH 21/36] dix: pixmap sharing infrastructure (v2)

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 10:32 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:20 PM, Keith Packard kei...@keithp.com wrote:

 though a void * that just cases an fd is probably okay.

 That was my thinking. void * covers a lot of sins.

Okay I'll convert the 3 patches to use void *.

I won't repost them, can you R-b this one then with that in mind?

Dave.
___
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 23/36] randr: add initial scanout pixmap support (v2)

2012-07-03 Thread Dave Airlie
On Tue, Jul 3, 2012 at 12:51 AM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 From: Dave Airlie airl...@redhat.com

 When randr notices a crtc configuration request for a slave device,
 it checks if the slave allocated pixmap exists and is suitable,
 if not it allocates a new shared pixmap from the master, shares
 it to the slave, and starts the master tracking damage to it,
 to keep it updated from the current front pixmap.

 If the resize means the crtc is no longer used it will destroy
 the slave pixmap.

 This adds the concept of a scanout_pixmap to the randr_crtc object,
 and also adds a master pixmap pointer to the pixmap object, along
 with defining some pixmap helper functions for getting pixmap
 box/regions.

 I'd rather store the master pixmap somewhere else; it looks like you
 could stick it in the crtc pretty easily, right next to the
 scanout_pixmap member.

 Otherwise, this looks good to me.

The master_pixmap is also used in DRI2 code later for sharing, the last patch.

So it made sense to stick it in the pixmap when I had two users, I
suppose I could
try and stash it somewhere else in each user but it might get a bit ugly.

Dave.
___
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 19/36] dix: add ability to link output slave gpus to the current gpu

2012-07-03 Thread Dave Airlie
On Tue, Jul 3, 2012 at 12:41 AM, Keith Packard kei...@keithp.com wrote:

 Dave Airlie airl...@gmail.com writes:

 +void
 +AttachOutputGPU(ScreenPtr pScreen, ScreenPtr new)
 +{
 +assert(new-isGPU);
 +xorg_list_add(new-output_head, pScreen-output_slave_list);
 +new-current_master = pScreen;
 +}

 I'd love an assert that new isn't already on a list somehow?

I'm not really sure you can tell that, since list members don't have
to make sense
by default,

I could add
assert(!new-current_master);
which should be true.

Dave.
___
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 25/36] randr: fixup constrain to work with slave screens.

2012-07-03 Thread Dave Airlie
On Tue, Jul 3, 2012 at 1:02 AM, Keith Packard kei...@keithp.com wrote:
 Adam Jackson a...@nwnk.net writes:

 I'm not sure this makes sense?  The list of slave outputs is the list of
 outputs from other screens connected to _this_ screen's CRTC list,
 right?  So _this_ screen's CRTC list is all you need to walk.

 Nope, the slave screens keep their own OUTPUTs and CRTCs, there's a
 later patch which lists all of these for RandR requests. I think there's
 a ton more to fix in randr though; lots of places assume that there's
 only one list of crtcs for each screen...

I don't think its that bad, I think I got most of them, once the user has looked
up the crtc XID, you don't care about the screen that often.

Dave.
___
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 16/36] xf86: load modesetting driver on Linux hotplug

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 11:55 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 From: Dave Airlie airl...@redhat.com

 This driver is currently really the only choice for a hotplug
 at the moment, we can refine this in the future to try and
 pick the driver names.

 We have to reload this as it can get unloaded it not used
 in the original probe.

 Should xf86platformAddDevice be doing this module load instead? It's got
 the module name sitting there. Duplicating kludges for modesetting
 across two functions seems sub-optimal after all.

Yeah I've merged this into the hotplug functions in xf86platformBus,

once I get to having some hw other than modesetting driven USB
devices, I will probably revisit,
the split out a bit more, but this is will work perfectly well for all
current use cases.

Dave.
___
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 35/36] dix/randr: add a hook into screen to replace scanout pixmap

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 10:22 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Mon, Jul 2, 2012 at 6:13 AM, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com

 For DRI2 in some offload cases we need to set a new pixmap on the crtc,
 this hook allows dri2 to call into randr to do the necessary work to set
 a pixmap as the scanout pixmap for the crtc the drawable is currently on.

 This is really only to be used for unredirected full screen apps in 
 composited
 environments.

 Not directly related to this patch, but are crtcs transforms handled
 properly with slaves?  E.g., randr rotation?

Oh good question, at the moment no, and I probably need to block
rotation on slaves
until it is I suppose.

The problem with transforms at least for the intel LVDS slaved to the
nvidia, is the nvidia
pcopy engine can't do transforms, and we definitely want to use it for
copies if we can.

For USB devices it should be a lot easier to hook onto the rotate
code, if still a bit messy.

Dave.
___
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 32/36] xf86/cursor: fallback to sw cursor if we have slaves present.

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 10:16 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Mon, Jul 2, 2012 at 6:13 AM, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com

 Current USB devices have no hw rendered cursors, so we need the
 master GPU to render the cursor, so whenever we plug in a
 slave device, fallback to sw rendered cursors.

 What about when we have a non-USB output slave that can do hw cursors?

We need to overhaul the cursor APIs from what I can see to enable any other
scenario, and I have this on my things to look at for 1.14 most likely,

Dave.
___
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] uload unused input modules

2012-07-03 Thread Michal Suchanek
Hello,

since the patches got lost somewhere without even generating an error
sending as plain email.

Changes since v1:

 - split out module 'query' functions
 - rename some variables
 - fix some whitespace and braces

Thanks

Michal

From a86d41ee113f76776a9410205ef6f90c78e41d22 Mon Sep 17 00:00:00 2001
From: Michal Suchanek hramr...@gmail.com
Date: Mon, 4 Jun 2012 14:22:14 +0200
Subject: [PATCH 1/4] xfree86/loader: Do not unload sibling modules.
To: xorg-devel@lists.x.org

Ordering of sibling modules is internal to the loader implementation.

Unloading siblings following the requested module in the sibling list
leads to unexpected unloading of seemingly random modules.

Signed-off-by: Michal Suchanek hramr...@gmail.com
---
 hw/xfree86/loader/loadmod.c |   19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 706b9b3..e94265a 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -1103,10 +1103,23 @@ UnloadModuleOrDriver(ModuleDescPtr mod)
 LoaderUnload(mod-name, mod-handle);
 }

-if (mod-child)
+while (mod-child)
 UnloadModuleOrDriver(mod-child);
-if (mod-sib)
-UnloadModuleOrDriver(mod-sib);
+
+if (mod-parent) {
+ModuleDescPtr sib_list = mod-parent-child;
+if (sib_list == mod) {
+mod-parent-child = mod-sib;
+} else {
+while (sib_list) {
+if (sib_list-sib == mod) {
+sib_list-sib = mod-sib;
+break;
+}
+sib_list = sib_list-sib;
+}
+}
+}
 free(mod-path);
 free(mod-name);
 free(mod);
-- 
1.7.10

From 04aee58b6b497613a9b0c2c9253cfbcbc3f7e249 Mon Sep 17 00:00:00 2001
From: Michal Suchanek hramr...@gmail.com
Date: Mon, 2 Jul 2012 10:54:53 +0200
Subject: [PATCH 2/4] xfree86/loader: Split off some loader code into separate
 query functions.
To: xorg-devel@lists.x.org

 * Add CanUnloadModule

Signed-off-by: Michal Suchanek hramr...@gmail.com
---
 hw/xfree86/loader/loaderProcs.h |5 +
 hw/xfree86/loader/loadmod.c |   36 ++--
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/loader/loaderProcs.h b/hw/xfree86/loader/loaderProcs.h
index a7b752b..c33039a 100644
--- a/hw/xfree86/loader/loaderProcs.h
+++ b/hw/xfree86/loader/loaderProcs.h
@@ -88,6 +88,11 @@ unsigned long LoaderGetModuleVersion(ModuleDescPtr mod);
 void LoaderResetOptions(void);
 void LoaderSetOptions(unsigned long);

+int IsBuiltinModule(ModuleDescPtr mod);
+int IsDuplicated(ModuleDescPtr mod);
+int IsDuplicateModule(ModuleDescPtr mod);
+int CanUnloadModule(ModuleDescPtr mod);
+
 /* Options for LoaderSetOptions */
 #define LDR_OPT_ABI_MISMATCH_NONFATAL  0x0001

diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index e94265a..4844e18 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -803,6 +803,19 @@ NewModuleDesc(const char *name)
 return mdp;
 }

+/* An advisory function. When true is returned unloading the module should be
+ * safe. Uload can be attempted nonetheless. */
+int CanUnloadModule(ModuleDescPtr mod)
+{
+if (IsBuiltinModule(mod))
+return 0;
+if (IsDuplicated(mod))
+return 0;
+/* Neither of the above can be true for modules for which IsDuplicateModule
+ * is true. */
+return 1;
+}
+
 ModuleDescPtr
 DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent)
 {
@@ -1083,10 +1096,29 @@ UnloadModule(pointer mod)
 UnloadModuleOrDriver((ModuleDescPtr) mod);
 }

+/* Is given module builtin? */
+int IsBuiltinModule(ModuleDescPtr mod)
+{
+return (mod == (ModuleDescPtr) 1);
+}
+
+/* Is given module duplicate of another? */
+int IsDuplicateModule(ModuleDescPtr mod)
+{
+return (mod-TearDownData == ModuleDuplicated);
+}
+
+/* Do duplicates of given module exist? */
+int IsDuplicated(ModuleDescPtr mod)
+{
+/* cannot tell */
+return 1;
+}
+
 static void
 UnloadModuleOrDriver(ModuleDescPtr mod)
 {
-if (mod == (ModuleDescPtr) 1)
+if (IsBuiltinModule(mod))
 return;

 if (mod == NULL || mod-name == NULL)
@@ -1097,7 +1129,7 @@ UnloadModuleOrDriver(ModuleDescPtr mod)
 else
 xf86MsgVerb(X_INFO, 3, UnloadModule: \%s\\n, mod-name);

-if (mod-TearDownData != ModuleDuplicated) {
+if (!IsDuplicateModule(mod)) {
 if ((mod-TearDownProc)  (mod-TearDownData))
 mod-TearDownProc(mod-TearDownData);
 LoaderUnload(mod-name, mod-handle);
-- 
1.7.10

From a2bdabbe9e5610b6c0ad80fb3659039da8800970 Mon Sep 17 00:00:00 2001
From: Michal Suchanek hramr...@gmail.com
Date: Mon, 4 Jun 2012 17:29:37 +0200
Subject: [PATCH 3/4] xfree86: unload unused input drivers
To: xorg-devel@lists.x.org

Signed-off-by: Michal Suchanek hramr...@gmail.com
---
 hw/xfree86/common/xf86Helper.c |   22 +-
 

Re: [PATCH 11/36] xfree86: add framework for provider support in ddx.

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 8:35 PM, Dave Airlie airl...@gmail.com wrote:
 On Mon, Jul 2, 2012 at 8:07 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 +xf86ProviderPtr
 +xf86ProviderCreate(ScrnInfoPtr scrn,
 +   const xf86ProviderFuncsRec *funcs, const char *name)
 +{
 +xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
 +xf86ProviderPtr provider;
 +int len;
 +
 +if (xf86_config-provider)
 +return xf86_config-provider;

 A 'create' function shouldn't return an existing object; that's more
 like a 'get' function. Do you expect callers to not know if the object
 exists yet?

 I suppose it shouldn't happen, I was just being careful, we should
 only have one provider object per device, maybe I should just make
 that an assert.
   * Requests that the driver resize the screen.
 @@ -681,6 +731,7 @@ typedef struct _xf86CrtcConfig {
  /* callback when crtc configuration changes */
  xf86_crtc_notify_proc_ptr xf86_crtc_notify;

 +xf86ProviderPtr provider;
  } xf86CrtcConfigRec, *xf86CrtcConfigPtr;

 Could the elements of 'provider' just become members of the crtc config rec?

 will check that tomorrow, probably could do alright, but I just liked
 keeping things in an object.

 @@ -1552,6 +1552,19 @@ xf86RandR12CreateObjects12(ScreenPtr pScreen)
  output-funcs-create_resources(output);
  RRPostPendingProperties(output-randr_output);
  }
 +
 +{
 +xf86ProviderPtr provider = config-provider;
 +provider-randr_provider = RRProviderCreate(pScreen, 
 provider-name,
 +
 strlen(provider-name), provider);
 +
 +if (provider-scrn-is_gpu) {
 +RRProviderSetRolesAbilities(provider-randr_provider, 
 provider-scrn-roles,
 +provider-scrn-abilities);
 +RRProviderSetCurrentRole(provider-randr_provider, 
 provider-scrn-current_role);
 +}
 +}
 +

 Ok, I'm lost here -- this assumes that config-provider exists, and yet
 xf86ProviderCreate isn't being called with some default values in case
 the driver doesn't do that yet.

 Yeah I should check here in case drivers don't create a provider object 
 alright.

I could move the provider stuff into the toplevel struct alright,
though that would mean
a one provider per GPU model which is what I think is correct.

I suppose I need to review the randr protocol bits first before I decide.

Dave.
___
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] randr: add provider object (v7)

2012-07-03 Thread Dave Airlie
(forgot list first time I sent this).

On Mon, Jul 2, 2012 at 10:03 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:54 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:


 Yes thats going to be the standard on switchable GPU machines, two masters
 and the ability to jump between them. In that case max master is one, and 
 you'd
 have to set the provider roles. If maxmaster  1 then xinerama
 emulation is available.

 In those machines, isn't the limitation that only one of them can drive
 the LVDS panel at a time? Can't you have the internal GPU driving the
 LVDS while the external GPU drives other outputs?

 Yes but you don't configure it in xinerama mode for that, there are
 mux and muxless configurations.

 I think I understand what the hardware does now, just trying to figure
 out how to provide a reasonable description of that to applications
 while not just providing a big 'muxless/muxed' switch, which seems
 restricted to precisely how the hardware that we have works today.

 In mux configuration, you switch the mux between GPUs when the master
 is switched.

 Right, the mux just rewires things so that the other GPU is hooked up to
 the LVDS. I'd expect the LVDS outputs to reflect a suitable connection
 status for these changes.

 The only question is how you drive the mux switch. Is this switch
 selectable per-output? Or is is global? And, how do we label which
 outputs are affected by a global switch?

We don't really know with 100% certainty since the specs for all these
things are closed. We've done a lot of RE work, and it mostly appears
to be a single global switch that turns any connected outputs. There is
a table in the intel bios which can tell you about which outputs are muxed
etc, but this isn't always present. Again we also have laptops that have
a mux but don't expose this table, as they only have the MUX so the
BIOS can pick IGP/discrete for Vista, and Windows 7 operates in
muxless mode.

The current problem is I'm not sure any OS exposes muxless and mux
in one OS. Mac OSX always uses muxed, Vista the same, and I think
Windows 7 always exposes muxless if the bios reports optimus support
or the AMD equivalent.

 In muxless, you go from have Intel connected to the LVDS as the
 master, to the nvidia being the master with the intel being and output
 slave driving the LVDS.

 Right, the 'master' bit *also* selects which GPU applications will end
 up using by default. And, in this mode, really the only thing the
 'master' bit is doing is select which rendering engine new applications
 will end up talking to. Presumably, applications could continue to use
 the Intel GPU as a rendering slave somehow? It's pretty easy to imagine
 wanting to use the on-board graphics for rendering video content while
 the nVidia GPU is busy with OpenGL.

That would again be a xinerama style situation though a lot more extreme
and this isn't a use case I have expressed any interest in supporting. I've
explicitly limited the use cases for this to avoid any sink holes. Like
there are loads of what-if scenarios here, but I'm only interested in getting
Linux to the level that Windows has been for years, and any diversions
are just pushing the timeframe for that out.

 So, much like the current RandR 'primary' output, we'll still need a
 provider which is marked as being that which a normal GL application
 will target for rasterizing.

Again a corner case that you might thing is useful but not sure I've
seen any indication of how we'd ever support or expose this from a desktop
env.


 Because we want to expose GPU level properties on the provider, also
 currently the offload slave isn't a rendering slave per-say, there are
 no circumstances where want to use the IGP GPU as an offload slave,
 its simply not a configuration we should be exposing, and I see no
 reason to advertise it.

 Not for GL, no. But, for video, yes. Especially as the IGP will have an
 overlay that the IGP video rasterizer can target for zero-copy operation...

Again most people want to use the nvidia for video, vdpau kicks the ass
of other things, at least under MacOSX they just switch off the intel
when nvidia is running, and under Windows they don't expose it anymore
to the OS. I'm not sure I want to start adding use cases they nobody
has explored, it will just lead into ratholes.

 I don't think it would represent reality though, and I'd like to keep
 a provider representing a whole GPU.

 Right, a GPU can have two pieces -- rasterizer and scanout engine, and
 you can enable them separately.

I'm staying adamant that one provider is one GPU, however I could accept that
splitting the master role into two roles and allowing GPUs to have
multiple roles
at once might make sense.


 Not sure what you mean conflicting outputs, you mean mux'ed ones where
 we'd have an LVDS on each GPU? or do you mean crazy
 connectors where EDID is wired to the intel, but data 

Re: [PATCH 18/36] privates: when pixmap privates increase, bump the totalPixmapSize

2012-07-03 Thread Dave Airlie
On Tue, Jul 3, 2012 at 12:41 AM, Keith Packard kei...@keithp.com wrote:
 Adam Jackson a...@nwnk.net writes:

 On 7/2/12 6:13 AM, Dave Airlie wrote:
 From: Dave Airlie airl...@redhat.com

 This bumps totalPixmapSize in all attached screens.

 Aieee.  totalPixmapSize is now monotonically increasing every time you
 plug something in.  Until it wraps, of course, and then bang.  Not sure
 what a better solution looks like, but IWBNI host storage was shared if
 possible.

 Ok, so how about we fix the privates stuff so that drivers can allocate
 per-screen private data by sticking a DevPrivateKeyRec inside their
 screen private structure? And then we make it so you can allocate
 per-screen private data after the server starts, but not other kinds?

 I think that'll be pretty straightforward, shouldn't cause performance
 regressions in drivers as they generally have a pointer to their screen
 private where they're currently using privates.

 With that, changing clients to use the serverClient mechanism for
 privates (a pointer instead of a directly allocated chunk), and also
 using that same scheme for glyphs and we should be able to make this work.


Let me start by saying fucking privates.

So I'm not sure what you mean by per-screen private data, drivers have
requirements for per-pixmap private data as well, currently by modesetting
driver stashes the slave framebuffer id from the kernel into a pixmap private.

Hence why I have to adjust the total pixmap size, though as ajax pointed
out its a bit monotonically increasing for sanity, I'll have to track
down it and see.

Dave.
___
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 17/36] privates: remove asserts that stop dynamic increasing of privates size.

2012-07-03 Thread Dave Airlie
On Mon, Jul 2, 2012 at 10:10 PM, Adam Jackson a...@nwnk.net wrote:
 On 7/2/12 6:13 AM, Dave Airlie wrote:

 From: Dave Airlie airl...@redhat.com

 These asserts stop us increasing the size of privates after create screen
 resources has been called, however for dynamic screens we need to be able
 to increase private size as new drivers are loaded.

 In theory any object allocated by an older driver won't try and use newer
 privates and any object allocated by a new driver should be okay.


 I... don't know if I believe this.  What objects did you find that had this
 property?


When I say older/newer driver I mean, loaded at startup, secondary
loaded drivers btw.

So far the modesetting driver is the only thing to exercise this, for
its offload stuff
it needs a pixmap private allocated, so I hit this problem and the
subsequent one.

Now if we allocate 20 pixmaps before the second driver is loaded,
they'll look like

PixmapHeader + privates + pixmap data. the privates will have been handed out
to the current list of keys register on PRIVATE_PIXMAP.

Now we bump the size, and we allocate pixmap 21,
PixmapHeader + privates + newpriv + pixmap data. Now the new driver
will never get to see pixmaps 1-20 since there is no way it should ever
see pixmaps allocated on the other drivers, and pixmap 21 and future
pixmap will work fine. If driver 2 unloads the pixmaps 21 etc should
all be destroyed, and in that case we probably should decrease the
total pixmap size again.

but privates have always longed remained something I run away screaming from
everytime I see them.

Dave.
___
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 35/36] dix/randr: add a hook into screen to replace scanout pixmap

2012-07-03 Thread Alex Deucher
On Tue, Jul 3, 2012 at 6:09 AM, Dave Airlie airl...@gmail.com wrote:
 On Mon, Jul 2, 2012 at 10:22 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Mon, Jul 2, 2012 at 6:13 AM, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com

 For DRI2 in some offload cases we need to set a new pixmap on the crtc,
 this hook allows dri2 to call into randr to do the necessary work to set
 a pixmap as the scanout pixmap for the crtc the drawable is currently on.

 This is really only to be used for unredirected full screen apps in 
 composited
 environments.

 Not directly related to this patch, but are crtcs transforms handled
 properly with slaves?  E.g., randr rotation?

 Oh good question, at the moment no, and I probably need to block
 rotation on slaves
 until it is I suppose.

 The problem with transforms at least for the intel LVDS slaved to the
 nvidia, is the nvidia
 pcopy engine can't do transforms, and we definitely want to use it for
 copies if we can.

It might make sense to add a cap to the output gpu (can_copy) or
something like that.  Then it could take care of the transform in the
copy.  The output gpu's engine is probably mostly idle at that point
anyway so in some cases, it may be more efficient to use it rather
than the render GPU's engine.

Alex


 For USB devices it should be a lot easier to hook onto the rotate
 code, if still a bit messy.

 Dave.
___
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] randr: add provider object (v7)

2012-07-03 Thread Alex Deucher
On Tue, Jul 3, 2012 at 7:57 AM, Dave Airlie airl...@gmail.com wrote:
 (forgot list first time I sent this).

 On Mon, Jul 2, 2012 at 10:03 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:54 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:


 Yes thats going to be the standard on switchable GPU machines, two masters
 and the ability to jump between them. In that case max master is one, and 
 you'd
 have to set the provider roles. If maxmaster  1 then xinerama
 emulation is available.

 In those machines, isn't the limitation that only one of them can drive
 the LVDS panel at a time? Can't you have the internal GPU driving the
 LVDS while the external GPU drives other outputs?

 Yes but you don't configure it in xinerama mode for that, there are
 mux and muxless configurations.

 I think I understand what the hardware does now, just trying to figure
 out how to provide a reasonable description of that to applications
 while not just providing a big 'muxless/muxed' switch, which seems
 restricted to precisely how the hardware that we have works today.

 In mux configuration, you switch the mux between GPUs when the master
 is switched.

 Right, the mux just rewires things so that the other GPU is hooked up to
 the LVDS. I'd expect the LVDS outputs to reflect a suitable connection
 status for these changes.

 The only question is how you drive the mux switch. Is this switch
 selectable per-output? Or is is global? And, how do we label which
 outputs are affected by a global switch?

 We don't really know with 100% certainty since the specs for all these
 things are closed. We've done a lot of RE work, and it mostly appears
 to be a single global switch that turns any connected outputs. There is
 a table in the intel bios which can tell you about which outputs are muxed
 etc, but this isn't always present. Again we also have laptops that have
 a mux but don't expose this table, as they only have the MUX so the
 BIOS can pick IGP/discrete for Vista, and Windows 7 operates in
 muxless mode.

 The current problem is I'm not sure any OS exposes muxless and mux
 in one OS. Mac OSX always uses muxed, Vista the same, and I think
 Windows 7 always exposes muxless if the bios reports optimus support
 or the AMD equivalent.

All new AMD systems are muxless and I suspect most other vendors are
doing the same.  I'm wondering if there is any reason to bother with
proper muxed support at all?  We should be able to treat muxed systems
as muxless just fine.

Alex
___
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] xfree86: add autoAddGPU option (v2)

2012-07-03 Thread Dave Airlie
From: Dave Airlie airl...@redhat.com

This option is to stop the X server adding non-primary devices as
gpu screens.

v2: fix per Keith's suggestion.

Signed-off-by: Dave Airlie airl...@redhat.com
---
 hw/xfree86/common/xf86Config.c  |   15 ++-
 hw/xfree86/common/xf86Globals.c |9 +++--
 hw/xfree86/common/xf86Privstr.h |2 ++
 hw/xfree86/common/xf86platformBus.c |7 +++
 4 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index b22b617..edc0d3d 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -712,7 +712,8 @@ typedef enum {
 FLAG_AUTO_ENABLE_DEVICES,
 FLAG_GLX_VISUALS,
 FLAG_DRI2,
-FLAG_USE_SIGIO
+FLAG_USE_SIGIO,
+FLAG_AUTO_ADD_GPU,
 } FlagValues;
 
 /**
@@ -770,6 +771,8 @@ static OptionInfoRec FlagOptions[] = {
  {0}, FALSE},
 {FLAG_USE_SIGIO, UseSIGIO, OPTV_BOOLEAN,
  {0}, FALSE},
+{FLAG_AUTO_ADD_GPU, AutoAddGPU, OPTV_BOOLEAN,
+ {0}, FALSE},
 {-1, NULL, OPTV_NONE,
  {0}, FALSE},
 };
@@ -862,6 +865,16 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, 
XF86OptionPtr layoutopts)
 xf86Msg(from, %sutomatically enabling devices\n,
 xf86Info.autoEnableDevices ? A : Not a);
 
+if (xf86IsOptionSet(FlagOptions, FLAG_AUTO_ADD_GPU)) {
+xf86GetOptValBool(FlagOptions, FLAG_AUTO_ADD_GPU,
+  xf86Info.autoAddGPU);
+from = X_CONFIG;
+}
+else {
+from = X_DEFAULT;
+}
+xf86Msg(from, %sutomatically adding GPU devices\n,
+xf86Info.autoAddGPU ? A : Not a);
 /*
  * Set things up based on the config file information.  Some of these
  * settings may be overridden later when the command line options are
diff --git a/hw/xfree86/common/xf86Globals.c b/hw/xfree86/common/xf86Globals.c
index bb08917..7df7a80 100644
--- a/hw/xfree86/common/xf86Globals.c
+++ b/hw/xfree86/common/xf86Globals.c
@@ -126,11 +126,16 @@ xf86InfoRec xf86Info = {
 #if defined(CONFIG_HAL) || defined(CONFIG_UDEV) || defined(CONFIG_WSCONS)
 .forceInputDevices = FALSE,
 .autoAddDevices = TRUE,
-.autoEnableDevices = TRUE
+.autoEnableDevices = TRUE,
 #else
 .forceInputDevices = TRUE,
 .autoAddDevices = FALSE,
-.autoEnableDevices = FALSE
+.autoEnableDevices = FALSE,
+#endif
+#if defined(CONFIG_UDEV_KMS)
+.autoAddGPU = TRUE,
+#else
+.autoAddGPU = FALSE,
 #endif
 };
 
diff --git a/hw/xfree86/common/xf86Privstr.h b/hw/xfree86/common/xf86Privstr.h
index e78cd40..e20be03 100644
--- a/hw/xfree86/common/xf86Privstr.h
+++ b/hw/xfree86/common/xf86Privstr.h
@@ -110,6 +110,8 @@ typedef struct {
 
 Bool dri2;
 MessageType dri2From;
+
+Bool autoAddGPU;
 } xf86InfoRec, *xf86InfoPtr;
 
 #ifdef DPMSExtension
diff --git a/hw/xfree86/common/xf86platformBus.c 
b/hw/xfree86/common/xf86platformBus.c
index 0cc6c0a..3bfb22e 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -373,6 +373,13 @@ xf86platformProbeDev(DriverPtr drvp)
 continue;
 }
 
+/* if autoaddgpu devices is enabled then go find a few more and add them 
as GPU screens */
+if (xf86Info.autoAddGPU  numDevs) {
+for (j = 0; j  xf86_num_platform_devices; j++) {
+probeSingleDevice(xf86_platform_devices[j], drvp, devList[0], 
PLATFORM_PROBE_GPU_SCREEN);
+}
+}
+
 return foundScreen;
 }
 
-- 
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


Re: [PATCH] randr: add provider object (v7)

2012-07-03 Thread Dave Airlie
On Tue, Jul 3, 2012 at 2:57 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 7:57 AM, Dave Airlie airl...@gmail.com wrote:
 (forgot list first time I sent this).

 On Mon, Jul 2, 2012 at 10:03 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:54 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:


 Yes thats going to be the standard on switchable GPU machines, two 
 masters
 and the ability to jump between them. In that case max master is one, 
 and you'd
 have to set the provider roles. If maxmaster  1 then xinerama
 emulation is available.

 In those machines, isn't the limitation that only one of them can drive
 the LVDS panel at a time? Can't you have the internal GPU driving the
 LVDS while the external GPU drives other outputs?

 Yes but you don't configure it in xinerama mode for that, there are
 mux and muxless configurations.

 I think I understand what the hardware does now, just trying to figure
 out how to provide a reasonable description of that to applications
 while not just providing a big 'muxless/muxed' switch, which seems
 restricted to precisely how the hardware that we have works today.

 In mux configuration, you switch the mux between GPUs when the master
 is switched.

 Right, the mux just rewires things so that the other GPU is hooked up to
 the LVDS. I'd expect the LVDS outputs to reflect a suitable connection
 status for these changes.

 The only question is how you drive the mux switch. Is this switch
 selectable per-output? Or is is global? And, how do we label which
 outputs are affected by a global switch?

 We don't really know with 100% certainty since the specs for all these
 things are closed. We've done a lot of RE work, and it mostly appears
 to be a single global switch that turns any connected outputs. There is
 a table in the intel bios which can tell you about which outputs are muxed
 etc, but this isn't always present. Again we also have laptops that have
 a mux but don't expose this table, as they only have the MUX so the
 BIOS can pick IGP/discrete for Vista, and Windows 7 operates in
 muxless mode.

 The current problem is I'm not sure any OS exposes muxless and mux
 in one OS. Mac OSX always uses muxed, Vista the same, and I think
 Windows 7 always exposes muxless if the bios reports optimus support
 or the AMD equivalent.

 All new AMD systems are muxless and I suspect most other vendors are
 doing the same.  I'm wondering if there is any reason to bother with
 proper muxed support at all?  We should be able to treat muxed systems
 as muxless just fine.


Apple hw is the only one I know off persisting with a muxed design,

Whether it buys us much supporting the mux on it I'm not sure, I've no
idea to what degree they power down the intel hardware on it.

Dave.
___
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] dri2: add initial prime support. (v1.2)

2012-07-03 Thread Dave Airlie
From: Dave Airlie airl...@redhat.com

This adds the initial prime support for dri2 offload. The main thing is
when we get a connection from a prime client, we stored the information
and mark all drawables from that client as prime. We then create all
buffers for that drawable on the prime device dri2screen.

Then DRI2UpdatePrime is provided which drivers can call to get a shared
pixmap which they can use as the front buffer. The driver is then
responsible for doing the back-front copy to the shared buffer.

prime requires a compositing manager be run, but it handles the case where
a window get un-redirected by allocating a new pixmap and pointing the crtc
at it while the client is in that state.

Currently prime can't handle pageflipping, so always does straight copy swap,

v1.1: renumber on top of master.
v1.2: fix auth on top of master.

Signed-off-by: Dave Airlie airl...@redhat.com
---
 glx/glxdri2.c |2 +-
 hw/xfree86/dri2/dri2.c|  265 +
 hw/xfree86/dri2/dri2.h|   25 -
 hw/xfree86/dri2/dri2ext.c |4 +-
 4 files changed, 267 insertions(+), 29 deletions(-)

diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index 7b76c3a..984f115 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -840,7 +840,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
 return NULL;
 
 if (!xf86LoaderCheckSymbol(DRI2Connect) ||
-!DRI2Connect(pScreen, DRI2DriverDRI,
+!DRI2Connect(serverClient, pScreen, DRI2DriverDRI,
  screen-fd, driverName, deviceName)) {
 LogMessage(X_INFO,
AIGLX: Screen %d is not DRI2 capable\n, pScreen-myNum);
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index d3b3c73..0f87820 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -45,7 +45,7 @@
 #include dixstruct.h
 #include dri2.h
 #include xf86VGAarbiter.h
-
+#include damage.h
 #include xf86.h
 
 CARD8 dri2_major;   /* version of DRI2 supported by DDX */
@@ -63,6 +63,17 @@ static DevPrivateKeyRec dri2PixmapPrivateKeyRec;
 
 #define dri2PixmapPrivateKey (dri2PixmapPrivateKeyRec)
 
+static DevPrivateKeyRec dri2ClientPrivateKeyRec;
+
+#define dri2ClientPrivateKey (dri2ClientPrivateKeyRec)
+
+#define dri2ClientPrivate(_pClient) 
(dixLookupPrivate((_pClient)-devPrivates, \
+  dri2ClientPrivateKey))
+
+typedef struct _DRI2Client {
+int prime_id;
+} DRI2ClientRec, *DRI2ClientPtr;
+
 static RESTYPE dri2DrawableRes;
 
 typedef struct _DRI2Screen *DRI2ScreenPtr;
@@ -87,6 +98,9 @@ typedef struct _DRI2Drawable {
 int swap_limit; /* for N-buffering */
 unsigned long serialNumber;
 Bool needInvalidate;
+int prime_id;
+PixmapPtr prime_slave_pixmap;
+PixmapPtr redirectpixmap;
 } DRI2DrawableRec, *DRI2DrawablePtr;
 
 typedef struct _DRI2Screen {
@@ -113,14 +127,47 @@ typedef struct _DRI2Screen {
 HandleExposuresProcPtr HandleExposures;
 
 ConfigNotifyProcPtr ConfigNotify;
+DRI2CreateBuffer2ProcPtr CreateBuffer2;
+DRI2DestroyBuffer2ProcPtr DestroyBuffer2;
+DRI2CopyRegion2ProcPtr CopyRegion2;
 } DRI2ScreenRec;
 
+static void
+destroy_buffer(DrawablePtr pDraw, DRI2BufferPtr buffer, int prime_id);
+
 static DRI2ScreenPtr
 DRI2GetScreen(ScreenPtr pScreen)
 {
 return dixLookupPrivate(pScreen-devPrivates, dri2ScreenPrivateKey);
 }
 
+static ScreenPtr
+GetScreenPrime(ScreenPtr master, int prime_id)
+{
+ScreenPtr slave;
+int i;
+
+if (prime_id == 0 || xorg_list_is_empty(master-offload_slave_list)) {
+return master;
+}
+i = 0;
+xorg_list_for_each_entry(slave, master-offload_slave_list, offload_head) 
{
+if (i == (prime_id - 1))
+break;
+i++;
+}
+if (!slave)
+return master;
+return slave;
+}
+
+static DRI2ScreenPtr
+DRI2GetScreenPrime(ScreenPtr master, int prime_id)
+{
+ScreenPtr slave = GetScreenPrime(master, prime_id);
+return DRI2GetScreen(slave);
+}
+  
 static DRI2DrawablePtr
 DRI2GetDrawable(DrawablePtr pDraw)
 {
@@ -187,7 +234,8 @@ DRI2AllocateDrawable(DrawablePtr pDraw)
 xorg_list_init(pPriv-reference_list);
 pPriv-serialNumber = DRI2DrawableSerial(pDraw);
 pPriv-needInvalidate = FALSE;
-
+pPriv-redirectpixmap = NULL;
+pPriv-prime_slave_pixmap = NULL;
 if (pDraw-type == DRAWABLE_WINDOW) {
 pWin = (WindowPtr) pDraw;
 dixSetPrivate(pWin-devPrivates, dri2WindowPrivateKey, pPriv);
@@ -286,6 +334,7 @@ DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID 
id,
DRI2InvalidateProcPtr invalidate, void *priv)
 {
 DRI2DrawablePtr pPriv;
+DRI2ClientPtr dri2_client = dri2ClientPrivate(client);
 XID dri2_id;
 int rc;
 
@@ -295,6 +344,8 @@ DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID 
id,
 if (pPriv == NULL)
 return BadAlloc;
 
+pPriv-prime_id = dri2_client-prime_id;
+
 dri2_id = 

Re: [PATCH] randr: add provider object (v7)

2012-07-03 Thread Alex Deucher
On Tue, Jul 3, 2012 at 10:10 AM, Dave Airlie airl...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 2:57 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 7:57 AM, Dave Airlie airl...@gmail.com wrote:
 (forgot list first time I sent this).

 On Mon, Jul 2, 2012 at 10:03 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:54 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:


 Yes thats going to be the standard on switchable GPU machines, two 
 masters
 and the ability to jump between them. In that case max master is one, 
 and you'd
 have to set the provider roles. If maxmaster  1 then xinerama
 emulation is available.

 In those machines, isn't the limitation that only one of them can drive
 the LVDS panel at a time? Can't you have the internal GPU driving the
 LVDS while the external GPU drives other outputs?

 Yes but you don't configure it in xinerama mode for that, there are
 mux and muxless configurations.

 I think I understand what the hardware does now, just trying to figure
 out how to provide a reasonable description of that to applications
 while not just providing a big 'muxless/muxed' switch, which seems
 restricted to precisely how the hardware that we have works today.

 In mux configuration, you switch the mux between GPUs when the master
 is switched.

 Right, the mux just rewires things so that the other GPU is hooked up to
 the LVDS. I'd expect the LVDS outputs to reflect a suitable connection
 status for these changes.

 The only question is how you drive the mux switch. Is this switch
 selectable per-output? Or is is global? And, how do we label which
 outputs are affected by a global switch?

 We don't really know with 100% certainty since the specs for all these
 things are closed. We've done a lot of RE work, and it mostly appears
 to be a single global switch that turns any connected outputs. There is
 a table in the intel bios which can tell you about which outputs are muxed
 etc, but this isn't always present. Again we also have laptops that have
 a mux but don't expose this table, as they only have the MUX so the
 BIOS can pick IGP/discrete for Vista, and Windows 7 operates in
 muxless mode.

 The current problem is I'm not sure any OS exposes muxless and mux
 in one OS. Mac OSX always uses muxed, Vista the same, and I think
 Windows 7 always exposes muxless if the bios reports optimus support
 or the AMD equivalent.

 All new AMD systems are muxless and I suspect most other vendors are
 doing the same.  I'm wondering if there is any reason to bother with
 proper muxed support at all?  We should be able to treat muxed systems
 as muxless just fine.


 Apple hw is the only one I know off persisting with a muxed design,

 Whether it buys us much supporting the mux on it I'm not sure, I've no
 idea to what degree they power down the intel hardware on it.

I doubt they can do anything more than on the PC side otherwise the PC
side probably would have done it too.

Alex
___
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 25/36] randr: fixup constrain to work with slave screens.

2012-07-03 Thread Chris Bagwell
On Mon, Jul 2, 2012 at 5:13 AM, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com

 Current code constrains the cursor to the crtcs on the master
 device, for slave outputs to work we have to include their crtcs
 in the constrain calculations.

 Signed-off-by: Dave Airlie airl...@redhat.com
 ---
  randr/rrcrtc.c |   57 
 +---
  1 file changed, 46 insertions(+), 11 deletions(-)

 diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
 index 29b02a9..e5fe059 100644
 --- a/randr/rrcrtc.c
 +++ b/randr/rrcrtc.c
 @@ -1544,18 +1544,10 @@ ProcRRGetCrtcTransform(ClientPtr client)
  return Success;
  }

 -void
 -RRConstrainCursorHarder(DeviceIntPtr pDev, ScreenPtr pScreen, int mode, int 
 *x,
 -int *y)
 +static Bool check_all_screen_crtcs(ScreenPtr pScreen, int *x, int *y)
  {
  rrScrPriv(pScreen);
  int i;
 -
 -/* intentional dead space - let it float */
 -if (pScrPriv-discontiguous)
 -return;
 -
 -/* if we're moving inside a crtc, we're fine */
  for (i = 0; i  pScrPriv-numCrtcs; i++) {
  RRCrtcPtr crtc = pScrPriv-crtcs[i];

 @@ -1567,8 +1559,15 @@ RRConstrainCursorHarder(DeviceIntPtr pDev, ScreenPtr 
 pScreen, int mode, int *x,
  crtc_bounds(crtc, left, right, top, bottom);

  if ((*x = left)  (*x  right)  (*y = top)  (*y  bottom))
 -return;
 +return TRUE;
  }
 +return FALSE;
 +}
 +

I'm hoping I can take advantage of this section being reviewed as its
related to bug #39949.

This new check_all_screen_crtcs()'s is still calling unmodified
crtc_bounds().  That function computes bounds using
crtc-mode-mode.height/width.

If you are scaling display to greater than real size or using panning
then the bounds check causes cursor to get stuck to the smaller
portion of original screen size.

There is a proposed patch in bug report but perhaps this patch series
allows a better interface?  But probably not since I guess its
concerned more with interfaces to those original screen sizes being
hot plugged.

Chris
___
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 xorg-gtest] Rename headers to use dashes only

2012-07-03 Thread Chase Douglas

On 07/02/2012 08:17 PM, Peter Hutterer wrote:

Mixing dashes and spaces in xorg-gtest_foobar.h is a painful naming
convention. Use dashes only instead, and provide compat headers plus a
warning for those using the old header files.

Those users should be using xorg-gtest.h anyway.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net


So, basically this is one huge bike-shed commit. Since we're moving 
around what are supposed to be private implementation (even if it is in 
a public location), I'm ok with the change in theory. I'd rather leave 
it as is, but I have too many other things going on to care about this 
right now.


Reviewed-by: Chase Douglas chase.doug...@canonical.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


Re: [PATCH] randr: add provider object (v7)

2012-07-03 Thread Dave Airlie
On Tue, Jul 3, 2012 at 3:25 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 10:10 AM, Dave Airlie airl...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 2:57 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 7:57 AM, Dave Airlie airl...@gmail.com wrote:
 (forgot list first time I sent this).

 On Mon, Jul 2, 2012 at 10:03 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:54 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:


 Yes thats going to be the standard on switchable GPU machines, two 
 masters
 and the ability to jump between them. In that case max master is one, 
 and you'd
 have to set the provider roles. If maxmaster  1 then xinerama
 emulation is available.

 In those machines, isn't the limitation that only one of them can drive
 the LVDS panel at a time? Can't you have the internal GPU driving the
 LVDS while the external GPU drives other outputs?

 Yes but you don't configure it in xinerama mode for that, there are
 mux and muxless configurations.

 I think I understand what the hardware does now, just trying to figure
 out how to provide a reasonable description of that to applications
 while not just providing a big 'muxless/muxed' switch, which seems
 restricted to precisely how the hardware that we have works today.

 In mux configuration, you switch the mux between GPUs when the master
 is switched.

 Right, the mux just rewires things so that the other GPU is hooked up to
 the LVDS. I'd expect the LVDS outputs to reflect a suitable connection
 status for these changes.

 The only question is how you drive the mux switch. Is this switch
 selectable per-output? Or is is global? And, how do we label which
 outputs are affected by a global switch?

 We don't really know with 100% certainty since the specs for all these
 things are closed. We've done a lot of RE work, and it mostly appears
 to be a single global switch that turns any connected outputs. There is
 a table in the intel bios which can tell you about which outputs are muxed
 etc, but this isn't always present. Again we also have laptops that have
 a mux but don't expose this table, as they only have the MUX so the
 BIOS can pick IGP/discrete for Vista, and Windows 7 operates in
 muxless mode.

 The current problem is I'm not sure any OS exposes muxless and mux
 in one OS. Mac OSX always uses muxed, Vista the same, and I think
 Windows 7 always exposes muxless if the bios reports optimus support
 or the AMD equivalent.

 All new AMD systems are muxless and I suspect most other vendors are
 doing the same.  I'm wondering if there is any reason to bother with
 proper muxed support at all?  We should be able to treat muxed systems
 as muxless just fine.


 Apple hw is the only one I know off persisting with a muxed design,

 Whether it buys us much supporting the mux on it I'm not sure, I've no
 idea to what degree they power down the intel hardware on it.

 I doubt they can do anything more than on the PC side otherwise the PC
 side probably would have done it too.


Well they designed a MUX that wasn't pure shit, they have the ability
to do glitchess mux changes, where they set the intel refreshing the display
at 50Hz and the discrete at 60Hz, then when the vblanks crossover the chip
notices and does the switch. I don't think anyone in PC land had the
brains/balls
 to do such a thing ;-)

Dave.
___
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 xorg-gtest 1/9] Add a few linebreaks into the standard error notice

2012-07-03 Thread Chase Douglas

Everything in the series looks good.

Reviewed-by: Chase Douglas chase.doug...@canonical.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


Re: [PATCH] randr: add provider object (v7)

2012-07-03 Thread Alex Deucher
On Tue, Jul 3, 2012 at 10:49 AM, Dave Airlie airl...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 3:25 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 10:10 AM, Dave Airlie airl...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 2:57 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Tue, Jul 3, 2012 at 7:57 AM, Dave Airlie airl...@gmail.com wrote:
 (forgot list first time I sent this).

 On Mon, Jul 2, 2012 at 10:03 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:54 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:


 Yes thats going to be the standard on switchable GPU machines, two 
 masters
 and the ability to jump between them. In that case max master is one, 
 and you'd
 have to set the provider roles. If maxmaster  1 then xinerama
 emulation is available.

 In those machines, isn't the limitation that only one of them can drive
 the LVDS panel at a time? Can't you have the internal GPU driving the
 LVDS while the external GPU drives other outputs?

 Yes but you don't configure it in xinerama mode for that, there are
 mux and muxless configurations.

 I think I understand what the hardware does now, just trying to figure
 out how to provide a reasonable description of that to applications
 while not just providing a big 'muxless/muxed' switch, which seems
 restricted to precisely how the hardware that we have works today.

 In mux configuration, you switch the mux between GPUs when the master
 is switched.

 Right, the mux just rewires things so that the other GPU is hooked up to
 the LVDS. I'd expect the LVDS outputs to reflect a suitable connection
 status for these changes.

 The only question is how you drive the mux switch. Is this switch
 selectable per-output? Or is is global? And, how do we label which
 outputs are affected by a global switch?

 We don't really know with 100% certainty since the specs for all these
 things are closed. We've done a lot of RE work, and it mostly appears
 to be a single global switch that turns any connected outputs. There is
 a table in the intel bios which can tell you about which outputs are muxed
 etc, but this isn't always present. Again we also have laptops that have
 a mux but don't expose this table, as they only have the MUX so the
 BIOS can pick IGP/discrete for Vista, and Windows 7 operates in
 muxless mode.

 The current problem is I'm not sure any OS exposes muxless and mux
 in one OS. Mac OSX always uses muxed, Vista the same, and I think
 Windows 7 always exposes muxless if the bios reports optimus support
 or the AMD equivalent.

 All new AMD systems are muxless and I suspect most other vendors are
 doing the same.  I'm wondering if there is any reason to bother with
 proper muxed support at all?  We should be able to treat muxed systems
 as muxless just fine.


 Apple hw is the only one I know off persisting with a muxed design,

 Whether it buys us much supporting the mux on it I'm not sure, I've no
 idea to what degree they power down the intel hardware on it.

 I doubt they can do anything more than on the PC side otherwise the PC
 side probably would have done it too.


 Well they designed a MUX that wasn't pure shit, they have the ability
 to do glitchess mux changes, where they set the intel refreshing the display
 at 50Hz and the discrete at 60Hz, then when the vblanks crossover the chip
 notices and does the switch. I don't think anyone in PC land had the
 brains/balls
  to do such a thing ;-)

Does that work properly with DP or eDP?
___
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 xorg-gtest 01/16] Add a new class representing the X server

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

This class is a Process, so it's a drop-in replacement for the current
Environment, but it provides a few methods to talk to the server being
started.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  configure.ac|2 +-
  include/Makefile.am |1 +
  include/xorg/gtest/xorg-gtest-xserver.h |  113 ++
  include/xorg/gtest/xorg-gtest.h |1 +
  src/Makefile.am |1 +
  src/environment.cpp |   23 ++--
  src/xorg-gtest-all.cpp  |1 +
  src/xserver.cpp |  192 +++
  8 files changed, 323 insertions(+), 11 deletions(-)
  create mode 100644 include/xorg/gtest/xorg-gtest-xserver.h
  create mode 100644 src/xserver.cpp

diff --git a/configure.ac b/configure.ac
index 6e9e458..e46cc0b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,7 +25,7 @@ XORG_DEFAULT_OPTIONS
  XORG_ENABLE_INTEGRATION_TESTS([yes])
  XORG_WITH_DOXYGEN

-PKG_CHECK_MODULES(X11, x11)
+PKG_CHECK_MODULES(X11, x11 xi)

  # Check for Google Test
  CHECK_GTEST
diff --git a/include/Makefile.am b/include/Makefile.am
index 9ff5f2a..4a6ce03 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -34,6 +34,7 @@ nobase_include_HEADERS = \
xorg/gtest/xorg-gtest-environment.h \
xorg/gtest/xorg-gtest-process.h \
xorg/gtest/xorg-gtest-test.h \
+   xorg/gtest/xorg-gtest-xserver.h \
xorg/gtest/evemu/xorg-gtest-device.h \
xorg/gtest/xorg-gtest.h \
$(compat_headers)
diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
new file mode 100644
index 000..78f72ce
--- /dev/null
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -0,0 +1,113 @@
+/***
+ *
+ * X testing environment - Google Test helper class to communicate with the
+ * server
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * 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 and this permission notice (including the next
+ * paragraph) 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 THE
+ * AUTHORS OR COPYRIGHT HOLDERS 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.
+ *
+ 
**/
+
+
+#ifndef XORG_GTEST_XSERVER_H
+#define XORG_GTEST_XSERVER_H
+
+#include gtest/gtest.h
+#include xorg/gtest/xorg-gtest.h
+#include X11/Xlib.h
+
+namespace xorg {
+namespace testing {
+
+/**
+ * @class XServer xorg-gtest_xserver.h xorg/gtest/xorg-gtest_xserver.h
+ *
+ * Miscellaneous interfaces to communicate with the X server.
+ */
+class XServer : public xorg::testing::Process {
+  public:
+XServer();
+
+/**
+ * @param [in] display_number The display number the server runs on
+ */


Please provide a brief documentation for each function.


+void SetDisplayNumber(unsigned int display_number);
+
+/**
+ * Get the display string that may be used for XOpenDisplay to this
+ * server. This string is effectively :display_number.
+ *
+ * @return The display string used for XOpenDisplay() to this server.
+ */
+const char* GetDisplayString(void);
+
+/**
+ * Wait for a specific device to be added to the server.
+ *
+ * @param [in] display The X display connection
+ * @param [in] nameThe name of the device to wait for
+ * @param [in] timeout The timeout in milliseconds
+ *
+ * @return Whether the device was added
+ */
+static bool WaitForDevice(::Display *display, const std::string name, 
time_t timeout = 1000);
+
+/**
+ * Wait for an event on the X connection.
+ *
+ * @param [in] display The X display connection
+ * @param [in] timeout The timeout in milliseconds
+ *
+ * @return Whether an event is available
+ */
+static bool WaitForEvent(::Display *display, time_t timeout = 1000);
+
+/**
+ * Wait for an event of a specific type on the X connection.
+ *

Re: [PATCH xorg-gtest 02/16] xserver: add WaitForConnections()

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Moved from Environment to XServer class

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-xserver.h |5 +
  src/environment.cpp |   34 +-
  src/xserver.cpp |   35 +++
  3 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 78f72ce..c62970d 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -47,6 +47,11 @@ class XServer : public xorg::testing::Process {
  XServer();

  /**
+ * Waits until this server is ready to take connections.
+ */
+void WaitForConnections(void);
+
+/**
   * @param [in] display_number The display number the server runs on
   */
  void SetDisplayNumber(unsigned int display_number);
diff --git a/src/environment.cpp b/src/environment.cpp
index 43e1e70..5e7a156 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -147,42 +147,10 @@ void xorg::testing::Environment::SetUp() {
  -logfile, d_-path_to_log_file.c_str(),
  -config, d_-path_to_conf.c_str(),
  NULL);
-
d_-server.SetDisplayNumber(d_-display);
+  d_-server.WaitForConnections();

Process::SetEnv(DISPLAY, display_string, true);
-
-  for (int i = 0; i  10; ++i) {
-test_display = XOpenDisplay(NULL);
-
-if (test_display) {
-  XCloseDisplay(test_display);
-  return;
-}
-
-int status;
-int pid = waitpid(d_-server.Pid(), status, WNOHANG);
-if (pid == d_-server.Pid()) {
-  std::string message;
-  message += X server failed to start on display ;
-  message += display_string;
-  message += . Ensure that the \dummy\ video driver is installed.\n
- If the X.org server is older than 1.12, 
- tests will need to be run as root.\nCheck ;
-  message += d_-path_to_log_file;
-  message +=  for any errors;
-  throw std::runtime_error(message);
-} else if (pid == 0) {
-  sleep(1); /* Give the dummy X server some time to start */
-} else if (pid == -1) {
-  throw std::runtime_error(Could not get status of dummy X server 
-   process);
-} else {
-  throw std::runtime_error(Invalid child PID returned by 
Process::Wait());
-}
-  }
-
-  throw std::runtime_error(Unable to open connection to dummy X server);
  }

  void xorg::testing::Environment::TearDown() {
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 28130d7..ba847f1 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -41,6 +41,7 @@
  #include stdexcept
  #include vector

+#include X11/Xlib.h
  #include X11/extensions/XInput2.h

  struct xorg::testing::XServer::Private {
@@ -190,3 +191,37 @@ bool xorg::testing::XServer::WaitForDevice(::Display 
*display, const std::string

  return false;
  }
+
+void xorg::testing::XServer::WaitForConnections(void) {
+  for (int i = 0; i  10; ++i) {
+Display *test_display = XOpenDisplay(GetDisplayString());
+
+if (test_display) {
+  XCloseDisplay(test_display);
+  return;
+}
+
+int status;
+int pid = waitpid(Pid(), status, WNOHANG);
+if (pid == Pid()) {
+  std::string message;
+  message += X server failed to start on display ;
+  message +=  GetDisplayString();
+  message += . Ensure that the \dummy\ video driver is installed.\n
+ If the X.org server is older than 1.12, 
+ tests will need to be run as root.\nCheck ;
+  //message += d_-path_to_log_file;


How do you plan to fix this ^^?

If you get to it later on in the patch set, feel free to disregard :).


+  message +=  for any errors;
+  throw std::runtime_error(message);
+} else if (pid == 0) {
+  sleep(1); /* Give the dummy X server some time to start */
+} else if (pid == -1) {
+  throw std::runtime_error(Could not get status of dummy X server 
+   process);
+} else {
+  throw std::runtime_error(Invalid child PID returned by 
Process::Wait());
+}
+  }
+
+  throw std::runtime_error(Unable to open connection to dummy X server);
+}



___
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 xorg-gtest 04/16] xserver: move starting the process into the XServer object

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

The API currently means we lose the ability to pass arbitrary options (aside
from display/logfile/conf path), but this should be of minor impact only.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-xserver.h |6 ++
  src/environment.cpp |7 +--
  src/xserver.cpp |9 +
  3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index c5cb32d..5c5ce99 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -47,6 +47,12 @@ class XServer : public xorg::testing::Process {
  XServer();

  /**
+ * Start a new server
+ * @param [in] program Path to the XServer binary
+ */
+void Start(std::string program);


Instead of defining a new method altogether, I think we should be make 
xorg::testing::Process::Start virtual and override it here.


I also don't want to lose the ability to pass arbitrary arguments in. 
Somewhere down the road someone will want to pass a specific option in. 
For example, there may be a test to see if a specific argument modifies 
the server behavior. Can we make XServer::Start() work with var args?



+
+/**
   * Waits until this server is ready to take connections.
   */
  void WaitForConnections(void);
diff --git a/src/environment.cpp b/src/environment.cpp
index 2b3abca..7ed23b3 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -144,12 +144,7 @@ void xorg::testing::Environment::SetUp() {
d_-server.SetDisplayNumber(d_-display);
d_-server.SetLogfilePath(d_-path_to_log_file);
d_-server.SetConfigPath(d_-path_to_conf);
-  d_-server.Start(d_-path_to_server, d_-path_to_server.c_str(),
-display_string,
--logverbose, 10,
--logfile, d_-path_to_log_file.c_str(),
--config, d_-path_to_conf.c_str(),
-NULL);
+  d_-server.Start(d_-path_to_server);
d_-server.WaitForConnections();

Process::SetEnv(DISPLAY, display_string, true);
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 7f0483b..38394f3 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -249,3 +249,12 @@ void xorg::testing::XServer::WaitForConnections(void) {

throw std::runtime_error(Unable to open connection to dummy X server);
  }
+
+void xorg::testing::XServer::Start(std::string program) {
+  Process::Start(program, program.c_str(),
+ GetDisplayString(),
+ -logverbose, 10,
+ -logfile, d_-path_to_logfile.c_str(),
+ -config, d_-path_to_conf.c_str(),
+ NULL);
+}



___
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 xorg-gtest 05/16] xserver: move testing startup to the XServer object

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net


It makes sense to move the startup to the XServer object, but the 
environment should then contain an XServer object and start it. The 
point of the environment is that it starts up an xserver automatically.


The xorg-gtest user is free to not use the environment. But when they 
use the environment, it should start a server.



---
  include/xorg/gtest/xorg-gtest-xserver.h |2 ++
  src/environment.cpp |   35 ---
  src/xserver.cpp |   40 +++
  3 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 5c5ce99..52a2fd0 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -129,6 +129,8 @@ class XServer : public xorg::testing::Process {
  XServer(const XServer);
  XServer operator=(const XServer);

+void TestStartup(void);
+
  };
  } // namespace testing
  } // namespace xorg
diff --git a/src/environment.cpp b/src/environment.cpp
index 7ed23b3..69972a4 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -106,41 +106,6 @@ void xorg::testing::Environment::SetUp() {
static char display_string[6];
snprintf(display_string, 6, :%d, d_-display);

-  Display* test_display = XOpenDisplay(display_string);
-  if (test_display) {
-XCloseDisplay(test_display);
-std::string message;
-message += A server is already running on ;
-message += display_string;
-message += .;
-throw std::runtime_error(message);
-  }
-
-  /* The Xorg server won't start unless the log file and the old log file are
-   * writable. */
-  std::ofstream log_test;
-  log_test.open(d_-path_to_log_file.c_str(), std::ofstream::out);
-  log_test.close();
-  if (log_test.fail()) {
-std::string message;
-message += X.org server log file ;
-message += d_-path_to_log_file;
-message +=  is not writable.;
-throw std::runtime_error(message);
-  }
-
-  std::string old_log_file = d_-path_to_log_file.c_str();
-  old_log_file += .old;
-  log_test.open(old_log_file.c_str(), std::ofstream::out);
-  log_test.close();
-  if (log_test.fail()) {
-std::string message;
-message += X.org old server log file ;
-message += old_log_file;
-message +=  is not writable.;
-throw std::runtime_error(message);
-  }
-
d_-server.SetDisplayNumber(d_-display);
d_-server.SetLogfilePath(d_-path_to_log_file);
d_-server.SetConfigPath(d_-path_to_conf);
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 38394f3..1a46dbb 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -41,6 +41,7 @@
  #include cstring
  #include stdexcept
  #include vector
+#include fstream

  #include X11/Xlib.h
  #include X11/extensions/XInput2.h
@@ -250,7 +251,46 @@ void xorg::testing::XServer::WaitForConnections(void) {
throw std::runtime_error(Unable to open connection to dummy X server);
  }

+void xorg::testing::XServer::TestStartup(void) {
+  Display* test_display = XOpenDisplay(GetDisplayString());
+  if (test_display) {
+XCloseDisplay(test_display);
+std::string message;
+message += A server is already running on ;
+message += GetDisplayString();
+message += .;
+throw std::runtime_error(message);
+  }
+
+  /* The Xorg server won't start unless the log file and the old log file are
+   * writable. */
+  std::ofstream log_test;
+  log_test.open(d_-path_to_logfile.c_str(), std::ofstream::out);
+  log_test.close();
+  if (log_test.fail()) {
+std::string message;
+message += X.org server log file ;
+message += d_-path_to_logfile;
+message +=  is not writable.;
+throw std::runtime_error(message);
+  }
+
+  std::string old_log_file = d_-path_to_logfile.c_str();
+  old_log_file += .old;
+  log_test.open(old_log_file.c_str(), std::ofstream::out);
+  log_test.close();
+  if (log_test.fail()) {
+std::string message;
+message += X.org old server log file ;
+message += old_log_file;
+message +=  is not writable.;
+throw std::runtime_error(message);
+  }
+
+}
+
  void xorg::testing::XServer::Start(std::string program) {
+  TestStartup();
Process::Start(program, program.c_str(),
   GetDisplayString(),
   -logverbose, 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: [PATCH xorg-gtest 06/16] xserver: move Terminate and Kill handling here

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-xserver.h |   13 
  src/environment.cpp |   31 +++-
  src/xserver.cpp |   34 +++
  3 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 52a2fd0..821b01f 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -53,6 +53,19 @@ class XServer : public xorg::testing::Process {
  void Start(std::string program);

  /**
+ * Terminates this server process. Will signal the server to terminate
+ * multiple times before giving up.
+ *
+ * @return false if the server did not terminate, true otherwise
+ */
+bool Terminate(void);
+
+/**
+ * Kills the server. With a vengeance.
+ */
+bool Kill(void);


We don't need to recreate these functions. We've already inherited them 
from xorg::testing::Process. Those implementations should work 
automatically if we set up the XServer class properly.



+
+/**
   * Waits until this server is ready to take connections.
   */
  void WaitForConnections(void);
diff --git a/src/environment.cpp b/src/environment.cpp
index 69972a4..b041236 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -116,35 +116,10 @@ void xorg::testing::Environment::SetUp() {
  }

  void xorg::testing::Environment::TearDown() {
-  if (d_-server.Terminate()) {
-for (int i = 0; i  10; i++) {
-  int status;
-  int pid = waitpid(d_-server.Pid(), status, WNOHANG);
-
-  if (pid == d_-server.Pid())
-return;
-
-  sleep(1); /* Give the dummy X server more time to shut down */
-}
-  }
-
-  Kill();
+  if (!d_-server.Terminate())
+Kill();
  }

  void xorg::testing::Environment::Kill() {
-  if (!d_-server.Kill())
-std::cerr  Warning: Failed to kill dummy Xorg server: 
-   std::strerror(errno)  \n;
-
-  for (int i = 0; i  10; i++) {
-int status;
-int pid = waitpid(d_-server.Pid(), status, WNOHANG);
-
-if (pid == d_-server.Pid())
-  return;
-
-  sleep(1); /* Give the dummy X server more time to shut down */
-  }
-
-  std::cerr  Warning: Dummy X server did not shut down\n;
+  d_-server.Kill();
  }
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 1a46dbb..bd1e2f9 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -298,3 +298,37 @@ void xorg::testing::XServer::Start(std::string program) {
   -config, d_-path_to_conf.c_str(),
   NULL);
  }
+
+bool xorg::testing::XServer::Terminate(void) {
+  if (Process::Terminate()) {
+for (int i = 0; i  10; i++) {
+  int status;
+  int pid = waitpid(Pid(), status, WNOHANG);
+
+  if (pid == Pid())
+return true;
+
+  sleep(1); /* Give the dummy X server more time to shut down */
+}
+  }
+  return false;
+}
+
+bool xorg::testing::XServer::Kill(void) {
+  if (!Process::Kill())
+std::cerr  Warning: Failed to kill dummy Xorg server: 
+   std::strerror(errno)  \n;
+
+  for (int i = 0; i  10; i++) {
+int status;
+int pid = waitpid(Pid(), status, WNOHANG);
+
+if (pid == Pid())
+  return true;
+
+  sleep(1); /* Give the dummy X server more time to shut down */
+  }
+
+  std::cerr  Warning: Dummy X server did not shut down\n;
+  return false;
+}



___
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 xorg-gtest 07/16] xserver: add Start() for default binary path

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-xserver.h |4 
  src/xserver.cpp |4 
  2 files changed, 8 insertions(+)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 821b01f..1cd 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -47,6 +47,10 @@ class XServer : public xorg::testing::Process {
  XServer();

  /**
+ * Start a new server, using the default binary
+ */
+void Start(void);


This can be accomplished by defining the previous definition of Start() 
like:


void Start(std::string program = std::string());

(or something like that, the idea is to provide a default value)

Then, in the implementationm check program.empty(). If it is, then use 
the default value. If it isn't, then used the passed in value.


However, this interface just feels wrong. Why have we defined an 
internal path to the server, and then defined a method that ignores it? 
Ideally, at the XServer object construction time we either set a default 
program or we pass in the initial program. We then only provide getters 
and setters, and a method to start the program.



+/**
   * Start a new server
   * @param [in] program Path to the XServer binary
   */
diff --git a/src/xserver.cpp b/src/xserver.cpp
index bd1e2f9..160eadc 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -289,6 +289,10 @@ void xorg::testing::XServer::TestStartup(void) {

  }

+void xorg::testing::XServer::Start(void) {
+  Start(d_-path_to_server);
+}
+
  void xorg::testing::XServer::Start(std::string program) {
TestStartup();
Process::Start(program, program.c_str(),



___
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 xorg-gtest 08/16] xserver: return used display number from Start()

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Doesn't do much yet, but once we support -displayfd we can hook it up here.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-xserver.h |7 +--
  src/xserver.cpp |9 ++---
  2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 1cd..5cab01b 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -48,13 +48,16 @@ class XServer : public xorg::testing::Process {

  /**
   * Start a new server, using the default binary
+ *
+ * @return The display number this server is running on
   */
-void Start(void);
+unsigned int Start(void);
  /**
   * Start a new server
   * @param [in] program Path to the XServer binary
+ * @return The display number this server is running on
   */
-void Start(std::string program);
+unsigned int Start(std::string program);


I would rather we separate the starting from the getting of the display 
number. Basically, it's not obvious that you have to get the return 
value of Start() to determine the display number unless you look at the 
documentation. Can we add a getter method instead? It also would allow 
getting the display number after the server has already been started.




  /**
   * Terminates this server process. Will signal the server to terminate
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 160eadc..f5fd245 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -289,11 +289,11 @@ void xorg::testing::XServer::TestStartup(void) {

  }

-void xorg::testing::XServer::Start(void) {
-  Start(d_-path_to_server);
+unsigned int xorg::testing::XServer::Start(void) {
+  return Start(d_-path_to_server);
  }

-void xorg::testing::XServer::Start(std::string program) {
+unsigned int xorg::testing::XServer::Start(std::string program) {
TestStartup();
Process::Start(program, program.c_str(),
   GetDisplayString(),
@@ -301,6 +301,9 @@ void xorg::testing::XServer::Start(std::string program) {
   -logfile, d_-path_to_logfile.c_str(),
   -config, d_-path_to_conf.c_str(),
   NULL);
+
+  /* FIXME: use -displayfd here once the released servers support it */
+  return d_-display_number;
  }

  bool xorg::testing::XServer::Terminate(void) {



___
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 xorg-gtest 09/16] environment: remove default settings

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Keep those in the server only, not the environment. And only override the
build-in ones when they've been set by main.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  src/environment.cpp |   22 +-
  1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/environment.cpp b/src/environment.cpp
index b041236..01b2148 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -28,7 +28,6 @@
  #include xorg/gtest/xorg-gtest-environment.h
  #include xorg/gtest/xorg-gtest-process.h
  #include xorg/gtest/xorg-gtest-xserver.h
-#include defines.h

  #include sys/types.h
  #include unistd.h
@@ -44,11 +43,8 @@
  #include X11/Xlib.h

  struct xorg::testing::Environment::Private {
-  Private()
-  : path_to_conf(DUMMY_CONF_PATH), path_to_log_file(DEFAULT_XORG_LOGFILE),
-path_to_server(DEFAULT_XORG_SERVER), display(DEFAULT_DISPLAY) {
+  Private() : display(-1) {
}
-
std::string path_to_conf;
std::string path_to_log_file;
std::string path_to_server;
@@ -103,15 +99,23 @@ int xorg::testing::Environment::display() const
  }

  void xorg::testing::Environment::SetUp() {
+  unsigned int display_used;
static char display_string[6];
snprintf(display_string, 6, :%d, d_-display);

-  d_-server.SetDisplayNumber(d_-display);
-  d_-server.SetLogfilePath(d_-path_to_log_file);
-  d_-server.SetConfigPath(d_-path_to_conf);
-  d_-server.Start(d_-path_to_server);
+  if (d_-display = 0)
+d_-server.SetDisplayNumber(d_-display);
+  if (d_-path_to_log_file.length())
+d_-server.SetLogfilePath(d_-path_to_log_file);
+  if (d_-path_to_conf.length())
+d_-server.SetConfigPath(d_-path_to_conf);
+  if (d_-path_to_server.length())
+display_used = d_-server.Start(d_-path_to_server);
+  else
+display_used = d_-server.Start();
d_-server.WaitForConnections();

+  snprintf(display_string, 6, :%d, display_used);
Process::SetEnv(DISPLAY, display_string, true);
  }




Rather than store the values in two places (environment and server 
objects), we can simply remove the values from the environment and 
manipulate the server directly. Either:


* The environment has a method like:

XServer Environment::XServer();

And then you modify things like:

environment.XServer().SetDisplayNumber(value);

* Provide pass through functions like:

void Environment::SetDisplayNumber(int value) {
  d_-server.SetDisplayNumber(value);
}

And then you modify things like:

environment.SetDisplayNumber(value);

The first approach is a bit easier to code (less boilerplate 
pass-through functions), but it violates the Tell, don't ask policy 
that many object-oriented APIs follow. I would prefer the second 
approach because I think it is easier for the end-user to follow and 
understand. They don't have to hunt around in multiple classes to do 
what they want to do.


-- Chase
___
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] randr: add provider object (v7)

2012-07-03 Thread Keith Packard
Dave Airlie airl...@gmail.com writes:

 The current problem is I'm not sure any OS exposes muxless and mux
 in one OS. Mac OSX always uses muxed, Vista the same, and I think
 Windows 7 always exposes muxless if the bios reports optimus support
 or the AMD equivalent.

So we pick the simplest mux option that supports known Apple hardware,
which appears to use a global mux, at least as far as we can tell.

 That would again be a xinerama style situation though a lot more extreme
 and this isn't a use case I have expressed any interest in supporting. I've
 explicitly limited the use cases for this to avoid any sink holes. Like
 there are loads of what-if scenarios here, but I'm only interested in getting
 Linux to the level that Windows has been for years, and any diversions
 are just pushing the timeframe for that out.

Sounds good. One GPU at a time is certainly easier to manage
today.

 Again a corner case that you might thing is useful but not sure I've
 seen any indication of how we'd ever support or expose this from a desktop
 env.

Oh, I can imagine it -- DRI2 could provide a list of GPUs, the first of
which was the 'preferred' one. Something for a later adventure though.

 Again most people want to use the nvidia for video, vdpau kicks the ass
 of other things, at least under MacOSX they just switch off the intel
 when nvidia is running, and under Windows they don't expose it anymore
 to the OS. I'm not sure I want to start adding use cases they nobody
 has explored, it will just lead into ratholes.

I thought you were already providing this in the form of an off-load GPU
though. Are you saying that when you enable an off-load GPU, you disable
the internal GPU?

 I'm staying adamant that one provider is one GPU, however I could accept that
 splitting the master role into two roles and allowing GPUs to have
 multiple roles
 at once might make sense.

Right, what I was suggesting was that a 'master' provider is actually
just the union of an 'output slave' and a 'renderer'. You're already 

 Again we've currently got no idea how to do this, no manufacturer support
 or instructions. So I'm weary of introducing protocol for something
 I've no indications will ever be useful.

Are you saying that when you throw the mux switch that *all* of the
outputs from the disabled output slave are turned off? That's certainly
simpler than allowing some outputs to continue operating while others
are disabled. I'm pretty sure there are laptops where the external
monitor ports are connected only to the external GPU while the internal
panel can be switched, and I was trying to find a way to describe that
to clients so they would know which outputs were going to be disabled
when the mux was thrown.

 there is no usually. but Apple hw is always muxed, PC hardware
 is tending towards always being muxless. Not sure you can find a muxed
 PC new anymore.

Presumably cheaper this way...


 To map back to your model:

  * An output-only device (display link) would have hasGPU clear.

  * A GPU with no outputs (muxless external GPU) would have no crtcs or
outputs.

  * A 'master' device would have isPrimaryGPU and isActiveOutput both
set.

  * An 'output slave' device would have isPrimaryGPU clear and
isActiveOutput set

  * A rendering off-load engine would have neither set.

  * Setting 'isPrimaryGPU' and clearing 'isActiveOutput' would also work,
allowing for a muxless system to GPU-offload video and use the
overlays while using the external GPU for GL content.

 I think I like this up until the last one,

Cool. Just an attempt to try and describe things in a slightly more
orthogonal fashion.

 I'm still totally unsure of the last use case, what its for.

Are you saying it won't work, or that people won't use it?

 You seem to be advocating for some 3rd scenario, where the IGP
 is slaved to the discrete GPU, so it controls rendering/compositing,
 but we can expose the overlays on the IGP somehow, my thinking on this
 is probably wait for wayland, since I've no idea how we'd even do that on
 X now, or maybe I've totally missed the scenario.

Ignoring the overlays, which do seem complicated, aren't we already
going to be in this situation wrt the GPU? Is the compositing manager
going to restart and switch to the discrete GPU when we make that the
default for new applications?

-- 
keith.pack...@intel.com


pgptl1PgLjqmi.pgp
Description: PGP signature
___
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 xorg-gtest 10/16] xserver: replace separate paths with an option/value map

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-xserver.h |   18 
  src/xserver.cpp |   35 ---
  2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 5cab01b..707888e 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -82,10 +82,16 @@ class XServer : public xorg::testing::Process {
   */
  void SetDisplayNumber(unsigned int display_number);
  /**
+ * Set the logfile path for the server. This call is equivalent to
+ * XServer::SetOption(-logfile, path_to_logfile)
+ *
   * @param [in] path_to_logfile The path to the log file
   */
  void SetLogfilePath(std::string path_to_logfile);
  /**
+ * Set the logfile path for the server. This call is equivalent to
+ * XServer::SetOption(-config, path_to_conf)
+ *
   * @param [in] path_to_conf The path to the xorg.conf file
   */
  void SetConfigPath(std::string path_to_conf);
@@ -103,6 +109,18 @@ class XServer : public xorg::testing::Process {
  const char* GetDisplayString(void);

  /**
+ * Set startup options for the server. This is a more generic call
+ * to XServer::SetLogfilePath and XServer::SetConfigPath.
+ *
+ * For arguments that do not take/need a value, use the empty string as
+ * value.
+ *
+ * @param [in] key Commandline option
+ * @param [in] value Option value (if any)
+ */
+void SetOption(std::string key, std::string value);
+
+/**
   * Wait for a specific device to be added to the server.
   *
   * @param [in] display The X display connection
diff --git a/src/xserver.cpp b/src/xserver.cpp
index f5fd245..765feb1 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -42,6 +42,7 @@
  #include stdexcept
  #include vector
  #include fstream
+#include map

  #include X11/Xlib.h
  #include X11/extensions/XInput2.h
@@ -50,16 +51,22 @@ struct xorg::testing::XServer::Private {
Private()
: display_number(DEFAULT_DISPLAY),
  display_string(),
-path_to_logfile(DEFAULT_XORG_LOGFILE),
-path_to_conf(DUMMY_CONF_PATH),
  path_to_server(DEFAULT_XORG_SERVER) {
+
+std::string key, value;
+key = std::string(-config);
+value = std::string(DUMMY_CONF_PATH);
+options[key] = value;
+
+key = std::string(-logfile);
+value = std::string(DEFAULT_XORG_LOGFILE);
+options[key] = value;
}

unsigned int display_number;
std::string display_string;
-  std::string path_to_logfile;
-  std::string path_to_conf;
std::string path_to_server;
+  std::mapstd::string, std::string options;
  };

  xorg::testing::XServer::XServer() : d_(new Private) {
@@ -79,11 +86,11 @@ const char* xorg::testing::XServer::GetDisplayString(void) {
  }

  void xorg::testing::XServer::SetLogfilePath(std::string path_to_logfile) {
-d_-path_to_logfile = path_to_logfile;
+  SetOption(-logfile, path_to_logfile);
  }

  void xorg::testing::XServer::SetConfigPath(std::string path_to_conf) {
-d_-path_to_conf = path_to_conf;
+  SetOption(-config, path_to_conf);
  }

  void xorg::testing::XServer::SetServerPath(std::string path_to_server) {
@@ -235,7 +242,7 @@ void xorg::testing::XServer::WaitForConnections(void) {
message += . Ensure that the \dummy\ video driver is installed.\n
   If the X.org server is older than 1.12, 
   tests will need to be run as root.\nCheck ;
-  message += d_-path_to_logfile;
+  message += d_-options[-logfile];
message +=  for any errors;
throw std::runtime_error(message);
  } else if (pid == 0) {
@@ -265,17 +272,17 @@ void xorg::testing::XServer::TestStartup(void) {
/* The Xorg server won't start unless the log file and the old log file are
 * writable. */
std::ofstream log_test;
-  log_test.open(d_-path_to_logfile.c_str(), std::ofstream::out);
+  log_test.open(d_-options[-logfile].c_str(), std::ofstream::out);
log_test.close();
if (log_test.fail()) {
  std::string message;
  message += X.org server log file ;
-message += d_-path_to_logfile;
+message += d_-options[-logfile];
  message +=  is not writable.;
  throw std::runtime_error(message);
}

-  std::string old_log_file = d_-path_to_logfile.c_str();
+  std::string old_log_file = d_-options[-logfile].c_str();
old_log_file += .old;
log_test.open(old_log_file.c_str(), std::ofstream::out);
log_test.close();
@@ -298,8 +305,8 @@ unsigned int xorg::testing::XServer::Start(std::string 
program) {
Process::Start(program, program.c_str(),
   GetDisplayString(),
   -logverbose, 10,
- -logfile, d_-path_to_logfile.c_str(),
- -config, 

Re: [PATCH xorg-gtest 11/16] environment: use SetOption, instead of separate calls

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  src/environment.cpp |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/environment.cpp b/src/environment.cpp
index 01b2148..f28bec4 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -106,9 +106,9 @@ void xorg::testing::Environment::SetUp() {
if (d_-display = 0)
  d_-server.SetDisplayNumber(d_-display);
if (d_-path_to_log_file.length())
-d_-server.SetLogfilePath(d_-path_to_log_file);
+d_-server.SetOption(-logfile, d_-path_to_log_file);
if (d_-path_to_conf.length())
-d_-server.SetConfigPath(d_-path_to_conf);
+d_-server.SetOption(-config, d_-path_to_log_file);
if (d_-path_to_server.length())
  display_used = d_-server.Start(d_-path_to_server);
else


If we're going to do this, then we might as well just get rid of 
SetLogFilePath() and SetConfigPath().


I commented about this block not being necessary in a previous patch, so 
I'm going to give a tentative Reviewed-by in case I'm proven wrong :).


Reviewed-by: Chase Douglas chase.doug...@canonical.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


Re: [PATCH xorg-gtest 12/16] process: add Start(program, vectorchar*)

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Same thing as the va_list but takes a std::vector of arguments instead.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-process.h |   16 
  src/process.cpp |   20 +---
  2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/include/xorg/gtest/xorg-gtest-process.h 
b/include/xorg/gtest/xorg-gtest-process.h
index 47aefcf..c41cd9e 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -93,6 +93,22 @@ class Process {
/**
 * Starts a program as a child process.
 *
+   * See 'man execvp' for further information on the elements in
+   * the vector.
+   *
+   * @param program The program to start.
+   * @param args Vector of arguments passed to the program.
+   *
+   * @throws std::runtime_error on failure.
+   *
+   * @post If successful: Child process forked and program started.
+   * @post If successful: Subsequent calls to Pid() return child process pid.
+   */
+  void Start(const std::string program, std::vectorchar* args);


I don't like exporting a public API that is bad merely because it makes 
the implementation easier. In this case, one would expect that we pass 
in a vector of std::string arguments. Requiring the user to manually 
allocate c strings is a pain, especially if you want to do it in an 
exception-safe manner. Let's use a vector of strings here and do 
whatever we need to in the implementation to make it work.



+
+  /**
+   * Starts a program as a child process.
+   *
 * See 'man execvp' for further information on the variadic argument list.
 *
 * @param program The program to start.
diff --git a/src/process.cpp b/src/process.cpp
index e79b694..4c4a738 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -48,7 +48,7 @@ xorg::testing::Process::Process() : d_(new Private) {
d_-pid = -1;
  }

-void xorg::testing::Process::Start(const std::string program, va_list args) {
+void xorg::testing::Process::Start(const std::string program, std::vectorchar* 
argv) {
if (d_-pid != -1)
  throw std::runtime_error(Attempting to start an already started 
process);

@@ -61,18 +61,24 @@ void xorg::testing::Process::Start(const std::string 
program, va_list args) {
  close(1);
  close(2);

-std::vectorchar* argv;
-
-do
-  argv.push_back(va_arg(args, char*));
-while (argv.back());
-
+if (argv.back())
+  argv.push_back(NULL);
  execvp(program.c_str(), argv[0]);

  throw std::runtime_error(Failed to start process);
}
  }

+void xorg::testing::Process::Start(const std::string program, va_list args) {
+  std::vectorchar* argv;
+
+  do {
+argv.push_back(va_arg(args, char*));
+  } while (argv.back());
+
+  Start(program, argv);
+}
+
  void xorg::testing::Process::Start(const std::string program, ...) {
va_list list;
va_start(list, program);



___
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 xorg-gtest 13/16] xserver: use new Process::Start(vector) call to populate argument list

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

There's probably some better way to do this than strdup()ing everything,
send me a patch if you know how.


Hopefully it will be easier if we use a vector of std::strings, as I 
mentioned in the previous patch review :).



Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  src/xserver.cpp |   24 ++--
  1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/xserver.cpp b/src/xserver.cpp
index 765feb1..cc78d59 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -301,13 +301,25 @@ unsigned int xorg::testing::XServer::Start(void) {
  }

  unsigned int xorg::testing::XServer::Start(std::string program) {
+  std::vectorchar* args;
+  std::mapstd::string, std::string::iterator it;
+
TestStartup();
-  Process::Start(program, program.c_str(),
- GetDisplayString(),
- -logverbose, 10,
- -logfile, d_-options[-logfile].c_str(),
- -config, d_-options[-config].c_str(),
- NULL);
+
+  args.push_back(strdup(program.c_str()));
+  args.push_back(strdup(GetDisplayString()));
+
+  for (it = d_-options.begin(); it != d_-options.end(); it++) {
+args.push_back(strdup(it-first.c_str()));
+if (it-second.length())
+  args.push_back(strdup(it-second.c_str()));
+  }
+
+  Process::Start(program, args);
+
+  std::vectorchar*::iterator vit;
+  for (vit = args.begin(); vit != args.end(); vit++)
+free(*vit);

/* FIXME: use -displayfd here once the released servers support it */
return d_-display_number;



___
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/36] xfree86: add platform bus hotplug support (v2)

2012-07-03 Thread Keith Packard
Dave Airlie airl...@gmail.com writes:

 no further changes needed, granted I've no idea what drivers other
 than modesetting will ever do proper hotplug,
 I suppose I could get a real PCIE dock at some point and hotplug
 radeon or nouveau, but there is a lot of work
 kernel side before we could even start on the userspace drivers.

Definitely an adventure for a future time (where we imagine things like
Thunder Bolt monitors with integrated GPUs).

 with dual-gpu machines even if we power off the discrete we never
 unplug it, its all abstracted in the kernel.

Cool.

If we aren't dealing with hot-plug drivers (aside from modesetting),
then perhaps the privates issue is far less daunting than we feared.
The modesetting driver uses no privates, and so we need only get the
full set of privates allocated from non-hotplug drivers at server
startup time, leaving the current private allocation system in place.

-- 
keith.pack...@intel.com


pgpey4rVPkATC.pgp
Description: PGP signature
___
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 xorg-gtest 14/16] xserver: update documentation

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-xserver.h |   21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/xorg/gtest/xorg-gtest-xserver.h 
b/include/xorg/gtest/xorg-gtest-xserver.h
index 707888e..6eb2241 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -40,7 +40,26 @@ namespace testing {
  /**
   * @class XServer xorg-gtest_xserver.h xorg/gtest/xorg-gtest_xserver.h
   *
- * Miscellaneous interfaces to communicate with the X server.
+ * Class representing the X server process.
+ *
+ * @code
+ * XServer server;
+ * server.SetOption(-logfile, /tmp/Xserver.log);
+ *
+ * try {
+ *   server.Start();
+ * } catch (const std::runtime_error e) {
+ *   std::cerr  Problem starting the X server:   e.what()  std::endl;
+ * }


Generally, code shouldn't be using try...catch blocks unless it really 
expects a specific problem. If the X server won't start, then just let 
the test crash out (gtest will catch it and print the message for you).


The reason I mention this is that I don't think we generally want to 
lead people to adding try...catch blocks everwhere in their code, which 
is the wrong way to do proper exception handling. When done properly, 
exception handling is a lot easier, and usually don't have to use any 
try...catch blocks.


TBH, I've only recently learned how to do exception handling properly, 
so there's a chance you'll come across some of my own code that does it 
wrong too :).


All of that said, we probably should add an @throws doxygen comment to 
state that it throws a std::runtime_error if the process fails to start.



+ * ...
+ *
+ * if (!server.Terminate()) {
+ *   std::cerr  Problem terminating server ... killing now ...  
std::endl;
+ *   if (!server.Kill())
+ * std::cerr  Problem killing server  std::endl;
+ * }
+ * @endcode
   */
  class XServer : public xorg::testing::Process {
public:



___
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 18/36] privates: when pixmap privates increase, bump the totalPixmapSize

2012-07-03 Thread Keith Packard
Dave Airlie airl...@gmail.com writes:

 So I'm not sure what you mean by per-screen private data, drivers have
 requirements for per-pixmap private data as well, currently by modesetting
 driver stashes the slave framebuffer id from the kernel into a pixmap
 private.

By per-screen private data, I mean private data in objects which are
permanently associated with only one screen -- windows, pixmaps, gcs,
pictures, etc.

Those objects will only ever be used by one driver, and so private space
for all other drivers need not be allocated in them; DIX should be
overlaying those allocations so that each object contains only storage
for the driver which will use it.

Expanding the current private API to support this, with the private
structures themselves stored in a screen-private record so that it too
is per-screen, would save memory in multi-GPU systems, as well as making
it possible to reserve private space for new screens after the server
has started.

 Hence why I have to adjust the total pixmap size, though as ajax pointed
 out its a bit monotonically increasing for sanity, I'll have to track
 down it and see.

Given that the relevant drivers aren't ever being unloaded, I'm curious
as to why we see this increasing? Surely once the privates are allocated
once, they'll simply get re-used each time?

-- 
keith.pack...@intel.com


pgp9zTVTMDLCq.pgp
Description: PGP signature
___
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 19/36] dix: add ability to link output slave gpus to the current gpu

2012-07-03 Thread Keith Packard
Dave Airlie airl...@gmail.com writes:

 I could add
 assert(!new-current_master);
 which should be true.

Yeah, that's probably a good idea for now; I fear mis-use of this API
will end with slaves being on multiple masters...

-- 
keith.pack...@intel.com


pgpyTbWgfZTTx.pgp
Description: PGP signature
___
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 21/36] dix: pixmap sharing infrastructure (v2)

2012-07-03 Thread Keith Packard
Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 10:32 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 On Mon, Jul 2, 2012 at 8:20 PM, Keith Packard kei...@keithp.com wrote:

 though a void * that just cases an fd is probably okay.

 That was my thinking. void * covers a lot of sins.

 Okay I'll convert the 3 patches to use void *.

 I won't repost them, can you R-b this one then with that in mind?

Yes, Reviewed-by: Keith Packard kei...@keithp.com

-- 
keith.pack...@intel.com


pgpWapyzlVIoW.pgp
Description: PGP signature
___
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 23/36] randr: add initial scanout pixmap support (v2)

2012-07-03 Thread Keith Packard
Dave Airlie airl...@gmail.com writes:

 So it made sense to stick it in the pixmap when I had two users, I
 suppose I could
 try and stash it somewhere else in each user but it might get a bit
 ugly.

Meh. It's a bit weird to have it in the pixmap, but I guess it does seem
simplest.

-- 
keith.pack...@intel.com


pgpcfueRsCiux.pgp
Description: PGP signature
___
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 xorg-gtest 15/16] test: add SetDisplayString()

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

Don't rely on the environment alone, take the display string if it's been
set.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/xorg-gtest-test.h |7 +++
  src/test.cpp |8 +++-
  2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/xorg/gtest/xorg-gtest-test.h 
b/include/xorg/gtest/xorg-gtest-test.h
index 3b78a17..3338c80 100644
--- a/include/xorg/gtest/xorg-gtest-test.h
+++ b/include/xorg/gtest/xorg-gtest-test.h
@@ -88,6 +88,13 @@ class Test : public ::testing::Test {
 */
::Display* Display() const;

+  /**
+   * Set the display string used for XOpenDisplay()
+   *


I think we need to add a detailed comment like:


In order to set the display string, create a subclass and override 
SetUp(). In the subclass SetUp(), set the display string and then call 
xorg::testing::Test::Setup().



IIRC, that's the only way this method can be used effectively.


+   * @param display The string representing the display connection, or NULL
+   */
+  void SetDisplayString(const char *display);


Let's pass in a const std::string instead. It's easy to convert to a c 
string in the implementation, you can still pass a string literal into 
the function (it gets converted automatically), and it's a more natural 
c++ api.



+
/** @cond Implementation */
struct Private;
std::auto_ptrPrivate d_;
diff --git a/src/test.cpp b/src/test.cpp
index e3e65e4..499915b 100644
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -33,16 +33,18 @@

  struct xorg::testing::Test::Private {
::Display* display;
+  const char *display_string;
  };

  xorg::testing::Test::Test() : d_(new Private) {
d_-display = NULL;
+  d_-display_string = NULL;
  }

  xorg::testing::Test::~Test() {}

  void xorg::testing::Test::SetUp() {
-  d_-display = XOpenDisplay(NULL);
+  d_-display = XOpenDisplay(d_-display_string);
if (!d_-display)
  throw std::runtime_error(Failed to open connection to display);
  }
@@ -56,3 +58,7 @@ void xorg::testing::Test::TearDown() {
  ::Display* xorg::testing::Test::Display() const {
return d_-display;
  }
+
+void xorg::testing::Test::SetDisplayString(const char *display) {
+  d_-display_string = display;
+}



___
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 xorg-gtest 16/16] Add Device::GetDeviceNode() to return device node path from an evemu device

2012-07-03 Thread Chase Douglas

On 07/02/2012 11:44 PM, Peter Hutterer wrote:

evemu doesn't export this information and even evemu-device just trawls
through the file system to print this info. So do the same here, noting the
time before evemu_create() and the ctime of the new device file. If the
latter is later than the former and the device names match, we can assume
this is our device.


I just want to point out that it's a deficiency in uinput, not in evemu, 
in case anyone had any thoughts of trying to fix evemu instead.



Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  include/xorg/gtest/evemu/xorg-gtest-device.h |   13 ++
  src/device.cpp   |   65 +-
  2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/include/xorg/gtest/evemu/xorg-gtest-device.h 
b/include/xorg/gtest/evemu/xorg-gtest-device.h
index 5f8faa2..359e65b 100644
--- a/include/xorg/gtest/evemu/xorg-gtest-device.h
+++ b/include/xorg/gtest/evemu/xorg-gtest-device.h
@@ -75,6 +75,17 @@ class Device {
 */
void Play(const std::string path) const;

+  /**
+   * Return the /dev/input/eventX device node for this device.
+   *
+   * Note that evemu doesn't know the device node, so we traverse the file
+   * system looking for it. There is a tiny chance of the device node being
+   * wrong.
+   *
+   * @return The string representing the device node
+   */
+  const char* GetDeviceNode(void);
+
   private:
struct Private;
std::auto_ptrPrivate d_;
@@ -82,6 +93,8 @@ class Device {
/* Disable copy constructor  assignment operator */
Device(const Device);
Device operator=(const Device);
+
+  void GuessDeviceNode(time_t ctime);
  };

  } // namespace evemu
diff --git a/src/device.cpp b/src/device.cpp
index 226d4e0..555e3e0 100644
--- a/src/device.cpp
+++ b/src/device.cpp
@@ -28,18 +28,74 @@
  #include xorg/gtest/evemu/xorg-gtest-device.h

  #include fcntl.h
+#include dirent.h

  #include stdexcept

  #include gtest/gtest.h

+#define SYS_INPUT_DIR /sys/class/input
+#define DEV_INPUT_DIR /dev/input/
+
  struct xorg::testing::evemu::Device::Private {
-  Private() : fd(-1), device(NULL) {}
+  Private() : fd(-1), device(NULL), device_node() {}

int fd;
struct evemu_device* device;
+  std::string device_node;
  };

+static int _event_device_compare(const struct dirent **a,
+ const struct dirent **b) {
+  int na, nb;
+
+  sscanf((*a)-d_name, event%d, na);
+  sscanf((*b)-d_name, event%d, nb);
+
+  return (na  nb) ? 1 : (na  nb) ? -1 : 0;
+
+}
+
+static int _event_device_filter(const struct dirent *d) {
+  return (strncmp(event, d-d_name, sizeof(event) - 1) == 0);
+}
+
+void xorg::testing::evemu::Device::GuessDeviceNode(time_t ctime) {
+  struct dirent **event_devices;
+  int n_event_devices;
+
+  n_event_devices = scandir(SYS_INPUT_DIR, event_devices,
+_event_device_filter, _event_device_compare);
+
+  if (n_event_devices  0) {
+std::cerr  Failed to guess device node.  std::endl;
+return;
+  }
+
+  bool found = false;
+  for (int i = 0; i  n_event_devices  !found; i++) {
+std::stringstream s;
+s  DEV_INPUT_DIR  event_devices[i]-d_name;
+
+int fd = open(s.str().c_str(), O_RDONLY);
+char device_name[256];
+
+ioctl(fd, EVIOCGNAME(sizeof(device_name)), device_name);
+if (strcmp(device_name, evemu_get_name(d_-device)) == 0) {
+  struct stat buf;
+  if (fstat(fd, buf) == 0) {
+if (buf.st_ctime = ctime) {
+  d_-device_node = s.str();
+  found = true;
+}
+  }
+}
+close(fd);
+free(event_devices[i]);


We need to free each event device entry, but if we break out of the 
loops early because we've found a matching device, then we won't free 
the rest of the devices in the array.



+  }
+  free(event_devices);
+}
+
  xorg::testing::evemu::Device::Device(const std::string path)
  : d_(new Private) {
static const char UINPUT_NODE[] = /dev/uinput;
@@ -68,11 +124,14 @@ xorg::testing::evemu::Device::Device(const std::string 
path)
  throw std::runtime_error(Failed to open uinput node);
}

+  time_t ctime = time(NULL);
if (evemu_create(d_-device, d_-fd)  0) {
  close(d_-fd);
  evemu_delete(d_-device);
  throw std::runtime_error(Failed to create evemu device);
}
+
+  GuessDeviceNode(ctime);
  }

  void xorg::testing::evemu::Device::Play(const std::string path) const {
@@ -88,6 +147,10 @@ void xorg::testing::evemu::Device::Play(const std::string 
path) const {
fclose(file);
  }

+const char* xorg::testing::evemu::Device::GetDeviceNode(void) {
+  return d_-device_node.length()  0 ? d_-device_node.c_str() : NULL;
+}
+
  xorg::testing::evemu::Device::~Device() {
close(d_-fd);
evemu_delete(d_-device);



___
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 18/36] privates: when pixmap privates increase, bump the totalPixmapSize

2012-07-03 Thread Dave Airlie
On Tue, Jul 3, 2012 at 7:26 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@gmail.com writes:

 So I'm not sure what you mean by per-screen private data, drivers have
 requirements for per-pixmap private data as well, currently by modesetting
 driver stashes the slave framebuffer id from the kernel into a pixmap
 private.

 By per-screen private data, I mean private data in objects which are
 permanently associated with only one screen -- windows, pixmaps, gcs,
 pictures, etc.

 Those objects will only ever be used by one driver, and so private space
 for all other drivers need not be allocated in them; DIX should be
 overlaying those allocations so that each object contains only storage
 for the driver which will use it.

This sounds like what we want, do we have any examples of this sort of
thing already, I could take a look tomorrow, though it would be a bit of
a change to drivers that currently allocate privates.

 Expanding the current private API to support this, with the private
 structures themselves stored in a screen-private record so that it too
 is per-screen, would save memory in multi-GPU systems, as well as making
 it possible to reserve private space for new screens after the server
 has started.

 Hence why I have to adjust the total pixmap size, though as ajax pointed
 out its a bit monotonically increasing for sanity, I'll have to track
 down it and see.

 Given that the relevant drivers aren't ever being unloaded, I'm curious
 as to why we see this increasing? Surely once the privates are allocated
 once, they'll simply get re-used each time?

well modesetting Init adds the private allocation, but since nobody binds to
it initially it never gets its private allocated, until something hotplugs later
then it increases after CSR has run.

Dave.
___
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 25/36] randr: fixup constrain to work with slave screens.

2012-07-03 Thread Keith Packard
Dave Airlie airl...@gmail.com writes:

 I don't think its that bad, I think I got most of them, once the user has 
 looked
 up the crtc XID, you don't care about the screen that often.

Seems like the following will need changes:

ProcRRSelectInput

Must send CRTC/OUTPUT events for slave outputs too

TellChanged

This one has some changes, but it doesn't send CRTC or
OUTPUT events for output slave screens.

RRFirstOutput

There may only be slave outputs running, so it needs to
look there as well

RRModesForScreen

ProcRRSetScreenSize

Screen size needs to cover the slave outputs as well

RRXineramaScreenCount
ProcRRXineramaQueryScreens

Xinerama needs to report all of the crtcs, not just the
master ones.

-- 
keith.pack...@intel.com


pgp4IplrVQ4cq.pgp
Description: PGP signature
___
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: add initial prime support. (v1.2)

2012-07-03 Thread Mario Kleiner

On 03.07.2012, at 16:22, Dave Airlie wrote:

 From: Dave Airlie airl...@redhat.com
 
 This adds the initial prime support for dri2 offload. The main thing is
 when we get a connection from a prime client, we stored the information
 and mark all drawables from that client as prime. We then create all
 buffers for that drawable on the prime device dri2screen.
 
 Then DRI2UpdatePrime is provided which drivers can call to get a shared
 pixmap which they can use as the front buffer. The driver is then
 responsible for doing the back-front copy to the shared buffer.
 
 prime requires a compositing manager be run, but it handles the case where
 a window get un-redirected by allocating a new pixmap and pointing the crtc
 at it while the client is in that state.
 
 Currently prime can't handle pageflipping, so always does straight copy swap,
 
 v1.1: renumber on top of master.
 v1.2: fix auth on top of master.
 
 Signed-off-by: Dave Airlie airl...@redhat.com
 ---
 glx/glxdri2.c |2 +-
 hw/xfree86/dri2/dri2.c|  265 +
 hw/xfree86/dri2/dri2.h|   25 -
 hw/xfree86/dri2/dri2ext.c |4 +-
 4 files changed, 267 insertions(+), 29 deletions(-)
 
 diff --git a/glx/glxdri2.c b/glx/glxdri2.c
 index 7b76c3a..984f115 100644
 --- a/glx/glxdri2.c
 +++ b/glx/glxdri2.c
 @@ -840,7 +840,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
 return NULL;
 
 if (!xf86LoaderCheckSymbol(DRI2Connect) ||
 -!DRI2Connect(pScreen, DRI2DriverDRI,
 +!DRI2Connect(serverClient, pScreen, DRI2DriverDRI,
  screen-fd, driverName, deviceName)) {
 LogMessage(X_INFO,
AIGLX: Screen %d is not DRI2 capable\n, pScreen-myNum);
 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index d3b3c73..0f87820 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -45,7 +45,7 @@
 #include dixstruct.h
 #include dri2.h
 #include xf86VGAarbiter.h
 -
 +#include damage.h
 #include xf86.h
 
 CARD8 dri2_major;   /* version of DRI2 supported by DDX */
 @@ -63,6 +63,17 @@ static DevPrivateKeyRec dri2PixmapPrivateKeyRec;
 
 #define dri2PixmapPrivateKey (dri2PixmapPrivateKeyRec)
 
 +static DevPrivateKeyRec dri2ClientPrivateKeyRec;
 +
 +#define dri2ClientPrivateKey (dri2ClientPrivateKeyRec)
 +
 +#define dri2ClientPrivate(_pClient) 
 (dixLookupPrivate((_pClient)-devPrivates, \
 +  dri2ClientPrivateKey))
 +
 +typedef struct _DRI2Client {
 +int prime_id;
 +} DRI2ClientRec, *DRI2ClientPtr;
 +
 static RESTYPE dri2DrawableRes;
 
 typedef struct _DRI2Screen *DRI2ScreenPtr;
 @@ -87,6 +98,9 @@ typedef struct _DRI2Drawable {
 int swap_limit; /* for N-buffering */
 unsigned long serialNumber;
 Bool needInvalidate;
 +int prime_id;
 +PixmapPtr prime_slave_pixmap;
 +PixmapPtr redirectpixmap;
 } DRI2DrawableRec, *DRI2DrawablePtr;
 
 typedef struct _DRI2Screen {
 @@ -113,14 +127,47 @@ typedef struct _DRI2Screen {
 HandleExposuresProcPtr HandleExposures;
 
 ConfigNotifyProcPtr ConfigNotify;
 +DRI2CreateBuffer2ProcPtr CreateBuffer2;
 +DRI2DestroyBuffer2ProcPtr DestroyBuffer2;
 +DRI2CopyRegion2ProcPtr CopyRegion2;
 } DRI2ScreenRec;
 
 +static void
 +destroy_buffer(DrawablePtr pDraw, DRI2BufferPtr buffer, int prime_id);
 +
 static DRI2ScreenPtr
 DRI2GetScreen(ScreenPtr pScreen)
 {
 return dixLookupPrivate(pScreen-devPrivates, dri2ScreenPrivateKey);
 }
 
 +static ScreenPtr
 +GetScreenPrime(ScreenPtr master, int prime_id)
 +{
 +ScreenPtr slave;
 +int i;
 +
 +if (prime_id == 0 || xorg_list_is_empty(master-offload_slave_list)) {
 +return master;
 +}
 +i = 0;
 +xorg_list_for_each_entry(slave, master-offload_slave_list, 
 offload_head) {
 +if (i == (prime_id - 1))
 +break;
 +i++;
 +}
 +if (!slave)
 +return master;
 +return slave;
 +}
 +
 +static DRI2ScreenPtr
 +DRI2GetScreenPrime(ScreenPtr master, int prime_id)
 +{
 +ScreenPtr slave = GetScreenPrime(master, prime_id);
 +return DRI2GetScreen(slave);
 +}
 +  
 static DRI2DrawablePtr
 DRI2GetDrawable(DrawablePtr pDraw)
 {
 @@ -187,7 +234,8 @@ DRI2AllocateDrawable(DrawablePtr pDraw)
 xorg_list_init(pPriv-reference_list);
 pPriv-serialNumber = DRI2DrawableSerial(pDraw);
 pPriv-needInvalidate = FALSE;
 -
 +pPriv-redirectpixmap = NULL;
 +pPriv-prime_slave_pixmap = NULL;
 if (pDraw-type == DRAWABLE_WINDOW) {
 pWin = (WindowPtr) pDraw;
 dixSetPrivate(pWin-devPrivates, dri2WindowPrivateKey, pPriv);
 @@ -286,6 +334,7 @@ DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, 
 XID id,
DRI2InvalidateProcPtr invalidate, void *priv)
 {
 DRI2DrawablePtr pPriv;
 +DRI2ClientPtr dri2_client = dri2ClientPrivate(client);
 XID dri2_id;
 int rc;
 
 @@ -295,6 +344,8 @@ DRI2CreateDrawable(ClientPtr client, 

Re: [PATCH 18/36] privates: when pixmap privates increase, bump the totalPixmapSize

2012-07-03 Thread Keith Packard
Dave Airlie airl...@gmail.com writes:

 On Tue, Jul 3, 2012 at 7:26 PM, Keith Packard kei...@keithp.com wrote:

 This sounds like what we want, do we have any examples of this sort of
 thing already, I could take a look tomorrow, though it would be a bit of
 a change to drivers that currently allocate privates.

Not that I know of. In ancient times, cfb and mfb shared a block of
privates by using the same private index with different contents.

Adding this notion to the privates code should be pretty easy; it just
makes the screen-related object sizes screen-specific.

 well modesetting Init adds the private allocation, but since nobody binds to
 it initially it never gets its private allocated, until something hotplugs 
 later
 then it increases after CSR has run.

I can't find any private allocation done by the modesetting driver; did
I miss it?

-- 
keith.pack...@intel.com


pgp1jvzUuMiTA.pgp
Description: PGP signature
___
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

  1   2   >