Re: [Mesa-dev] [PATCH 7.11] gallium/dri: Handle xserver that doesn't send needless DRI2 invalidate events

2012-01-15 Thread Ville Syrjälä
On Fri, Jan 13, 2012 at 09:10:15AM +, Dave Airlie wrote:
 On Sun, Dec 18, 2011 at 4:22 PM, Ville Syrjälä syrj...@sci.fi wrote:
  Ever since xserver commit 531869448d07e00ae241120b59f35709d59c,
  the server no longer sends invalidate events to clients, unless they
  have performed a GetBuffers request since the drawable was last
  invalidated.
 
  If the drawable gets invalidated immediately after the GetBuffers
  request was processed by the X server, it's possible that Xlib
  will process the invalidate event while waiting for the GetBuffers
  reply. So the server, thinking the client knows that the buffers
  are invalid, is waiting for another GetBuffers request before
  sending any more invalidate events. The client, on the other hand,
  believes the buffers to be valid, and thus is expecting to receive
  another invalidate event before it has to send another GetBuffers
  request. The end result is that the client never again sends
  a GetBuffers request.
 
  To avoid this problem, take a snapshot of lastStamp before
  doing GetBuffers, and retry if the snapshot and the current
  lastStamp no longer match after the GetBuffers reply has been
  processed.
 
  Signed-off-by: Ville Syrjälä syrj...@sci.fi
  ---
  It looks like master should already handle this, as there's a
  retry loop inside st_framebuffer_validate(). I didn't test that
  in practice though.
 
 I'd really like to know if master can handle it before pulling a patch
 that isn't in master into 7.11.

I now managed to test master as well, and in fact it's also affected
by this bug.

After thinking about it a bit more I can see why that is.
st_framebuffer_validate() has the retry loop all right, but when it
calls dri_st_framebuffer_validate() during the retry,
dri_st_framebuffer_validate() thinks that the buffers are valid and
doesn't call allocate_textures(). Hence the retry loop actually does
nothing useful in this case.

So the 7.11 fix can be applied to master as well (the patch applies to
master cleanly), or we could go for a slightly more simple fix for
master due to the pre-existing retry loop. I'll leave that decision up
to someone else.

This is the simple approach:

diff --git a/src/gallium/state_trackers/dri/common/dri_drawable.c 
b/src/gallium/state_trackers/dri/common/dri_drawable.c
index 65b3d04..6effb11 100644
--- a/src/gallium/state_trackers/dri/common/dri_drawable.c
+++ b/src/gallium/state_trackers/dri/common/dri_drawable.c
@@ -53,6 +53,7 @@ dri_st_framebuffer_validate(struct st_framebuffer_iface 
*stfbi,
unsigned statt_mask, new_mask;
boolean new_stamp;
int i;
+   unsigned int lastStamp;
 
statt_mask = 0x0;
for (i = 0; i  count; i++)
@@ -66,7 +67,8 @@ dri_st_framebuffer_validate(struct st_framebuffer_iface 
*stfbi,
 * client stamp.  It has the value of the server stamp when last
 * checked.
 */
-   new_stamp = (drawable-texture_stamp != drawable-dPriv-lastStamp);
+   lastStamp = drawable-dPriv-lastStamp;
+   new_stamp = (drawable-texture_stamp != lastStamp);
 
if (new_stamp || new_mask || screen-broken_invalidate) {
   if (new_stamp  drawable-update_drawable_info)
@@ -80,7 +82,7 @@ dri_st_framebuffer_validate(struct st_framebuffer_iface 
*stfbi,
 statt_mask |= (1  i);
   }
 
-  drawable-texture_stamp = drawable-dPriv-lastStamp;
+  drawable-texture_stamp = lastStamp;
   drawable-texture_mask = statt_mask;
}
 

To verify my bugfix I used xtrace like so:
xtrace -n -k | tee mesa.log | grep -B1 Invalidate | grep Get

When the bug is tripped that will print the GetBuffers reply that was
being processed when the bad InvalidateBuffers event was received.

Using that xtrace incantation and glxgears I verified that both the
original patch and the simple patch fix the bug.

-- 
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 7.11] gallium/dri: Handle xserver that doesn't send needless DRI2 invalidate events

2012-01-13 Thread Ville Syrjälä
PING PING PING! No one cares about Mesa 7.11 anymore?

On Sat, Jan 07, 2012 at 12:12:24AM +0200, Ville Syrjälä wrote:
 On Sun, Dec 18, 2011 at 06:22:01PM +0200, Ville Syrjälä wrote:
  Ever since xserver commit 531869448d07e00ae241120b59f35709d59c,
  the server no longer sends invalidate events to clients, unless they
  have performed a GetBuffers request since the drawable was last
  invalidated.
  
  If the drawable gets invalidated immediately after the GetBuffers
  request was processed by the X server, it's possible that Xlib
  will process the invalidate event while waiting for the GetBuffers
  reply. So the server, thinking the client knows that the buffers
  are invalid, is waiting for another GetBuffers request before
  sending any more invalidate events. The client, on the other hand,
  believes the buffers to be valid, and thus is expecting to receive
  another invalidate event before it has to send another GetBuffers
  request. The end result is that the client never again sends
  a GetBuffers request.
  
  To avoid this problem, take a snapshot of lastStamp before
  doing GetBuffers, and retry if the snapshot and the current
  lastStamp no longer match after the GetBuffers reply has been
  processed.
  
  Signed-off-by: Ville Syrjälä syrj...@sci.fi
 
 Ping. Anyone interested in having Mesa 7.11 work with
 xserver = 1.11?
 
 -- 
 Ville Syrjälä
 syrj...@sci.fi
 http://www.sci.fi/~syrjala/

-- 
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 7.11] gallium/dri: Handle xserver that doesn't send needless DRI2 invalidate events

