[PATCH 0/5] ***[WIP] introduce syncfile as fence reuturn ***

[PATCH 0/5] ***[WIP] introduce syncfile as fence reuturn ***

[Chunming Zhou]

*** BLURB HERE ***

Chunming Zhou (5):
  drm/amdgpu: introduce sync file for CS returning fence
  drm/amdgpu: add syncfile chunk support
  drm/amdgpu: add syncfile support to wait ioctl
  drm/amdgpu: add syncfile fence support to wait_any/all ioctl
  drm/amdgpu: expand mapping ioctl to return fence to UMD by using
    syncfile

 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 90 ++++++++++++++++++++++++---------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 24 +++++++--
 include/uapi/drm/amdgpu_drm.h           | 13 ++++-
 3 files changed, 96 insertions(+), 31 deletions(-)

-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/5] drm/amdgpu: introduce sync file for CS returning fence

[Chunming Zhou]

Change-Id: If32e17d8fbdf1e158a34eca3de8687a8fc30f797
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 11 +++++++++++
 include/uapi/drm/amdgpu_drm.h          |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index be01199..3b049a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -25,6 +25,7 @@
  *    Jerome Glisse <glisse@freedesktop.org>
  */
 #include <linux/pagemap.h>
+#include <linux/sync_file.h>
 #include <drm/drmP.h>
 #include <drm/amdgpu_drm.h>
 #include "amdgpu.h"
@@ -1043,6 +1044,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	struct amdgpu_ring *ring = p->job->ring;
 	struct amd_sched_entity *entity = &p->ctx->rings[ring->idx].entity;
 	struct amdgpu_job *job;
+	int fd;
+	struct sync_file *sync_file;
 	int r;
 
 	job = p->job;
@@ -1057,6 +1060,14 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	job->owner = p->filp;
 	job->fence_ctx = entity->fence_context;
 	p->fence = dma_fence_get(&job->base.s_fence->finished);
+	sync_file = sync_file_create(p->fence);
+	if (!sync_file) {
+		amdgpu_job_free(job);
+		return -ENOMEM;
+	}
+	fd = get_unused_fd_flags(O_CLOEXEC);
+	fd_install(fd, sync_file->file);
+	cs->out.fence_fd = fd;
 	cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, p->fence);
 	job->uf_sequence = cs->out.handle;
 	amdgpu_job_free_resources(job);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 016f7be..d0cd4f0 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -530,6 +530,8 @@ struct drm_amdgpu_cs_in {
 };
 
 struct drm_amdgpu_cs_out {
+	int32_t fence_fd;
+	int32_t _pad;
 	__u64 handle;
 };
 
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/5] drm/amdgpu: add syncfile chunk support

[Chunming Zhou]

Change-Id: I9a9e8d474c2f31e71bdf9841a518fa48b3e5c33b
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 63 +++++++++++++++++++++-------------
 include/uapi/drm/amdgpu_drm.h          |  3 ++
 2 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 3b049a4..d048802 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -155,6 +155,7 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)
 			break;
 
 		case AMDGPU_CHUNK_ID_DEPENDENCIES:
+		case AMDGPU_CHUNK_ID_SYNCFILE:
 			break;
 
 		default:
