[Mesa-dev] [PATCH v4 1/4] dri: Extend __DRIbackgroundCallableExtensionRec to include a callback that checks for thread safety

[Marek Olšák]

On Sun, May 21, 2017 at 4:59 PM, Gregory Hainaut
<gregory.hainaut at gmail.com> wrote:
> DRI-drivers could call Xlib functions, for example to allocate a new back
> buffer.
>
> When glthread is enabled, the driver runs mostly on a separate thread.
> Therefore we need to guarantee the thread safety between libX11 calls
> from the applications (not aware of the extra thread) and the ones from
> the driver.
>
> See discussion thread:
>    https://lists.freedesktop.org/archives/mesa-dev/2017-April/152547.html
>
> Fortunately, Xlib allows to lock display to ensure thread safety but
> XInitThreads must be called first by the application to initialize the lock
> function pointer. This patch will allow to check XInitThreads was called
> to allow glthread on GLX or EGL platform.
>
> Note: a tentative was done to port libX11 code to XCB but it didn't solve fully
> thread safety.
> See discussion thread:
>    https://lists.freedesktop.org/archives/mesa-dev/2017-April/153137.html
>
> Note: Nvidia forces the driver to call XInitThreads. Quoting their manpage:
> "The NVIDIA OpenGL driver will automatically attempt to enable Xlib
> thread-safe mode if needed. However, it might not be possible in some
> situations, such as when the NVIDIA OpenGL driver library is dynamically
> loaded after Xlib has been loaded and initialized. If that is the case,
> threaded optimizations will stay disabled unless the application is
> modified to call XInitThreads() before initializing Xlib or to link
> directly against the NVIDIA OpenGL driver library. Alternatively, using
> the LD_PRELOAD environment variable to include the NVIDIA OpenGL driver
> library should also achieve the desired result."
>
> v2: based on Nicolai and Matt feedback
> Use C style comment
> v3: based on Emil feedback
> split the patch in 3
> s/isGlThreadSafe/isThreadSafe/
>
> Signed-off-by: Gregory Hainaut <gregory.hainaut at gmail.com>
> ---
>  include/GL/internal/dri_interface.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index c83056aa70..8381f704e4 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -1714,13 +1714,24 @@ 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);
> +
> +   /**
> +    * Indicate that it is multithread safe to use glthread.  For GLX/EGL
> +    * platforms using Xlib, that involves calling XInitThreads, before
> +    * opening an X display.
> +    *
> +    * \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.

There should be a comment that this function is only supported if the
extension version is at least 2.

Marek

> +    */
> +   GLboolean (*isThreadSafe)(void *loaderPrivate);
>  };
>
>  #endif
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev