[PATCH 1/3] drm/amd: fix init order of sched job

[PATCH 1/3] drm/amd: fix init order of sched job

[Chunming Zhou]

Change-Id: Iced391f5c24a79ad7aecae33e22ff089f68f1337
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 80bc4f7..3c258c3 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -460,9 +460,9 @@ int amd_sched_job_init(struct amd_sched_job *job,
 	job->sched = sched;
 	job->s_entity = entity;
 	job->s_fence = amd_sched_fence_create(entity, owner);
-	job->id = atomic64_inc_return(&sched->job_id_count);
 	if (!job->s_fence)
 		return -ENOMEM;
+	job->id = atomic64_inc_return(&sched->job_id_count);
 
 	INIT_WORK(&job->finish_work, amd_sched_job_finish);
 	INIT_LIST_HEAD(&job->node);
-- 
1.9.1

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

[PATCH 2/3] drm/amdgpu: fix dependency issue

[Chunming Zhou]

The problem is that executing the jobs in the right order doesn't give you the right result
because consecutive jobs executed on the same engine are pipelined.
In other words job B does it buffer read before job A has written it's result.

Change-Id: I9065abf5bafbda7a92702ca11477315d25c9a347
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c        |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |  4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  2 +-
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 17 +++++++++++++++++
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
 6 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index dd2bd9a..787acd7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1169,6 +1169,7 @@ struct amdgpu_job {
 	void			*owner;
 	uint64_t		fence_ctx; /* the fence_context this job uses */
 	bool                    vm_needs_flush;
+	bool			need_pipeline_sync;
 	unsigned		vm_id;
 	uint64_t		vm_pd_addr;
 	uint32_t		gds_base, gds_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 06202e2..2c6624d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -167,6 +167,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 		return r;
 	}
 
+	if (ring->funcs->emit_pipeline_sync && job && job->need_pipeline_sync)
+		amdgpu_ring_emit_pipeline_sync(ring);
 	if (vm) {
 		amdgpu_ring_insert_nop(ring, extra_nop); /* prevent CE go too fast than DE */
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 690ef3d..cfa97ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -57,6 +57,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
 	(*job)->vm = vm;
 	(*job)->ibs = (void *)&(*job)[1];
 	(*job)->num_ibs = num_ibs;
+	(*job)->need_pipeline_sync = false;
 
 	amdgpu_sync_create(&(*job)->sync);
 
@@ -152,6 +153,9 @@ static struct fence *amdgpu_job_dependency(struct amd_sched_job *sched_job)
 		fence = amdgpu_sync_get_fence(&job->sync);
 	}
 
+	if (amd_sched_dependency_optimized(fence, sched_job->s_entity))
+		job->need_pipeline_sync = true;
+
 	return fence;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index b35b853..b4f83fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -738,7 +738,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job)
 	if (ring->funcs->init_cond_exec)
 		patch_offset = amdgpu_ring_init_cond_exec(ring);
 
-	if (ring->funcs->emit_pipeline_sync)
+	if (ring->funcs->emit_pipeline_sync && !job->need_pipeline_sync)
 		amdgpu_ring_emit_pipeline_sync(ring);
 
 	if (ring->funcs->emit_vm_flush && vm_flush_needed) {
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 3c258c3..fec870a 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -235,6 +235,23 @@ static void amd_sched_entity_clear_dep(struct fence *f, struct fence_cb *cb)
 	fence_put(f);
 }
 
