[PATCH 2/2] shm: Expose SHM segments to backends

Chris Wilson chris at chris-wilson.co.uk
Sat Feb 14 02:36:45 PST 2015


For drivers that can map system memory into their video memory, the
overhead in utilizing SHM memory directly is in the cost of creating
that mapping. SHM segments are typically much longer lived than either
the Pixmap or Image created within them, and so exposing those for the
driver to map amoritizes the cost of that mapping.

A typical example would be the Chromium web browser which creates a SHM
transport buffer but then recreates a SHM Pixmap to perform its upload
for every frame. Recreating a mapping for the SHM Pixmap on every frame
is approximately an extra 25% CPU overhead for a UMA driver using the GPU
to do the compositing transfer from SHM transport buffer to the screen.

In order to expose the SHM segments, we turn the ShmDesc into a regular
object by extending it with dixPrivates and add a destroy hook. Then the
ShmDescPtr is added to the SHM CreatePixmap, PutImage, GetImage interfaces
and hooks provided for the driver. The driver may not implement any of
these, in which case we fallback to the current implementation that
wraps the normal PutImage and GetImage interface.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 Xext/shm.c         | 224 ++++++++++++++++++++++++++++++++++++-----------------
 Xext/shmint.h      |  34 ++++++--
 dix/privates.c     |   1 +
 include/privates.h |   1 +
 4 files changed, 184 insertions(+), 76 deletions(-)

diff --git a/Xext/shm.c b/Xext/shm.c
index 52d9974..9877b0a 100644
--- a/Xext/shm.c
+++ b/Xext/shm.c
@@ -94,11 +94,14 @@ in this Software without prior written authorization from The Open Group.
 
 typedef struct _ShmScrPrivateRec {
     CloseScreenProcPtr CloseScreen;
-    ShmFuncsPtr shmFuncs;
+    ShmFuncs2 shmFuncs;
     DestroyPixmapProcPtr destroyPixmap;
 } ShmScrPrivateRec;
 
-static PixmapPtr fbShmCreatePixmap(XSHM_CREATE_PIXMAP_ARGS);
+static PixmapPtr
+fbShmCreatePixmap(ScreenPtr pScreen,
+                  int width, int height, int depth,
+                  ShmDescPtr shmdesc, int offset);
 static int ShmDetachSegment(void *value, XID shmseg);
 static void ShmResetProc(ExtensionEntry *extEntry);
 static void SShmCompletionEvent(xShmCompletionEvent *from,
@@ -118,8 +121,6 @@ static DevPrivateKeyRec shmScrPrivateKeyRec;
 static DevPrivateKeyRec shmPixmapPrivateKeyRec;
 
 #define shmPixmapPrivateKey (&shmPixmapPrivateKeyRec)
-static ShmFuncs miFuncs = { NULL, NULL };
-static ShmFuncs fbFuncs = { fbShmCreatePixmap, NULL };
 
 #define ShmGetScreenPriv(s) ((ShmScrPrivateRec *)dixLookupPrivate(&(s)->devPrivates, shmScrPrivateKey))
 
@@ -236,11 +237,43 @@ ShmResetProc(ExtensionEntry * extEntry)
 }
 
 void
+ShmRegisterFuncs2(ScreenPtr pScreen, const ShmFuncs2 *funcs)
+{
+    if (funcs->version != 2)
+        return;
+
+    if (!ShmRegisterPrivates())
+        return;
+
+    ShmInitScreenPriv(pScreen)->shmFuncs = *funcs;
+}
+
+static PixmapPtr
+ShmCreatePixmap1(ScreenPtr pScreen,
+                 int width, int height, int depth,
+                 ShmDescPtr shmdesc, int offset)
+{
+    ShmScrPrivateRec *screen_priv = ShmGetScreenPriv(pScreen);
+    return (*screen_priv->shmFuncs._CreatePixmap1) (pScreen, width, height, depth,
+                                                    shmdesc->addr + offset);
+}
+
+void
 ShmRegisterFuncs(ScreenPtr pScreen, ShmFuncsPtr funcs)
 {
+    ShmFuncs2 funcs2;
+
     if (!ShmRegisterPrivates())
         return;
-    ShmInitScreenPriv(pScreen)->shmFuncs = funcs;
+
+    memset(&funcs2, 0, sizeof(funcs2));
+    funcs2.version = SHM_VERSION;
+    if (funcs && funcs->CreatePixmap) {
+        funcs2._CreatePixmap1 = funcs->CreatePixmap;
+        funcs2.CreatePixmap = ShmCreatePixmap1;
+    }
+
+    ShmRegisterFuncs2(pScreen, &funcs2);
 }
 
 static Bool
