Bug 102241 - gallium/wgl: SwapBuffers freezing regularly with swap interval enabled
Summary: gallium/wgl: SwapBuffers freezing regularly with swap interval enabled
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Other (show other bugs)
Version: 17.1
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-16 08:40 UTC by frank.richter
Modified: 2017-08-17 00:35 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Check for negative time stamp difference in wait_swap_interval() (1.03 KB, patch)
2017-08-16 08:40 UTC, frank.richter
Details | Splinter Review
Improve conversion to nanoseconds in Windows os_time_get_nano() (1.13 KB, patch)
2017-08-16 08:40 UTC, frank.richter
Details | Splinter Review

Description frank.richter 2017-08-16 08:40:02 UTC
Created attachment 133536 [details] [review]
Check for negative time stamp difference in wait_swap_interval()

If swap interval is enabled the gallium/wgl SwapBuffers may freeze up if os_time_get_nano() happens to return a time stamp smaller than the last requested.

wait_swap_interval() does only check for a time stamp difference smaller than the 'swap period', but does not check for a negative difference. If that occurs it may continue passing a negative sleep duration to os_time_sleep(), which converts it to an unsigned DWORD, which may lead to very, very long sleep times (perceived as the app being frozen).

However, due to the way os_time_get_nano() is implemented on Windows, the time stamp going backwards may happen far more often than one would expect...
Doing a back-of-the-envelope calculation this can happens every 100 min or so:
* The time stamp is based on the "tick count" returned by QueryPerformanceCounter(). It's a 64-bit value. so we have 1.84467*10^10 different possible values.
* However, the first step in os_time_get_nano() is to multiply the tick count by 10^9, which may shift half of the digits out left, leaving us with 1.84467*10^10 different possible values.
* Next, that value is divided by the QPC frequency (ticks per second). On my computer that's about 3*10^6 (I guess all modern systems have frequencies in that ballpark), so our range of possible time stamp values covers ~6150 seconds... about 100 mins or so, after which we can expect the time stamp to roll over!

Attached you can find patches to address these issues:
* A simple check in wait_swap_interval() to ignore negative time stamp differences.
* A change of the way os_time_get_nano() converts ticks to nanoseconds that avoids the initial multiplication with 10^9 potentially discarding half the tick count digits.
Comment 1 frank.richter 2017-08-16 08:40:55 UTC
Created attachment 133537 [details] [review]
Improve conversion to nanoseconds in Windows os_time_get_nano()
Comment 2 Brian Paul 2017-08-16 13:21:17 UTC
Thanks for the patches.  I'll do a little clean-up then post them for review on mesa-dev in case anyone else has comments.  I'll push to master in a day or so if there's no additional feedback.
Comment 3 Brian Paul 2017-08-17 00:35:07 UTC
Patches pushed as d90e05ad487e9fe7e17c293814ac8549d9d686d8 and 7fb7287ce72066db7dffd918226bf15c3131a871.


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.