On Fri, 2018-05-04 at 14:14 +0200, Mario Kleiner wrote: > The function is ported from intel-ddx uxa backend around > 2013, where its stated purpose was to apply a vblank_offset > to msc values to correct for problems with those kernel > provided msc values. Some (somewhat magic and puzzling to > myself) heuristic tried to guess if provided values were > unreasonable and tried to adapt the corrective vblank_offset > to account for that. > > Except: It wasn't applied to kernel provided msc values, > but the values delivered by clients via DRI2 or Present, > so valid client targetmsc values, e.g., requesting a vblank > event > 1000 vblanks in the future, triggered the offset > correction in arbitrarily wrong ways, leading to wrong msc > values being returned and thereby vblank events queued to > the kernel for the wrong time. This causes glXSwapBuffersMscOML > and glXWaitForMscOML to swap / return immediately whenever a > swap/wait in > 1000 vblanks is requested. > > The original code was also written to only deal with 32 bit mscs, > but server 1.20 modesetting ddx can now use new Linux 4.15+ kernel > vblank api to process true 64 bit msc's, which may confuse the > heuristic even more due to 32 bit integer truncation/wrapping. > > This code caused various problems in the intel-ddx in the > past since year 2013, and was removed there in 2015 by Chris > Wilson in commit 42ebe2ef9646be5c4586868cf332b4cd79bb4618: > > " uxa: Remove the filtering of bogus Present MSC values > > If the intention was to filter the return values from the kernel, the > filtering would have been applied to the kernel values and not to the > incoming values from Present. This filtering introduces crazy integer > promotion and truncation bugs all because Present feeds garbage into its > vblank requests. > > " > > Indeed, i found a Mesa bug yesterday which can cause Mesa's > PresentPixmap request to spuriously feed garbage targetMSC's > into the driver under some conditions. However, while other > video drivers seem to cope relatively well with that, modesetting > ddx causes KDE-5's plasmashell to lock up badly quite frequently, > and my suspicion is that the code removed in this commit is one > major source of the extra fragility. > > Also my own tests fail for any swap scheduled more than 1000 > vblanks into the future, which is not uncommon for some scientific > applications. > > Iow. modesetting's swap scheduling seems to be more robust without > this function afaics. > > Signed-off-by: Mario Kleiner <[email protected]> > Cc: Chris Wilson <[email protected]> > Cc: Keith Packard <[email protected]> > Cc: Adam Jackson <[email protected]>
Fixing things by deleting them! The best kind of patch. Merged, thanks: remote: I: patch #220463 updated using rev 73f0ed2d928afc692ed057eb3d7627328a6e5b12. remote: I: 1 patch(es) updated to state Accepted. To ssh://git.freedesktop.org/git/xorg/xserver f5ded22e14..73f0ed2d92 master -> master - ajax _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
