Re: [PATCH synaptics] conf: ship a quirk for Cypress touchpads
On Mon, 2014-03-24 at 09:45 +1000, Peter Hutterer wrote: other than that this seems to be the best solution, thanks. Reviewed-by: Peter Hutterer peter.hutte...@who-t.net though I'll still ship the xorg.conf snippet in fedora until this filters down. The kernel package maintainers might not mind carrying it... -- Adam Williamson Fedora QA Community Monkey IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net http://www.happyassassin.net ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] fb: fix fast-path detection
The width parameter is used to restrict the blit fast-path (memcpy) when the source and destination rows overlap. This check was added in [0]. Unfortunately, the calculation to determine if source and destination lines overlapped was incorrect: (1) it trys to convert width from pixels to bytes, but width is actually in bits, not pixels. (2) it adds this byte offset to a FbBits * which implicitly converts the offset to pixels (FbBits). Fix both of these by converting width from bits to pixels, and let the pointer arithmetic convert to byte addresses. For example: A 32-bpp 1366 pixel-wide row will have width = 1366 * 32 = 43712 bits bpp = 32 (bpp 3) = 4 width * (bpp 3) = 174848 pixels (FbBits *)width = 699392 bytes So, careful would be true if the destination line is withing 699392 bytes, instead of just within its 1366 * 4 = 5464 byte row. This bug causes us to take the slow path for large non-overlapping rows that are close in memory. As a data point, XGetImage(1366x768) on my ARM chromebook was taking ~140 ms, but with this fixed, it now takes about 60 ms. XGetImage() - exaGetImage() - fbGetImage - fbBlt() [0] commit e32cc0b4c85c78cd8743a6e1680dcc79054b57ce Author: Adam Jackson a...@redhat.com Date: Thu Apr 21 16:37:11 2011 -0400 fb: Fix memcpy abuse The memcpy fast path implicitly assumes that the copy walks left-to-right. That's not something memcpy guarantees, and newer glibc on some processors will indeed break that assumption. Since we walk a line at a time, check the source and destination against the width of the blit to determine whether we can be sloppy enough to allow memcpy. (Having done this, we can remove the check for !reverse as well.) Signed-off-by: Daniel Kurtz djku...@chromium.org --- fb/fbblt.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/fb/fbblt.c b/fb/fbblt.c index 72a05f6..804137c 100644 --- a/fb/fbblt.c +++ b/fb/fbblt.c @@ -40,13 +40,13 @@ } void -fbBlt(FbBits * srcLine, - FbStride srcStride, - int srcX, - FbBits * dstLine, - FbStride dstStride, - int dstX, - int width, +fbBlt(FbBits * srcLine,/* pixels */ + FbStride srcStride, /* pixels */ + int srcX,/* bits */ + FbBits * dstLine,/* pixels */ + FbStride dstStride, /* pixels */ + int dstX,/* bits */ + int width, /* bits */ int height, int alu, FbBits pm, int bpp, Bool reverse, Bool upsidedown) { FbBits *src, *dst; @@ -66,8 +66,9 @@ fbBlt(FbBits * srcLine, return; } -careful = !((srcLine dstLine srcLine + width * (bpp 3) dstLine) || -(dstLine srcLine dstLine + width * (bpp 3) srcLine)) +/* We must be careful if src and dst regions overlap */ +careful = ((srcLine dstLine srcLine + (width / bpp) dstLine) || +(dstLine srcLine dstLine + (width / bpp) srcLine)) || (bpp 7); if (alu == GXcopy pm == FB_ALLONES !careful -- 1.9.1.423.g4596e3a ___ xorg-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 synaptics] conf: ship a quirk for Cypress touchpads
Hi, On 03/24/2014 12:45 AM, Peter Hutterer wrote: On Fri, Mar 21, 2014 at 11:03:36AM +0100, Hans de Goede wrote: Hi, On 03/21/2014 10:31 AM, Hans de Goede wrote: Hi, On 03/20/2014 11:18 PM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- conf/11-x11-synaptics.fdi | 9 + conf/50-synaptics.conf| 12 2 files changed, 21 insertions(+) diff --git a/conf/11-x11-synaptics.fdi b/conf/11-x11-synaptics.fdi index a898875..44dfce3 100644 --- a/conf/11-x11-synaptics.fdi +++ b/conf/11-x11-synaptics.fdi @@ -34,6 +34,15 @@ match key=info.product contains=Apple|bcm5974 merge key=input.x11_options.SoftButtonAreas type=string0 0 0 0 0 0 0 0/merge /match + +!--The Cypress touchpads provide BTN_RIGHT in firmware, together with +clickfinger, and two-finger scrolling. Disable Clickpads, otherwise we +get flaky button behaviour. +https://bugs.freedesktop.org/show_bug.cgi?id=70819 +https://bugs.freedesktop.org/show_bug.cgi?id=76341 -- I'm not so sure that using a xorg.conf snippet for this is the best way to go about this, I would prefer to see a fix to the kernel to not report the clickpad flag for non clickpads. Esp. as some newer or future Cypress touchpads may very well get rid of the firmware mode and do everything in the drivers as other vendors do. I really think that CyPS/2 Cypress Trackpad is a way to generic match string and is going to bite us in the future. At a minimum we should use the still to be merged DMI matching here to limit this to certain laptop models, but IHMO it would be much better to simply fix this in the kernel. For starters we could add a dmi quirk in the kernel for the XPS 13, until we figure out a better way to decide which Cypress Trackpads use this firmware emulation and which models don't. And maybe the one in the Dell XPS 13 even has a command we can send to it to turn of the emulation too, which would allow us to really fix 70819 too. So I've spend some time reading the kernel cypress_ps2 driver, conclusions: 1) It can only do multi-touch for upto 2 fingers, it can detect tripple / quad fingers being down but will only report coordinates for 2. 2) It seems to do a lot of processing in firmware, so in general at least for the currently supported generation not reporting INPUT_PROP_BUTTONPAD seems to be the right thing to do for all currently supported Cypress Trackpads. I still believe the kernel is the right place to fix this. Adam, can you build a kernel with the attached patch, remove your xorg.conf changes and then test that setup please ? Regards, Hans From 4b0668abbea0068cbdf7f9b0d149ccc245d79fab Mon Sep 17 00:00:00 2001 From: Hans de Goede hdego...@redhat.com Date: Fri, 21 Mar 2014 10:55:11 +0100 Subject: [PATCH] input: cypress_ps2: Don't report the cypress PS/2 trackpads as a button pad The cypress PS/2 trackpad models supported by the cypress_ps2 driver do right button emulation in firmware, filtering out a bunch of events userspace needs to do clickpad handling on its own. So reporting them as clickpads actually causes a bunch of issues: https://bugs.freedesktop.org/show_bug.cgi?id=70819 https://bugs.freedesktop.org/show_bug.cgi?id=76341 Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/input/mouse/cypress_ps2.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/input/mouse/cypress_ps2.c b/drivers/input/mouse/cypress_ps2.c index 87095e2..8af34ff 100644 --- a/drivers/input/mouse/cypress_ps2.c +++ b/drivers/input/mouse/cypress_ps2.c @@ -409,7 +409,6 @@ static int cypress_set_input_params(struct input_dev *input, __clear_bit(REL_X, input-relbit); __clear_bit(REL_Y, input-relbit); -__set_bit(INPUT_PROP_BUTTONPAD, input-propbit); __set_bit(EV_KEY, input-evbit); __set_bit(BTN_LEFT, input-keybit); __set_bit(BTN_RIGHT, input-keybit); -- 1.9.0 I recommend that you expand the commit message with a blurb on why it causes issues before you send it to the lkml, it'll make it easier to review. Specifically that BTN_RIGHT is emulated in firmware based on the finger position, and that no motion events are sent when the finger is in the button area. other than that this seems to be the best solution, thanks. Reviewed-by: Peter Hutterer peter.hutte...@who-t.net Thanks for the review, I've send this upstream with an amended commit msg. though I'll still ship the xorg.conf snippet in fedora until this filters down. Ok. Regards, Hans ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 4/5] xkb: Repurpose XkbCopyDeviceKeymap to apply a given keymap to a device
On 12/03/2014 06:43, Peter Hutterer wrote: On Fri, Mar 07, 2014 at 02:32:27PM -0800, Kristian Høgsberg wrote: From: Rui Matos This will also make it useful for cases when we have a new keymap to apply to a device but don't have a source device. Reviewed-by: Kristian Høgsberg krh-1OA22m9ORUweIZ0/mpf...@public.gmane.org merged, thanks. diff --git a/include/xkbsrv.h b/include/xkbsrv.h index 253612e..5d8e409 100644 --- a/include/xkbsrv.h +++ b/include/xkbsrv.h @@ -820,8 +820,8 @@ extern _X_EXPORT void XkbSendNewKeyboardNotify(DeviceIntPtr /* kbd */ , extern Bool XkbCopyKeymap(XkbDescPtr /* dst */ , XkbDescPtr /* src */ ); -extern _X_EXPORT Bool XkbCopyDeviceKeymap(DeviceIntPtr /* dst */ , - DeviceIntPtr /* src */ ); +extern _X_EXPORT Bool XkbDeviceApplyKeymap(DeviceIntPtr /* dst */ , + XkbDescPtr /* src */ ); This removes XkbCopyDeviceKeymap() from the server's exported symbols generated with sdksyms, but it is used by xf86-video-nested. Not sure how to fix this, but Google turns up a fedora patch [2] to restore it. [1] http://tinderbox.x.org/builds/2014-03-24-0010/logs/xf86-video-nested/#build [2] https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-20131104/1141772.html ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 3/3] systemd-logind: Monitor systemd-logind going away
When we're using server managed-fds through systemd-logind, systemd-logind *must* keep running while we are using it, as it does things like drmSetMaster and drmDropMaster for us on vt-switch. On a systemd-logind restart, we cannot simply re-connect since we will then get a different fd for the /dev/dri/card# node, and we've tied a lot of state to the old fd. I've discussed this with the systemd people, and in the future there me be a restart mechanism were systemd-logind passed fds from the old logind to the new logind. But for now there answer is simply: Don't restart systemd-logind, and there never really is a good reason to restart it. So to ensure unpleasentness if people do decide to restart systemd-logind anyways (or when it crashes), monitor logind going away and make this a fatal error. This avoids getting a hard-hung machine on the next vt-switch and will hopefully quickly educate users to not restart systemd-logind while they have an X session using it active. Signed-off-by: Hans de Goede hdego...@redhat.com --- hw/xfree86/os-support/linux/systemd-logind.c | 34 ++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c index 62858b0..d11519b 100644 --- a/hw/xfree86/os-support/linux/systemd-logind.c +++ b/hw/xfree86/os-support/linux/systemd-logind.c @@ -310,10 +310,31 @@ message_filter(DBusConnection * connection, DBusMessage * message, void *data) dbus_int32_t major, minor; char *pause_str; -if (strcmp(dbus_message_get_path(message), info-session) != 0) +dbus_error_init(error); + +if (dbus_message_is_signal(message, + org.freedesktop.DBus, NameOwnerChanged)) { +char *name, *old_owner, *new_owner; + +dbus_message_get_args(message, error, + DBUS_TYPE_STRING, name, + DBUS_TYPE_STRING, old_owner, + DBUS_TYPE_STRING, new_owner, DBUS_TYPE_INVALID); +if (dbus_error_is_set(error)) { +LogMessage(X_ERROR, systemd-logind: NameOwnerChanged: %s\n, + error.message); +dbus_error_free(error); +return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; +} + +if (name strcmp(name, org.freedesktop.login1) == 0) +FatalError(systemd-logind disappeared (stopped / restarted?)\n); + return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; +} -dbus_error_init(error); +if (strcmp(dbus_message_get_path(message), info-session) != 0) +return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; if (dbus_message_is_signal(message, org.freedesktop.login1.Session, PauseDevice)) { @@ -472,6 +493,15 @@ connect_hook(DBusConnection *connection, void *data) goto cleanup; } +dbus_bus_add_match(connection, + type='signal',sender='org.freedesktop.DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',path='/org/freedesktop/DBus', +error); +if (dbus_error_is_set(error)) { +LogMessage(X_ERROR, systemd-logind: could not add match: %s\n, + error.message); +goto cleanup; +} + /* * HdG: This is not useful with systemd = 208 since the signal only * contains invalidated property names there, rather than property, val -- 1.9.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] fb: fix fast-path detection
On Mon, 2014-03-24 at 15:29 +0800, Daniel Kurtz wrote: This bug causes us to take the slow path for large non-overlapping rows that are close in memory. As a data point, XGetImage(1366x768) on my ARM chromebook was taking ~140 ms, but with this fixed, it now takes about 60 ms. XGetImage() - exaGetImage() - fbGetImage - fbBlt() Nice catch. Reviewed-by: Adam Jackson a...@redhat.com - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 2/3] xf86LogInit: log to XDG_DATA_HOME when not running as root
When no logfile was specified (xf86LogFileFrom == X_DEFAULT) and we're not running as root log to $XDG_DATA_HOME/xorg/Xorg.#.log as Xorg won't be able to log to the default /var/log/... when it is not running as root. Signed-off-by: Hans de Goede hdego...@redhat.com --- hw/xfree86/common/xf86Helper.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c index 12a8771..b9d6c1b 100644 --- a/hw/xfree86/common/xf86Helper.c +++ b/hw/xfree86/common/xf86Helper.c @@ -1220,13 +1220,27 @@ xf86ErrorF(const char *format, ...) void xf86LogInit(void) { -char *lf = NULL; +char *env, *lf = NULL; +char buf[PATH_MAX]; #define LOGSUFFIX .log #define LOGOLDSUFFIX .old /* Get the log file name */ if (xf86LogFileFrom == X_DEFAULT) { +/* When not running as root, we won't be able to write to /var/log */ +if (geteuid() != 0) { +if ((env = getenv(XDG_DATA_HOME))) +snprintf(buf, sizeof(buf), %s/xorg, env); +else if ((env = getenv(HOME))) +snprintf(buf, sizeof(buf), %s/.local/share/xorg, env); + +if (env) { +(void)mkdir(buf, 0777); +strlcat(buf, /Xorg., sizeof(buf)); +xf86LogFile = buf; +} +} /* Append the display number and .log */ if (asprintf(lf, %s%%s LOGSUFFIX, xf86LogFile) == -1) FatalError(Cannot allocate space for the log file name\n); -- 1.9.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 1/3] Buildsys: Create SUID_WRAPPER_DIR before using it
Signed-off-by: Hans de Goede hdego...@redhat.com --- hw/xfree86/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index a315bbc..5cdecd9 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -105,6 +105,7 @@ if INSTALL_SETUID chmod u+s $(DESTDIR)$(bindir)/Xorg endif if SUID_WRAPPER + $(MKDIR_P) $(DESTDIR)$(SUID_WRAPPER_DIR) mv $(DESTDIR)$(bindir)/Xorg $(DESTDIR)$(SUID_WRAPPER_DIR)/Xorg.bin ${INSTALL} -m 755 Xorg.sh $(DESTDIR)$(bindir)/Xorg -chown root $(DESTDIR)$(SUID_WRAPPER_DIR)/Xorg.wrap chmod u+s $(DESTDIR)$(SUID_WRAPPER_DIR)/Xorg.wrap -- 1.9.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xf86-input-mouse] do not drop the result of protocol detection
In MousePickProtocol() with protocol PROT_AUTO we probe for the protocol to use but drop the result in most cases. This was causing DEVICE_INIT and DEVICE_ON to fail to be called with the VUID protocol. Git history suggests that this code was originally meant to cover both PS/2 auto-detection and OS- specific detection, but that only the first case was implemented at the time. Now that only the second is needed dropping the result to keep the protocol as PROT_AUTO is presumably no longer useful and seems to actively breaking things. Signed-off-by: Michael Thayer michael.tha...@oracle.com --- src/mouse.c |7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/mouse.c b/src/mouse.c index 2da2b4d..ff8d799 100644 --- a/src/mouse.c +++ b/src/mouse.c @@ -843,11 +843,8 @@ MousePickProtocol(InputInfoPtr pInfo, const char* device, { const char *osProt; if (osInfo-SetupAuto (osProt = osInfo-SetupAuto(pInfo,NULL))) { -MouseProtocolID id = ProtocolNameToID(osProt); -if (id == PROT_UNKNOWN || id == PROT_UNSUP) { -protocolID = id; -protocol = osProt; -} +protocolID = ProtocolNameToID(osProt); +protocol = osProt; } } -- ORACLE Deutschland B.V. Co. KG Michael Thayer Werkstrasse 24 VirtualBox engineering 71384 Weinstadt, Germany mailto:michael.tha...@oracle.com Hauptverwaltung: Riesstr. 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Geschäftsführer: Jürgen Kunz Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Astrid Kepper, Val Maher ___ xorg-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/2] A couple of build fixes
Jon TURNEY (2): Fix ephyr build with --disable-glamor Fix build when configured --enable-debug hw/kdrive/ephyr/hostx.c| 5 - hw/xfree86/parser/DRI.c| 1 + hw/xfree86/parser/Extensions.c | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) -- 1.8.3.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 2/2] Fix build when configured --enable-debug
Include os.h for ErrorF() to fix implicit-function-declaration warnings when configured with --enable-debug. hw/xfree86/parser/DRI.c: In function 'xf86parseDRISection': hw/xfree86/parser/DRI.c:87:5: error: implicit declaration of function 'ErrorF' [-Werror=implicit-function-declaration] hw/xfree86/parser/Extensions.c: In function 'xf86parseExtensionsSection': hw/xfree86/parser/Extensions.c:77:5: error: implicit declaration of function 'ErrorF' [-Werror=implicit-function-declaration] Signed-off-by: Jon TURNEY jon.tur...@dronecode.org.uk --- hw/xfree86/parser/DRI.c| 1 + hw/xfree86/parser/Extensions.c | 1 + 2 files changed, 2 insertions(+) diff --git a/hw/xfree86/parser/DRI.c b/hw/xfree86/parser/DRI.c index ad053f7..6be32d7 100644 --- a/hw/xfree86/parser/DRI.c +++ b/hw/xfree86/parser/DRI.c @@ -31,6 +31,7 @@ #include xorg-config.h #endif +#include os.h #include xf86Parser.h #include xf86tokens.h #include Configint.h diff --git a/hw/xfree86/parser/Extensions.c b/hw/xfree86/parser/Extensions.c index b5ba72e..a6fcb56 100644 --- a/hw/xfree86/parser/Extensions.c +++ b/hw/xfree86/parser/Extensions.c @@ -35,6 +35,7 @@ #include xorg-config.h #endif +#include os.h #include xf86Parser.h #include xf86tokens.h #include Configint.h -- 1.8.3.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 1/2] Fix ephyr build with --disable-glamor
See http://tinderbox.x.org/builds/2014-03-23-0010/logs/xserver/#build Signed-off-by: Jon TURNEY jon.tur...@dronecode.org.uk --- hw/kdrive/ephyr/hostx.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/kdrive/ephyr/hostx.c b/hw/kdrive/ephyr/hostx.c index 3260d95..3c8b49d 100644 --- a/hw/kdrive/ephyr/hostx.c +++ b/hw/kdrive/ephyr/hostx.c @@ -728,13 +728,16 @@ hostx_screen_init(KdScreenInfo *screen, scrpriv-win_width = width; scrpriv-win_height = height; +#ifdef GLAMOR if (ephyr_glamor) { *bytes_per_line = 0; *bits_per_pixel = 0; ephyr_glamor_set_window_size(scrpriv-glamor, scrpriv-win_width, scrpriv-win_height); return NULL; -} else if (host_depth_matches_server(scrpriv)) { +} else +#endif +if (host_depth_matches_server(scrpriv)) { *bytes_per_line = scrpriv-ximg-stride; *bits_per_pixel = scrpriv-ximg-bpp; -- 1.8.3.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] fb: fix fast-path detection
Daniel Kurtz djku...@chromium.org writes: + int srcX,/* bits */ + FbBits * dstLine,/* pixels */ + FbStride dstStride, /* pixels */ FbStride is in FbBits units, not pixels (yes, at 32bpp, it's the same) + int dstX,/* bits */ + int width, /* bits */ int height, int alu, FbBits pm, int bpp, Bool reverse, Bool upsidedown) -careful = !((srcLine dstLine srcLine + width * (bpp 3) dstLine) || -(dstLine srcLine dstLine + width * (bpp 3) srcLine)) +/* We must be careful if src and dst regions overlap */ +careful = ((srcLine dstLine srcLine + (width / bpp) dstLine) || +(dstLine srcLine dstLine + (width / bpp) srcLine)) I don't think that's correct either (although closer). srcLine + (width FB_SHIFT) is what you want as that converts width from bits to FbBits units. -- keith.pack...@intel.com pgpa8HCQZn58x.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 14/20] glamor: stub out lines
Markus Wick mar...@selfnet.de writes: I don't get it, why are horizontal lines so much faster than vertical ones? Presumably due to memory access patterns. When doing a horizontal line, you're hitting all of the bytes in several cache lines, rather than only a few bytes in each one. And, you're using fewer pages overall, which reduces TLB abuse. Intel X tiling (used for scanout buffers) is 128 x 8 pixels, so a single page holds 128 pixels of a horizontal line, but only 8 pixels in a vertical line. Memory is organized linearly within the tile, so horizontal lines across the tile will write 128 pixels with only 8 cache lines, while a vertical line will write 8 pixels and do read-modify-write access on 8 cache lines. Given that, I'd expect vertical lines to be about 32 times slower than horizontal ones. There's also the impact on the TLB -- vertical lines hit 16 times as many pages, so if you're TLB isn't large enough, you'll end up missing a lot, and that also hurts performance. This sounds like software rasterization for GL_LINES. Should this be treated as a mesa bug or shall we write a workaround using triangles? GL_LINES give us the correct semantics; I'd hate to try and match that with triangles. Even still, uxa is significantly faster than glamor on this one; uxa has a simple test for a request that contains only vertical and horizontal lines and fills those as rectangles. We could easily do the same thing in glamor, which should yield better performance for this case. In fact, I suspect we could do the test for all vertical/horizontal with the CPU and let a vertex shader compute rectangles from the existing line coordinates. Might be a fun exercise. 1: intel-glamor-line.perf 2: intel-uxa.perf 1 2 Operation - - 402.0 1070.0 ( 2.662) 500-pixel horizontal line segment 177000.0 986.0 (55.706) 500-pixel vertical line segment -- keith.pack...@intel.com pgpayh6fyVFOy.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] Handle -displayfd and an explicit display number sensibly
On Wed, Mar 19, 2014 at 7:26 AM, Jon TURNEY jon.tur...@dronecode.org.uk wrote: Handle -displayfd and an explicit display number sensibly, e.g. use the explicitly specified display number, and write it to the displayfd I don't think the two options were meant to be used together, but I can see how that would be useful. Non-signal based ready notification while letting the parent process pick the display is useful for Xwayland as well. Signed-off-by: Jon TURNEY jon.tur...@dronecode.org.uk --- dix/globals.c| 3 ++- include/opaque.h | 1 + os/connection.c | 11 +-- os/utils.c | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/dix/globals.c b/dix/globals.c index 9738e9c..5a21f07 100644 --- a/dix/globals.c +++ b/dix/globals.c @@ -127,7 +127,8 @@ int defaultColorVisualClass = -1; int monitorResolution = 0; const char *display; -int displayfd; +int displayfd = 0; The parent process creates the fd in our process environment and could pick 0 as the display fd. Let's initialize to -1 which is never a valid fd. +Bool explicit_display = FALSE; char *ConnectionInfo; CARD32 TimeOutValue = DEFAULT_TIMEOUT * MILLI_PER_SECOND; diff --git a/include/opaque.h b/include/opaque.h index 73d40c3..0d917b0 100644 --- a/include/opaque.h +++ b/include/opaque.h @@ -51,6 +51,7 @@ extern _X_EXPORT int defaultScreenSaverBlanking; extern _X_EXPORT int defaultScreenSaverAllowExposures; extern _X_EXPORT const char *display; extern _X_EXPORT int displayfd; +extern _X_EXPORT Bool explicit_display; extern _X_EXPORT int defaultBackingStore; extern _X_EXPORT Bool disableBackingStore; diff --git a/os/connection.c b/os/connection.c index ddf4f0a..23a4f6a 100644 --- a/os/connection.c +++ b/os/connection.c @@ -351,8 +351,8 @@ void NotifyParentProcess(void) { #if !defined(WIN32) -if (dynamic_display[0]) { -write(displayfd, dynamic_display, strlen(dynamic_display)); +if (displayfd) { +write(displayfd, display, strlen(display)); write(displayfd, \n, 1); close(displayfd); } @@ -404,15 +404,14 @@ CreateWellKnownSockets(void) FD_ZERO(WellKnownConnections); /* display is initialized to 0 by main(). It is then set to the display - * number if specified on the command line, or to NULL when the -displayfd - * option is used. */ -if (display) { + * number if specified on the command line. */ +if ((displayfd == 0) || explicit_display) { if (TryCreateSocket(atoi(display), partial) ListenTransCount = 1) if (!PartialNetwork partial) FatalError (Failed to establish all listening sockets); } -else { /* -displayfd */ +else { /* -displayfd and no explicit display number */ Bool found = 0; for (i = 0; i 65535 - X_TCP_PORT; i++) { if (TryCreateSocket(i, partial) !partial) { diff --git a/os/utils.c b/os/utils.c index 497779b..8ea6e7a 100644 --- a/os/utils.c +++ b/os/utils.c @@ -666,6 +666,7 @@ ProcessCommandLine(int argc, char *argv[]) else if (argv[i][0] == ':') { /* initialize display */ display = argv[i]; +explicit_display = TRUE; display++; if (!VerifyDisplayName(display)) { ErrorF(Bad display name: %s\n, display); @@ -736,7 +737,6 @@ ProcessCommandLine(int argc, char *argv[]) else if (strcmp(argv[i], -displayfd) == 0) { if (++i argc) { displayfd = atoi(argv[i]); -display = NULL; #ifdef LOCK_SERVER nolock = TRUE; #endif -- 1.8.3.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] xkb: Fix ISOLock
It should be. Some of the tests rely on linux/input.h to get KEY_* definitions, but you could just copy that and it'll work fine. The only test which will break would be interactive.c. It seems you are right. I am not sure that how to run the tests properly, but at least, I have been able to run test/state. Right. We've got a fairly comprehensive test suite covering the full keyboard mechanics, that covers a lot of dark corners you don't usually see in manual testing. Very nice. That's why I'd like to see it there: so we can write tests ensuring it works, but also to ensure it doesn't break any other expectations. That is a long way, I am afraid. To demonstrate the correctness of the X-server implementation by means of a libxkbcommon implementation, the implementations and the environment they live in must be equivalent. Even though the structure in src/state.c is very similar to that in xkbActions.c, there are differences: 1. As far as I can see, libxkbcommon does not handle group latches, set/lock controls, and button actions. That is half of the actions affected by ISOLock. 2. Autorepeat handling is completely different. libxkbcommon seems to assume that it does receive balanced press/release events (this is what I gather from reference count handling). In the X-server, autorepeating key presses have no matching releases. 3. Modifier latch handling in libxkbcommon is quite different to what is now in the X-server, and looks suspicious: - Autorepeat handling is missing. If I am right about how autorepeat in libxkbcommon works, there is no way to tell autorepeat from regular presses/releases. Is autorepeat handling even possible? - Base modifiers can get stuck. The same issue used to be in the X-server, see http://patchwork.freedesktop.org/patch/13192/. Regards, Andreas ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 14/20] glamor: stub out lines
Am 2014-03-24 20:47, schrieb Keith Packard: Even still, uxa is significantly faster than glamor on this one; uxa has a simple test for a request that contains only vertical and horizontal lines and fills those as rectangles. We could easily do the same thing in glamor, which should yield better performance for this case. In fact, I suspect we could do the test for all vertical/horizontal with the CPU and let a vertex shader compute rectangles from the existing line coordinates. Might be a fun exercise. 1: intel-glamor-line.perf 2: intel-uxa.perf 1 2 Operation - - 402.0 1070.0 ( 2.662) 500-pixel horizontal line segment 177000.0 986.0 (55.706) 500-pixel vertical line segment But shouldn't UXA be affected as much as glamor by cache issues? My test results on ivb glamor vs SNA looks funny as SNA seems to be as slow as glamor: glamor: 300 reps @ 0.0004 msec (235.0/sec): 500-pixel horizontal line segment 60 reps @ 0.0018 msec (565000.0/sec): 500-pixel vertical line segment SNA: 800 reps @ 0.0001 msec (729.0/sec): 500-pixel horizontal line segment 40 reps @ 0.0024 msec (413000.0/sec): 500-pixel vertical line segment As UXA is so much faster, could it be because of line rasterization limits of the gpu itself? ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 14/20] glamor: stub out lines
Markus Wick mar...@selfnet.de writes: But shouldn't UXA be affected as much as glamor by cache issues? One would think so, but the 2D engine has a very different memory path than the 3D engine, and it's barely possible that the fact that there are a sequence of adjacent lines for this test is letting the 2D engine somehow merge stuff together. All very mysterious. One thing I might try is to separate the lines by a pixel to see if that causes a huge drop in UXA performance. -- keith.pack...@intel.com pgp4R01a274G5.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
[PULL] more xwayland prep work, restore XkbCopyDeviceKeymap
The following changes since commit 0e531fbb97868b9a869044fc5a4f6cb58de6751e: xkb: add XkbLoadKeymapFromString (2014-03-19 08:37:15 +1000) are available in the git repository at: git://people.freedesktop.org/~whot/xserver for-keith for you to fetch changes up to 78167a98a8631ee3fad145ddc051ceb8487b9683: xkb: Restore XkbCopyDeviceKeymap (2014-03-25 08:50:35 +1000) Adam Jackson (1): xkb: Restore XkbCopyDeviceKeymap Kristian Høgsberg (4): test: Don't add TEST_LDADD to list test os: Always compile ListenOnOpenFD() and export it os: Add a mechanism to prevent creating any listen sockets os: Add AddClientOnOpenFD() to create a new client for an file descriptor include/opaque.h | 1 + include/os.h | 6 +++--- include/xkbsrv.h | 3 +++ os/connection.c | 41 +++-- os/utils.c | 4 ++-- test/Makefile.am | 1 - xkb/xkbUtils.c | 6 ++ 7 files changed, 50 insertions(+), 12 deletions(-) pgpTSN4kAsjNo.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] xkb: Fix ISOLock
Hi, On 24 March 2014 20:53, wettstein...@solnet.ch wrote: That is a long way, I am afraid. To demonstrate the correctness of the X-server implementation by means of a libxkbcommon implementation, the implementations and the environment they live in must be equivalent. Even though the structure in src/state.c is very similar to that in xkbActions.c, there are differences: Right you are. Sadly they are separate codebases, as one of the things that made xkbcommon practical was specifying that keymaps must be immutable. 1. As far as I can see, libxkbcommon does not handle group latches, set/lock controls, and button actions. That is half of the actions affected by ISOLock. True. 2. Autorepeat handling is completely different. libxkbcommon seems to assume that it does receive balanced press/release events (this is what I gather from reference count handling). In the X-server, autorepeating key presses have no matching releases. Autorepeat is separate in that xkbcommon only specifies which keys should autorepeat; it doesn't provide autorepeat intrinsically. 3. Modifier latch handling in libxkbcommon is quite different to what is now in the X-server, and looks suspicious: - Autorepeat handling is missing. If I am right about how autorepeat in libxkbcommon works, there is no way to tell autorepeat from regular presses/releases. Is autorepeat handling even possible? - Base modifiers can get stuck. The same issue used to be in the X-server, see http://patchwork.freedesktop.org/patch/13192/. xkbcommon expects to not handle repeat events at all. I'll have to take a look at the server patch (xkbcommon was developed from the xserver/xkb/ base before your changes) and see if it still makes sense. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 4/5] xkb: Repurpose XkbCopyDeviceKeymap to apply a given keymap to a device
On Mon, Mar 24, 2014 at 02:51:40PM +, Jon TURNEY wrote: On 12/03/2014 06:43, Peter Hutterer wrote: On Fri, Mar 07, 2014 at 02:32:27PM -0800, Kristian Høgsberg wrote: From: Rui Matos This will also make it useful for cases when we have a new keymap to apply to a device but don't have a source device. Reviewed-by: Kristian Høgsberg krh-1OA22m9ORUweIZ0/mpf...@public.gmane.org merged, thanks. diff --git a/include/xkbsrv.h b/include/xkbsrv.h index 253612e..5d8e409 100644 --- a/include/xkbsrv.h +++ b/include/xkbsrv.h @@ -820,8 +820,8 @@ extern _X_EXPORT void XkbSendNewKeyboardNotify(DeviceIntPtr /* kbd */ , extern Bool XkbCopyKeymap(XkbDescPtr /* dst */ , XkbDescPtr /* src */ ); -extern _X_EXPORT Bool XkbCopyDeviceKeymap(DeviceIntPtr /* dst */ , - DeviceIntPtr /* src */ ); +extern _X_EXPORT Bool XkbDeviceApplyKeymap(DeviceIntPtr /* dst */ , + XkbDescPtr /* src */ ); This removes XkbCopyDeviceKeymap() from the server's exported symbols generated with sdksyms, but it is used by xf86-video-nested. Not sure how to fix this, but Google turns up a fedora patch [2] to restore it. [1] http://tinderbox.x.org/builds/2014-03-24-0010/logs/xf86-video-nested/#build [2] https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-20131104/1141772.html thanks. I've taken the patch and added it to my pull request here: http://patchwork.freedesktop.org/patch/22785/ we can drop XkbCopyDeviceKeymap next release once the callers are fixed up. 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