On 03/02/2016 10:33 AM, Kyriazis, George wrote:
Brian,

I looked for a trivial/tri test in the mesa repo and I only found the 
gallium/trivial/tri test.  This one I found is loading the device pipes 
directly and is not using glx so it's not affected by my change.  It shows some 
valgrind memory leaks upon exit regardless of my change.

Or maybe are you talking about a different  test?

Yeah, check out the mesa demos tree at git.freedesktop.org/git/mesa/demos


-Brian


Thank you,

George

-----Original Message-----
From: Brian Paul [mailto:bri...@vmware.com]
Sent: Tuesday, March 1, 2016 8:45 PM
To: Kyriazis, George <george.kyria...@intel.com>; mesa-
d...@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH v2] Support unlimited number of display
connections

Hmm, I applied your patch locally and ran the trivial/tri demo and saw a few
valgrind errors upon exit (invalid memory reads).  Can you look into that?

-Brian

On 03/01/2016 07:20 PM, Kyriazis, George wrote:
Thanks Brian,

Since I don't have git write privileges, can somebody check this in for me?

I am working with Tim Rowley @ Intel for the openswr driver.

George

-----Original Message-----
From: Brian Paul [mailto:bri...@vmware.com]
Sent: Tuesday, March 1, 2016 6:55 PM
To: Kyriazis, George <george.kyria...@intel.com>; mesa-
d...@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH v2] Support unlimited number of
display connections

On 03/01/2016 05:44 PM, George Kyriazis wrote:
There is a limit of 10 display connections, which was a problem for
apps/tests that were continuously opening/closing display
connections.

This fix uses XAddExtension() and XESetCloseDisplay() to keep track
of the status of the display connections from the X server, freeing
mesa-related data as X displays get destroyed by the X server.

Poster child is the VTK "TimingTests"

v2: Added missing initializer in struct

---
    src/gallium/state_trackers/glx/xlib/xm_api.c | 122
++++++++++++++++++++++-----
    1 file changed, 103 insertions(+), 19 deletions(-)

diff --git a/src/gallium/state_trackers/glx/xlib/xm_api.c
b/src/gallium/state_trackers/glx/xlib/xm_api.c
index 2f5e1f5..2f1bfae 100644
--- a/src/gallium/state_trackers/glx/xlib/xm_api.c
+++ b/src/gallium/state_trackers/glx/xlib/xm_api.c
@@ -110,14 +110,6 @@ void xmesa_set_driver( const struct xm_driver
*templ )
    }


-/*
- * XXX replace this with a linked list, or better yet, try to
attach the
- * gallium/mesa extra bits to the X Display object with XAddExtension().
- */
-#define MAX_DISPLAYS 10
-static struct xmesa_display Displays[MAX_DISPLAYS]; -static int
NumDisplays = 0;
-
    static int
    xmesa_get_param(struct st_manager *smapi,
                    enum st_manager_param param) @@ -130,34 +122,125
@@ xmesa_get_param(struct st_manager *smapi,
       }
    }

+/* linked list of XMesaDisplay hooks per display */ typedef struct
+_XMesaExtDisplayInfo {
+   struct _XMesaExtDisplayInfo *next;
+   Display *display;
+   XExtCodes *codes;
+   struct xmesa_display mesaDisplay; } XMesaExtDisplayInfo;
+
+typedef struct _XMesaExtInfo {
+   XMesaExtDisplayInfo *head;
+   int ndisplays;
+} XMesaExtInfo;
+
+static XMesaExtInfo MesaExtInfo;
+
+/* hook to delete XMesaDisplay on XDestroyDisplay */ static int
+xmesa_close_display(Display *display, XExtCodes *codes) {
+   XMesaExtDisplayInfo *info, *prev;
+
+   assert(MesaExtInfo.ndisplays > 0);
+   assert(MesaExtInfo.head);
+
+   _XLockMutex(_Xglobal_lock);
+   /* first find display */
+   prev = NULL;
+   for (info = MesaExtInfo.head; info; info = info->next) {
+      if (info->display == display) {
+         prev = info;
+         break;
+      }
+   }
+
+   if (info == NULL) {
+      /* no display found */
+      _XUnlockMutex(_Xglobal_lock);
+      return 0;
+   }
+
+   /* remove display entry from list */
+   if (prev != MesaExtInfo.head) {
+      prev->next = info->next;
+   } else {
+      MesaExtInfo.head = info->next;
+   }
+   MesaExtInfo.ndisplays--;
+
+   _XUnlockMutex(_Xglobal_lock);
+
+   /* don't forget to clean up mesaDisplay */
+   XMesaDisplay xmdpy = &info->mesaDisplay;
+
+   if (xmdpy->screen) {
+      xmdpy->screen->destroy(xmdpy->screen);
+   }
+   free(xmdpy->smapi);
+
+   XFree((char *) info);
+   return 1;
+}
+
    static XMesaDisplay
    xmesa_init_display( Display *display )
    {
       pipe_static_mutex(init_mutex);
       XMesaDisplay xmdpy;
-   int i;
+   XMesaExtDisplayInfo *info;
+
+   if (display == NULL) {
+      return NULL;
+   }

       pipe_mutex_lock(init_mutex);

-   /* Look for XMesaDisplay which corresponds to 'display' */
-   for (i = 0; i < NumDisplays; i++) {
-      if (Displays[i].display == display) {
+   /* Look for XMesaDisplay which corresponds to this display */
+   info = MesaExtInfo.head;
+   while(info) {
+      if (info->display == display) {
             /* Found it */
             pipe_mutex_unlock(init_mutex);
-         return &Displays[i];
+         return  &info->mesaDisplay;
          }
+      info = info->next;
       }

-   /* Create new XMesaDisplay */
+   /* Not found.  Create new XMesaDisplay */
+   /* first allocate X-related resources and hook destroy callback
+ */

-   assert(NumDisplays < MAX_DISPLAYS);
-   xmdpy = &Displays[NumDisplays];
-   NumDisplays++;
-
-   if (!xmdpy->display && display) {
+   /* allocate mesa display info */
+   info = (XMesaExtDisplayInfo *) Xmalloc(sizeof(XMesaExtDisplayInfo));
+   if (info == NULL) {
+      pipe_mutex_unlock(init_mutex);
+      return NULL;
+   }
+   info->display = display;
+   info->codes = XAddExtension(display);
+   if (info->codes == NULL) {
+      /* could not allocate extension.  Fail */
+      Xfree(info);
+      pipe_mutex_unlock(init_mutex);
+      return NULL;
+   }
+   XESetCloseDisplay(display, info->codes->extension,
xmesa_close_display);
+   xmdpy = &info->mesaDisplay; /* to be filled out below */
+
+   /* chain to the list of displays */
+   _XLockMutex(_Xglobal_lock);
+   info->next = MesaExtInfo.head;
+   MesaExtInfo.head = info;
+   MesaExtInfo.ndisplays++;
+   _XUnlockMutex(_Xglobal_lock);
+
+   /* now create the new XMesaDisplay info */
+   if (display) {
          xmdpy->display = display;
          xmdpy->screen = driver.create_pipe_screen(display);
          xmdpy->smapi = CALLOC_STRUCT(st_manager);
+      xmdpy->pipe = NULL;
          if (xmdpy->smapi) {
             xmdpy->smapi->screen = xmdpy->screen;
             xmdpy->smapi->get_param = xmesa_get_param; @@ -185,6
+268,7 @@ xmesa_init_display( Display *display )
       return xmdpy;
    }

+


/***************************************************************
*******/
    /*****                     X Utility Functions                    *****/


/***************************************************************
*******/


LGTM.  Reviewed-by: Brian Paul <bri...@vmware.com>



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to