Attachment 39258 Details for Bug 29065 – the option 2: updated fix sent to xorg-driver-ati mailing list (version 6)

[patch] the option 2: updated fix sent to xorg-driver-ati mailing list (version 6)

0001-radeon-proper-DRI2-pending-events-handling-when-clie.patch (text/plain), 11.21 KB, created by Oldrich Jedlicka on 2010-10-07 09:17:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Oldrich Jedlicka

Created: 2010-10-07 09:17:25 UTC

Size: 11.21 KB

patch

obsolete

>From 636133305abb7add62da456e37cefec68ccac3a2 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Old=C5=99ich=20Jedli=C4=8Dka?= <oldium.pro@seznam.cz>
>Date: Mon, 6 Sep 2010 21:14:52 +0200
>Subject: [PATCH] radeon: proper DRI2 pending events handling when client gone. (v6)
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>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).
>
>v6: Removed xorg_list.h, made DRI2 scheduling features dependent on
>list.h presence.
>---
> configure.ac      |    5 ++
> src/radeon_dri2.c |  167 ++++++++++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 144 insertions(+), 28 deletions(-)
>
>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/radeon_dri2.c b/src/radeon_dri2.c
>index ed7fdd6..0750e3d 100644
>--- a/src/radeon_dri2.c
>+++ b/src/radeon_dri2.c
>@@ -38,6 +38,10 @@
> #include "radeon_dri2.h"
> #include "radeon_version.h"
> 
>+#if HAVE_LIST_H
>+#include "list.h"
>+#endif
>+
> #ifdef RADEON_DRI2
> 
> #include "radeon_bo_gem.h"
>@@ -46,6 +50,10 @@
> #define USE_DRI2_1_1_0
> #endif
> 
>+#if DRI2INFOREC_VERSION >= 4 && HAVE_LIST_H
>+#define USE_DRI2_SCHEDULING
>+#endif
>+
> #if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,6,99,0, 0)
> typedef DRI2BufferPtr BufferPtr;
> #else
>@@ -365,7 +373,7 @@ radeon_dri2_copy_region(DrawablePtr drawable,
> }
> 
> 
>-#if DRI2INFOREC_VERSION >= 4
>+#ifdef USE_DRI2_SCHEDULING
> 
> enum DRI2FrameEventType {
>     DRI2_SWAP,
>@@ -376,7 +384,6 @@ enum DRI2FrameEventType {
> typedef struct _DRI2FrameEvent {
>     XID drawable_id;
>     ClientPtr client;
>-    int client_index;
>     enum DRI2FrameEventType type;
>     int frame;
> 
>@@ -385,8 +392,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 +501,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 +527,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 +538,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 +616,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 +637,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 +725,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 +791,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 +923,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);
>@@ -841,7 +935,7 @@ blit_fallback:
>     return TRUE;
> }
> 
>-#endif /* DRI2INFOREC_VERSION >= 4 */
>+#endif /* USE_DRI2_SCHEDULING */
> 
> 
> Bool
>@@ -850,7 +944,7 @@ radeon_dri2_screen_init(ScreenPtr pScreen)
>     ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
>     RADEONInfoPtr info = RADEONPTR(pScrn);
>     DRI2InfoRec dri2_info = { 0 };
>-#if DRI2INFOREC_VERSION >= 4
>+#ifdef USE_DRI2_SCHEDULING
>     const char *driverNames[1];
> #endif
> 
>@@ -883,7 +977,7 @@ radeon_dri2_screen_init(ScreenPtr pScreen)
> #endif
>     dri2_info.CopyRegion = radeon_dri2_copy_region;
> 
>-#if DRI2INFOREC_VERSION >= 4
>+#ifdef USE_DRI2_SCHEDULING
>     if (info->dri->pKernelDRMVersion->version_minor >= 4) {
>         dri2_info.version = 4;
>         dri2_info.ScheduleSwap = radeon_dri2_schedule_swap;
>@@ -895,6 +989,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 +1014,9 @@ void radeon_dri2_close_screen(ScreenPtr pScreen)
>     ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
>     RADEONInfoPtr info = RADEONPTR(pScrn);
> 
>+#ifdef USE_DRI2_SCHEDULING
>+    DeleteCallback(&ClientStateCallback, radeon_dri2_client_state_changed, 0);
>+#endif
>     DRI2CloseScreen(pScreen);
>     drmFree(info->dri2.device_name);
> }
>-- 
>1.7.3.1
>

Actions: View | Splinter Review

Attachments on bug 29065: 37054 | 37055 | 38119 | 38139 | 38209 | 38210 | 38211 | 38484 | 38703 | 38943 | 39128 | 39258

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.