Re: [PATCH 2/3] ddx/dri2: Repair broken pageflip swap scheduling.

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Mon, 10 Dec 2012 10:48:29 -0800
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> > On 15.10.12 16:52, Chris Wilson wrote:
> >  > On Mon, 15 Oct 2012 16:46:52 +0200, Mario Kleiner 
> > <mario.kleiner@tuebingen.mpg.de> wrote:
> >  >> Hi Chris,
> >  >>
> >  >> can you please check & merge at least the first two patches of the
> >  >> series into the intel ddx?
> > 
> > Thanks for the quick reply.
> > 
> >  >
> >  > The first is along the right path, but I think you want to base the
> >  > split on divisor==0.
> > 
> > I don't think so, unless i misunderstand what you mean? The optimization 
> > of I830DRI2ScheduleFlip()'ing immediately should only happen in the case 
> > where current_msc is >= target_msc, ie., if the client really requests 
> > swap at next possible vblank, regardless what divisor is, once we've 
> > entered the whole ...
> > 
> > if (divisor == 0 || current_msc < *target_msc) {
> > 
> > ... block. Checking for divisor == 0 would be a nice little cleanup if 
> > only either (divisor == 0) or (current_msc < *target_msc) could be true. 
> > But it can happen that both (divisor == 0) and (current_msc < 
> > *target_msc) and then a check for (divisor == 0) wouldn't tell you if 
> > target_msc is in the future and the kernel vblank mechanism should 
> > schedule swap in the future, or if it is time for an immediate flip.
> > 
> > Also i tested with various distances between successive swap and with 
> > divisor 0 vs. non-zero, so at least it works as advertised with the 
> > current patch.
> > 
> > So that patch should be ok.
> 
> Yeah I don't understand the flip schedule at the top there either; if
> target_msc is out in the future, why would we schedule a page flip
> immediately just because divisor == 0?
> 
> Maybe it should look like this instead?
> 
> if (divisor == 0 || current_msc < *target_msc) {
> 	if (divisor == 0 && current_msc >= *target_msc)
> 		if (flip && I830DRI2ScheduleFlip(intel,	draw, swap_info))
> 			return TRUE;

commit 2ab29a1688cd313768d928e87e145570f35b4a70
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Mon Dec 10 14:55:32 2012 -0800

    dri2: don't schedule a flip prematurely at ScheduleSwap time

Mario, can you make sure this works for you?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center