[Mesa-dev] [PATCH v2] Support unlimited number of display connections

[Brian Paul]

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 <brianp at vmware.com>