Re: [PATCH RFC] configurable maximum number of clients
On 05/28/15 08:56 AM, Olivier Fourdan wrote: Make the maximum number of client user configurable, either from the command line or from xorg.conf This patch works by keeping the MAXCLIENTS define (of 512) as the maximum allowed number of clients, but allowing the actual limit to be set by the user (default to 256). There is a limit size of 29 bits to be used to store both the client ID and the X resources ID, so by reducing the number of clients allowed to connect to the X server, the user can increase the number of X resources per client. Parts of this patch are based on a similar patch from Adam Jackson a...@redhat.com Signed-off-by: Adam Jackson a...@redhat.com Signed-off-by: Olivier Fourdan ofour...@redhat.com Looking back at the notes from when the Solaris Xsun made similar changes long ago (on an X11R6.4 code base), they say we had to change: - MAXCLIENTS - MAXSOCKS - OPEN_MAX - TRANS_OPEN_MAX - XFD_SETSIZE (If it's useful that Xsun code base is available, but without source history, at https://java.net/projects/solaris-x11/downloads/directory/openXsun .) I see in the current Xorg code it says * MAXSOCKS is used only for initialising MaxClients when no other method * like sysconf(_SC_OPEN_MAX) is not supported. and OPEN_MAX seems to just be used to set MAXSOCKS, and their use in InitConnectionLimits() seems to be used as an upper limit on MaxClients when other methods fail - should their default values be bumped from 256 to 512 as well? Or should they just be eliminated except for Cygwin now? XFD_SETSIZE also still seems to be set to 256 in proto/x11proto/Xpoll.h.in which would limit the number of file descriptors you can monitor in the select() call in the main WaitForSomething() loop. Similarly, I see Cygwin has set CFLAGS=$CFLAGS -DFD_SETSIZE=256 in configure.ac which may need to be bumped as well. Or have you managed to get 512 clients to be connected and working without changing those? -- -Alan Coopersmith- alan.coopersm...@oracle.com Oracle Solaris Engineering - http://blogs.oracle.com/alanc ___ xorg-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] glamor: don't do render ops with matching source/dest (v3)
Dave Airlie airl...@gmail.com writes: From: Dave Airlie airl...@redhat.com XRender defines this, GL really doesn't like it. kwin 4.x and qt 4.x seem to make this happen for the gradient in the titlebar, and on radeonsi/r600 hw this draws all kinds of wrong. v2: bump this up a level, and check it earlier. (I assume the was for this case.) v3: add same code to largepixmap paths (Keith) Reviewed-by: Eric Anholt e...@anholt.net signature.asc 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] glamor: don't do render ops with matching source/dest
I wonder if we should tell the Qt people about this so they can get back on the fast path? In any case, makes sense. Reviewed-by: Jasper St. Pierre jstpie...@mecheye.net On Wed, May 27, 2015 at 10:31 PM, Dave Airlie airl...@gmail.com wrote: From: Dave Airlie airl...@redhat.com XRender defines this, GL really doesn't like it. kwin 4.x and qt 4.x seem to make this happen for the gradient in the titlebar, and on radeonsi/r600 hw this draws all kinds of wrong. Signed-off-by: Dave Airlie airl...@redhat.com --- glamor/glamor_render.c | 8 1 file changed, 8 insertions(+) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index efca367..b3046b4 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -1149,6 +1149,14 @@ glamor_composite_with_shader(CARD8 op, glamor_composite_shader *shader = NULL, *shader_ca = NULL; struct blendinfo op_info, op_info_ca; +if (source_pixmap == dest_pixmap) { +glamor_fallback(source and dest pixmaps are the same\n); +return ret; +} +if (mask_pixmap == dest_pixmap) { +glamor_fallback(mask and dest pixmaps are the same\n); +return ret; +} if (!glamor_composite_choose_shader(op, source, mask, dest, source_pixmap, mask_pixmap, dest_pixmap, source_pixmap_priv, mask_pixmap_priv, -- 1.8.3.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel -- Jasper ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3] present: Query the Window's CRTC every time
On 28.05.2015 18:30, Chris Wilson wrote: On Thu, May 28, 2015 at 06:27:34PM +0900, Michel Dänzer wrote: On 28.05.2015 18:07, Chris Wilson wrote: On Thu, May 28, 2015 at 05:56:15PM +0900, Michel Dänzer wrote: On 28.05.2015 17:38, Chris Wilson wrote: On Thu, May 28, 2015 at 04:59:14PM +0900, Michel Dänzer wrote: The patch below is an alternative fix for the problem I'm seeing, while preserving the window CRTC for MSC waits when possible. Your description sounds like it might work for you as well, Chris. Can you try it? diff --git a/present/present.c b/present/present.c index a634601..dc58f25 100644 --- a/present/present.c +++ b/present/present.c @@ -713,6 +713,7 @@ present_pixmap(WindowPtr window, uint64_tust; uint64_ttarget_msc; uint64_tcrtc_msc; +RRCrtcPtr new_crtc; int ret; present_vblank_ptr vblank, tmp; ScreenPtr screen = window-drawable.pScreen; @@ -734,7 +735,21 @@ present_pixmap(WindowPtr window, target_crtc = present_get_crtc(window); } -present_get_ust_msc(screen, target_crtc, ust, crtc_msc); +if (present_get_ust_msc(screen, target_crtc, ust, crtc_msc) != Success) { +/* Maybe we need to try a different CRTC? + */ +new_crtc = present_get_crtc(window); + +if (new_crtc != target_crtc +present_get_ust_msc(screen, new_crtc, ust, crtc_msc) == Success) +target_crtc = new_crtc; +else { +/* Fall back to fake MSC handling + */ +target_crtc = NULL; +present_fake_get_ust_msc(screen, ust, crtc_msc); +} +} It survived for one more CRTC change, but still feeds passed msc into the wait_vblank. By how much is it off? Does it cause a hang? Just a few -1000 msc, which is a pretty long wait (it would be if we honoured it at least). The kernel should immediately return / send the event in that case. Given that it is supposedly an impossible condition in present_pixmap, it is simple evidence of a bug. And that bug doesn't occur with your patch? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v3 2/3] linux: Add a may_fail paramter to linux_parse_vt_settings
typo in the subject: parameter On Tue, May 19, 2015 at 10:10:20AM +0200, Hans de Goede wrote: linux_parse_vt_settings() was split out of xf86OpenConsole so that it can be called earlier during systemd-logind init, but it is possible to run the xserver in such a way that xf86OpenConsole() is never used. The FatalError calls in linux_parse_vt_settings() may stop the Xorg xserver from working when e.g. no /dev/tty0 is present in such a setup. This commit adds a may_fail parameter to linux_parse_vt_settings() which can be used to make linux_parse_vt_settings() fail silenty with an error return in this case, rather then calling FatalError(). Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net otherwise Cheers, Peter --- Changes in v3: -New patch in v3 of the patch-set --- hw/xfree86/os-support/linux/linux.h| 2 +- hw/xfree86/os-support/linux/lnx_init.c | 29 + 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/hw/xfree86/os-support/linux/linux.h b/hw/xfree86/os-support/linux/linux.h index 8cb8e3d..83506fd 100644 --- a/hw/xfree86/os-support/linux/linux.h +++ b/hw/xfree86/os-support/linux/linux.h @@ -26,7 +26,7 @@ #ifndef XF86_LINUX_H #define XF86_LINUX_H -void linux_parse_vt_settings(void); +int linux_parse_vt_settings(int may_fail); int linux_get_keeptty(void); #endif diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c index 22c61bf..12ddf91 100644 --- a/hw/xfree86/os-support/linux/lnx_init.c +++ b/hw/xfree86/os-support/linux/lnx_init.c @@ -80,8 +80,8 @@ switch_to(int vt, const char *from) #pragma GCC diagnostic push #pragma GCC diagnostic ignored -Wformat-nonliteral -void -linux_parse_vt_settings(void) +int +linux_parse_vt_settings(int may_fail) { int i, fd = -1, ret, current_vt = -1; struct vt_stat vts; @@ -93,7 +93,7 @@ linux_parse_vt_settings(void) static int vt_settings_parsed = 0; if (vt_settings_parsed) -return; +return 1; /* * setup the virtual terminal manager @@ -110,24 +110,36 @@ linux_parse_vt_settings(void) i++; } -if (fd 0) +if (fd 0) { +if (may_fail) +return 0; FatalError(parse_vt_settings: Cannot open /dev/tty0 (%s)\n, strerror(errno)); +} if (xf86Info.ShareVTs) { SYSCALL(ret = ioctl(fd, VT_GETSTATE, vts)); -if (ret 0) +if (ret 0) { +if (may_fail) +return 0; FatalError(parse_vt_settings: Cannot find the current VT (%s)\n, strerror(errno)); +} xf86Info.vtno = vts.v_active; } else { SYSCALL(ret = ioctl(fd, VT_OPENQRY, xf86Info.vtno)); -if (ret 0) +if (ret 0) { +if (may_fail) +return 0; FatalError(parse_vt_settings: Cannot find a free VT: %s\n, strerror(errno)); -if (xf86Info.vtno == -1) +} +if (xf86Info.vtno == -1) { +if (may_fail) +return 0; FatalError(parse_vt_settings: Cannot find a free VT\n); +} } close(fd); } @@ -151,6 +163,7 @@ linux_parse_vt_settings(void) } vt_settings_parsed = 1; +return 1; } int @@ -168,7 +181,7 @@ xf86OpenConsole(void) const char *vcs[] = { /dev/vc/%d, /dev/tty%d, NULL }; if (serverGeneration == 1) { -linux_parse_vt_settings(); +linux_parse_vt_settings(FALSE); if (!KeepTty) { pid_t ppid = getppid(); -- 2.4.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] glamor: don't do render ops with matching source/dest (v3)
From: Dave Airlie airl...@redhat.com XRender defines this, GL really doesn't like it. kwin 4.x and qt 4.x seem to make this happen for the gradient in the titlebar, and on radeonsi/r600 hw this draws all kinds of wrong. v2: bump this up a level, and check it earlier. (I assume the was for this case.) v3: add same code to largepixmap paths (Keith) Signed-off-by: Dave Airlie airl...@redhat.com --- glamor/glamor_largepixmap.c | 9 + glamor/glamor_render.c | 9 - 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/glamor/glamor_largepixmap.c b/glamor/glamor_largepixmap.c index 391f376..da3fb61 100644 --- a/glamor/glamor_largepixmap.c +++ b/glamor/glamor_largepixmap.c @@ -1055,6 +1055,15 @@ glamor_composite_largepixmap_region(CARD8 op, int source_repeat_type = 0, mask_repeat_type = 0; int ok = TRUE; +if (source_pixmap == dest_pixmap) { +glamor_fallback(source and dest pixmaps are the same\n); +return FALSE; +} +if (mask_pixmap == dest_pixmap) { +glamor_fallback(mask and dest pixmaps are the same\n); +return FALSE; +} + if (source-repeat) source_repeat_type = source-repeatType; else diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index efca367..05eee91 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -1481,7 +1481,14 @@ glamor_composite_clipped_region(CARD8 op, } } -/*X, self copy? */ +if (temp_src_pixmap == dest_pixmap) { +glamor_fallback(source and dest pixmaps are the same\n); +goto out; +} +if (temp_mask_pixmap == dest_pixmap) { +glamor_fallback(mask and dest pixmaps are the same\n); +goto out; +} x_dest += dest-pDrawable-x; y_dest += dest-pDrawable-y; -- 1.8.3.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3] present: Query the Window's CRTC every time
On 27.05.2015 15:51, Chris Wilson wrote: On Tue, May 26, 2015 at 02:30:32PM -0700, Keith Packard wrote: Michel Dänzer mic...@daenzer.net writes: The old code also called present_get_crtc() unless pixmap == NULL, so the problem couldn't affect flips but only MSC waits. The original code was trying to let the client control which CRTC to track for each window. For PresentPixmap requests, there's an explicit CRTC argument, which allows the client to select the right one. For PresentNotifyMSC, there's no such argument. So, the code was trying to respect the client's wishes by using whichever CRTC was last associated with the window -- either implicitly through the last call to present_get_crtc or explicitly from the last crtc passed to PresentPixmap for the same window. Probably the right thing to do is to add an explicit CRTC parameter to PresentNotifyMSC, and then have both requests call present_get_crtc when an explicit CRTC is not provided. That doesn't solve the problem when an explicitly requested CRTC is disabled though, so this fix doesn't address the root cause, only one of the symptoms. The problem I was hitting was that this code was running for an MSC wait when the CRTC referenced by window_priv-crtc was already disabled for DPMS off. This caused hangs at the GNOME lock screen. This patch seems to fix that problem. Why isn't your queue_vblank function bailing when asked to queue a request for a CRTC which is disabled? If it simply fails, present_execute will get called immediately and the client would continue happily along. Oh, but it does fail gracefully. The problem is not that but that it sends a garbage msc to a valid CRTC. The patch below is an alternative fix for the problem I'm seeing, while preserving the window CRTC for MSC waits when possible. Your description sounds like it might work for you as well, Chris. Can you try it? diff --git a/present/present.c b/present/present.c index a634601..dc58f25 100644 --- a/present/present.c +++ b/present/present.c @@ -713,6 +713,7 @@ present_pixmap(WindowPtr window, uint64_tust; uint64_ttarget_msc; uint64_tcrtc_msc; +RRCrtcPtr new_crtc; int ret; present_vblank_ptr vblank, tmp; ScreenPtr screen = window-drawable.pScreen; @@ -734,7 +735,21 @@ present_pixmap(WindowPtr window, target_crtc = present_get_crtc(window); } -present_get_ust_msc(screen, target_crtc, ust, crtc_msc); +if (present_get_ust_msc(screen, target_crtc, ust, crtc_msc) != Success) { +/* Maybe we need to try a different CRTC? + */ +new_crtc = present_get_crtc(window); + +if (new_crtc != target_crtc +present_get_ust_msc(screen, new_crtc, ust, crtc_msc) == Success) +target_crtc = new_crtc; +else { +/* Fall back to fake MSC handling + */ +target_crtc = NULL; +present_fake_get_ust_msc(screen, ust, crtc_msc); +} +} target_msc = present_window_to_crtc_msc(window, target_crtc, window_msc, crtc_msc); -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PULL 1.17.x] unaccelerated valuator masks
This is needed for the SDL + libinput combination, but mainly I want this in the offical branch so we have a standardised ABI bump that distros can rely on (input abi bumped to 21.1) The following changes since commit 27846c49e883a47d80a33fd2979ee4357f776ffd: backtrace.c: Fix word cast to a pointer (2015-05-27 09:56:34 -0400) are available in the git repository at: git://people.freedesktop.org/~whot/xserver server-1.17-branch for you to fetch changes up to a4da79c1fed0bc3dd4e9b63054e8c920ec156f93: dix: hook up the unaccelerated valuator masks (2015-05-28 15:56:30 +1000) Peter Hutterer (2): dix: Add unaccelerated valuators to the ValuatorMask dix: hook up the unaccelerated valuator masks dix/getevents.c| 31 +++- dix/inpututils.c | 82 +++--- hw/xfree86/common/xf86Module.h | 2 +- hw/xfree86/common/xf86Xinput.c | 4 +++ include/input.h| 15 include/inpututils.h | 2 ++ 6 files changed, 121 insertions(+), 15 deletions(-) pgp3fjvJNrIuk.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 2/3] present: Query the Window's CRTC every time
On Thu, May 28, 2015 at 04:59:14PM +0900, Michel Dänzer wrote: On 27.05.2015 15:51, Chris Wilson wrote: On Tue, May 26, 2015 at 02:30:32PM -0700, Keith Packard wrote: Michel Dänzer mic...@daenzer.net writes: The old code also called present_get_crtc() unless pixmap == NULL, so the problem couldn't affect flips but only MSC waits. The original code was trying to let the client control which CRTC to track for each window. For PresentPixmap requests, there's an explicit CRTC argument, which allows the client to select the right one. For PresentNotifyMSC, there's no such argument. So, the code was trying to respect the client's wishes by using whichever CRTC was last associated with the window -- either implicitly through the last call to present_get_crtc or explicitly from the last crtc passed to PresentPixmap for the same window. Probably the right thing to do is to add an explicit CRTC parameter to PresentNotifyMSC, and then have both requests call present_get_crtc when an explicit CRTC is not provided. That doesn't solve the problem when an explicitly requested CRTC is disabled though, so this fix doesn't address the root cause, only one of the symptoms. The problem I was hitting was that this code was running for an MSC wait when the CRTC referenced by window_priv-crtc was already disabled for DPMS off. This caused hangs at the GNOME lock screen. This patch seems to fix that problem. Why isn't your queue_vblank function bailing when asked to queue a request for a CRTC which is disabled? If it simply fails, present_execute will get called immediately and the client would continue happily along. Oh, but it does fail gracefully. The problem is not that but that it sends a garbage msc to a valid CRTC. The patch below is an alternative fix for the problem I'm seeing, while preserving the window CRTC for MSC waits when possible. Your description sounds like it might work for you as well, Chris. Can you try it? diff --git a/present/present.c b/present/present.c index a634601..dc58f25 100644 --- a/present/present.c +++ b/present/present.c @@ -713,6 +713,7 @@ present_pixmap(WindowPtr window, uint64_tust; uint64_ttarget_msc; uint64_tcrtc_msc; +RRCrtcPtr new_crtc; int ret; present_vblank_ptr vblank, tmp; ScreenPtr screen = window-drawable.pScreen; @@ -734,7 +735,21 @@ present_pixmap(WindowPtr window, target_crtc = present_get_crtc(window); } -present_get_ust_msc(screen, target_crtc, ust, crtc_msc); +if (present_get_ust_msc(screen, target_crtc, ust, crtc_msc) != Success) { +/* Maybe we need to try a different CRTC? + */ +new_crtc = present_get_crtc(window); + +if (new_crtc != target_crtc +present_get_ust_msc(screen, new_crtc, ust, crtc_msc) == Success) +target_crtc = new_crtc; +else { +/* Fall back to fake MSC handling + */ +target_crtc = NULL; +present_fake_get_ust_msc(screen, ust, crtc_msc); +} +} It survived for one more CRTC change, but still feeds passed msc into the wait_vblank. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3] present: Query the Window's CRTC every time
On Thu, May 28, 2015 at 05:56:15PM +0900, Michel Dänzer wrote: On 28.05.2015 17:38, Chris Wilson wrote: On Thu, May 28, 2015 at 04:59:14PM +0900, Michel Dänzer wrote: The patch below is an alternative fix for the problem I'm seeing, while preserving the window CRTC for MSC waits when possible. Your description sounds like it might work for you as well, Chris. Can you try it? diff --git a/present/present.c b/present/present.c index a634601..dc58f25 100644 --- a/present/present.c +++ b/present/present.c @@ -713,6 +713,7 @@ present_pixmap(WindowPtr window, uint64_tust; uint64_ttarget_msc; uint64_tcrtc_msc; +RRCrtcPtr new_crtc; int ret; present_vblank_ptr vblank, tmp; ScreenPtr screen = window-drawable.pScreen; @@ -734,7 +735,21 @@ present_pixmap(WindowPtr window, target_crtc = present_get_crtc(window); } -present_get_ust_msc(screen, target_crtc, ust, crtc_msc); +if (present_get_ust_msc(screen, target_crtc, ust, crtc_msc) != Success) { +/* Maybe we need to try a different CRTC? + */ +new_crtc = present_get_crtc(window); + +if (new_crtc != target_crtc +present_get_ust_msc(screen, new_crtc, ust, crtc_msc) == Success) +target_crtc = new_crtc; +else { +/* Fall back to fake MSC handling + */ +target_crtc = NULL; +present_fake_get_ust_msc(screen, ust, crtc_msc); +} +} It survived for one more CRTC change, but still feeds passed msc into the wait_vblank. By how much is it off? Does it cause a hang? Just a few -1000 msc, which is a pretty long wait (it would be if we honoured it at least). What's your test-case? Mine is the GNOME lock screen, i.e. basically DPMS off vs vblank waits and flips. xf86-video-intel/test/present-test -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3] present: Query the Window's CRTC every time
On 28.05.2015 17:38, Chris Wilson wrote: On Thu, May 28, 2015 at 04:59:14PM +0900, Michel Dänzer wrote: On 27.05.2015 15:51, Chris Wilson wrote: On Tue, May 26, 2015 at 02:30:32PM -0700, Keith Packard wrote: Michel Dänzer mic...@daenzer.net writes: The old code also called present_get_crtc() unless pixmap == NULL, so the problem couldn't affect flips but only MSC waits. The original code was trying to let the client control which CRTC to track for each window. For PresentPixmap requests, there's an explicit CRTC argument, which allows the client to select the right one. For PresentNotifyMSC, there's no such argument. So, the code was trying to respect the client's wishes by using whichever CRTC was last associated with the window -- either implicitly through the last call to present_get_crtc or explicitly from the last crtc passed to PresentPixmap for the same window. Probably the right thing to do is to add an explicit CRTC parameter to PresentNotifyMSC, and then have both requests call present_get_crtc when an explicit CRTC is not provided. That doesn't solve the problem when an explicitly requested CRTC is disabled though, so this fix doesn't address the root cause, only one of the symptoms. The problem I was hitting was that this code was running for an MSC wait when the CRTC referenced by window_priv-crtc was already disabled for DPMS off. This caused hangs at the GNOME lock screen. This patch seems to fix that problem. Why isn't your queue_vblank function bailing when asked to queue a request for a CRTC which is disabled? If it simply fails, present_execute will get called immediately and the client would continue happily along. Oh, but it does fail gracefully. The problem is not that but that it sends a garbage msc to a valid CRTC. The patch below is an alternative fix for the problem I'm seeing, while preserving the window CRTC for MSC waits when possible. Your description sounds like it might work for you as well, Chris. Can you try it? diff --git a/present/present.c b/present/present.c index a634601..dc58f25 100644 --- a/present/present.c +++ b/present/present.c @@ -713,6 +713,7 @@ present_pixmap(WindowPtr window, uint64_tust; uint64_ttarget_msc; uint64_tcrtc_msc; +RRCrtcPtr new_crtc; int ret; present_vblank_ptr vblank, tmp; ScreenPtr screen = window-drawable.pScreen; @@ -734,7 +735,21 @@ present_pixmap(WindowPtr window, target_crtc = present_get_crtc(window); } -present_get_ust_msc(screen, target_crtc, ust, crtc_msc); +if (present_get_ust_msc(screen, target_crtc, ust, crtc_msc) != Success) { +/* Maybe we need to try a different CRTC? + */ +new_crtc = present_get_crtc(window); + +if (new_crtc != target_crtc +present_get_ust_msc(screen, new_crtc, ust, crtc_msc) == Success) +target_crtc = new_crtc; +else { +/* Fall back to fake MSC handling + */ +target_crtc = NULL; +present_fake_get_ust_msc(screen, ust, crtc_msc); +} +} It survived for one more CRTC change, but still feeds passed msc into the wait_vblank. By how much is it off? Does it cause a hang? What's your test-case? Mine is the GNOME lock screen, i.e. basically DPMS off vs vblank waits and flips. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] glamor: don't do render ops with matching source/dest (v2)
On 28 May 2015 at 16:09, Keith Packard kei...@keithp.com wrote: Dave Airlie airl...@gmail.com writes: From: Dave Airlie airl...@redhat.com XRender defines this, GL really doesn't like it. kwin 4.x and qt 4.x seem to make this happen for the gradient in the titlebar, and on radeonsi/r600 hw this draws all kinds of wrong. v2: bump this up a level, and check it earlier. (I assume the was for this case.) You're missing the matching case in the large_pixmap code The large pixmap code eventually calls this code, once you go crazy. 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] glamor: don't do render ops with matching source/dest (v2)
On 28 May 2015 at 16:17, Dave Airlie airl...@gmail.com wrote: On 28 May 2015 at 16:09, Keith Packard kei...@keithp.com wrote: Dave Airlie airl...@gmail.com writes: From: Dave Airlie airl...@redhat.com XRender defines this, GL really doesn't like it. kwin 4.x and qt 4.x seem to make this happen for the gradient in the titlebar, and on radeonsi/r600 hw this draws all kinds of wrong. v2: bump this up a level, and check it earlier. (I assume the was for this case.) You're missing the matching case in the large_pixmap code The large pixmap code eventually calls this code, once you go crazy. Though it asserts on failure, so yes I probably do need that :) 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 1/5] modesetting: Improve the ms_flush_drm_events() API.
On 22.04.2015 09:58, Kenneth Graunke wrote: Previously, ms_flush_drm_events() returned a boolean value, and it was very easy to interpret the meaning incorrectly. Now, we return an integer value. The possible outcomes of this call are: - poll() raised an error (formerly TRUE, now -1 - poll's return value) - poll() said there are no events (formerly TRUE, now 0). - drmHandleEvent() raised an error (formerly FALSE, now the negative value returned by drmHandleEvent). - An event was successfully handled (formerly TRUE, now 1). The nice part is that this allows you to distinguish errors ( 0), nothing to do (= 0), and success (1). We no longer conflate errors with success. Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- hw/xfree86/drivers/modesetting/present.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c index 359e113..3e8e72a 100644 --- a/hw/xfree86/drivers/modesetting/present.c +++ b/hw/xfree86/drivers/modesetting/present.c @@ -72,8 +72,11 @@ ms_present_get_ust_msc(RRCrtcPtr crtc, CARD64 *ust, CARD64 *msc) /* * Flush the DRM event queue when full; makes space for new events. + * + * Returns a negative value on error, 0 if there was nothing to process, + * or 1 if we handled any events. */ -static Bool +static int ms_flush_drm_events(ScreenPtr screen) { ScrnInfoPtr scrn = xf86ScreenToScrn(screen); @@ -86,10 +89,19 @@ ms_flush_drm_events(ScreenPtr screen) r = poll(p, 1, 0); } while (r == -1 (errno == EINTR || errno == EAGAIN)); +/* If there was an error, r will be 0. Return that. If there was + * nothing to process, r == 0. Return that. + */ if (r = 0) -return TRUE; +return r; AFAICT the only functional change of this patch is that this case of poll() returning = 0 is now treated as failure, whereas it was previously treated as success. Not sure that warrants the API change, but anyway I think it would be good to have that one-liner bugfix in a separate change instead of buried in the API change. +/* Try to handle the event. If there was an error, return it. */ +r = drmHandleEvent(ms-fd, ms-event_context); +if (r 0) +return r; -return drmHandleEvent(ms-fd, ms-event_context) = 0; +/* Otherwise return 1 to indicate that we handled an event. */ +return 1; } /* @@ -159,7 +171,10 @@ ms_present_queue_vblank(RRCrtcPtr crtc, ret = drmWaitVBlank(ms-fd, vbl); if (!ret) break; -if (errno != EBUSY || !ms_flush_drm_events(screen)) +/* If we hit EBUSY, then try to flush events. If we can't, then + * this is an error. + */ +if (errno != EBUSY || ms_flush_drm_events(screen) = 0) return BadAlloc; } DebugPresent((\t\tmq %lld seq %u msc %llu (hw msc %u)\n, -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] glamor: don't do render ops with matching source/dest (v2)
Dave Airlie airl...@gmail.com writes: From: Dave Airlie airl...@redhat.com XRender defines this, GL really doesn't like it. kwin 4.x and qt 4.x seem to make this happen for the gradient in the titlebar, and on radeonsi/r600 hw this draws all kinds of wrong. v2: bump this up a level, and check it earlier. (I assume the was for this case.) You're missing the matching case in the large_pixmap code -- -keith signature.asc 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 2/3] present: Query the Window's CRTC every time
On 28.05.2015 18:07, Chris Wilson wrote: On Thu, May 28, 2015 at 05:56:15PM +0900, Michel Dänzer wrote: On 28.05.2015 17:38, Chris Wilson wrote: On Thu, May 28, 2015 at 04:59:14PM +0900, Michel Dänzer wrote: The patch below is an alternative fix for the problem I'm seeing, while preserving the window CRTC for MSC waits when possible. Your description sounds like it might work for you as well, Chris. Can you try it? diff --git a/present/present.c b/present/present.c index a634601..dc58f25 100644 --- a/present/present.c +++ b/present/present.c @@ -713,6 +713,7 @@ present_pixmap(WindowPtr window, uint64_tust; uint64_ttarget_msc; uint64_tcrtc_msc; +RRCrtcPtr new_crtc; int ret; present_vblank_ptr vblank, tmp; ScreenPtr screen = window-drawable.pScreen; @@ -734,7 +735,21 @@ present_pixmap(WindowPtr window, target_crtc = present_get_crtc(window); } -present_get_ust_msc(screen, target_crtc, ust, crtc_msc); +if (present_get_ust_msc(screen, target_crtc, ust, crtc_msc) != Success) { +/* Maybe we need to try a different CRTC? + */ +new_crtc = present_get_crtc(window); + +if (new_crtc != target_crtc +present_get_ust_msc(screen, new_crtc, ust, crtc_msc) == Success) +target_crtc = new_crtc; +else { +/* Fall back to fake MSC handling + */ +target_crtc = NULL; +present_fake_get_ust_msc(screen, ust, crtc_msc); +} +} It survived for one more CRTC change, but still feeds passed msc into the wait_vblank. By how much is it off? Does it cause a hang? Just a few -1000 msc, which is a pretty long wait (it would be if we honoured it at least). The kernel should immediately return / send the event in that case. What's your test-case? Mine is the GNOME lock screen, i.e. basically DPMS off vs vblank waits and flips. xf86-video-intel/test/present-test Thanks, I'll take a look at that when I get a chance. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3] present: Query the Window's CRTC every time
On Thu, May 28, 2015 at 06:27:34PM +0900, Michel Dänzer wrote: On 28.05.2015 18:07, Chris Wilson wrote: On Thu, May 28, 2015 at 05:56:15PM +0900, Michel Dänzer wrote: On 28.05.2015 17:38, Chris Wilson wrote: On Thu, May 28, 2015 at 04:59:14PM +0900, Michel Dänzer wrote: The patch below is an alternative fix for the problem I'm seeing, while preserving the window CRTC for MSC waits when possible. Your description sounds like it might work for you as well, Chris. Can you try it? diff --git a/present/present.c b/present/present.c index a634601..dc58f25 100644 --- a/present/present.c +++ b/present/present.c @@ -713,6 +713,7 @@ present_pixmap(WindowPtr window, uint64_tust; uint64_ttarget_msc; uint64_tcrtc_msc; +RRCrtcPtr new_crtc; int ret; present_vblank_ptr vblank, tmp; ScreenPtr screen = window-drawable.pScreen; @@ -734,7 +735,21 @@ present_pixmap(WindowPtr window, target_crtc = present_get_crtc(window); } -present_get_ust_msc(screen, target_crtc, ust, crtc_msc); +if (present_get_ust_msc(screen, target_crtc, ust, crtc_msc) != Success) { +/* Maybe we need to try a different CRTC? + */ +new_crtc = present_get_crtc(window); + +if (new_crtc != target_crtc +present_get_ust_msc(screen, new_crtc, ust, crtc_msc) == Success) +target_crtc = new_crtc; +else { +/* Fall back to fake MSC handling + */ +target_crtc = NULL; +present_fake_get_ust_msc(screen, ust, crtc_msc); +} +} It survived for one more CRTC change, but still feeds passed msc into the wait_vblank. By how much is it off? Does it cause a hang? Just a few -1000 msc, which is a pretty long wait (it would be if we honoured it at least). The kernel should immediately return / send the event in that case. Given that it is supposedly an impossible condition in present_pixmap, it is simple evidence of a bug. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel