[PATCH 6/9] DRI2: Reference count buffers across SwapBuffers
Pauli
ext-pauli.nieminen at nokia.com
Thu Feb 3 09:48:50 PST 2011
From: Christopher James Halse Rogers <christopher.halse.rogers at canonical.com>
The SwapBuffers request requires that we trigger the swap at some point
in the future. Sane drivers implement this by passing this request to
something that will trigger a callback with the buffer pointers
at the appropriate time.
The client can cause those buffers to be freed in X before the trigger
occurs, most easily by quitting. This leads to a server crash in
the driver when trying to do the swap.
See http://bugs.freedesktop.org/show_bug.cgi?id=29065 for Radeon and
https://bugs.freedesktop.org/show_bug.cgi?id=28080 for Intel.
Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers at canonical.com>
Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
---
hw/xfree86/dri2/dri2.c | 80 +++++++++++++++++++++++++++++++++++++++---------
hw/xfree86/dri2/dri2.h | 1 +
2 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 7746a1d..b50206e 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -108,12 +108,35 @@ typedef struct _DRI2Screen {
ConfigNotifyProcPtr ConfigNotify;
} DRI2ScreenRec;
+typedef struct _DRI2SwapCompleteDataRec {
+ DRI2BufferPtr src;
+ DRI2BufferPtr dest;
+ void * data;
+} DRI2SwapCompleteDataRec, *DRI2SwapCompleteDataPtr;
+
static DRI2ScreenPtr
DRI2GetScreen(ScreenPtr pScreen)
{
return dixLookupPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey);
}
+static void
+buffer_ref(DRI2BufferPtr buffer)
+{
+ buffer->refcnt++;
+}
+
+static void
+buffer_unref(DRI2DrawablePtr pPriv, DRI2BufferPtr buffer)
+{
+ DRI2ScreenPtr ds = pPriv->dri2_screen;
+
+ buffer->refcnt--;
+ if (buffer->refcnt == 0) {
+ (*ds->DestroyBuffer)(pPriv, buffer);
+ }
+}
+
DRI2DrawablePtr
DRI2GetDrawable(DrawablePtr pDraw)
{
@@ -325,7 +348,6 @@ DRI2DestroyDrawable(ClientPtr client, DRI2DrawablePtr pPriv, XID id)
static int DRI2DrawableGone(pointer p, XID id)
{
DRI2DrawablePtr pPriv = p;
- DRI2ScreenPtr ds = pPriv->dri2_screen;
DRI2DrawableRefPtr ref, next;
DrawablePtr pDraw = pPriv->drawable;
int i;
@@ -355,7 +377,7 @@ static int DRI2DrawableGone(pointer p, XID id)
if (pPriv->buffers != NULL) {
for (i = 0; i < pPriv->bufferCount; i++)
- (*ds->DestroyBuffer)(pPriv, pPriv->buffers[i]);
+ buffer_unref(pPriv, pPriv->buffers[i]);
free(pPriv->buffers);
}
@@ -405,6 +427,7 @@ allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds,
|| !dimensions_match
|| (pPriv->buffers[old_buf]->format != format)) {
*buffer = (*ds->CreateBuffer)(pDraw, attachment, format);
+ buffer_ref(*buffer);
pPriv->serialNumber = DRI2DrawableSerial(pDraw);
return TRUE;
@@ -419,14 +442,12 @@ static void
update_dri2_drawable_buffers(DRI2DrawablePtr pPriv, DrawablePtr pDraw,
DRI2BufferPtr *buffers, int *out_count, int *width, int *height)
{
- DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen);
int i;
if (pPriv->buffers != NULL) {
for (i = 0; i < pPriv->bufferCount; i++) {
- if (pPriv->buffers[i] != NULL) {
- (*ds->DestroyBuffer)(pPriv, pPriv->buffers[i]);
- }
+ if (pPriv->buffers[i] != NULL)
+ buffer_unref(pPriv, pPriv->buffers[i]);
}
free(pPriv->buffers);
@@ -562,7 +583,7 @@ err_out:
for (i = 0; i < count; i++) {
if (buffers[i] != NULL)
- (*ds->DestroyBuffer)(pPriv, buffers[i]);
+ buffer_unref(pPriv, buffers[i]);
}
free(buffers);
@@ -771,11 +792,20 @@ DRI2WakeClient(ClientPtr client, DRI2DrawablePtr pPriv, int frame,
}
}
+static void
+free_swap_complete_data (DRI2DrawablePtr pPriv, DRI2SwapCompleteDataPtr pSwapData)
+{
+ buffer_unref(pPriv, pSwapData->src);
+ buffer_unref(pPriv, pSwapData->dest);
+ free(pSwapData);
+}
+
void
DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame,
unsigned int tv_sec, unsigned int tv_usec, int type,
DRI2SwapEventPtr swap_complete, void *swap_data)
{
+ DRI2SwapCompleteDataPtr pSwapData = swap_data;
DrawablePtr pDraw = pPriv->drawable;
CARD64 ust = 0;
@@ -783,11 +813,6 @@ DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame,
pPriv->swap_count++;
pPriv->refcnt--;
- if (pPriv->refcnt == 0) {
- DRI2DrawableGone(pPriv, 0);
- return;
- }
-
if (pDraw) {
BoxRec box;
RegionRec region;
@@ -803,12 +828,18 @@ DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame,
ust = ((CARD64)tv_sec * 1000000) + tv_usec;
if (swap_complete)
- swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count);
+ swap_complete(client, pSwapData->data, type, ust, frame,
+ pPriv->swap_count);
pPriv->last_swap_msc = frame;
pPriv->last_swap_ust = ust;
DRI2WakeClient(client, pPriv, frame, tv_sec, tv_usec);
+
+ free_swap_complete_data(pPriv, pSwapData);
+
+ if (pPriv->refcnt == 0)
+ DRI2DrawableGone(pPriv, 0);
}
Bool
@@ -837,6 +868,7 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
ScreenPtr pScreen = pPriv->dri2_screen->screen;
DRI2ScreenPtr ds = pPriv->dri2_screen;
DRI2BufferPtr pDestBuffer = NULL, pSrcBuffer = NULL;
+ DRI2SwapCompleteDataPtr pSwapData;
int ret, i;
CARD64 ust, current_msc;
@@ -858,6 +890,23 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
return BadDrawable;
}
+ pSwapData = malloc(sizeof *pSwapData);
+ if (pSwapData == NULL)
+ return BadAlloc;
+
+ /*
+ * Take a reference to the buffers we're exchanging.
+ * This ensures that these buffers will remain valid until the swap
+ * is completed.
+ *
+ * DRI2SwapComplete is required to unref these buffers.
+ */
+ buffer_ref(pSrcBuffer);
+ buffer_ref(pDestBuffer);
+ pSwapData->src = pSrcBuffer;
+ pSwapData->dest = pDestBuffer;
+ pSwapData->data = data;
+
/* Old DDX or no swap interval, just blit */
if (!ds->ScheduleSwap || !pPriv->swap_interval) {
BoxRec box;
@@ -874,7 +923,7 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
(*ds->CopyRegion)(pDraw, ®ion, pDestBuffer, pSrcBuffer);
DRI2SwapComplete(client, pPriv, target_msc, 0, 0, DRI2_BLIT_COMPLETE,
- func, data);
+ func, pSwapData);
return Success;
}
@@ -914,12 +963,13 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
pPriv->swapsPending++;
pPriv->refcnt++;
ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer,
- swap_target, divisor, remainder, func, data);
+ swap_target, divisor, remainder, func, pSwapData);
if (!ret) {
pPriv->swapsPending--; /* didn't schedule */
pPriv->refcnt--;
xf86DrvMsg(pScreen->myNum, X_ERROR,
"[DRI2] %s: driver failed to schedule swap\n", __func__);
+ free_swap_complete_data(pPriv, pSwapData);
return BadDrawable;
}
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index c324b8f..6a5dd98 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -44,6 +44,7 @@ typedef struct {
unsigned int flags;
unsigned int format;
void *driverPrivate;
+ unsigned int refcnt;
} DRI2BufferRec, *DRI2BufferPtr;
struct _DRI2Drawable;
--
1.7.0.4
More information about the xorg-devel
mailing list