[PATCH] radeon: proper DRI2 pending events handling when client gone. (v5)

Oldřich Jedlička oldium.pro at seznam.cz
Sun Oct 3 01:25:28 PDT 2010


Properly handle asynchronous DRI2 events for disconnected clients.
Track client's pending requests and mark them as invalid when the
client disconnects.

This is based on the version from Alban Browaeys in bug #29065.

v1 (Alban Browaeys): Based upon a detailed explanation from Oldřich
Jedlička and comments from Christopher James Halse Rogers.
on http://lists.x.org/archives/xorg-driver-ati/2010-August/016780.html .

v2: Updated version to apply on master. Removed unnecessary
client_index field from _DRI2FrameEvent. Added freeing/removing from
list to failed paths of radeon_dri2_schedule_wait_msc and
radeon_dri2_schedule_swap.

v3: Adopt to older xorg-server that doesn't have dixRegisterPrivateKey.

v4: Conditional include of list.h, unreachable return removed.

v5: Distribute list.h as xorg_list.h, remove xorg-server version check.
Use the version from xorg-server when available (checked in
configure.ac).
---
 configure.ac      |    5 ++
 src/Makefile.am   |    3 +-
 src/radeon_dri2.c |  157 +++++++++++++++++++++++++++++++++++++++++++++--------
 src/xorg_list.h   |  103 +++++++++++++++++++++++++++++++++++
 4 files changed, 243 insertions(+), 25 deletions(-)
 create mode 100644 src/xorg_list.h