@@ -268,7 +301,12 @@ ShmDestroyPixmap(PixmapPtr pPixmap)
 void
 ShmRegisterFbFuncs(ScreenPtr pScreen)
 {
-    ShmRegisterFuncs(pScreen, &fbFuncs);
+    ShmFuncs2 funcs;
+
+    memset(&funcs, 0, sizeof(funcs));
+    funcs.version = SHM_VERSION;
+    funcs.CreatePixmap = fbShmCreatePixmap;
+    ShmRegisterFuncs2(pScreen, &funcs);
 }
 
 static int
@@ -390,7 +428,8 @@ ProcShmAttach(ClientPtr client)
         shmdesc->refcnt++;
     }
     else {
-        shmdesc = malloc(sizeof(ShmDescRec));
+        shmdesc = (ShmDescPtr) dixAllocateObjectWithPrivates(ShmDescRec,
+                                                             PRIVATE_SHM);
         if (!shmdesc)
             return BadAlloc;
 #ifdef SHM_FD_PASSING
@@ -399,7 +438,7 @@ ProcShmAttach(ClientPtr client)
         shmdesc->addr = shmat(stuff->shmid, 0,
                               stuff->readOnly ? SHM_RDONLY : 0);
         if ((shmdesc->addr == ((char *) -1)) || SHMSTAT(stuff->shmid, &buf)) {
-            free(shmdesc);
+            dixFreeObjectWithPrivates(shmdesc, PRIVATE_SHM);
             return BadAccess;
         }
 
@@ -409,7 +448,7 @@ ProcShmAttach(ClientPtr client)
 
         if (shm_access(client, &(SHM_PERM(buf)), stuff->readOnly) == -1) {
             shmdt(shmdesc->addr);
-            free(shmdesc);
+            dixFreeObjectWithPrivates(shmdesc, PRIVATE_SHM);
             return BadAccess;
         }
 
@@ -431,9 +470,18 @@ ShmDetachSegment(void *value, /* must conform to DeleteType */
 {
     ShmDescPtr shmdesc = (ShmDescPtr) value;
     ShmDescPtr *prev;
+    int i;
 
     if (--shmdesc->refcnt)
         return TRUE;
+
+    for (i = 0; i < screenInfo.numScreens; i++) {
+        ShmScrPrivateRec *screen_priv =
+            ShmGetScreenPriv(screenInfo.screens[i]);
+        if (screen_priv->shmFuncs.Destroy)
+            (*screen_priv->shmFuncs.Destroy) (screenInfo.screens[i], shmdesc);
+    }
+
 #if SHM_FD_PASSING
     if (shmdesc->is_fd) {
         if (shmdesc->busfault)
@@ -444,7 +492,7 @@ ShmDetachSegment(void *value, /* must conform to DeleteType */
         shmdt(shmdesc->addr);
     for (prev = &Shmsegs; *prev != shmdesc; prev = &(*prev)->next);
     *prev = shmdesc->next;
-    free(shmdesc);
+    dixFreeObjectWithPrivates(shmdesc, PRIVATE_SHM);
     return Success;
 }
 
@@ -518,6 +566,8 @@ ProcShmPutImage(ClientPtr client)
     DrawablePtr pDraw;
     long length;
     ShmDescPtr shmdesc;
+    ShmScrPrivateRec *screen_priv;
+    int rc;
 
     REQUEST(xShmPutImageReq);
 
@@ -575,25 +625,42 @@ ProcShmPutImage(ClientPtr client)
         return BadValue;
     }
 
-    if ((((stuff->format == ZPixmap) && (stuff->srcX == 0)) ||
-         ((stuff->format != ZPixmap) &&
-          (stuff->srcX < screenInfo.bitmapScanlinePad) &&
-          ((stuff->format == XYBitmap) ||
-           ((stuff->srcY == 0) &&
-            (stuff->srcHeight == stuff->totalHeight))))) &&
-        ((stuff->srcX + stuff->srcWidth) == stuff->totalWidth))
-        (*pGC->ops->PutImage) (pDraw, pGC, stuff->depth,
-                               stuff->dstX, stuff->dstY,
-                               stuff->totalWidth, stuff->srcHeight,
-                               stuff->srcX, stuff->format,
-                               shmdesc->addr + stuff->offset +
-                               (stuff->srcY * length));
-    else
-        doShmPutImage(pDraw, pGC, stuff->depth, stuff->format,
-                      stuff->totalWidth, stuff->totalHeight,
-                      stuff->srcX, stuff->srcY,
-                      stuff->srcWidth, stuff->srcHeight,
-                      stuff->dstX, stuff->dstY, shmdesc->addr + stuff->offset);
+    rc = BadImplementation;
+    screen_priv = ShmGetScreenPriv(pDraw->pScreen);
+    if (screen_priv->shmFuncs.PutImage)
+            rc = (*screen_priv->shmFuncs.PutImage) (pDraw, pGC, stuff->depth, stuff->format,
+                                                    stuff->srcX, stuff->srcY,
+                                                    stuff->srcWidth, stuff->srcHeight,
+                                                    stuff->dstX, stuff->dstY,
+                                                    stuff->totalWidth, stuff->totalHeight,
+                                                    shmdesc, stuff->offset);
+
+    if (rc == BadImplementation) {
+        if ((((stuff->format == ZPixmap) && (stuff->srcX == 0)) ||
+             ((stuff->format != ZPixmap) &&
+              (stuff->srcX < screenInfo.bitmapScanlinePad) &&
+              ((stuff->format == XYBitmap) ||
+               ((stuff->srcY == 0) &&
+                (stuff->srcHeight == stuff->totalHeight))))) &&
+            ((stuff->srcX + stuff->srcWidth) == stuff->totalWidth)) {
+            (*pGC->ops->PutImage) (pDraw, pGC, stuff->depth,
+                                   stuff->dstX, stuff->dstY,
+                                   stuff->totalWidth, stuff->srcHeight,
+                                   stuff->srcX, stuff->format,
+                                   shmdesc->addr + stuff->offset +
+                                   (stuff->srcY * length));
+        }
+        else
+            doShmPutImage(pDraw, pGC, stuff->depth, stuff->format,
+                          stuff->totalWidth, stuff->totalHeight,
+                          stuff->srcX, stuff->srcY,
+                          stuff->srcWidth, stuff->srcHeight,
+                          stuff->dstX, stuff->dstY, shmdesc->addr + stuff->offset);
+        rc = Success;
+    }
+
+    if (rc)
+        return rc;
 
     if (stuff->sendEvent) {
         xShmCompletionEvent ev = {
@@ -618,6 +685,7 @@ ProcShmGetImage(ClientPtr client)
     Mask plane = 0;
     xShmGetImageReply xgi;
     ShmDescPtr shmdesc;
+    ShmScrPrivateRec *screen_priv;
     VisualID visual = None;
     int rc;
 
@@ -678,30 +746,44 @@ ProcShmGetImage(ClientPtr client)
     VERIFY_SHMSIZE(shmdesc, stuff->offset, length, client);
     xgi.size = length;
 
-    if (length == 0) {
-        /* nothing to do */
-    }
-    else if (stuff->format == ZPixmap) {
-        (*pDraw->pScreen->GetImage) (pDraw, stuff->x, stuff->y,
-                                     stuff->width, stuff->height,
-                                     stuff->format, stuff->planeMask,
-                                     shmdesc->addr + stuff->offset);
-    }
-    else {
-
-        length = stuff->offset;
-        for (; plane; plane >>= 1) {
-            if (stuff->planeMask & plane) {
-                (*pDraw->pScreen->GetImage) (pDraw,
-                                             stuff->x, stuff->y,
-                                             stuff->width, stuff->height,
-                                             stuff->format, plane,
-                                             shmdesc->addr + length);
-                length += lenPer;
+    screen_priv = ShmGetScreenPriv(pDraw->pScreen);
+    if (length == 0)
+        rc = Success; /* nothing to do */
+    else if (screen_priv->shmFuncs.GetImage)
+        rc = (*screen_priv->shmFuncs.GetImage) (pDraw, stuff->x, stuff->y,
+                                                stuff->width, stuff->height,
+                                                stuff->format, stuff->planeMask,
+                                                shmdesc, stuff->offset);
+    else
+        rc = BadImplementation;
+
+    if (rc == BadImplementation) {
+        if (stuff->format == ZPixmap) {
+            (*pDraw->pScreen->GetImage) (pDraw, stuff->x, stuff->y,
+                                         stuff->width, stuff->height,
+                                         stuff->format, stuff->planeMask,
+                                         shmdesc->addr + stuff->offset);
+        }
+        else {
+            length = stuff->offset;
+            for (; plane; plane >>= 1) {
+                if (stuff->planeMask & plane) {
+                    (*pDraw->pScreen->GetImage) (pDraw,
+                                                 stuff->x, stuff->y,
+                                                 stuff->width, stuff->height,
+                                                 stuff->format, plane,
+                                                 shmdesc->addr + length);
+                    length += lenPer;
+                }
             }
         }
+
+	rc = Success;
     }
 
+    if (rc)
+            return rc;
+
     if (client->swapped) {
         swaps(&xgi.sequenceNumber);
         swapl(&xgi.length);
@@ -963,12 +1045,12 @@ ProcPanoramiXShmCreatePixmap(ClientPtr client)
         pScreen = screenInfo.screens[j];
 
         screen_priv = ShmGetScreenPriv(pScreen);
-        pMap = (*screen_priv->shmFuncs->CreatePixmap) (pScreen,
-                                                       stuff->width,
-                                                       stuff->height,
-                                                       stuff->depth,
-                                                       shmdesc->addr +
-                                                       stuff->offset);
+        pMap = (*screen_priv->shmFuncs.CreatePixmap) (pScreen,
+                                                      stuff->width,
+                                                      stuff->height,
+                                                      stuff->depth,
+                                                      shmdesc,
+                                                      stuff->offset);
 
         if (pMap) {
             dixSetPrivate(&pMap->devPrivates, shmPixmapPrivateKey, shmdesc);
@@ -1000,7 +1082,8 @@ ProcPanoramiXShmCreatePixmap(ClientPtr client)
 
 static PixmapPtr
 fbShmCreatePixmap(ScreenPtr pScreen,
-                  int width, int height, int depth, char *addr)
+                  int width, int height, int depth,
+                  ShmDescPtr shmdesc, int offset)
 {
     PixmapPtr pPixmap;
 
@@ -1011,7 +1094,7 @@ fbShmCreatePixmap(ScreenPtr pScreen,
     if (!(*pScreen->ModifyPixmapHeader) (pPixmap, width, height, depth,
                                          BitsPerPixel(depth),
                                          PixmapBytePad(width, depth),
-                                         (void *) addr)) {
+                                         (void *) (shmdesc->addr + offset))) {
         (*pScreen->DestroyPixmap) (pPixmap);
         return NullPixmap;
     }
@@ -1075,10 +1158,9 @@ ProcShmCreatePixmap(ClientPtr client)
 
     VERIFY_SHMSIZE(shmdesc, stuff->offset, size, client);
     screen_priv = ShmGetScreenPriv(pDraw->pScreen);
-    pMap = (*screen_priv->shmFuncs->CreatePixmap) (pDraw->pScreen, stuff->width,
-                                                   stuff->height, stuff->depth,
-                                                   shmdesc->addr +
-                                                   stuff->offset);
+    pMap = (*screen_priv->shmFuncs.CreatePixmap) (pDraw->pScreen, stuff->width,
+                                                  stuff->height, stuff->depth,
+                                                  shmdesc, stuff->offset);
     if (pMap) {
         rc = XaceHook(XACE_RESOURCE_ACCESS, client, stuff->pid, RT_PIXMAP,
                       pMap, RT_NONE, NULL, DixCreateAccess);
@@ -1135,7 +1217,8 @@ ProcShmAttachFd(ClientPtr client)
         return BadMatch;
     }
 
-    shmdesc = malloc(sizeof(ShmDescRec));
+    shmdesc = (ShmDescPtr) dixAllocateObjectWithPrivates(ShmDescRec,
+							 PRIVATE_SHM);
     if (!shmdesc) {
         close(fd);
         return BadAlloc;
@@ -1147,8 +1230,8 @@ ProcShmAttachFd(ClientPtr client)
                          fd, 0);
 
     close(fd);
-    if (shmdesc->addr == ((char *) -1)) {
-        free(shmdesc);
+    if ((shmdesc->addr == (char *) -1)) {
+        dixFreeObjectWithPrivates(shmdesc, PRIVATE_SHM);
         return BadAccess;
     }
 
@@ -1160,7 +1243,7 @@ ProcShmAttachFd(ClientPtr client)
     shmdesc->busfault = busfault_register_mmap(shmdesc->addr, shmdesc->size, ShmBusfaultNotify, shmdesc);
     if (!shmdesc->busfault) {
         munmap(shmdesc->addr, shmdesc->size);
-        free(shmdesc);
+        dixFreeObjectWithPrivates(shmdesc, PRIVATE_SHM);
         return BadAlloc;
     }
 
@@ -1226,7 +1309,8 @@ ProcShmCreateSegment(ClientPtr client)
         close(fd);
         return BadAlloc;
     }
-    shmdesc = malloc(sizeof(ShmDescRec));
+    shmdesc = (ShmDescPtr) dixAllocateObjectWithPrivates(ShmDescRec,
+							 PRIVATE_SHM);
     if (!shmdesc) {
         close(fd);
         return BadAlloc;
@@ -1239,7 +1323,7 @@ ProcShmCreateSegment(ClientPtr client)
 
     if (shmdesc->addr == ((char *) -1)) {
         close(fd);
-        free(shmdesc);
+        dixFreeObjectWithPrivates(shmdesc, PRIVATE_SHM);
         return BadAccess;
     }
 
@@ -1251,7 +1335,7 @@ ProcShmCreateSegment(ClientPtr client)
     if (!shmdesc->busfault) {
         close(fd);
         munmap(shmdesc->addr, shmdesc->size);
-        free(shmdesc);
+        dixFreeObjectWithPrivates(shmdesc, PRIVATE_SHM);
         return BadAlloc;
     }
 
@@ -1482,9 +1566,7 @@ ShmExtensionInit(void)
         for (i = 0; i < screenInfo.numScreens; i++) {
             ShmScrPrivateRec *screen_priv =
                 ShmInitScreenPriv(screenInfo.screens[i]);
-            if (!screen_priv->shmFuncs)
-                screen_priv->shmFuncs = &miFuncs;
-            if (!screen_priv->shmFuncs->CreatePixmap)
+            if (!screen_priv->shmFuncs.CreatePixmap)
                 sharedPixmaps = xFalse;
         }
         if (sharedPixmaps)
diff --git a/Xext/shmint.h b/Xext/shmint.h
index 9dadea7..b7ab721 100644
--- a/Xext/shmint.h
+++ b/Xext/shmint.h
@@ -51,11 +51,6 @@
     int		/* depth */, \
     char *                      /* addr */
 
-typedef struct _ShmFuncs {
-    PixmapPtr (*CreatePixmap) (XSHM_CREATE_PIXMAP_ARGS);
-    void (*PutImage) (XSHM_PUT_IMAGE_ARGS);
-} ShmFuncs, *ShmFuncsPtr;
-
 #if XTRANS_SEND_FDS
 #define SHM_FD_PASSING  1
 #endif
@@ -72,6 +67,7 @@ typedef struct _ShmDesc {
     struct busfault *busfault;
     XID resource;
 #endif
+    PrivateRec *devPrivates;
 } ShmDescRec, *ShmDescPtr;
 
 #ifdef SHM_FD_PASSING
@@ -80,10 +76,38 @@ typedef struct _ShmDesc {
 #define SHMDESC_IS_FD(shmdesc)  (0)
 #endif
 
+typedef struct _ShmFuncs {
+    PixmapPtr (*CreatePixmap) (XSHM_CREATE_PIXMAP_ARGS);
+    void (*PutImage) (XSHM_PUT_IMAGE_ARGS);
+} ShmFuncs, *ShmFuncsPtr;
+
+typedef struct _ShmFuncs2 {
+    int version;
+#define SHM_VERSION 2
+
+    void (*Destroy) (ScreenPtr screen, ShmDescPtr shmdesc);
+
+    PixmapPtr (*CreatePixmap) (ScreenPtr screen, int width, int height, int depth,
+                               ShmDescPtr shmdesc, int offset);
+    PixmapPtr (*_CreatePixmap1) (XSHM_CREATE_PIXMAP_ARGS);
+
+    int (*PutImage) (DrawablePtr draw, GCPtr gc, int depth, unsigned int format,
+		     int sx, int sy, int sw, int sh,
+		     int dx, int dy, int dw, int dh,
+                     ShmDescPtr shmdesc, int offset);
+    int (*GetImage) (DrawablePtr draw,
+                     int x, int y, int w, int h,
+                     int format, int mask,
+                     ShmDescPtr shmdesc, int offset);
+} ShmFuncs2, *ShmFuncs2Ptr;
+
 extern _X_EXPORT void
  ShmRegisterFuncs(ScreenPtr pScreen, ShmFuncsPtr funcs);
 
 extern _X_EXPORT void
+ ShmRegisterFuncs2(ScreenPtr pScreen, const ShmFuncs2 *funcs);
+
+extern _X_EXPORT void
  ShmRegisterFbFuncs(ScreenPtr pScreen);
 
 extern _X_EXPORT RESTYPE ShmSegType;
diff --git a/dix/privates.c b/dix/privates.c
index e03b225..2bb7e35 100644
--- a/dix/privates.c
+++ b/dix/privates.c
@@ -110,6 +110,7 @@ static const char *key_names[PRIVATE_LAST] = {
     [PRIVATE_GLYPHSET] = "GLYPHSET",
     [PRIVATE_PICTURE] = "PICTURE",
     [PRIVATE_SYNC_FENCE] = "SYNC_FENCE",
+    [PRIVATE_SHM] = "SHM",
 };
 
 static const Bool screen_specific_private[PRIVATE_LAST] = {
diff --git a/include/privates.h b/include/privates.h
index 7d1461c..16615f8 100644
--- a/include/privates.h
+++ b/include/privates.h
@@ -51,6 +51,7 @@ typedef enum {
     PRIVATE_GLYPHSET,
     PRIVATE_PICTURE,
     PRIVATE_SYNC_FENCE,
+    PRIVATE_SHM,
 
     /* last private type */
     PRIVATE_LAST,
-- 
2.1.4



More information about the xorg-devel mailing list