[PATCH 2/2] DRI2: Add error message when working around driver bug

[Pauli Nieminen]

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 <ext-pauli.nieminen at nokia.com> 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 <ext-pauli.nieminen at nokia.com>
> >> CC: Kristian Høgsberg <krh at bitplanet.net>
> >> ---
> >>  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, &current_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.

But if you really want only verbose message I'm happy to change the patch.

> -mario