* [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
@ 2017-06-26 21:19 Dave Airlie
[not found] ` <20170626211922.7493-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 18+ messages in thread
From: Dave Airlie @ 2017-06-26 21:19 UTC (permalink / raw)
To: dri-devel, amd-gfx
From: Dave Airlie <airlied@redhat.com>
This adds the corresponding code for libdrm to use the new
kernel interfaces for semaphores.
This will be used by radv to implement shared semaphores.
TODO: Version checks.
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
amdgpu/amdgpu.h | 28 +++++++++
amdgpu/amdgpu_cs.c | 161 ++++++++++++++++++++++++++++++++++++++++++++---
include/drm/amdgpu_drm.h | 28 +++++++++
3 files changed, 208 insertions(+), 9 deletions(-)
diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
index 7b26a04..747e248 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -129,6 +129,8 @@ typedef struct amdgpu_va *amdgpu_va_handle;
*/
typedef struct amdgpu_semaphore *amdgpu_semaphore_handle;
+typedef uint32_t amdgpu_sem_handle;
+
/*--------------------------------------------------------------------------*/
/* -------------------------- Structures ---------------------------------- */
/*--------------------------------------------------------------------------*/
@@ -365,6 +367,16 @@ struct amdgpu_cs_request {
struct amdgpu_cs_fence_info fence_info;
};
+struct amdgpu_cs_request_sem {
+ /*
+ *
+ */
+ uint32_t number_of_wait_sem;
+ uint32_t *wait_sems;
+ uint32_t number_of_signal_sem;
+ uint32_t *signal_sems;
+};
+
/**
* Structure which provide information about GPU VM MC Address space
* alignments requirements
@@ -882,6 +894,12 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
struct amdgpu_cs_request *ibs_request,
uint32_t number_of_requests);
+int amdgpu_cs_submit_sem(amdgpu_context_handle context,
+ uint64_t flags,
+ struct amdgpu_cs_request *ibs_request,
+ struct amdgpu_cs_request_sem *ibs_sem,
+ uint32_t number_of_requests);
+
/**
* Query status of Command Buffer Submission
*
@@ -1255,4 +1273,14 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem);
*/
const char *amdgpu_get_marketing_name(amdgpu_device_handle dev);
+int amdgpu_cs_create_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle *sem);
+int amdgpu_cs_export_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle sem,
+ int *shared_handle);
+int amdgpu_cs_import_sem(amdgpu_device_handle dev,
+ int shared_handle,
+ amdgpu_sem_handle *sem);
+int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle sem);
#endif /* #ifdef _AMDGPU_H_ */
diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
index fb5b3a8..7283327 100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -170,7 +170,8 @@ int amdgpu_cs_query_reset_state(amdgpu_context_handle context,
* \sa amdgpu_cs_submit()
*/
static int amdgpu_cs_submit_one(amdgpu_context_handle context,
- struct amdgpu_cs_request *ibs_request)
+ struct amdgpu_cs_request *ibs_request,
+ struct amdgpu_cs_request_sem *sem_request)
{
union drm_amdgpu_cs cs;
uint64_t *chunk_array;
@@ -178,9 +179,11 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
struct drm_amdgpu_cs_chunk_data *chunk_data;
struct drm_amdgpu_cs_chunk_dep *dependencies = NULL;
struct drm_amdgpu_cs_chunk_dep *sem_dependencies = NULL;
+ struct drm_amdgpu_cs_chunk_sem *wait_sem_dependencies = NULL;
+ struct drm_amdgpu_cs_chunk_sem *signal_sem_dependencies = NULL;
struct list_head *sem_list;
amdgpu_semaphore_handle sem, tmp;
- uint32_t i, size, sem_count = 0;
+ uint32_t i, j, size, sem_count = 0;
bool user_fence;
int r = 0;
@@ -196,7 +199,7 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
}
user_fence = (ibs_request->fence_info.handle != NULL);
- size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1;
+ size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1 + (sem_request ? 2 : 0);
chunk_array = alloca(sizeof(uint64_t) * size);
chunks = alloca(sizeof(struct drm_amdgpu_cs_chunk) * size);
@@ -308,6 +311,45 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
chunks[i].chunk_data = (uint64_t)(uintptr_t)sem_dependencies;
}
+ if (sem_request) {
+ if (sem_request->number_of_wait_sem) {
+ wait_sem_dependencies = malloc(sizeof(struct drm_amdgpu_cs_chunk_sem) * sem_request->number_of_wait_sem);
+ if (!wait_sem_dependencies) {
+ r = -ENOMEM;
+ goto error_unlock;
+ }
+ for (j = 0; j < sem_request->number_of_wait_sem; j++) {
+ struct drm_amdgpu_cs_chunk_sem *dep = &wait_sem_dependencies[j];
+ dep->handle = sem_request->wait_sems[j];
+ }
+ i = cs.in.num_chunks++;
+
+ /* dependencies chunk */
+ chunk_array[i] = (uint64_t)(uintptr_t)&chunks[i];
+ chunks[i].chunk_id = AMDGPU_CHUNK_ID_SEM_WAIT;
+ chunks[i].length_dw = sizeof(struct drm_amdgpu_cs_chunk_sem) / 4 * sem_request->number_of_wait_sem;
+ chunks[i].chunk_data = (uint64_t)(uintptr_t)wait_sem_dependencies;
+ }
+ if (sem_request->number_of_signal_sem) {
+ signal_sem_dependencies = malloc(sizeof(struct drm_amdgpu_cs_chunk_sem) * sem_request->number_of_signal_sem);
+ if (!signal_sem_dependencies) {
+ r = -ENOMEM;
+ goto error_unlock;
+ }
+ for (j = 0; j < sem_request->number_of_signal_sem; j++) {
+ struct drm_amdgpu_cs_chunk_sem *dep = &signal_sem_dependencies[j];
+ dep->handle = sem_request->signal_sems[j];
+ }
+ i = cs.in.num_chunks++;
+
+ /* dependencies chunk */
+ chunk_array[i] = (uint64_t)(uintptr_t)&chunks[i];
+ chunks[i].chunk_id = AMDGPU_CHUNK_ID_SEM_SIGNAL;
+ chunks[i].length_dw = sizeof(struct drm_amdgpu_cs_chunk_sem) / 4 * sem_request->number_of_signal_sem;
+ chunks[i].chunk_data = (uint64_t)(uintptr_t)signal_sem_dependencies;
+ }
+ }
+
r = drmCommandWriteRead(context->dev->fd, DRM_AMDGPU_CS,
&cs, sizeof(cs));
if (r)
@@ -319,17 +361,20 @@ error_unlock:
pthread_mutex_unlock(&context->sequence_mutex);
free(dependencies);
free(sem_dependencies);
+ free(wait_sem_dependencies);
+ free(signal_sem_dependencies);
return r;
}
-int amdgpu_cs_submit(amdgpu_context_handle context,
- uint64_t flags,
- struct amdgpu_cs_request *ibs_request,
- uint32_t number_of_requests)
+int amdgpu_cs_submit_sem(amdgpu_context_handle context,
+ uint64_t flags,
+ struct amdgpu_cs_request *ibs_request,
+ struct amdgpu_cs_request_sem *ibs_sem,
+ uint32_t number_of_requests)
{
uint32_t i;
int r;
-
+ bool has_sems = ibs_sem ? true : false;
if (NULL == context)
return -EINVAL;
if (NULL == ibs_request)
@@ -337,15 +382,28 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
r = 0;
for (i = 0; i < number_of_requests; i++) {
- r = amdgpu_cs_submit_one(context, ibs_request);
+ r = amdgpu_cs_submit_one(context, ibs_request, has_sems ? ibs_sem : NULL);
if (r)
break;
ibs_request++;
+ if (has_sems)
+ ibs_sem++;
}
return r;
}
+int amdgpu_cs_submit(amdgpu_context_handle context,
+ uint64_t flags,
+ struct amdgpu_cs_request *ibs_request,
+ uint32_t number_of_requests)
+{
+ return amdgpu_cs_submit_sem(context, flags,
+ ibs_request, NULL,
+ number_of_requests);
+}
+
+
/**
* Calculate absolute timeout.
*
@@ -542,3 +600,88 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem)
{
return amdgpu_cs_unreference_sem(sem);
}
+
+
+int amdgpu_cs_create_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle *sem)
+{
+ union drm_amdgpu_sem args;
+ int r;
+
+ if (NULL == dev)
+ return -EINVAL;
+
+ /* Create the context */
+ memset(&args, 0, sizeof(args));
+ args.in.op = AMDGPU_SEM_OP_CREATE_SEM;
+ r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
+ if (r)
+ return r;
+
+ *sem = args.out.handle;
+
+ return 0;
+}
+
+int amdgpu_cs_export_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle sem,
+ int *shared_handle)
+{
+ union drm_amdgpu_sem args;
+ int r;
+
+ if (NULL == dev)
+ return -EINVAL;
+
+ /* Create the context */
+ memset(&args, 0, sizeof(args));
+ args.in.op = AMDGPU_SEM_OP_EXPORT_SEM;
+ args.in.handle = sem;
+ r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
+ if (r)
+ return r;
+ *shared_handle = args.out.fd;
+ return 0;
+}
+
+int amdgpu_cs_import_sem(amdgpu_device_handle dev,
+ int shared_handle,
+ amdgpu_sem_handle *sem)
+{
+ union drm_amdgpu_sem args;
+ int r;
+
+ if (NULL == dev)
+ return -EINVAL;
+
+ /* Create the context */
+ memset(&args, 0, sizeof(args));
+ args.in.op = AMDGPU_SEM_OP_IMPORT_SEM;
+ args.in.handle = shared_handle;
+ r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
+ if (r)
+ return r;
+ *sem = args.out.handle;
+ return 0;
+}
+
+
+int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle sem)
+{
+ union drm_amdgpu_sem args;
+ int r;
+
+ if (NULL == dev)
+ return -EINVAL;
+
+ /* Create the context */
+ memset(&args, 0, sizeof(args));
+ args.in.op = AMDGPU_SEM_OP_DESTROY_SEM;
+ args.in.handle = sem;
+ r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
+ if (r)
+ return r;
+
+ return 0;
+}
diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
index d8f2497..fa0bfe2 100644
--- a/include/drm/amdgpu_drm.h
+++ b/include/drm/amdgpu_drm.h
@@ -50,6 +50,7 @@ extern "C" {
#define DRM_AMDGPU_WAIT_CS 0x09
#define DRM_AMDGPU_GEM_OP 0x10
#define DRM_AMDGPU_GEM_USERPTR 0x11
+#define DRM_AMDGPU_SEM 0x13
#define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
#define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
@@ -63,6 +64,7 @@ extern "C" {
#define DRM_IOCTL_AMDGPU_WAIT_CS DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_CS, union drm_amdgpu_wait_cs)
#define DRM_IOCTL_AMDGPU_GEM_OP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op)
#define DRM_IOCTL_AMDGPU_GEM_USERPTR DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
+#define DRM_IOCTL_AMDGPU_SEM DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_SEM, union drm_amdgpu_sem)
#define AMDGPU_GEM_DOMAIN_CPU 0x1
#define AMDGPU_GEM_DOMAIN_GTT 0x2
@@ -303,6 +305,26 @@ union drm_amdgpu_wait_cs {
struct drm_amdgpu_wait_cs_out out;
};
+#define AMDGPU_SEM_OP_CREATE_SEM 0
+#define AMDGPU_SEM_OP_IMPORT_SEM 1
+#define AMDGPU_SEM_OP_EXPORT_SEM 2
+#define AMDGPU_SEM_OP_DESTROY_SEM 3
+
+struct drm_amdgpu_sem_in {
+ __u32 op;
+ __u32 handle;
+};
+
+struct drm_amdgpu_sem_out {
+ __u32 fd;
+ __u32 handle;
+};
+
+union drm_amdgpu_sem {
+ struct drm_amdgpu_sem_in in;
+ struct drm_amdgpu_sem_out out;
+};
+
#define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
#define AMDGPU_GEM_OP_SET_PLACEMENT 1
@@ -358,6 +380,8 @@ struct drm_amdgpu_gem_va {
#define AMDGPU_CHUNK_ID_IB 0x01
#define AMDGPU_CHUNK_ID_FENCE 0x02
#define AMDGPU_CHUNK_ID_DEPENDENCIES 0x03
+#define AMDGPU_CHUNK_ID_SEM_WAIT 0x04
+#define AMDGPU_CHUNK_ID_SEM_SIGNAL 0x05
struct drm_amdgpu_cs_chunk {
uint32_t chunk_id;
@@ -422,6 +446,10 @@ struct drm_amdgpu_cs_chunk_fence {
uint32_t offset;
};
+struct drm_amdgpu_cs_chunk_sem {
+ uint32_t handle;
+};
+
struct drm_amdgpu_cs_chunk_data {
union {
struct drm_amdgpu_cs_chunk_ib ib_data;
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <20170626211922.7493-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-06-27 2:59 ` Dave Airlie
0 siblings, 0 replies; 18+ messages in thread
From: Dave Airlie @ 2017-06-27 2:59 UTC (permalink / raw)
To: dri-devel, amd-gfx mailing list
Ignore this, all kinds of patches from wrong tree stuff going on here.
Dave.
On 27 June 2017 at 07:19, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> This adds the corresponding code for libdrm to use the new
> kernel interfaces for semaphores.
>
> This will be used by radv to implement shared semaphores.
>
> TODO: Version checks.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
> amdgpu/amdgpu.h | 28 +++++++++
> amdgpu/amdgpu_cs.c | 161 ++++++++++++++++++++++++++++++++++++++++++++---
> include/drm/amdgpu_drm.h | 28 +++++++++
> 3 files changed, 208 insertions(+), 9 deletions(-)
>
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index 7b26a04..747e248 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -129,6 +129,8 @@ typedef struct amdgpu_va *amdgpu_va_handle;
> */
> typedef struct amdgpu_semaphore *amdgpu_semaphore_handle;
>
> +typedef uint32_t amdgpu_sem_handle;
> +
> /*--------------------------------------------------------------------------*/
> /* -------------------------- Structures ---------------------------------- */
> /*--------------------------------------------------------------------------*/
> @@ -365,6 +367,16 @@ struct amdgpu_cs_request {
> struct amdgpu_cs_fence_info fence_info;
> };
>
> +struct amdgpu_cs_request_sem {
> + /*
> + *
> + */
> + uint32_t number_of_wait_sem;
> + uint32_t *wait_sems;
> + uint32_t number_of_signal_sem;
> + uint32_t *signal_sems;
> +};
> +
> /**
> * Structure which provide information about GPU VM MC Address space
> * alignments requirements
> @@ -882,6 +894,12 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
> struct amdgpu_cs_request *ibs_request,
> uint32_t number_of_requests);
>
> +int amdgpu_cs_submit_sem(amdgpu_context_handle context,
> + uint64_t flags,
> + struct amdgpu_cs_request *ibs_request,
> + struct amdgpu_cs_request_sem *ibs_sem,
> + uint32_t number_of_requests);
> +
> /**
> * Query status of Command Buffer Submission
> *
> @@ -1255,4 +1273,14 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem);
> */
> const char *amdgpu_get_marketing_name(amdgpu_device_handle dev);
>
> +int amdgpu_cs_create_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle *sem);
> +int amdgpu_cs_export_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem,
> + int *shared_handle);
> +int amdgpu_cs_import_sem(amdgpu_device_handle dev,
> + int shared_handle,
> + amdgpu_sem_handle *sem);
> +int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem);
> #endif /* #ifdef _AMDGPU_H_ */
> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
> index fb5b3a8..7283327 100644
> --- a/amdgpu/amdgpu_cs.c
> +++ b/amdgpu/amdgpu_cs.c
> @@ -170,7 +170,8 @@ int amdgpu_cs_query_reset_state(amdgpu_context_handle context,
> * \sa amdgpu_cs_submit()
> */
> static int amdgpu_cs_submit_one(amdgpu_context_handle context,
> - struct amdgpu_cs_request *ibs_request)
> + struct amdgpu_cs_request *ibs_request,
> + struct amdgpu_cs_request_sem *sem_request)
> {
> union drm_amdgpu_cs cs;
> uint64_t *chunk_array;
> @@ -178,9 +179,11 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
> struct drm_amdgpu_cs_chunk_data *chunk_data;
> struct drm_amdgpu_cs_chunk_dep *dependencies = NULL;
> struct drm_amdgpu_cs_chunk_dep *sem_dependencies = NULL;
> + struct drm_amdgpu_cs_chunk_sem *wait_sem_dependencies = NULL;
> + struct drm_amdgpu_cs_chunk_sem *signal_sem_dependencies = NULL;
> struct list_head *sem_list;
> amdgpu_semaphore_handle sem, tmp;
> - uint32_t i, size, sem_count = 0;
> + uint32_t i, j, size, sem_count = 0;
> bool user_fence;
> int r = 0;
>
> @@ -196,7 +199,7 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
> }
> user_fence = (ibs_request->fence_info.handle != NULL);
>
> - size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1;
> + size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1 + (sem_request ? 2 : 0);
>
> chunk_array = alloca(sizeof(uint64_t) * size);
> chunks = alloca(sizeof(struct drm_amdgpu_cs_chunk) * size);
> @@ -308,6 +311,45 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
> chunks[i].chunk_data = (uint64_t)(uintptr_t)sem_dependencies;
> }
>
> + if (sem_request) {
> + if (sem_request->number_of_wait_sem) {
> + wait_sem_dependencies = malloc(sizeof(struct drm_amdgpu_cs_chunk_sem) * sem_request->number_of_wait_sem);
> + if (!wait_sem_dependencies) {
> + r = -ENOMEM;
> + goto error_unlock;
> + }
> + for (j = 0; j < sem_request->number_of_wait_sem; j++) {
> + struct drm_amdgpu_cs_chunk_sem *dep = &wait_sem_dependencies[j];
> + dep->handle = sem_request->wait_sems[j];
> + }
> + i = cs.in.num_chunks++;
> +
> + /* dependencies chunk */
> + chunk_array[i] = (uint64_t)(uintptr_t)&chunks[i];
> + chunks[i].chunk_id = AMDGPU_CHUNK_ID_SEM_WAIT;
> + chunks[i].length_dw = sizeof(struct drm_amdgpu_cs_chunk_sem) / 4 * sem_request->number_of_wait_sem;
> + chunks[i].chunk_data = (uint64_t)(uintptr_t)wait_sem_dependencies;
> + }
> + if (sem_request->number_of_signal_sem) {
> + signal_sem_dependencies = malloc(sizeof(struct drm_amdgpu_cs_chunk_sem) * sem_request->number_of_signal_sem);
> + if (!signal_sem_dependencies) {
> + r = -ENOMEM;
> + goto error_unlock;
> + }
> + for (j = 0; j < sem_request->number_of_signal_sem; j++) {
> + struct drm_amdgpu_cs_chunk_sem *dep = &signal_sem_dependencies[j];
> + dep->handle = sem_request->signal_sems[j];
> + }
> + i = cs.in.num_chunks++;
> +
> + /* dependencies chunk */
> + chunk_array[i] = (uint64_t)(uintptr_t)&chunks[i];
> + chunks[i].chunk_id = AMDGPU_CHUNK_ID_SEM_SIGNAL;
> + chunks[i].length_dw = sizeof(struct drm_amdgpu_cs_chunk_sem) / 4 * sem_request->number_of_signal_sem;
> + chunks[i].chunk_data = (uint64_t)(uintptr_t)signal_sem_dependencies;
> + }
> + }
> +
> r = drmCommandWriteRead(context->dev->fd, DRM_AMDGPU_CS,
> &cs, sizeof(cs));
> if (r)
> @@ -319,17 +361,20 @@ error_unlock:
> pthread_mutex_unlock(&context->sequence_mutex);
> free(dependencies);
> free(sem_dependencies);
> + free(wait_sem_dependencies);
> + free(signal_sem_dependencies);
> return r;
> }
>
> -int amdgpu_cs_submit(amdgpu_context_handle context,
> - uint64_t flags,
> - struct amdgpu_cs_request *ibs_request,
> - uint32_t number_of_requests)
> +int amdgpu_cs_submit_sem(amdgpu_context_handle context,
> + uint64_t flags,
> + struct amdgpu_cs_request *ibs_request,
> + struct amdgpu_cs_request_sem *ibs_sem,
> + uint32_t number_of_requests)
> {
> uint32_t i;
> int r;
> -
> + bool has_sems = ibs_sem ? true : false;
> if (NULL == context)
> return -EINVAL;
> if (NULL == ibs_request)
> @@ -337,15 +382,28 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
>
> r = 0;
> for (i = 0; i < number_of_requests; i++) {
> - r = amdgpu_cs_submit_one(context, ibs_request);
> + r = amdgpu_cs_submit_one(context, ibs_request, has_sems ? ibs_sem : NULL);
> if (r)
> break;
> ibs_request++;
> + if (has_sems)
> + ibs_sem++;
> }
>
> return r;
> }
>
> +int amdgpu_cs_submit(amdgpu_context_handle context,
> + uint64_t flags,
> + struct amdgpu_cs_request *ibs_request,
> + uint32_t number_of_requests)
> +{
> + return amdgpu_cs_submit_sem(context, flags,
> + ibs_request, NULL,
> + number_of_requests);
> +}
> +
> +
> /**
> * Calculate absolute timeout.
> *
> @@ -542,3 +600,88 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem)
> {
> return amdgpu_cs_unreference_sem(sem);
> }
> +
> +
> +int amdgpu_cs_create_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle *sem)
> +{
> + union drm_amdgpu_sem args;
> + int r;
> +
> + if (NULL == dev)
> + return -EINVAL;
> +
> + /* Create the context */
> + memset(&args, 0, sizeof(args));
> + args.in.op = AMDGPU_SEM_OP_CREATE_SEM;
> + r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
> + if (r)
> + return r;
> +
> + *sem = args.out.handle;
> +
> + return 0;
> +}
> +
> +int amdgpu_cs_export_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem,
> + int *shared_handle)
> +{
> + union drm_amdgpu_sem args;
> + int r;
> +
> + if (NULL == dev)
> + return -EINVAL;
> +
> + /* Create the context */
> + memset(&args, 0, sizeof(args));
> + args.in.op = AMDGPU_SEM_OP_EXPORT_SEM;
> + args.in.handle = sem;
> + r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
> + if (r)
> + return r;
> + *shared_handle = args.out.fd;
> + return 0;
> +}
> +
> +int amdgpu_cs_import_sem(amdgpu_device_handle dev,
> + int shared_handle,
> + amdgpu_sem_handle *sem)
> +{
> + union drm_amdgpu_sem args;
> + int r;
> +
> + if (NULL == dev)
> + return -EINVAL;
> +
> + /* Create the context */
> + memset(&args, 0, sizeof(args));
> + args.in.op = AMDGPU_SEM_OP_IMPORT_SEM;
> + args.in.handle = shared_handle;
> + r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
> + if (r)
> + return r;
> + *sem = args.out.handle;
> + return 0;
> +}
> +
> +
> +int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem)
> +{
> + union drm_amdgpu_sem args;
> + int r;
> +
> + if (NULL == dev)
> + return -EINVAL;
> +
> + /* Create the context */
> + memset(&args, 0, sizeof(args));
> + args.in.op = AMDGPU_SEM_OP_DESTROY_SEM;
> + args.in.handle = sem;
> + r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
> + if (r)
> + return r;
> +
> + return 0;
> +}
> diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
> index d8f2497..fa0bfe2 100644
> --- a/include/drm/amdgpu_drm.h
> +++ b/include/drm/amdgpu_drm.h
> @@ -50,6 +50,7 @@ extern "C" {
> #define DRM_AMDGPU_WAIT_CS 0x09
> #define DRM_AMDGPU_GEM_OP 0x10
> #define DRM_AMDGPU_GEM_USERPTR 0x11
> +#define DRM_AMDGPU_SEM 0x13
>
> #define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
> #define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
> @@ -63,6 +64,7 @@ extern "C" {
> #define DRM_IOCTL_AMDGPU_WAIT_CS DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_CS, union drm_amdgpu_wait_cs)
> #define DRM_IOCTL_AMDGPU_GEM_OP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op)
> #define DRM_IOCTL_AMDGPU_GEM_USERPTR DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
> +#define DRM_IOCTL_AMDGPU_SEM DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_SEM, union drm_amdgpu_sem)
>
> #define AMDGPU_GEM_DOMAIN_CPU 0x1
> #define AMDGPU_GEM_DOMAIN_GTT 0x2
> @@ -303,6 +305,26 @@ union drm_amdgpu_wait_cs {
> struct drm_amdgpu_wait_cs_out out;
> };
>
> +#define AMDGPU_SEM_OP_CREATE_SEM 0
> +#define AMDGPU_SEM_OP_IMPORT_SEM 1
> +#define AMDGPU_SEM_OP_EXPORT_SEM 2
> +#define AMDGPU_SEM_OP_DESTROY_SEM 3
> +
> +struct drm_amdgpu_sem_in {
> + __u32 op;
> + __u32 handle;
> +};
> +
> +struct drm_amdgpu_sem_out {
> + __u32 fd;
> + __u32 handle;
> +};
> +
> +union drm_amdgpu_sem {
> + struct drm_amdgpu_sem_in in;
> + struct drm_amdgpu_sem_out out;
> +};
> +
> #define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
> #define AMDGPU_GEM_OP_SET_PLACEMENT 1
>
> @@ -358,6 +380,8 @@ struct drm_amdgpu_gem_va {
> #define AMDGPU_CHUNK_ID_IB 0x01
> #define AMDGPU_CHUNK_ID_FENCE 0x02
> #define AMDGPU_CHUNK_ID_DEPENDENCIES 0x03
> +#define AMDGPU_CHUNK_ID_SEM_WAIT 0x04
> +#define AMDGPU_CHUNK_ID_SEM_SIGNAL 0x05
>
> struct drm_amdgpu_cs_chunk {
> uint32_t chunk_id;
> @@ -422,6 +446,10 @@ struct drm_amdgpu_cs_chunk_fence {
> uint32_t offset;
> };
>
> +struct drm_amdgpu_cs_chunk_sem {
> + uint32_t handle;
> +};
> +
> struct drm_amdgpu_cs_chunk_data {
> union {
> struct drm_amdgpu_cs_chunk_ib ib_data;
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <20170314005054.7487-2-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-03-15 10:30 ` Emil Velikov
0 siblings, 0 replies; 18+ messages in thread
From: Emil Velikov @ 2017-03-15 10:30 UTC (permalink / raw)
To: Dave Airlie; +Cc: ML dri-devel, amd-gfx mailing list
Hi Dave,
Barring the other discussions, allow me to put a couple of trivial suggestions:
Please re-wrap the long lines to follow existing code style.
On 14 March 2017 at 00:50, Dave Airlie <airlied@gmail.com> wrote:
> @@ -882,6 +894,12 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
> struct amdgpu_cs_request *ibs_request,
> uint32_t number_of_requests);
>
> +int amdgpu_cs_submit_sem(amdgpu_context_handle context,
> + uint64_t flags,
> + struct amdgpu_cs_request *ibs_request,
> + struct amdgpu_cs_request_sem *ibs_sem,
> + uint32_t number_of_requests);
> +
> /**
> * Query status of Command Buffer Submission
> *
> @@ -1255,4 +1273,14 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem);
> */
> const char *amdgpu_get_marketing_name(amdgpu_device_handle dev);
>
> +int amdgpu_cs_create_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle *sem);
> +int amdgpu_cs_export_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem,
> + int *shared_handle);
> +int amdgpu_cs_import_sem(amdgpu_device_handle dev,
> + int shared_handle,
> + amdgpu_sem_handle *sem);
> +int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem);
The new symbols should be added to the amdgpu-symbol-check test.
If in doubt - run `make -C amdgpu check'
> --- a/include/drm/amdgpu_drm.h
> +++ b/include/drm/amdgpu_drm.h
Please sync this as PATCH 1/2 via "make headers_install" + cp + git
commit -asm "....Generated using make headers_install.\nGenerated from
$tree/branch commit $sha."
There's a handful of other changes that are missing/should be merged.
> @@ -50,6 +50,7 @@ extern "C" {
> +struct drm_amdgpu_cs_chunk_sem {
> + uint32_t handle;
> +};
> +
Seems unused in the UAPI header - might what to add a note ?
Also sizeof(struct drm_amdgpu_cs_chunk_sem) is not multiple of 64bit -
worth mentioning that it's safe and/or why ?
Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
2017-03-15 8:48 ` Daniel Vetter
@ 2017-03-15 9:35 ` Christian König
0 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2017-03-15 9:35 UTC (permalink / raw)
To: Daniel Vetter, Marek Olšák
Cc: Mao, David, dri-devel, amd-gfx mailing list
Am 15.03.2017 um 09:48 schrieb Daniel Vetter:
> On Wed, Mar 15, 2017 at 01:01:19AM +0100, Marek Olšák wrote:
>> While it's nice that you are all having fun here, I don't think that's
>> the way to communicate this.
>>
>> The truth is, if AMD had an open source driver using the semaphores
>> (e.g. Vulkan) and if the libdrm semaphore code was merged, Dave
>> wouldn't be able to change it, ever. If a dependent open source
>> project relies on some libdrm interface, that interface is set in
>> stone. So AMD wouldn't be able to change it either. Unfortunately,
>> such an open source project doesn't exist, so the community can assume
>> that this libdrm code is still in the "initial design phase". Dave has
>> an upper hand here, because he actually has an open source project
>> that will use this, while AMD doesn't (yet). However, AMD can still
>> negotiate some details here, i.e. do a proper review.
> Fully agreed, that's why there was this "internal" qualifier. If you do
> this internally, then it's not final, if you discuss it here on the m-l,
> it actually matters. So drag your internal teams into the public, and it's
> all fine.
Unfortunately it's not done with that. You also need to raise company
wide awareness of changed needs.
For example the concept that changes aren't allowed to break older
upstream components is completely new to most teams inside AMD.
It's a rather painful learning curve when you want to move projects from
closed source to open source.
Christian.
> -Daniel
>
>> Marek
>>
>>
>> On Tue, Mar 14, 2017 at 7:10 PM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>> Am 14.03.2017 um 18:45 schrieb Harry Wentland:
>>>> On 2017-03-14 06:04 AM, zhoucm1 wrote:
>>>>>
>>>>>
>>>>> On 2017年03月14日 17:20, Christian König wrote:
>>>>>> Am 14.03.2017 um 09:54 schrieb Daniel Vetter:
>>>>>>> On Tue, Mar 14, 2017 at 11:30:40AM +0800, zhoucm1 wrote:
>>>>>>>>
>>>>>>>> On 2017年03月14日 10:52, Dave Airlie wrote:
>>>>>>>>> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
>>>>>>>>>> Hi Dave,
>>>>>>>>>>
>>>>>>>>>> Could you tell me why you create your new one patch? I remember I
>>>>>>>>>> send out
>>>>>>>>>> our the whole implementation, Why not directly review our patches?
>>>>>>>>> This is patch review, I'm not sure what you are expecting in terms of
>>>>>>>>> direct review.
>>>>>>>>>
>>>>>>>>> The patches you sent out were reviewed by Christian, he stated he
>>>>>>>>> thinks this should
>>>>>>>>> use sync_file, I was interested to see if this was actually possible,
>>>>>>>>> so I just adapted
>>>>>>>>> the patches to check if it was possible to avoid adding a lot of amd
>>>>>>>>> specific code
>>>>>>>>> for something that isn't required to be. Hence why I've sent this as
>>>>>>>>> an rfc, as I want
>>>>>>>>> to see if others think using sync_file is a good or bad idea. We may
>>>>>>>>> end up going
>>>>>>>>> back to the non-sync_file based patches if this proves to be a bad
>>>>>>>>> idea, so far it
>>>>>>>>> doesn't look like one.
>>>>>>>>>
>>>>>>>>> I also reviewed the patches and noted it shouldn't add the
>>>>>>>>> wait/signal
>>>>>>>>> interfaces,
>>>>>>>>> that it should use the chunks on command submission, so while I was
>>>>>>>>> in
>>>>>>>>> there I re
>>>>>>>>> wrote that as well.
>>>>>>>> Yeah, then I'm ok with this, just our internal team has used this
>>>>>>>> implementation, they find some gaps between yours and previous, they
>>>>>>>> could
>>>>>>>> need to change their's again, they are annoyance for this.
>>>>>>> This is why you _must_ get anything you're doing discussed in upstream
>>>>>>> first. Your internal teams simply do not have design authority on stuff
>>>>>>> like that. And yes it takes forever to get formerly proprietary
>>>>>>> internal-only teams to understand this, speaking from years of
>>>>>>> experience
>>>>>>> implement a proper upstream-design-first approach to feature
>>>>>>> development
>>>>>>> here at intel.
>>>>>>
>>>>>> "internal teams simply do not have design authority on stuff like that"
>>>>>>
>>>>>> Can I print that on a t-shirt and start to sell it?
>>>>> I guess you cannot dress it to go to office..:)
>>>>>
>>>> I'd wear it to the office.
>>>>
>>>> https://www.customink.com/lab?cid=hkp0-00ay-6vjg
>>>
>>> I'm only at an AMD office every few years, so that's probably not worth it.
>>>
>>> Anyway it's really something we should keep in mind if we want to upstream
>>> things.
>>>
>>> Christian.
>>>
>>>
>>>> Harry
>>>>
>>>>> David Zhou
>>>>>>
>>>>>> Christian.
>>>>>>
>>>>>>> -Daniel
>>>>>>
>>>>>>
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <CAAxE2A7xRWhkp-OC59iy2i91uj5mtVTHR0uHfDf745L-ixxHcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-03-15 8:48 ` Daniel Vetter
2017-03-15 9:35 ` Christian König
0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2017-03-15 8:48 UTC (permalink / raw)
To: Marek Olšák
Cc: zhoucm1, Mao, David, Harry Wentland, amd-gfx mailing list,
Christian König, dri-devel, Daniel Vetter, Dave Airlie
On Wed, Mar 15, 2017 at 01:01:19AM +0100, Marek Olšák wrote:
> While it's nice that you are all having fun here, I don't think that's
> the way to communicate this.
>
> The truth is, if AMD had an open source driver using the semaphores
> (e.g. Vulkan) and if the libdrm semaphore code was merged, Dave
> wouldn't be able to change it, ever. If a dependent open source
> project relies on some libdrm interface, that interface is set in
> stone. So AMD wouldn't be able to change it either. Unfortunately,
> such an open source project doesn't exist, so the community can assume
> that this libdrm code is still in the "initial design phase". Dave has
> an upper hand here, because he actually has an open source project
> that will use this, while AMD doesn't (yet). However, AMD can still
> negotiate some details here, i.e. do a proper review.
Fully agreed, that's why there was this "internal" qualifier. If you do
this internally, then it's not final, if you discuss it here on the m-l,
it actually matters. So drag your internal teams into the public, and it's
all fine.
-Daniel
>
> Marek
>
>
> On Tue, Mar 14, 2017 at 7:10 PM, Christian König
> <deathsimple@vodafone.de> wrote:
> > Am 14.03.2017 um 18:45 schrieb Harry Wentland:
> >>
> >> On 2017-03-14 06:04 AM, zhoucm1 wrote:
> >>>
> >>>
> >>>
> >>> On 2017年03月14日 17:20, Christian König wrote:
> >>>>
> >>>> Am 14.03.2017 um 09:54 schrieb Daniel Vetter:
> >>>>>
> >>>>> On Tue, Mar 14, 2017 at 11:30:40AM +0800, zhoucm1 wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 2017年03月14日 10:52, Dave Airlie wrote:
> >>>>>>>
> >>>>>>> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
> >>>>>>>>
> >>>>>>>> Hi Dave,
> >>>>>>>>
> >>>>>>>> Could you tell me why you create your new one patch? I remember I
> >>>>>>>> send out
> >>>>>>>> our the whole implementation, Why not directly review our patches?
> >>>>>>>
> >>>>>>> This is patch review, I'm not sure what you are expecting in terms of
> >>>>>>> direct review.
> >>>>>>>
> >>>>>>> The patches you sent out were reviewed by Christian, he stated he
> >>>>>>> thinks this should
> >>>>>>> use sync_file, I was interested to see if this was actually possible,
> >>>>>>> so I just adapted
> >>>>>>> the patches to check if it was possible to avoid adding a lot of amd
> >>>>>>> specific code
> >>>>>>> for something that isn't required to be. Hence why I've sent this as
> >>>>>>> an rfc, as I want
> >>>>>>> to see if others think using sync_file is a good or bad idea. We may
> >>>>>>> end up going
> >>>>>>> back to the non-sync_file based patches if this proves to be a bad
> >>>>>>> idea, so far it
> >>>>>>> doesn't look like one.
> >>>>>>>
> >>>>>>> I also reviewed the patches and noted it shouldn't add the
> >>>>>>> wait/signal
> >>>>>>> interfaces,
> >>>>>>> that it should use the chunks on command submission, so while I was
> >>>>>>> in
> >>>>>>> there I re
> >>>>>>> wrote that as well.
> >>>>>>
> >>>>>> Yeah, then I'm ok with this, just our internal team has used this
> >>>>>> implementation, they find some gaps between yours and previous, they
> >>>>>> could
> >>>>>> need to change their's again, they are annoyance for this.
> >>>>>
> >>>>> This is why you _must_ get anything you're doing discussed in upstream
> >>>>> first. Your internal teams simply do not have design authority on stuff
> >>>>> like that. And yes it takes forever to get formerly proprietary
> >>>>> internal-only teams to understand this, speaking from years of
> >>>>> experience
> >>>>> implement a proper upstream-design-first approach to feature
> >>>>> development
> >>>>> here at intel.
> >>>>
> >>>>
> >>>> "internal teams simply do not have design authority on stuff like that"
> >>>>
> >>>> Can I print that on a t-shirt and start to sell it?
> >>>
> >>> I guess you cannot dress it to go to office..:)
> >>>
> >>
> >> I'd wear it to the office.
> >>
> >> https://www.customink.com/lab?cid=hkp0-00ay-6vjg
> >
> >
> > I'm only at an AMD office every few years, so that's probably not worth it.
> >
> > Anyway it's really something we should keep in mind if we want to upstream
> > things.
> >
> > Christian.
> >
> >
> >>
> >> Harry
> >>
> >>> David Zhou
> >>>>
> >>>>
> >>>> Christian.
> >>>>
> >>>>> -Daniel
> >>>>
> >>>>
> >>>>
> >>>
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> >
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
2017-03-14 18:10 ` Christian König
@ 2017-03-15 0:01 ` Marek Olšák
[not found] ` <CAAxE2A7xRWhkp-OC59iy2i91uj5mtVTHR0uHfDf745L-ixxHcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 18+ messages in thread
From: Marek Olšák @ 2017-03-15 0:01 UTC (permalink / raw)
To: Christian König; +Cc: Mao, David, dri-devel, amd-gfx mailing list
While it's nice that you are all having fun here, I don't think that's
the way to communicate this.
The truth is, if AMD had an open source driver using the semaphores
(e.g. Vulkan) and if the libdrm semaphore code was merged, Dave
wouldn't be able to change it, ever. If a dependent open source
project relies on some libdrm interface, that interface is set in
stone. So AMD wouldn't be able to change it either. Unfortunately,
such an open source project doesn't exist, so the community can assume
that this libdrm code is still in the "initial design phase". Dave has
an upper hand here, because he actually has an open source project
that will use this, while AMD doesn't (yet). However, AMD can still
negotiate some details here, i.e. do a proper review.
Marek
On Tue, Mar 14, 2017 at 7:10 PM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 14.03.2017 um 18:45 schrieb Harry Wentland:
>>
>> On 2017-03-14 06:04 AM, zhoucm1 wrote:
>>>
>>>
>>>
>>> On 2017年03月14日 17:20, Christian König wrote:
>>>>
>>>> Am 14.03.2017 um 09:54 schrieb Daniel Vetter:
>>>>>
>>>>> On Tue, Mar 14, 2017 at 11:30:40AM +0800, zhoucm1 wrote:
>>>>>>
>>>>>>
>>>>>> On 2017年03月14日 10:52, Dave Airlie wrote:
>>>>>>>
>>>>>>> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
>>>>>>>>
>>>>>>>> Hi Dave,
>>>>>>>>
>>>>>>>> Could you tell me why you create your new one patch? I remember I
>>>>>>>> send out
>>>>>>>> our the whole implementation, Why not directly review our patches?
>>>>>>>
>>>>>>> This is patch review, I'm not sure what you are expecting in terms of
>>>>>>> direct review.
>>>>>>>
>>>>>>> The patches you sent out were reviewed by Christian, he stated he
>>>>>>> thinks this should
>>>>>>> use sync_file, I was interested to see if this was actually possible,
>>>>>>> so I just adapted
>>>>>>> the patches to check if it was possible to avoid adding a lot of amd
>>>>>>> specific code
>>>>>>> for something that isn't required to be. Hence why I've sent this as
>>>>>>> an rfc, as I want
>>>>>>> to see if others think using sync_file is a good or bad idea. We may
>>>>>>> end up going
>>>>>>> back to the non-sync_file based patches if this proves to be a bad
>>>>>>> idea, so far it
>>>>>>> doesn't look like one.
>>>>>>>
>>>>>>> I also reviewed the patches and noted it shouldn't add the
>>>>>>> wait/signal
>>>>>>> interfaces,
>>>>>>> that it should use the chunks on command submission, so while I was
>>>>>>> in
>>>>>>> there I re
>>>>>>> wrote that as well.
>>>>>>
>>>>>> Yeah, then I'm ok with this, just our internal team has used this
>>>>>> implementation, they find some gaps between yours and previous, they
>>>>>> could
>>>>>> need to change their's again, they are annoyance for this.
>>>>>
>>>>> This is why you _must_ get anything you're doing discussed in upstream
>>>>> first. Your internal teams simply do not have design authority on stuff
>>>>> like that. And yes it takes forever to get formerly proprietary
>>>>> internal-only teams to understand this, speaking from years of
>>>>> experience
>>>>> implement a proper upstream-design-first approach to feature
>>>>> development
>>>>> here at intel.
>>>>
>>>>
>>>> "internal teams simply do not have design authority on stuff like that"
>>>>
>>>> Can I print that on a t-shirt and start to sell it?
>>>
>>> I guess you cannot dress it to go to office..:)
>>>
>>
>> I'd wear it to the office.
>>
>> https://www.customink.com/lab?cid=hkp0-00ay-6vjg
>
>
> I'm only at an AMD office every few years, so that's probably not worth it.
>
> Anyway it's really something we should keep in mind if we want to upstream
> things.
>
> Christian.
>
>
>>
>> Harry
>>
>>> David Zhou
>>>>
>>>>
>>>> Christian.
>>>>
>>>>> -Daniel
>>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <8dea3303-480d-c29a-22ec-42adf3dab4ce-5C7GfCeVMHo@public.gmane.org>
2017-03-14 17:54 ` Daniel Vetter
@ 2017-03-14 18:10 ` Christian König
2017-03-15 0:01 ` Marek Olšák
1 sibling, 1 reply; 18+ messages in thread
From: Christian König @ 2017-03-14 18:10 UTC (permalink / raw)
To: Harry Wentland, zhoucm1, Daniel Vetter
Cc: Dave Airlie, amd-gfx mailing list, dri-devel, Mao, David
Am 14.03.2017 um 18:45 schrieb Harry Wentland:
> On 2017-03-14 06:04 AM, zhoucm1 wrote:
>>
>>
>> On 2017年03月14日 17:20, Christian König wrote:
>>> Am 14.03.2017 um 09:54 schrieb Daniel Vetter:
>>>> On Tue, Mar 14, 2017 at 11:30:40AM +0800, zhoucm1 wrote:
>>>>>
>>>>> On 2017年03月14日 10:52, Dave Airlie wrote:
>>>>>> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> Could you tell me why you create your new one patch? I remember I
>>>>>>> send out
>>>>>>> our the whole implementation, Why not directly review our patches?
>>>>>> This is patch review, I'm not sure what you are expecting in
>>>>>> terms of
>>>>>> direct review.
>>>>>>
>>>>>> The patches you sent out were reviewed by Christian, he stated he
>>>>>> thinks this should
>>>>>> use sync_file, I was interested to see if this was actually
>>>>>> possible,
>>>>>> so I just adapted
>>>>>> the patches to check if it was possible to avoid adding a lot of amd
>>>>>> specific code
>>>>>> for something that isn't required to be. Hence why I've sent this as
>>>>>> an rfc, as I want
>>>>>> to see if others think using sync_file is a good or bad idea. We may
>>>>>> end up going
>>>>>> back to the non-sync_file based patches if this proves to be a bad
>>>>>> idea, so far it
>>>>>> doesn't look like one.
>>>>>>
>>>>>> I also reviewed the patches and noted it shouldn't add the
>>>>>> wait/signal
>>>>>> interfaces,
>>>>>> that it should use the chunks on command submission, so while I
>>>>>> was in
>>>>>> there I re
>>>>>> wrote that as well.
>>>>> Yeah, then I'm ok with this, just our internal team has used this
>>>>> implementation, they find some gaps between yours and previous, they
>>>>> could
>>>>> need to change their's again, they are annoyance for this.
>>>> This is why you _must_ get anything you're doing discussed in upstream
>>>> first. Your internal teams simply do not have design authority on
>>>> stuff
>>>> like that. And yes it takes forever to get formerly proprietary
>>>> internal-only teams to understand this, speaking from years of
>>>> experience
>>>> implement a proper upstream-design-first approach to feature
>>>> development
>>>> here at intel.
>>>
>>> "internal teams simply do not have design authority on stuff like that"
>>>
>>> Can I print that on a t-shirt and start to sell it?
>> I guess you cannot dress it to go to office..:)
>>
>
> I'd wear it to the office.
>
> https://www.customink.com/lab?cid=hkp0-00ay-6vjg
I'm only at an AMD office every few years, so that's probably not worth it.
Anyway it's really something we should keep in mind if we want to
upstream things.
Christian.
>
> Harry
>
>> David Zhou
>>>
>>> Christian.
>>>
>>>> -Daniel
>>>
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <8dea3303-480d-c29a-22ec-42adf3dab4ce-5C7GfCeVMHo@public.gmane.org>
@ 2017-03-14 17:54 ` Daniel Vetter
2017-03-14 18:10 ` Christian König
1 sibling, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2017-03-14 17:54 UTC (permalink / raw)
To: Harry Wentland
Cc: zhoucm1, Mao, David, amd-gfx mailing list, Christian König,
dri-devel, Dave Airlie
On Tue, Mar 14, 2017 at 6:45 PM, Harry Wentland <harry.wentland@amd.com> wrote:
>>> "internal teams simply do not have design authority on stuff like that"
>>>
>>> Can I print that on a t-shirt and start to sell it?
>>
>> I guess you cannot dress it to go to office..:)
>>
>
> I'd wear it to the office.
>
> https://www.customink.com/lab?cid=hkp0-00ay-6vjg
Can we have a picture of the entire amd team wearing these? :-)
Anyway, I approve.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <58C7C032.7060706-5C7GfCeVMHo@public.gmane.org>
@ 2017-03-14 17:45 ` Harry Wentland
[not found] ` <8dea3303-480d-c29a-22ec-42adf3dab4ce-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 18+ messages in thread
From: Harry Wentland @ 2017-03-14 17:45 UTC (permalink / raw)
To: zhoucm1, Christian König, Daniel Vetter
Cc: Dave Airlie, amd-gfx mailing list, dri-devel, Mao, David
On 2017-03-14 06:04 AM, zhoucm1 wrote:
>
>
> On 2017年03月14日 17:20, Christian König wrote:
>> Am 14.03.2017 um 09:54 schrieb Daniel Vetter:
>>> On Tue, Mar 14, 2017 at 11:30:40AM +0800, zhoucm1 wrote:
>>>>
>>>> On 2017年03月14日 10:52, Dave Airlie wrote:
>>>>> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
>>>>>> Hi Dave,
>>>>>>
>>>>>> Could you tell me why you create your new one patch? I remember I
>>>>>> send out
>>>>>> our the whole implementation, Why not directly review our patches?
>>>>> This is patch review, I'm not sure what you are expecting in terms of
>>>>> direct review.
>>>>>
>>>>> The patches you sent out were reviewed by Christian, he stated he
>>>>> thinks this should
>>>>> use sync_file, I was interested to see if this was actually possible,
>>>>> so I just adapted
>>>>> the patches to check if it was possible to avoid adding a lot of amd
>>>>> specific code
>>>>> for something that isn't required to be. Hence why I've sent this as
>>>>> an rfc, as I want
>>>>> to see if others think using sync_file is a good or bad idea. We may
>>>>> end up going
>>>>> back to the non-sync_file based patches if this proves to be a bad
>>>>> idea, so far it
>>>>> doesn't look like one.
>>>>>
>>>>> I also reviewed the patches and noted it shouldn't add the wait/signal
>>>>> interfaces,
>>>>> that it should use the chunks on command submission, so while I was in
>>>>> there I re
>>>>> wrote that as well.
>>>> Yeah, then I'm ok with this, just our internal team has used this
>>>> implementation, they find some gaps between yours and previous, they
>>>> could
>>>> need to change their's again, they are annoyance for this.
>>> This is why you _must_ get anything you're doing discussed in upstream
>>> first. Your internal teams simply do not have design authority on stuff
>>> like that. And yes it takes forever to get formerly proprietary
>>> internal-only teams to understand this, speaking from years of
>>> experience
>>> implement a proper upstream-design-first approach to feature development
>>> here at intel.
>>
>> "internal teams simply do not have design authority on stuff like that"
>>
>> Can I print that on a t-shirt and start to sell it?
> I guess you cannot dress it to go to office..:)
>
I'd wear it to the office.
https://www.customink.com/lab?cid=hkp0-00ay-6vjg
Harry
> David Zhou
>>
>> Christian.
>>
>>> -Daniel
>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
2017-03-14 9:20 ` Christian König
@ 2017-03-14 10:04 ` zhoucm1
[not found] ` <58C7C032.7060706-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 18+ messages in thread
From: zhoucm1 @ 2017-03-14 10:04 UTC (permalink / raw)
To: Christian König, Daniel Vetter
Cc: dri-devel, amd-gfx mailing list, Mao, David
On 2017年03月14日 17:20, Christian König wrote:
> Am 14.03.2017 um 09:54 schrieb Daniel Vetter:
>> On Tue, Mar 14, 2017 at 11:30:40AM +0800, zhoucm1 wrote:
>>>
>>> On 2017年03月14日 10:52, Dave Airlie wrote:
>>>> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
>>>>> Hi Dave,
>>>>>
>>>>> Could you tell me why you create your new one patch? I remember I
>>>>> send out
>>>>> our the whole implementation, Why not directly review our patches?
>>>> This is patch review, I'm not sure what you are expecting in terms of
>>>> direct review.
>>>>
>>>> The patches you sent out were reviewed by Christian, he stated he
>>>> thinks this should
>>>> use sync_file, I was interested to see if this was actually possible,
>>>> so I just adapted
>>>> the patches to check if it was possible to avoid adding a lot of amd
>>>> specific code
>>>> for something that isn't required to be. Hence why I've sent this as
>>>> an rfc, as I want
>>>> to see if others think using sync_file is a good or bad idea. We may
>>>> end up going
>>>> back to the non-sync_file based patches if this proves to be a bad
>>>> idea, so far it
>>>> doesn't look like one.
>>>>
>>>> I also reviewed the patches and noted it shouldn't add the wait/signal
>>>> interfaces,
>>>> that it should use the chunks on command submission, so while I was in
>>>> there I re
>>>> wrote that as well.
>>> Yeah, then I'm ok with this, just our internal team has used this
>>> implementation, they find some gaps between yours and previous, they
>>> could
>>> need to change their's again, they are annoyance for this.
>> This is why you _must_ get anything you're doing discussed in upstream
>> first. Your internal teams simply do not have design authority on stuff
>> like that. And yes it takes forever to get formerly proprietary
>> internal-only teams to understand this, speaking from years of
>> experience
>> implement a proper upstream-design-first approach to feature development
>> here at intel.
>
> "internal teams simply do not have design authority on stuff like that"
>
> Can I print that on a t-shirt and start to sell it?
I guess you cannot dress it to go to office..:)
David Zhou
>
> Christian.
>
>> -Daniel
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
2017-03-14 8:54 ` Daniel Vetter
@ 2017-03-14 9:20 ` Christian König
2017-03-14 10:04 ` zhoucm1
0 siblings, 1 reply; 18+ messages in thread
From: Christian König @ 2017-03-14 9:20 UTC (permalink / raw)
To: Daniel Vetter, zhoucm1; +Cc: dri-devel, amd-gfx mailing list, Mao, David
Am 14.03.2017 um 09:54 schrieb Daniel Vetter:
> On Tue, Mar 14, 2017 at 11:30:40AM +0800, zhoucm1 wrote:
>>
>> On 2017年03月14日 10:52, Dave Airlie wrote:
>>> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
>>>> Hi Dave,
>>>>
>>>> Could you tell me why you create your new one patch? I remember I send out
>>>> our the whole implementation, Why not directly review our patches?
>>> This is patch review, I'm not sure what you are expecting in terms of
>>> direct review.
>>>
>>> The patches you sent out were reviewed by Christian, he stated he
>>> thinks this should
>>> use sync_file, I was interested to see if this was actually possible,
>>> so I just adapted
>>> the patches to check if it was possible to avoid adding a lot of amd
>>> specific code
>>> for something that isn't required to be. Hence why I've sent this as
>>> an rfc, as I want
>>> to see if others think using sync_file is a good or bad idea. We may
>>> end up going
>>> back to the non-sync_file based patches if this proves to be a bad
>>> idea, so far it
>>> doesn't look like one.
>>>
>>> I also reviewed the patches and noted it shouldn't add the wait/signal
>>> interfaces,
>>> that it should use the chunks on command submission, so while I was in
>>> there I re
>>> wrote that as well.
>> Yeah, then I'm ok with this, just our internal team has used this
>> implementation, they find some gaps between yours and previous, they could
>> need to change their's again, they are annoyance for this.
> This is why you _must_ get anything you're doing discussed in upstream
> first. Your internal teams simply do not have design authority on stuff
> like that. And yes it takes forever to get formerly proprietary
> internal-only teams to understand this, speaking from years of experience
> implement a proper upstream-design-first approach to feature development
> here at intel.
"internal teams simply do not have design authority on stuff like that"
Can I print that on a t-shirt and start to sell it?
Christian.
> -Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
2017-03-14 4:16 ` Dave Airlie
@ 2017-03-14 8:56 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2017-03-14 8:56 UTC (permalink / raw)
To: Dave Airlie; +Cc: amd-gfx mailing list, dri-devel, Mao, David
On Tue, Mar 14, 2017 at 02:16:11PM +1000, Dave Airlie wrote:
> On 14 March 2017 at 13:30, zhoucm1 <david1.zhou@amd.com> wrote:
> >
> >
> > On 2017年03月14日 10:52, Dave Airlie wrote:
> >>
> >> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
> >>>
> >>> Hi Dave,
> >>>
> >>> Could you tell me why you create your new one patch? I remember I send
> >>> out
> >>> our the whole implementation, Why not directly review our patches?
> >>
> >> This is patch review, I'm not sure what you are expecting in terms of
> >> direct review.
> >>
> >> The patches you sent out were reviewed by Christian, he stated he
> >> thinks this should
> >> use sync_file, I was interested to see if this was actually possible,
> >> so I just adapted
> >> the patches to check if it was possible to avoid adding a lot of amd
> >> specific code
> >> for something that isn't required to be. Hence why I've sent this as
> >> an rfc, as I want
> >> to see if others think using sync_file is a good or bad idea. We may
> >> end up going
> >> back to the non-sync_file based patches if this proves to be a bad
> >> idea, so far it
> >> doesn't look like one.
> >>
> >> I also reviewed the patches and noted it shouldn't add the wait/signal
> >> interfaces,
> >> that it should use the chunks on command submission, so while I was in
> >> there I re
> >> wrote that as well.
> >
> > Yeah, then I'm ok with this, just our internal team has used this
> > implementation, they find some gaps between yours and previous, they could
> > need to change their's again, they are annoyance for this.
>
> This is unfortunate, but the more internal development done, the more
> this will happen,
> especially in areas where you might interact with the kernel. I'd
> suggest trying to
> develop stuff in the open much earlier (don't start anything in-house).
>
> Now that we have an open vulkan driver it might be that most features
> the internal
> vulkan driver requires will eventually be wanted by us,
Yeah that's another aspect, radv is the open vulkan driver for amd hw,
which means it gets to drive uapi.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
2017-03-14 3:30 ` zhoucm1
[not found] ` <58C763E0.1-5C7GfCeVMHo@public.gmane.org>
@ 2017-03-14 8:54 ` Daniel Vetter
2017-03-14 9:20 ` Christian König
1 sibling, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2017-03-14 8:54 UTC (permalink / raw)
To: zhoucm1; +Cc: amd-gfx mailing list, dri-devel, Mao, David
On Tue, Mar 14, 2017 at 11:30:40AM +0800, zhoucm1 wrote:
>
>
> On 2017年03月14日 10:52, Dave Airlie wrote:
> > On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
> > > Hi Dave,
> > >
> > > Could you tell me why you create your new one patch? I remember I send out
> > > our the whole implementation, Why not directly review our patches?
> > This is patch review, I'm not sure what you are expecting in terms of
> > direct review.
> >
> > The patches you sent out were reviewed by Christian, he stated he
> > thinks this should
> > use sync_file, I was interested to see if this was actually possible,
> > so I just adapted
> > the patches to check if it was possible to avoid adding a lot of amd
> > specific code
> > for something that isn't required to be. Hence why I've sent this as
> > an rfc, as I want
> > to see if others think using sync_file is a good or bad idea. We may
> > end up going
> > back to the non-sync_file based patches if this proves to be a bad
> > idea, so far it
> > doesn't look like one.
> >
> > I also reviewed the patches and noted it shouldn't add the wait/signal
> > interfaces,
> > that it should use the chunks on command submission, so while I was in
> > there I re
> > wrote that as well.
> Yeah, then I'm ok with this, just our internal team has used this
> implementation, they find some gaps between yours and previous, they could
> need to change their's again, they are annoyance for this.
This is why you _must_ get anything you're doing discussed in upstream
first. Your internal teams simply do not have design authority on stuff
like that. And yes it takes forever to get formerly proprietary
internal-only teams to understand this, speaking from years of experience
implement a proper upstream-design-first approach to feature development
here at intel.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <58C763E0.1-5C7GfCeVMHo@public.gmane.org>
@ 2017-03-14 4:16 ` Dave Airlie
2017-03-14 8:56 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Dave Airlie @ 2017-03-14 4:16 UTC (permalink / raw)
To: zhoucm1; +Cc: dri-devel, amd-gfx mailing list, Mao, David
On 14 March 2017 at 13:30, zhoucm1 <david1.zhou@amd.com> wrote:
>
>
> On 2017年03月14日 10:52, Dave Airlie wrote:
>>
>> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
>>>
>>> Hi Dave,
>>>
>>> Could you tell me why you create your new one patch? I remember I send
>>> out
>>> our the whole implementation, Why not directly review our patches?
>>
>> This is patch review, I'm not sure what you are expecting in terms of
>> direct review.
>>
>> The patches you sent out were reviewed by Christian, he stated he
>> thinks this should
>> use sync_file, I was interested to see if this was actually possible,
>> so I just adapted
>> the patches to check if it was possible to avoid adding a lot of amd
>> specific code
>> for something that isn't required to be. Hence why I've sent this as
>> an rfc, as I want
>> to see if others think using sync_file is a good or bad idea. We may
>> end up going
>> back to the non-sync_file based patches if this proves to be a bad
>> idea, so far it
>> doesn't look like one.
>>
>> I also reviewed the patches and noted it shouldn't add the wait/signal
>> interfaces,
>> that it should use the chunks on command submission, so while I was in
>> there I re
>> wrote that as well.
>
> Yeah, then I'm ok with this, just our internal team has used this
> implementation, they find some gaps between yours and previous, they could
> need to change their's again, they are annoyance for this.
This is unfortunate, but the more internal development done, the more
this will happen,
especially in areas where you might interact with the kernel. I'd
suggest trying to
develop stuff in the open much earlier (don't start anything in-house).
Now that we have an open vulkan driver it might be that most features
the internal
vulkan driver requires will eventually be wanted by us,
Dave.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <CAPM=9tzegdtiyEmbW+PqmVeQ+bXrNtrdKXuE3kB0dPMKnMir+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-03-14 3:30 ` zhoucm1
[not found] ` <58C763E0.1-5C7GfCeVMHo@public.gmane.org>
2017-03-14 8:54 ` Daniel Vetter
0 siblings, 2 replies; 18+ messages in thread
From: zhoucm1 @ 2017-03-14 3:30 UTC (permalink / raw)
To: Dave Airlie; +Cc: dri-devel, amd-gfx mailing list, Mao, David
On 2017年03月14日 10:52, Dave Airlie wrote:
> On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
>> Hi Dave,
>>
>> Could you tell me why you create your new one patch? I remember I send out
>> our the whole implementation, Why not directly review our patches?
> This is patch review, I'm not sure what you are expecting in terms of
> direct review.
>
> The patches you sent out were reviewed by Christian, he stated he
> thinks this should
> use sync_file, I was interested to see if this was actually possible,
> so I just adapted
> the patches to check if it was possible to avoid adding a lot of amd
> specific code
> for something that isn't required to be. Hence why I've sent this as
> an rfc, as I want
> to see if others think using sync_file is a good or bad idea. We may
> end up going
> back to the non-sync_file based patches if this proves to be a bad
> idea, so far it
> doesn't look like one.
>
> I also reviewed the patches and noted it shouldn't add the wait/signal
> interfaces,
> that it should use the chunks on command submission, so while I was in
> there I re
> wrote that as well.
Yeah, then I'm ok with this, just our internal team has used this
implementation, they find some gaps between yours and previous, they
could need to change their's again, they are annoyance for this.
Regards,
David Zhou
>
> Dave.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
2017-03-14 2:00 ` zhoucm1
@ 2017-03-14 2:52 ` Dave Airlie
[not found] ` <CAPM=9tzegdtiyEmbW+PqmVeQ+bXrNtrdKXuE3kB0dPMKnMir+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 18+ messages in thread
From: Dave Airlie @ 2017-03-14 2:52 UTC (permalink / raw)
To: zhoucm1; +Cc: dri-devel, amd-gfx mailing list, Mao, David
On 14 March 2017 at 12:00, zhoucm1 <david1.zhou@amd.com> wrote:
> Hi Dave,
>
> Could you tell me why you create your new one patch? I remember I send out
> our the whole implementation, Why not directly review our patches?
This is patch review, I'm not sure what you are expecting in terms of
direct review.
The patches you sent out were reviewed by Christian, he stated he
thinks this should
use sync_file, I was interested to see if this was actually possible,
so I just adapted
the patches to check if it was possible to avoid adding a lot of amd
specific code
for something that isn't required to be. Hence why I've sent this as
an rfc, as I want
to see if others think using sync_file is a good or bad idea. We may
end up going
back to the non-sync_file based patches if this proves to be a bad
idea, so far it
doesn't look like one.
I also reviewed the patches and noted it shouldn't add the wait/signal
interfaces,
that it should use the chunks on command submission, so while I was in
there I re
wrote that as well.
Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
2017-03-14 0:50 ` [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores Dave Airlie
@ 2017-03-14 2:00 ` zhoucm1
2017-03-14 2:52 ` Dave Airlie
[not found] ` <20170314005054.7487-2-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 18+ messages in thread
From: zhoucm1 @ 2017-03-14 2:00 UTC (permalink / raw)
To: Dave Airlie, amd-gfx, dri-devel, Mao, David
Hi Dave,
Could you tell me why you create your new one patch? I remember I send
out our the whole implementation, Why not directly review our patches?
Thanks,
David Zhou
On 2017年03月14日 08:50, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> This adds the corresponding code for libdrm to use the new
> kernel interfaces for semaphores.
>
> This will be used by radv to implement shared semaphores.
>
> TODO: Version checks.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
> amdgpu/amdgpu.h | 28 +++++++++
> amdgpu/amdgpu_cs.c | 161 ++++++++++++++++++++++++++++++++++++++++++++---
> include/drm/amdgpu_drm.h | 28 +++++++++
> 3 files changed, 208 insertions(+), 9 deletions(-)
>
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index 7b26a04..747e248 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -129,6 +129,8 @@ typedef struct amdgpu_va *amdgpu_va_handle;
> */
> typedef struct amdgpu_semaphore *amdgpu_semaphore_handle;
>
> +typedef uint32_t amdgpu_sem_handle;
> +
> /*--------------------------------------------------------------------------*/
> /* -------------------------- Structures ---------------------------------- */
> /*--------------------------------------------------------------------------*/
> @@ -365,6 +367,16 @@ struct amdgpu_cs_request {
> struct amdgpu_cs_fence_info fence_info;
> };
>
> +struct amdgpu_cs_request_sem {
> + /*
> + *
> + */
> + uint32_t number_of_wait_sem;
> + uint32_t *wait_sems;
> + uint32_t number_of_signal_sem;
> + uint32_t *signal_sems;
> +};
> +
> /**
> * Structure which provide information about GPU VM MC Address space
> * alignments requirements
> @@ -882,6 +894,12 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
> struct amdgpu_cs_request *ibs_request,
> uint32_t number_of_requests);
>
> +int amdgpu_cs_submit_sem(amdgpu_context_handle context,
> + uint64_t flags,
> + struct amdgpu_cs_request *ibs_request,
> + struct amdgpu_cs_request_sem *ibs_sem,
> + uint32_t number_of_requests);
> +
> /**
> * Query status of Command Buffer Submission
> *
> @@ -1255,4 +1273,14 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem);
> */
> const char *amdgpu_get_marketing_name(amdgpu_device_handle dev);
>
> +int amdgpu_cs_create_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle *sem);
> +int amdgpu_cs_export_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem,
> + int *shared_handle);
> +int amdgpu_cs_import_sem(amdgpu_device_handle dev,
> + int shared_handle,
> + amdgpu_sem_handle *sem);
> +int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem);
> #endif /* #ifdef _AMDGPU_H_ */
> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
> index fb5b3a8..7283327 100644
> --- a/amdgpu/amdgpu_cs.c
> +++ b/amdgpu/amdgpu_cs.c
> @@ -170,7 +170,8 @@ int amdgpu_cs_query_reset_state(amdgpu_context_handle context,
> * \sa amdgpu_cs_submit()
> */
> static int amdgpu_cs_submit_one(amdgpu_context_handle context,
> - struct amdgpu_cs_request *ibs_request)
> + struct amdgpu_cs_request *ibs_request,
> + struct amdgpu_cs_request_sem *sem_request)
> {
> union drm_amdgpu_cs cs;
> uint64_t *chunk_array;
> @@ -178,9 +179,11 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
> struct drm_amdgpu_cs_chunk_data *chunk_data;
> struct drm_amdgpu_cs_chunk_dep *dependencies = NULL;
> struct drm_amdgpu_cs_chunk_dep *sem_dependencies = NULL;
> + struct drm_amdgpu_cs_chunk_sem *wait_sem_dependencies = NULL;
> + struct drm_amdgpu_cs_chunk_sem *signal_sem_dependencies = NULL;
> struct list_head *sem_list;
> amdgpu_semaphore_handle sem, tmp;
> - uint32_t i, size, sem_count = 0;
> + uint32_t i, j, size, sem_count = 0;
> bool user_fence;
> int r = 0;
>
> @@ -196,7 +199,7 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
> }
> user_fence = (ibs_request->fence_info.handle != NULL);
>
> - size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1;
> + size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1 + (sem_request ? 2 : 0);
>
> chunk_array = alloca(sizeof(uint64_t) * size);
> chunks = alloca(sizeof(struct drm_amdgpu_cs_chunk) * size);
> @@ -308,6 +311,45 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
> chunks[i].chunk_data = (uint64_t)(uintptr_t)sem_dependencies;
> }
>
> + if (sem_request) {
> + if (sem_request->number_of_wait_sem) {
> + wait_sem_dependencies = malloc(sizeof(struct drm_amdgpu_cs_chunk_sem) * sem_request->number_of_wait_sem);
> + if (!wait_sem_dependencies) {
> + r = -ENOMEM;
> + goto error_unlock;
> + }
> + for (j = 0; j < sem_request->number_of_wait_sem; j++) {
> + struct drm_amdgpu_cs_chunk_sem *dep = &wait_sem_dependencies[j];
> + dep->handle = sem_request->wait_sems[j];
> + }
> + i = cs.in.num_chunks++;
> +
> + /* dependencies chunk */
> + chunk_array[i] = (uint64_t)(uintptr_t)&chunks[i];
> + chunks[i].chunk_id = AMDGPU_CHUNK_ID_SEM_WAIT;
> + chunks[i].length_dw = sizeof(struct drm_amdgpu_cs_chunk_sem) / 4 * sem_request->number_of_wait_sem;
> + chunks[i].chunk_data = (uint64_t)(uintptr_t)wait_sem_dependencies;
> + }
> + if (sem_request->number_of_signal_sem) {
> + signal_sem_dependencies = malloc(sizeof(struct drm_amdgpu_cs_chunk_sem) * sem_request->number_of_signal_sem);
> + if (!signal_sem_dependencies) {
> + r = -ENOMEM;
> + goto error_unlock;
> + }
> + for (j = 0; j < sem_request->number_of_signal_sem; j++) {
> + struct drm_amdgpu_cs_chunk_sem *dep = &signal_sem_dependencies[j];
> + dep->handle = sem_request->signal_sems[j];
> + }
> + i = cs.in.num_chunks++;
> +
> + /* dependencies chunk */
> + chunk_array[i] = (uint64_t)(uintptr_t)&chunks[i];
> + chunks[i].chunk_id = AMDGPU_CHUNK_ID_SEM_SIGNAL;
> + chunks[i].length_dw = sizeof(struct drm_amdgpu_cs_chunk_sem) / 4 * sem_request->number_of_signal_sem;
> + chunks[i].chunk_data = (uint64_t)(uintptr_t)signal_sem_dependencies;
> + }
> + }
> +
> r = drmCommandWriteRead(context->dev->fd, DRM_AMDGPU_CS,
> &cs, sizeof(cs));
> if (r)
> @@ -319,17 +361,20 @@ error_unlock:
> pthread_mutex_unlock(&context->sequence_mutex);
> free(dependencies);
> free(sem_dependencies);
> + free(wait_sem_dependencies);
> + free(signal_sem_dependencies);
> return r;
> }
>
> -int amdgpu_cs_submit(amdgpu_context_handle context,
> - uint64_t flags,
> - struct amdgpu_cs_request *ibs_request,
> - uint32_t number_of_requests)
> +int amdgpu_cs_submit_sem(amdgpu_context_handle context,
> + uint64_t flags,
> + struct amdgpu_cs_request *ibs_request,
> + struct amdgpu_cs_request_sem *ibs_sem,
> + uint32_t number_of_requests)
> {
> uint32_t i;
> int r;
> -
> + bool has_sems = ibs_sem ? true : false;
> if (NULL == context)
> return -EINVAL;
> if (NULL == ibs_request)
> @@ -337,15 +382,28 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
>
> r = 0;
> for (i = 0; i < number_of_requests; i++) {
> - r = amdgpu_cs_submit_one(context, ibs_request);
> + r = amdgpu_cs_submit_one(context, ibs_request, has_sems ? ibs_sem : NULL);
> if (r)
> break;
> ibs_request++;
> + if (has_sems)
> + ibs_sem++;
> }
>
> return r;
> }
>
> +int amdgpu_cs_submit(amdgpu_context_handle context,
> + uint64_t flags,
> + struct amdgpu_cs_request *ibs_request,
> + uint32_t number_of_requests)
> +{
> + return amdgpu_cs_submit_sem(context, flags,
> + ibs_request, NULL,
> + number_of_requests);
> +}
> +
> +
> /**
> * Calculate absolute timeout.
> *
> @@ -542,3 +600,88 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem)
> {
> return amdgpu_cs_unreference_sem(sem);
> }
> +
> +
> +int amdgpu_cs_create_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle *sem)
> +{
> + union drm_amdgpu_sem args;
> + int r;
> +
> + if (NULL == dev)
> + return -EINVAL;
> +
> + /* Create the context */
> + memset(&args, 0, sizeof(args));
> + args.in.op = AMDGPU_SEM_OP_CREATE_SEM;
> + r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
> + if (r)
> + return r;
> +
> + *sem = args.out.handle;
> +
> + return 0;
> +}
> +
> +int amdgpu_cs_export_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem,
> + int *shared_handle)
> +{
> + union drm_amdgpu_sem args;
> + int r;
> +
> + if (NULL == dev)
> + return -EINVAL;
> +
> + /* Create the context */
> + memset(&args, 0, sizeof(args));
> + args.in.op = AMDGPU_SEM_OP_EXPORT_SEM;
> + args.in.handle = sem;
> + r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
> + if (r)
> + return r;
> + *shared_handle = args.out.fd;
> + return 0;
> +}
> +
> +int amdgpu_cs_import_sem(amdgpu_device_handle dev,
> + int shared_handle,
> + amdgpu_sem_handle *sem)
> +{
> + union drm_amdgpu_sem args;
> + int r;
> +
> + if (NULL == dev)
> + return -EINVAL;
> +
> + /* Create the context */
> + memset(&args, 0, sizeof(args));
> + args.in.op = AMDGPU_SEM_OP_IMPORT_SEM;
> + args.in.handle = shared_handle;
> + r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
> + if (r)
> + return r;
> + *sem = args.out.handle;
> + return 0;
> +}
> +
> +
> +int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
> + amdgpu_sem_handle sem)
> +{
> + union drm_amdgpu_sem args;
> + int r;
> +
> + if (NULL == dev)
> + return -EINVAL;
> +
> + /* Create the context */
> + memset(&args, 0, sizeof(args));
> + args.in.op = AMDGPU_SEM_OP_DESTROY_SEM;
> + args.in.handle = sem;
> + r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
> + if (r)
> + return r;
> +
> + return 0;
> +}
> diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
> index d8f2497..fa0bfe2 100644
> --- a/include/drm/amdgpu_drm.h
> +++ b/include/drm/amdgpu_drm.h
> @@ -50,6 +50,7 @@ extern "C" {
> #define DRM_AMDGPU_WAIT_CS 0x09
> #define DRM_AMDGPU_GEM_OP 0x10
> #define DRM_AMDGPU_GEM_USERPTR 0x11
> +#define DRM_AMDGPU_SEM 0x13
>
> #define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
> #define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
> @@ -63,6 +64,7 @@ extern "C" {
> #define DRM_IOCTL_AMDGPU_WAIT_CS DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_CS, union drm_amdgpu_wait_cs)
> #define DRM_IOCTL_AMDGPU_GEM_OP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op)
> #define DRM_IOCTL_AMDGPU_GEM_USERPTR DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
> +#define DRM_IOCTL_AMDGPU_SEM DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_SEM, union drm_amdgpu_sem)
>
> #define AMDGPU_GEM_DOMAIN_CPU 0x1
> #define AMDGPU_GEM_DOMAIN_GTT 0x2
> @@ -303,6 +305,26 @@ union drm_amdgpu_wait_cs {
> struct drm_amdgpu_wait_cs_out out;
> };
>
> +#define AMDGPU_SEM_OP_CREATE_SEM 0
> +#define AMDGPU_SEM_OP_IMPORT_SEM 1
> +#define AMDGPU_SEM_OP_EXPORT_SEM 2
> +#define AMDGPU_SEM_OP_DESTROY_SEM 3
> +
> +struct drm_amdgpu_sem_in {
> + __u32 op;
> + __u32 handle;
> +};
> +
> +struct drm_amdgpu_sem_out {
> + __u32 fd;
> + __u32 handle;
> +};
> +
> +union drm_amdgpu_sem {
> + struct drm_amdgpu_sem_in in;
> + struct drm_amdgpu_sem_out out;
> +};
> +
> #define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
> #define AMDGPU_GEM_OP_SET_PLACEMENT 1
>
> @@ -358,6 +380,8 @@ struct drm_amdgpu_gem_va {
> #define AMDGPU_CHUNK_ID_IB 0x01
> #define AMDGPU_CHUNK_ID_FENCE 0x02
> #define AMDGPU_CHUNK_ID_DEPENDENCIES 0x03
> +#define AMDGPU_CHUNK_ID_SEM_WAIT 0x04
> +#define AMDGPU_CHUNK_ID_SEM_SIGNAL 0x05
>
> struct drm_amdgpu_cs_chunk {
> uint32_t chunk_id;
> @@ -422,6 +446,10 @@ struct drm_amdgpu_cs_chunk_fence {
> uint32_t offset;
> };
>
> +struct drm_amdgpu_cs_chunk_sem {
> + uint32_t handle;
> +};
> +
> struct drm_amdgpu_cs_chunk_data {
> union {
> struct drm_amdgpu_cs_chunk_ib ib_data;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores
[not found] ` <20170314005054.7487-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-03-14 0:50 ` Dave Airlie
2017-03-14 2:00 ` zhoucm1
[not found] ` <20170314005054.7487-2-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 2 replies; 18+ messages in thread
From: Dave Airlie @ 2017-03-14 0:50 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
From: Dave Airlie <airlied@redhat.com>
This adds the corresponding code for libdrm to use the new
kernel interfaces for semaphores.
This will be used by radv to implement shared semaphores.
TODO: Version checks.
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
amdgpu/amdgpu.h | 28 +++++++++
amdgpu/amdgpu_cs.c | 161 ++++++++++++++++++++++++++++++++++++++++++++---
include/drm/amdgpu_drm.h | 28 +++++++++
3 files changed, 208 insertions(+), 9 deletions(-)
diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
index 7b26a04..747e248 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -129,6 +129,8 @@ typedef struct amdgpu_va *amdgpu_va_handle;
*/
typedef struct amdgpu_semaphore *amdgpu_semaphore_handle;
+typedef uint32_t amdgpu_sem_handle;
+
/*--------------------------------------------------------------------------*/
/* -------------------------- Structures ---------------------------------- */
/*--------------------------------------------------------------------------*/
@@ -365,6 +367,16 @@ struct amdgpu_cs_request {
struct amdgpu_cs_fence_info fence_info;
};
+struct amdgpu_cs_request_sem {
+ /*
+ *
+ */
+ uint32_t number_of_wait_sem;
+ uint32_t *wait_sems;
+ uint32_t number_of_signal_sem;
+ uint32_t *signal_sems;
+};
+
/**
* Structure which provide information about GPU VM MC Address space
* alignments requirements
@@ -882,6 +894,12 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
struct amdgpu_cs_request *ibs_request,
uint32_t number_of_requests);
+int amdgpu_cs_submit_sem(amdgpu_context_handle context,
+ uint64_t flags,
+ struct amdgpu_cs_request *ibs_request,
+ struct amdgpu_cs_request_sem *ibs_sem,
+ uint32_t number_of_requests);
+
/**
* Query status of Command Buffer Submission
*
@@ -1255,4 +1273,14 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem);
*/
const char *amdgpu_get_marketing_name(amdgpu_device_handle dev);
+int amdgpu_cs_create_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle *sem);
+int amdgpu_cs_export_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle sem,
+ int *shared_handle);
+int amdgpu_cs_import_sem(amdgpu_device_handle dev,
+ int shared_handle,
+ amdgpu_sem_handle *sem);
+int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle sem);
#endif /* #ifdef _AMDGPU_H_ */
diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
index fb5b3a8..7283327 100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -170,7 +170,8 @@ int amdgpu_cs_query_reset_state(amdgpu_context_handle context,
* \sa amdgpu_cs_submit()
*/
static int amdgpu_cs_submit_one(amdgpu_context_handle context,
- struct amdgpu_cs_request *ibs_request)
+ struct amdgpu_cs_request *ibs_request,
+ struct amdgpu_cs_request_sem *sem_request)
{
union drm_amdgpu_cs cs;
uint64_t *chunk_array;
@@ -178,9 +179,11 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
struct drm_amdgpu_cs_chunk_data *chunk_data;
struct drm_amdgpu_cs_chunk_dep *dependencies = NULL;
struct drm_amdgpu_cs_chunk_dep *sem_dependencies = NULL;
+ struct drm_amdgpu_cs_chunk_sem *wait_sem_dependencies = NULL;
+ struct drm_amdgpu_cs_chunk_sem *signal_sem_dependencies = NULL;
struct list_head *sem_list;
amdgpu_semaphore_handle sem, tmp;
- uint32_t i, size, sem_count = 0;
+ uint32_t i, j, size, sem_count = 0;
bool user_fence;
int r = 0;
@@ -196,7 +199,7 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
}
user_fence = (ibs_request->fence_info.handle != NULL);
- size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1;
+ size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1 + (sem_request ? 2 : 0);
chunk_array = alloca(sizeof(uint64_t) * size);
chunks = alloca(sizeof(struct drm_amdgpu_cs_chunk) * size);
@@ -308,6 +311,45 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context,
chunks[i].chunk_data = (uint64_t)(uintptr_t)sem_dependencies;
}
+ if (sem_request) {
+ if (sem_request->number_of_wait_sem) {
+ wait_sem_dependencies = malloc(sizeof(struct drm_amdgpu_cs_chunk_sem) * sem_request->number_of_wait_sem);
+ if (!wait_sem_dependencies) {
+ r = -ENOMEM;
+ goto error_unlock;
+ }
+ for (j = 0; j < sem_request->number_of_wait_sem; j++) {
+ struct drm_amdgpu_cs_chunk_sem *dep = &wait_sem_dependencies[j];
+ dep->handle = sem_request->wait_sems[j];
+ }
+ i = cs.in.num_chunks++;
+
+ /* dependencies chunk */
+ chunk_array[i] = (uint64_t)(uintptr_t)&chunks[i];
+ chunks[i].chunk_id = AMDGPU_CHUNK_ID_SEM_WAIT;
+ chunks[i].length_dw = sizeof(struct drm_amdgpu_cs_chunk_sem) / 4 * sem_request->number_of_wait_sem;
+ chunks[i].chunk_data = (uint64_t)(uintptr_t)wait_sem_dependencies;
+ }
+ if (sem_request->number_of_signal_sem) {
+ signal_sem_dependencies = malloc(sizeof(struct drm_amdgpu_cs_chunk_sem) * sem_request->number_of_signal_sem);
+ if (!signal_sem_dependencies) {
+ r = -ENOMEM;
+ goto error_unlock;
+ }
+ for (j = 0; j < sem_request->number_of_signal_sem; j++) {
+ struct drm_amdgpu_cs_chunk_sem *dep = &signal_sem_dependencies[j];
+ dep->handle = sem_request->signal_sems[j];
+ }
+ i = cs.in.num_chunks++;
+
+ /* dependencies chunk */
+ chunk_array[i] = (uint64_t)(uintptr_t)&chunks[i];
+ chunks[i].chunk_id = AMDGPU_CHUNK_ID_SEM_SIGNAL;
+ chunks[i].length_dw = sizeof(struct drm_amdgpu_cs_chunk_sem) / 4 * sem_request->number_of_signal_sem;
+ chunks[i].chunk_data = (uint64_t)(uintptr_t)signal_sem_dependencies;
+ }
+ }
+
r = drmCommandWriteRead(context->dev->fd, DRM_AMDGPU_CS,
&cs, sizeof(cs));
if (r)
@@ -319,17 +361,20 @@ error_unlock:
pthread_mutex_unlock(&context->sequence_mutex);
free(dependencies);
free(sem_dependencies);
+ free(wait_sem_dependencies);
+ free(signal_sem_dependencies);
return r;
}
-int amdgpu_cs_submit(amdgpu_context_handle context,
- uint64_t flags,
- struct amdgpu_cs_request *ibs_request,
- uint32_t number_of_requests)
+int amdgpu_cs_submit_sem(amdgpu_context_handle context,
+ uint64_t flags,
+ struct amdgpu_cs_request *ibs_request,
+ struct amdgpu_cs_request_sem *ibs_sem,
+ uint32_t number_of_requests)
{
uint32_t i;
int r;
-
+ bool has_sems = ibs_sem ? true : false;
if (NULL == context)
return -EINVAL;
if (NULL == ibs_request)
@@ -337,15 +382,28 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
r = 0;
for (i = 0; i < number_of_requests; i++) {
- r = amdgpu_cs_submit_one(context, ibs_request);
+ r = amdgpu_cs_submit_one(context, ibs_request, has_sems ? ibs_sem : NULL);
if (r)
break;
ibs_request++;
+ if (has_sems)
+ ibs_sem++;
}
return r;
}
+int amdgpu_cs_submit(amdgpu_context_handle context,
+ uint64_t flags,
+ struct amdgpu_cs_request *ibs_request,
+ uint32_t number_of_requests)
+{
+ return amdgpu_cs_submit_sem(context, flags,
+ ibs_request, NULL,
+ number_of_requests);
+}
+
+
/**
* Calculate absolute timeout.
*
@@ -542,3 +600,88 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem)
{
return amdgpu_cs_unreference_sem(sem);
}
+
+
+int amdgpu_cs_create_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle *sem)
+{
+ union drm_amdgpu_sem args;
+ int r;
+
+ if (NULL == dev)
+ return -EINVAL;
+
+ /* Create the context */
+ memset(&args, 0, sizeof(args));
+ args.in.op = AMDGPU_SEM_OP_CREATE_SEM;
+ r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
+ if (r)
+ return r;
+
+ *sem = args.out.handle;
+
+ return 0;
+}
+
+int amdgpu_cs_export_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle sem,
+ int *shared_handle)
+{
+ union drm_amdgpu_sem args;
+ int r;
+
+ if (NULL == dev)
+ return -EINVAL;
+
+ /* Create the context */
+ memset(&args, 0, sizeof(args));
+ args.in.op = AMDGPU_SEM_OP_EXPORT_SEM;
+ args.in.handle = sem;
+ r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
+ if (r)
+ return r;
+ *shared_handle = args.out.fd;
+ return 0;
+}
+
+int amdgpu_cs_import_sem(amdgpu_device_handle dev,
+ int shared_handle,
+ amdgpu_sem_handle *sem)
+{
+ union drm_amdgpu_sem args;
+ int r;
+
+ if (NULL == dev)
+ return -EINVAL;
+
+ /* Create the context */
+ memset(&args, 0, sizeof(args));
+ args.in.op = AMDGPU_SEM_OP_IMPORT_SEM;
+ args.in.handle = shared_handle;
+ r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
+ if (r)
+ return r;
+ *sem = args.out.handle;
+ return 0;
+}
+
+
+int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
+ amdgpu_sem_handle sem)
+{
+ union drm_amdgpu_sem args;
+ int r;
+
+ if (NULL == dev)
+ return -EINVAL;
+
+ /* Create the context */
+ memset(&args, 0, sizeof(args));
+ args.in.op = AMDGPU_SEM_OP_DESTROY_SEM;
+ args.in.handle = sem;
+ r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_SEM, &args, sizeof(args));
+ if (r)
+ return r;
+
+ return 0;
+}
diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
index d8f2497..fa0bfe2 100644
--- a/include/drm/amdgpu_drm.h
+++ b/include/drm/amdgpu_drm.h
@@ -50,6 +50,7 @@ extern "C" {
#define DRM_AMDGPU_WAIT_CS 0x09
#define DRM_AMDGPU_GEM_OP 0x10
#define DRM_AMDGPU_GEM_USERPTR 0x11
+#define DRM_AMDGPU_SEM 0x13
#define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
#define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
@@ -63,6 +64,7 @@ extern "C" {
#define DRM_IOCTL_AMDGPU_WAIT_CS DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_CS, union drm_amdgpu_wait_cs)
#define DRM_IOCTL_AMDGPU_GEM_OP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op)
#define DRM_IOCTL_AMDGPU_GEM_USERPTR DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
+#define DRM_IOCTL_AMDGPU_SEM DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_SEM, union drm_amdgpu_sem)
#define AMDGPU_GEM_DOMAIN_CPU 0x1
#define AMDGPU_GEM_DOMAIN_GTT 0x2
@@ -303,6 +305,26 @@ union drm_amdgpu_wait_cs {
struct drm_amdgpu_wait_cs_out out;
};
+#define AMDGPU_SEM_OP_CREATE_SEM 0
+#define AMDGPU_SEM_OP_IMPORT_SEM 1
+#define AMDGPU_SEM_OP_EXPORT_SEM 2
+#define AMDGPU_SEM_OP_DESTROY_SEM 3
+
+struct drm_amdgpu_sem_in {
+ __u32 op;
+ __u32 handle;
+};
+
+struct drm_amdgpu_sem_out {
+ __u32 fd;
+ __u32 handle;
+};
+
+union drm_amdgpu_sem {
+ struct drm_amdgpu_sem_in in;
+ struct drm_amdgpu_sem_out out;
+};
+
#define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
#define AMDGPU_GEM_OP_SET_PLACEMENT 1
@@ -358,6 +380,8 @@ struct drm_amdgpu_gem_va {
#define AMDGPU_CHUNK_ID_IB 0x01
#define AMDGPU_CHUNK_ID_FENCE 0x02
#define AMDGPU_CHUNK_ID_DEPENDENCIES 0x03
+#define AMDGPU_CHUNK_ID_SEM_WAIT 0x04
+#define AMDGPU_CHUNK_ID_SEM_SIGNAL 0x05
struct drm_amdgpu_cs_chunk {
uint32_t chunk_id;
@@ -422,6 +446,10 @@ struct drm_amdgpu_cs_chunk_fence {
uint32_t offset;
};
+struct drm_amdgpu_cs_chunk_sem {
+ uint32_t handle;
+};
+
struct drm_amdgpu_cs_chunk_data {
union {
struct drm_amdgpu_cs_chunk_ib ib_data;
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-06-27 2:59 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-26 21:19 [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores Dave Airlie
[not found] ` <20170626211922.7493-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-06-27 2:59 ` Dave Airlie
-- strict thread matches above, loose matches on Subject: below --
2017-03-14 0:50 [rfc] amdgpu/sync_file shared semaphores Dave Airlie
[not found] ` <20170314005054.7487-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-03-14 0:50 ` [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores Dave Airlie
2017-03-14 2:00 ` zhoucm1
2017-03-14 2:52 ` Dave Airlie
[not found] ` <CAPM=9tzegdtiyEmbW+PqmVeQ+bXrNtrdKXuE3kB0dPMKnMir+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-14 3:30 ` zhoucm1
[not found] ` <58C763E0.1-5C7GfCeVMHo@public.gmane.org>
2017-03-14 4:16 ` Dave Airlie
2017-03-14 8:56 ` Daniel Vetter
2017-03-14 8:54 ` Daniel Vetter
2017-03-14 9:20 ` Christian König
2017-03-14 10:04 ` zhoucm1
[not found] ` <58C7C032.7060706-5C7GfCeVMHo@public.gmane.org>
2017-03-14 17:45 ` Harry Wentland
[not found] ` <8dea3303-480d-c29a-22ec-42adf3dab4ce-5C7GfCeVMHo@public.gmane.org>
2017-03-14 17:54 ` Daniel Vetter
2017-03-14 18:10 ` Christian König
2017-03-15 0:01 ` Marek Olšák
[not found] ` <CAAxE2A7xRWhkp-OC59iy2i91uj5mtVTHR0uHfDf745L-ixxHcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-15 8:48 ` Daniel Vetter
2017-03-15 9:35 ` Christian König
[not found] ` <20170314005054.7487-2-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-03-15 10:30 ` Emil Velikov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.