From: Dave Airlie <airl...@redhat.com> While developing slave output devices, I tried to enable a swcursor on the Intel device for the primary cursor, depending on whether the usb device was plugged in, and I hit a SIGIO locking problem in libdrm_intel.
I wondered why MPX didn't hit this and discovered the comment /* Hack: We mustn't call into ->MoveCursor for anything but the * VCP, as this may cause a non-HW rendered cursor to be rendered during * SIGIO. This again leads to allocs during SIGIO which leads to SIGABRT. */ This is incorrect though, as the real problem is doing rendering of any SW cursor, even the VCP cursor can be SW. So this is a first attempt to push down the logic into the DDX, It changes Move/SetCursor to return a value, and undoes the changes if necessary. It then adds an atomic to denote if the input code is running under sigio. If it is it fails to call move cursor for any sw cursors. TODO: fix SetCursor properly to fallback if in sigio input paths This at least works that I can run x11perf with the hack gone and MPX still runs fine. Signed-off-by: Dave Airlie <airl...@redhat.com> --- hw/xfree86/common/xf86Events.c | 4 ++ hw/xfree86/common/xf86Globals.c | 2 + hw/xfree86/common/xf86InPriv.h | 2 + hw/xfree86/common/xf86Priv.h | 1 + hw/xfree86/common/xf86VGAarbiter.c | 12 ++++-- hw/xfree86/common/xf86VGAarbiterPriv.h | 4 +- hw/xfree86/ramdac/xf86Cursor.c | 50 ++++++++++++++++++--------- hw/xnest/Cursor.c | 6 ++- hw/xnest/XNCursor.h | 4 +- hw/xquartz/xpr/xprCursor.c | 9 +++-- hw/xwin/wincursor.c | 6 ++- mi/mipointer.c | 57 ++++++++++++++++++++------------ mi/mipointer.h | 4 +- mi/misprite.c | 12 +++--- 14 files changed, 112 insertions(+), 61 deletions(-) diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c index 5896f22..5b1af27 100644 --- a/hw/xfree86/common/xf86Events.c +++ b/hw/xfree86/common/xf86Events.c @@ -73,6 +73,8 @@ #include "inputstr.h" #include "xf86Xinput.h" +#include "xf86InPriv.h" + #include "mi.h" #include "mipointer.h" @@ -295,8 +297,10 @@ xf86SigioReadInput(int fd, void *closure) int errno_save = errno; InputInfoPtr pInfo = closure; + input_in_sigio = 1; pInfo->read_input(pInfo); + input_in_sigio = 0; errno = errno_save; } diff --git a/hw/xfree86/common/xf86Globals.c b/hw/xfree86/common/xf86Globals.c index 0071004..44b1b79 100644 --- a/hw/xfree86/common/xf86Globals.c +++ b/hw/xfree86/common/xf86Globals.c @@ -197,3 +197,5 @@ Bool xf86VidModeAllowNonLocal = FALSE; #endif RootWinPropPtr *xf86RegisteredPropertiesTable = NULL; Bool xorgHWAccess = FALSE; + +volatile sig_atomic_t input_in_sigio; diff --git a/hw/xfree86/common/xf86InPriv.h b/hw/xfree86/common/xf86InPriv.h index 5826ac3..2fa2318 100644 --- a/hw/xfree86/common/xf86InPriv.h +++ b/hw/xfree86/common/xf86InPriv.h @@ -33,6 +33,8 @@ #ifndef _xf86InPriv_h #define _xf86InPriv_h +extern volatile sig_atomic_t input_in_sigio; + /* xf86Globals.c */ extern InputDriverPtr *xf86InputDriverList; extern int xf86NumInputDrivers; diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h index 6c5efea..dcfc60f 100644 --- a/hw/xfree86/common/xf86Priv.h +++ b/hw/xfree86/common/xf86Priv.h @@ -35,6 +35,7 @@ #ifndef _XF86PRIV_H #define _XF86PRIV_H +#include <signal.h> #include "xf86Privstr.h" #include "propertyst.h" #include "input.h" diff --git a/hw/xfree86/common/xf86VGAarbiter.c b/hw/xfree86/common/xf86VGAarbiter.c index b9b46f6..05383b0 100644 --- a/hw/xfree86/common/xf86VGAarbiter.c +++ b/hw/xfree86/common/xf86VGAarbiter.c @@ -938,25 +938,29 @@ VGAarbiterSpriteUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, return val; } -static void +static Bool VGAarbiterSpriteSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCur, int x, int y) { + Bool ret; SPRITE_PROLOG; VGAGet(pScreen); - PointPriv->spriteFuncs->SetCursor(pDev, pScreen, pCur, x, y); + ret = PointPriv->spriteFuncs->SetCursor(pDev, pScreen, pCur, x, y); VGAPut(); SPRITE_EPILOG; + return ret; } -static void +static Bool VGAarbiterSpriteMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) { + Bool ret; SPRITE_PROLOG; VGAGet(pScreen); - PointPriv->spriteFuncs->MoveCursor(pDev, pScreen, x, y); + ret = PointPriv->spriteFuncs->MoveCursor(pDev, pScreen, x, y); VGAPut(); SPRITE_EPILOG; + return ret; } static Bool diff --git a/hw/xfree86/common/xf86VGAarbiterPriv.h b/hw/xfree86/common/xf86VGAarbiterPriv.h index ebc8854..963a595 100644 --- a/hw/xfree86/common/xf86VGAarbiterPriv.h +++ b/hw/xfree86/common/xf86VGAarbiterPriv.h @@ -248,9 +248,9 @@ static Bool VGAarbiterSpriteRealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCur); static Bool VGAarbiterSpriteUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCur); -static void VGAarbiterSpriteSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, +static Bool VGAarbiterSpriteSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCur, int x, int y); -static void VGAarbiterSpriteMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, +static Bool VGAarbiterSpriteMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y); static Bool VGAarbiterDeviceCursorInitialize(DeviceIntPtr pDev, ScreenPtr pScreen); diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c index 7fd70fc..fc5883d 100644 --- a/hw/xfree86/ramdac/xf86Cursor.c +++ b/hw/xfree86/ramdac/xf86Cursor.c @@ -3,11 +3,15 @@ #include <xorg-config.h> #endif +#include <signal.h> #include "xf86.h" #include "xf86CursorPriv.h" #include "colormapst.h" #include "cursorstr.h" +#include "xf86Xinput.h" +#include "xf86InPriv.h" + /* FIXME: This was added with the ABI change of the miPointerSpriteFuncs for * MPX. * inputInfo is needed to pass the core pointer as the default argument into @@ -16,7 +20,7 @@ * Externing inputInfo is not the nice way to do it but it works. */ #include "inputstr.h" -extern InputInfo inputInfo; +//extern InputInfo inputInfo; DevPrivateKeyRec xf86CursorScreenKeyRec; @@ -24,8 +28,8 @@ DevPrivateKeyRec xf86CursorScreenKeyRec; static Bool xf86CursorRealizeCursor(DeviceIntPtr, ScreenPtr, CursorPtr); static Bool xf86CursorUnrealizeCursor(DeviceIntPtr, ScreenPtr, CursorPtr); -static void xf86CursorSetCursor(DeviceIntPtr, ScreenPtr, CursorPtr, int, int); -static void xf86CursorMoveCursor(DeviceIntPtr, ScreenPtr, int, int); +static Bool xf86CursorSetCursor(DeviceIntPtr, ScreenPtr, CursorPtr, int, int); +static Bool xf86CursorMoveCursor(DeviceIntPtr, ScreenPtr, int, int); static Bool xf86DeviceCursorInitialize(DeviceIntPtr, ScreenPtr); static void xf86DeviceCursorCleanup(DeviceIntPtr, ScreenPtr); @@ -296,7 +300,7 @@ xf86CursorUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs) return (*ScreenPriv->spriteFuncs->UnrealizeCursor) (pDev, pScreen, pCurs); } -static void +static Bool xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs, int x, int y) { @@ -304,11 +308,13 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs, (xf86CursorScreenPtr) dixLookupPrivate(&pScreen->devPrivates, xf86CursorScreenKey); xf86CursorInfoPtr infoPtr = ScreenPriv->CursorInfoPtr; + Bool ret; if (pCurs == NullCursor) { /* means we're supposed to remove the cursor */ + ret = TRUE; if (ScreenPriv->SWCursor || !(GetMaster(pDev, MASTER_POINTER) == inputInfo.pointer)) - (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, NullCursor, x, + ret = (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, NullCursor, x, y); else if (ScreenPriv->isUp) { xf86SetCursor(pScreen, NullCursor, x, y); @@ -317,7 +323,7 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs, if (ScreenPriv->CurrentCursor) FreeCursor(ScreenPriv->CurrentCursor, None); ScreenPriv->CurrentCursor = NullCursor; - return; + return ret; } /* only update for VCP, otherwise we get cursor jumps when removing a @@ -359,15 +365,15 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs, (pCurs->bits->height <= infoPtr->MaxHeight) && (pCurs->bits->width <= infoPtr->MaxWidth) && (!infoPtr->UseHWCursor || (*infoPtr->UseHWCursor) (pScreen, pCurs)))))) { if (ScreenPriv->SWCursor) /* remove the SW cursor */ - (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, - NullCursor, x, y); + ret = (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, + NullCursor, x, y); xf86SetCursor(pScreen, pCurs, x, y); ScreenPriv->SWCursor = FALSE; ScreenPriv->isUp = TRUE; miPointerSetWaitForUpdate(pScreen, !infoPtr->pScrn->silkenMouse); - return; + return TRUE; } miPointerSetWaitForUpdate(pScreen, TRUE); @@ -391,16 +397,16 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs, if (pCurs->bits->emptyMask && !ScreenPriv->showTransparent) pCurs = NullCursor; - (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, pCurs, x, y); + return (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, pCurs, x, y); } -static void +static Bool xf86CursorMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) { xf86CursorScreenPtr ScreenPriv = (xf86CursorScreenPtr) dixLookupPrivate(&pScreen->devPrivates, xf86CursorScreenKey); - + Bool ret; /* only update coordinate state for first sprite, otherwise we get jumps when removing a sprite. The second sprite is never HW rendered anyway */ if (GetMaster(pDev, MASTER_POINTER) == inputInfo.pointer) { @@ -408,15 +414,25 @@ xf86CursorMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) ScreenPriv->y = y; if (ScreenPriv->CursorToRestore) - xf86CursorSetCursor(pDev, pScreen, ScreenPriv->CursorToRestore, x, + ret = xf86CursorSetCursor(pDev, pScreen, ScreenPriv->CursorToRestore, x, y); else if (ScreenPriv->SWCursor) - (*ScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y); - else if (ScreenPriv->isUp) + if (input_in_sigio == 1) + ret = FALSE; + else + ret = (*ScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y); + else if (ScreenPriv->isUp) { xf86MoveCursor(pScreen, x, y); + ret = TRUE; + } + return ret; + } + else { + if (input_in_sigio) + return FALSE; + else + return (*ScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y); } - else - (*ScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y); } void diff --git a/hw/xnest/Cursor.c b/hw/xnest/Cursor.c index 285e10e..76900d1 100644 --- a/hw/xnest/Cursor.c +++ b/hw/xnest/Cursor.c @@ -134,7 +134,7 @@ xnestRecolorCursor(ScreenPtr pScreen, CursorPtr pCursor, Bool displayed) xnestCursor(pCursor, pScreen), &fg_color, &bg_color); } -void +Bool xnestSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, int x, int y) { @@ -143,11 +143,13 @@ xnestSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, int x, xnestDefaultWindows[pScreen->myNum], xnestCursor(pCursor, pScreen)); } + return TRUE; } -void +Bool xnestMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) { + return TRUE; } Bool diff --git a/hw/xnest/XNCursor.h b/hw/xnest/XNCursor.h index 1a3c6f4..6016cc7 100644 --- a/hw/xnest/XNCursor.h +++ b/hw/xnest/XNCursor.h @@ -44,9 +44,9 @@ Bool xnestRealizeCursor(DeviceIntPtr pDev, Bool xnestUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor); void xnestRecolorCursor(ScreenPtr pScreen, CursorPtr pCursor, Bool displayed); -void xnestSetCursor(DeviceIntPtr pDev, +Bool xnestSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, int x, int y); -void xnestMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y); +Bool xnestMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y); Bool xnestDeviceCursorInitialize(DeviceIntPtr pDev, ScreenPtr pScreen); void xnestDeviceCursorCleanup(DeviceIntPtr pDev, ScreenPtr pScreen); #endif /* XNESTCURSOR_H */ diff --git a/hw/xquartz/xpr/xprCursor.c b/hw/xquartz/xpr/xprCursor.c index 0392a46..91d5380 100644 --- a/hw/xquartz/xpr/xprCursor.c +++ b/hw/xquartz/xpr/xprCursor.c @@ -219,7 +219,7 @@ QuartzUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) * QuartzSetCursor * Set the cursor sprite and position. */ -static void +static Bool QuartzSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, int x, int y) @@ -243,15 +243,18 @@ QuartzSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, ScreenPriv->cursorVisible = TRUE; } } + return TRUE; } /* * QuartzMoveCursor * Move the cursor. This is a noop for us. */ -static void +static Bool QuartzMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) -{} +{ + return TRUE; +} /* =========================================================================== diff --git a/hw/xwin/wincursor.c b/hw/xwin/wincursor.c index b56104f..d86103e 100644 --- a/hw/xwin/wincursor.c +++ b/hw/xwin/wincursor.c @@ -457,7 +457,7 @@ winUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) * winSetCursor * Set the cursor sprite and position. */ -static void +static Bool winSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, int x, int y) { @@ -520,15 +520,17 @@ winSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, int x, pScreenPriv->cursor.visible = TRUE; } } + return TRUE; } /* * winMoveCursor * Move the cursor. This is a noop for us. */ -static void +static Bool winMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) { + return TRUE; } static Bool diff --git a/mi/mipointer.c b/mi/mipointer.c index de6698a..d70c081 100644 --- a/mi/mipointer.c +++ b/mi/mipointer.c @@ -396,6 +396,7 @@ miPointerUpdateSprite(DeviceIntPtr pDev) CursorPtr pCursor; int x, y, devx, devy; miPointerPtr pPointer; + Bool ret; if (!pDev || !pDev->coreEvents) return; @@ -432,12 +433,14 @@ miPointerUpdateSprite(DeviceIntPtr pDev) FALSE); } (*pScreenPriv->screenFuncs->CrossScreen) (pScreen, TRUE); - (*pScreenPriv->spriteFuncs->SetCursor) + ret = (*pScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, pPointer->pCursor, x, y); - pPointer->devx = x; - pPointer->devy = y; - pPointer->pSpriteCursor = pPointer->pCursor; - pPointer->pSpriteScreen = pScreen; + if (ret == FALSE) { + pPointer->devx = x; + pPointer->devy = y; + pPointer->pSpriteCursor = pPointer->pCursor; + pPointer->pSpriteScreen = pScreen; + } } /* * if the cursor has changed, display the new one @@ -447,17 +450,26 @@ miPointerUpdateSprite(DeviceIntPtr pDev) if (!pCursor || (pCursor->bits->emptyMask && !pScreenPriv->showTransparent)) pCursor = NullCursor; - (*pScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, pCursor, x, y); - - pPointer->devx = x; - pPointer->devy = y; - pPointer->pSpriteCursor = pPointer->pCursor; + ret = (*pScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen, pCursor, x, y); + if (ret == TRUE) { + pPointer->devx = x; + pPointer->devy = y; + pPointer->pSpriteCursor = pPointer->pCursor; + } } else if (x != devx || y != devy) { + int old_devx, old_devy; + old_devx = pPointer->devx; + old_devy = pPointer->devy; pPointer->devx = x; pPointer->devy = y; - if (pPointer->pCursor && !pPointer->pCursor->bits->emptyMask) - (*pScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y); + if (pPointer->pCursor && !pPointer->pCursor->bits->emptyMask) { + ret = (*pScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y); + if (ret == FALSE) { + pPointer->devx = old_devx; + pPointer->devy = old_devy; + } + } } } @@ -527,21 +539,24 @@ static void miPointerMoveNoEvent(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) { miPointerPtr pPointer; - + Bool ret; SetupScreen(pScreen); pPointer = MIPOINTER(pDev); - /* Hack: We mustn't call into ->MoveCursor for anything but the - * VCP, as this may cause a non-HW rendered cursor to be rendered during - * SIGIO. This again leads to allocs during SIGIO which leads to SIGABRT. - */ - if (GetMaster(pDev, MASTER_POINTER) == inputInfo.pointer - &&!pScreenPriv->waitForUpdate && pScreen == pPointer->pSpriteScreen) { + { + int old_devx, old_devy; + old_devx = pPointer->devx; + old_devy = pPointer->devy; pPointer->devx = x; pPointer->devy = y; - if (pPointer->pCursor && !pPointer->pCursor->bits->emptyMask) - (*pScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y); + if (pPointer->pCursor && !pPointer->pCursor->bits->emptyMask) { + ret = (*pScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y); + if (ret == FALSE) { + pPointer->devx = old_devx; + pPointer->devy = old_devy; + } + } } pPointer->x = x; diff --git a/mi/mipointer.h b/mi/mipointer.h index 1500e21..37f3269 100644 --- a/mi/mipointer.h +++ b/mi/mipointer.h @@ -39,13 +39,13 @@ typedef struct _miPointerSpriteFuncRec { ScreenPtr /* pScr */ , CursorPtr /* pCurs */ ); - void (*SetCursor) (DeviceIntPtr /* pDev */ , + Bool (*SetCursor) (DeviceIntPtr /* pDev */ , ScreenPtr /* pScr */ , CursorPtr /* pCurs */ , int /* x */ , int /* y */ ); - void (*MoveCursor) (DeviceIntPtr /* pDev */ , + Bool (*MoveCursor) (DeviceIntPtr /* pDev */ , ScreenPtr /* pScr */ , int /* x */ , int /* y */ diff --git a/mi/misprite.c b/mi/misprite.c index c9fcabc..90b88cf 100644 --- a/mi/misprite.c +++ b/mi/misprite.c @@ -227,9 +227,9 @@ static Bool miSpriteRealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor); static Bool miSpriteUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor); -static void miSpriteSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, +static Bool miSpriteSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, int x, int y); -static void miSpriteMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, +static Bool miSpriteMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y); miPointerSpriteFuncRec miSpritePointerFuncs = { @@ -725,7 +725,7 @@ miSpriteUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) return miDCUnrealizeCursor(pScreen, pCursor); } -static void +static Bool miSpriteSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor, int x, int y) { @@ -777,10 +777,10 @@ miSpriteSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, miSpriteSaveUnderCursor(pDev, pScreen); miSpriteRestoreCursor(pDev, pScreen); } - + return TRUE; } -static void +static Bool miSpriteMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) { CursorPtr pCursor; @@ -790,7 +790,7 @@ miSpriteMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y) pCursor = MISPRITE(pDev)->pCursor; - miSpriteSetCursor(pDev, pScreen, pCursor, x, y); + return miSpriteSetCursor(pDev, pScreen, pCursor, x, y); } static Bool -- 1.7.6 _______________________________________________ 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