xserver: Branch 'xwayland-23.1' - 3 commits
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Tue Mar 28 19:08:20 UTC 2023
glamor/glamor.c | 7 ++-----
glamor/glamor_priv.h | 1 +
glamor/glamor_sync.c | 3 +--
glamor/glamor_utils.h | 11 +++++++++++
hw/xwayland/xwayland-present.c | 27 +++++++++++++++++++++++++++
hw/xwayland/xwayland-present.h | 3 +++
hw/xwayland/xwayland-window.c | 39 +++++++++++++++------------------------
7 files changed, 60 insertions(+), 31 deletions(-)
New commits:
commit 468e2d90b4c9358c16d1c397dc8d6e23efe07a88
Author: Michel Dänzer <mdaenzer at redhat.com>
Date: Fri Mar 10 12:39:30 2023 +0100
xwayland: Prevent nested xwl_present_for_each_frame_callback calls
It could happen with the following call path:
frame_callback
xwl_present_frame_callback
xwl_present_msc_bump
xwl_present_execute
xwl_present_flip
xwl_window_create_frame_callback
The nested loop called xwl_present_reset_timer, which may end up calling
xorg_list_del for the entry after the one frame_callback started the
chain for. This resulted in the outer loop never terminating, because
its next element wasn't hooked up to the list anymore.
We avoid this by calling xwl_present_reset_timer as needed in
frame_callback, and bailing from xwl_window_create_frame_callback if it
was called from the former.
We also catch nested calls and FatalError if they ever happen again due
to another bug.
v2:
* Leave xwl_present_reset_timer call in xwl_present_frame_callback,
needed if xwl_present_msc_bump didn't hook up the window to the frame
callback list again.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1442
(cherry picked from commit 754d6b6dd0a9ec42d75247596a8885bf54352ee7)
diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index e8a173774..189e7cfd6 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -89,16 +89,31 @@ xwl_present_event_from_id(uint64_t event_id)
return (struct xwl_present_event*)(uintptr_t)event_id;
}
+static Bool entered_for_each_frame_callback;
+
+Bool
+xwl_present_entered_for_each_frame_callback(void)
+{
+ return entered_for_each_frame_callback;
+}
+
void
xwl_present_for_each_frame_callback(struct xwl_window *xwl_window,
void iter_func(struct xwl_present_window *))
{
struct xwl_present_window *xwl_present_window, *tmp;
+ if (entered_for_each_frame_callback)
+ FatalError("Nested xwl_present_for_each_frame_callback call");
+
+ entered_for_each_frame_callback = TRUE;
+
xorg_list_for_each_entry_safe(xwl_present_window, tmp,
&xwl_window->frame_callback_list,
frame_callback_list)
iter_func(xwl_present_window);
+
+ entered_for_each_frame_callback = FALSE;
}
static void
diff --git a/hw/xwayland/xwayland-present.h b/hw/xwayland/xwayland-present.h
index 620e6c5ca..806272089 100644
--- a/hw/xwayland/xwayland-present.h
+++ b/hw/xwayland/xwayland-present.h
@@ -61,6 +61,7 @@ struct xwl_present_event {
PixmapPtr pixmap;
};
+Bool xwl_present_entered_for_each_frame_callback(void);
void xwl_present_for_each_frame_callback(struct xwl_window *xwl_window,
void iter_func(struct xwl_present_window *));
void xwl_present_reset_timer(struct xwl_present_window *xwl_present_window);
diff --git a/hw/xwayland/xwayland-window.c b/hw/xwayland/xwayland-window.c
index b80a7bf67..6b7f38605 100644
--- a/hw/xwayland/xwayland-window.c
+++ b/hw/xwayland/xwayland-window.c
@@ -1226,8 +1226,16 @@ frame_callback(void *data,
xwl_window->frame_callback = NULL;
#ifdef GLAMOR_HAS_GBM
- if (xwl_window->xwl_screen->present)
+ if (xwl_window->xwl_screen->present) {
xwl_present_for_each_frame_callback(xwl_window, xwl_present_frame_callback);
+
+ /* If xwl_window_create_frame_callback was called from
+ * xwl_present_frame_callback, need to make sure all fallback timers
+ * are adjusted correspondingly.
+ */
+ if (xwl_window->frame_callback)
+ xwl_present_for_each_frame_callback(xwl_window, xwl_present_reset_timer);
+ }
#endif
}
@@ -1243,7 +1251,11 @@ xwl_window_create_frame_callback(struct xwl_window *xwl_window)
xwl_window);
#ifdef GLAMOR_HAS_GBM
- if (xwl_window->xwl_screen->present)
+ /* If we get called from frame_callback, it will take care of calling
+ * xwl_present_reset_timer.
+ */
+ if (xwl_window->xwl_screen->present &&
+ !xwl_present_entered_for_each_frame_callback())
xwl_present_for_each_frame_callback(xwl_window, xwl_present_reset_timer);
#endif
}
commit 95119c9c60cac486b7191de2df4dc3445b56dbd1
Author: Michel Dänzer <mdaenzer at redhat.com>
Date: Thu Mar 9 18:44:25 2023 +0100
xwayland: Refactor xwl_present_for_each_frame_callback helper
Preparation for following changes, no functional change intended.
(cherry picked from commit 4d1cd7cdc22013ed8de17d3218b9790b7027e1fe)
diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index 2c0e1a05c..e8a173774 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -89,6 +89,18 @@ xwl_present_event_from_id(uint64_t event_id)
return (struct xwl_present_event*)(uintptr_t)event_id;
}
+void
+xwl_present_for_each_frame_callback(struct xwl_window *xwl_window,
+ void iter_func(struct xwl_present_window *))
+{
+ struct xwl_present_window *xwl_present_window, *tmp;
+
+ xorg_list_for_each_entry_safe(xwl_present_window, tmp,
+ &xwl_window->frame_callback_list,
+ frame_callback_list)
+ iter_func(xwl_present_window);
+}
+
static void
xwl_present_free_timer(struct xwl_present_window *xwl_present_window)
{
diff --git a/hw/xwayland/xwayland-present.h b/hw/xwayland/xwayland-present.h
index ab7f04a3b..620e6c5ca 100644
--- a/hw/xwayland/xwayland-present.h
+++ b/hw/xwayland/xwayland-present.h
@@ -61,6 +61,8 @@ struct xwl_present_event {
PixmapPtr pixmap;
};
+void xwl_present_for_each_frame_callback(struct xwl_window *xwl_window,
+ void iter_func(struct xwl_present_window *));
void xwl_present_reset_timer(struct xwl_present_window *xwl_present_window);
void xwl_present_frame_callback(struct xwl_present_window *xwl_present_window);
Bool xwl_present_init(ScreenPtr screen);
diff --git a/hw/xwayland/xwayland-window.c b/hw/xwayland/xwayland-window.c
index e159743a0..b80a7bf67 100644
--- a/hw/xwayland/xwayland-window.c
+++ b/hw/xwayland/xwayland-window.c
@@ -1090,15 +1090,8 @@ xwl_unrealize_window(WindowPtr window)
xwl_dmabuf_feedback_destroy(&xwl_window->feedback);
#ifdef GLAMOR_HAS_GBM
- if (xwl_screen->present) {
- struct xwl_present_window *xwl_present_window, *tmp;
-
- xorg_list_for_each_entry_safe(xwl_present_window, tmp,
- &xwl_window->frame_callback_list,
- frame_callback_list) {
- xwl_present_unrealize_window(xwl_present_window);
- }
- }
+ if (xwl_window->xwl_screen->present)
+ xwl_present_for_each_frame_callback(xwl_window, xwl_present_unrealize_window);
#endif
release_wl_surface_for_window(xwl_window);
@@ -1233,15 +1226,8 @@ frame_callback(void *data,
xwl_window->frame_callback = NULL;
#ifdef GLAMOR_HAS_GBM
- if (xwl_window->xwl_screen->present) {
- struct xwl_present_window *xwl_present_window, *tmp;
-
- xorg_list_for_each_entry_safe(xwl_present_window, tmp,
- &xwl_window->frame_callback_list,
- frame_callback_list) {
- xwl_present_frame_callback(xwl_present_window);
- }
- }
+ if (xwl_window->xwl_screen->present)
+ xwl_present_for_each_frame_callback(xwl_window, xwl_present_frame_callback);
#endif
}
@@ -1257,15 +1243,8 @@ xwl_window_create_frame_callback(struct xwl_window *xwl_window)
xwl_window);
#ifdef GLAMOR_HAS_GBM
- if (xwl_window->xwl_screen->present) {
- struct xwl_present_window *xwl_present_window, *tmp;
-
- xorg_list_for_each_entry_safe(xwl_present_window, tmp,
- &xwl_window->frame_callback_list,
- frame_callback_list) {
- xwl_present_reset_timer(xwl_present_window);
- }
- }
+ if (xwl_window->xwl_screen->present)
+ xwl_present_for_each_frame_callback(xwl_window, xwl_present_reset_timer);
#endif
}
commit f217a68d59659d7d685322ea56fec42d80703326
Author: Joshua Ashton <joshua at froggi.es>
Date: Fri Mar 17 12:31:44 2023 +0000
glamor: Don't glFlush/ctx switch unless any work has been performed
`glamor_make_current` is always called before any calls to GL.
Apply some dirty-tracking to whenever we call `glamor_make_current` so
that we can avoid a decent amount of redundant GL work on each
Dispatch cycle.
Gamescope previously was waking up an empty Xwayland server with an
XQueryPointer and I noticed a significant amount of churn doing
redundant GL work.
This has been addressed on the Gamescope side as well, but avoiding any
useless GL context switches and flushes when glamor is doing nothing
is still beneficial for CPU and power usage on portable devices.
Signed-off-by: Joshua Ashton <joshua at froggi.es>
Reviewed-by: Emma Anholt <emma at anholt.net>
Acked-by: Olivier Fourdan <ofourdan at redhat.com
(cherry picked from commit 89163917e1976db4ae4863ad5337fa14bf348081)
diff --git a/glamor/glamor.c b/glamor/glamor.c
index f15b5a18a..0cbc89ee4 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -271,9 +271,7 @@ void
glamor_block_handler(ScreenPtr screen)
{
glamor_screen_private *glamor_priv = glamor_get_screen_private(screen);
-
- glamor_make_current(glamor_priv);
- glFlush();
+ glamor_flush(glamor_priv);
}
static void
@@ -281,8 +279,7 @@ _glamor_block_handler(ScreenPtr screen, void *timeout)
{
glamor_screen_private *glamor_priv = glamor_get_screen_private(screen);
- glamor_make_current(glamor_priv);
- glFlush();
+ glamor_flush(glamor_priv);
screen->BlockHandler = glamor_priv->saved_procs.block_handler;
screen->BlockHandler(screen, timeout);
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index da20bc5aa..1032b880b 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -314,6 +314,7 @@ typedef struct glamor_screen_private {
Bool suppress_gl_out_of_memory_logging;
Bool logged_any_fbo_allocation_failure;
Bool logged_any_pbo_allocation_failure;
+ Bool dirty;
/* xv */
glamor_program xv_prog;
diff --git a/glamor/glamor_sync.c b/glamor/glamor_sync.c
index 907e0c613..3f98be400 100644
--- a/glamor/glamor_sync.c
+++ b/glamor/glamor_sync.c
@@ -52,8 +52,7 @@ glamor_sync_fence_set_triggered (SyncFence *fence)
struct glamor_sync_fence *glamor_fence = glamor_get_sync_fence(fence);
/* Flush pending rendering operations */
- glamor_make_current(glamor);
- glFlush();
+ glamor_flush(glamor);
fence->funcs.SetTriggered = glamor_fence->set_triggered;
fence->funcs.SetTriggered(fence);
diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h
index 93a933eed..bee48d989 100644
--- a/glamor/glamor_utils.h
+++ b/glamor/glamor_utils.h
@@ -672,6 +672,17 @@ glamor_make_current(glamor_screen_private *glamor_priv)
lastGLContext = glamor_priv->ctx.ctx;
glamor_priv->ctx.make_current(&glamor_priv->ctx);
}
+ glamor_priv->dirty = TRUE;
+}
+
+static inline void
+glamor_flush(glamor_screen_private *glamor_priv)
+{
+ if (glamor_priv->dirty) {
+ glamor_make_current(glamor_priv);
+ glFlush();
+ glamor_priv->dirty = FALSE;
+ }
}
static inline BoxRec
More information about the xorg-commit
mailing list