Re: [PATCH] DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's

2010-06-17 Thread Mario Kleiner

Hi Keith and Peter,

this patch has been reviewed by Jesse Barnes and now successfully  
tested by the guy who submitted the bug report and by me. Fixes  
Bugzilla bug #28383, a quite ugly hang of application windows  
bufferswapping, including crashes when moving them between different  
crtc's of different refresh rate.


It would be good to apply it to 1.9 and 1.8.2 if possible, for a more  
enjoyable multi-display OpenGL experience.


thanks,
-mario

On Jun 14, 2010, at 6:13 PM, Jesse Barnes wrote:


On Sun, 13 Jun 2010 18:05:26 +0200
Mario Kleiner mario.klei...@tuebingen.mpg.de wrote:

Detect if a drawable has been moved from an original crtc to a new  
crtc

with a lower current vblank count than the original crtc inbetween
glXSwapBuffers() calls. Reinitialize drawable's last_swap_target
before scheduling next swap if such a move has taken place.

last_swap_target defines the baseline for scheduling the next swap.
If a movement between crtc's is not taken into account, the swap may
schedule for a vblank count on the new crtc far in the future,  
resulting

in a apparent hang of the drawable for a long time.

Fixes Bugzilla bug #28383.

Signed-off-by: Mario Kleiner mario.klei...@tuebingen.mpg.de
---
 hw/xfree86/dri2/dri2.c |   15 +++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index d33b0d1..1f80022 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -756,6 +756,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr  
pDraw, CARD64 target_msc,

 DRI2DrawablePtr pPriv;
 DRI2BufferPtr   pDestBuffer = NULL, pSrcBuffer = NULL;
 int ret, i;