+bool amd_sched_dependency_optimized(struct fence* fence,
+				    struct amd_sched_entity *entity)
+{
+	struct amd_gpu_scheduler *sched = entity->sched;
+	struct amd_sched_fence *s_fence;
+
+	if (!fence || fence_is_signaled(fence))
+		return false;
+	if (fence->context == entity->fence_context)
+		return true;
+	s_fence = to_amd_sched_fence(fence);
+	if (s_fence && s_fence->sched == sched)
+		return true;
+
+	return false;
+}
+
 static bool amd_sched_entity_add_dependency_cb(struct amd_sched_entity *entity)
 {
 	struct amd_gpu_scheduler *sched = entity->sched;
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 2531b41..bfda90f 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -162,4 +162,6 @@ int amd_sched_job_init(struct amd_sched_job *job,
 		       void *owner);
 void amd_sched_hw_job_reset(struct amd_gpu_scheduler *sched);
 void amd_sched_job_recovery(struct amd_gpu_scheduler *sched);
+bool amd_sched_dependency_optimized(struct fence* fence,
+				    struct amd_sched_entity *entity);
 #endif
-- 
1.9.1

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

Re: [PATCH 2/3] drm/amdgpu: fix dependency issue

[Zhang, Jerry (Junwei)]

On 05/09/2017 04:14 PM, Chunming Zhou wrote:
> The problem is that executing the jobs in the right order doesn't give you the right result
> because consecutive jobs executed on the same engine are pipelined.
> In other words job B does it buffer read before job A has written it's result.
>
> Change-Id: I9065abf5bafbda7a92702ca11477315d25c9a347
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c        |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |  4 ++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  2 +-
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 17 +++++++++++++++++
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>   6 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index dd2bd9a..787acd7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1169,6 +1169,7 @@ struct amdgpu_job {
>   	void			*owner;
>   	uint64_t		fence_ctx; /* the fence_context this job uses */
>   	bool                    vm_needs_flush;
> +	bool			need_pipeline_sync;
>   	unsigned		vm_id;
>   	uint64_t		vm_pd_addr;
>   	uint32_t		gds_base, gds_size;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 06202e2..2c6624d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -167,6 +167,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>   		return r;
>   	}
>
> +	if (ring->funcs->emit_pipeline_sync && job && job->need_pipeline_sync)
> +		amdgpu_ring_emit_pipeline_sync(ring);
>   	if (vm) {
>   		amdgpu_ring_insert_nop(ring, extra_nop); /* prevent CE go too fast than DE */
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 690ef3d..cfa97ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -57,6 +57,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>   	(*job)->vm = vm;
>   	(*job)->ibs = (void *)&(*job)[1];
>   	(*job)->num_ibs = num_ibs;
> +	(*job)->need_pipeline_sync = false;
>
>   	amdgpu_sync_create(&(*job)->sync);
>
> @@ -152,6 +153,9 @@ static struct fence *amdgpu_job_dependency(struct amd_sched_job *sched_job)
>   		fence = amdgpu_sync_get_fence(&job->sync);
>   	}
>
> +	if (amd_sched_dependency_optimized(fence, sched_job->s_entity))
> +		job->need_pipeline_sync = true;
> +
>   	return fence;
>   }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index b35b853..b4f83fc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -738,7 +738,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job)
>   	if (ring->funcs->init_cond_exec)
>   		patch_offset = amdgpu_ring_init_cond_exec(ring);
>
> -	if (ring->funcs->emit_pipeline_sync)
> +	if (ring->funcs->emit_pipeline_sync && !job->need_pipeline_sync)

Perhaps we could emit pipeline sync regardless of the flag need_pipeline_sync.
Could you explain the reason about this change?

Jerry

