Re: [PATCH RFC] configurable maximum number of clients

2015-05-28 Thread Alan Coopersmith

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)

2015-05-28 Thread Eric Anholt
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

2015-05-28 Thread Jasper St. Pierre
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

2015-05-28 Thread Michel Dänzer
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

2015-05-28 Thread Peter Hutterer
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)

2015-05-28 Thread Dave Airlie
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

2015-05-28 Thread Michel Dänzer
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

2015-05-28 Thread Peter Hutterer
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

2015-05-28 Thread Chris Wilson
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

2015-05-28 Thread Chris Wilson
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

2015-05-28 Thread Michel Dänzer
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)

2015-05-28 Thread Dave Airlie
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)

2015-05-28 Thread Dave Airlie
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.

2015-05-28 Thread Michel Dänzer
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)

2015-05-28 Thread Keith Packard
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

2015-05-28 Thread Michel Dänzer
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

2015-05-28 Thread Chris Wilson
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