Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Marek Olšák mar...@gmail.com writes:
On Tue, Apr 21, 2015 at 3:50 PM, Francisco Jerez curroje...@riseup.net
wrote:
Marek Olšák mar...@gmail.com writes:
From: Marek Olšák marek.ol...@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 000..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?
I'll change intptr_t to void*. I don't trust that bool is the same in
both C and C++ and int seems to be a safe choice for an ABI.
Any sensible ABI [the same thing that guarantees that C's and C++'s
int are the same type ;)] that as far as I'm aware clover has ever
been compiled for should give identical representations to C's _Bool and
C++'s bool. Not sure what your concern is?
+#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?
OK.
+ 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
[Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
From: Marek Olšák marek.ol...@amd.com
v2: - move interop.cpp to clover/api
- change intptr_t to void* in the interface
- add a virtual function fence() to simplify some code
v3: - use bool in the interface
---
src/gallium/include/state_tracker/opencl_interop.h | 40 +++
src/gallium/state_trackers/clover/Makefile.sources | 1 +
src/gallium/state_trackers/clover/api/interop.cpp | 60 ++
src/gallium/state_trackers/clover/core/event.hpp | 8 +++
src/gallium/targets/opencl/opencl.sym | 1 +
5 files changed, 110 insertions(+)
create mode 100644 src/gallium/include/state_tracker/opencl_interop.h
create mode 100644 src/gallium/state_trackers/clover/api/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 000..4983644
--- /dev/null
+++ b/src/gallium/include/state_tracker/opencl_interop.h
@@ -0,0 +1,40 @@
+/**
+ *
+ * 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.
+ */
+
+typedef bool (*opencl_dri_event_add_ref_t)(void *cl_event);
+typedef bool (*opencl_dri_event_release_t)(void *cl_event);
+typedef bool (*opencl_dri_event_wait_t)(void *cl_event, uint64_t timeout);
+typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(void
*cl_event);
+
+#endif /* OPENCL_INTEROP_H */
diff --git a/src/gallium/state_trackers/clover/Makefile.sources
b/src/gallium/state_trackers/clover/Makefile.sources
index 5b3344c..9e30c69 100644
--- a/src/gallium/state_trackers/clover/Makefile.sources
+++ b/src/gallium/state_trackers/clover/Makefile.sources
@@ -4,6 +4,7 @@ CPP_SOURCES := \
api/dispatch.cpp \
api/dispatch.hpp \
api/event.cpp \
+ api/interop.cpp \
api/kernel.cpp \
api/memory.cpp \
api/platform.cpp \
diff --git a/src/gallium/state_trackers/clover/api/interop.cpp
b/src/gallium/state_trackers/clover/api/interop.cpp
new file mode 100644
index 000..8d18200
--- /dev/null
+++ b/src/gallium/state_trackers/clover/api/interop.cpp
@@ -0,0 +1,60 @@
+//
+// 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
+#include api/util.hpp
+
+using namespace clover;
+
+extern C {
+
+PUBLIC bool
+opencl_dri_event_add_ref(cl_event event)
+{
+ return clRetainEvent(event) == CL_SUCCESS;
+}
+
+PUBLIC bool
+opencl_dri_event_release(cl_event event)
+{
+
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Alright. I suppose it's okay to use bool, but hypothetically a gallium
driver could have an OpenCL stack that isn't clover and the interop
interface should work with it too.
Marek
On Mon, Apr 27, 2015 at 2:46 PM, Francisco Jerez curroje...@riseup.net wrote:
Marek Olšák mar...@gmail.com writes:
On Tue, Apr 21, 2015 at 3:50 PM, Francisco Jerez curroje...@riseup.net
wrote:
Marek Olšák mar...@gmail.com writes:
From: Marek Olšák marek.ol...@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 000..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?
I'll change intptr_t to void*. I don't trust that bool is the same in
both C and C++ and int seems to be a safe choice for an ABI.
Any sensible ABI [the same thing that guarantees that C's and C++'s
int are the same type ;)] that as far as I'm aware clover has ever
been compiled for should give identical representations to C's _Bool and
C++'s bool. Not sure what your concern is?
+#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?
OK.
+ return _fence;
+ }
+
private:
virtual
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Marek Olšák mar...@gmail.com writes:
From: Marek Olšák marek.ol...@amd.com
v2: - move interop.cpp to clover/api
- change intptr_t to void* in the interface
- add a virtual function fence() to simplify some code
v3: - use bool in the interface
---
src/gallium/include/state_tracker/opencl_interop.h | 40 +++
src/gallium/state_trackers/clover/Makefile.sources | 1 +
src/gallium/state_trackers/clover/api/interop.cpp | 60
++
src/gallium/state_trackers/clover/core/event.hpp | 8 +++
src/gallium/targets/opencl/opencl.sym | 1 +
5 files changed, 110 insertions(+)
create mode 100644 src/gallium/include/state_tracker/opencl_interop.h
create mode 100644 src/gallium/state_trackers/clover/api/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 000..4983644
--- /dev/null
+++ b/src/gallium/include/state_tracker/opencl_interop.h
@@ -0,0 +1,40 @@
+/**
+ *
+ * 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.
+ */
+
+typedef bool (*opencl_dri_event_add_ref_t)(void *cl_event);
+typedef bool (*opencl_dri_event_release_t)(void *cl_event);
+typedef bool (*opencl_dri_event_wait_t)(void *cl_event, uint64_t timeout);
+typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(void
*cl_event);
+
+#endif /* OPENCL_INTEROP_H */
diff --git a/src/gallium/state_trackers/clover/Makefile.sources
b/src/gallium/state_trackers/clover/Makefile.sources
index 5b3344c..9e30c69 100644
--- a/src/gallium/state_trackers/clover/Makefile.sources
+++ b/src/gallium/state_trackers/clover/Makefile.sources
@@ -4,6 +4,7 @@ CPP_SOURCES := \
api/dispatch.cpp \
api/dispatch.hpp \
api/event.cpp \
+ api/interop.cpp \
api/kernel.cpp \
api/memory.cpp \
api/platform.cpp \
diff --git a/src/gallium/state_trackers/clover/api/interop.cpp
b/src/gallium/state_trackers/clover/api/interop.cpp
new file mode 100644
index 000..8d18200
--- /dev/null
+++ b/src/gallium/state_trackers/clover/api/interop.cpp
@@ -0,0 +1,60 @@
+//
+// 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
+#include api/util.hpp
+
+using namespace clover;
+
+extern C {
+
+PUBLIC bool
[Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
From: Marek Olšák marek.ol...@amd.com
v2: - move interop.cpp to clover/api
- change intptr_t to void* in the interface
- add a virtual function fence() to simplify some code
v3: - use bool in the interface
v4: - enclose the last two interop functions in try..catch
---
src/gallium/include/state_tracker/opencl_interop.h | 40 ++
src/gallium/state_trackers/clover/Makefile.sources | 1 +
src/gallium/state_trackers/clover/api/interop.cpp | 64 ++
src/gallium/state_trackers/clover/core/event.hpp | 8 +++
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/api/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 000..4983644
--- /dev/null
+++ b/src/gallium/include/state_tracker/opencl_interop.h
@@ -0,0 +1,40 @@
+/**
+ *
+ * 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.
+ */
+
+typedef bool (*opencl_dri_event_add_ref_t)(void *cl_event);
+typedef bool (*opencl_dri_event_release_t)(void *cl_event);
+typedef bool (*opencl_dri_event_wait_t)(void *cl_event, uint64_t timeout);
+typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(void
*cl_event);
+
+#endif /* OPENCL_INTEROP_H */
diff --git a/src/gallium/state_trackers/clover/Makefile.sources
b/src/gallium/state_trackers/clover/Makefile.sources
index 5b3344c..9e30c69 100644
--- a/src/gallium/state_trackers/clover/Makefile.sources
+++ b/src/gallium/state_trackers/clover/Makefile.sources
@@ -4,6 +4,7 @@ CPP_SOURCES := \
api/dispatch.cpp \
api/dispatch.hpp \
api/event.cpp \
+ api/interop.cpp \
api/kernel.cpp \
api/memory.cpp \
api/platform.cpp \
diff --git a/src/gallium/state_trackers/clover/api/interop.cpp
b/src/gallium/state_trackers/clover/api/interop.cpp
new file mode 100644
index 000..ea0c7c7
--- /dev/null
+++ b/src/gallium/state_trackers/clover/api/interop.cpp
@@ -0,0 +1,64 @@
+//
+// 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
+#include api/util.hpp
+
+using namespace clover;
+
+extern C {
+
+PUBLIC bool
+opencl_dri_event_add_ref(cl_event event)
+{
+ return clRetainEvent(event) == CL_SUCCESS;
+}
+
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Marek Olšák mar...@gmail.com writes: From: Marek Olšák marek.ol...@amd.com v2: - move interop.cpp to clover/api - change intptr_t to void* in the interface - add a virtual function fence() to simplify some code v3: - use bool in the interface v4: - enclose the last two interop functions in try..catch Looks good, thanks, Reviewed-by: Francisco Jerez curroje...@riseup.net --- src/gallium/include/state_tracker/opencl_interop.h | 40 ++ src/gallium/state_trackers/clover/Makefile.sources | 1 + src/gallium/state_trackers/clover/api/interop.cpp | 64 ++ src/gallium/state_trackers/clover/core/event.hpp | 8 +++ 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/api/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 000..4983644 --- /dev/null +++ b/src/gallium/include/state_tracker/opencl_interop.h @@ -0,0 +1,40 @@ +/** + * + * 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. + */ + +typedef bool (*opencl_dri_event_add_ref_t)(void *cl_event); +typedef bool (*opencl_dri_event_release_t)(void *cl_event); +typedef bool (*opencl_dri_event_wait_t)(void *cl_event, uint64_t timeout); +typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(void *cl_event); + +#endif /* OPENCL_INTEROP_H */ diff --git a/src/gallium/state_trackers/clover/Makefile.sources b/src/gallium/state_trackers/clover/Makefile.sources index 5b3344c..9e30c69 100644 --- a/src/gallium/state_trackers/clover/Makefile.sources +++ b/src/gallium/state_trackers/clover/Makefile.sources @@ -4,6 +4,7 @@ CPP_SOURCES := \ api/dispatch.cpp \ api/dispatch.hpp \ api/event.cpp \ + api/interop.cpp \ api/kernel.cpp \ api/memory.cpp \ api/platform.cpp \ diff --git a/src/gallium/state_trackers/clover/api/interop.cpp b/src/gallium/state_trackers/clover/api/interop.cpp new file mode 100644 index 000..ea0c7c7 --- /dev/null +++ b/src/gallium/state_trackers/clover/api/interop.cpp @@ -0,0 +1,64 @@ +// +// 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. +// +
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
On Tue, Apr 21, 2015 at 3:50 PM, Francisco Jerez curroje...@riseup.net wrote:
Marek Olšák mar...@gmail.com writes:
From: Marek Olšák marek.ol...@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 000..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?
I'll change intptr_t to void*. I don't trust that bool is the same in
both C and C++ and int seems to be a safe choice for an ABI.
+#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?
OK.
+ 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 000..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.
OK.
Marek
___
mesa-dev mailing list
[Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
From: Marek Olšák marek.ol...@amd.com
v2: - move interop.cpp to clover/api
- change intptr_t to void* in the interface
- add a virtual function fence() to simplify some code
---
src/gallium/include/state_tracker/opencl_interop.h | 42 +++
src/gallium/state_trackers/clover/Makefile.sources | 1 +
src/gallium/state_trackers/clover/api/interop.cpp | 60 ++
src/gallium/state_trackers/clover/core/event.hpp | 8 +++
src/gallium/targets/opencl/opencl.sym | 1 +
5 files changed, 112 insertions(+)
create mode 100644 src/gallium/include/state_tracker/opencl_interop.h
create mode 100644 src/gallium/state_trackers/clover/api/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 000..0c9f34a
--- /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)(void *cl_event);
+typedef int (*opencl_dri_event_release_t)(void *cl_event);
+typedef int (*opencl_dri_event_wait_t)(void *cl_event, uint64_t timeout);
+typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(void
*cl_event);
+
+#endif /* OPENCL_INTEROP_H */
diff --git a/src/gallium/state_trackers/clover/Makefile.sources
b/src/gallium/state_trackers/clover/Makefile.sources
index 5b3344c..9e30c69 100644
--- a/src/gallium/state_trackers/clover/Makefile.sources
+++ b/src/gallium/state_trackers/clover/Makefile.sources
@@ -4,6 +4,7 @@ CPP_SOURCES := \
api/dispatch.cpp \
api/dispatch.hpp \
api/event.cpp \
+ api/interop.cpp \
api/kernel.cpp \
api/memory.cpp \
api/platform.cpp \
diff --git a/src/gallium/state_trackers/clover/api/interop.cpp
b/src/gallium/state_trackers/clover/api/interop.cpp
new file mode 100644
index 000..2bec839
--- /dev/null
+++ b/src/gallium/state_trackers/clover/api/interop.cpp
@@ -0,0 +1,60 @@
+//
+// 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
+#include api/util.hpp
+
+using namespace clover;
+
+extern C {
+
+PUBLIC int
+opencl_dri_event_add_ref(cl_event _event)
+{
+ return clRetainEvent(_event) == CL_SUCCESS;
+}
+
+PUBLIC int
+opencl_dri_event_release(cl_event
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
On Tue, Apr 21, 2015 at 5:45 PM, Emil Velikov emil.l.veli...@gmail.com wrote: Hmm pretty sure I've read somewhere (old version of GCC's manual?) that such warnings are enabled by default and cannot be controlled by the compiler (for C++ of course). I'm not an C++ expect so it could be that the standard does not require declarations/prototypes for non static functions ? A prototype is only required by C++ when a function is called. Functions don't need prototypes for themselves. Only their call sites need them. Seems like we're building clover without even a single -W flag - perhaps we could add some (not with this patch of course) ? Yes, I agree. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Only C warns about that. I didn't see any warning with C++.
Marek
On Tue, Apr 21, 2015 at 3:44 PM, Emil Velikov emil.l.veli...@gmail.com wrote:
On 14/04/15 22:19, Marek Olšák wrote:
From: Marek Olšák marek.ol...@amd.com
---
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 000..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);
+
+#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 {
+ 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 000..bb80cf5
--- /dev/null
+++ b/src/gallium/state_trackers/clover/core/interop.cpp
@@ -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
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Hmm pretty sure I've read somewhere (old version of GCC's manual?) that
such warnings are enabled by default and cannot be controlled by the
compiler (for C++ of course). I'm not an C++ expect so it could be that
the standard does not require declarations/prototypes for non static
functions ?
Seems like we're building clover without even a single -W flag - perhaps
we could add some (not with this patch of course) ?
-Emil
On 21/04/15 13:13, Marek Olšák wrote:
Only C warns about that. I didn't see any warning with C++.
Marek
On Tue, Apr 21, 2015 at 3:44 PM, Emil Velikov emil.l.veli...@gmail.com
wrote:
On 14/04/15 22:19, Marek Olšák wrote:
From: Marek Olšák marek.ol...@amd.com
---
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 000..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);
+
+#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 {
+ 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 000..bb80cf5
--- /dev/null
+++ b/src/gallium/state_trackers/clover/core/interop.cpp
@@ -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
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Marek Olšák mar...@gmail.com writes:
From: Marek Olšák marek.ol...@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 000..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 000..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,
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
On 14/04/15 22:19, Marek Olšák wrote:
From: Marek Olšák marek.ol...@amd.com
---
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 000..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);
+
+#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 {
+ 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 000..bb80cf5
--- /dev/null
+++ b/src/gallium/state_trackers/clover/core/interop.cpp
@@ -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
[Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
From: Marek Olšák marek.ol...@amd.com
---
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 000..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);
+
+#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 {
+ 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 000..bb80cf5
--- /dev/null
+++ b/src/gallium/state_trackers/clover/core/interop.cpp
@@ -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