>   		amdgpu_ring_emit_pipeline_sync(ring);
>
>   	if (ring->funcs->emit_vm_flush && vm_flush_needed) {
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index 3c258c3..fec870a 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -235,6 +235,23 @@ static void amd_sched_entity_clear_dep(struct fence *f, struct fence_cb *cb)
>   	fence_put(f);
>   }
>
> +bool amd_sched_dependency_optimized(struct fence* fence,
> +				    struct amd_sched_entity *entity)
> +{
> +	struct amd_gpu_scheduler *sched = entity->sched;
> +	struct amd_sched_fence *s_fence;
> +
> +	if (!fence || fence_is_signaled(fence))
> +		return false;
> +	if (fence->context == entity->fence_context)
> +		return true;
> +	s_fence = to_amd_sched_fence(fence);
> +	if (s_fence && s_fence->sched == sched)
> +		return true;
> +
> +	return false;
> +}
> +
>   static bool amd_sched_entity_add_dependency_cb(struct amd_sched_entity *entity)
>   {
>   	struct amd_gpu_scheduler *sched = entity->sched;
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> index 2531b41..bfda90f 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> @@ -162,4 +162,6 @@ int amd_sched_job_init(struct amd_sched_job *job,
>   		       void *owner);
>   void amd_sched_hw_job_reset(struct amd_gpu_scheduler *sched);
>   void amd_sched_job_recovery(struct amd_gpu_scheduler *sched);
> +bool amd_sched_dependency_optimized(struct fence* fence,
> +				    struct amd_sched_entity *entity);
>   #endif
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/amdgpu: fix dependency issue

[Christian König]

Am 09.05.2017 um 10:14 schrieb Chunming Zhou:
> The problem is that executing the jobs in the right order doesn't give you the right result
> because consecutive jobs executed on the same engine are pipelined.
> In other words job B does it buffer read before job A has written it's result.
>
> Change-Id: I9065abf5bafbda7a92702ca11477315d25c9a347
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c        |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |  4 ++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  2 +-
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 17 +++++++++++++++++
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>   6 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index dd2bd9a..787acd7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1169,6 +1169,7 @@ struct amdgpu_job {
>   	void			*owner;
>   	uint64_t		fence_ctx; /* the fence_context this job uses */
>   	bool                    vm_needs_flush;
> +	bool			need_pipeline_sync;
>   	unsigned		vm_id;
>   	uint64_t		vm_pd_addr;
>   	uint32_t		gds_base, gds_size;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 06202e2..2c6624d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -167,6 +167,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>   		return r;
>   	}
>   
> +	if (ring->funcs->emit_pipeline_sync && job && job->need_pipeline_sync)
> +		amdgpu_ring_emit_pipeline_sync(ring);

We should probably clean that handling up at some point, but for now it 
should work.

Patch is Reviewed-by: Christian König <christian.koenig@amd.com>.

>   	if (vm) {
>   		amdgpu_ring_insert_nop(ring, extra_nop); /* prevent CE go too fast than DE */
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 690ef3d..cfa97ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -57,6 +57,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>   	(*job)->vm = vm;
>   	(*job)->ibs = (void *)&(*job)[1];
>   	(*job)->num_ibs = num_ibs;
> +	(*job)->need_pipeline_sync = false;
>   
>   	amdgpu_sync_create(&(*job)->sync);
>   
> @@ -152,6 +153,9 @@ static struct fence *amdgpu_job_dependency(struct amd_sched_job *sched_job)
>   		fence = amdgpu_sync_get_fence(&job->sync);
>   	}
>   
> +	if (amd_sched_dependency_optimized(fence, sched_job->s_entity))
> +		job->need_pipeline_sync = true;
> +
>   	return fence;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index b35b853..b4f83fc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -738,7 +738,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job)
>   	if (ring->funcs->init_cond_exec)
>   		patch_offset = amdgpu_ring_init_cond_exec(ring);
>   
> -	if (ring->funcs->emit_pipeline_sync)
> +	if (ring->funcs->emit_pipeline_sync && !job->need_pipeline_sync)
>   		amdgpu_ring_emit_pipeline_sync(ring);
>   
>   	if (ring->funcs->emit_vm_flush && vm_flush_needed) {
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index 3c258c3..fec870a 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -235,6 +235,23 @@ static void amd_sched_entity_clear_dep(struct fence *f, struct fence_cb *cb)
>   	fence_put(f);
>   }
>   
> +bool amd_sched_dependency_optimized(struct fence* fence,
> +				    struct amd_sched_entity *entity)
> +{
> +	struct amd_gpu_scheduler *sched = entity->sched;
> +	struct amd_sched_fence *s_fence;
> +
> +	if (!fence || fence_is_signaled(fence))
> +		return false;
> +	if (fence->context == entity->fence_context)
> +		return true;
> +	s_fence = to_amd_sched_fence(fence);
> +	if (s_fence && s_fence->sched == sched)
> +		return true;
> +
> +	return false;
> +}
> +
>   static bool amd_sched_entity_add_dependency_cb(struct amd_sched_entity *entity)
>   {
>   	struct amd_gpu_scheduler *sched = entity->sched;
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> index 2531b41..bfda90f 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> @@ -162,4 +162,6 @@ int amd_sched_job_init(struct amd_sched_job *job,
>   		       void *owner);
>   void amd_sched_hw_job_reset(struct amd_gpu_scheduler *sched);
>   void amd_sched_job_recovery(struct amd_gpu_scheduler *sched);
> +bool amd_sched_dependency_optimized(struct fence* fence,
> +				    struct amd_sched_entity *entity);
>   #endif


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

[PATCH 3/3] drm/amdgpu: add sched sync for amdgpu job

[Chunming Zhou]

Change-Id: I26d3a2794272ba94b25753d4bf367326d12f6939
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 6 +++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 ++++-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 787acd7..ef018bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1162,6 +1162,7 @@ struct amdgpu_job {
 	struct amdgpu_vm	*vm;
 	struct amdgpu_ring	*ring;
 	struct amdgpu_sync	sync;
+	struct amdgpu_sync	sched_sync;
 	struct amdgpu_ib	*ibs;
 	struct fence		*fence; /* the hw fence */
 	uint32_t		preamble_status;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 2c6624d..d7f75bc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -167,8 +167,12 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 		return r;
 	}
 
-	if (ring->funcs->emit_pipeline_sync && job && job->need_pipeline_sync)
+	if (ring->funcs->emit_pipeline_sync && job &&
+	    (tmp = amdgpu_sync_get_fence(&job->sched_sync))) {
+		job->need_pipeline_sync = true;
 		amdgpu_ring_emit_pipeline_sync(ring);
+		fence_put(tmp);
+	}
 	if (vm) {
 		amdgpu_ring_insert_nop(ring, extra_nop); /* prevent CE go too fast than DE */
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index cfa97ab..fa0c8b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -60,6 +60,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
 	(*job)->need_pipeline_sync = false;
 
 	amdgpu_sync_create(&(*job)->sync);
+	amdgpu_sync_create(&(*job)->sched_sync);
 
 	return 0;
 }
@@ -98,6 +99,7 @@ static void amdgpu_job_free_cb(struct amd_sched_job *s_job)
 
 	fence_put(job->fence);
 	amdgpu_sync_free(&job->sync);
+	amdgpu_sync_free(&job->sched_sync);
 	kfree(job);
 }
 
@@ -107,6 +109,7 @@ void amdgpu_job_free(struct amdgpu_job *job)
 
 	fence_put(job->fence);
 	amdgpu_sync_free(&job->sync);
+	amdgpu_sync_free(&job->sched_sync);
 	kfree(job);
 }
 
@@ -154,7 +157,7 @@ static struct fence *amdgpu_job_dependency(struct amd_sched_job *sched_job)
 	}
 
 	if (amd_sched_dependency_optimized(fence, sched_job->s_entity))
-		job->need_pipeline_sync = true;
+		amdgpu_sync_fence(job->adev, &job->sched_sync, fence);
 
 	return fence;
 }
-- 
1.9.1

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

Re: [PATCH 3/3] drm/amdgpu: add sched sync for amdgpu job

[Christian König]

What's the background of this change? E.g. why it is needed?

Christian.

Am 09.05.2017 um 10:14 schrieb Chunming Zhou:
> Change-Id: I26d3a2794272ba94b25753d4bf367326d12f6939
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 6 +++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 ++++-
>   3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 787acd7..ef018bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1162,6 +1162,7 @@ struct amdgpu_job {
>   	struct amdgpu_vm	*vm;
>   	struct amdgpu_ring	*ring;
>   	struct amdgpu_sync	sync;
> +	struct amdgpu_sync	sched_sync;
>   	struct amdgpu_ib	*ibs;
>   	struct fence		*fence; /* the hw fence */
>   	uint32_t		preamble_status;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 2c6624d..d7f75bc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -167,8 +167,12 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>   		return r;
>   	}
>   
> -	if (ring->funcs->emit_pipeline_sync && job && job->need_pipeline_sync)
> +	if (ring->funcs->emit_pipeline_sync && job &&
> +	    (tmp = amdgpu_sync_get_fence(&job->sched_sync))) {
> +		job->need_pipeline_sync = true;
>   		amdgpu_ring_emit_pipeline_sync(ring);
> +		fence_put(tmp);
> +	}
>   	if (vm) {
>   		amdgpu_ring_insert_nop(ring, extra_nop); /* prevent CE go too fast than DE */
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index cfa97ab..fa0c8b1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -60,6 +60,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>   	(*job)->need_pipeline_sync = false;
>   
>   	amdgpu_sync_create(&(*job)->sync);
> +	amdgpu_sync_create(&(*job)->sched_sync);
>   
>   	return 0;
>   }
> @@ -98,6 +99,7 @@ static void amdgpu_job_free_cb(struct amd_sched_job *s_job)
>   
>   	fence_put(job->fence);
>   	amdgpu_sync_free(&job->sync);
> +	amdgpu_sync_free(&job->sched_sync);
>   	kfree(job);
>   }
>   
> @@ -107,6 +109,7 @@ void amdgpu_job_free(struct amdgpu_job *job)
>   
>   	fence_put(job->fence);
>   	amdgpu_sync_free(&job->sync);
> +	amdgpu_sync_free(&job->sched_sync);
>   	kfree(job);
>   }
>   
> @@ -154,7 +157,7 @@ static struct fence *amdgpu_job_dependency(struct amd_sched_job *sched_job)
>   	}
>   
>   	if (amd_sched_dependency_optimized(fence, sched_job->s_entity))
> -		job->need_pipeline_sync = true;
> +		amdgpu_sync_fence(job->adev, &job->sched_sync, fence);
>   
>   	return fence;
>   }


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

Re: [PATCH 3/3] drm/amdgpu: add sched sync for amdgpu job

[zhoucm1]

On 2017年05月09日 19:16, Christian König wrote:
> What's the background of this change? E.g. why it is needed?
price question, maybe I should describe more in patch comment, I think 
this is an improvement for previous patch, the sched_sync is to store 
fence that could be skipped as scheduled, when job is executed, we 
didn't need pipeline_sync if all fences in sched_sync are signalled, 
otherwise insert pipeline_sync still.

Regards,
David Zhou
>
> Christian.
>
> Am 09.05.2017 um 10:14 schrieb Chunming Zhou:
>> Change-Id: I26d3a2794272ba94b25753d4bf367326d12f6939
>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 6 +++++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 ++++-
>>   3 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 787acd7..ef018bf 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1162,6 +1162,7 @@ struct amdgpu_job {
>>       struct amdgpu_vm    *vm;
>>       struct amdgpu_ring    *ring;
>>       struct amdgpu_sync    sync;
>> +    struct amdgpu_sync    sched_sync;
>>       struct amdgpu_ib    *ibs;
>>       struct fence        *fence; /* the hw fence */
>>       uint32_t        preamble_status;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 2c6624d..d7f75bc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -167,8 +167,12 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, 
>> unsigned num_ibs,
>>           return r;
>>       }
>>   -    if (ring->funcs->emit_pipeline_sync && job && 
>> job->need_pipeline_sync)
>> +    if (ring->funcs->emit_pipeline_sync && job &&
>> +        (tmp = amdgpu_sync_get_fence(&job->sched_sync))) {
>> +        job->need_pipeline_sync = true;
>>           amdgpu_ring_emit_pipeline_sync(ring);
>> +        fence_put(tmp);
>> +    }
>>       if (vm) {
>>           amdgpu_ring_insert_nop(ring, extra_nop); /* prevent CE go 
>> too fast than DE */
>>   diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> index cfa97ab..fa0c8b1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> @@ -60,6 +60,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, 
>> unsigned num_ibs,
>>       (*job)->need_pipeline_sync = false;
>>         amdgpu_sync_create(&(*job)->sync);
>> +    amdgpu_sync_create(&(*job)->sched_sync);
>>         return 0;
>>   }
>> @@ -98,6 +99,7 @@ static void amdgpu_job_free_cb(struct amd_sched_job 
>> *s_job)
>>         fence_put(job->fence);
>>       amdgpu_sync_free(&job->sync);
>> +    amdgpu_sync_free(&job->sched_sync);
>>       kfree(job);
>>   }
>>   @@ -107,6 +109,7 @@ void amdgpu_job_free(struct amdgpu_job *job)
>>         fence_put(job->fence);
>>       amdgpu_sync_free(&job->sync);
>> +    amdgpu_sync_free(&job->sched_sync);
>>       kfree(job);
>>   }
>>   @@ -154,7 +157,7 @@ static struct fence 
>> *amdgpu_job_dependency(struct amd_sched_job *sched_job)
>>       }
>>         if (amd_sched_dependency_optimized(fence, sched_job->s_entity))
>> -        job->need_pipeline_sync = true;
>> +        amdgpu_sync_fence(job->adev, &job->sched_sync, fence);
>>         return fence;
>>   }
>
>

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

