From 22c4300ee25a20e1f815e46225bf0de9cfd6748f Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Wed, 7 Oct 2009 09:00:02 -0700 Subject: [PATCH 1/7] Simplify XineramifyXv. The structure of the adaptor-matching algorithm was obscured by trying to use "continue" and "break" as the only control-flow primitives. It's a lot more clear if you add "return" to that set. Signed-off-by: Jamey Sharp Reviewed-by: Adam Jackson --- Xext/xvdisp.c | 149 ++++++++++++++++++++++---------------------------- 1 file changed, 65 insertions(+), 84 deletions(-) diff --git a/Xext/xvdisp.c b/Xext/xvdisp.c index b6fc34ff9..4345672ab 100644 --- a/Xext/xvdisp.c +++ b/Xext/xvdisp.c @@ -1850,110 +1850,91 @@ XineramaXvPutStill(ClientPtr client) return result; } +static Bool +isImageAdaptor(XvAdaptorPtr pAdapt) +{ + return (pAdapt->type & XvImageMask) && (pAdapt->nImages > 0); +} + +static Bool +hasOverlay(XvAdaptorPtr pAdapt) +{ + int i; + for(i = 0; i < pAdapt->nAttributes; i++) + if(!strcmp(pAdapt->pAttributes[i].name, "XV_COLORKEY")) + return TRUE; + return FALSE; +} + +static XvAdaptorPtr +matchAdaptor(ScreenPtr pScreen, XvAdaptorPtr refAdapt, Bool isOverlay) +{ + int i; + XvScreenPtr xvsp = dixLookupPrivate(&pScreen->devPrivates, XvGetScreenKey()); + /* Do not try to go on if xv is not supported on this screen */ + if(xvsp == NULL) + return NULL; + + /* if the adaptor has the same name it's a perfect match */ + for(i = 0; i < xvsp->nAdaptors; i++) { + XvAdaptorPtr pAdapt = xvsp->pAdaptors + i; + if(!strcmp(refAdapt->name, pAdapt->name)) + return pAdapt; + } + + /* otherwise we only look for XvImage adaptors */ + if(!isImageAdaptor(refAdapt)) + return NULL; + + /* prefer overlay/overlay non-overlay/non-overlay pairing */ + for(i = 0; i < xvsp->nAdaptors; i++) { + XvAdaptorPtr pAdapt = xvsp->pAdaptors + i; + if(isImageAdaptor(pAdapt) && isOverlay == hasOverlay(pAdapt)) + return pAdapt; + } + + /* but we'll take any XvImage pairing if we can get it */ + for(i = 0; i < xvsp->nAdaptors; i++) { + XvAdaptorPtr pAdapt = xvsp->pAdaptors + i; + if(isImageAdaptor(pAdapt)) + return pAdapt; + } + return NULL; +} + void XineramifyXv(void) { - ScreenPtr pScreen, screen0 = screenInfo.screens[0]; - XvScreenPtr xvsp0 = (XvScreenPtr)dixLookupPrivate(&screen0->devPrivates, - XvGetScreenKey()); - XvAdaptorPtr refAdapt, pAdapt; - XvAttributePtr pAttr; - XvScreenPtr xvsp; - Bool isOverlay, hasOverlay; - PanoramiXRes *port; + XvScreenPtr xvsp0 = dixLookupPrivate(&screenInfo.screens[0]->devPrivates, XvGetScreenKey()); XvAdaptorPtr MatchingAdaptors[MAXSCREENS]; - int i, j, k, l; + int i, j, k; XvXRTPort = CreateNewResourceType(XineramaDeleteResource, "XvXRTPort"); if (!xvsp0 || !XvXRTPort) return; for(i = 0; i < xvsp0->nAdaptors; i++) { - refAdapt = xvsp0->pAdaptors + i; - - bzero(MatchingAdaptors, sizeof(XvAdaptorPtr) * MAXSCREENS); - - MatchingAdaptors[0] = refAdapt; - + Bool isOverlay; + XvAdaptorPtr refAdapt = xvsp0->pAdaptors + i; if(!(refAdapt->type & XvInputMask)) continue; - - isOverlay = FALSE; - for(j = 0; j < refAdapt->nAttributes; j++) { - pAttr = refAdapt->pAttributes + j; - if(!strcmp(pAttr->name, "XV_COLORKEY")) { - isOverlay = TRUE; - break; - } - } - - for(j = 1; j < PanoramiXNumScreens; j++) { - pScreen = screenInfo.screens[j]; - xvsp = (XvScreenPtr)dixLookupPrivate(&pScreen->devPrivates, - XvGetScreenKey()); - /* Do not try to go on if xv is not supported on this screen */ - if (xvsp==NULL) continue ; - - /* if the adaptor has the same name it's a perfect match */ - for(k = 0; k < xvsp->nAdaptors; k++) { - pAdapt = xvsp->pAdaptors + k; - if(!strcmp(refAdapt->name, pAdapt->name)) { - MatchingAdaptors[j] = pAdapt; - break; - } - } - if(MatchingAdaptors[j]) continue; /* found it */ - - /* otherwise we only look for XvImage adaptors */ - if(!(refAdapt->type & XvImageMask)) continue; - if(refAdapt->nImages <= 0) continue; - - /* prefer overlay/overlay non-overlay/non-overlay pairing */ - for(k = 0; k < xvsp->nAdaptors; k++) { - pAdapt = xvsp->pAdaptors + k; - if((pAdapt->type & XvImageMask) && (pAdapt->nImages > 0)) { - hasOverlay = FALSE; - for(l = 0; l < pAdapt->nAttributes; l++) { - if(!strcmp(pAdapt->pAttributes[l].name, "XV_COLORKEY")) { - hasOverlay = TRUE; - break; - } - } - if(isOverlay && hasOverlay) { - MatchingAdaptors[j] = pAdapt; - break; - } - else if(!isOverlay && !hasOverlay) { - MatchingAdaptors[j] = pAdapt; - break; - } - } - } - - if(MatchingAdaptors[j]) continue; /* found it */ - - /* but we'll take any XvImage pairing if we can get it */ - - for(k = 0; k < xvsp->nAdaptors; k++) { - pAdapt = xvsp->pAdaptors + k; - if((pAdapt->type & XvImageMask) && (pAdapt->nImages > 0)) { - MatchingAdaptors[j] = pAdapt; - break; - } - } - } + + MatchingAdaptors[0] = refAdapt; + isOverlay = hasOverlay(refAdapt); + for(j = 1; j < PanoramiXNumScreens; j++) + MatchingAdaptors[j] = matchAdaptor(screenInfo.screens[j], refAdapt, isOverlay); /* now create a resource for each port */ for(j = 0; j < refAdapt->nPorts; j++) { - if(!(port = xalloc(sizeof(PanoramiXRes)))) + PanoramiXRes *port = xalloc(sizeof(PanoramiXRes)); + if(!port) break; - port->info[0].id = MatchingAdaptors[0]->base_id + j; - AddResource(port->info[0].id, XvXRTPort, port); - for(k = 1; k < PanoramiXNumScreens; k++) { + for(k = 0; k < PanoramiXNumScreens; k++) { if(MatchingAdaptors[k] && (MatchingAdaptors[k]->nPorts > j)) port->info[k].id = MatchingAdaptors[k]->base_id + j; else port->info[k].id = 0; } + AddResource(port->info[0].id, XvXRTPort, port); } } From da0217891904bc48d5f0b7ea5c62c8ea0e9b95f9 Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Wed, 21 Apr 2010 22:26:28 -0700 Subject: [PATCH 2/7] Track screens' installed colormaps as screen privates. Several DDXes allow each screen to have at most one (or in some cases, exactly one) installed colormap. These all use the same pattern: Declare a global-lifetime array of MAXSCREENS ColormapPtrs, and index it by screen number. This patch converts most of those to use screen privates instead. Signed-off-by: Jamey Sharp Acked-by: Tiago Vignatti --- fb/fbcmap.c | 16 ++++++++-------- hw/vfb/InitOutput.c | 23 +++++++++++++---------- hw/xnest/Color.c | 22 +++++++++------------- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/fb/fbcmap.c b/fb/fbcmap.c index 2ff3234b5..b775bc335 100644 --- a/fb/fbcmap.c +++ b/fb/fbcmap.c @@ -36,16 +36,18 @@ #error "You should be compiling fbcmap_mi.c instead of fbcmap.c!" #endif +static int cmapScrPrivateKeyIndex; +static DevPrivateKey cmapScrPrivateKey = &cmapScrPrivateKeyIndex; - -ColormapPtr FbInstalledMaps[MAXSCREENS]; +#define GetInstalledColormap(s) ((ColormapPtr) dixLookupPrivate(&(s)->devPrivates, cmapScrPrivateKey)) +#define SetInstalledColormap(s,c) (dixSetPrivate(&(s)->devPrivates, cmapScrPrivateKey, c)) int fbListInstalledColormaps(ScreenPtr pScreen, Colormap *pmaps) { /* By the time we are processing requests, we can guarantee that there * is always a colormap installed */ - *pmaps = FbInstalledMaps[pScreen->myNum]->mid; + *pmaps = GetInstalledColormap(pScreen)->mid; return (1); } @@ -53,8 +55,7 @@ fbListInstalledColormaps(ScreenPtr pScreen, Colormap *pmaps) void fbInstallColormap(ColormapPtr pmap) { - int index = pmap->pScreen->myNum; - ColormapPtr oldpmap = FbInstalledMaps[index]; + ColormapPtr oldpmap = GetInstalledColormap(pmap->pScreen); if(pmap != oldpmap) { @@ -63,7 +64,7 @@ fbInstallColormap(ColormapPtr pmap) if(oldpmap != (ColormapPtr)None) WalkTree(pmap->pScreen, TellLostMap, (char *)&oldpmap->mid); /* Install pmap */ - FbInstalledMaps[index] = pmap; + SetInstalledColormap(pmap->pScreen, pmap); WalkTree(pmap->pScreen, TellGainedMap, (char *)&pmap->mid); } } @@ -71,8 +72,7 @@ fbInstallColormap(ColormapPtr pmap) void fbUninstallColormap(ColormapPtr pmap) { - int index = pmap->pScreen->myNum; - ColormapPtr curpmap = FbInstalledMaps[index]; + ColormapPtr curpmap = GetInstalledColormap(pmap->pScreen); if(pmap == curpmap) { diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c index 60915fdbf..b2baa197f 100644 --- a/hw/vfb/InitOutput.c +++ b/hw/vfb/InitOutput.c @@ -427,14 +427,18 @@ ddxProcessArgument(int argc, char *argv[], int i) return 0; } -static ColormapPtr InstalledMaps[MAXSCREENS]; +static int cmapScrPrivateKeyIndex; +static DevPrivateKey cmapScrPrivateKey = &cmapScrPrivateKeyIndex; + +#define GetInstalledColormap(s) ((ColormapPtr) dixLookupPrivate(&(s)->devPrivates, cmapScrPrivateKey)) +#define SetInstalledColormap(s,c) (dixSetPrivate(&(s)->devPrivates, cmapScrPrivateKey, c)) static int vfbListInstalledColormaps(ScreenPtr pScreen, Colormap *pmaps) { /* By the time we are processing requests, we can guarantee that there * is always a colormap installed */ - *pmaps = InstalledMaps[pScreen->myNum]->mid; + *pmaps = GetInstalledColormap(pScreen)->mid; return (1); } @@ -442,8 +446,7 @@ vfbListInstalledColormaps(ScreenPtr pScreen, Colormap *pmaps) static void vfbInstallColormap(ColormapPtr pmap) { - int index = pmap->pScreen->myNum; - ColormapPtr oldpmap = InstalledMaps[index]; + ColormapPtr oldpmap = GetInstalledColormap(pmap->pScreen); if (pmap != oldpmap) { @@ -459,7 +462,7 @@ vfbInstallColormap(ColormapPtr pmap) if(oldpmap != (ColormapPtr)None) WalkTree(pmap->pScreen, TellLostMap, (char *)&oldpmap->mid); /* Install pmap */ - InstalledMaps[index] = pmap; + SetInstalledColormap(pmap->pScreen, pmap); WalkTree(pmap->pScreen, TellGainedMap, (char *)&pmap->mid); entries = pmap->pVisual->ColormapEntries; @@ -500,7 +503,7 @@ vfbInstallColormap(ColormapPtr pmap) static void vfbUninstallColormap(ColormapPtr pmap) { - ColormapPtr curpmap = InstalledMaps[pmap->pScreen->myNum]; + ColormapPtr curpmap = GetInstalledColormap(pmap->pScreen); if(pmap == curpmap) { @@ -521,7 +524,7 @@ vfbStoreColors(ColormapPtr pmap, int ndef, xColorItem *pdefs) XWDColor *pXWDCmap; int i; - if (pmap != InstalledMaps[pmap->pScreen->myNum]) + if (pmap != GetInstalledColormap(pmap->pScreen)) { return; } @@ -830,10 +833,10 @@ vfbCloseScreen(int index, ScreenPtr pScreen) /* * XXX probably lots of stuff to clean. For now, - * clear InstalledMaps[] so that server reset works correctly. + * clear installed colormaps so that server reset works correctly. */ - for (i = 0; i < MAXSCREENS; i++) - InstalledMaps[i] = NULL; + for (i = 0; i < screenInfo.numScreens; i++) + SetInstalledColormap(screenInfo.screens[i], NULL); return pScreen->CloseScreen(index, pScreen); } diff --git a/hw/xnest/Color.c b/hw/xnest/Color.c index dc749478f..2e6de15e4 100644 --- a/hw/xnest/Color.c +++ b/hw/xnest/Color.c @@ -34,7 +34,11 @@ is" without express or implied warranty. #include "XNWindow.h" #include "Args.h" -static ColormapPtr InstalledMaps[MAXSCREENS]; +static int cmapScrPrivateKeyIndex; +static DevPrivateKey cmapScrPrivateKey = &cmapScrPrivateKeyIndex; + +#define GetInstalledColormap(s) ((ColormapPtr) dixLookupPrivate(&(s)->devPrivates, cmapScrPrivateKey)) +#define SetInstalledColormap(s,c) (dixSetPrivate(&(s)->devPrivates, cmapScrPrivateKey, c)) Bool xnestCreateColormap(ColormapPtr pCmap) @@ -332,11 +336,7 @@ xnestDirectUninstallColormaps(ScreenPtr pScreen) void xnestInstallColormap(ColormapPtr pCmap) { - int index; - ColormapPtr pOldCmap; - - index = pCmap->pScreen->myNum; - pOldCmap = InstalledMaps[index]; + ColormapPtr pOldCmap = GetInstalledColormap(pCmap->pScreen); if(pCmap != pOldCmap) { @@ -346,7 +346,7 @@ xnestInstallColormap(ColormapPtr pCmap) if(pOldCmap != (ColormapPtr)None) WalkTree(pCmap->pScreen, TellLostMap, (pointer)&pOldCmap->mid); - InstalledMaps[index] = pCmap; + SetInstalledColormap(pCmap->pScreen, pCmap); WalkTree(pCmap->pScreen, TellGainedMap, (pointer)&pCmap->mid); xnestSetInstalledColormapWindows(pCmap->pScreen); @@ -357,11 +357,7 @@ xnestInstallColormap(ColormapPtr pCmap) void xnestUninstallColormap(ColormapPtr pCmap) { - int index; - ColormapPtr pCurCmap; - - index = pCmap->pScreen->myNum; - pCurCmap = InstalledMaps[index]; + ColormapPtr pCurCmap = GetInstalledColormap(pCmap->pScreen); if(pCmap == pCurCmap) { @@ -382,7 +378,7 @@ int xnestListInstalledColormaps(ScreenPtr pScreen, Colormap *pCmapIDs) { if (xnestInstalledDefaultColormap) { - *pCmapIDs = InstalledMaps[pScreen->myNum]->mid; + *pCmapIDs = GetInstalledColormap(pScreen)->mid; return 1; } else From eeb84547556b943af2acff207e034823205c7dfe Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Mon, 26 Apr 2010 18:04:25 -0700 Subject: [PATCH 3/7] Delete redundant scrnum field from Xvfb private screen-info struct. The screen number can be inferred from the position in the vfbScreens array, and it was only used in two places, so it was hardly important. Signed-off-by: Jamey Sharp Reviewed-by: Tiago Vignatti --- hw/vfb/InitOutput.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c index b2baa197f..6d827eb1a 100644 --- a/hw/vfb/InitOutput.c +++ b/hw/vfb/InitOutput.c @@ -77,7 +77,6 @@ from The Open Group. typedef struct { - int scrnum; int width; int paddedBytesWidth; int paddedWidth; @@ -141,7 +140,6 @@ vfbInitializeDefaultScreens(void) for (i = 0; i < MAXSCREENS; i++) { - vfbScreens[i].scrnum = i; vfbScreens[i].width = VFB_DEFAULT_WIDTH; vfbScreens[i].height = VFB_DEFAULT_HEIGHT; vfbScreens[i].depth = VFB_DEFAULT_DEPTH; @@ -598,7 +596,7 @@ vfbAllocateMmappedFramebuffer(vfbScreenInfoPtr pvfb) char dummyBuffer[DUMMY_BUFFER_SIZE]; int currentFileSize, writeThisTime; - sprintf(pvfb->mmap_file, "%s/Xvfb_screen%d", pfbdir, pvfb->scrnum); + sprintf(pvfb->mmap_file, "%s/Xvfb_screen%d", pfbdir, (int) (pvfb - vfbScreens)); if (-1 == (pvfb->mmap_fd = open(pvfb->mmap_file, O_CREAT|O_RDWR, 0666))) { perror("open"); @@ -671,7 +669,7 @@ vfbAllocateSharedMemoryFramebuffer(vfbScreenInfoPtr pvfb) return; } - ErrorF("screen %d shmid %d\n", pvfb->scrnum, pvfb->shmid); + ErrorF("screen %d shmid %d\n", (int) (pvfb - vfbScreens), pvfb->shmid); } #endif /* HAS_SHM */ From 20e84b0b44e8b3b40a3ecab5b2e64a27de247b16 Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Mon, 26 Apr 2010 18:09:23 -0700 Subject: [PATCH 4/7] Xvfb: Simplify screen option processing. Inspired by Jon Turney's "Xwin: Simplify screen option processing" patch, which does something similar for the Xwin server. Besides making the code more readable, this eliminates most of Xvfb's references to MAXSCREENS. Signed-off-by: Jamey Sharp Reviewed-by: Tiago Vignatti --- hw/vfb/InitOutput.c | 91 ++++++++++++++------------------------------- 1 file changed, 27 insertions(+), 64 deletions(-) diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c index 6d827eb1a..f71082854 100644 --- a/hw/vfb/InitOutput.c +++ b/hw/vfb/InitOutput.c @@ -105,6 +105,14 @@ typedef struct static int vfbNumScreens; static vfbScreenInfo vfbScreens[MAXSCREENS]; +static vfbScreenInfo defaultScreenInfo = { + .width = VFB_DEFAULT_WIDTH, + .height = VFB_DEFAULT_HEIGHT, + .depth = VFB_DEFAULT_DEPTH, + .blackPixel = VFB_DEFAULT_BLACKPIXEL, + .whitePixel = VFB_DEFAULT_WHITEPIXEL, + .lineBias = VFB_DEFAULT_LINEBIAS, +}; static Bool vfbPixmapDepths[33]; #ifdef HAS_MMAP static char *pfbdir = NULL; @@ -112,7 +120,6 @@ static char *pfbdir = NULL; typedef enum { NORMAL_MEMORY_FB, SHARED_MEMORY_FB, MMAPPED_FILE_FB } fbMemType; static fbMemType fbmemtype = NORMAL_MEMORY_FB; static char needswap = 0; -static int lastScreen = -1; static Bool Render = TRUE; #define swapcopy16(_dst, _src) \ @@ -133,24 +140,6 @@ vfbInitializePixmapDepths(void) vfbPixmapDepths[i] = FALSE; } -static void -vfbInitializeDefaultScreens(void) -{ - int i; - - for (i = 0; i < MAXSCREENS; i++) - { - vfbScreens[i].width = VFB_DEFAULT_WIDTH; - vfbScreens[i].height = VFB_DEFAULT_HEIGHT; - vfbScreens[i].depth = VFB_DEFAULT_DEPTH; - vfbScreens[i].blackPixel = VFB_DEFAULT_BLACKPIXEL; - vfbScreens[i].whitePixel = VFB_DEFAULT_WHITEPIXEL; - vfbScreens[i].lineBias = VFB_DEFAULT_LINEBIAS; - vfbScreens[i].pfbMemory = NULL; - } - vfbNumScreens = 1; -} - static int vfbBitsPerPixel(int depth) { @@ -265,14 +254,20 @@ int ddxProcessArgument(int argc, char *argv[], int i) { static Bool firstTime = TRUE; + static int lastScreen = -1; + vfbScreenInfo *currentScreen; if (firstTime) { - vfbInitializeDefaultScreens(); vfbInitializePixmapDepths(); firstTime = FALSE; } + if (lastScreen == -1) + currentScreen = &defaultScreenInfo; + else + currentScreen = &vfbScreens[lastScreen]; + #define CHECK_FOR_REQUIRED_ARGUMENTS(num) \ if (((i + num) >= argc) || (!argv[i + num])) { \ ErrorF("Required argument to %s not specified\n", argv[i]); \ @@ -292,6 +287,10 @@ ddxProcessArgument(int argc, char *argv[], int i) FatalError("Invalid screen number %d passed to -screen\n", screenNum); } + + for (; vfbNumScreens <= screenNum; ++vfbNumScreens) + vfbScreens[vfbNumScreens] = defaultScreenInfo; + if (3 != sscanf(argv[i+2], "%dx%dx%d", &vfbScreens[screenNum].width, &vfbScreens[screenNum].height, @@ -303,8 +302,6 @@ ddxProcessArgument(int argc, char *argv[], int i) argv[i+2], screenNum); } - if (screenNum >= vfbNumScreens) - vfbNumScreens = screenNum + 1; lastScreen = screenNum; return 3; } @@ -346,61 +343,22 @@ ddxProcessArgument(int argc, char *argv[], int i) if (strcmp (argv[i], "-blackpixel") == 0) /* -blackpixel n */ { - Pixel pix; CHECK_FOR_REQUIRED_ARGUMENTS(1); - pix = atoi(argv[++i]); - if (-1 == lastScreen) - { - int i; - for (i = 0; i < MAXSCREENS; i++) - { - vfbScreens[i].blackPixel = pix; - } - } - else - { - vfbScreens[lastScreen].blackPixel = pix; - } + currentScreen->blackPixel = atoi(argv[++i]); return 2; } if (strcmp (argv[i], "-whitepixel") == 0) /* -whitepixel n */ { - Pixel pix; CHECK_FOR_REQUIRED_ARGUMENTS(1); - pix = atoi(argv[++i]); - if (-1 == lastScreen) - { - int i; - for (i = 0; i < MAXSCREENS; i++) - { - vfbScreens[i].whitePixel = pix; - } - } - else - { - vfbScreens[lastScreen].whitePixel = pix; - } + currentScreen->whitePixel = atoi(argv[++i]); return 2; } if (strcmp (argv[i], "-linebias") == 0) /* -linebias n */ { - unsigned int linebias; CHECK_FOR_REQUIRED_ARGUMENTS(1); - linebias = atoi(argv[++i]); - if (-1 == lastScreen) - { - int i; - for (i = 0; i < MAXSCREENS; i++) - { - vfbScreens[i].lineBias = linebias; - } - } - else - { - vfbScreens[lastScreen].lineBias = linebias; - } + currentScreen->lineBias = atoi(argv[++i]); return 2; } @@ -993,6 +951,11 @@ InitOutput(ScreenInfo *screenInfo, int argc, char **argv) /* initialize screens */ + if (vfbNumScreens < 1) + { + vfbScreens[0] = defaultScreenInfo; + vfbNumScreens = 1; + } for (i = 0; i < vfbNumScreens; i++) { if (-1 == AddScreen(vfbScreenInit, argc, argv)) From f9e3a2955d2ca73604c68fc9d51405581b832edb Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Mon, 26 Apr 2010 18:23:27 -0700 Subject: [PATCH 5/7] Make Xvfb independent of MAXSCREENS. If a -screen option specifies a screen number higher than any previously specified, reallocate the vfb-private array of screen-info structs. If built with a DIX that still has a MAXSCREENS limit, asking for too many screens won't be detected until InitOutput calls AddScreen. Signed-off-by: Jamey Sharp Reviewed-by: Tiago Vignatti --- hw/vfb/InitOutput.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c index f71082854..29857877e 100644 --- a/hw/vfb/InitOutput.c +++ b/hw/vfb/InitOutput.c @@ -104,7 +104,7 @@ typedef struct } vfbScreenInfo, *vfbScreenInfoPtr; static int vfbNumScreens; -static vfbScreenInfo vfbScreens[MAXSCREENS]; +static vfbScreenInfo *vfbScreens; static vfbScreenInfo defaultScreenInfo = { .width = VFB_DEFAULT_WIDTH, .height = VFB_DEFAULT_HEIGHT, @@ -280,7 +280,7 @@ ddxProcessArgument(int argc, char *argv[], int i) int screenNum; CHECK_FOR_REQUIRED_ARGUMENTS(2); screenNum = atoi(argv[i+1]); - if (screenNum < 0 || screenNum >= MAXSCREENS) + if (screenNum < 0) { ErrorF("Invalid screen number %d\n", screenNum); UseMsg(); @@ -288,8 +288,14 @@ ddxProcessArgument(int argc, char *argv[], int i) screenNum); } - for (; vfbNumScreens <= screenNum; ++vfbNumScreens) - vfbScreens[vfbNumScreens] = defaultScreenInfo; + if (vfbNumScreens <= screenNum) + { + vfbScreens = xrealloc(vfbScreens, sizeof(*vfbScreens) * (screenNum + 1)); + if (!vfbScreens) + FatalError("Not enough memory for screen %d\n", screenNum); + for (; vfbNumScreens <= screenNum; ++vfbNumScreens) + vfbScreens[vfbNumScreens] = defaultScreenInfo; + } if (3 != sscanf(argv[i+2], "%dx%dx%d", &vfbScreens[screenNum].width, @@ -953,7 +959,7 @@ InitOutput(ScreenInfo *screenInfo, int argc, char **argv) if (vfbNumScreens < 1) { - vfbScreens[0] = defaultScreenInfo; + vfbScreens = &defaultScreenInfo; vfbNumScreens = 1; } for (i = 0; i < vfbNumScreens; i++) From a1c2acfe798c57e5be7e5f6c111a6ce91400487a Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Wed, 21 Apr 2010 18:05:45 -0700 Subject: [PATCH 6/7] xfree86: use screen privates for exclusive DGA clients. Most DGA requests allow at most one client to be using DGA on each screen. Instead of keeping track of the current client in a MAXSCREEN-sized array, track it in a per-screen private. Signed-off-by: Jamey Sharp Acked-by: Tiago Vignatti Reviewed-by: Aaron Plattner --- hw/xfree86/dixmods/extmod/xf86dga2.c | 70 +++++++++++++++------------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/hw/xfree86/dixmods/extmod/xf86dga2.c b/hw/xfree86/dixmods/extmod/xf86dga2.c index 5367bcc42..038551467 100644 --- a/hw/xfree86/dixmods/extmod/xf86dga2.c +++ b/hw/xfree86/dixmods/extmod/xf86dga2.c @@ -57,12 +57,12 @@ static void XDGAResetProc(ExtensionEntry *extEntry); static void DGAClientStateChange (CallbackListPtr*, pointer, pointer); -static ClientPtr DGAClients[MAXSCREENS]; - unsigned char DGAReqCode = 0; int DGAErrorBase; int DGAEventBase; +static int DGAScreenPrivateKeyIndex; +static DevPrivateKey DGAScreenPrivateKey = &DGAScreenPrivateKeyIndex; static int DGAClientPrivateKeyIndex; static DevPrivateKey DGAClientPrivateKey = &DGAClientPrivateKeyIndex; static int DGACallbackRefCount = 0; @@ -73,6 +73,11 @@ typedef struct { int minor; } DGAPrivRec, *DGAPrivPtr; +#define DGA_GETCLIENT(idx) ((ClientPtr) \ + dixLookupPrivate(&screenInfo.screens[idx]->devPrivates, DGAScreenPrivateKey)) +#define DGA_SETCLIENT(idx,p) \ + dixSetPrivate(&screenInfo.screens[idx]->devPrivates, DGAScreenPrivateKey, p) + #define DGA_GETPRIV(c) ((DGAPrivPtr) \ dixLookupPrivate(&(c)->devPrivates, DGAClientPrivateKey)) #define DGA_SETPRIV(c,p) \ @@ -93,9 +98,6 @@ XFree86DGAExtensionInit(INITARGS) StandardMinorOpcode))) { int i; - for(i = 0; i < MAXSCREENS; i++) - DGAClients[i] = NULL; - DGAReqCode = (unsigned char)extEntry->base; DGAErrorBase = extEntry->errorBase; DGAEventBase = extEntry->eventBase; @@ -282,7 +284,7 @@ DGAClientStateChange ( int i; for(i = 0; i < screenInfo.numScreens; i++) { - if(DGAClients[i] == pci->client) { + if(DGA_GETCLIENT(i) == pci->client) { client = pci->client; break; } @@ -294,7 +296,7 @@ DGAClientStateChange ( XDGAModeRec mode; PixmapPtr pPix; - DGAClients[i] = NULL; + DGA_SETCLIENT(i, NULL); DGASelectInput(i, NULL, 0); DGASetMode(i, 0, &mode, &pPix); @@ -311,10 +313,12 @@ ProcXDGASetMode(ClientPtr client) XDGAModeRec mode; xXDGAModeInfo info; PixmapPtr pPix; + ClientPtr owner; int size; if (stuff->screen > screenInfo.numScreens) return BadValue; + owner = DGA_GETCLIENT(stuff->screen); REQUEST_SIZE_MATCH(xXDGASetModeReq); rep.type = X_Reply; @@ -326,16 +330,15 @@ ProcXDGASetMode(ClientPtr client) if (!DGAAvailable(stuff->screen)) return DGAErrorBase + XF86DGANoDirectVideoMode; - if(DGAClients[stuff->screen] && - (DGAClients[stuff->screen] != client)) + if(owner && owner != client) return DGAErrorBase + XF86DGANoDirectVideoMode; if(!stuff->mode) { - if(DGAClients[stuff->screen]) { + if(owner) { if(--DGACallbackRefCount == 0) DeleteCallback(&ClientStateCallback, DGAClientStateChange, NULL); } - DGAClients[stuff->screen] = NULL; + DGA_SETCLIENT(stuff->screen, NULL); DGASelectInput(stuff->screen, NULL, 0); DGASetMode(stuff->screen, 0, &mode, &pPix); WriteToClient(client, sz_xXDGASetModeReply, (char*)&rep); @@ -345,12 +348,12 @@ ProcXDGASetMode(ClientPtr client) if(Success != DGASetMode(stuff->screen, stuff->mode, &mode, &pPix)) return BadValue; - if(!DGAClients[stuff->screen]) { + if(!owner) { if(DGACallbackRefCount++ == 0) AddCallback (&ClientStateCallback, DGAClientStateChange, NULL); } - DGAClients[stuff->screen] = client; + DGA_SETCLIENT(stuff->screen, client); if(pPix) { if(AddResource(stuff->pid, RT_PIXMAP, (pointer)(pPix))) { @@ -405,7 +408,7 @@ ProcXDGASetViewport(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGASetViewportReq); @@ -425,7 +428,7 @@ ProcXDGAInstallColormap(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGAInstallColormapReq); @@ -451,12 +454,12 @@ ProcXDGASelectInput(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGASelectInputReq); - if(DGAClients[stuff->screen] == client) + if(DGA_GETCLIENT(stuff->screen) == client) DGASelectInput(stuff->screen, client, stuff->mask); return (client->noClientException); @@ -471,7 +474,7 @@ ProcXDGAFillRectangle(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGAFillRectangleReq); @@ -491,7 +494,7 @@ ProcXDGACopyArea(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGACopyAreaReq); @@ -512,7 +515,7 @@ ProcXDGACopyTransparentArea(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGACopyTransparentAreaReq); @@ -534,7 +537,7 @@ ProcXDGAGetViewportStatus(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGAGetViewportStatusReq); @@ -557,7 +560,7 @@ ProcXDGASync(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGASyncReq); @@ -602,7 +605,7 @@ ProcXDGAChangePixmapMode(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGAChangePixmapModeReq); @@ -633,7 +636,7 @@ ProcXDGACreateColormap(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if(DGAClients[stuff->screen] != client) + if(DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXDGACreateColormapReq); @@ -713,18 +716,19 @@ ProcXF86DGADirectVideo(ClientPtr client) int num; PixmapPtr pix; XDGAModeRec mode; + ClientPtr owner; REQUEST(xXF86DGADirectVideoReq); if (stuff->screen > screenInfo.numScreens) return BadValue; + owner = DGA_GETCLIENT(stuff->screen); REQUEST_SIZE_MATCH(xXF86DGADirectVideoReq); if (!DGAAvailable(stuff->screen)) return DGAErrorBase + XF86DGANoDirectVideoMode; - if (DGAClients[stuff->screen] && - (DGAClients[stuff->screen] != client)) + if (owner && owner != client) return DGAErrorBase + XF86DGANoDirectVideoMode; if (stuff->enable & XF86DGADirectGraphics) { @@ -743,19 +747,19 @@ ProcXF86DGADirectVideo(ClientPtr client) /* We need to track the client and attach the teardown callback */ if (stuff->enable & (XF86DGADirectGraphics | XF86DGADirectKeyb | XF86DGADirectMouse)) { - if (!DGAClients[stuff->screen]) { + if (!owner) { if (DGACallbackRefCount++ == 0) AddCallback (&ClientStateCallback, DGAClientStateChange, NULL); } - DGAClients[stuff->screen] = client; + DGA_SETCLIENT(stuff->screen, client); } else { - if (DGAClients[stuff->screen]) { + if (owner) { if (--DGACallbackRefCount == 0) DeleteCallback(&ClientStateCallback, DGAClientStateChange, NULL); } - DGAClients[stuff->screen] = NULL; + DGA_SETCLIENT(stuff->screen, NULL); } return (client->noClientException); @@ -800,7 +804,7 @@ ProcXF86DGASetViewPort(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if (DGAClients[stuff->screen] != client) + if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXF86DGASetViewPortReq); @@ -864,7 +868,7 @@ ProcXF86DGAInstallColormap(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if (DGAClients[stuff->screen] != client) + if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXF86DGAInstallColormapReq); @@ -913,7 +917,7 @@ ProcXF86DGAViewPortChanged(ClientPtr client) if (stuff->screen > screenInfo.numScreens) return BadValue; - if (DGAClients[stuff->screen] != client) + if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; REQUEST_SIZE_MATCH(xXF86DGAViewPortChangedReq); From b5b8f91b82d7b150c926dd3fecee6c3aafff6e39 Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Thu, 22 Apr 2010 21:35:17 -0700 Subject: [PATCH 7/7] xfree86: use screen privates for Xv offscreen images. This replaces a globally-allocated array that depended on MAXSCREENS. Signed-off-by: Jamey Sharp Acked-by: Tiago Vignatti Reviewed-by: Aaron Plattner --- hw/xfree86/common/xf86xv.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/hw/xfree86/common/xf86xv.c b/hw/xfree86/common/xf86xv.c index bdcc4fc2b..2cc2f6093 100644 --- a/hw/xfree86/common/xf86xv.c +++ b/hw/xfree86/common/xf86xv.c @@ -186,7 +186,9 @@ typedef struct { int num; } OffscreenImageRec; -static OffscreenImageRec OffscreenImages[MAXSCREENS]; +static int OffscreenPrivateKeyIndex; +static DevPrivateKey OffscreenPrivateKey = &OffscreenPrivateKeyIndex; +#define GetOffscreenImage(pScreen) ((OffscreenImageRec *) dixLookupPrivate(&(pScreen)->devPrivates, OffscreenPrivateKey)) Bool xf86XVRegisterOffscreenImages( @@ -194,9 +196,18 @@ xf86XVRegisterOffscreenImages( XF86OffscreenImagePtr images, int num ){ - OffscreenImages[pScreen->myNum].num = num; - OffscreenImages[pScreen->myNum].images = images; + OffscreenImageRec *OffscreenImage; + /* This function may be called before xf86XVScreenInit, so there's + * no better place than this to call dixRequestPrivate to ensure we + * have space reserved. After the first call it is a no-op. */ + if(!dixRequestPrivate(OffscreenPrivateKey, sizeof(OffscreenImageRec)) || + !(OffscreenImage = GetOffscreenImage(pScreen))) + /* Every X.org driver assumes this function always succeeds, so + * just die on allocation failure. */ + FatalError("Could not allocate private storage for XV offscreen images.\n"); + OffscreenImage->num = num; + OffscreenImage->images = images; return TRUE; } @@ -205,8 +216,9 @@ xf86XVQueryOffscreenImages( ScreenPtr pScreen, int *num ){ - *num = OffscreenImages[pScreen->myNum].num; - return OffscreenImages[pScreen->myNum].images; + OffscreenImageRec *OffscreenImage = GetOffscreenImage(pScreen); + *num = OffscreenImage->num; + return OffscreenImage->images; } @@ -1177,9 +1189,6 @@ xf86XVCloseScreen(int i, ScreenPtr pScreen) XvAdaptorPtr pa; int c; - /* Clear offscreen images */ - memset(&OffscreenImages[pScreen->myNum], 0, sizeof(OffscreenImages[0])); - if(!ScreenPriv) return TRUE; if(ScreenPriv->videoGC) {