Hi Pierre,
On Tue, May 04, 2010 at 10:21:55PM +0200, ext Pierre-Loup A. Griffais wrote:
>
> I just reproduced something that sounds like what you're describing with two
> R520 cards (one X screen per card) and the 'radeon' driver. However, it seems
> unrelated to my change; that's what the hang looks like:
>
> 575 VGAGet();
> (gdb) bt
> #0 VGAarbiterCreateGC (pGC=0x83ebab0)
> at ../../../../hw/xfree86/common/xf86VGAarbiter.c:575
> #1 0x080777ba in CreateGC (pDrawable=0x82d8d78, mask=<value optimized out>,
> pval=0xbffff534, pStatus=0xbffff53c, gcid=0, client=0x81ffca8)
> at ../../dix/gc.c:647
> #2 0x0819e612 in miDCMakeGC (pWin=0x82b5530) at ../../mi/midispcur.c:422
> #3 0x0819e7c4 in miDCDeviceInitialize (pDev=0x83ebdf0, pScreen=0x8263688)
> at ../../mi/midispcur.c:790
> #4 0x081c48cf in miSpriteDeviceCursorInitialize (pDev=0x83ebdf0,
> pScreen=0x8263688) at ../../mi/misprite.c:949
> #5 0x08186364 in xf86DeviceCursorInitialize (pDev=0x83ebdf0,
> pScreen=0x8263688) at ../../../../hw/xfree86/ramdac/xf86Cursor.c:453
> #6 0x081672ba in VGAarbiterDeviceCursorInitialize (pDev=0x83ebdf0,
> pScreen=0x8263688) at ../../../../hw/xfree86/common/xf86VGAarbiter.c:1035
> #7 0x080a1e0c in miPointerDeviceInitialize (pDev=0x83ebdf0,
> pScreen=0x8263688)
> at ../../mi/mipointer.c:283
> #8 0x08087ed5 in ActivateDevice (dev=0x83ebdf0, sendevent=1 '\001')
> at ../../dix/devices.c:477
> #9 0x08088f08 in InitCoreDevices () at ../../dix/devices.c:610
> #10 0x08066d18 in main (argc=1, argv=0xbffff8a4, envp=0xbffff8ac)
> at ../../dix/main.c:255
>
> The reason my change exposes this bug is that it creates a GC attached to the
> second screen upfront. If I roll it back, I still get the same hang after
> trying
> to move a SW cursor to the second screen of connecting an X client to the
> second
> screen.
I'm still getting a very weird lock-up with your patch. I can get it even with
hw cursor. Seems not related at all with the log bellow when radeon POST bios,
so I guess your commit added a regression. Just reverting it solves the
problem - and sorry, I don't know this code in depth to start dig the reason.
BTW, Peter did you test this patch there with multiple cards? I'd revert this
patch meanwhile (attached).
> Looking at the X log, I see:
>
> (II) RADEON(1): PCIE card detected
> (II) Loading sub module "int10"
> (II) LoadModule: "int10"
> (II) Reloading /usr/lib/xorg/modules/libint10.so
> (II) RADEON(1): initializing int10
> (EE) RADEON(1): Cannot read V_BIOS (3) Input/output error
> (WW) RADEON(1): Failed to read PCI ROM!
> (II) RADEON(1): Attempting to read un-POSTed bios
>
> and in the kernel log:
>
> [ 1240.582149] pci 0000:05:00.0: Invalid ROM contents
>
> That means the VGA arbiter tried to switch VGA access to an un-posted device,
> which is presumably the cause of the hang. It seems like the X screen should
> fail ScreenInit() and get discarded after initializing int10 fails. Whatever
> the
> reason behind that is, the driver ought to fail more gracefully.
>
> In any case, I'm guessing you have similar spew in your logs?
no. My logs are "normal", without any apparent errors.
Thank you,
Tiago
>From efe63a191e72eef40454a2bd7cf3a8f0d0d41389 Mon Sep 17 00:00:00 2001
From: Tiago Vignatti <tiago.vigna...@nokia.com>
Date: Wed, 19 May 2010 17:29:25 +0300
Subject: [PATCH] Revert "mi: don't thrash resources when displaying the software cursor across screens"
This reverts commit 518f3b189b6c8aa28b62837d14309fd06163ccbb.
Conflicts:
mi/midispcur.c
---
mi/midispcur.c | 269 +++++++++++++++++++++++++++++++++----------------------
1 files changed, 161 insertions(+), 108 deletions(-)
diff --git a/mi/midispcur.c b/mi/midispcur.c
index 1acc469..ccc8ffa 100644
--- a/mi/midispcur.c
+++ b/mi/midispcur.c
@@ -59,9 +59,9 @@ static DevPrivateKey miDCScreenKey = &miDCScreenKeyIndex;
static Bool miDCCloseScreen(int index, ScreenPtr pScreen);
-/* per device per-screen private data */
-static int miDCSpriteKeyIndex[MAXSCREENS];
-static DevPrivateKey miDCSpriteKey = miDCSpriteKeyIndex;
+/* per device private data */
+static int miDCSpriteKeyIndex;
+static DevPrivateKey miDCSpriteKey = &miDCSpriteKeyIndex;
typedef struct {
GCPtr pSourceGC, pMaskGC;
@@ -75,10 +75,10 @@ typedef struct {
#endif
} miDCBufferRec, *miDCBufferPtr;
-#define MIDCBUFFER(dev, screen) \
+#define MIDCBUFFER(dev) \
((DevHasCursor(dev)) ? \
- (miDCBufferPtr)dixLookupPrivate(&dev->devPrivates, miDCSpriteKey + (screen)->myNum) : \
- (miDCBufferPtr)dixLookupPrivate(&dev->u.master->devPrivates, miDCSpriteKey + (screen)->myNum))
+ (miDCBufferPtr)dixLookupPrivate(&dev->devPrivates, miDCSpriteKey) : \
+ (miDCBufferPtr)dixLookupPrivate(&dev->u.master->devPrivates, miDCSpriteKey))
/*
* The core pointer buffer will point to the index of the virtual core pointer
@@ -158,6 +158,10 @@ miDCInitialize (ScreenPtr pScreen, miPointerScreenFuncPtr screenFuncs)
return TRUE;
}
+#define tossGC(gc) (gc ? FreeGC (gc, (GContext) 0) : 0)
+#define tossPix(pix) (pix ? (*pScreen->DestroyPixmap) (pix) : TRUE)
+#define tossPict(pict) (pict ? FreePicture (pict, 0) : 0)
+
static Bool
miDCCloseScreen (int index, ScreenPtr pScreen)
{
@@ -179,6 +183,7 @@ miDCRealizeCursor (ScreenPtr pScreen, CursorPtr pCursor)
}
#ifdef ARGB_CURSOR
+#define EnsurePicture(picture,draw,win) (picture || miDCMakePicture(&picture,draw,win))
static VisualPtr
miDCGetWindowVisual (WindowPtr pWin)
@@ -410,8 +415,12 @@ miDCPutBits (
(*maskGC->ops->PushPixels) (maskGC, pPriv->maskBits, pDrawable, w, h, x, y);
}
+#define EnsureGC(gc,win) (gc || miDCMakeGC(&gc, win))
+
static GCPtr
-miDCMakeGC(WindowPtr pWin)
+miDCMakeGC(
+ GCPtr *ppGC,
+ WindowPtr pWin)
{
GCPtr pGC;
int status;
@@ -422,6 +431,7 @@ miDCMakeGC(WindowPtr pWin)
pGC = CreateGC((DrawablePtr)pWin,
GCSubwindowMode|GCGraphicsExposures, gcvals, &status,
(XID)0, serverClient);
+ *ppGC = pGC;
return pGC;
}
@@ -446,11 +456,22 @@ miDCPutUpCursor (DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor,
pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates,
miDCScreenKey);
pWin = WindowTable[pScreen->myNum];
- pBuffer = MIDCBUFFER(pDev, pScreen);
+ pBuffer = MIDCBUFFER(pDev);
#ifdef ARGB_CURSOR
if (pPriv->pPicture)
{
+ /* see comment in miDCPutUpCursor */
+ if (pBuffer->pRootPicture &&
+ pBuffer->pRootPicture->pDrawable &&
+ pBuffer->pRootPicture->pDrawable->pScreen != pScreen)
+ {
+ tossPict(pBuffer->pRootPicture);
+ pBuffer->pRootPicture = NULL;
+ }
+
+ if (!EnsurePicture(pBuffer->pRootPicture, &pWin->drawable, pWin))
+ return FALSE;
CompositePicture (PictOpOver,
pPriv->pPicture,
NULL,
@@ -463,6 +484,33 @@ miDCPutUpCursor (DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor,
else
#endif
{
+ /**
+ * XXX: Before MPX, the sourceGC and maskGC were attached to the
+ * screen, and would switch as the screen switches. With mpx we have
+ * the GC's attached to the device now, so each time we switch screen
+ * we need to make sure the GC's are allocated on the new screen.
+ * This is ... not optimal. (whot)
+ */
+ if (pBuffer->pSourceGC && pScreen != pBuffer->pSourceGC->pScreen)
+ {
+ tossGC(pBuffer->pSourceGC);
+ pBuffer->pSourceGC = NULL;
+ }
+
+ if (pBuffer->pMaskGC && pScreen != pBuffer->pMaskGC->pScreen)
+ {
+ tossGC(pBuffer->pMaskGC);
+ pBuffer->pMaskGC = NULL;
+ }
+
+ if (!EnsureGC(pBuffer->pSourceGC, pWin))
+ return FALSE;
+ if (!EnsureGC(pBuffer->pMaskGC, pWin))
+ {
+ FreeGC (pBuffer->pSourceGC, (GContext) 0);
+ pBuffer->pSourceGC = 0;
+ return FALSE;
+ }
miDCPutBits ((DrawablePtr)pWin, pPriv,
pBuffer->pSourceGC, pBuffer->pMaskGC,
x, y, pCursor->bits->width, pCursor->bits->height,
@@ -483,7 +531,7 @@ miDCSaveUnderCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates,
miDCScreenKey);
- pBuffer = MIDCBUFFER(pDev, pScreen);
+ pBuffer = MIDCBUFFER(pDev);
pSave = pBuffer->pSave;
pWin = WindowTable[pScreen->myNum];
@@ -496,7 +544,14 @@ miDCSaveUnderCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
if (!pSave)
return FALSE;
}
-
+ /* see comment in miDCPutUpCursor */
+ if (pBuffer->pSaveGC && pBuffer->pSaveGC->pScreen != pScreen)
+ {
+ tossGC(pBuffer->pSaveGC);
+ pBuffer->pSaveGC = NULL;
+ }
+ if (!EnsureGC(pBuffer->pSaveGC, pWin))
+ return FALSE;
pGC = pBuffer->pSaveGC;
if (pSave->drawable.serialNumber != pGC->serialNumber)
ValidateGC ((DrawablePtr) pSave, pGC);
@@ -517,13 +572,20 @@ miDCRestoreUnderCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates,
miDCScreenKey);
- pBuffer = MIDCBUFFER(pDev, pScreen);
+ pBuffer = MIDCBUFFER(pDev);
pSave = pBuffer->pSave;
pWin = WindowTable[pScreen->myNum];
if (!pSave)
return FALSE;
-
+ /* see comment in miDCPutUpCursor */
+ if (pBuffer->pRestoreGC && pBuffer->pRestoreGC->pScreen != pScreen)
+ {
+ tossGC(pBuffer->pRestoreGC);
+ pBuffer->pRestoreGC = NULL;
+ }
+ if (!EnsureGC(pBuffer->pRestoreGC, pWin))
+ return FALSE;
pGC = pBuffer->pRestoreGC;
if (pWin->drawable.serialNumber != pGC->serialNumber)
ValidateGC ((DrawablePtr) pWin, pGC);
@@ -545,7 +607,7 @@ miDCChangeSave (DeviceIntPtr pDev, ScreenPtr pScreen,
pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates,
miDCScreenKey);
- pBuffer = MIDCBUFFER(pDev, pScreen);
+ pBuffer = MIDCBUFFER(pDev);
pSave = pBuffer->pSave;
pWin = WindowTable[pScreen->myNum];
@@ -554,7 +616,14 @@ miDCChangeSave (DeviceIntPtr pDev, ScreenPtr pScreen,
*/
if (!pSave)
return FALSE;
-
+ /* see comment in miDCPutUpCursor */
+ if (pBuffer->pRestoreGC && pBuffer->pRestoreGC->pScreen != pScreen)
+ {
+ tossGC(pBuffer->pRestoreGC);
+ pBuffer->pRestoreGC = NULL;
+ }
+ if (!EnsureGC(pBuffer->pRestoreGC, pWin))
+ return FALSE;
pGC = pBuffer->pRestoreGC;
if (pWin->drawable.serialNumber != pGC->serialNumber)
ValidateGC ((DrawablePtr) pWin, pGC);
@@ -593,7 +662,14 @@ miDCChangeSave (DeviceIntPtr pDev, ScreenPtr pScreen,
(*pGC->ops->CopyArea) ((DrawablePtr) pSave, (DrawablePtr) pWin, pGC,
0, sourcey, -dx, copyh, x + dx, desty);
}
-
+ /* see comment in miDCPutUpCursor */
+ if (pBuffer->pSaveGC && pBuffer->pSaveGC->pScreen != pScreen)
+ {
+ tossGC(pBuffer->pSaveGC);
+ pBuffer->pSaveGC = NULL;
+ }
+ if (!EnsureGC(pBuffer->pSaveGC, pWin))
+ return FALSE;
pGC = pBuffer->pSaveGC;
if (pSave->drawable.serialNumber != pGC->serialNumber)
ValidateGC ((DrawablePtr) pSave, pGC);
@@ -690,7 +766,7 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor,
pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates,
miDCScreenKey);
pWin = WindowTable[pScreen->myNum];
- pBuffer = MIDCBUFFER(pDev, pScreen);
+ pBuffer = MIDCBUFFER(pDev);
pTemp = pBuffer->pTemp;
if (!pTemp ||
@@ -733,9 +809,17 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor,
#ifdef ARGB_CURSOR
if (pPriv->pPicture)
{
- if (!pBuffer->pTempPicture)
- miDCMakePicture(&pBuffer->pTempPicture, &pTemp->drawable, pWin);
+ /* see comment in miDCPutUpCursor */
+ if (pBuffer->pTempPicture &&
+ pBuffer->pTempPicture->pDrawable &&
+ pBuffer->pTempPicture->pDrawable->pScreen != pScreen)
+ {
+ tossPict(pBuffer->pTempPicture);
+ pBuffer->pTempPicture = NULL;
+ }
+ if (!EnsurePicture(pBuffer->pTempPicture, &pTemp->drawable, pWin))
+ return FALSE;
CompositePicture (PictOpOver,
pPriv->pPicture,
NULL,
@@ -748,12 +832,38 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor,
else
#endif
{
+ if (!pBuffer->pPixSourceGC)
+ {
+ pBuffer->pPixSourceGC = CreateGC ((DrawablePtr)pTemp,
+ GCGraphicsExposures, &gcval, &status, (XID)0, serverClient);
+ if (!pBuffer->pPixSourceGC)
+ return FALSE;
+ }
+ if (!pBuffer->pPixMaskGC)
+ {
+ pBuffer->pPixMaskGC = CreateGC ((DrawablePtr)pTemp,
+ GCGraphicsExposures, &gcval, &status, (XID)0, serverClient);
+ if (!pBuffer->pPixMaskGC)
+ return FALSE;
+ }
miDCPutBits ((DrawablePtr)pTemp, pPriv,
pBuffer->pPixSourceGC, pBuffer->pPixMaskGC,
dx, dy, pCursor->bits->width, pCursor->bits->height,
source, mask);
}
+ /* see comment in miDCPutUpCursor */
+ if (pBuffer->pRestoreGC && pBuffer->pRestoreGC->pScreen != pScreen)
+ {
+ tossGC(pBuffer->pRestoreGC);
+ pBuffer->pRestoreGC = NULL;
+ }
+ /*
+ * copy the temporary pixmap onto the screen
+ */
+
+ if (!EnsureGC(pBuffer->pRestoreGC, pWin))
+ return FALSE;
pGC = pBuffer->pRestoreGC;
if (pWin->drawable.serialNumber != pGC->serialNumber)
ValidateGC ((DrawablePtr) pWin, pGC);
@@ -767,108 +877,51 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor,
static Bool
miDCDeviceInitialize(DeviceIntPtr pDev, ScreenPtr pScreen)
{
- miDCBufferPtr pBuffer;
- WindowPtr pWin;
- XID gcval = FALSE;
- int status;
- int i;
-
- if (!DevHasCursor(pDev))
- return TRUE;
-
- for (i = 0; i < screenInfo.numScreens; i++)
- {
- pScreen = screenInfo.screens[i];
-
- pBuffer = malloc(sizeof(miDCBufferRec));
- if (!pBuffer)
- goto failure;
-
- dixSetPrivate(&pDev->devPrivates, miDCSpriteKey + pScreen->myNum, pBuffer);
- pWin = WindowTable[pScreen->myNum];
-
- pBuffer->pSourceGC = miDCMakeGC(pWin);
- if (!pBuffer->pSourceGC)
- goto failure;
-
- pBuffer->pMaskGC = miDCMakeGC(pWin);
- if (!pBuffer->pMaskGC)
- goto failure;
-
- pBuffer->pSaveGC = miDCMakeGC(pWin);
- if (!pBuffer->pSaveGC)
- goto failure;
-
- pBuffer->pRestoreGC = miDCMakeGC(pWin);
- if (!pBuffer->pRestoreGC)
- goto failure;
-
- pBuffer->pMoveGC = CreateGC ((DrawablePtr)pWin,
- GCGraphicsExposures, &gcval, &status, (XID)0, serverClient);
- if (!pBuffer->pMoveGC)
- goto failure;
-
- pBuffer->pPixSourceGC = CreateGC ((DrawablePtr)pWin,
- GCGraphicsExposures, &gcval, &status, (XID)0, serverClient);
- if (!pBuffer->pPixSourceGC)
- goto failure;
-
- pBuffer->pPixMaskGC = CreateGC ((DrawablePtr)pWin,
- GCGraphicsExposures, &gcval, &status, (XID)0, serverClient);
- if (!pBuffer->pPixMaskGC)
- goto failure;
-
+ miDCBufferPtr pBuffer;
+
+ pBuffer = xalloc(sizeof(miDCBufferRec));
+ dixSetPrivate(&pDev->devPrivates, miDCSpriteKey, pBuffer);
+
+ pBuffer->pSourceGC =
+ pBuffer->pMaskGC =
+ pBuffer->pSaveGC =
+ pBuffer->pRestoreGC =
+ pBuffer->pMoveGC =
+ pBuffer->pPixSourceGC =
+ pBuffer->pPixMaskGC = NULL;
#ifdef ARGB_CURSOR
- miDCMakePicture(&pBuffer->pRootPicture, &pWin->drawable, pWin);
- if (!pBuffer->pRootPicture)
- goto failure;
-
- pBuffer->pTempPicture = NULL;
+ pBuffer->pRootPicture = NULL;
+ pBuffer->pTempPicture = NULL;
#endif
-
- // these get (re)allocated lazily depending on the cursor size
- pBuffer->pSave = pBuffer->pTemp = NULL;
- }
+ pBuffer->pSave = pBuffer->pTemp = NULL;
return TRUE;
-
-failure:
-
- miDCDeviceCleanup(pDev, pScreen);
-
- return FALSE;
}
static void
miDCDeviceCleanup(DeviceIntPtr pDev, ScreenPtr pScreen)
{
miDCBufferPtr pBuffer;
- int i;
if (DevHasCursor(pDev))
{
- for (i = 0; i < screenInfo.numScreens; i++)
- {
- pScreen = screenInfo.screens[i];
-
- pBuffer = MIDCBUFFER(pDev, pScreen);
-
- if (pBuffer)
- {
- if (pBuffer->pSourceGC) FreeGC(pBuffer->pSourceGC, (GContext) 0);
- if (pBuffer->pMaskGC) FreeGC(pBuffer->pMaskGC, (GContext) 0);
- if (pBuffer->pSaveGC) FreeGC(pBuffer->pSaveGC, (GContext) 0);
- if (pBuffer->pRestoreGC) FreeGC(pBuffer->pRestoreGC, (GContext) 0);
- if (pBuffer->pMoveGC) FreeGC(pBuffer->pMoveGC, (GContext) 0);
- if (pBuffer->pPixSourceGC) FreeGC(pBuffer->pPixSourceGC, (GContext) 0);
- if (pBuffer->pPixMaskGC) FreeGC(pBuffer->pPixMaskGC, (GContext) 0);
-
- if (pBuffer->pSave) (*pScreen->DestroyPixmap)(pBuffer->pSave);
- if (pBuffer->pTemp) (*pScreen->DestroyPixmap)(pBuffer->pTemp);
-
- free(pBuffer);
- dixSetPrivate(&pDev->devPrivates, miDCSpriteKey + pScreen->myNum, NULL);
- }
- }
+ pBuffer = MIDCBUFFER(pDev);
+ tossGC (pBuffer->pSourceGC);
+ tossGC (pBuffer->pMaskGC);
+ tossGC (pBuffer->pSaveGC);
+ tossGC (pBuffer->pRestoreGC);
+ tossGC (pBuffer->pMoveGC);
+ tossGC (pBuffer->pPixSourceGC);
+ tossGC (pBuffer->pPixMaskGC);
+ tossPix (pBuffer->pSave);
+ tossPix (pBuffer->pTemp);
+#ifdef ARGB_CURSOR
+#if 0 /* This has been free()d before */
+ tossPict (pScreenPriv->pRootPicture);
+#endif
+ tossPict (pBuffer->pTempPicture);
+#endif
+ xfree(pBuffer);
+ dixSetPrivate(&pDev->devPrivates, miDCSpriteKey, NULL);
}
}
--
1.6.0.4
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
