xserver: Branch 'master' - 11 commits
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Wed Mar 1 10:17:30 UTC 2023
hw/xfree86/drivers/modesetting/dri2.c | 3
hw/xfree86/drivers/modesetting/driver.c | 6
hw/xfree86/drivers/modesetting/driver.h | 11 +
hw/xfree86/drivers/modesetting/drmmode_display.c | 1
hw/xfree86/drivers/modesetting/drmmode_display.h | 1
hw/xfree86/drivers/modesetting/pageflip.c | 205 ++++++++++++++++++++---
hw/xfree86/drivers/modesetting/present.c | 26 ++
hw/xfree86/drivers/modesetting/vblank.c | 13 +
present/present.h | 6
present/present_scmd.c | 50 +++--
10 files changed, 270 insertions(+), 52 deletions(-)
New commits:
commit f490622fca4ec175193f68587b9c647a2ab515f4
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Tue Feb 14 19:41:50 2023 -0800
present: Document the TearFree flip reasons in PresentFlipReason
Adding new flip reasons after the TearFree ones would break the assumption
that `reason >= PRESENT_FLIP_REASON_DRIVER_TEARFREE` implies either of the
TearFree reasons. Document this in the PresentFlipReason enum in order to
save someone a very bad headache in the future.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/present/present.h b/present/present.h
index 1d7b0ce42..a3f34a929 100644
--- a/present/present.h
+++ b/present/present.h
@@ -30,6 +30,12 @@
typedef enum {
PRESENT_FLIP_REASON_UNKNOWN,
PRESENT_FLIP_REASON_BUFFER_FORMAT,
+
+ /* Don't add new flip reasons after the TearFree ones, since it's expected
+ * that the TearFree reasons are the highest ones in order to allow doing
+ * `reason >= PRESENT_FLIP_REASON_DRIVER_TEARFREE` to check if a reason is
+ * PRESENT_FLIP_REASON_DRIVER_TEARFREE{_FLIPPING}.
+ */
PRESENT_FLIP_REASON_DRIVER_TEARFREE,
PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING
} PresentFlipReason;
commit 1fab978a9585cc7fea483583bc1c76e9e48e4f35
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Sun Jan 22 23:31:52 2023 -0800
present: Fix inaccurate PresentCompleteNotify timing for TearFree
The timing of PresentCompleteNotify events is inaccurate when a driver uses
TearFree because there's no way to know exactly when a presentation will
appear on the display without receiving a notification directly from the
driver indicating that the TearFree flip containing a presentation's pixmap
is actually visible on the display.
To fix the inaccurate PresentCompleteNotify timing, make use of the new
assumption that drivers which export TearFree permit a NULL pixmap to be
passed to their flip callback in order to make a presentation track the
exact TearFree flip responsible for rendering it onto the display.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Acked-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/present/present_scmd.c b/present/present_scmd.c
index 59feb001b..d378f0016 100644
--- a/present/present_scmd.c
+++ b/present/present_scmd.c
@@ -646,30 +646,48 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
present_execute_copy(vblank, crtc_msc);
- /* The presentation will be visible at the next vblank with TearFree, so
- * the PresentComplete notification needs to be sent at the next vblank.
- * If TearFree is already flipping then the presentation will be visible
- * at the *next* next vblank.
+ /* With TearFree, there's no way to tell exactly when the presentation
+ * will be visible except by waiting for a notification from the kernel
+ * driver indicating that the page flip is complete. This is because the
+ * CRTC's MSC can change while the target MSC is calculated and even
+ * while the page flip IOCTL is sent to the kernel due to scheduling
+ * delays and/or unfortunate timing. Even worse, a page flip isn't
+ * actually guaranteed to be finished after one vblank; it may be
+ * several MSCs until a flip actually finishes depending on delays and
+ * load in hardware.
+ *
+ * So, to get a notification from the driver with TearFree active, the
+ * driver expects a present_flip() call with a NULL pixmap to indicate
+ * that this is a fake flip for a pixmap that's already been copied to
+ * the primary scanout, which will then be flipped by TearFree. TearFree
+ * will then send a notification once the flip containing this pixmap is
+ * complete.
+ *
+ * If the fake flip attempt fails, then fall back to just enqueuing a
+ * vblank event targeting the next MSC.
*/
- if (!vblank->queued) {
+ if (!vblank->queued &&
+ vblank->reason >= PRESENT_FLIP_REASON_DRIVER_TEARFREE) {
uint64_t completion_msc = crtc_msc + 1;
- switch (vblank->reason) {
- case PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING:
- if (vblank->exec_msc < crtc_msc)
+ /* If TearFree is already flipping then the presentation will be
+ * visible at the *next* next vblank. This calculation only matters
+ * for the vblank event fallback.
+ */
+ if (vblank->reason == PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING &&
+ vblank->exec_msc < crtc_msc)
completion_msc++;
- case PRESENT_FLIP_REASON_DRIVER_TEARFREE:
- if (Success == screen_priv->queue_vblank(screen,
- window,
- vblank->crtc,
- vblank->event_id,
- completion_msc)) {
- /* Ensure present_execute_post() runs at the next MSC */
- vblank->exec_msc = vblank->target_msc;
- vblank->queued = TRUE;
- }
- default:
- break;
+
+ /* Try the fake flip first and then fall back to a vblank event */
+ if (present_flip(vblank->crtc, vblank->event_id, 0, NULL, TRUE) ||
+ Success == screen_priv->queue_vblank(screen,
+ window,
+ vblank->crtc,
+ vblank->event_id,
+ completion_msc)) {
+ /* Ensure present_execute_post() runs at the next execution */
+ vblank->exec_msc = vblank->target_msc;
+ vblank->queued = TRUE;
}
}
commit d4bd39f1a5db2c5ee5ff251f1982bbe6aa5c0f7f
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Sun Jan 22 22:16:58 2023 -0800
present: Prevent double vblank enqueue on error when TearFree is used
It's possible for present_execute_copy to enqueue a vblank even when
TearFree is used, specifically when the present_queue_vblank in
present_scmd_pixmap fails and the subsequent vblank enqueue in
present_execute_copy somehow doesn't. This could happen if the DRM event
queue is exhausted when present_queue_vblank is called, but is no longer
exhausted by the time present_execute_copy is reached.
This exceedingly unlikely chain of events can lead to a vblank getting
enqueued a second time by the TearFree machinery in present_execute, which
is not good.
Although this scenario is very unlikely, prevent it by first checking that
the vblank wasn't enqueued by present_execute_copy before attempting to
enqueue it for TearFree.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Acked-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/present/present_scmd.c b/present/present_scmd.c
index 200ded348..59feb001b 100644
--- a/present/present_scmd.c
+++ b/present/present_scmd.c
@@ -554,7 +554,6 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
WindowPtr window = vblank->window;
ScreenPtr screen = window->drawable.pScreen;
present_screen_priv_ptr screen_priv = present_screen_priv(screen);
- uint64_t completion_msc;
if (vblank && vblank->crtc) {
screen_priv=present_screen_priv(vblank->crtc->pScreen);
}
@@ -652,23 +651,26 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
* If TearFree is already flipping then the presentation will be visible
* at the *next* next vblank.
*/
- completion_msc = crtc_msc + 1;
- switch (vblank->reason) {
- case PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING:
- if (vblank->exec_msc < crtc_msc)
- completion_msc++;
- case PRESENT_FLIP_REASON_DRIVER_TEARFREE:
- if (Success == screen_priv->queue_vblank(screen,
- window,
- vblank->crtc,
- vblank->event_id,
- completion_msc)) {
- /* Ensure present_execute_post() runs at the next MSC */
- vblank->exec_msc = vblank->target_msc;
- vblank->queued = TRUE;
+ if (!vblank->queued) {
+ uint64_t completion_msc = crtc_msc + 1;
+
+ switch (vblank->reason) {
+ case PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING:
+ if (vblank->exec_msc < crtc_msc)
+ completion_msc++;
+ case PRESENT_FLIP_REASON_DRIVER_TEARFREE:
+ if (Success == screen_priv->queue_vblank(screen,
+ window,
+ vblank->crtc,
+ vblank->event_id,
+ completion_msc)) {
+ /* Ensure present_execute_post() runs at the next MSC */
+ vblank->exec_msc = vblank->target_msc;
+ vblank->queued = TRUE;
+ }
+ default:
+ break;
}
- default:
- break;
}
if (vblank->queued) {
commit 53b02054f36a49bd45e954f0eef1029152af78b7
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Tue Feb 14 19:41:39 2023 -0800
modesetting: Support accurate DRI presentation timing with TearFree
When using TearFree, DRI clients have no way of accurately knowing when
their copied pixmaps appear on the display without utilizing the kernel
driver's notification indicating that the TearFree flip containing their
pixmap is complete. This is because the target CRTC's MSC can change while
the predicted completion MSC is calculated and even while the page flip
IOCTL is sent to the kernel due to scheduling delays and/or unfortunate
timing. Even worse, a page flip isn't actually guaranteed to be finished
after one vblank; it may be several MSCs until a flip actually finishes
depending on delays and load in hardware.
As a result, DRI clients may be off by one or more MSCs when they naively
expect their pixmaps to be visible at MSC+1 with TearFree enabled. This,
for example, makes it impossible for DRI clients to achieve precise A/V
synchronization when TearFree is enabled.
This change therefore adds a way for DRI clients to receive a notification
straight from the TearFree flip-done handler to know exactly when their
pixmaps appear on the display. This is done by checking for a NULL pixmap
pointer to modesetting's DRI flip routine, which indicates that the DRI
client has copied its pixmap and wants TearFree to send a notification when
the copied pixmap appears on the display as part of a TearFree flip. The
existing PageFlip scaffolding is reused to achieve this with minimal churn.
The Present extension will be updated in an upcoming change to utilize this
new mechanism for DRI clients' presentations.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Acked-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
index 5176b6d40..9a69452bd 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -655,8 +655,11 @@ ms_tearfree_do_flips(ScreenPtr pScreen)
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
- if (!ms_tearfree_is_active_on_crtc(crtc))
+ if (!ms_tearfree_is_active_on_crtc(crtc)) {
+ /* Notify any lingering DRI clients waiting for a flip to finish */
+ ms_tearfree_dri_abort_all(crtc);
continue;
+ }
/* Skip if the last flip is still pending, a DRI client is flipping, or
* there isn't any damage on the front buffer.
diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h
index a54bf28ff..e302c067d 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -242,6 +242,14 @@ Bool ms_do_pageflip(ScreenPtr screen,
ms_pageflip_abort_proc pageflip_abort,
const char *log_prefix);
+Bool
+ms_tearfree_dri_abort(xf86CrtcPtr crtc,
+ Bool (*match)(void *data, void *match_data),
+ void *match_data);
+
+void
+ms_tearfree_dri_abort_all(xf86CrtcPtr crtc);
+
Bool ms_do_tearfree_flip(ScreenPtr screen, xf86CrtcPtr crtc);
#endif
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 8f8e4060a..e8b542163 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -2529,6 +2529,7 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_res
drmmode_crtc->drmmode = drmmode;
drmmode_crtc->vblank_pipe = drmmode_crtc_vblank_pipe(num);
xorg_list_init(&drmmode_crtc->mode_list);
+ xorg_list_init(&drmmode_crtc->tearfree.dri_flip_list);
drmmode_crtc->next_msc = UINT64_MAX;
props = drmModeObjectGetProperties(drmmode->fd, mode_res->crtcs[num],
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 145cb8cc7..b4074a30f 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.h
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
@@ -176,6 +176,7 @@ typedef struct {
typedef struct {
drmmode_shadow_fb_rec buf[2];
+ struct xorg_list dri_flip_list;
uint32_t back_idx;
uint32_t flip_seq;
} drmmode_tearfree_rec, *drmmode_tearfree_ptr;
diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c
index a804841fa..c1ee542b7 100644
--- a/hw/xfree86/drivers/modesetting/pageflip.c
+++ b/hw/xfree86/drivers/modesetting/pageflip.c
@@ -99,6 +99,8 @@ struct ms_crtc_pageflip {
Bool on_reference_crtc;
/* reference to the ms_flipdata */
struct ms_flipdata *flipdata;
+ struct xorg_list node;
+ uint32_t tearfree_seq;
};
/**
@@ -142,7 +144,8 @@ ms_pageflip_handler(uint64_t msc, uint64_t ust, void *data)
flipdata->fe_usec,
flipdata->event);
- drmModeRmFB(ms->fd, flipdata->old_fb_id);
+ if (flipdata->old_fb_id)
+ drmModeRmFB(ms->fd, flipdata->old_fb_id);
}
ms_pageflip_free(flip);
}
@@ -309,6 +312,64 @@ ms_print_pageflip_error(int screen_index, const char *log_prefix,
}
}
+static Bool
+ms_tearfree_dri_flip(modesettingPtr ms, xf86CrtcPtr crtc, void *event,
+ ms_pageflip_handler_proc pageflip_handler,
+ ms_pageflip_abort_proc pageflip_abort)
+{
+ drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
+ struct ms_crtc_pageflip *flip;
+ struct ms_flipdata *flipdata;
+ RegionRec region;
+ RegionPtr dirty;
+
+ if (!ms_tearfree_is_active_on_crtc(crtc))
+ return FALSE;
+
+ /* Check for damage on the primary scanout to know if TearFree will flip */
+ dirty = DamageRegion(ms->damage);
+ if (RegionNil(dirty))
+ return FALSE;
+
+ /* Compute how much of the current damage intersects with this CRTC */
+ RegionInit(®ion, &crtc->bounds, 0);
+ RegionIntersect(®ion, ®ion, dirty);
+
+ /* No damage on this CRTC means no TearFree flip. This means the DRI client
+ * didn't change this CRTC's contents at all with its presentation, possibly
+ * because its window is fully occluded by another window on this CRTC.
+ */
+ if (RegionNil(®ion))
+ return FALSE;
+
+ flip = calloc(1, sizeof(*flip));
+ if (!flip)
+ return FALSE;
+
+ flipdata = calloc(1, sizeof(*flipdata));
+ if (!flipdata) {
+ free(flip);
+ return FALSE;
+ }
+
+ /* Only track the DRI client's fake flip on the reference CRTC, which aligns
+ * with the behavior of Present when a client copies its pixmap rather than
+ * directly flipping it onto the display.
+ */
+ flip->on_reference_crtc = TRUE;
+ flip->flipdata = flipdata;
+ flip->tearfree_seq = trf->flip_seq;
+ flipdata->screen = xf86ScrnToScreen(crtc->scrn);
+ flipdata->event = event;
+ flipdata->flip_count = 1;
+ flipdata->event_handler = pageflip_handler;
+ flipdata->abort_handler = pageflip_abort;
+
+ /* Keep the list in FIFO order so that clients are notified in order */
+ xorg_list_append(&flip->node, &trf->dri_flip_list);
+ return TRUE;
+}
Bool
ms_do_pageflip(ScreenPtr screen,
@@ -327,6 +388,22 @@ ms_do_pageflip(ScreenPtr screen,
uint32_t flags;
int i;
struct ms_flipdata *flipdata;
+
+ /* A NULL pixmap indicates this DRI client's pixmap is to be flipped through
+ * TearFree instead. The pixmap is already copied to the primary scanout at
+ * this point, so all that's left is to wire up this fake flip to TearFree
+ * so that TearFree can send a notification to the DRI client when the
+ * pixmap actually appears on the display. This is the only way to let DRI
+ * clients accurately know when their pixmaps appear on the display when
+ * TearFree is enabled.
+ */
+ if (!new_front) {
+ if (!ms_tearfree_dri_flip(ms, ref_crtc, event, pageflip_handler,
+ pageflip_abort))
+ goto error_free_event;
+ return TRUE;
+ }
+
ms->glamor.block_handler(screen);
new_front_bo.gbm = ms->glamor.gbm_bo_from_pixmap(screen, new_front);
@@ -478,6 +555,69 @@ error_free_event:
return FALSE;
}
+Bool
+ms_tearfree_dri_abort(xf86CrtcPtr crtc,
+ Bool (*match)(void *data, void *match_data),
+ void *match_data)
+{
+ drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
+ struct ms_crtc_pageflip *flip;
+
+ /* The window is getting destroyed; abort without notifying the client */
+ xorg_list_for_each_entry(flip, &trf->dri_flip_list, node) {
+ if (match(flip->flipdata->event, match_data)) {
+ xorg_list_del(&flip->node);
+ ms_pageflip_abort(flip);
+ return TRUE;
+ }
+ }
+
+ return FALSE;
+}
+
+void
+ms_tearfree_dri_abort_all(xf86CrtcPtr crtc)
+{
+ drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
+ struct ms_crtc_pageflip *flip, *tmp;
+ uint64_t usec = 0, msc = 0;
+
+ /* Nothing to abort if there aren't any DRI clients waiting for a flip */
+ if (xorg_list_is_empty(&trf->dri_flip_list))
+ return;
+
+ /* Even though we're aborting, these clients' pixmaps were actually blitted,
+ * so technically the presentation isn't aborted. That's why the normal
+ * handler is called instead of the abort handler, along with the current
+ * time and MSC for this CRTC.
+ */
+ ms_get_crtc_ust_msc(crtc, &usec, &msc);
+ xorg_list_for_each_entry_safe(flip, tmp, &trf->dri_flip_list, node)
+ ms_pageflip_handler(msc, usec, flip);
+ xorg_list_init(&trf->dri_flip_list);
+}
+
+static void
+ms_tearfree_dri_notify(drmmode_tearfree_ptr trf, uint64_t msc, uint64_t usec)
+{
+ struct ms_crtc_pageflip *flip, *tmp;
+
+ xorg_list_for_each_entry_safe(flip, tmp, &trf->dri_flip_list, node) {
+ /* If a TearFree flip was already pending at the time this DRI client's
+ * pixmap was copied, then the pixmap isn't contained in this TearFree
+ * flip, but will be part of the next TearFree flip instead.
+ */
+ if (flip->tearfree_seq) {
+ flip->tearfree_seq = 0;
+ } else {
+ xorg_list_del(&flip->node);
+ ms_pageflip_handler(msc, usec, flip);
+ }
+ }
+}
+
static void
ms_tearfree_flip_abort(void *data)
{
@@ -486,6 +626,7 @@ ms_tearfree_flip_abort(void *data)
drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
trf->flip_seq = 0;
+ ms_tearfree_dri_abort_all(crtc);
}
static void
@@ -498,6 +639,9 @@ ms_tearfree_flip_handler(uint64_t msc, uint64_t usec, void *data)
/* Swap the buffers and complete the flip */
trf->back_idx ^= 1;
trf->flip_seq = 0;
+
+ /* Notify DRI clients that their pixmaps are now visible on the display */
+ ms_tearfree_dri_notify(trf, msc, usec);
}
Bool
@@ -509,8 +653,14 @@ ms_do_tearfree_flip(ScreenPtr screen, xf86CrtcPtr crtc)
seq = ms_drm_queue_alloc(crtc, crtc, ms_tearfree_flip_handler,
ms_tearfree_flip_abort);
- if (!seq)
+ if (!seq) {
+ /* Need to notify the DRI clients if a sequence wasn't allocated. Once a
+ * sequence is allocated, explicitly performing this cleanup isn't
+ * necessary since it's already done as part of aborting the sequence.
+ */
+ ms_tearfree_dri_abort_all(crtc);
goto no_flip;
+ }
/* Copy the damage to the back buffer and then flip it at the vblank */
drmmode_copy_damage(crtc, trf->buf[idx].px, &trf->buf[idx].dmg, TRUE);
diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c
index 89fca1a9c..3d8f16aa9 100644
--- a/hw/xfree86/drivers/modesetting/present.c
+++ b/hw/xfree86/drivers/modesetting/present.c
@@ -165,6 +165,13 @@ ms_present_abort_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
{
ScreenPtr screen = crtc->pScreen;
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
+#ifdef GLAMOR_HAS_GBM
+ xf86CrtcPtr xf86_crtc = crtc->devPrivate;
+
+ /* Check if this is a fake flip routed through TearFree and abort it */
+ if (ms_tearfree_dri_abort(xf86_crtc, ms_present_event_match, &event_id))
+ return;
+#endif
ms_drm_abort(scrn, ms_present_event_match, &event_id);
}
@@ -364,7 +371,9 @@ ms_present_flip(RRCrtcPtr crtc,
Bool ret;
struct ms_present_vblank_event *event;
- if (!ms_present_check_flip(crtc, ms->flip_window, pixmap, sync_flip, NULL))
+ /* A NULL pixmap means this is a fake flip to be routed through TearFree */
+ if (pixmap &&
+ !ms_present_check_flip(crtc, ms->flip_window, pixmap, sync_flip, NULL))
return FALSE;
event = calloc(1, sizeof(struct ms_present_vblank_event));
@@ -377,6 +386,12 @@ ms_present_flip(RRCrtcPtr crtc,
event->event_id = event_id;
event->unflip = FALSE;
+ /* Register the fake flip (indicated by a NULL pixmap) with TearFree */
+ if (!pixmap)
+ return ms_do_pageflip(screen, NULL, event, xf86_crtc, FALSE,
+ ms_present_flip_handler, ms_present_flip_abort,
+ "Present-TearFree-flip");
+
/* A window can only flip if it covers the entire X screen.
* Only one window can flip at a time.
*
commit 9d1997f72aa431612961cba0e4c2cbf40c07f7c4
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Wed Jan 25 18:06:20 2023 -0800
modesetting: Ensure vblank events always run in sequential order
It is possible for vblank events to run out of order with respect to one
another because the event which was queued to the kernel has the privilege
of running before all other events are handled. This allows kernel-queued
events to run before other, older events which should've run first.
Although this isn't a huge problem now, it will become more problematic
after the next change which ties DRI client notifications to TearFree page
flips. This increases the likelihood of DRI clients erroneously receiving
presentation-completion notifications out of order; i.e., a client could
receive a notification for a newer pixmap it submitted *before* receiving a
notification for an older pixmap.
Ensure vblank events always run in sequential order by removing the bias
towards kernel-queued events, and therefore forcing them to run at their
sequential position in the queue like other events.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/hw/xfree86/drivers/modesetting/vblank.c b/hw/xfree86/drivers/modesetting/vblank.c
index 4d250aa34..1c5f2578f 100644
--- a/hw/xfree86/drivers/modesetting/vblank.c
+++ b/hw/xfree86/drivers/modesetting/vblank.c
@@ -573,10 +573,15 @@ ms_drm_sequence_handler(int fd, uint64_t frame, uint64_t ns, Bool is64bit, uint6
if (q->seq == seq) {
crtc = q->crtc;
msc = ms_kernel_msc_to_crtc_msc(crtc, frame, is64bit);
- xorg_list_del(&q->list);
- if (!q->aborted)
- q->handler(msc, ns / 1000, q->data);
- free(q);
+
+ /* Write the current MSC to this event to ensure its handler runs in
+ * the loop below. This is done because we don't want to run the
+ * handler right now, since we need to ensure all events are handled
+ * in FIFO order with respect to one another. Otherwise, if this
+ * event were handled first just because it was queued to the
+ * kernel, it could run before older events expiring at this MSC.
+ */
+ q->msc = msc;
break;
}
}
commit 18b14ea1f68b0acac4bfb48d51e749db4ec417e4
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Tue Feb 14 19:30:08 2023 -0800
modesetting: Introduce ms_tearfree_is_active_on_crtc helper
There is more than one place with the confusing TearFree state check for a
CRTC. Instead of open-coding the TearFree check everywhere, introduce a
helper, ms_tearfree_is_active_on_crtc, to cover the TearFree state checks.
Suggested-by: Martin Roukala <martin.roukala at mupuf.org>
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
index 3f185489e..5176b6d40 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -655,8 +655,7 @@ ms_tearfree_do_flips(ScreenPtr pScreen)
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
- /* Skip disabled CRTCs and those which aren't using TearFree */
- if (!trf->buf[0].px || !crtc->scrn->vtSema || !xf86_crtc_on(crtc))
+ if (!ms_tearfree_is_active_on_crtc(crtc))
continue;
/* Skip if the last flip is still pending, a DRI client is flipping, or
diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h
index 605fd8530..a54bf28ff 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -249,3 +249,4 @@ Bool ms_do_tearfree_flip(ScreenPtr screen, xf86CrtcPtr crtc);
int ms_flush_drm_events(ScreenPtr screen);
Bool ms_window_has_variable_refresh(modesettingPtr ms, WindowPtr win);
void ms_present_set_screen_vrr(ScrnInfoPtr scrn, Bool vrr_enabled);
+Bool ms_tearfree_is_active_on_crtc(xf86CrtcPtr crtc);
diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c
index 94437c0f3..a804841fa 100644
--- a/hw/xfree86/drivers/modesetting/pageflip.c
+++ b/hw/xfree86/drivers/modesetting/pageflip.c
@@ -529,3 +529,13 @@ no_flip:
return TRUE;
}
#endif
+
+Bool
+ms_tearfree_is_active_on_crtc(xf86CrtcPtr crtc)
+{
+ drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
+
+ /* If TearFree is enabled, XServer owns the VT, and the CRTC is active */
+ return trf->buf[0].px && crtc->scrn->vtSema && xf86_crtc_on(crtc);
+}
diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c
index acc299f3b..89fca1a9c 100644
--- a/hw/xfree86/drivers/modesetting/present.c
+++ b/hw/xfree86/drivers/modesetting/present.c
@@ -334,8 +334,7 @@ no_flip:
drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
- /* Check if TearFree is active on this CRTC and tell Present about it */
- if (trf->buf[0].px && scrn->vtSema && xf86_crtc_on(xf86_crtc)) {
+ if (ms_tearfree_is_active_on_crtc(xf86_crtc)) {
if (trf->flip_seq)
/* The driver has a TearFree flip pending */
*reason = PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING;
commit 8b5fd556582ba040aad5beeeb4c024e5dae40eba
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Sun Jan 22 21:02:45 2023 -0800
modesetting: Improve TearFree state check in ms_present_check_flip
Check that the VT is owned and that the CRTC is on before exporting info to
Present stating that TearFree is available. Also, since `trf->buf[0].px` is
checked, the `ms->drmmode.tearfree_enable` check is redundant and can
therefore be removed.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c
index e147f3eda..acc299f3b 100644
--- a/hw/xfree86/drivers/modesetting/present.c
+++ b/hw/xfree86/drivers/modesetting/present.c
@@ -329,12 +329,13 @@ ms_present_check_flip(RRCrtcPtr crtc,
no_flip:
/* Export some info about TearFree if Present can't flip anyway */
- if (reason && ms->drmmode.tearfree_enable) {
+ if (reason) {
xf86CrtcPtr xf86_crtc = crtc->devPrivate;
drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
- if (trf->buf[0].px) {
+ /* Check if TearFree is active on this CRTC and tell Present about it */
+ if (trf->buf[0].px && scrn->vtSema && xf86_crtc_on(xf86_crtc)) {
if (trf->flip_seq)
/* The driver has a TearFree flip pending */
*reason = PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING;
commit 35975d90546931de10b014a222a6051f34c62e57
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Sun Jan 22 20:59:32 2023 -0800
modesetting: Fix memory leak on ms_do_pageflip error
The event allocation for ms_do_pageflip is leaked on error because callers
of ms_do_pageflip have no way of knowing whether or not a page flip
succeeded for any CRTCs. If a page flip succeeded for at least one CRTC,
then it's not safe for the caller to free the event allocation, and the
allocation won't be leaked. The event allocation is only leaked when not a
single CRTC's page flip succeeded.
Since all callers of ms_do_pageflip allocate the event pointer, and all of
them intentionally leak the event allocation when ms_do_pageflip returns an
error, just free the event pointer inside ms_do_pageflip when a page flip
doesn't succeed for any CRTC.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c
index 08a1bcd31..94437c0f3 100644
--- a/hw/xfree86/drivers/modesetting/pageflip.c
+++ b/hw/xfree86/drivers/modesetting/pageflip.c
@@ -336,7 +336,7 @@ ms_do_pageflip(ScreenPtr screen,
xf86DrvMsg(scrn->scrnIndex, X_ERROR,
"%s: Failed to get GBM BO for flip to new front.\n",
log_prefix);
- return FALSE;
+ goto error_free_event;
}
flipdata = calloc(1, sizeof(struct ms_flipdata));
@@ -344,7 +344,7 @@ ms_do_pageflip(ScreenPtr screen,
drmmode_bo_destroy(&ms->drmmode, &new_front_bo);
xf86DrvMsg(scrn->scrnIndex, X_ERROR,
"%s: Failed to allocate flipdata.\n", log_prefix);
- return FALSE;
+ goto error_free_event;
}
flipdata->event = event;
@@ -465,11 +465,16 @@ error_out:
drmmode_bo_destroy(&ms->drmmode, &new_front_bo);
/* if only the local reference - free the structure,
* else drop the local reference and return */
- if (flipdata->flip_count == 1)
+ if (flipdata->flip_count == 1) {
free(flipdata);
- else
+ } else {
flipdata->flip_count--;
+ return FALSE;
+ }
+error_free_event:
+ /* Free the event since the caller has no way to know it's safe to free */
+ free(event);
return FALSE;
}
commit be864d8e185fe9e699fa661e70e02b319384dee6
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Sun Jan 22 20:57:14 2023 -0800
modesetting: Pass CRTC pointer to TearFree flip handlers
The CRTC pointer will soon be needed in the TearFree flip handlers, so pass
it in instead of passing in drmmode_tearfree_ptr.
No functional change.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c
index b0ff9655b..08a1bcd31 100644
--- a/hw/xfree86/drivers/modesetting/pageflip.c
+++ b/hw/xfree86/drivers/modesetting/pageflip.c
@@ -476,7 +476,9 @@ error_out:
static void
ms_tearfree_flip_abort(void *data)
{
- drmmode_tearfree_ptr trf = data;
+ xf86CrtcPtr crtc = data;
+ drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
trf->flip_seq = 0;
}
@@ -484,7 +486,9 @@ ms_tearfree_flip_abort(void *data)
static void
ms_tearfree_flip_handler(uint64_t msc, uint64_t usec, void *data)
{
- drmmode_tearfree_ptr trf = data;
+ xf86CrtcPtr crtc = data;
+ drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
/* Swap the buffers and complete the flip */
trf->back_idx ^= 1;
@@ -498,7 +502,7 @@ ms_do_tearfree_flip(ScreenPtr screen, xf86CrtcPtr crtc)
drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree;
uint32_t idx = trf->back_idx, seq;
- seq = ms_drm_queue_alloc(crtc, trf, ms_tearfree_flip_handler,
+ seq = ms_drm_queue_alloc(crtc, crtc, ms_tearfree_flip_handler,
ms_tearfree_flip_abort);
if (!seq)
goto no_flip;
commit 1fd9d79ae0dac5ae5f84f8012aa2764fcb777ea1
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Sun Jan 22 14:58:23 2023 -0800
modesetting: Pass reference CRTC pointer to ms_do_pageflip
Rather than passing the reference CRTC's vblank pipe to ms_do_pageflip,
just pass the pointer to the reference CRTC directly instead. This is
clearer and more useful than the vblank pipe, since the vblank pipe is only
used to identify whether or not a given CRTC is the reference CRTC.
No functional change.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/hw/xfree86/drivers/modesetting/dri2.c b/hw/xfree86/drivers/modesetting/dri2.c
index 8d1b742ef..34ddec424 100644
--- a/hw/xfree86/drivers/modesetting/dri2.c
+++ b/hw/xfree86/drivers/modesetting/dri2.c
@@ -483,7 +483,6 @@ ms_dri2_schedule_flip(ms_dri2_frame_event_ptr info)
modesettingPtr ms = modesettingPTR(scrn);
ms_dri2_buffer_private_ptr back_priv = info->back->driverPrivate;
struct ms_dri2_vblank_event *event;
- drmmode_crtc_private_ptr drmmode_crtc = info->crtc->driver_private;
event = calloc(1, sizeof(struct ms_dri2_vblank_event));
if (!event)
@@ -495,7 +494,7 @@ ms_dri2_schedule_flip(ms_dri2_frame_event_ptr info)
event->event_data = info->event_data;
if (ms_do_pageflip(screen, back_priv->pixmap, event,
- drmmode_crtc->vblank_pipe, FALSE,
+ info->crtc, FALSE,
ms_dri2_flip_handler,
ms_dri2_flip_abort,
"DRI2-flip")) {
diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h
index 3f2b1d1ae..605fd8530 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -236,7 +236,7 @@ typedef void (*ms_pageflip_abort_proc)(modesettingPtr ms, void *data);
Bool ms_do_pageflip(ScreenPtr screen,
PixmapPtr new_front,
void *event,
- int ref_crtc_vblank_pipe,
+ xf86CrtcPtr ref_crtc,
Bool async,
ms_pageflip_handler_proc pageflip_handler,
ms_pageflip_abort_proc pageflip_abort,
diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c
index 5a4ad1dba..b0ff9655b 100644
--- a/hw/xfree86/drivers/modesetting/pageflip.c
+++ b/hw/xfree86/drivers/modesetting/pageflip.c
@@ -204,11 +204,10 @@ enum queue_flip_status {
static int
queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,
struct ms_flipdata *flipdata,
- int ref_crtc_vblank_pipe, uint32_t flags)
+ xf86CrtcPtr ref_crtc, uint32_t flags)
{
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
modesettingPtr ms = modesettingPTR(scrn);
- drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
struct ms_crtc_pageflip *flip;
uint32_t seq;
@@ -220,7 +219,7 @@ queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,
/* Only the reference crtc will finally deliver its page flip
* completion event. All other crtc's events will be discarded.
*/
- flip->on_reference_crtc = (drmmode_crtc->vblank_pipe == ref_crtc_vblank_pipe);
+ flip->on_reference_crtc = crtc == ref_crtc;
flip->flipdata = flipdata;
seq = ms_drm_queue_alloc(crtc, flip, ms_pageflip_handler, ms_pageflip_abort);
@@ -315,7 +314,7 @@ Bool
ms_do_pageflip(ScreenPtr screen,
PixmapPtr new_front,
void *event,
- int ref_crtc_vblank_pipe,
+ xf86CrtcPtr ref_crtc,
Bool async,
ms_pageflip_handler_proc pageflip_handler,
ms_pageflip_abort_proc pageflip_abort,
@@ -393,7 +392,6 @@ ms_do_pageflip(ScreenPtr screen,
for (i = 0; i < config->num_crtc; i++) {
enum queue_flip_status flip_status;
xf86CrtcPtr crtc = config->crtc[i];
- drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
if (!xf86_crtc_on(crtc))
continue;
@@ -414,13 +412,11 @@ ms_do_pageflip(ScreenPtr screen,
* outputs in a "clone-mode" or "mirror-mode" configuration.
*/
if (ms->drmmode.can_async_flip && ms->drmmode.async_flip_secondaries &&
- (drmmode_crtc->vblank_pipe != ref_crtc_vblank_pipe) &&
- (ref_crtc_vblank_pipe >= 0))
+ ref_crtc && crtc != ref_crtc)
flags |= DRM_MODE_PAGE_FLIP_ASYNC;
flip_status = queue_flip_on_crtc(screen, crtc, flipdata,
- ref_crtc_vblank_pipe,
- flags);
+ ref_crtc, flags);
switch (flip_status) {
case QUEUE_FLIP_ALLOC_FAILED:
diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c
index 642f7baaf..e147f3eda 100644
--- a/hw/xfree86/drivers/modesetting/present.c
+++ b/hw/xfree86/drivers/modesetting/present.c
@@ -361,7 +361,6 @@ ms_present_flip(RRCrtcPtr crtc,
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
modesettingPtr ms = modesettingPTR(scrn);
xf86CrtcPtr xf86_crtc = crtc->devPrivate;
- drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
Bool ret;
struct ms_present_vblank_event *event;
@@ -389,7 +388,7 @@ ms_present_flip(RRCrtcPtr crtc,
ms_present_set_screen_vrr(scrn, TRUE);
}
- ret = ms_do_pageflip(screen, pixmap, event, drmmode_crtc->vblank_pipe, !sync_flip,
+ ret = ms_do_pageflip(screen, pixmap, event, xf86_crtc, !sync_flip,
ms_present_flip_handler, ms_present_flip_abort,
"Present-flip");
if (ret)
@@ -421,7 +420,7 @@ ms_present_unflip(ScreenPtr screen, uint64_t event_id)
event->unflip = TRUE;
if (ms_present_check_unflip(NULL, screen->root, pixmap, TRUE, NULL) &&
- ms_do_pageflip(screen, pixmap, event, -1, FALSE,
+ ms_do_pageflip(screen, pixmap, event, NULL, FALSE,
ms_present_flip_handler, ms_present_flip_abort,
"Present-unflip")) {
return;
commit 7288b4d1051a4902384fa2b3c12cacbd3caf04b0
Author: Sultan Alsawaf <sultan at kerneltoast.com>
Date: Sun Jan 22 14:56:36 2023 -0800
modesetting: Remove redundant GLAMOR_HAS_GBM #ifdef from ms_do_pageflip
This #ifdef is redundant since ms_do_pageflip is already enclosed within a
larger GLAMOR_HAS_GBM #ifdef.
No functional change.
Signed-off-by: Sultan Alsawaf <sultan at kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala at mupuf.org>
diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c
index 8d57047ef..5a4ad1dba 100644
--- a/hw/xfree86/drivers/modesetting/pageflip.c
+++ b/hw/xfree86/drivers/modesetting/pageflip.c
@@ -321,9 +321,6 @@ ms_do_pageflip(ScreenPtr screen,
ms_pageflip_abort_proc pageflip_abort,
const char *log_prefix)
{
-#ifndef GLAMOR_HAS_GBM
- return FALSE;
-#else
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
modesettingPtr ms = modesettingPTR(scrn);
xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -478,7 +475,6 @@ error_out:
flipdata->flip_count--;
return FALSE;
-#endif /* GLAMOR_HAS_GBM */
}
static void
More information about the xorg-commit
mailing list