From c4d54816f2ee4883d8f9bcf4595474fb58c95146 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Thu, 4 Mar 2010 09:19:13 -0800 Subject: [PATCH 01/12] DRI2: fixup handling of last_swap_target We need to initialize the swap target, which is passed to the driver to schedule events. Rather than using -1 to indicate that the field is uninitialized, just make sure we initialize it at drawable creation time. Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes --- hw/xfree86/dri2/dri2.c | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 48618e1a5..d60bd5e18 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -116,9 +116,11 @@ DRI2GetDrawable(DrawablePtr pDraw) int DRI2CreateDrawable(DrawablePtr pDraw) { + DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); WindowPtr pWin; PixmapPtr pPixmap; DRI2DrawablePtr pPriv; + CARD64 ust; pPriv = DRI2GetDrawable(pDraw); if (pPriv != NULL) @@ -141,7 +143,10 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->swap_count = 0; pPriv->target_sbc = -1; pPriv->swap_interval = 1; - pPriv->last_swap_target = -1; + /* Initialize last swap target from DDX if possible */ + if (!ds->GetMSC || !(*ds->GetMSC)(pDraw, &ust, &pPriv->last_swap_target)) + pPriv->last_swap_target = 0; + pPriv->swap_limit = 1; /* default to double buffering */ if (pDraw->type == DRAWABLE_WINDOW) @@ -579,7 +584,6 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); DRI2DrawablePtr pPriv; DRI2BufferPtr pDestBuffer = NULL, pSrcBuffer = NULL; - CARD64 ust; int ret, i; pPriv = DRI2GetDrawable(pDraw); @@ -620,27 +624,6 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, return Success; } - /* - * In the simple glXSwapBuffers case, all params will be 0, and we just - * need to schedule a swap for the last swap target + the swap interval. - * If the last swap target hasn't been set yet, call into the driver - * to get the current count. - */ - if (target_msc == 0 && divisor == 0 && remainder == 0 && - pPriv->last_swap_target < 0) { - ret = (*ds->GetMSC)(pDraw, &ust, &target_msc); - if (!ret) { - xf86DrvMsg(pScreen->myNum, X_ERROR, - "[DRI2] %s: driver failed to return current MSC\n", - __func__); - return BadDrawable; - } - } - - /* First swap needs to initialize last_swap_target */ - if (pPriv->last_swap_target < 0) - pPriv->last_swap_target = target_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 From 4c8ec49826a46eb3b36c69d2ad3f82320c179c38 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Thu, 4 Mar 2010 09:54:15 -0800 Subject: [PATCH 02/12] DRI2: make target_sbc signed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to track invalid targets as well as 0 targets, so just make it signed so our comparisons work like they should. Reviewed-by: Mario Kleiner Reported-by: Kristian Høgsberg Signed-off-by: Jesse Barnes --- hw/xfree86/dri2/dri2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index d60bd5e18..ec4f982b4 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -62,7 +62,7 @@ typedef struct _DRI2Drawable { ClientPtr blockedClient; int swap_interval; CARD64 swap_count; - CARD64 target_sbc; /* -1 means no SBC wait outstanding */ + int64_t target_sbc; /* -1 means no SBC wait outstanding */ CARD64 last_swap_target; /* most recently queued swap target */ int swap_limit; /* for N-buffering */ } DRI2DrawableRec, *DRI2DrawablePtr; From 0de4974b90b10fa6a447cdf980b4a114c6c9e5a8 Mon Sep 17 00:00:00 2001 From: Mario Kleiner Date: Sun, 21 Feb 2010 05:25:59 +0100 Subject: [PATCH 03/12] DRI2: Fix glitches in DRI2SwapComplete() and DRI2WakeupClient() DRI2SwapComplete(): Increment pPriv->swap_count++; before calling into callback for INTEL_swap_events extension, so the swap event contains the current SBC after swap completion instead of the previous one. DRI2WakeupClient: Check for pPriv->target_sbc <= pPriv->swap_count, had wrong comparison pPriv->target_sbc >= pPriv->swap_count for unblocking of clients of DRI2WaitSBC(). Signed-off-by: Mario Kleiner --- hw/xfree86/dri2/dri2.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index ec4f982b4..cb227be3f 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -509,7 +509,7 @@ DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame, * blocked due to GLX activity during a swap. */ if (pPriv->target_sbc != -1 && - pPriv->target_sbc >= pPriv->swap_count) { + pPriv->target_sbc <= pPriv->swap_count) { ProcDRI2WaitMSCReply(client, ((CARD64)tv_sec * 1000000) + tv_usec, frame, pPriv->swap_count); pPriv->target_sbc = -1; @@ -546,13 +546,13 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, return; } + pPriv->swapsPending--; + pPriv->swap_count++; + ust = ((CARD64)tv_sec * 1000000) + tv_usec; if (swap_complete) swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count); - pPriv->swapsPending--; - pPriv->swap_count++; - DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec); } From 751e8c09d34df4b41e8d8384a3ec1bf5cb8ca028 Mon Sep 17 00:00:00 2001 From: Mario Kleiner Date: Sun, 21 Feb 2010 05:26:00 +0100 Subject: [PATCH 04/12] DRI2WaitSbc(): Fixes for correct semantic of glXWaitForSbcOML() Added implementation for case target_sbc == 0. In that case, the function shall schedule a wait until all pending swaps for the drawable have completed. Fix for non-blocking case. Old implementation returned random, uninitialized values for (ust,msc,sbc) if it returned immediately without scheduling a wait due to sbc >= target_sbc. Now if function doesn't schedule a wait, but returns immediately, it returns the (ust,msc,sbc) of the most recently completed swap, i.e., the UST and MSC corresponding to the time when the returned current SBC was reached. Signed-off-by: Mario Kleiner --- hw/xfree86/dri2/dri2.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index cb227be3f..ef838ff6b 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -64,6 +64,8 @@ typedef struct _DRI2Drawable { CARD64 swap_count; int64_t target_sbc; /* -1 means no SBC wait outstanding */ CARD64 last_swap_target; /* most recently queued swap target */ + CARD64 last_swap_msc; /* msc at completion of most recent swap */ + CARD64 last_swap_ust; /* ust at completion of most recent swap */ int swap_limit; /* for N-buffering */ } DRI2DrawableRec, *DRI2DrawablePtr; @@ -148,6 +150,8 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->last_swap_target = 0; pPriv->swap_limit = 1; /* default to double buffering */ + pPriv->last_swap_msc = 0; + pPriv->last_swap_ust = 0; if (pDraw->type == DRAWABLE_WINDOW) { @@ -553,6 +557,9 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, if (swap_complete) swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count); + pPriv->last_swap_msc = frame; + pPriv->last_swap_ust = ust; + DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec); } @@ -727,8 +734,22 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, if (pPriv == NULL) return BadDrawable; - if (pPriv->swap_count >= target_sbc) - return Success; + /* target_sbc == 0 means to block until all pending swaps are + * finished. Recalculate target_sbc to get that behaviour. + */ + if (target_sbc == 0) + target_sbc = pPriv->swap_count + pPriv->swapsPending; + + /* If current swap count already >= target_sbc, + * return immediately with (ust, msc, sbc) triplet of + * most recent completed swap. + */ + if (pPriv->swap_count >= target_sbc) { + *sbc = pPriv->swap_count; + *msc = pPriv->last_swap_msc; + *ust = pPriv->last_swap_ust; + return Success; + } pPriv->target_sbc = target_sbc; DRI2BlockClient(client, pDraw); From b180e43977710b56ccfd6780f204ddcc952987a1 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Thu, 4 Mar 2010 10:31:59 -0800 Subject: [PATCH 05/12] DRI2: fix swapbuffers handling of SBC and target MSC Returns expected SBC after completion of swap to caller, as required by OML_sync_control spec, instead of the last_swap_target value. Passes target_msc, divisor, remainder, correctly for glXSwapBuffersMscOML() call, while retaining old behaviour for simple glXSwapBuffers() call. An OML swap can have a 0 target_msc, which just means it needs to satisfy the divisor/remainder equation. Pass this down to the driver as needed so we can support it. Signed-off-by: Jesse Barnes Signed-off-by: Mario Kleiner --- hw/xfree86/dri2/dri2.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index ef838ff6b..354b72434 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -632,11 +632,20 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_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. + * In the simple glXSwapBuffers case, all params will be 0, and we just + * need to schedule a swap for the last swap target + the swap interval. */ - *swap_target = pPriv->last_swap_target + pPriv->swap_interval; + if (target_msc == 0 && divisor == 0 && remainder == 0) { + /* + * 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; + } ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer, swap_target, divisor, remainder, func, data); @@ -649,6 +658,11 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, pPriv->swapsPending++; pPriv->last_swap_target = *swap_target; + /* According to spec, return expected swapbuffers count SBC after this swap + * will complete. + */ + *swap_target = pPriv->swap_count + pPriv->swapsPending; + return Success; } From 8476d99231cb725c090305d60f1c1c889d25c8dc Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Fri, 5 Mar 2010 09:15:24 -0800 Subject: [PATCH 06/12] DRI2: drawable lifetime fixes Handle drawable destruction and lifetime correctly. Check whether the drawable priv is valid in DRI2SwapInterval(), DRI2WaitSBC() and DRI2WaitMSC(); it may have gone away, so be sure to check it before using it. If more than 1 outstanding swap is queued, we may complete several after an app has exited. If we free it after the first one completes and the refcount reaches 0, we'll crash the server on subsequent completions. So delay freeing until all swaps complete and remove the error message as this is a normal occurence. To do this properly, we must also avoid destroying drawables in DRI2DestroyDrawable() if a swap or wait event is pending. And finally, make sure we free drawables in DRI2WaitMSCComplete() if necessary (i.e. if the refcount has reached 0 and this MSC was the last pending event on the object). Reported-by: Mario Kleiner Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes --- hw/xfree86/dri2/dri2.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 354b72434..58f478f49 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -492,6 +492,10 @@ DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame, AttendClient(pPriv->blockedClient); pPriv->blockedClient = NULL; + + /* If there's still a swap pending, let DRI2SwapComplete free it */ + if (pPriv->refCount == 0 && pPriv->swapsPending == 0) + DRI2FreeDrawable(pDraw); } static void @@ -543,13 +547,6 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, return; } - if (pPriv->refCount == 0) { - xf86DrvMsg(pScreen->myNum, X_ERROR, - "[DRI2] %s: bad drawable refcount\n", __func__); - DRI2FreeDrawable(pDraw); - return; - } - pPriv->swapsPending--; pPriv->swap_count++; @@ -561,6 +558,13 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, pPriv->last_swap_ust = ust; DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec); + + /* + * It's normal for the app to have exited with a swap outstanding, but + * don't free the drawable until they're all complete. + */ + if (pPriv->swapsPending == 0 && pPriv->refCount == 0) + DRI2FreeDrawable(pDraw); } Bool @@ -669,10 +673,16 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, void DRI2SwapInterval(DrawablePtr pDrawable, int interval) { + ScreenPtr pScreen = pDrawable->pScreen; DRI2DrawablePtr pPriv = DRI2GetDrawable(pDrawable); - /* fixme: check against arbitrary max? */ + if (pPriv == NULL) { + xf86DrvMsg(pScreen->myNum, X_ERROR, + "[DRI2] %s: bad drawable\n", __func__); + return; + } + /* fixme: check against arbitrary max? */ pPriv->swap_interval = interval; } @@ -721,7 +731,7 @@ DRI2WaitMSC(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, Bool ret; pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL) + if (pPriv == NULL || pPriv->refCount == 0) return BadDrawable; /* Old DDX just completes immediately */ @@ -745,7 +755,7 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, DRI2DrawablePtr pPriv; pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL) + if (pPriv == NULL || pPriv->refCount == 0) return BadDrawable; /* target_sbc == 0 means to block until all pending swaps are @@ -794,10 +804,10 @@ DRI2DestroyDrawable(DrawablePtr pDraw) xfree(pPriv->buffers); } - /* If the window is destroyed while we have a swap pending, don't + /* If the window is destroyed while we have a swap or wait pending, don't * actually free the priv yet. We'll need it in the DRI2SwapComplete() * callback and we'll free it there once we're done. */ - if (!pPriv->swapsPending) + if (!pPriv->swapsPending && !pPriv->blockedClient) DRI2FreeDrawable(pDraw); } From 87ca6320f26eb3129e3c19056e1d8fa5c1784723 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Fri, 5 Mar 2010 09:49:03 -0800 Subject: [PATCH 07/12] DRI2: handle swap_interval of 0 correctly A 0 swap interval means that swaps shouldn't be sync'd to vblank, so just complete the swap immediately in that case. Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes --- hw/xfree86/dri2/dri2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 58f478f49..9825a55ce 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -616,8 +616,8 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, return BadDrawable; } - /* Old DDX, just blit */ - if (!ds->ScheduleSwap) { + /* Old DDX or no swap interval, just blit */ + if (!ds->ScheduleSwap || !pPriv->swap_interval) { BoxRec box; RegionRec region; From db1c7cb604167baf49e61be4c09ccf7b592c4af3 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Mon, 8 Mar 2010 12:38:37 -0800 Subject: [PATCH 08/12] DRI2: advertise lowest supported DRI2 protocol version Update our supported DRI2 protocol version as each driver does DRI2ScreenInit, since depending on available kernel features, each DDX may support different callbacks and therefore protocol. Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes --- hw/xfree86/dri2/dri2.c | 12 ++++++++++++ hw/xfree86/dri2/dri2.h | 3 +++ hw/xfree86/dri2/dri2ext.c | 4 ++-- include/protocol-versions.h | 4 ---- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 9825a55ce..3fc7f4ee2 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -45,6 +45,9 @@ #include "xf86.h" +CARD8 dri2_major; /* version of DRI2 supported by DDX */ +CARD8 dri2_minor; + static int dri2ScreenPrivateKeyIndex; static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex; static int dri2WindowPrivateKeyIndex; @@ -848,6 +851,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) "VDPAU", /* DRI2DriverVDPAU */ }; unsigned int i; + CARD8 cur_minor; if (info->version < 3) return FALSE; @@ -864,6 +868,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) ds->fd = info->fd; ds->deviceName = info->deviceName; + dri2_major = 1; ds->CreateBuffer = info->CreateBuffer; ds->DestroyBuffer = info->DestroyBuffer; @@ -873,8 +878,15 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) ds->ScheduleSwap = info->ScheduleSwap; ds->ScheduleWaitMSC = info->ScheduleWaitMSC; ds->GetMSC = info->GetMSC; + cur_minor = 2; + } else { + cur_minor = 1; } + /* Initialize minor if needed and set to minimum provied by DDX */ + if (!dri2_minor || dri2_minor > cur_minor) + dri2_minor = cur_minor; + if (info->version == 3 || info->numDrivers == 0) { /* Driver too old: use the old-style driverName field */ ds->numDrivers = 1; diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 1c8626b44..066cc3947 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -46,6 +46,9 @@ typedef struct { void *driverPrivate; } DRI2BufferRec, *DRI2BufferPtr; +extern CARD8 dri2_major; /* version of DRI2 supported by DDX */ +extern CARD8 dri2_minor; + typedef DRI2BufferRec DRI2Buffer2Rec, *DRI2Buffer2Ptr; typedef void (*DRI2SwapEventPtr)(ClientPtr client, void *data, int type, CARD64 ust, CARD64 msc, CARD64 sbc); diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index bd92fd304..7a9f8caa6 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -80,8 +80,8 @@ ProcDRI2QueryVersion(ClientPtr client) rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; - rep.majorVersion = SERVER_DRI2_MAJOR_VERSION; - rep.minorVersion = SERVER_DRI2_MINOR_VERSION; + rep.majorVersion = dri2_major; + rep.minorVersion = dri2_minor; if (client->swapped) { swaps(&rep.sequenceNumber, n); diff --git a/include/protocol-versions.h b/include/protocol-versions.h index c74b7faf0..97ef5dad5 100644 --- a/include/protocol-versions.h +++ b/include/protocol-versions.h @@ -51,10 +51,6 @@ #define SERVER_DMX_MINOR_VERSION 2 #define SERVER_DMX_PATCH_VERSION 20040604 -/* DRI2 */ -#define SERVER_DRI2_MAJOR_VERSION 1 -#define SERVER_DRI2_MINOR_VERSION 2 - /* Generic event extension */ #define SERVER_GE_MAJOR_VERSION 1 #define SERVER_GE_MINOR_VERSION 0 From 0294ff2a5cadddc8fcc77ba9a851f979f0b91fc3 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Mon, 8 Mar 2010 12:39:54 -0800 Subject: [PATCH 09/12] DRI2: throttle swaps at submission time too We need to throttle swaps here in addition to when the context is made current to avoid causing problems with clients that just swap. Throttling here also ensures our swaps get ordered as long as we block the client occasionally. Reported-by: Mario Kleiner Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes --- hw/xfree86/dri2/dri2ext.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 7a9f8caa6..094d54dc0 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -384,6 +384,13 @@ ProcDRI2SwapBuffers(ClientPtr client) DixReadAccess | DixWriteAccess, &pDrawable, &status)) return status; + /* + * Ensures an out of control client can't exhaust our swap queue, and + * also orders swaps. + */ + if (DRI2ThrottleClient(client, pDrawable)) + return client->noClientException; + target_msc = vals_to_card64(stuff->target_msc_lo, stuff->target_msc_hi); divisor = vals_to_card64(stuff->divisor_lo, stuff->divisor_hi); remainder = vals_to_card64(stuff->remainder_lo, stuff->remainder_hi); From b00d435ddf2e9817e33bfd5f7e9b905442dc23c7 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Mon, 8 Mar 2010 12:41:25 -0800 Subject: [PATCH 10/12] DRI2: handle swapsPending better Avoid a potential swapsPending underflow by incrementing it before ScheduleSwap, which may complete it immediately. And be sure to decrement it again in case the schedule failed. Reported-by: Mario Kleiner Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes --- hw/xfree86/dri2/dri2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 3fc7f4ee2..8a671226f 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -654,15 +654,16 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, *swap_target = target_msc; } + pPriv->swapsPending++; ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer, swap_target, divisor, remainder, func, data); if (!ret) { + pPriv->swapsPending--; /* didn't schedule */ xf86DrvMsg(pScreen->myNum, X_ERROR, "[DRI2] %s: driver failed to schedule swap\n", __func__); return BadDrawable; } - pPriv->swapsPending++; pPriv->last_swap_target = *swap_target; /* According to spec, return expected swapbuffers count SBC after this swap From 5933b0abc6a76aaea84aa534df89900cd795c888 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Mon, 8 Mar 2010 15:10:47 -0800 Subject: [PATCH 11/12] DRI2: prevent swap wakes from waking MSC waiters If a few swaps were queued leading to a throttle related block on the client, and then the client submitted an MSC wait, one of the previous swap wakeups could have caused the MSC wait to complete early. Add a flag for this to prevent a swap wake from prematurely waking an MSC waiter. Reported-by: Mario Kleiner Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes --- hw/xfree86/dri2/dri2.c | 37 ++++++++++++++++++++++++++----------- hw/xfree86/dri2/dri2.h | 1 + 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 8a671226f..8b4c36f49 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -63,6 +63,7 @@ typedef struct _DRI2Drawable { int bufferCount; unsigned int swapsPending; ClientPtr blockedClient; + Bool blockedOnMsc; int swap_interval; CARD64 swap_count; int64_t target_sbc; /* -1 means no SBC wait outstanding */ @@ -145,6 +146,7 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->bufferCount = 0; pPriv->swapsPending = 0; pPriv->blockedClient = NULL; + pPriv->blockedOnMsc = FALSE; pPriv->swap_count = 0; pPriv->target_sbc = -1; pPriv->swap_interval = 1; @@ -402,6 +404,15 @@ DRI2ThrottleClient(ClientPtr client, DrawablePtr pDraw) return FALSE; } +static void +__DRI2BlockClient(ClientPtr client, DRI2DrawablePtr pPriv) +{ + if (pPriv->blockedClient == NULL) { + IgnoreClient(client); + pPriv->blockedClient = client; + } +} + void DRI2BlockClient(ClientPtr client, DrawablePtr pDraw) { @@ -411,10 +422,8 @@ DRI2BlockClient(ClientPtr client, DrawablePtr pDraw) if (pPriv == NULL) return; - if (pPriv->blockedClient == NULL) { - IgnoreClient(client); - pPriv->blockedClient = client; - } + __DRI2BlockClient(client, pPriv); + pPriv->blockedOnMsc = TRUE; } int @@ -495,6 +504,7 @@ DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame, AttendClient(pPriv->blockedClient); pPriv->blockedClient = NULL; + pPriv->blockedOnMsc = FALSE; /* If there's still a swap pending, let DRI2SwapComplete free it */ if (pPriv->refCount == 0 && pPriv->swapsPending == 0) @@ -516,8 +526,12 @@ DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame, } /* - * Swap completed. Either wake up an SBC waiter or a client that was - * blocked due to GLX activity during a swap. + * Swap completed. + * Wake the client iff: + * - it was waiting on SBC + * - was blocked due to GLX make current + * - was blocked due to swap throttling + * - is not blocked due to an MSC wait */ if (pPriv->target_sbc != -1 && pPriv->target_sbc <= pPriv->swap_count) { @@ -527,10 +541,11 @@ DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame, AttendClient(pPriv->blockedClient); pPriv->blockedClient = NULL; - } else if (pPriv->target_sbc == -1) { - if (pPriv->blockedClient) + } else if (pPriv->target_sbc == -1 && !pPriv->blockedOnMsc) { + if (pPriv->blockedClient) { AttendClient(pPriv->blockedClient); - pPriv->blockedClient = NULL; + pPriv->blockedClient = NULL; + } } } @@ -582,7 +597,7 @@ DRI2WaitSwap(ClientPtr client, DrawablePtr pDrawable) pPriv->blockedClient == NULL) { ResetCurrentRequest(client); client->sequence--; - DRI2BlockClient(client, pDrawable); + __DRI2BlockClient(client, pPriv); return TRUE; } @@ -780,7 +795,7 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, } pPriv->target_sbc = target_sbc; - DRI2BlockClient(client, pDraw); + __DRI2BlockClient(client, pPriv); return Success; } diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 066cc3947..e1881baa9 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -257,6 +257,7 @@ extern _X_EXPORT Bool DRI2CanFlip(DrawablePtr pDraw); extern _X_EXPORT Bool DRI2CanExchange(DrawablePtr pDraw); +/* Note: use *only* for MSC related waits */ extern _X_EXPORT void DRI2BlockClient(ClientPtr client, DrawablePtr pDraw); extern _X_EXPORT void DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, From 165a4a9c7de0fcc6ef6a6421736b412ccb35965e Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Tue, 23 Mar 2010 09:47:08 -0700 Subject: [PATCH 12/12] GLX/DRI2: expose swap control extensions if DDX support is present Export DDX swap control status from the DRI2 module and check for it in GLX when initializing extensions. Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes --- glx/glxdri2.c | 23 +++++++++-------------- hw/xfree86/dri2/dri2.c | 8 ++++++++ hw/xfree86/dri2/dri2.h | 2 ++ 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/glx/glxdri2.c b/glx/glxdri2.c index edd29b0e1..e791bf6bf 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -616,6 +616,7 @@ glxDRILeaveVT (int index, int flags) static void initializeExtensions(__GLXDRIscreen *screen) { + ScreenPtr pScreen = screen->base.pScreen; const __DRIextension **extensions; int i; @@ -625,10 +626,17 @@ initializeExtensions(__GLXDRIscreen *screen) "GLX_MESA_copy_sub_buffer"); LogMessage(X_INFO, "AIGLX: enabled GLX_MESA_copy_sub_buffer\n"); - /* FIXME: only if DDX supports it */ __glXEnableExtension(screen->glx_enable_bits, "GLX_INTEL_swap_event"); LogMessage(X_INFO, "AIGLX: enabled GLX_INTEL_swap_event\n"); + if (DRI2HasSwapControl(pScreen)) { + __glXEnableExtension(screen->glx_enable_bits, + "GLX_SGI_swap_control"); + __glXEnableExtension(screen->glx_enable_bits, + "GLX_MESA_swap_control"); + LogMessage(X_INFO, "AIGLX: enabled GLX_SGI_swap_control and GLX_MESA_swap_control\n"); + } + for (i = 0; extensions[i]; i++) { #ifdef __DRI_READ_DRAWABLE if (strcmp(extensions[i]->name, __DRI_READ_DRAWABLE) == 0) { @@ -639,19 +647,6 @@ initializeExtensions(__GLXDRIscreen *screen) } #endif -#ifdef __DRI_SWAP_CONTROL - if (strcmp(extensions[i]->name, __DRI_SWAP_CONTROL) == 0) { - screen->swapControl = - (const __DRIswapControlExtension *) extensions[i]; - __glXEnableExtension(screen->glx_enable_bits, - "GLX_SGI_swap_control"); - __glXEnableExtension(screen->glx_enable_bits, - "GLX_MESA_swap_control"); - - LogMessage(X_INFO, "AIGLX: enabled GLX_SGI_swap_control and GLX_MESA_swap_control\n"); - } -#endif - #ifdef __DRI_TEX_BUFFER if (strcmp(extensions[i]->name, __DRI_TEX_BUFFER) == 0) { screen->texBuffer = diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 8b4c36f49..2bdb73392 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -830,6 +830,14 @@ DRI2DestroyDrawable(DrawablePtr pDraw) DRI2FreeDrawable(pDraw); } +Bool +DRI2HasSwapControl(ScreenPtr pScreen) +{ + DRI2ScreenPtr ds = DRI2GetScreen(pScreen); + + return (ds->ScheduleSwap && ds->GetMSC); +} + Bool DRI2Connect(ScreenPtr pScreen, unsigned int driverType, int *fd, const char **driverName, const char **deviceName) diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index e1881baa9..ce8a5df41 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -188,6 +188,8 @@ extern _X_EXPORT Bool DRI2ScreenInit(ScreenPtr pScreen, extern _X_EXPORT void DRI2CloseScreen(ScreenPtr pScreen); +extern _X_EXPORT Bool DRI2HasSwapControl(ScreenPtr pScreen); + extern _X_EXPORT Bool DRI2Connect(ScreenPtr pScreen, unsigned int driverType, int *fd,