Bug 26240 - OML_sync_control broken with older DRI2 servers
Summary: OML_sync_control broken with older DRI2 servers
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: GLX (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Jesse Barnes
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-26 03:44 UTC by Pierre Ossman
Modified: 2010-04-12 14:53 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
only enable OML if server supports it (2.34 KB, patch)
2010-03-04 15:35 UTC, Jesse Barnes
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre Ossman 2010-01-26 03:44:56 UTC
Commit daf7fe69f7bd0caa955d30b43fc35b7ce0069b6b changed mesa to use new DRI2 operations to implement the OML_sync_control extension. Unfortunately it broke the extension when using mesa with a server that doesn't support those latest extensions to DRI2 (e.g. current Xorg 1.7.x).
Comment 1 Pierre Ossman 2010-01-26 03:45:43 UTC
Adding Jesse Barnes as cc as he made the relevant change.
Comment 2 Jesse Barnes 2010-01-26 08:42:01 UTC
I'll check it out.  What did these routines return on old servers?  I'll try to preserve the same behavior...
Comment 3 Pierre Ossman 2010-01-27 10:50:17 UTC
I'm afraid I haven't analyzed it deeply. I just noticed that xbmc no longer vsynced, and I bisected it to that commit.

(I also checked xbmc's code and confirmed it uses that extension)
Comment 4 Jesse Barnes 2010-01-27 10:55:15 UTC
I'll take a look...
Comment 5 Giacomo Perale 2010-02-25 11:21:57 UTC
Any news on this?
Comment 6 Jesse Barnes 2010-02-25 11:48:11 UTC
No sorry, been working on other bugs.  I'll get to it eventually.
Comment 7 Jesse Barnes 2010-03-04 13:46:09 UTC
Looking at the Mesa code it seems like it should do the right thing with old servers...  Pulling down the xbmc code now to see how they use OML.
Comment 8 Jesse Barnes 2010-03-04 15:35:15 UTC
Created attachment 33772 [details] [review]
only enable OML if server supports it

Not sure why we were enabling it unconditionally before, but this seems to fix my test case at least.  We'll need to get this into Mesa 7.8 to avoid breaking things.
Comment 9 Pierre Ossman 2010-03-05 09:40:11 UTC
I'm not sure I follow here. If it was always turned on previously, but didn't work properly without a recent DRI2 X server, how does this improve things?
Comment 10 Jesse Barnes 2010-03-05 09:55:00 UTC
It was turned on before but would only work correctly in specific cases.  So this patch disables it when we can't guarantee it will work, but enables it if we can.  Apps check for extensions anyway, so it should be safe and fix the issue with XMBC, and make the 7.8 release a bit more honest.
Comment 11 Jesse Barnes 2010-03-05 09:55:41 UTC
Oh btw, I think old Mesa exposed the OML extension but didn't actually bother to provide the synchronization guarantees it specs, so we were advertising it falsely.  This patch fixes that.
Comment 12 Jesse Barnes 2010-03-05 11:28:17 UTC
Hm, my test program was wrong.  Before the commit I just pushed I don't think we'd ever advertise OML_sync_control as a supported GLX extension.  It would be in the client extension string, indicating that Mesa *could* support it with server or direct support, but we never enabled either until now.

So maybe XBMC is buggy and is trying to use the OML funcs w/o checking first?

What does glxinfo report for you in the broken and working cases?
Comment 13 Pierre Ossman 2010-03-16 14:14:55 UTC
(sorry for the late reply)

This is glxinfo when working (everything at 833acea, except src/glx which is at cca66db):

# glxinfo | grep OML
    GLX_OML_swap_method, GLX_SGI_make_current_read, GLX_SGI_swap_control, 
    GLX_MESA_swap_frame_usage, GLX_OML_swap_method, GLX_OML_sync_control, 
    GLX_MESA_swap_frame_usage, GLX_OML_swap_method, GLX_SGI_make_current_read, 

Unfortunately, it seems I cannot make things break anymore. My X server has been updated to 1.7.5.901, and I guess that version supports the new DRI2 stuff.
Comment 14 Jesse Barnes 2010-04-12 14:53:07 UTC
I think this one is fixed now.


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.