@@ -983,34 +984,47 @@ static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p,
 		sizeof(struct drm_amdgpu_cs_chunk_dep);
 
 	for (i = 0; i < num_deps; ++i) {
-		struct amdgpu_ring *ring;
-		struct amdgpu_ctx *ctx;
-		struct dma_fence *fence;
+		if (chunk->chunk_id == AMDGPU_CHUNK_ID_DEPENDENCIES) {
+			struct amdgpu_ring *ring;
+			struct amdgpu_ctx *ctx;
+			struct dma_fence *fence;
 
-		ctx = amdgpu_ctx_get(fpriv, deps[i].ctx_id);
-		if (ctx == NULL)
-			return -EINVAL;
+			ctx = amdgpu_ctx_get(fpriv, deps[i].ctx_id);
+			if (ctx == NULL)
+				return -EINVAL;
 
-		r = amdgpu_queue_mgr_map(p->adev, &ctx->queue_mgr,
-					 deps[i].ip_type,
-					 deps[i].ip_instance,
-					 deps[i].ring, &ring);
-		if (r) {
-			amdgpu_ctx_put(ctx);
-			return r;
-		}
+			r = amdgpu_queue_mgr_map(p->adev, &ctx->queue_mgr,
+						deps[i].ip_type,
+						deps[i].ip_instance,
+						deps[i].ring, &ring);
+			if (r) {
+				amdgpu_ctx_put(ctx);
+				return r;
+			}
 
-		fence = amdgpu_ctx_get_fence(ctx, ring,
-					     deps[i].handle);
-		if (IS_ERR(fence)) {
-			r = PTR_ERR(fence);
-			amdgpu_ctx_put(ctx);
-			return r;
-		} else if (fence) {
+			fence = amdgpu_ctx_get_fence(ctx, ring,
+						deps[i].handle);
+			if (IS_ERR(fence)) {
+				r = PTR_ERR(fence);
+				amdgpu_ctx_put(ctx);
+				return r;
+			} else if (fence) {
+				r = amdgpu_sync_fence(p->adev, &p->job->sync,
+							fence);
+				dma_fence_put(fence);
+				amdgpu_ctx_put(ctx);
+				if (r)
+					return r;
+			}
+		} else if (chunk->chunk_id == AMDGPU_CHUNK_ID_SYNCFILE) {
+			struct dma_fence *fence;
+
+			WARN_ONCE(deps[i].sf_fd < 0,
+				  "invalid syncfile fd handle!");
+			fence = sync_file_get_fence(deps[i].sf_fd);
 			r = amdgpu_sync_fence(p->adev, &p->job->sync,
-					      fence);
+						fence);
 			dma_fence_put(fence);
-			amdgpu_ctx_put(ctx);
 			if (r)
 				return r;
 		}
@@ -1028,7 +1042,8 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
 
 		chunk = &p->chunks[i];
 
-		if (chunk->chunk_id == AMDGPU_CHUNK_ID_DEPENDENCIES) {
+		if ((chunk->chunk_id == AMDGPU_CHUNK_ID_DEPENDENCIES) ||
+			(chunk->chunk_id == AMDGPU_CHUNK_ID_SYNCFILE)) {
 			r = amdgpu_cs_process_fence_dep(p, chunk);
 			if (r)
 				return r;
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index d0cd4f0..19b4fea 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -511,6 +511,7 @@ 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_SYNCFILE	0x04
 
 struct drm_amdgpu_cs_chunk {
 	__u32		chunk_id;
@@ -573,6 +574,8 @@ struct drm_amdgpu_cs_chunk_dep {
 	__u32 ring;
 	__u32 ctx_id;
 	__u64 handle;
+	int32_t sf_fd;
+	int32_t _pad;
 };
 
 struct drm_amdgpu_cs_chunk_fence {
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/5] drm/amdgpu: add syncfile support to wait ioctl

[Chunming Zhou]

Change-Id: I79155cef5ee798899cd51a88825512cbf0d8ad82
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 10 +++++++++-
 include/uapi/drm/amdgpu_drm.h          |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d048802..6c0ea9a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1178,6 +1178,14 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data,
 	if (amdgpu_kms_vram_lost(adev, fpriv))
 		return -ENODEV;
 
+	if (wait->in.sf_fd > 0) {
+		struct dma_fence *fence = sync_file_get_fence(wait->in.sf_fd);
+
+		WARN_ONCE(fence, "get fence failed from syncfile fd!");
+		r = kcl_fence_wait_timeout(fence, true, timeout);
+		dma_fence_put(fence);
+		goto out;
+	}
 	ctx = amdgpu_ctx_get(filp->driver_priv, wait->in.ctx_id);
 	if (ctx == NULL)
 		return -EINVAL;
@@ -1202,7 +1210,7 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data,
 	amdgpu_ctx_put(ctx);
 	if (r < 0)
 		return r;
-
+out:
 	memset(wait, 0, sizeof(*wait));
 	wait->out.status = (r == 0);
 
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 19b4fea..ce110d9 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -399,6 +399,8 @@ struct drm_amdgpu_wait_cs_in {
 	__u32 ip_instance;
 	__u32 ring;
 	__u32 ctx_id;
+	int32_t sf_fd;
+	int32_t _pad;
 };
 
 struct drm_amdgpu_wait_cs_out {
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/5] drm/amdgpu: add syncfile fence support to wait_any/all ioctl

[Chunming Zhou]

Change-Id: Ibe77f213560846ec12901e366129d9368750b7ba
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 ++++++
 include/uapi/drm/amdgpu_drm.h          | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 6c0ea9a..a2fbdaa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1233,6 +1233,12 @@ static struct dma_fence *amdgpu_cs_get_fence(struct amdgpu_device *adev,
 	struct dma_fence *fence;
 	int r;
 
+	if (user->sf_fd > 0) {
+		struct dma_fence *fence = sync_file_get_fence(user->sf_fd);
+
+		WARN_ONCE(fence, "get fence failed from syncfile fd!");
+		return fence;
+	}
 	ctx = amdgpu_ctx_get(filp->driver_priv, user->ctx_id);
 	if (ctx == NULL)
 		return ERR_PTR(-EINVAL);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index ce110d9..cf3fb62 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -419,6 +419,8 @@ struct drm_amdgpu_fence {
 	__u32 ip_instance;
 	__u32 ring;
 	__u64 seq_no;
+	int32_t sf_fd;
+	int32_t _pad;
 };
 
 struct drm_amdgpu_wait_fences_in {
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/5] drm/amdgpu: expand mapping ioctl to return fence to UMD by using syncfile

[Chunming Zhou]

Change-Id: I88e56d34ec5e90fe8001e73d52e893a38938361e
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 24 +++++++++++++++++++-----
 include/uapi/drm/amdgpu_drm.h           |  4 +++-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 973c0f0..bf37279 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -27,6 +27,7 @@
  */
 #include <linux/ktime.h>
 #include <linux/pagemap.h>
+#include <linux/sync_file.h>
 #include <drm/drmP.h>
 #include <drm/amdgpu_drm.h>
 #include "amdgpu.h"
@@ -693,7 +694,8 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 				    struct amdgpu_vm *vm,
 				    struct amdgpu_bo_va *bo_va,
 				    struct list_head *list,
-				    uint32_t operation)
+				    uint32_t operation,
+				    int *sf_fd)
 {
 	int r;
 
@@ -709,9 +711,18 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 		goto error;
 
 	if (operation == AMDGPU_VA_OP_MAP ||
-	    operation == AMDGPU_VA_OP_REPLACE)
-		r = amdgpu_vm_bo_update(adev, bo_va, false);
+	    operation == AMDGPU_VA_OP_REPLACE) {
+		int fd;
+		struct sync_file *sync_file;
 
+		r = amdgpu_vm_bo_update(adev, bo_va, false);
+		sync_file = sync_file_create(bo_va->last_pt_update);
+		if (!sync_file)
+			goto error;
+		fd = get_unused_fd_flags(O_CLOEXEC);
+		fd_install(fd, sync_file->file);
+		*sf_fd = fd;
+	}
 error:
 	if (r && r != -ERESTARTSYS)
 		DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
@@ -737,6 +748,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 	struct ww_acquire_ctx ticket;
 	struct list_head list, duplicates;
 	uint64_t va_flags;
+	int sf_fd;
 	int r = 0;
 
 	if (args->va_address < AMDGPU_VA_RESERVED_SIZE) {
@@ -839,9 +851,11 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 	default:
 		break;
 	}
-	if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) && !amdgpu_vm_debug)
+	if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) && !amdgpu_vm_debug) {
 		amdgpu_gem_va_update_vm(adev, &fpriv->vm, bo_va, &list,
-					args->operation);
+					args->operation, &sf_fd);
+		args->sf_fd = sf_fd;
+	}
 
 error_backoff:
 	ttm_eu_backoff_reservation(&ticket, &list);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index cf3fb62..6b97d33 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -487,7 +487,9 @@ struct drm_amdgpu_gem_op {
 struct drm_amdgpu_gem_va {
 	/** GEM object handle */
 	__u32 handle;
-	__u32 _pad;
+	/** sf_fd is an OUTPUT from syncfile, it's present to mapping completion
+	 */
+	int32_t sf_fd;
 	/** AMDGPU_VA_OP_* */
 	__u32 operation;
 	/** AMDGPU_VM_PAGE_* */
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/5] ***[WIP] introduce syncfile as fence reuturn ***

[Christian König]

NAK for the CS part (but I think that was just as prove of concept anyway).

Dave's sync object implementation is already done for this, the problem 
is that isn't merged yet into amd-staging-4.12 (but should be in 4-13).

The VM part looks similar to what I had in mind, but we should base this 
on Dave's sync_obj stuff as well.

Regards,
Christian.

Am 12.09.2017 um 12:23 schrieb Chunming Zhou:
> *** BLURB HERE ***
>
> Chunming Zhou (5):
>    drm/amdgpu: introduce sync file for CS returning fence
>    drm/amdgpu: add syncfile chunk support
>    drm/amdgpu: add syncfile support to wait ioctl
>    drm/amdgpu: add syncfile fence support to wait_any/all ioctl
>    drm/amdgpu: expand mapping ioctl to return fence to UMD by using
>      syncfile
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 90 ++++++++++++++++++++++++---------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 24 +++++++--
>   include/uapi/drm/amdgpu_drm.h           | 13 ++++-
>   3 files changed, 96 insertions(+), 31 deletions(-)
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/5] ***[WIP] introduce syncfile as fence reuturn ***