2012-01-13 Thread Dave Airlie
On Sun, Dec 18, 2011 at 4:22 PM, Ville Syrjälä syrj...@sci.fi wrote:
 Ever since xserver commit 531869448d07e00ae241120b59f35709d59c,
 the server no longer sends invalidate events to clients, unless they
 have performed a GetBuffers request since the drawable was last
 invalidated.

 If the drawable gets invalidated immediately after the GetBuffers
 request was processed by the X server, it's possible that Xlib
 will process the invalidate event while waiting for the GetBuffers
 reply. So the server, thinking the client knows that the buffers
 are invalid, is waiting for another GetBuffers request before
 sending any more invalidate events. The client, on the other hand,
 believes the buffers to be valid, and thus is expecting to receive
 another invalidate event before it has to send another GetBuffers
 request. The end result is that the client never again sends
 a GetBuffers request.

 To avoid this problem, take a snapshot of lastStamp before
 doing GetBuffers, and retry if the snapshot and the current
 lastStamp no longer match after the GetBuffers reply has been
 processed.

 Signed-off-by: Ville Syrjälä syrj...@sci.fi
 ---
 It looks like master should already handle this, as there's a
 retry loop inside st_framebuffer_validate(). I didn't test that
 in practice though.

I'd really like to know if master can handle it before pulling a patch
that isn't in master into 7.11.

Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 7.11] gallium/dri: Handle xserver that doesn't send needless DRI2 invalidate events

2012-01-13 Thread Ville Syrjälä
On Fri, Jan 13, 2012 at 09:10:15AM +, Dave Airlie wrote:
 On Sun, Dec 18, 2011 at 4:22 PM, Ville Syrjälä syrj...@sci.fi wrote:
  Ever since xserver commit 531869448d07e00ae241120b59f35709d59c,
  the server no longer sends invalidate events to clients, unless they
  have performed a GetBuffers request since the drawable was last
  invalidated.
 
  If the drawable gets invalidated immediately after the GetBuffers
  request was processed by the X server, it's possible that Xlib
  will process the invalidate event while waiting for the GetBuffers
  reply. So the server, thinking the client knows that the buffers
  are invalid, is waiting for another GetBuffers request before
  sending any more invalidate events. The client, on the other hand,
  believes the buffers to be valid, and thus is expecting to receive
  another invalidate event before it has to send another GetBuffers
  request. The end result is that the client never again sends
  a GetBuffers request.
 
  To avoid this problem, take a snapshot of lastStamp before
  doing GetBuffers, and retry if the snapshot and the current
  lastStamp no longer match after the GetBuffers reply has been
  processed.
 
  Signed-off-by: Ville Syrjälä syrj...@sci.fi
  ---
  It looks like master should already handle this, as there's a
  retry loop inside st_framebuffer_validate(). I didn't test that
  in practice though.
 
 I'd really like to know if master can handle it before pulling a patch
 that isn't in master into 7.11.

Well, for someone using master, it should be easy to test. Just run
glxgears and resize the window aggressively. So far I've been too lazy
to figure out what, if anything, I need to upgrade to try master.

-- 
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 7.11] gallium/dri: Handle xserver that doesn't send needless DRI2 invalidate events

2012-01-06 Thread Ville Syrjälä
On Sun, Dec 18, 2011 at 06:22:01PM +0200, Ville Syrjälä wrote:
 Ever since xserver commit 531869448d07e00ae241120b59f35709d59c,
 the server no longer sends invalidate events to clients, unless they
 have performed a GetBuffers request since the drawable was last
 invalidated.
 
 If the drawable gets invalidated immediately after the GetBuffers
 request was processed by the X server, it's possible that Xlib
 will process the invalidate event while waiting for the GetBuffers
 reply. So the server, thinking the client knows that the buffers
 are invalid, is waiting for another GetBuffers request before
 sending any more invalidate events. The client, on the other hand,
 believes the buffers to be valid, and thus is expecting to receive
 another invalidate event before it has to send another GetBuffers
 request. The end result is that the client never again sends
 a GetBuffers request.
 
 To avoid this problem, take a snapshot of lastStamp before
 doing GetBuffers, and retry if the snapshot and the current
 lastStamp no longer match after the GetBuffers reply has been
 processed.
 
 Signed-off-by: Ville Syrjälä syrj...@sci.fi

Ping. Anyone interested in having Mesa 7.11 work with
xserver = 1.11?

-- 
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 7.11] gallium/dri: Handle xserver that doesn't send needless DRI2 invalidate events

2011-12-18 Thread Ville Syrjälä
Ever since xserver commit 531869448d07e00ae241120b59f35709d59c,
the server no longer sends invalidate events to clients, unless they
have performed a GetBuffers request since the drawable was last
invalidated.

If the drawable gets invalidated immediately after the GetBuffers
request was processed by the X server, it's possible that Xlib
will process the invalidate event while waiting for the GetBuffers
reply. So the server, thinking the client knows that the buffers
are invalid, is waiting for another GetBuffers request before
sending any more invalidate events. The client, on the other hand,
believes the buffers to be valid, and thus is expecting to receive
another invalidate event before it has to send another GetBuffers
request. The end result is that the client never again sends
a GetBuffers request.

To avoid this problem, take a snapshot of lastStamp before
doing GetBuffers, and retry if the snapshot and the current
lastStamp no longer match after the GetBuffers reply has been
processed.

Signed-off-by: Ville Syrjälä syrj...@sci.fi
---
It looks like master should already handle this, as there's a
retry loop inside st_framebuffer_validate(). I didn't test that
in practice though.

 .../state_trackers/dri/common/dri_drawable.c   |   30 +++
 1 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/gallium/state_trackers/dri/common/dri_drawable.c 
b/src/gallium/state_trackers/dri/common/dri_drawable.c
index 28a33ac..6748fbc 100644
--- a/src/gallium/state_trackers/dri/common/dri_drawable.c
+++ b/src/gallium/state_trackers/dri/common/dri_drawable.c
@@ -51,6 +51,7 @@ dri_st_framebuffer_validate(struct st_framebuffer_iface 
*stfbi,
unsigned statt_mask, new_mask;
boolean new_stamp;
int i;
+   unsigned int lastStamp;
 
statt_mask = 0x0;
for (i = 0; i  count; i++)
@@ -64,23 +65,26 @@ dri_st_framebuffer_validate(struct st_framebuffer_iface 
*stfbi,
 * least for DRI1.  dPriv-lastStamp is the client stamp.  It has the value
 * of the server stamp when last checked.
 */
-   new_stamp = (drawable-texture_stamp != drawable-dPriv-lastStamp);
+   do {
+  lastStamp = drawable-dPriv-lastStamp;
+  new_stamp = (drawable-texture_stamp != lastStamp);
 
-   if (new_stamp || new_mask || screen-broken_invalidate) {
-  if (new_stamp  drawable-update_drawable_info)
- drawable-update_drawable_info(drawable);
+  if (new_stamp || new_mask || screen-broken_invalidate) {
+ if (new_stamp  drawable-update_drawable_info)
+drawable-update_drawable_info(drawable);
 
-  drawable-allocate_textures(drawable, statts, count);
+ drawable-allocate_textures(drawable, statts, count);
 
-  /* add existing textures */
-  for (i = 0; i  ST_ATTACHMENT_COUNT; i++) {
- if (drawable-textures[i])
-statt_mask |= (1  i);
-  }
+ /* add existing textures */
+ for (i = 0; i  ST_ATTACHMENT_COUNT; i++) {
+if (drawable-textures[i])
+   statt_mask |= (1  i);
+ }
 
-  drawable-texture_stamp = drawable-dPriv-lastStamp;
-  drawable-texture_mask = statt_mask;
-   }
+ drawable-texture_stamp = lastStamp;
+ drawable-texture_mask = statt_mask;
+  }
+   } while (lastStamp != drawable-dPriv-lastStamp);
 
if (!out)
   return TRUE;
-- 
1.7.3.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev