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

[Kyriazis, George]

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:brianp at vmware.com]
> Sent: Tuesday, March 1, 2016 6:55 PM
> To: Kyriazis, George <george.kyriazis at intel.com>; mesa-
> dev at 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 <brianp at vmware.com>