From 6fed073ef84a7dee5af88746973fb7150f412c7b Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Aug 27 2010 04:43:01 +0000 Subject: - fix dri2 pixmap refcounting bug --- 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 +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 +Signed-off-by: Dave Airlie +--- + 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 6.13.1-0.4.20100705git37b348059 +- fix dri2 pixmap refcounting bug + * Wed Aug 25 2010 Dave Airlie 6.13.1-0.3.20100705git37b348059 - fix flushing in dri2 for direct rendered comp managers.