From 5c7cfd4c6978834551848e1be62af240102e39b5 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 15 Oct 2013 10:11:20 +1000 Subject: [PATCH 1/7] sync: compress two if statements No functional changes, just merges a > and == condition into a >= condition. Signed-off-by: Peter Hutterer Reviewed-by: Adam Jackson Reviewed-by: Keith Packard --- Xext/sync.c | 51 +++++++++++++++++++-------------------------------- 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/Xext/sync.c b/Xext/sync.c index 9ae5b3981..f93ac18aa 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -1031,42 +1031,29 @@ SyncComputeBracketValues(SyncCounter * pCounter) } else if (pTrigger->test_type == XSyncNegativeTransition && ct != XSyncCounterNeverIncreases) { - if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) && - XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less)) - { - psci->bracket_less = pTrigger->test_value; - pnewltval = &psci->bracket_less; - } - else if (XSyncValueEqual(pCounter->value, pTrigger->test_value) && - XSyncValueGreaterThan(pTrigger->test_value, - psci->bracket_less)) { - /* - * The value is exactly equal to our threshold. We want one - * more event in the negative direction to ensure we pick up - * when the value is less than this threshold. - */ - psci->bracket_less = pTrigger->test_value; - pnewltval = &psci->bracket_less; + if (XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value) && + XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less)) { + /* + * If the value is exactly equal to our threshold, we want one + * more event in the negative direction to ensure we pick up + * when the value is less than this threshold. + */ + psci->bracket_less = pTrigger->test_value; + pnewltval = &psci->bracket_less; } } else if (pTrigger->test_type == XSyncPositiveTransition && ct != XSyncCounterNeverDecreases) { - if (XSyncValueLessThan(pCounter->value, pTrigger->test_value) && - XSyncValueLessThan(pTrigger->test_value, psci->bracket_greater)) - { - psci->bracket_greater = pTrigger->test_value; - pnewgtval = &psci->bracket_greater; - } - else if (XSyncValueEqual(pCounter->value, pTrigger->test_value) && - XSyncValueLessThan(pTrigger->test_value, - psci->bracket_greater)) { - /* - * The value is exactly equal to our threshold. We want one - * more event in the positive direction to ensure we pick up - * when the value *exceeds* this threshold. - */ - psci->bracket_greater = pTrigger->test_value; - pnewgtval = &psci->bracket_greater; + if (XSyncValueLessOrEqual(pCounter->value, pTrigger->test_value) && + XSyncValueLessThan(pTrigger->test_value, psci->bracket_greater)) { + /* + * If the value is exactly equal to our threshold, we + * want one more event in the positive direction to + * ensure we pick up when the value *exceeds* this + * threshold. + */ + psci->bracket_greater = pTrigger->test_value; + pnewgtval = &psci->bracket_greater; } } } /* end for each trigger */ From 2efe49c1029f959fe80879bcf50df42e8b80451d Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 16 Oct 2013 13:01:01 +1000 Subject: [PATCH 2/7] sync: always call BracketValues when recalculating upper/lower brackets Both ServertimeBracketValues and IdleTimeBracketValues copy the value into there SysCounter privates. Call it for a NULL set as well, so we don't end up with stale pointers and we can remove the block/wakeup handlers. Signed-off-by: Peter Hutterer Reviewed-by: Adam Jackson Reviewed-by: Keith Packard --- Xext/sync.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Xext/sync.c b/Xext/sync.c index f93ac18aa..eaf063741 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -1058,9 +1058,8 @@ SyncComputeBracketValues(SyncCounter * pCounter) } } /* end for each trigger */ - if (pnewgtval || pnewltval) { - (*psci->BracketValues) ((pointer) pCounter, pnewltval, pnewgtval); - } + (*psci->BracketValues) ((pointer) pCounter, pnewltval, pnewgtval); + } /* From b7c9bd9cf276e92a73be57ff2ed32b47a80f13fb Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 16 Oct 2013 09:21:47 +1000 Subject: [PATCH 3/7] sync: supply the counter from IdleTimeBlockHandler The main idletime counter has an initialized deviceid, might as well be supplying it properly. Without this, we'd only ever check the XIAllDevices counter, so the wait would never be adjusted for the device-specific triggers. Signed-off-by: Peter Hutterer Reviewed-by: Adam Jackson Reviewed-by: Keith Packard --- Xext/sync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Xext/sync.c b/Xext/sync.c index eaf063741..f8eb02a60 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -2624,7 +2624,7 @@ IdleTimeBlockHandler(pointer pCounter, struct timeval **wt, pointer LastSelectMa return; old_idle = counter->value; - IdleTimeQueryValue(NULL, &idle); + IdleTimeQueryValue(counter, &idle); counter->value = idle; /* push, so CheckTrigger works */ if (less && XSyncValueLessOrEqual(idle, *less)) { From efc1035ca958f2c9d266338a308518a0834b1773 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 16 Oct 2013 09:36:01 +1000 Subject: [PATCH 4/7] dix: provide accessor methods for the last device event time And now that we have the accessors, localize it. No functional changes, just preparing for a future change. Signed-off-by: Peter Hutterer Reviewed-by: Adam Jackson Reviewed-by: Keith Packard --- Xext/saver.c | 6 ++---- Xext/sync.c | 2 +- dix/events.c | 43 +++++++++++++++++++++++++++++++------------ dix/globals.c | 1 - dix/window.c | 4 +--- include/dix.h | 5 +++++ include/dixstruct.h | 1 - os/WaitFor.c | 2 +- os/xdmcp.c | 2 +- 9 files changed, 42 insertions(+), 24 deletions(-) diff --git a/Xext/saver.c b/Xext/saver.c index fe81bc4d7..e06f40837 100644 --- a/Xext/saver.c +++ b/Xext/saver.c @@ -392,9 +392,7 @@ ScreenSaverFreeSuspend(pointer value, XID id) DeviceIntPtr dev; UpdateCurrentTimeIf(); nt_list_for_each_entry(dev, inputInfo.devices, next) - lastDeviceEventTime[dev->id] = currentTime; - lastDeviceEventTime[XIAllDevices] = currentTime; - lastDeviceEventTime[XIAllMasterDevices] = currentTime; + NoticeTime(dev, currentTime); SetScreenSaverTimer(); } } @@ -681,7 +679,7 @@ ProcScreenSaverQueryInfo(ClientPtr client) pPriv = GetScreenPrivate(pDraw->pScreen); UpdateCurrentTime(); - lastInput = GetTimeInMillis() - lastDeviceEventTime[XIAllDevices].milliseconds; + lastInput = GetTimeInMillis() - LastEventTime(XIAllDevices).milliseconds; rep = (xScreenSaverQueryInfoReply) { .type = X_Reply, diff --git a/Xext/sync.c b/Xext/sync.c index f8eb02a60..ed891b169 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -2605,7 +2605,7 @@ IdleTimeQueryValue(pointer pCounter, CARD64 * pValue_return) } else deviceid = XIAllDevices; - idle = GetTimeInMillis() - lastDeviceEventTime[deviceid].milliseconds; + idle = GetTimeInMillis() - LastEventTime(deviceid).milliseconds; XSyncIntsToValue(pValue_return, idle, 0); } diff --git a/dix/events.c b/dix/events.c index 7a1b1c3c6..c803721f3 100644 --- a/dix/events.c +++ b/dix/events.c @@ -262,6 +262,8 @@ InputInfo inputInfo; EventSyncInfoRec syncEvents; +static TimeStamp lastDeviceEventTime[MAXDEVICES]; + /** * The root window the given device is currently on. */ @@ -1043,33 +1045,47 @@ XineramaGetCursorScreen(DeviceIntPtr pDev) #define TIMESLOP (5 * 60 * 1000) /* 5 minutes */ static void -MonthChangedOrBadTime(InternalEvent *ev) +MonthChangedOrBadTime(CARD32 *ms) { /* If the ddx/OS is careless about not processing timestamped events from * different sources in sorted order, then it's possible for time to go * backwards when it should not. Here we ensure a decent time. */ - if ((currentTime.milliseconds - ev->any.time) > TIMESLOP) + if ((currentTime.milliseconds - *ms) > TIMESLOP) currentTime.months++; else - ev->any.time = currentTime.milliseconds; + *ms = currentTime.milliseconds; +} + +void +NoticeTime(const DeviceIntPtr dev, TimeStamp time) +{ + lastDeviceEventTime[XIAllDevices] = currentTime; + lastDeviceEventTime[dev->id] = currentTime; } static void -NoticeTime(InternalEvent *ev, DeviceIntPtr dev) +NoticeTimeMillis(const DeviceIntPtr dev, CARD32 *ms) { - if (ev->any.time < currentTime.milliseconds) - MonthChangedOrBadTime(ev); - currentTime.milliseconds = ev->any.time; - lastDeviceEventTime[XIAllDevices] = currentTime; - lastDeviceEventTime[dev->id] = currentTime; + TimeStamp time; + if (*ms < currentTime.milliseconds) + MonthChangedOrBadTime(ms); + time.months = currentTime.months; + time.milliseconds = *ms; + NoticeTime(dev, time); } void NoticeEventTime(InternalEvent *ev, DeviceIntPtr dev) { if (!syncEvents.playingEvents) - NoticeTime(ev, dev); + NoticeTimeMillis(dev, &ev->any.time); +} + +TimeStamp +LastEventTime(int deviceid) +{ + return lastDeviceEventTime[deviceid]; } /************************************************************************** @@ -1093,7 +1109,7 @@ EnqueueEvent(InternalEvent *ev, DeviceIntPtr device) if (!xorg_list_is_empty(&syncEvents.pending)) tail = xorg_list_last_entry(&syncEvents.pending, QdEventRec, next); - NoticeTime((InternalEvent *)event, device); + NoticeTimeMillis(device, &ev->any.time); /* Fix for key repeating bug. */ if (device->key != NULL && device->key->xkbInfo != NULL && @@ -5276,8 +5292,11 @@ InitEvents(void) inputInfo.pointer = (DeviceIntPtr) NULL; for (i = 0; i < MAXDEVICES; i++) { + DeviceIntRec dummy; memcpy(&event_filters[i], default_filter, sizeof(default_filter)); - lastDeviceEventTime[i] = currentTime; + + dummy.id = i; + NoticeTime(&dummy, currentTime); } syncEvents.replayDev = (DeviceIntPtr) NULL; diff --git a/dix/globals.c b/dix/globals.c index 332b91f5c..ad9145b01 100644 --- a/dix/globals.c +++ b/dix/globals.c @@ -122,7 +122,6 @@ Bool party_like_its_1989 = FALSE; Bool whiteRoot = FALSE; TimeStamp currentTime; -TimeStamp lastDeviceEventTime[MAXDEVICES]; int defaultColorVisualClass = -1; int monitorResolution = 0; diff --git a/dix/window.c b/dix/window.c index cff341b65..92df1eb4c 100644 --- a/dix/window.c +++ b/dix/window.c @@ -3089,9 +3089,7 @@ dixSaveScreens(ClientPtr client, int on, int mode) DeviceIntPtr dev; UpdateCurrentTimeIf(); nt_list_for_each_entry(dev, inputInfo.devices, next) - lastDeviceEventTime[dev->id] = currentTime; - lastDeviceEventTime[XIAllDevices] = currentTime; - lastDeviceEventTime[XIAllMasterDevices] = currentTime; + NoticeTime(dev, currentTime); } SetScreenSaverTimer(); } diff --git a/include/dix.h b/include/dix.h index 171e56e11..fd2490fbd 100644 --- a/include/dix.h +++ b/include/dix.h @@ -314,9 +314,14 @@ GetCurrentRootWindow(DeviceIntPtr pDev); extern _X_EXPORT WindowPtr GetSpriteWindow(DeviceIntPtr pDev); +extern _X_EXPORT void +NoticeTime(const DeviceIntPtr dev, + TimeStamp time); extern _X_EXPORT void NoticeEventTime(InternalEvent *ev, DeviceIntPtr dev); +extern _X_EXPORT TimeStamp +LastEventTime(int deviceid); extern void EnqueueEvent(InternalEvent * /* ev */ , diff --git a/include/dixstruct.h b/include/dixstruct.h index 0be7f0e27..7711cde99 100644 --- a/include/dixstruct.h +++ b/include/dixstruct.h @@ -144,7 +144,6 @@ typedef struct _WorkQueue { } WorkQueueRec; extern _X_EXPORT TimeStamp currentTime; -extern _X_EXPORT TimeStamp lastDeviceEventTime[MAXDEVICES]; extern _X_EXPORT int CompareTimeStamps(TimeStamp /*a */ , diff --git a/os/WaitFor.c b/os/WaitFor.c index 393890f19..c5f4cd78b 100644 --- a/os/WaitFor.c +++ b/os/WaitFor.c @@ -561,7 +561,7 @@ NextDPMSTimeout(INT32 timeout) static CARD32 ScreenSaverTimeoutExpire(OsTimerPtr timer, CARD32 now, pointer arg) { - INT32 timeout = now - lastDeviceEventTime[XIAllDevices].milliseconds; + INT32 timeout = now - LastEventTime(XIAllDevices).milliseconds; CARD32 nextTimeout = 0; #ifdef DPMSExtension diff --git a/os/xdmcp.c b/os/xdmcp.c index 0538ac53e..11f11333d 100644 --- a/os/xdmcp.c +++ b/os/xdmcp.c @@ -1391,7 +1391,7 @@ recv_alive_msg(unsigned length) if (SessionRunning && AliveSessionID == SessionID) { /* backoff dormancy period */ state = XDM_RUN_SESSION; - if ((GetTimeInMillis() - lastDeviceEventTime[XIAllDevices].milliseconds) > + if ((GetTimeInMillis() - LastEventTime(XIAllDevices).milliseconds) > keepaliveDormancy * 1000) { keepaliveDormancy <<= 1; if (keepaliveDormancy > XDM_MAX_DORMANCY) From 06b87aa528d7a739ba20101a1f83b1a428691a01 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 16 Oct 2013 10:08:46 +1000 Subject: [PATCH 5/7] sync: if the idle time was reset, force alarms to trigger (#70476) The time between the idle reset and the IdleTimeWakeupHandler to be called is indeterminate. Clients with an PositiveTransition or NegativeTransition alarm on a low threshold may miss an alarm. Work around this by keeping a reset flag for each device. When the WakeupHandler triggers and the reset flag is set, we force a re-calculation of everything and pretend the current idle time is zero. Immediately after is the next calculation with the real idle time. Relatively reproducible test case: Set up a XSyncNegativeTransition alarm for a threshold of 1 ms. May trigger, may not. X.Org Bug 70476 Signed-off-by: Peter Hutterer Reviewed-by: Adam Jackson Reviewed-by: Keith Packard --- Xext/sync.c | 32 +++++++++++++++++++++++++++++--- dix/events.c | 38 ++++++++++++++++++++++++++++++++++---- include/dix.h | 6 ++++++ 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/Xext/sync.c b/Xext/sync.c index ed891b169..ab46d8972 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -2685,6 +2685,15 @@ IdleTimeBlockHandler(pointer pCounter, struct timeval **wt, pointer LastSelectMa counter->value = old_idle; /* pop */ } +static void +IdleTimeCheckBrackets(SyncCounter *counter, XSyncValue idle, XSyncValue *less, XSyncValue *greater) +{ + if ((greater && XSyncValueGreaterOrEqual(idle, *greater)) || + (less && XSyncValueLessOrEqual(idle, *less))) { + SyncChangeCounter(counter, idle); + } +} + static void IdleTimeWakeupHandler(pointer pCounter, int rc, pointer LastSelectMask) { @@ -2699,10 +2708,24 @@ IdleTimeWakeupHandler(pointer pCounter, int rc, pointer LastSelectMask) IdleTimeQueryValue(pCounter, &idle); - if ((greater && XSyncValueGreaterOrEqual(idle, *greater)) || - (less && XSyncValueLessOrEqual(idle, *less))) { - SyncChangeCounter(counter, idle); + /* + There is no guarantee for the WakeupHandler to be called within a specific + timeframe. Idletime may go to 0, but by the time we get here, it may be + non-zero and alarms for a pos. transition on 0 won't get triggered. + https://bugs.freedesktop.org/show_bug.cgi?id=70476 + */ + if (LastEventTimeWasReset(priv->deviceid)) { + LastEventTimeToggleResetFlag(priv->deviceid, FALSE); + if (!XSyncValueIsZero(idle)) { + XSyncValue zero; + XSyncIntsToValue(&zero, 0, 0); + IdleTimeCheckBrackets(counter, zero, less, greater); + less = priv->value_less; + greater = priv->value_greater; + } } + + IdleTimeCheckBrackets(counter, idle, less, greater); } static void @@ -2720,6 +2743,9 @@ IdleTimeBracketValues(pointer pCounter, CARD64 * pbracket_less, IdleTimeWakeupHandler, pCounter); } else if (!registered && (pbracket_less || pbracket_greater)) { + /* Reset flag must be zero so we don't force a idle timer reset on + the first wakeup */ + LastEventTimeToggleResetAll(FALSE); RegisterBlockAndWakeupHandlers(IdleTimeBlockHandler, IdleTimeWakeupHandler, pCounter); } diff --git a/dix/events.c b/dix/events.c index c803721f3..4632bb7db 100644 --- a/dix/events.c +++ b/dix/events.c @@ -262,7 +262,10 @@ InputInfo inputInfo; EventSyncInfoRec syncEvents; -static TimeStamp lastDeviceEventTime[MAXDEVICES]; +static struct DeviceEventTime { + Bool reset; + TimeStamp time; +} lastDeviceEventTime[MAXDEVICES]; /** * The root window the given device is currently on. @@ -1060,8 +1063,11 @@ MonthChangedOrBadTime(CARD32 *ms) void NoticeTime(const DeviceIntPtr dev, TimeStamp time) { - lastDeviceEventTime[XIAllDevices] = currentTime; - lastDeviceEventTime[dev->id] = currentTime; + lastDeviceEventTime[XIAllDevices].time = currentTime; + lastDeviceEventTime[dev->id].time = currentTime; + + LastEventTimeToggleResetFlag(dev->id, TRUE); + LastEventTimeToggleResetFlag(XIAllDevices, TRUE); } static void @@ -1085,7 +1091,30 @@ NoticeEventTime(InternalEvent *ev, DeviceIntPtr dev) TimeStamp LastEventTime(int deviceid) { - return lastDeviceEventTime[deviceid]; + return lastDeviceEventTime[deviceid].time; +} + +Bool +LastEventTimeWasReset(int deviceid) +{ + return lastDeviceEventTime[deviceid].reset; +} + +void +LastEventTimeToggleResetFlag(int deviceid, Bool state) +{ + lastDeviceEventTime[deviceid].reset = state; +} + +void +LastEventTimeToggleResetAll(Bool state) +{ + DeviceIntPtr dev; + nt_list_for_each_entry(dev, inputInfo.devices, next) { + LastEventTimeToggleResetFlag(dev->id, FALSE); + } + LastEventTimeToggleResetFlag(XIAllDevices, FALSE); + LastEventTimeToggleResetFlag(XIAllMasterDevices, FALSE); } /************************************************************************** @@ -5297,6 +5326,7 @@ InitEvents(void) dummy.id = i; NoticeTime(&dummy, currentTime); + LastEventTimeToggleResetFlag(i, FALSE); } syncEvents.replayDev = (DeviceIntPtr) NULL; diff --git a/include/dix.h b/include/dix.h index fd2490fbd..fa7ccd4a3 100644 --- a/include/dix.h +++ b/include/dix.h @@ -322,6 +322,12 @@ NoticeEventTime(InternalEvent *ev, DeviceIntPtr dev); extern _X_EXPORT TimeStamp LastEventTime(int deviceid); +extern _X_EXPORT Bool +LastEventTimeWasReset(int deviceid); +extern _X_EXPORT void +LastEventTimeToggleResetFlag(int deviceid, Bool state); +extern _X_EXPORT void +LastEventTimeToggleResetAll(Bool state); extern void EnqueueEvent(InternalEvent * /* ev */ , From e57ec99b03b2ad840c384a97ab2766ce9da0f5cc Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 16 Oct 2013 16:31:15 +1000 Subject: [PATCH 6/7] sync: always set the brackets (#59644) The current code sets bracket_greater to the first trigger after the current value, and bracket_less to the last trigger before the current value. For example, the idle timer with three negative and three positive transitions would set this: nt1 nt2 nt3 |--------|------|--|------- idle --|---|--|-----> t pt1 pt2 pt3 bracket_less == nt2 bracket_greater == pt2 This is an optimization so we can skip code paths in the block/wakeup handlers if the current value doesn't meet any of the trigger requirements. Those handlers largely do a if (bracket_less is less than current value && bracket_greater is greater than current value) return, nothing to do However, unless the bracket values are updated at the correct time, the following may happen: nt |--------------|---------- idle ------|--------> t pt In this case, neither bracket is set, we're past the pos transition and not yet at the neg transition. idle may now go past nt, but the brackets are not updated. If idle is then reset to 0, no alarm is triggered for nt. Likewise, idle may now go past pt and no alarm is triggered. Changing an alarm or triggering an alarm will re-calculate the brackets, so this bug is somewhat random. If any other client triggers an alarm when the brackets are wrongly NULL, the recalculation will set them this bug may not appear. This patch changes the behavior, so that the brackets are always the nearest positive or negative transitions to the current counter value. In the example above, nt will trigger a wakeup and a re-calculation of the brackets, so that going past it in the negative direction will then cause the proper alarm triggers. Or, in Keith's words: Timer currently past a positive trigger No bracket values, because no trigger in range Timer moves backwards before the positive trigger Brackets not reset, even though there is now a trigger in range Timer moves forward past the positive trigger Trigger doesn't fire because brackets not set Setting the LT bracket in this case will cause everything to get re-evaluated when the sync value moves backwards before the trigger value. X.Org Bug 59644 Signed-off-by: Peter Hutterer Reviewed-by: Adam Jackson Reviewed-by: Keith Packard --- Xext/sync.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Xext/sync.c b/Xext/sync.c index ab46d8972..ad3dec252 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -1019,6 +1019,11 @@ SyncComputeBracketValues(SyncCounter * pCounter) psci->bracket_greater = pTrigger->test_value; pnewgtval = &psci->bracket_greater; } + else if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) && + XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less)) { + psci->bracket_less = pTrigger->test_value; + pnewltval = &psci->bracket_less; + } } else if (pTrigger->test_type == XSyncNegativeComparison && ct != XSyncCounterNeverDecreases) { @@ -1028,6 +1033,11 @@ SyncComputeBracketValues(SyncCounter * pCounter) psci->bracket_less = pTrigger->test_value; pnewltval = &psci->bracket_less; } + else if (XSyncValueLessThan(pCounter->value, pTrigger->test_value) && + XSyncValueLessThan(pTrigger->test_value, psci->bracket_greater)) { + psci->bracket_greater = pTrigger->test_value; + pnewgtval = &psci->bracket_greater; + } } else if (pTrigger->test_type == XSyncNegativeTransition && ct != XSyncCounterNeverIncreases) { @@ -1041,6 +1051,11 @@ SyncComputeBracketValues(SyncCounter * pCounter) psci->bracket_less = pTrigger->test_value; pnewltval = &psci->bracket_less; } + else if (XSyncValueLessThan(pCounter->value, pTrigger->test_value) && + XSyncValueLessThan(pTrigger->test_value, psci->bracket_greater)) { + psci->bracket_greater = pTrigger->test_value; + pnewgtval = &psci->bracket_greater; + } } else if (pTrigger->test_type == XSyncPositiveTransition && ct != XSyncCounterNeverDecreases) { @@ -1055,6 +1070,11 @@ SyncComputeBracketValues(SyncCounter * pCounter) psci->bracket_greater = pTrigger->test_value; pnewgtval = &psci->bracket_greater; } + else if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) && + XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less)) { + psci->bracket_less = pTrigger->test_value; + pnewltval = &psci->bracket_less; + } } } /* end for each trigger */ From 2523a445a09a75a8baf642608d099a5e12d5437f Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 17 Oct 2013 12:02:27 +1000 Subject: [PATCH 7/7] sync: split updating and triggering a counter up Now that the brackets are always the nearest change points (regardless of transition) we need to update the counters whenever we check for any updates. Otherwise we end up with a situation where counter->value is out of date and an alarm doesn't trigger because we're still using the value from last time something actually triggered. Signed-off-by: Peter Hutterer Reviewed-by: Adam Jackson Reviewed-by: Keith Packard --- Xext/sync.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Xext/sync.c b/Xext/sync.c index ad3dec252..b2ee92e37 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -699,6 +699,14 @@ SyncAwaitTriggerFired(SyncTrigger * pTrigger) FreeResource(pAwaitUnion->header.delete_id, RT_NONE); } +static CARD64 +SyncUpdateCounter(SyncCounter *pCounter, CARD64 newval) +{ + CARD64 oldval = pCounter->value; + pCounter->value = newval; + return oldval; +} + /* This function should always be used to change a counter's value so that * any triggers depending on the counter will be checked. */ @@ -708,8 +716,7 @@ SyncChangeCounter(SyncCounter * pCounter, CARD64 newval) SyncTriggerList *ptl, *pnext; CARD64 oldval; - oldval = pCounter->value; - pCounter->value = newval; + oldval = SyncUpdateCounter(pCounter, newval); /* run through triggers to see if any become true */ for (ptl = pCounter->sync.pTriglist; ptl; ptl = pnext) { @@ -2712,6 +2719,8 @@ IdleTimeCheckBrackets(SyncCounter *counter, XSyncValue idle, XSyncValue *less, X (less && XSyncValueLessOrEqual(idle, *less))) { SyncChangeCounter(counter, idle); } + else + SyncUpdateCounter(counter, idle); } static void