diff --git a/configure.ac b/configure.ac
index decc46f..5dbf65a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -233,6 +233,11 @@ AC_CHECK_DECL(XSERVER_LIBPCIACCESS,
 	      [XSERVER_LIBPCIACCESS=yes],[XSERVER_LIBPCIACCESS=no],
 	      [#include "xorg-server.h"])
 
+AC_CHECK_HEADERS([list.h],
+		 [], [],
+		 [#include <X11/Xdefs.h>
+		  #include "xorg-server.h"])
+
 CPPFLAGS="$SAVE_CPPFLAGS"
 
 AM_CONDITIONAL(USE_EXA, test "x$USE_EXA" = xyes)
diff --git a/src/Makefile.am b/src/Makefile.am
index 033047e..696bf71 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -165,4 +165,5 @@ EXTRA_DIST = \
 	pcidb/parse_pci_ids.pl \
 	radeon_atombios.h \
 	radeon_dri2.h \
-	drmmode_display.h
+	drmmode_display.h \
+	xorg_list.h
diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
index ed7fdd6..014dfd7 100644
--- a/src/radeon_dri2.c
+++ b/src/radeon_dri2.c
@@ -38,6 +38,12 @@
 #include "radeon_dri2.h"
 #include "radeon_version.h"
 
+#if HAVE_LIST_H
+#include "list.h"
+#else
+#include "xorg_list.h"
+#endif
+
 #ifdef RADEON_DRI2
 
 #include "radeon_bo_gem.h"
@@ -376,7 +382,6 @@ enum DRI2FrameEventType {
 typedef struct _DRI2FrameEvent {
     XID drawable_id;
     ClientPtr client;
-    int client_index;
     enum DRI2FrameEventType type;
     int frame;
 
@@ -385,8 +390,86 @@ typedef struct _DRI2FrameEvent {
     void *event_data;
     DRI2BufferPtr front;
     DRI2BufferPtr back;
+
+    Bool valid;
+
+    struct list link;
 } DRI2FrameEventRec, *DRI2FrameEventPtr;
 
+typedef struct _DRI2ClientEvents {
+    struct list   reference_list;
+} DRI2ClientEventsRec, *DRI2ClientEventsPtr;
+
+#if HAS_DEVPRIVATEKEYREC
+
+static DevPrivateKeyRec DRI2ClientEventsPrivateKeyRec;
+#define DRI2ClientEventsPrivateKey (&DRI2ClientEventsPrivateKeyRec)
+
+#else
+
+static int DRI2ClientEventsPrivateKeyIndex;
+DevPrivateKey DRI2ClientEventsPrivateKey = &DRI2ClientEventsPrivateKeyIndex;
+
+#endif /* HAS_DEVPRIVATEKEYREC */
+
+#define GetDRI2ClientEvents(pClient)	((DRI2ClientEventsPtr) \
+    dixLookupPrivate(&(pClient)->devPrivates, DRI2ClientEventsPrivateKey))
+
+static int
+ListAddDRI2ClientEvents(ClientPtr client, struct list *entry)
+{
+    DRI2ClientEventsPtr pClientPriv;
+    pClientPriv = GetDRI2ClientEvents(client);
+
+    if (!pClientPriv) {
+        return BadAlloc;
+    }
+
+    list_add(entry, &pClientPriv->reference_list);
+    return 0;
+}
+
+static void
+ListDelDRI2ClientEvents(ClientPtr client, struct list *entry)
+{
+    DRI2ClientEventsPtr pClientPriv;
+    pClientPriv = GetDRI2ClientEvents(client);
+
+    if (!pClientPriv) {
+        return;
+    }
+    list_del(entry);
+}
+
+static void
+radeon_dri2_client_state_changed(CallbackListPtr *ClientStateCallback, pointer data, pointer calldata)
+{
+    DRI2ClientEventsPtr pClientEventsPriv;
+    DRI2FrameEventPtr ref;
+    NewClientInfoRec *clientinfo = calldata;
+    ClientPtr pClient = clientinfo->client;
+    pClientEventsPriv = GetDRI2ClientEvents(pClient);
+
+    switch (pClient->clientState) {
+    case ClientStateInitial:
+        list_init(&pClientEventsPriv->reference_list);
+        break;
+    case ClientStateRunning:
+        break;
+
+    case ClientStateRetained:
+    case ClientStateGone:
+        if (pClientEventsPriv) {
+            list_for_each_entry(ref, &pClientEventsPriv->reference_list, link) {
+                ref->valid = FALSE;
+            }
+        }
+        break;
+    default:
+        break;
+    }
+}
+
 static void
 radeon_dri2_ref_buffer(BufferPtr buffer)
 {
@@ -416,28 +499,17 @@ void radeon_dri2_frame_event_handler(unsigned int frame, unsigned int tv_sec,
     BoxRec box;
     RegionRec region;
 
+    if (!event->valid)
+        goto cleanup;
+
     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;
-    }
+    if (status != Success)
+        goto cleanup;
 
     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;
+    client = event->client;
 
     switch (event->type) {
     case DRI2_FLIP:
@@ -453,8 +525,6 @@ void radeon_dri2_frame_event_handler(unsigned int frame, unsigned int tv_sec,
         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(client, drawable, frame, tv_sec, tv_usec);
@@ -466,6 +536,11 @@ void radeon_dri2_frame_event_handler(unsigned int frame, unsigned int tv_sec,
         break;
     }
 
+cleanup:
+    radeon_dri2_unref_buffer(event->front);
+    radeon_dri2_unref_buffer(event->back);
+    if (event->valid)
+        ListDelDRI2ClientEvents(event->client, &event->link);
     free(event);
 }
 
@@ -539,7 +614,7 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
     ScreenPtr screen = draw->pScreen;
     ScrnInfoPtr scrn = xf86Screens[screen->myNum];
     RADEONInfoPtr info = RADEONPTR(scrn);
-    DRI2FrameEventPtr wait_info;
+    DRI2FrameEventPtr wait_info = NULL;
     drmVBlank vbl;
     int ret, crtc = radeon_dri2_drawable_crtc(draw);
     CARD64 current_msc;
@@ -560,8 +635,14 @@ 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;
+    wait_info->valid = TRUE;
+
+    if (ListAddDRI2ClientEvents(client, &wait_info->link)) {
+        xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+                "add events to client private failed.\n");
+        goto out_complete;
+    }
 
     /* Get current count */
     vbl.request.type = DRM_VBLANK_RELATIVE;
@@ -642,6 +723,10 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
     return TRUE;
 
 out_complete:
+    if (wait_info) {
+        ListDelDRI2ClientEvents(wait_info->client, &wait_info->link);
+        free(wait_info);
+    }
     DRI2WaitMSCComplete(client, draw, target_msc, 0, 0);
     return TRUE;
 }
@@ -704,11 +789,16 @@ 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;
+    swap_info->valid = TRUE;
+    if (ListAddDRI2ClientEvents(client, &swap_info->link)) {
+        xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+                "add events to client private failed.\n");
+        goto blit_fallback;
+    }
 
     /* Get current count */
     vbl.request.type = DRM_VBLANK_RELATIVE;
@@ -831,8 +921,10 @@ blit_fallback:
     radeon_dri2_copy_region(draw, &region, front, back);
 
     DRI2SwapComplete(client, draw, 0, 0, 0, DRI2_BLIT_COMPLETE, func, data);
-    if (swap_info)
+    if (swap_info) {
+        ListDelDRI2ClientEvents(swap_info->client, &swap_info->link);
         free(swap_info);
+    }
 
     radeon_dri2_unref_buffer(front);
     radeon_dri2_unref_buffer(back);
@@ -895,6 +987,20 @@ radeon_dri2_screen_init(ScreenPtr pScreen)
     } else {
         xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "You need a newer kernel for sync extension\n");
     }
+
+#if HAS_DIXREGISTERPRIVATEKEY
+    if (!dixRegisterPrivateKey(DRI2ClientEventsPrivateKey, PRIVATE_CLIENT, sizeof(DRI2ClientEventsRec))) {
+        xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "DRI2 registering private key to client failed\n");
+        return FALSE;
+    }
+#else
+    if (!dixRequestPrivate(DRI2ClientEventsPrivateKey, sizeof(DRI2ClientEventsRec))) {
+        xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "DRI2 requesting private key to client failed\n");
+        return FALSE;
+    }
+#endif
+
+    AddCallback(&ClientStateCallback, radeon_dri2_client_state_changed, 0);
 #endif
 
     info->dri2.enabled = DRI2ScreenInit(pScreen, &dri2_info);
