From ba00ee027ffabfb2407aace261c8593ea12932d9 Mon Sep 17 00:00:00 2001 From: Adam Jackson Date: Thu, 26 Oct 2017 13:40:57 -0400 Subject: [PATCH xserver 1/4] animcur: Use fixed-size screen private X-NVConfidentiality: public Reviewed-by: Robert Morell Tested-by: Robert Morell Signed-off-by: Adam Jackson (cherry picked from commit 3abbdb7318018584a27220737bd92081ce8ee67c) (cherry picked from commit 26841b2c9ea03fda8b2d0da254e0344fd2a3afce) --- render/animcur.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/render/animcur.c b/render/animcur.c index 52e6b8b79fe5..3f85f9a4f6ce 100644 --- a/render/animcur.c +++ b/render/animcur.c @@ -77,12 +77,9 @@ static CursorBits animCursorBits = { static DevPrivateKeyRec AnimCurScreenPrivateKeyRec; -#define AnimCurScreenPrivateKey (&AnimCurScreenPrivateKeyRec) - #define IsAnimCur(c) ((c) && ((c)->bits == &animCursorBits)) #define GetAnimCur(c) ((AnimCurPtr) ((((char *)(c) + CURSOR_REC_SIZE)))) -#define GetAnimCurScreen(s) ((AnimCurScreenPtr)dixLookupPrivate(&(s)->devPrivates, AnimCurScreenPrivateKey)) -#define SetAnimCurScreen(s,p) dixSetPrivate(&(s)->devPrivates, AnimCurScreenPrivateKey, p) +#define GetAnimCurScreen(s) ((AnimCurScreenPtr)dixLookupPrivate(&(s)->devPrivates, &AnimCurScreenPrivateKeyRec)) #define Wrap(as,s,elt,func) (((as)->elt = (s)->elt), (s)->elt = func) #define Unwrap(as,s,elt) ((s)->elt = (as)->elt) @@ -101,9 +98,7 @@ AnimCurCloseScreen(ScreenPtr pScreen) Unwrap(as, pScreen, RealizeCursor); Unwrap(as, pScreen, UnrealizeCursor); Unwrap(as, pScreen, RecolorCursor); - SetAnimCurScreen(pScreen, 0); ret = (*pScreen->CloseScreen) (pScreen); - free(as); return ret; } @@ -308,15 +303,13 @@ AnimCurInit(ScreenPtr pScreen) { AnimCurScreenPtr as; - if (!dixRegisterPrivateKey(&AnimCurScreenPrivateKeyRec, PRIVATE_SCREEN, 0)) + if (!dixRegisterPrivateKey(&AnimCurScreenPrivateKeyRec, PRIVATE_SCREEN, + sizeof(AnimCurScreenRec))) return FALSE; - as = (AnimCurScreenPtr) malloc(sizeof(AnimCurScreenRec)); - if (!as) - return FALSE; + as = GetAnimCurScreen(pScreen); as->timer = TimerSet(NULL, TimerAbsolute, 0, AnimCurTimerNotify, pScreen); if (!as->timer) { - free(as); return FALSE; } as->timer_set = FALSE; @@ -329,7 +322,6 @@ AnimCurInit(ScreenPtr pScreen) Wrap(as, pScreen, RealizeCursor, AnimCurRealizeCursor); Wrap(as, pScreen, UnrealizeCursor, AnimCurUnrealizeCursor); Wrap(as, pScreen, RecolorCursor, AnimCurRecolorCursor); - SetAnimCurScreen(pScreen, as); return TRUE; } -- 2.16.1 From 879fbcf641a1f4a1da29fe9732779b4645f37555 Mon Sep 17 00:00:00 2001 From: Adam Jackson Date: Thu, 26 Oct 2017 13:53:06 -0400 Subject: [PATCH xserver 2/4] animcur: Return the next interval directly from the timer callback X-NVConfidentiality: public If the return value is non-zero here, DoTimer() will automatically rearm the timer for the new (relative) delay. 'soonest' is in absolute time, so subtract off 'now' and return that. Reviewed-by: Robert Morell Tested-by: Robert Morell Signed-off-by: Adam Jackson (cherry picked from commit cc3241a712684f8c7147f5688e9ee3ecb5a93b87) (cherry picked from commit 354c48304d27f75b7c33c03a0adb050c37788ccf) --- render/animcur.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/render/animcur.c b/render/animcur.c index 3f85f9a4f6ce..26a6026ae40e 100644 --- a/render/animcur.c +++ b/render/animcur.c @@ -169,10 +169,9 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void *arg) } if (activeDevice) - TimerSet(as->timer, TimerAbsolute, soonest, AnimCurTimerNotify, pScreen); - else - as->timer_set = FALSE; + return soonest - now; + as->timer_set = FALSE; return 0; } -- 2.16.1 From 9cbd0d84607ee19fae82ac6c1ecee0c5958966a0 Mon Sep 17 00:00:00 2001 From: Adam Jackson Date: Thu, 26 Oct 2017 15:24:39 -0400 Subject: [PATCH xserver 3/4] animcur: Run the timer from the device, not the screen X-NVConfidentiality: public This is very slightly more efficient since the callback now doesn't need to walk every input device, instead we know exactly which device's cursor is being updated. AnimCurTimerNotify() gets outdented nicely as a result. A more important side effect is that we can stop using the TimerAbsolute mode and just pass in the relative delay. In AnimCurSetCursorPosition, we no longer need to rearm the timer with the new screen; it is enough to update the device's state. In AnimCurDisplayCursor we need to notice when we're switching from animated cursor to regular and cancel the existing timer. Reviewed-by: Robert Morell Tested-by: Robert Morell Signed-off-by: Adam Jackson (cherry picked from commit 094a63d56fbfb9e23210cc9ac538fb198af37cee) (cherry picked from commit 693f0e21d55d6e9fe792d91e76e4168aa813db71) --- render/animcur.c | 87 +++++++++++++++++++------------------------------------- 1 file changed, 29 insertions(+), 58 deletions(-) diff --git a/render/animcur.c b/render/animcur.c index 26a6026ae40e..9393b4018dd3 100644 --- a/render/animcur.c +++ b/render/animcur.c @@ -55,6 +55,7 @@ typedef struct _AnimCurElt { typedef struct _AnimCur { int nelt; /* number of elements in the elts array */ AnimCurElt *elts; /* actually allocated right after the structure */ + OsTimerPtr timer; } AnimCurRec, *AnimCurPtr; typedef struct _AnimScrPriv { @@ -65,8 +66,6 @@ typedef struct _AnimScrPriv { RealizeCursorProcPtr RealizeCursor; UnrealizeCursorProcPtr UnrealizeCursor; RecolorCursorProcPtr RecolorCursor; - OsTimerPtr timer; - Bool timer_set; } AnimCurScreenRec, *AnimCurScreenPtr; static unsigned char empty[4]; @@ -130,49 +129,27 @@ AnimCurCursorLimits(DeviceIntPtr pDev, static CARD32 AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void *arg) { - ScreenPtr pScreen = arg; + DeviceIntPtr dev = arg; + ScreenPtr pScreen = dev->spriteInfo->anim.pScreen; AnimCurScreenPtr as = GetAnimCurScreen(pScreen); - DeviceIntPtr dev; - Bool activeDevice = FALSE; - CARD32 soonest = ~0; /* earliest time to wakeup again */ - - for (dev = inputInfo.devices; dev; dev = dev->next) { - if (IsPointerDevice(dev) && pScreen == dev->spriteInfo->anim.pScreen) { - if (!activeDevice) - activeDevice = TRUE; - - if ((INT32) (now - dev->spriteInfo->anim.time) >= 0) { - AnimCurPtr ac = GetAnimCur(dev->spriteInfo->anim.pCursor); - int elt = (dev->spriteInfo->anim.elt + 1) % ac->nelt; - DisplayCursorProcPtr DisplayCursor; - - /* - * Not a simple Unwrap/Wrap as this - * isn't called along the DisplayCursor - * wrapper chain. - */ - DisplayCursor = pScreen->DisplayCursor; - pScreen->DisplayCursor = as->DisplayCursor; - (void) (*pScreen->DisplayCursor) (dev, - pScreen, - ac->elts[elt].pCursor); - as->DisplayCursor = pScreen->DisplayCursor; - pScreen->DisplayCursor = DisplayCursor; - - dev->spriteInfo->anim.elt = elt; - dev->spriteInfo->anim.time = now + ac->elts[elt].delay; - } - if (soonest > dev->spriteInfo->anim.time) - soonest = dev->spriteInfo->anim.time; - } - } + AnimCurPtr ac = GetAnimCur(dev->spriteInfo->anim.pCursor); + int elt = (dev->spriteInfo->anim.elt + 1) % ac->nelt; + DisplayCursorProcPtr DisplayCursor = pScreen->DisplayCursor; - if (activeDevice) - return soonest - now; + /* + * Not a simple Unwrap/Wrap as this isn't called along the DisplayCursor + * wrapper chain. + */ + pScreen->DisplayCursor = as->DisplayCursor; + (void) (*pScreen->DisplayCursor) (dev, pScreen, ac->elts[elt].pCursor); + as->DisplayCursor = pScreen->DisplayCursor; + pScreen->DisplayCursor = DisplayCursor; - as->timer_set = FALSE; - return 0; + dev->spriteInfo->anim.elt = elt; + dev->spriteInfo->anim.time = now + ac->elts[elt].delay; + + return ac->elts[elt].delay; } static Bool @@ -198,17 +175,19 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) pDev->spriteInfo->anim.pCursor = pCursor; pDev->spriteInfo->anim.pScreen = pScreen; - if (!as->timer_set) { - TimerSet(as->timer, TimerAbsolute, pDev->spriteInfo->anim.time, - AnimCurTimerNotify, pScreen); - as->timer_set = TRUE; - } + ac->timer = TimerSet(ac->timer, 0, ac->elts[0].delay, + AnimCurTimerNotify, pDev); } } else ret = TRUE; } else { + CursorPtr old = pDev->spriteInfo->anim.pCursor; + + if (old && IsAnimCur(old)) + TimerCancel(GetAnimCur(old)->timer); + pDev->spriteInfo->anim.pCursor = 0; pDev->spriteInfo->anim.pScreen = 0; ret = (*pScreen->DisplayCursor) (pDev, pScreen, pCursor); @@ -227,12 +206,6 @@ AnimCurSetCursorPosition(DeviceIntPtr pDev, Unwrap(as, pScreen, SetCursorPosition); if (pDev->spriteInfo->anim.pCursor) { pDev->spriteInfo->anim.pScreen = pScreen; - - if (!as->timer_set) { - TimerSet(as->timer, TimerAbsolute, pDev->spriteInfo->anim.time, - AnimCurTimerNotify, pScreen); - as->timer_set = TRUE; - } } ret = (*pScreen->SetCursorPosition) (pDev, pScreen, x, y, generateEvent); Wrap(as, pScreen, SetCursorPosition, AnimCurSetCursorPosition); @@ -307,11 +280,6 @@ AnimCurInit(ScreenPtr pScreen) return FALSE; as = GetAnimCurScreen(pScreen); - as->timer = TimerSet(NULL, TimerAbsolute, 0, AnimCurTimerNotify, pScreen); - if (!as->timer) { - return FALSE; - } - as->timer_set = FALSE; Wrap(as, pScreen, CloseScreen, AnimCurCloseScreen); @@ -359,10 +327,14 @@ AnimCursorCreate(CursorPtr *cursors, CARD32 *deltas, int ncursor, pCursor->id = cid; + ac = GetAnimCur(pCursor); + ac->timer = TimerSet(NULL, 0, 0, AnimCurTimerNotify, NULL); + /* security creation/labeling check */ rc = XaceHook(XACE_RESOURCE_ACCESS, client, cid, RT_CURSOR, pCursor, RT_NONE, NULL, DixCreateAccess); if (rc != Success) { + TimerFree(ac->timer); dixFiniPrivates(pCursor, PRIVATE_CURSOR); free(pCursor); return rc; @@ -372,7 +344,6 @@ AnimCursorCreate(CursorPtr *cursors, CARD32 *deltas, int ncursor, * Fill in the AnimCurRec */ animCursorBits.refcnt++; - ac = GetAnimCur(pCursor); ac->nelt = ncursor; ac->elts = (AnimCurElt *) (ac + 1); -- 2.16.1 From 9385190f06012aaa897cedf57a3c6455863ab1bc Mon Sep 17 00:00:00 2001 From: Adam Jackson Date: Tue, 9 Jan 2018 10:54:05 -0500 Subject: [PATCH xserver 4/4] animcur: Fix transitions between animated cursors X-NVConfidentiality: public We weren't cancelling the old timer when changing cursors, making things go all crashy. Logically we could always cancel the timer first, but then we'd have to call TimerSet to re-arm ourselves, and GetTimeInMillis is potentially expensive. Reported-by: https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230967/#5230967 Signed-off-by: Adam Jackson Reviewed-by: Aaron Plattner Tested-by: Aaron Plattner (cherry picked from commit de60245e05c0d2528d4ff42557a044387e53315c) (cherry picked from commit 5e83ebd76738455c443a66024b0b5eb92930b36c) --- render/animcur.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/render/animcur.c b/render/animcur.c index 9393b4018dd3..e585a8f2379e 100644 --- a/render/animcur.c +++ b/render/animcur.c @@ -152,11 +152,20 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void *arg) return ac->elts[elt].delay; } +static void +AnimCurCancelTimer(DeviceIntPtr pDev) +{ + CursorPtr cur = pDev->spriteInfo->anim.pCursor; + + if (IsAnimCur(cur)) + TimerCancel(GetAnimCur(cur)->timer); +} + static Bool AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) { AnimCurScreenPtr as = GetAnimCurScreen(pScreen); - Bool ret; + Bool ret = TRUE; if (IsFloating(pDev)) return FALSE; @@ -166,8 +175,10 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) if (pCursor != pDev->spriteInfo->anim.pCursor) { AnimCurPtr ac = GetAnimCur(pCursor); - ret = (*pScreen->DisplayCursor) - (pDev, pScreen, ac->elts[0].pCursor); + AnimCurCancelTimer(pDev); + ret = (*pScreen->DisplayCursor) (pDev, pScreen, + ac->elts[0].pCursor); + if (ret) { pDev->spriteInfo->anim.elt = 0; pDev->spriteInfo->anim.time = @@ -179,15 +190,9 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) AnimCurTimerNotify, pDev); } } - else - ret = TRUE; } else { - CursorPtr old = pDev->spriteInfo->anim.pCursor; - - if (old && IsAnimCur(old)) - TimerCancel(GetAnimCur(old)->timer); - + AnimCurCancelTimer(pDev); pDev->spriteInfo->anim.pCursor = 0; pDev->spriteInfo->anim.pScreen = 0; ret = (*pScreen->DisplayCursor) (pDev, pScreen, pCursor); -- 2.16.1