Bug 27023

Summary: Incorrect calculation of bracket values when startOver = FALSE
Product: xorg Reporter: David James <davidjames>
Component: Server/GeneralAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium Keywords: patch
Version: git   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
Fix incorrect calculation of bracket values when startOver = FALSE none

Description David James 2010-03-11 11:04:44 UTC 
Currently, the SyncComputeBracketValues function in Xext/sync.c calculates bracket values incorrectly when startOver = FALSE. A patch to fix this will be included below.

Reproduction recipe:
  1. Setup a positive transition alarm for when idle time exceeds 2000ms
  2. Setup a positive transition alarm for when idle time exceeds 5000ms
  3. Become idle, and wait for the alarms to trigger.

Expected result:
  - When idle time exceeds 2000ms, the first alarm triggers.
  - When idle time exceeds 5000ms, the second alarm triggers.

Actual result:
  - When idle time exceeds 2000ms, the first alarm triggers.
  - When idle time exceeds 5000ms, the second alarm does not trigger.

Analysis:

 1) When you first setup the alarms, the SyncComputeBracketValues function is called with startOver = TRUE. In this case, the bracket values are calculated correctly, and the member variable "bracket_greater" on the idle time counter is set to 2000.
 2) When the first alarm triggers, the SyncComputeBracketValues function is called with startOver = FALSE, from SyncChangeCounter. In this case, the second alarm should be set up, and the member variable "bracket_greater" on the idle time counter should be set to 5000. However, because the test value (5000) is greater than the previous value of bracket_greater (2000), this alarm is not enabled.

Solution:

To fix this problem, I updated the SyncChangeCounter function to call SyncComputeBracketValues with startOver = TRUE. This causes "bracket_greater" to be reset and the alarm to be triggered correctly. After doing this, the startOver variable is always set to TRUE, so I also cleaned up the code and eliminated this parameter altogether.
Comment 1 David James 2010-03-11 11:15:25 UTC 
Created attachment 33963 [details] [review]
Fix incorrect calculation of bracket values when startOver = FALSE

Currently, SyncComputeBracketValues reuses old values of bracket_greater and bracket_less when startOver = FALSE. This can result in incorrect bracket values. To fix this issue, the startOver parameter is removed, and we do not reuse old values of bracket_greater and bracket_less.
Comment 2 Michel Dänzer 2010-03-12 01:00:26 UTC 
Please send the patch to the xorg-devel mailing list for review.
Comment 3 David James 2010-05-10 17:14:55 UTC 
FIXED in 758b8614477b53dc3de2b884fec5ccaf8a736432 / git://anongit.freedesktop.org/xorg/xserver

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.