From 6dbae4f4841ddfb70d6f2a973431f34d2b3cc4c0 Mon Sep 17 00:00:00 2001 From: Egbert Eich Date: Mon, 23 Mar 2009 12:58:28 +0100 Subject: [PATCH] Fixed SYNC extension trigger BlockHandler test. * Reworked ComputeBracketValues(): + Removed 'startOver' flag: we always seem to want to start over, there doesn't seem to be a reason why we should keep the old bracket values, moreover when this flag was set the semantics of setting the bracket values for the selected sync counter were flawed anyhow: the bracket whose value hadn't changed (if the other one did) was simply deleted. + Rewrote the bracket value calculation for Postitive/NegativeTransition: the calulation didn't cover all possible cases and was flawed anyhow. * Reworked previous patch to IdleTimeBlockHandler() (commit: 1f4fb022) + Simplified code. + Modified heuristics: pIdleTimeValueLess does not exclude pIdleTimeValueGreater: if no wakeup is scheduled for the former one check if there is one for the latter. + If no immediate wakeup is scheduled at all find schedule a wakeup for the next time an idle counter might trigger, do not wait for the latest idle counter to trigger. This fixes a problem introduced with commit 1f4fb022 where an idle counter expires unnoticed. Index: xorg-server-1.6.3.901/Xext/sync.c ================================================================================ --- xorg-server-1.7.99/Xext/sync.c +++ xorg-server-1.7.99/Xext/sync.c @@ -94,7 +94,7 @@ #define XSyncCAAllTrigger \ (XSyncCACounter | XSyncCAValueType | XSyncCAValue | XSyncCATestType) -static void SyncComputeBracketValues(SyncCounter *, Bool); +static void SyncComputeBracketValues(SyncCounter *); static void SyncInitServerTime(void); @@ -167,7 +167,7 @@ } if (IsSystemCounter(pTrigger->pCounter)) - SyncComputeBracketValues(pTrigger->pCounter, /*startOver*/ TRUE); + SyncComputeBracketValues(pTrigger->pCounter); } @@ -194,7 +194,7 @@ pTrigger->pCounter->pTriglist = pCur; if (IsSystemCounter(pTrigger->pCounter)) - SyncComputeBracketValues(pTrigger->pCounter, /*startOver*/ TRUE); + SyncComputeBracketValues(pTrigger->pCounter); return Success; } @@ -351,7 +351,7 @@ } else if (IsSystemCounter(pCounter)) { - SyncComputeBracketValues(pCounter, /*startOver*/ TRUE); + SyncComputeBracketValues(pCounter); } return Success; @@ -640,13 +640,14 @@ for (ptl = pCounter->pTriglist; ptl; ptl = pnext) { pnext = ptl->next; - if ((*ptl->pTrigger->CheckTrigger)(ptl->pTrigger, oldval)) + if ((*ptl->pTrigger->CheckTrigger)(ptl->pTrigger, oldval)) { (*ptl->pTrigger->TriggerFired)(ptl->pTrigger); + } } if (IsSystemCounter(pCounter)) { - SyncComputeBracketValues(pCounter, /* startOver */ FALSE); + SyncComputeBracketValues(pCounter); } } @@ -913,7 +914,7 @@ } static void -SyncComputeBracketValues(SyncCounter *pCounter, Bool startOver) +SyncComputeBracketValues(SyncCounter *pCounter) { SyncTriggerList *pCur; SyncTrigger *pTrigger; @@ -930,58 +931,53 @@ if (ct == XSyncCounterNeverChanges) return; - if (startOver) - { - XSyncMaxValue(&psci->bracket_greater); - XSyncMinValue(&psci->bracket_less); - } + XSyncMaxValue(&psci->bracket_greater); + XSyncMinValue(&psci->bracket_less); + for (pCur = pCounter->pTriglist; pCur; pCur = pCur->next) { pTrigger = pCur->pTrigger; - - if (pTrigger->test_type == XSyncPositiveComparison && - ct != XSyncCounterNeverIncreases) - { - 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 == XSyncNegativeComparison && - ct != XSyncCounterNeverDecreases) - { - 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 == XSyncNegativeTransition && - ct != XSyncCounterNeverIncreases) - { - if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) && - XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less)) - { + + if ( ( (pTrigger->test_type == XSyncPositiveComparison) + && ( XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value) + || (ct != XSyncCounterNeverIncreases + && XSyncValueLessThan(pCounter->value, pTrigger->test_value)) + ) + ) + || ((pTrigger->test_type == XSyncPositiveTransition) + && ( ( ct != XSyncCounterNeverDecreases + && XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value)) + || ( ct != XSyncCounterNeverIncreases + && XSyncValueLessThan(pCounter->value, pTrigger->test_value)) + ) + ) + ) { + if (XSyncValueLessThan(pTrigger->test_value, + psci->bracket_greater)) { + psci->bracket_greater = pTrigger->test_value; + pnewgtval = &psci->bracket_greater; + } + } else if ( ( (pTrigger->test_type == XSyncNegativeComparison) + && ( XSyncValueLessOrEqual(pCounter->value, pTrigger->test_value) + || (ct != XSyncCounterNeverDecreases + && XSyncValueGreaterThan(pCounter->value, pTrigger->test_value)) + ) + ) + || ((pTrigger->test_type == XSyncNegativeTransition) + && ( ( ct != XSyncCounterNeverDecreases + && XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value)) + || ( ct != XSyncCounterNeverIncreases + && XSyncValueLessThan(pCounter->value, pTrigger->test_value)) + ) + ) + ) { + if (XSyncValueGreaterThan(pTrigger->test_value, + psci->bracket_less)) { 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; - } - } } /* end for each trigger */ if (pnewgtval || pnewltval) @@ -2277,11 +2273,17 @@ } static void -IdleTimeBlockHandler(pointer env, struct timeval **wt, pointer LastSelectMask) -{ - XSyncValue idle, old_idle; - SyncTriggerList *list = IdleTimeCounter->pTriglist; - SyncTrigger *trig; +IdleTimeBlockHandler (pointer env, + struct timeval **wt, + pointer LastSelectMask) +{ + XSyncValue idle, old_idle; + SyncTriggerList *list = IdleTimeCounter->pTriglist; + SyncTrigger *trig; + unsigned long timeout = -1; + XSyncValue value; + Bool overflow; + if (!pIdleTimeValueLess && !pIdleTimeValueGreater) return; @@ -2299,46 +2301,57 @@ * want level or edge trigger. Check the trigger list against the * current idle time, and if any succeed, bomb out of select() * immediately so we can reschedule. + * NOTE: we need to check trigger because the idle timer can go + * both ways (XSyncCounterUnrestricted) so that we need to set + * pIdleTimeValueLess in ComputeBracketValues() in the edge + * triggered case even if the idle timer is already less. */ - - for (list = IdleTimeCounter->pTriglist; list; list = list->next) { + for (list = IdleTimeCounter->pTriglist; list; list = list->next) { trig = list->pTrigger; if (trig->CheckTrigger(trig, old_idle)) { - AdjustWaitForDelay(wt, 0); - break; + AdjustWaitForDelay(wt, 0); + IdleTimeCounter->value = old_idle; /* pop */ + return; } } } - else if (pIdleTimeValueGreater) + if (pIdleTimeValueGreater) { - /* - * There's a threshold in the positive direction. If we've been - * idle less than it, schedule a wakeup for sometime in the future. - * If we've been idle more than it, and someone wants to know about - * that level-triggered, schedule an immediate wakeup. - */ - unsigned long timeout = -1; - - if (XSyncValueLessThan (idle, *pIdleTimeValueGreater)) { - XSyncValue value; - Bool overflow; + /* + * There's a threshold in the positive direction. + * If we've been idle more than it, and someone wants to know about + * that level-triggered, schedule an immediate wakeup. + * NOTE: we need to check trigger because the idle timer can go + * both ways (XSyncCounterUnrestricted) so that we need to set + * pIdleTimeValueGreater in ComputeBracketValues() in the edge + * triggered case even if the idle timer is already greater. + */ - XSyncValueSubtract (&value, *pIdleTimeValueGreater, - idle, &overflow); - timeout = min(timeout, XSyncValueLow32 (value)); - } else { + if (XSyncValueGreaterOrEqual (idle, *pIdleTimeValueGreater)) { for (list = IdleTimeCounter->pTriglist; list; list = list->next) { - trig = list->pTrigger; + trig = list->pTrigger; if (trig->CheckTrigger(trig, old_idle)) { - timeout = min(timeout, 0); - break; + AdjustWaitForDelay (wt, 0); + IdleTimeCounter->value = old_idle; /* pop */ + return; } } } - - AdjustWaitForDelay (wt, timeout); } + /* + * If we don't have to wake up immediately we schedule a wakeup for the + * next time a trigger expires. + */ + for (list = IdleTimeCounter->pTriglist; list; list = list->next) { + trig = list->pTrigger; + if (XSyncValueLessThan (idle, trig->test_value)) { + XSyncValueSubtract (&value, trig->test_value, + idle, &overflow); + timeout = min(timeout,XSyncValueLow32 (value)); + } + } + AdjustWaitForDelay (wt, timeout); IdleTimeCounter->value = old_idle; /* pop */ }