[PATCH] dri2: Reference count DRI2 buffers
Christopher James Halse Rogers
christopher.halse.rogers at canonical.com
Thu Aug 19 01:57:21 PDT 2010
When a client calls ScheduleSwap we set up a kernel callback when the
relevent vblank event occurs. However, it's possible for the client
to go away between calling ScheduleSwap and the vblank event,
resulting in the buffers being destroyed before they're passed to
radeon_dri2_frame_event_handler.
Add reference-counting to the buffers and take a reference in
radeon_dri2_schedule_swap to ensure the buffers won't be destroyed
before the vblank event is dealt with.
Fixes: http://bugs.freedesktop.org/show_bug.cgi?id=29065
v2: Don't write completion events to the client if it has quit.
Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers at canonical.com>
---
src/radeon_dri2.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 66 insertions(+), 7 deletions(-)
diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
index 4cafbc6..c6636c6 100644
--- a/src/radeon_dri2.c
+++ b/src/radeon_dri2.c
@@ -55,6 +55,7 @@ typedef DRI2Buffer2Ptr BufferPtr;
struct dri2_buffer_priv {
PixmapPtr pixmap;
unsigned int attachment;
+ unsigned int refcnt;
};
@@ -244,6 +245,7 @@ radeon_dri2_create_buffer(DrawablePtr drawable,
buffers->flags = 0; /* not tiled */
privates->pixmap = pixmap;
privates->attachment = attachment;
+ privates->refcnt = 1;
return buffers;
}
@@ -275,13 +277,26 @@ radeon_dri2_destroy_buffer(DrawablePtr drawable, BufferPtr buffers)
if(buffers)
{
ScreenPtr pScreen = drawable->pScreen;
- struct dri2_buffer_priv *private;
+ struct dri2_buffer_priv *private = buffers->driverPrivate;
- private = buffers->driverPrivate;
- (*pScreen->DestroyPixmap)(private->pixmap);
+ /* Trying to free an already freed buffer is unlikely to end well */
+ if (private->refcnt == 0) {
+ ScrnInfoPtr scrn = xf86Screens[pScreen->myNum];
- free(buffers->driverPrivate);
- free(buffers);
+ xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+ "Attempted to destroy previously destroyed buffer.\
+ This is a programming error\n");
+ return;
+ }
+
+ private->refcnt--;
+ if (private->refcnt == 0)
+ {
+ (*pScreen->DestroyPixmap)(private->pixmap);
+
+ free(buffers->driverPrivate);
+ free(buffers);
+ }
}
}
#endif
@@ -362,6 +377,7 @@ enum DRI2FrameEventType {
typedef struct _DRI2FrameEvent {
XID drawable_id;
ClientPtr client;
+ int client_index;
enum DRI2FrameEventType type;
int frame;
@@ -372,11 +388,26 @@ typedef struct _DRI2FrameEvent {
DRI2BufferPtr back;
} DRI2FrameEventRec, *DRI2FrameEventPtr;
+static void
+radeon_dri2_ref_buffer(BufferPtr buffer)
+{
+ struct dri2_buffer_priv *private = buffer->driverPrivate;
+ private->refcnt++;
+}
+
+static void
+radeon_dri2_unref_buffer(BufferPtr buffer)
+{
+ struct dri2_buffer_priv *private = buffer->driverPrivate;
+ radeon_dri2_destroy_buffer(&(private->pixmap->drawable), buffer);
+}
+
void radeon_dri2_frame_event_handler(unsigned int frame, unsigned int tv_sec,
unsigned int tv_usec, void *event_data)
{
DRI2FrameEventPtr event = event_data;
DrawablePtr drawable;
+ ClientPtr client;
ScreenPtr screen;
ScrnInfoPtr scrn;
int status;
@@ -387,6 +418,8 @@ void radeon_dri2_frame_event_handler(unsigned int frame, unsigned int tv_sec,
status = dixLookupDrawable(&drawable, event->drawable_id, serverClient,
M_ANY, DixWriteAccess);
if (status != Success) {
+ radeon_dri2_unref_buffer(event->front);
+ radeon_dri2_unref_buffer(event->back);
free(event);
return;
}
@@ -394,6 +427,17 @@ void radeon_dri2_frame_event_handler(unsigned int frame, unsigned int tv_sec,
screen = drawable->pScreen;
scrn = xf86Screens[screen->myNum];
+ /* event->client may have quit between submitting a SwapBuffers request
+ * and this callback being triggered.
+ *
+ * Check our saved client pointer against the client in the saved client
+ * slot. This will catch almost all cases where the client that requested
+ * SwapBuffers has gone away, and will guarantee that there is at least a
+ * valid client to write the BufferSwapComplete event to.
+ */
+ client = event->client == clients[event->client_index] ?
+ event->client : NULL;
+
switch (event->type) {
case DRI2_FLIP:
case DRI2_SWAP:
@@ -405,11 +449,11 @@ void radeon_dri2_frame_event_handler(unsigned int frame, unsigned int tv_sec,
radeon_dri2_copy_region(drawable, ®ion, event->front, event->back);
swap_type = DRI2_BLIT_COMPLETE;
- DRI2SwapComplete(event->client, drawable, frame, tv_sec, tv_usec,
+ DRI2SwapComplete(client, drawable, frame, tv_sec, tv_usec,
swap_type, event->event_complete, event->event_data);
break;
case DRI2_WAITMSC:
- DRI2WaitMSCComplete(event->client, drawable, frame, tv_sec, tv_usec);
+ DRI2WaitMSCComplete(client, drawable, frame, tv_sec, tv_usec);
break;
default:
/* Unknown type */
@@ -418,6 +462,8 @@ void radeon_dri2_frame_event_handler(unsigned int frame, unsigned int tv_sec,
break;
}
+ radeon_dri2_unref_buffer(event->front);
+ radeon_dri2_unref_buffer(event->back);
free(event);
}
@@ -512,6 +558,7 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
wait_info->drawable_id = draw->id;
wait_info->client = client;
+ wait_info->client_index = client->index;
wait_info->type = DRI2_WAITMSC;
/* Get current count */
@@ -648,11 +695,19 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
swap_info->drawable_id = draw->id;
swap_info->client = client;
+ swap_info->client_index = client->index;
swap_info->event_complete = func;
swap_info->event_data = data;
swap_info->front = front;
swap_info->back = back;
+ /* radeon_dri2_frame_event_handler will get called some unknown time in the
+ * future with these buffers. Take a reference to ensure that they won't
+ * get destroyed before then.
+ */
+ radeon_dri2_ref_buffer(front);
+ radeon_dri2_ref_buffer(back);
+
/* Get current count */
vbl.request.type = DRM_VBLANK_RELATIVE;
if (crtc > 0)
@@ -776,6 +831,10 @@ blit_fallback:
DRI2SwapComplete(client, draw, 0, 0, 0, DRI2_BLIT_COMPLETE, func, data);
if (swap_info)
free(swap_info);
+
+ radeon_dri2_unref_buffer(front);
+ radeon_dri2_unref_buffer(back);
+
*target_msc = 0; /* offscreen, so zero out target vblank count */
return TRUE;
}
--
1.7.1
More information about the xorg-driver-ati
mailing list