@@ -906,6 +1012,9 @@ void radeon_dri2_close_screen(ScreenPtr pScreen)
     ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
     RADEONInfoPtr info = RADEONPTR(pScrn);
 
+#if DRI2INFOREC_VERSION >= 4
+    DeleteCallback(&ClientStateCallback, radeon_dri2_client_state_changed, 0);
+#endif
     DRI2CloseScreen(pScreen);
     drmFree(info->dri2.device_name);
 }
diff --git a/src/xorg_list.h b/src/xorg_list.h
new file mode 100644
index 0000000..4ce20a8
--- /dev/null
+++ b/src/xorg_list.h
@@ -0,0 +1,103 @@
+/*
+ * Copyright © 2010 Intel Corporation
+ * Copyright © 2010 Francisco Jerez <currojerez at riseup.net>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _LIST_H_
+#define _LIST_H_
+
+/* classic doubly-link circular list */
+struct list {
+    struct list *next, *prev;
+};
+
+static void
+list_init(struct list *list)
+{
+    list->next = list->prev = list;
+}
+
+static inline void
+__list_add(struct list *entry,
+	    struct list *prev,
+	    struct list *next)
+{
+    next->prev = entry;
+    entry->next = next;
+    entry->prev = prev;
+    prev->next = entry;
+}
+
+static inline void
+list_add(struct list *entry, struct list *head)
+{
+    __list_add(entry, head, head->next);
+}
+
+static inline void
+__list_del(struct list *prev, struct list *next)
+{
+    next->prev = prev;
+    prev->next = next;
+}
+
+static inline void
+list_del(struct list *entry)
+{
+    __list_del(entry->prev, entry->next);
+    list_init(entry);
+}
+
+static inline Bool
+list_is_empty(struct list *head)
+{
+    return head->next == head;
+}
+
+#ifndef container_of
+#define container_of(ptr, type, member) \
+    (type *)((char *)(ptr) - (char *) &((type *)0)->member)
+#endif
+
+#define list_entry(ptr, type, member) \
+    container_of(ptr, type, member)
+
+#define list_first_entry(ptr, type, member) \
+    list_entry((ptr)->next, type, member)
+
+#define __container_of(ptr, sample, member)				\
+    (void *)((char *)(ptr)						\
+	     - ((char *)&(sample)->member - (char *)(sample)))
+
+#define list_for_each_entry(pos, head, member)				\
+    for (pos = __container_of((head)->next, pos, member);		\
+	 &pos->member != (head);					\
+	 pos = __container_of(pos->member.next, pos, member))
+
+#define list_for_each_entry_safe(pos, tmp, head, member)		\
+    for (pos = __container_of((head)->next, pos, member),		\
+	 tmp = __container_of(pos->member.next, pos, member);		\
+	 &pos->member != (head);					\
+	 pos = tmp, tmp = __container_of(pos->member.next, tmp, member))
+
+#endif
-- 
1.7.3



More information about the xorg-driver-ati mailing list