[Mesa-dev] [RFC 1/2] glx|egl: allow to test if glthread is safe enough on X11 platform
Nicolai Hähnle
nhaehnle at gmail.com
Wed May 3 15:22:01 UTC 2017
On 28.04.2017 23:11, Gregory Hainaut wrote:
> I extended the struct __DRIbackgroundCallableExtensionRec because
> the other function pointer is already related for glthread.
>
> DRI2/DRI3 glx code path check that display can be locked (basically
> XInitThread was called)
>
> EGL code path is more tricky as we don't want to pull X11 header. Instead
> the code will assume that it is safe if X11 isn't used or there is no display
> (i.e. 100% XCB)
>
> The new function will be used in the next commit
>
> Signed-off-by: Gregory Hainaut <gregory.hainaut at gmail.com>
> ---
> include/GL/internal/dri_interface.h | 9 +++++++++
> src/egl/drivers/dri2/egl_dri2.c | 30 ++++++++++++++++++++++++++++++
> src/glx/dri2_glx.c | 9 +++++++++
> src/glx/dri3_glx.c | 8 ++++++++
> 4 files changed, 56 insertions(+)
>
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index 86efd1bdc9..28a52ccdb9 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -1713,13 +1713,22 @@ struct __DRIbackgroundCallableExtensionRec {
> * non-background thread (i.e. a thread that has already been bound to a
> * context using __DRIcoreExtensionRec::bindContext()); when this happens,
> * the \c loaderPrivate pointer must be equal to the pointer that was
> * passed to the driver when the currently bound context was created.
> *
> * This call should execute quickly enough that the driver can call it with
> * impunity whenever a background thread starts performing drawing
> * operations (e.g. it should just set a thread-local variable).
> */
> void (*setBackgroundContext)(void *loaderPrivate);
Add a space here.
> + /**
> + * Indicate that it is multithread safe to use glthread. Typically
> + * XInitThread was called in GLX setup.
> + *
> + * \param loaderPrivate is the value that was passed to to the driver when
> + * the context was created. This can be used by the loader to identify
> + * which context any callbacks are associated with.
> + */
> + GLboolean (*isGlThreadSafe)(void *loaderPrivate);
> };
>
> #endif
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 2cab7d00c1..df2db97bcf 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -85,24 +85,54 @@
>
> static void
> dri_set_background_context(void *loaderPrivate)
> {
> _EGLContext *ctx = _eglGetCurrentContext();
> _EGLThreadInfo *t = _eglGetCurrentThread();
>
> _eglBindContextToThread(ctx, t);
> }
>
> +static GLboolean
> +dri_is_glthread_safe(void *loaderPrivate)
> +{
> +#ifdef HAVE_X11_PLATFORM
> + struct dri2_egl_surface *dri2_surf = loaderPrivate;
> + _EGLDisplay *display = dri2_surf->base.Resource.Display;
> + Display *dpy;
> +
> + // Only the libX11 isn't safe
> + if (display->Platform != _EGL_PLATFORM_X11)
> + return true;
> +
> + // Will use pure XCB so no libX11 here either
> + if (display->PlatformDisplay == NULL)
> + return true;
> +
> + // In an ideal world we would check the X11 lock pointer
> + // (display->PlatformDisplay->lock_fns). Unfortunately it
> + // requires to know the full type. And we don't want to bring X11
> + // headers here.
> + //
> + // So let's assume an unsafe behavior. Modern EGL code shouldn't use
> + // libX11 anyway.
Change the comment style.
> + return false;
> +#else
> + return true;
> +#endif
> +}
> +
> const __DRIbackgroundCallableExtension background_callable_extension = {
> .base = { __DRI_BACKGROUND_CALLABLE, 1 },
>
> .setBackgroundContext = dri_set_background_context,
> + .isGlThreadSafe = dri_is_glthread_safe,
> };
You're adding a function pointer, so bump the extension version.
>
> const __DRIuseInvalidateExtension use_invalidate = {
> .base = { __DRI_USE_INVALIDATE, 1 }
> };
>
> EGLint dri2_to_egl_attribute_map[] = {
> 0,
> EGL_BUFFER_SIZE, /* __DRI_ATTRIB_BUFFER_SIZE */
> EGL_LEVEL, /* __DRI_ATTRIB_LEVEL */
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index 145f44d6e8..8f4d2f027f 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -946,20 +946,28 @@ dri2GetSwapInterval(__GLXDRIdrawable *pdraw)
> return priv->swap_interval;
> }
>
> static void
> driSetBackgroundContext(void *loaderPrivate)
> {
> struct dri2_context *pcp = (struct dri2_context *) loaderPrivate;
> __glXSetCurrentContext(&pcp->base);
> }
>
> +static GLboolean
> +driIsGlThreadSafe(void *loaderPrivate)
> +{
> + struct dri2_context *pcp = (struct dri2_context *) loaderPrivate;
> + return pcp->base.psc->dpy->lock_fns != NULL;
> +}
> +
> +
> static const __DRIdri2LoaderExtension dri2LoaderExtension = {
> .base = { __DRI_DRI2_LOADER, 3 },
>
> .getBuffers = dri2GetBuffers,
> .flushFrontBuffer = dri2FlushFrontBuffer,
> .getBuffersWithFormat = dri2GetBuffersWithFormat,
> };
>
> static const __DRIdri2LoaderExtension dri2LoaderExtension_old = {
> .base = { __DRI_DRI2_LOADER, 3 },
> @@ -970,20 +978,21 @@ static const __DRIdri2LoaderExtension dri2LoaderExtension_old = {
> };
>
> static const __DRIuseInvalidateExtension dri2UseInvalidate = {
> .base = { __DRI_USE_INVALIDATE, 1 }
> };
>
> static const __DRIbackgroundCallableExtension driBackgroundCallable = {
> .base = { __DRI_BACKGROUND_CALLABLE, 1 },
>
> .setBackgroundContext = driSetBackgroundContext,
> + .isGlThreadSafe = driIsGlThreadSafe,
> };
Again, bump the extension version.
>
> _X_HIDDEN void
> dri2InvalidateBuffers(Display *dpy, XID drawable)
> {
> __GLXDRIdrawable *pdraw =
> dri2GetGlxDrawableFromXDrawableId(dpy, drawable);
> struct dri2_screen *psc;
> struct dri2_drawable *pdp = (struct dri2_drawable *) pdraw;
>
> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
> index e1dc5aa4a8..cebf22059d 100644
> --- a/src/glx/dri3_glx.c
> +++ b/src/glx/dri3_glx.c
> @@ -496,37 +496,45 @@ dri3_flush_front_buffer(__DRIdrawable *driDrawable, void *loaderPrivate)
> loader_dri3_wait_gl(draw);
> }
>
> static void
> dri_set_background_context(void *loaderPrivate)
> {
> struct dri3_context *pcp = (struct dri3_context *)loaderPrivate;
> __glXSetCurrentContext(&pcp->base);
> }
>
> +static GLboolean
> +dri_is_glthread_safe(void *loaderPrivate)
> +{
> + struct dri3_context *pcp = (struct dri3_context *) loaderPrivate;
> + return pcp->base.psc->dpy->lock_fns != NULL;
> +}
> +
> /* The image loader extension record for DRI3
> */
> static const __DRIimageLoaderExtension imageLoaderExtension = {
> .base = { __DRI_IMAGE_LOADER, 1 },
>
> .getBuffers = loader_dri3_get_buffers,
> .flushFrontBuffer = dri3_flush_front_buffer,
> };
>
> const __DRIuseInvalidateExtension dri3UseInvalidate = {
> .base = { __DRI_USE_INVALIDATE, 1 }
> };
>
> static const __DRIbackgroundCallableExtension driBackgroundCallable = {
> .base = { __DRI_BACKGROUND_CALLABLE, 1 },
>
> .setBackgroundContext = dri_set_background_context,
> + .isGlThreadSafe = dri_is_glthread_safe,
> };
Same here.
Cheers,
Nicolai
>
> static const __DRIextension *loader_extensions[] = {
> &imageLoaderExtension.base,
> &dri3UseInvalidate.base,
> &driBackgroundCallable.base,
> NULL
> };
>
> /** dri3_swap_buffers
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list