When you resubmit the patch again, please add (v3) to the subject.
On Mon 08 May 2017, Gurchetan Singh wrote:
> Use the same fence implementation for drisw.c as dri2.c by making
> dri2FenceExtension an external variable. Since the fence implementation
> is not dri2.c specific, put it in a separate file. This is desirable for
> synchronization in virtual machines.
>
> v2: Don't depend on dri2.c for extensions (Emil)
> ---
> src/gallium/state_trackers/dri/Makefile.sources | 2 +
> src/gallium/state_trackers/dri/dri2.c | 203 +--------------------
> src/gallium/state_trackers/dri/dri_extensions.c | 229
> ++++++++++++++++++++++++
> src/gallium/state_trackers/dri/dri_extensions.h | 30 ++++
> src/gallium/state_trackers/dri/drisw.c | 2 +
> 5 files changed, 264 insertions(+), 202 deletions(-)
> create mode 100644 src/gallium/state_trackers/dri/dri_extensions.c
> create mode 100644 src/gallium/state_trackers/dri/dri_extensions.h
Usually, to make review easier, and to make the git log easier to
understand, it's better to break patches like this into two: patch
1 moves the code, and patch 2 makes the actual changes.
> diff --git a/src/gallium/state_trackers/dri/dri2.c
> b/src/gallium/state_trackers/dri/dri2.c
> index ed6004f836..2d95e668f8 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> -static bool
> -dri2_is_opencl_interop_loaded_locked(struct dri_screen *screen)
> -{
> - return screen->opencl_dri_event_add_ref &&
> - screen->opencl_dri_event_release &&
> - screen->opencl_dri_event_wait &&
> - screen->opencl_dri_event_get_fence;
> -}
> -
> -static bool
> -dri2_load_opencl_interop(struct dri_screen *screen)
> -{
> -#if defined(RTLD_DEFAULT)
> - bool success;
There's a bug here. Pre-patch, RTLD_DEFAULT is defined here and this
code gets built. I verified this with #error. Post-patch, in the
equivalent code in the new file, RTLD_DEFAULT is undefined, and this
code never gets built.
> -
> - mtx_lock(&screen->opencl_func_mutex);
> -
> - if (dri2_is_opencl_interop_loaded_locked(screen)) {
> - mtx_unlock(&screen->opencl_func_mutex);
> - return true;
> - }
> -
> - screen->opencl_dri_event_add_ref =
> - dlsym(RTLD_DEFAULT, "opencl_dri_event_add_ref");
> - screen->opencl_dri_event_release =
> - dlsym(RTLD_DEFAULT, "opencl_dri_event_release");
> - screen->opencl_dri_event_wait =
> - dlsym(RTLD_DEFAULT, "opencl_dri_event_wait");
> - screen->opencl_dri_event_get_fence =
> - dlsym(RTLD_DEFAULT, "opencl_dri_event_get_fence");
> -
> - success = dri2_is_opencl_interop_loaded_locked(screen);
> - mtx_unlock(&screen->opencl_func_mutex);
> - return success;
> -#else
> - return false;
> -#endif
> -}
> diff --git a/src/gallium/state_trackers/dri/dri_extensions.c
> b/src/gallium/state_trackers/dri/dri_extensions.c
> new file mode 100644
> index 0000000000..2fa7233aab
> --- /dev/null
> +++ b/src/gallium/state_trackers/dri/dri_extensions.c
> +static bool
> +dri2_is_opencl_interop_loaded_locked(struct dri_screen *screen)
> +{
> + return screen->opencl_dri_event_add_ref &&
> + screen->opencl_dri_event_release &&
> + screen->opencl_dri_event_wait &&
> + screen->opencl_dri_event_get_fence;
> +}
> +
> +static bool
> +dri2_load_opencl_interop(struct dri_screen *screen)
> +{
> +#if defined(RTLD_DEFAULT)
> + bool success;
> +
> + pipe_mutex_lock(screen->opencl_func_mutex);
pipe_mutex_lock/unlock need to be replaced with mtx_lock/unlock, because
the pipe_mutex wrappers no longer exist. See this commit.
commit e000b17f87bd960c4ce1c0892017023d4dc59609
Author: Brian Paul <bri...@vmware.com>
Date: Wed Apr 5 15:15:27 2017 -0600
winsys/svga: use c11 thread types/functions
Gallium no longer has wrappers for mutexes and conditi
> +
> + if (dri2_is_opencl_interop_loaded_locked(screen)) {
> + pipe_mutex_unlock(screen->opencl_func_mutex);
> + return true;
> + }
> +
> + screen->opencl_dri_event_add_ref =
> + dlsym(RTLD_DEFAULT, "opencl_dri_event_add_ref");
> + screen->opencl_dri_event_release =
> + dlsym(RTLD_DEFAULT, "opencl_dri_event_release");
> + screen->opencl_dri_event_wait =
> + dlsym(RTLD_DEFAULT, "opencl_dri_event_wait");
> + screen->opencl_dri_event_get_fence =
> + dlsym(RTLD_DEFAULT, "opencl_dri_event_get_fence");
> +
> + success = dri2_is_opencl_interop_loaded_locked(screen);
> + pipe_mutex_unlock(screen->opencl_func_mutex);
> + return success;
> +#else
> + return false;
> +#endif
> +}
Those are the only issues I found. But, I don't know this code well, so
you should probably seek the review of someone who touched it recently,
like Rob Herring, Rob Clark, or Tim Arceri.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
