On 11.12.12 00:00, Jesse Barnes wrote:
On Mon, 10 Dec 2012 10:48:29 -0800
Jesse Barnes <[email protected]> wrote:
On 15.10.12 16:52, Chris Wilson wrote:
> On Mon, 15 Oct 2012 16:46:52 +0200, Mario Kleiner
<[email protected]> 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 <[email protected]>
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?
Looks good for my purposes, ie., should fix glXSwapBuffersMscOML(),
which was my main point of grief, thanks a lot! I'll retest soonish for
extra paranoia. The divisor == 0 check is not really needed for the
logic to work, but doesn't hurt and maybe still nice to leave there for
readability to clarify the condition when the optimization should work.
However, it doesn't fix some cases for normal glXSwapBuffers() with a
swapinterval > 1. That requires always updating *target_msc properly,
and the early exit if the optimization is taken prevents that. Also the
swap_info->frame doesn't get updated in this case, which effectively
disables/skips some correctness test in I830DRI2FlipEventHandler().
If you want to fix those as well you'll basically end up with my
original patch, which moves the optimization a few lines down in the
function and adds updating of those two variables, minus lots of
comments in the code and commit message.
But i'm happy, i mostly need glXSwapBuffersMscOML() and the pageflip
timestamping to work properly.
-mario
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
