[Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events

Francisco Jerez currojerez at riseup.net
Tue Apr 21 06:50:48 PDT 2015


Marek Olšák <maraeo at gmail.com> writes:

> From: Marek Olšák <marek.olsak at amd.com>
>
Hi Marek, looks mostly OK to me, a few nits inline,

> ---
>  src/gallium/include/state_tracker/opencl_interop.h | 42 ++++++++++++++
>  src/gallium/state_trackers/clover/Makefile.sources |  1 +
>  src/gallium/state_trackers/clover/core/event.hpp   |  4 ++
>  src/gallium/state_trackers/clover/core/interop.cpp | 66 ++++++++++++++++++++++
>  src/gallium/targets/opencl/opencl.sym              |  1 +
>  5 files changed, 114 insertions(+)
>  create mode 100644 src/gallium/include/state_tracker/opencl_interop.h
>  create mode 100644 src/gallium/state_trackers/clover/core/interop.cpp
>
> diff --git a/src/gallium/include/state_tracker/opencl_interop.h b/src/gallium/include/state_tracker/opencl_interop.h
> new file mode 100644
> index 0000000..31a3bd3
> --- /dev/null
> +++ b/src/gallium/include/state_tracker/opencl_interop.h
> @@ -0,0 +1,42 @@
> +/**************************************************************************
> + *
> + * Copyright 2015 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + **************************************************************************/
> +
> +#ifndef OPENCL_INTEROP_H
> +#define OPENCL_INTEROP_H
> +
> +/* dlsym these without the "_t" suffix. You should get the correct symbols
> + * if the OpenCL driver is loaded.
> + *
> + * All functions return non-zero on success.
> + */
> +
> +typedef int (*opencl_dri_event_add_ref_t)(intptr_t cl_event);
> +typedef int (*opencl_dri_event_release_t)(intptr_t cl_event);
> +typedef int (*opencl_dri_event_wait_t)(intptr_t cl_event, uint64_t timeout);
> +typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(intptr_t cl_event);
> +

Just curious, is there any reason you use intptr_t here and elsewhere
rather than cl_event or void *?  The former is an typedef of an opaque
pointer anyway.  Using CL types would likely avoid some "ugly" casts
later on.  And maybe bool as return type?

> +#endif /* OPENCL_INTEROP_H */
> diff --git a/src/gallium/state_trackers/clover/Makefile.sources b/src/gallium/state_trackers/clover/Makefile.sources
> index 5b3344c..4c2d0f3 100644
> --- a/src/gallium/state_trackers/clover/Makefile.sources
> +++ b/src/gallium/state_trackers/clover/Makefile.sources
> @@ -22,6 +22,7 @@ CPP_SOURCES := \
>  	core/event.hpp \
>  	core/format.cpp \
>  	core/format.hpp \
> +	core/interop.cpp \
>  	core/kernel.cpp \
>  	core/kernel.hpp \
>  	core/memory.cpp \
> diff --git a/src/gallium/state_trackers/clover/core/event.hpp b/src/gallium/state_trackers/clover/core/event.hpp
> index 0e1359a..28f510f 100644
> --- a/src/gallium/state_trackers/clover/core/event.hpp
> +++ b/src/gallium/state_trackers/clover/core/event.hpp
> @@ -116,6 +116,10 @@ namespace clover {
>  
>        friend class command_queue;
>  
> +      struct pipe_fence_handle *get_fence() const {

The convention in the surrounding code is to name such accessors as the
object they access, how about "pipe_fence_handle *fence() const"?

> +         return _fence;
> +      }
> +
>     private:
>        virtual void fence(pipe_fence_handle *fence);
>        action profile(command_queue &q, const action &action) const;
> diff --git a/src/gallium/state_trackers/clover/core/interop.cpp b/src/gallium/state_trackers/clover/core/interop.cpp
> new file mode 100644
> index 0000000..bb80cf5
> --- /dev/null
> +++ b/src/gallium/state_trackers/clover/core/interop.cpp

This probably belongs in clover/api/, as it's technically implementing
an API, clover/core/ is all about core data structures and such.

> @@ -0,0 +1,66 @@
> +//
> +// Copyright 2015 Advanced Micro Devices, Inc.
> +// All Rights Reserved.
> +//
> +// Permission is hereby granted, free of charge, to any person obtaining a
> +// copy of this software and associated documentation files (the "Software"),
> +// to deal in the Software without restriction, including without limitation
> +// the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +// and/or sell copies of the Software, and to permit persons to whom the
> +// Software is furnished to do so, subject to the following conditions:
> +//
> +// The above copyright notice and this permission notice shall be included in
> +// all copies or substantial portions of the Software.
> +//
> +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +// THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> +// OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> +// ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> +// OTHER DEALINGS IN THE SOFTWARE.
> +//
> +
> +#include "core/event.hpp"
> +
> +using namespace clover;
> +
> +extern "C" {
> +
> +PUBLIC int
> +opencl_dri_event_add_ref(intptr_t event)
> +{
> +   return clRetainEvent((cl_event)event) == CL_SUCCESS;
> +}
> +
> +PUBLIC int
> +opencl_dri_event_release(intptr_t event)
> +{
> +   return clReleaseEvent((cl_event)event) == CL_SUCCESS;
> +}
> +
> +PUBLIC int
> +opencl_dri_event_wait(intptr_t cl_event, uint64_t timeout)
> +{
> +   event *e = (event*)cl_event;
> +
> +   if (!timeout) {
> +      return e->status() == CL_COMPLETE;
> +   }
> +
> +   e->wait();
> +   return true;
> +}
> +
> +PUBLIC struct pipe_fence_handle *
> +opencl_dri_event_get_fence(intptr_t cl_event)
> +{
> +   hard_event *e = dynamic_cast<hard_event*>((event*)cl_event);
> +
> +   if (e)
> +      return e->get_fence();
> +
> +   return NULL;
> +}
> +
> +}
> diff --git a/src/gallium/targets/opencl/opencl.sym b/src/gallium/targets/opencl/opencl.sym
> index ee8aacf..9fcc576 100644
> --- a/src/gallium/targets/opencl/opencl.sym
> +++ b/src/gallium/targets/opencl/opencl.sym
> @@ -1,6 +1,7 @@
>  {
>  	global:
>  		cl*;
> +		opencl_dri_*;
>  	local:
>  		*;
>  };
> -- 
> 2.1.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150421/8b56afa1/attachment.sig>


More information about the mesa-dev mailing list