[PATCH xserver] modesetting: Remove ms_crtc_msc_to_kernel_msc().

Adam Jackson ajax at redhat.com
Mon May 7 18:00:55 UTC 2018


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 <mario.kleiner.de at gmail.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Keith Packard <keithp at keithp.com>
> Cc: Adam Jackson <ajax at redhat.com>

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


More information about the xorg-devel mailing list