[PATCH xorg-gtest 5/9] Split usage help text into a helper function
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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()
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
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
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
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*)
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
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
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()
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
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
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
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
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
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
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*
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
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
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
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
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
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.
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)
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.
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)
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)
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.
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)
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)
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)
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
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.
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
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
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.
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
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.
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)
(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
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.
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
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)
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)
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)
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)
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)
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.
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
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)
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
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)
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
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()
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
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
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
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
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()
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
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)
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
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
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*)
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
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)
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
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
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
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)
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)
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()
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
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
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.
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)
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
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