Re: [PATCH 1/3] drm/amd: fix init order of sched job

[Zhang, Jerry (Junwei)]

On 05/09/2017 04:14 PM, Chunming Zhou wrote:
> Change-Id: Iced391f5c24a79ad7aecae33e22ff089f68f1337
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>

Good catch!
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

> ---
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index 80bc4f7..3c258c3 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -460,9 +460,9 @@ int amd_sched_job_init(struct amd_sched_job *job,
>   	job->sched = sched;
>   	job->s_entity = entity;
>   	job->s_fence = amd_sched_fence_create(entity, owner);
> -	job->id = atomic64_inc_return(&sched->job_id_count);
>   	if (!job->s_fence)
>   		return -ENOMEM;
> +	job->id = atomic64_inc_return(&sched->job_id_count);
>
>   	INIT_WORK(&job->finish_work, amd_sched_job_finish);
>   	INIT_LIST_HEAD(&job->node);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] drm/amd: fix init order of sched job

[Christian König]

Am 09.05.2017 um 10:33 schrieb Zhang, Jerry (Junwei):
> On 05/09/2017 04:14 PM, Chunming Zhou wrote:
>> Change-Id: Iced391f5c24a79ad7aecae33e22ff089f68f1337
>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>
> Good catch!
> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

Indeed, nice catch.

Reviewed-by: Christian König <christian.koenig@amd.com>

>
>> ---
>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c 
>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>> index 80bc4f7..3c258c3 100644
>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>> @@ -460,9 +460,9 @@ int amd_sched_job_init(struct amd_sched_job *job,
>>       job->sched = sched;
>>       job->s_entity = entity;
>>       job->s_fence = amd_sched_fence_create(entity, owner);
>> -    job->id = atomic64_inc_return(&sched->job_id_count);
>>       if (!job->s_fence)
>>           return -ENOMEM;
>> +    job->id = atomic64_inc_return(&sched->job_id_count);
>>
>>       INIT_WORK(&job->finish_work, amd_sched_job_finish);
>>       INIT_LIST_HEAD(&job->node);
>>
> _______________________________________________
> 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