[xorg-x11-drv-ati] - fix dri2 pixmap refcounting bug
Dave Airlie
airlied at fedoraproject.org
Fri Aug 27 04:43:16 UTC 2010
commit 6fed073ef84a7dee5af88746973fb7150f412c7b
Author: Dave Airlie <airlied at redhat.com>
Date: Fri Aug 27 14:43:01 2010 +1000
- fix dri2 pixmap refcounting bug
radeon-dri2-fix.patch | 207 +++++++++++++++++++++++++++++++++++++++++++++++++
xorg-x11-drv-ati.spec | 7 ++-
2 files changed, 213 insertions(+), 1 deletions(-)
---
diff --git a/radeon-dri2-fix.patch b/radeon-dri2-fix.patch
new file mode 100644
index 0000000..3555251
--- /dev/null
+++ b/radeon-dri2-fix.patch
@@ -0,0 +1,207 @@
+From 6a2c8587a4e05a8be2a2e975a6660942cfe115d6 Mon Sep 17 00:00:00 2001
+From: Christopher James Halse Rogers <christopher.halse.rogers at canonical.com>
+Date: Fri, 27 Aug 2010 13:14:33 +1000
+Subject: [PATCH] dri2: Reference count DRI2 buffers
+
+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.
+
+This parallels the approach taken by the Intel DDX in commit
+0d2392d44aae95d6b571d98f7ec323cf672a687f.
+
+Fixes: http://bugs.freedesktop.org/show_bug.cgi?id=29065
+
+v2: Don't write completion events to the client if it has quit.
+v3: Don't try to unref the NULL buffers from a DRI2_WAITMSC event.
+ Take a ref in schedule_swap earlier, so the offscreen fallback
+ doesn't incorrectly destroy the buffers.
+
+Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers at canonical.com>
+Signed-off-by: Dave Airlie <airlied at redhat.com>
+---
+ src/radeon_dri2.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++-----
+ 1 files changed, 69 insertions(+), 7 deletions(-)
+
+diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
+index 4ded9dc..ed7fdd6 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
+@@ -361,6 +376,7 @@ enum DRI2FrameEventType {
+ typedef struct _DRI2FrameEvent {
+ XID drawable_id;
+ ClientPtr client;
++ int client_index;
+ enum DRI2FrameEventType type;
+ int frame;
+
+@@ -371,11 +387,28 @@ 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)
++{
++ if (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;
+@@ -386,6 +419,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;
+ }
+@@ -393,6 +428,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 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:
+@@ -404,11 +450,14 @@ 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);
++
++ radeon_dri2_unref_buffer(event->front);
++ radeon_dri2_unref_buffer(event->back);
+ 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 */
+@@ -511,6 +560,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 */
+@@ -641,12 +691,20 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
+
+ swap_info = calloc(1, sizeof(DRI2FrameEventRec));
+
++ /* 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);
++
+ /* Drawable not displayed... just complete the swap */
+ if (crtc == -1 || !swap_info)
+ goto blit_fallback;
+
+ 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;
+@@ -775,6 +833,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
+
diff --git a/xorg-x11-drv-ati.spec b/xorg-x11-drv-ati.spec
index 09a0bce..ae69112 100644
--- a/xorg-x11-drv-ati.spec
+++ b/xorg-x11-drv-ati.spec
@@ -7,7 +7,7 @@
Summary: Xorg X11 ati video driver
Name: xorg-x11-drv-ati
Version: 6.13.1
-Release: 0.3.%{gitdate}git%{gitversion}%{?dist}
+Release: 0.4.%{gitdate}git%{gitversion}%{?dist}
URL: http://www.x.org
License: MIT
Group: User Interface/X Hardware Support
@@ -22,6 +22,7 @@ Patch6: radeon-6.9.0-bgnr-enable.patch
Patch10: radeon-6.12.2-lvds-default-modes.patch
Patch13: fix-default-modes.patch
Patch14: radeon-flush.patch
+Patch15: radeon-dri2-fix.patch
ExcludeArch: s390 s390x
@@ -50,6 +51,7 @@ X.Org X11 ati video driver.
%patch10 -p1 -b .lvds
%patch13 -p1 -b .def
%patch14 -p1 -b .flush
+%patch15 -p1 -b .dri2
%build
autoreconf -iv
@@ -83,6 +85,9 @@ rm -rf $RPM_BUILD_ROOT
%{_mandir}/man4/radeon.4*
%changelog
+* Fri Aug 27 2010 Dave Airlie <airlied at redhat.com> 6.13.1-0.4.20100705git37b348059
+- fix dri2 pixmap refcounting bug
+
* Wed Aug 25 2010 Dave Airlie <airlied at redhat.com> 6.13.1-0.3.20100705git37b348059
- fix flushing in dri2 for direct rendered comp managers.
More information about the scm-commits
mailing list