On Tue, 26 Oct 2010 11:39:11 +0300 Pauli Nieminen <[email protected]> wrote:
> On 26/10/10 03:00 +0200, ext Mario Kleiner wrote: > > On Oct 25, 2010, at 6:52 PM, Jesse Barnes wrote: > > > > > On Mon, 25 Oct 2010 17:13:58 +0300 > > > Pauli Nieminen <[email protected]> wrote: > > > > > >> There isn't API that allows application atomically query for msc > > >> changes > > >> and schedule swaps. If msc changes dramatically between query and > > >> scheduling application would schedule swap to happen at wrong time. > > >> > > >> Because of API limitations driver has to make msc increment for each > > >> vblank affecting the drawable. > > >> > > >> Signed-off-by: Pauli Nieminen <[email protected]> > > >> CC: Kristian Høgsberg <[email protected]> > > >> --- > > >> hw/xfree86/dri2/dri2.c | 7 +++++-- > > >> 1 files changed, 5 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c > > >> index d9b9d57..d70c115 100644 > > >> --- a/hw/xfree86/dri2/dri2.c > > >> +++ b/hw/xfree86/dri2/dri2.c > > >> @@ -860,9 +860,12 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr > > >> pDraw, CARD64 target_msc, > > >> if (!(*ds->GetMSC)(pDraw, &ust, ¤t_msc)) > > >> pPriv->last_swap_target = 0; > > >> > > >> - if (current_msc < pPriv->last_swap_target) > > >> + if (current_msc < pPriv->last_swap_target) { > > >> pPriv->last_swap_target = current_msc; > > >> - > > >> + xf86DrvMsg(pScreen->myNum, X_ERROR, > > >> + "[DRI2] %s: GetMSC returned swap count that is > > >> in " > > >> + "past. Working around driver bug.\n", __func__); > > >> + } > > >> } > > > > > > This one scares me a little. We added this so we could also catch > > > drawables moving between screens with different msc bases, so this > > > patch could cause a lot of false positives (no question that the specs > > > could use some additions here though). > > > > > > Making it a debug message that only shows up with -verbose would be > > > fine though. > > > > > > -- > > > Jesse Barnes, Intel Open Source Technology Center > > > > I agree with Jesse, as a debug message at -verbose it would be fine. > > > > The "fix" that you added to common code doesn't fix the root cause. In that > case error message is correct because common code is working around driver > bug for single case that happens to be common. Driver that hits that error > line shouldn't expose OML_sync_control because application can't use it > reliably in that case. It's not just OML_sync_control; it's also SGI_video_sync and SGI_swap_interval (also part of the core EGL spec). Disabling these due to unsynced multihead would be a bit harsh; it would also be hard because it's something that can change at runtime (what if a single head config with these extensions exposed becomes multihead with variable rate MSC due to a monitor getting plugged in?). That's why I think we need to fix the spec instead; it would be easy enough to add a new event to notify clients when the MSC rate and base change due to a move to a new pipe, someone just needs to write it up and code it (shouldn't be too hard). > But if you really want only verbose message I'm happy to change the patch. Yes please, until we get the spec and such worked out. These sorts of messages always confuse users; we end up with a bunch of bug reports and OSVs patching it out anyway. Thanks, -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
