xserver: Branch 'master'
Adam Jackson
ajax at kemper.freedesktop.org
Mon May 7 18:00:37 UTC 2018
hw/xfree86/drivers/modesetting/driver.h | 1
hw/xfree86/drivers/modesetting/drmmode_display.h | 1
hw/xfree86/drivers/modesetting/vblank.c | 45 +----------------------
3 files changed, 3 insertions(+), 44 deletions(-)
New commits:
commit 73f0ed2d928afc692ed057eb3d7627328a6e5b12
Author: Mario Kleiner <mario.kleiner.de at gmail.com>
Date: Fri May 4 14:14:10 2018 +0200
modesetting: Remove ms_crtc_msc_to_kernel_msc().
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>
Reviewed-by: Adam Jackson <ajax at redhat.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Keith Packard <keithp at keithp.com>
Tested-by: Mike Lothian <mike at fireburn.co.uk>
diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h
index b2a4e52f8..3a81d4d14 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -149,7 +149,6 @@ xf86CrtcPtr ms_dri2_crtc_covering_drawable(DrawablePtr pDraw);
int ms_get_crtc_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc);
-uint64_t ms_crtc_msc_to_kernel_msc(xf86CrtcPtr crtc, uint64_t expect);
uint64_t ms_kernel_msc_to_crtc_msc(xf86CrtcPtr crtc, uint64_t sequence);
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 74954b853..e46a292d3 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.h
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
@@ -180,7 +180,6 @@ typedef struct {
* lies, and we need to give a reliable 64-bit msc for GL, so we
* have to track and convert to a userland-tracked 64-bit msc.
*/
- int32_t vblank_offset;
uint32_t msc_prev;
uint64_t msc_high;
/** @} */
diff --git a/hw/xfree86/drivers/modesetting/vblank.c b/hw/xfree86/drivers/modesetting/vblank.c
index ae3018b4b..d1a6fc1bb 100644
--- a/hw/xfree86/drivers/modesetting/vblank.c
+++ b/hw/xfree86/drivers/modesetting/vblank.c
@@ -226,7 +226,6 @@ ms_queue_vblank(xf86CrtcPtr crtc, ms_queue_flag flags,
/* Queue an event at the specified sequence */
if (ms->has_queue_sequence || !ms->tried_queue_sequence) {
uint32_t drm_flags = 0;
- uint64_t kernel;
uint64_t kernel_queued;
ms->tried_queue_sequence = TRUE;
@@ -236,10 +235,8 @@ ms_queue_vblank(xf86CrtcPtr crtc, ms_queue_flag flags,
if (flags & MS_QUEUE_NEXT_ON_MISS)
drm_flags |= DRM_CRTC_SEQUENCE_NEXT_ON_MISS;
- kernel = ms_crtc_msc_to_kernel_msc(crtc, msc);
ret = drmCrtcQueueSequence(ms->fd, drmmode_crtc->mode_crtc->crtc_id,
- drm_flags,
- kernel, &kernel_queued, seq);
+ drm_flags, msc, &kernel_queued, seq);
if (ret == 0) {
if (msc_queued)
*msc_queued = ms_kernel_msc_to_crtc_msc(crtc, kernel_queued);
@@ -260,8 +257,7 @@ ms_queue_vblank(xf86CrtcPtr crtc, ms_queue_flag flags,
if (flags & MS_QUEUE_NEXT_ON_MISS)
vbl.request.type |= DRM_VBLANK_NEXTONMISS;
- vbl.request.sequence = (flags & MS_QUEUE_RELATIVE) ?
- msc : ms_crtc_msc_to_kernel_msc(crtc, msc);
+ vbl.request.sequence = msc;
vbl.request.signal = seq;
ret = drmWaitVBlank(ms->fd, &vbl);
if (ret == 0) {
@@ -280,14 +276,12 @@ ms_queue_vblank(xf86CrtcPtr crtc, ms_queue_flag flags,
/**
* Convert a 32-bit kernel MSC sequence number to a 64-bit local sequence
- * number, adding in the vblank_offset and high 32 bits, and dealing
- * with 64-bit wrapping
+ * number, adding in the high 32 bits, and dealing with 64-bit wrapping.
*/
uint64_t
ms_kernel_msc_to_crtc_msc(xf86CrtcPtr crtc, uint64_t sequence)
{
drmmode_crtc_private_rec *drmmode_crtc = crtc->driver_private;
- sequence += drmmode_crtc->vblank_offset;
if ((int32_t) (sequence - drmmode_crtc->msc_prev) < -0x40000000)
drmmode_crtc->msc_high += 0x100000000L;
@@ -307,39 +301,6 @@ ms_get_crtc_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc)
return Success;
}
-#define MAX_VBLANK_OFFSET 1000
-
-/**
- * Convert a 64-bit adjusted MSC value into a 64-bit kernel sequence number,
- * by subtracting out the vblank_offset term.
- *
- * This also updates the vblank_offset when it notices that the value should
- * change.
- */
-uint64_t
-ms_crtc_msc_to_kernel_msc(xf86CrtcPtr crtc, uint64_t expect)
-{
- drmmode_crtc_private_rec *drmmode_crtc = crtc->driver_private;
- uint64_t msc;
- uint64_t ust;
- int64_t diff;
-
- if (ms_get_crtc_ust_msc(crtc, &ust, &msc) == Success) {
- diff = expect - msc;
-
- /* We're way off here, assume that the kernel has lost its mind
- * and smack the vblank back to something sensible
- */
- if (diff < -MAX_VBLANK_OFFSET || MAX_VBLANK_OFFSET < diff) {
- drmmode_crtc->vblank_offset += (int32_t) diff;
- if (drmmode_crtc->vblank_offset > -MAX_VBLANK_OFFSET &&
- drmmode_crtc->vblank_offset < MAX_VBLANK_OFFSET)
- drmmode_crtc->vblank_offset = 0;
- }
- }
- return (expect - drmmode_crtc->vblank_offset);
-}
-
/**
* Check for pending DRM events and process them.
*/
More information about the xorg-commit
mailing list