[zhoucm1]

On 2017年09月12日 19:00, Christian König wrote:
> NAK for the CS part (but I think that was just as prove of concept 
> anyway).
>
> Dave's sync object implementation is already done for this, the 
> problem is that isn't merged yet into amd-staging-4.12 (but should be 
> in 4-13).
Yeah, I already realized it before, if only vm part, I agree syncobj can 
satisfy the synchronization between SDMA and other engine.
But from your old dependency perspective, we also should cover 
wait_ioctl and wait_any/all_ioctl, which should also can wait fence from 
syncfile, so it makes sense that cs_ioctl and va_ioctl return syncfile 
fd to UMD.
Compared with only retuning fence seq number, UMD can be convenience to 
pass fd to any where it needs to sync and as dependency or to semaphore, 
and they don't need to construct ip_type/ip_instance/ctx_id/ring any 
more, even the caching fences in ctx becomes no meanless if don't 
consider compatibility.

Regards,
David Zhou

>
> The VM part looks similar to what I had in mind, but we should base 
> this on Dave's sync_obj stuff as well.
>
> Regards,
> Christian.
>
> Am 12.09.2017 um 12:23 schrieb Chunming Zhou:
>> *** BLURB HERE ***
>>
>> Chunming Zhou (5):
>>    drm/amdgpu: introduce sync file for CS returning fence
>>    drm/amdgpu: add syncfile chunk support
>>    drm/amdgpu: add syncfile support to wait ioctl
>>    drm/amdgpu: add syncfile fence support to wait_any/all ioctl
>>    drm/amdgpu: expand mapping ioctl to return fence to UMD by using
>>      syncfile
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 90 
>> ++++++++++++++++++++++++---------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 24 +++++++--
>>   include/uapi/drm/amdgpu_drm.h           | 13 ++++-
>>   3 files changed, 96 insertions(+), 31 deletions(-)
>>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx