[Nouveau] [Patches][nouveau/ddx]: Improvements to bufferswap implementation and timestamping

[Mario Kleiner]

On 02/29/2012 08:17 AM, Ben Skeggs wrote:
> On Thu, 2012-02-16 at 00:45 +0100, Mario Kleiner wrote:
>> Hi,
> Hey Mario,
>
> What's your plan with this patchset?  Do you intend on taking Michel's
> comments into account?
>
> CC'ing Francisco as he had some comments on IRC.  I'd like to get this
> all sorted and merged so I can rebase the ported-to-new-libdrm ddx on
> top of it all and get that ready.
>

Hi Ben,

just sent out two updated patches wrt. to Michels feedback.

[1/10] Replaces the original [1/9] patch. The code is the same, but the 
commit message is changed to be less misleading about the cause of the 
problem. See the e-mail i think i sent about a week ago.

[10/10] Just for demonstration what i tried, not for application. The 
old [9/9] patch is still the best i can do at the moment. Following
Michels proposal, with 10/10 i added refcounting to nouveau_dri2_buffer 
objects and keep a reference while the swap is still pending. This 
prevents deletion of the buffer by the x-server and thereby prevents the 
server crash.

Problem: Now we have a display that doesn't crash, but flickers horribly 
while pageflipping with deferred swaps. I think it is because we still 
only have two buffers for double-buffering but pretend we're doing 
triple-buffering, but i could be totally wrong about the cause of the 
problem. The original patch [9/9] throttles the client in the special 
case where the pseudo-triplebuffering would result in flicker or crash, 
so it fixes the problem although in a rather hackish way.

Not sure if this can be done cleanly without a real n-buffering 
implementation in the x-server?

I think other than that there were no further comments and Francisco's 
original feedback from last year is already implemented in the patchset.

More feedback etc. welcome, but i usually can only work on this part 
time on weekends if it needs further improvements before merging.

Can the kms kernel patches be already merged? I think they are in a good 
shape now.

Thanks,
-mario


> Ben.
>
>>
>> here a set of patches against the nouveau-ddx. This is an extended and
>> revised set, based on Francisco Jerez feedback from autumn last year.
>>
>> [1/9] Makes pageflipping work again on X-Server 1.12rc. It apparently stopped
>> working somewhere around Xorg 1.11+.
>>
>> [2/9] Implements handling of pageflip completion events from the kernel.
>> Francisco Jerez argument against including it was that the x-server didn't
>> have a swaplimit api, so this couldn't be applied at the same time as the
>> pseudo triple-buffering hack which is in place in the current ddx.
>> Now we have the swaplimit api in 1.12, so this problem should be solved.
>>
>> [3/9] Makes use of the swaplimit api. A new xorg.conf option "SwapLimit"
>> allows to select a swaplimit of 1 or 2 for double-buffering or
>> triple-buffering. It defaults to 2 for Xorg 1.12+ and 1 for older servers.
>> This way, on 1.12+ nouveau retains the kind of triple buffering behaviour it
>> currently has, but swap completion timestamping (OML_sync_control,
>> INTEL_swap_events, and client swap throttling) works conforming to
>> the specs. On older servers it removes triple-buffering but makes
>> nouveau conform to the specs. This is important for apps that need
>> precise and reliable swap scheduling and timestamping.
>>
>> [4/9] A bug fix against corrupted desktop when switching from redirected
>> to non-redirected fullscreen windows under a desktop compositor.
>> Fixes FDO bug #35452.
>>
>> [5/9] Some fixes to swap scheduling, revised according to Francisco's
>> review.
>>
>> [6/9] Fixes swap throttling for non-fullscreen windows under a desktop
>> compositor. Split into a separate patch according to Francisco's
>> feedback.
>>
>> [7/9] An attempt to provide more sane swap completion events for non-
>> fullscreen windows. This is a bit of cheating, as it delivers the
>> events for the earliest point in time one would expect the swap
>> to complete, assuming only a lightly loaded gpu. Real completion
>> could be later. I think this is an "improvement" because the current
>> implementation delivers swap complete events with timestamps and
>> counts that signal swap completion before the client even requested
>> the swap.
>>
>> [8/9] This one adds Francisco's original triple-buffering hack back
>> for Xorg 1.11 and earlier servers if the xorg.conf option requests
>> a swaplimit>  1. Users can choose between "triple-buffering" but
>> broken timestamping or double-buffering with sane timestamping.
>>
>> [9/9] Fixes a corner case which could cause the ddx to segfault
>> with its current triple-buffering implementation.
>>
>> I've tested these on single-display and dual-display setups,
>> with/without compositor, with/without redirection and checked
>> the robustness and precision of the timestamps with special
>> measurement equipment, on XOrg 1.12-rc2 and 1.10. They work
>> pretty well for me and finally make nouveau very useable for
>> the kind of scientific applications that require precise
>> swap scheduling and timestamping, so i'd love to see them
>> reviewed and hopefully included into the ddx soon.
>>
>> A couple of things are a bit of hacks:
>>
>> [3/9] I think the setup of a default swap limit would be
>> better done in the x-server itself instead of the ddx. The
>> setup code is a bit awkward, hijacking the ddx->CreateBuffer
>> function to apply the swaplimit. A DRI2GetSwapLimit() function
>> is also missing from the server, which could help in the
>> future to track swaplimit changes by other clients than the ddx
>> itself. Unfortunately it is too late for the 1.12 release to do
>> this.
>>
>> [8/9] Don't know if this is still wanted/needed or not?
>>
>> [9/9] It fixes the problem and doesn't affect performance, but is
>> somewhat of a hack. I don't know how to do better, maybe somebody
>> else has a better solution?
>>
>> Thanks,
>> -mario
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
>
>