+CARD64  ust, current_msc;

 pPriv = DRI2GetDrawable(pDraw);
 if (pPriv == NULL) {
@@ -800,12 +801,26 @@ DRI2SwapBuffers(ClientPtr client,  
DrawablePtr pDraw, CARD64 target_msc,
  * need to schedule a swap for the last swap target + the  
swap interval.

  */
 if (target_msc == 0  divisor == 0  remainder == 0) {
+   /* If the current vblank count of the drawable's crtc is lower
+* than the count stored in last_swap_target from a previous swap
+* then reinitialize last_swap_target to the current crtc's msc,
+* otherwise the swap will hang. This will happen if the drawable
+	 * is moved to a crtc with a lower refresh rate, or a crtc that  
just

+* got enabled.
+*/
+   if (!(*ds-GetMSC)(pDraw, ust, current_msc))
+   pPriv-last_swap_target = 0;
+
+   if (current_msc  pPriv-last_swap_target)
+   pPriv-last_swap_target = current_msc;
+
/*
 	 * Swap target for this swap is last swap target + swap interval  
since

 * we have to account for the current swap count, interval, and the
 * number of pending swaps.
 */
*swap_target = pPriv-last_swap_target + pPriv-swap_interval;
+
 } else {
/* glXSwapBuffersMscOML could have a 0 target_msc, honor it */
*swap_target = target_msc;


Yeah, this looks ok.  Really we should write up GLX extension that
clarifies behavior in multi-head and display on/off situations  
too.  Is

that something you could do?

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

--
Jesse Barnes, Intel Open Source Technology Center



___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's

2010-06-17 Thread Peter Hutterer
On Fri, Jun 18, 2010 at 03:22:53AM +0200, Mario Kleiner wrote:
 Hi Keith and Peter,
 
 this patch has been reviewed by Jesse Barnes and now successfully  
 tested by the guy who submitted the bug report and by me. Fixes  
 Bugzilla bug #28383, a quite ugly hang of application windows  
 bufferswapping, including crashes when moving them between different  
 crtc's of different refresh rate.
 
 It would be good to apply it to 1.9 and 1.8.2 if possible, for a more  
 enjoyable multi-display OpenGL experience.

It's on my list, I'm just waiting for it to land on master.

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


Re: [PATCH] DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's

2010-06-14 Thread Jesse Barnes
On Sun, 13 Jun 2010 18:05:26 +0200
Mario Kleiner mario.klei...@tuebingen.mpg.de wrote:

 Detect if a drawable has been moved from an original crtc to a new crtc
 with a lower current vblank count than the original crtc inbetween
 glXSwapBuffers() calls. Reinitialize drawable's last_swap_target
 before scheduling next swap if such a move has taken place.
 
 last_swap_target defines the baseline for scheduling the next swap.
 If a movement between crtc's is not taken into account, the swap may
 schedule for a vblank count on the new crtc far in the future, resulting
 in a apparent hang of the drawable for a long time.
 
 Fixes Bugzilla bug #28383.
 
 Signed-off-by: Mario Kleiner mario.klei...@tuebingen.mpg.de
 ---
  hw/xfree86/dri2/dri2.c |   15 +++
  1 files changed, 15 insertions(+), 0 deletions(-)
 
 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index d33b0d1..1f80022 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -756,6 +756,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, 
 CARD64 target_msc,
  DRI2DrawablePtr pPriv;
  DRI2BufferPtr   pDestBuffer = NULL, pSrcBuffer = NULL;
  int ret, i;
 +CARD64  ust, current_msc;
  
  pPriv = DRI2GetDrawable(pDraw);
  if (pPriv == NULL) {
 @@ -800,12 +801,26 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, 
 CARD64 target_msc,
   * need to schedule a swap for the last swap target + the swap interval.
   */
  if (target_msc == 0  divisor == 0  remainder == 0) {
 + /* If the current vblank count of the drawable's crtc is lower
 +  * than the count stored in last_swap_target from a previous swap
 +  * then reinitialize last_swap_target to the current crtc's msc,
 +  * otherwise the swap will hang. This will happen if the drawable
 +  * is moved to a crtc with a lower refresh rate, or a crtc that just
 +  * got enabled.
 +  */
 + if (!(*ds-GetMSC)(pDraw, ust, current_msc))
 + pPriv-last_swap_target = 0;
 +
 + if (current_msc  pPriv-last_swap_target)
 + pPriv-last_swap_target = current_msc;
 +
   /*
* Swap target for this swap is last swap target + swap interval since
* we have to account for the current swap count, interval, and the
* number of pending swaps.
*/
   *swap_target = pPriv-last_swap_target + pPriv-swap_interval;
 +
  } else {
   /* glXSwapBuffersMscOML could have a 0 target_msc, honor it */
   *swap_target = target_msc;

Yeah, this looks ok.  Really we should write up GLX extension that
clarifies behavior in multi-head and display on/off situations too.  Is
that something you could do?

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH] DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's

2010-06-13 Thread Mario Kleiner

Hi

This patch fixes a hang in glXSwapBuffers if a user moves
a drawable from a fast running crtc, e.g., 60 Hz to a slower
running crtc, e.g., 50 Hz, when using the new DRI2 sync  swap bits.

It should fix Bugzilla bug #28383.

https://bugs.freedesktop.org/show_bug.cgi?id=28383

I've tested this on a R600 against master. I assume the poster
of that bug report will test it as well. He has tested a previous
version of the patch.

Jesse could you review it? Would be good if this could make
it into master and 1.8.2 as well.

thanks,
-mario

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH] DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's

2010-06-13 Thread Mario Kleiner
Detect if a drawable has been moved from an original crtc to a new crtc
with a lower current vblank count than the original crtc inbetween
glXSwapBuffers() calls. Reinitialize drawable's last_swap_target
before scheduling next swap if such a move has taken place.

last_swap_target defines the baseline for scheduling the next swap.
If a movement between crtc's is not taken into account, the swap may
schedule for a vblank count on the new crtc far in the future, resulting
in a apparent hang of the drawable for a long time.

Fixes Bugzilla bug #28383.

Signed-off-by: Mario Kleiner mario.klei...@tuebingen.mpg.de
---
 hw/xfree86/dri2/dri2.c |   15 +++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index d33b0d1..1f80022 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -756,6 +756,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 
target_msc,
 DRI2DrawablePtr pPriv;
 DRI2BufferPtr   pDestBuffer = NULL, pSrcBuffer = NULL;
 int ret, i;
+CARD64  ust, current_msc;
 
 pPriv = DRI2GetDrawable(pDraw);
 if (pPriv == NULL) {
@@ -800,12 +801,26 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, 
CARD64 target_msc,
  * need to schedule a swap for the last swap target + the swap interval.
  */
 if (target_msc == 0  divisor == 0  remainder == 0) {
+   /* If the current vblank count of the drawable's crtc is lower
+* than the count stored in last_swap_target from a previous swap
+* then reinitialize last_swap_target to the current crtc's msc,
+* otherwise the swap will hang. This will happen if the drawable
+* is moved to a crtc with a lower refresh rate, or a crtc that just
+* got enabled.
+*/
+   if (!(*ds-GetMSC)(pDraw, ust, current_msc))
+   pPriv-last_swap_target = 0;
+
+   if (current_msc  pPriv-last_swap_target)
+   pPriv-last_swap_target = current_msc;
+
/*
 * Swap target for this swap is last swap target + swap interval since
 * we have to account for the current swap count, interval, and the
 * number of pending swaps.
 */
*swap_target = pPriv-last_swap_target + pPriv-swap_interval;
+
 } else {
/* glXSwapBuffersMscOML could have a 0 target_msc, honor it */
*swap_target = target_msc;
-- 
1.6.6

___
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