[PATCH v2 06/10] DRI2: Reference count buffers across SwapBuffers
Pauli
ext-pauli.nieminen at nokia.com
Tue Feb 8 13:42:52 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 | 68 ++++++++++++++++++++++++++++++++++++------------
hw/xfree86/dri2/dri2.h | 1 +
2 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 1693546..5868fb4 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -109,8 +109,10 @@ typedef struct _DRI2Screen {
} DRI2ScreenRec;
typedef struct _DRI2SwapCompleteDataRec {
- ClientPtr client;
- void *data;
+ DRI2BufferPtr src;
+ DRI2BufferPtr dest;
+ ClientPtr client;
+ void * data;
} DRI2SwapCompleteDataRec, *DRI2SwapCompleteDataPtr;
static DRI2ScreenPtr
@@ -119,6 +121,23 @@ 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)
{
@@ -330,7 +349,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;
@@ -376,7 +394,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);
}
@@ -417,6 +435,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;
@@ -431,14 +450,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);
@@ -574,7 +591,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);
@@ -786,6 +803,14 @@ 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
DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame,
unsigned int tv_sec, unsigned int tv_usec, int type,
@@ -800,11 +825,6 @@ DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame,
pPriv->swap_count++;
pPriv->refcnt--;
- if (pPriv->refcnt <= 0) {
- DRI2DrawableGone(pPriv, 0);
- return;
- }
-
if (pDraw) {
BoxRec box;
RegionRec region;
@@ -828,7 +848,10 @@ DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame,
DRI2WakeClient(client, pPriv, frame, tv_sec, tv_usec);
- free(pSwapData);
+ free_swap_complete_data(pPriv, pSwapData);
+
+ if (pPriv->refcnt <= 0)
+ DRI2DrawableGone(pPriv, 0);
}
Bool
@@ -883,6 +906,17 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
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->client = client;
pSwapData->data = data;
@@ -942,11 +976,11 @@ 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--;
- free(pSwapData);
+ free_swap_complete_data(pPriv, pSwapData);
xf86DrvMsg(pScreen->myNum, X_ERROR,
"[DRI2] %s: driver failed to schedule swap\n", __func__);
return BadDrawable;
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index 4616dc5..c062a43 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