From 7de5188342e965ad8e2f11cd666e08487e65790a Mon Sep 17 00:00:00 2001 From: Faith Ekstrand Date: Mon, 7 Jul 2025 16:20:22 -0400 Subject: [PATCH] egl/x11: Rework LIBGL_KOPPER_DRI2 checks This moves the multibuffers check earlier to make it clear that this is an early-exit. We also delete kopper_without_multibuffers in favor of a local variable that doesn't leak all over everywhere. The new variable is named force_zink because that's really what we're doing: force-enable Zink even if the X11 checks fail. Reviewed-by: Adam Jackson Reviewed-By: Mike Blumenkrantz Part-of: --- src/egl/drivers/dri2/egl_dri2.c | 1 - src/egl/drivers/dri2/egl_dri2.h | 1 - src/egl/drivers/dri2/platform_x11.c | 47 +++++++++++++++++------------ 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index 2798a358d28..7efe0e807e3 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -1044,7 +1044,6 @@ dri2_display_create(_EGLDisplay *disp) dri2_dpy->fd_display_gpu = -1; dri2_dpy->multibuffers_available = true; dri2_dpy->kopper = disp->Options.Zink && !debug_get_bool_option("LIBGL_KOPPER_DISABLE", false); - dri2_dpy->kopper_without_modifiers = dri2_dpy->kopper && debug_get_bool_option("LIBGL_KOPPER_DRI2", false); disp->DriverData = (void *)dri2_dpy; return dri2_dpy; diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index 8524726c1ab..abf03b8b51e 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -240,7 +240,6 @@ struct dri2_egl_display { bool has_compression_modifiers; bool own_device; bool kopper; - bool kopper_without_modifiers; bool swrast; bool swrast_not_kms; int min_swap_interval; diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c index 3cef7330482..c2aa66cca05 100644 --- a/src/egl/drivers/dri2/platform_x11.c +++ b/src/egl/drivers/dri2/platform_x11.c @@ -1205,14 +1205,14 @@ check_xshm(struct dri2_egl_display *dri2_dpy) } static bool -platform_x11_finalize(_EGLDisplay *disp) +platform_x11_finalize(_EGLDisplay *disp, bool force_zink) { struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); if (!dri2_create_screen(disp)) return false; - if (!dri2_setup_device(disp, disp->Options.ForceSoftware || dri2_dpy->kopper_without_modifiers)) { + if (!dri2_setup_device(disp, disp->Options.ForceSoftware || force_zink)) { _eglError(EGL_NOT_INITIALIZED, "DRI2: failed to setup EGLDevice"); return false; } @@ -1246,7 +1246,7 @@ platform_x11_finalize(_EGLDisplay *disp) } static EGLBoolean -dri2_initialize_x11_swrast(_EGLDisplay *disp) +dri2_initialize_x11_swrast(_EGLDisplay *disp, bool force_zink) { struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); @@ -1258,7 +1258,7 @@ dri2_initialize_x11_swrast(_EGLDisplay *disp) dri2_dpy->loader_extensions = swrast_loader_extensions; } - if (!platform_x11_finalize(disp)) + if (!platform_x11_finalize(disp, force_zink)) return EGL_FALSE; /* Fill vtbl last to prevent accidentally calling virtual function during @@ -1287,7 +1287,7 @@ dri2_initialize_x11_dri3(_EGLDisplay *disp) dri2_dpy->loader_extensions = dri3_image_loader_extensions; - if (!platform_x11_finalize(disp)) + if (!platform_x11_finalize(disp, false)) return EGL_FALSE; loader_init_screen_resources(&dri2_dpy->screen_resources, dri2_dpy->conn, @@ -1309,6 +1309,12 @@ dri2_initialize_x11(_EGLDisplay *disp) { struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); + /* True if we're going to force-enable a HW Zink driver, even if the X + * server is missing a bunch of features. + */ + const bool force_zink = + disp->Options.Zink && debug_get_bool_option("LIBGL_KOPPER_DRI2", false); + /* * Every hardware driver_name is set using strdup. Doing the same in * here will allow is to simply free the memory at dri2_terminate(). @@ -1321,11 +1327,23 @@ dri2_initialize_x11(_EGLDisplay *disp) if (!dri2_get_xcb_connection(disp, dri2_dpy)) return EGL_FALSE; -#ifdef HAVE_LIBDRM - /* kopper_without_modifiers really means (zink && LIBGL_DRI3_DISABLE=true), - * so this conditional is effectively "is dri3 enabled" +#ifdef HAVE_X11_DRM + dri2_dpy->multibuffers_available = x11_dri3_has_multibuffer(dri2_dpy->conn); + + /* If we've selected Zink and we're not taking the swrast path then we need + * multibuffers or else import won't work. We shouldn't enable Zink in + * this case. The user is allowed to override this with LIBGL_KOPPER_DRI2. */ - if (!disp->Options.Zink || !debug_get_bool_option("LIBGL_KOPPER_DRI2", false)) { + if (disp->Options.Zink && !disp->Options.ForceSoftware && + !force_zink && !dri2_dpy->multibuffers_available) + return EGL_FALSE; +#endif + +#ifdef HAVE_LIBDRM + /* If LIBGL_KOPPER_DRI2 is enabled, skip the X11 render device checks. + * We're going to enable Zink anyway. + */ + if (!force_zink) { bool status = dri3_x11_connect(dri2_dpy, disp->Options.Zink, disp->Options.ForceSoftware); /* the status here is ignored for zink-with-kopper and swrast, * otherwise return whatever error/fallback status as failure @@ -1336,17 +1354,8 @@ dri2_initialize_x11(_EGLDisplay *disp) #endif dri2_detect_swrast(disp); -#ifdef HAVE_X11_DRM - dri2_dpy->multibuffers_available = x11_dri3_has_multibuffer(dri2_dpy->conn); - - if (disp->Options.Zink && !disp->Options.ForceSoftware && - !dri2_dpy->multibuffers_available && - !dri2_dpy->kopper_without_modifiers) - return EGL_FALSE; -#endif - if (disp->Options.ForceSoftware || dri2_dpy->kopper) - return dri2_initialize_x11_swrast(disp); + return dri2_initialize_x11_swrast(disp, force_zink); #ifdef HAVE_LIBDRM if (dri2_initialize_x11_dri3(disp))