dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] scheduler code cleanup
@ 2016-05-25 21:04 Alex Deucher
  2016-05-25 21:04 ` [PATCH 1/8] drm/amdgpu: fix coding style in the scheduler v2 Alex Deucher
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher

Just some general cleanup in the GPU scheduler.

Christian König (8):
  drm/amdgpu: fix coding style in the scheduler v2
  drm/amdgpu: remove begin_job/finish_job
  drm/amdgpu: remove duplicated timeout callback
  drm/amdgpu: fix coding style in amdgpu_job_free
  drm/amdgpu: remove use_shed hack in job cleanup
  drm/amdgpu: properly abstract scheduler timeout handling
  drm/amdgpu: move locking into the functions who need it
  drm/amdgpu: fix and cleanup job destruction

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       | 49 +++++++--------
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 89 ++++++++++++++-------------
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 42 ++++---------
 drivers/gpu/drm/amd/scheduler/sched_fence.c   | 19 ++----
 6 files changed, 86 insertions(+), 119 deletions(-)

-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/8] drm/amdgpu: fix coding style in the scheduler v2
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
@ 2016-05-25 21:04 ` Alex Deucher
  2016-05-25 21:04 ` [PATCH 2/8] drm/amdgpu: remove begin_job/finish_job Alex Deucher
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher, Christian König

From: Christian König <christian.koenig@amd.com>

v2: fix even more

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Monk.Liu <monk.liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 25 +++++++++++++++----------
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 21 ++++++++++++---------
 drivers/gpu/drm/amd/scheduler/sched_fence.c   |  9 +++++----
 3 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index c16248c..f5ac01db2 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -320,7 +320,9 @@ static bool amd_sched_entity_in(struct amd_sched_job *sched_job)
 }
 
 static void amd_sched_free_job(struct fence *f, struct fence_cb *cb) {
-	struct amd_sched_job *job = container_of(cb, struct amd_sched_job, cb_free_job);
+	struct amd_sched_job *job = container_of(cb, struct amd_sched_job,
+						 cb_free_job);
+
 	schedule_work(&job->work_free_job);
 }
 
@@ -341,7 +343,8 @@ void amd_sched_job_finish(struct amd_sched_job *s_job)
 						struct amd_sched_job, node);
 
 		if (next) {
-			INIT_DELAYED_WORK(&next->work_tdr, s_job->timeout_callback);
+			INIT_DELAYED_WORK(&next->work_tdr,
+					  s_job->timeout_callback);
 			amd_sched_job_get(next);
 			schedule_delayed_work(&next->work_tdr, sched->timeout);
 		}
@@ -353,7 +356,8 @@ void amd_sched_job_begin(struct amd_sched_job *s_job)
 	struct amd_gpu_scheduler *sched = s_job->sched;
 
 	if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
-		list_first_entry_or_null(&sched->ring_mirror_list, struct amd_sched_job, node) == s_job)
+	    list_first_entry_or_null(&sched->ring_mirror_list,
+				     struct amd_sched_job, node) == s_job)
 	{
 		INIT_DELAYED_WORK(&s_job->work_tdr, s_job->timeout_callback);
 		amd_sched_job_get(s_job);
@@ -374,7 +378,7 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job)
 
 	sched_job->use_sched = 1;
 	fence_add_callback(&sched_job->s_fence->base,
-					&sched_job->cb_free_job, amd_sched_free_job);
+			   &sched_job->cb_free_job, amd_sched_free_job);
 	trace_amd_sched_job(sched_job);
 	wait_event(entity->sched->job_scheduled,
 		   amd_sched_entity_in(sched_job));
@@ -382,11 +386,11 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job)
 
 /* init a sched_job with basic field */
 int amd_sched_job_init(struct amd_sched_job *job,
-						struct amd_gpu_scheduler *sched,
-						struct amd_sched_entity *entity,
-						void (*timeout_cb)(struct work_struct *work),
-						void (*free_cb)(struct kref *refcount),
-						void *owner, struct fence **fence)
+		       struct amd_gpu_scheduler *sched,
+		       struct amd_sched_entity *entity,
+		       void (*timeout_cb)(struct work_struct *work),
+		       void (*free_cb)(struct kref *refcount),
+		       void *owner, struct fence **fence)
 {
 	INIT_LIST_HEAD(&job->node);
 	kref_init(&job->refcount);
@@ -504,7 +508,8 @@ static int amd_sched_main(void *param)
 			if (r == -ENOENT)
 				amd_sched_process_job(fence, &s_fence->cb);
 			else if (r)
-				DRM_ERROR("fence add callback failed (%d)\n", r);
+				DRM_ERROR("fence add callback failed (%d)\n",
+					  r);
 			fence_put(fence);
 		} else {
 			DRM_ERROR("Failed to run job!\n");
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 070095a..690ae4b 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -94,7 +94,8 @@ struct amd_sched_job {
 extern const struct fence_ops amd_sched_fence_ops;
 static inline struct amd_sched_fence *to_amd_sched_fence(struct fence *f)
 {
-	struct amd_sched_fence *__f = container_of(f, struct amd_sched_fence, base);
+	struct amd_sched_fence *__f = container_of(f, struct amd_sched_fence,
+						   base);
 
 	if (__f->base.ops == &amd_sched_fence_ops)
 		return __f;
@@ -154,21 +155,23 @@ struct amd_sched_fence *amd_sched_fence_create(
 void amd_sched_fence_scheduled(struct amd_sched_fence *fence);
 void amd_sched_fence_signal(struct amd_sched_fence *fence);
 int amd_sched_job_init(struct amd_sched_job *job,
-					struct amd_gpu_scheduler *sched,
-					struct amd_sched_entity *entity,
-					void (*timeout_cb)(struct work_struct *work),
-					void (*free_cb)(struct kref* refcount),
-					void *owner, struct fence **fence);
+		       struct amd_gpu_scheduler *sched,
+		       struct amd_sched_entity *entity,
+		       void (*timeout_cb)(struct work_struct *work),
+		       void (*free_cb)(struct kref* refcount),
+		       void *owner, struct fence **fence);
 void amd_sched_job_pre_schedule(struct amd_gpu_scheduler *sched ,
-								struct amd_sched_job *s_job);
+				struct amd_sched_job *s_job);
 void amd_sched_job_finish(struct amd_sched_job *s_job);
 void amd_sched_job_begin(struct amd_sched_job *s_job);
-static inline void amd_sched_job_get(struct amd_sched_job *job) {
+static inline void amd_sched_job_get(struct amd_sched_job *job)
+{
 	if (job)
 		kref_get(&job->refcount);
 }
 
-static inline void amd_sched_job_put(struct amd_sched_job *job) {
+static inline void amd_sched_job_put(struct amd_sched_job *job)
+{
 	if (job)
 		kref_put(&job->refcount, job->free_callback);
 }
diff --git a/drivers/gpu/drm/amd/scheduler/sched_fence.c b/drivers/gpu/drm/amd/scheduler/sched_fence.c
index 2a732c4..6bdc9b7 100644
--- a/drivers/gpu/drm/amd/scheduler/sched_fence.c
+++ b/drivers/gpu/drm/amd/scheduler/sched_fence.c
@@ -27,7 +27,8 @@
 #include <drm/drmP.h>
 #include "gpu_scheduler.h"
 
-struct amd_sched_fence *amd_sched_fence_create(struct amd_sched_entity *s_entity, void *owner)
+struct amd_sched_fence *amd_sched_fence_create(struct amd_sched_entity *entity,
+					       void *owner)
 {
 	struct amd_sched_fence *fence = NULL;
 	unsigned seq;
@@ -38,12 +39,12 @@ struct amd_sched_fence *amd_sched_fence_create(struct amd_sched_entity *s_entity
 
 	INIT_LIST_HEAD(&fence->scheduled_cb);
 	fence->owner = owner;
-	fence->sched = s_entity->sched;
+	fence->sched = entity->sched;
 	spin_lock_init(&fence->lock);
 
-	seq = atomic_inc_return(&s_entity->fence_seq);
+	seq = atomic_inc_return(&entity->fence_seq);
 	fence_init(&fence->base, &amd_sched_fence_ops, &fence->lock,
-		   s_entity->fence_context, seq);
+		   entity->fence_context, seq);
 
 	return fence;
 }
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/8] drm/amdgpu: remove begin_job/finish_job
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
  2016-05-25 21:04 ` [PATCH 1/8] drm/amdgpu: fix coding style in the scheduler v2 Alex Deucher
@ 2016-05-25 21:04 ` Alex Deucher
  2016-05-25 21:04 ` [PATCH 3/8] drm/amdgpu: remove duplicated timeout callback Alex Deucher
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher, Christian König

From: Christian König <christian.koenig@amd.com>

Completely pointless and confusing to use a callback
to call into the same code file.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Monk.Liu <monk.liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |  2 --
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 13 +++++++++----
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  6 ------
 drivers/gpu/drm/amd/scheduler/sched_fence.c   | 10 ----------
 4 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index f0dafa5..001030b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -193,6 +193,4 @@ err:
 const struct amd_sched_backend_ops amdgpu_sched_ops = {
 	.dependency = amdgpu_job_dependency,
 	.run_job = amdgpu_job_run,
-	.begin_job = amd_sched_job_begin,
-	.finish_job = amd_sched_job_finish,
 };
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index f5ac01db2..821bc89 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -329,7 +329,7 @@ static void amd_sched_free_job(struct fence *f, struct fence_cb *cb) {
 /* job_finish is called after hw fence signaled, and
  * the job had already been deleted from ring_mirror_list
  */
-void amd_sched_job_finish(struct amd_sched_job *s_job)
+static void amd_sched_job_finish(struct amd_sched_job *s_job)
 {
 	struct amd_sched_job *next;
 	struct amd_gpu_scheduler *sched = s_job->sched;
@@ -351,7 +351,7 @@ void amd_sched_job_finish(struct amd_sched_job *s_job)
 	}
 }
 
-void amd_sched_job_begin(struct amd_sched_job *s_job)
+static void amd_sched_job_begin(struct amd_sched_job *s_job)
 {
 	struct amd_gpu_scheduler *sched = s_job->sched;
 
@@ -461,7 +461,7 @@ static void amd_sched_process_job(struct fence *f, struct fence_cb *cb)
 	/* remove job from ring_mirror_list */
 	spin_lock_irqsave(&sched->job_list_lock, flags);
 	list_del_init(&s_fence->s_job->node);
-	sched->ops->finish_job(s_fence->s_job);
+	amd_sched_job_finish(s_fence->s_job);
 	spin_unlock_irqrestore(&sched->job_list_lock, flags);
 
 	amd_sched_fence_signal(s_fence);
@@ -475,6 +475,7 @@ static int amd_sched_main(void *param)
 {
 	struct sched_param sparam = {.sched_priority = 1};
 	struct amd_gpu_scheduler *sched = (struct amd_gpu_scheduler *)param;
+	unsigned long flags;
 	int r, count;
 
 	sched_setscheduler(current, SCHED_FIFO, &sparam);
@@ -499,7 +500,11 @@ static int amd_sched_main(void *param)
 		s_fence = sched_job->s_fence;
 
 		atomic_inc(&sched->hw_rq_count);
-		amd_sched_job_pre_schedule(sched, sched_job);
+		spin_lock_irqsave(&sched->job_list_lock, flags);
+		list_add_tail(&sched_job->node, &sched->ring_mirror_list);
+		amd_sched_job_begin(sched_job);
+		spin_unlock_irqrestore(&sched->job_list_lock, flags);
+
 		fence = sched->ops->run_job(sched_job);
 		amd_sched_fence_scheduled(s_fence);
 		if (fence) {
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 690ae4b..69840d7 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -110,8 +110,6 @@ static inline struct amd_sched_fence *to_amd_sched_fence(struct fence *f)
 struct amd_sched_backend_ops {
 	struct fence *(*dependency)(struct amd_sched_job *sched_job);
 	struct fence *(*run_job)(struct amd_sched_job *sched_job);
-	void (*begin_job)(struct amd_sched_job *sched_job);
-	void (*finish_job)(struct amd_sched_job *sched_job);
 };
 
 enum amd_sched_priority {
@@ -160,10 +158,6 @@ int amd_sched_job_init(struct amd_sched_job *job,
 		       void (*timeout_cb)(struct work_struct *work),
 		       void (*free_cb)(struct kref* refcount),
 		       void *owner, struct fence **fence);
-void amd_sched_job_pre_schedule(struct amd_gpu_scheduler *sched ,
-				struct amd_sched_job *s_job);
-void amd_sched_job_finish(struct amd_sched_job *s_job);
-void amd_sched_job_begin(struct amd_sched_job *s_job);
 static inline void amd_sched_job_get(struct amd_sched_job *job)
 {
 	if (job)
diff --git a/drivers/gpu/drm/amd/scheduler/sched_fence.c b/drivers/gpu/drm/amd/scheduler/sched_fence.c
index 6bdc9b7..71931bc 100644
--- a/drivers/gpu/drm/amd/scheduler/sched_fence.c
+++ b/drivers/gpu/drm/amd/scheduler/sched_fence.c
@@ -58,16 +58,6 @@ void amd_sched_fence_signal(struct amd_sched_fence *fence)
 		FENCE_TRACE(&fence->base, "was already signaled\n");
 }
 
-void amd_sched_job_pre_schedule(struct amd_gpu_scheduler *sched ,
-				struct amd_sched_job *s_job)
-{
-	unsigned long flags;
-	spin_lock_irqsave(&sched->job_list_lock, flags);
-	list_add_tail(&s_job->node, &sched->ring_mirror_list);
-	sched->ops->begin_job(s_job);
-	spin_unlock_irqrestore(&sched->job_list_lock, flags);
-}
-
 void amd_sched_fence_scheduled(struct amd_sched_fence *s_fence)
 {
 	struct fence_cb *cur, *tmp;
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/8] drm/amdgpu: remove duplicated timeout callback
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
  2016-05-25 21:04 ` [PATCH 1/8] drm/amdgpu: fix coding style in the scheduler v2 Alex Deucher
  2016-05-25 21:04 ` [PATCH 2/8] drm/amdgpu: remove begin_job/finish_job Alex Deucher
@ 2016-05-25 21:04 ` Alex Deucher
  2016-05-25 21:04 ` [PATCH 4/8] drm/amdgpu: fix coding style in amdgpu_job_free Alex Deucher
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher, Christian König

From: Christian König <christian.koenig@amd.com>

No need for double housekeeping here.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Monk.Liu <monk.liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 5 +----
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 1 -
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 821bc89..f3fd80b 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -343,8 +343,6 @@ static void amd_sched_job_finish(struct amd_sched_job *s_job)
 						struct amd_sched_job, node);
 
 		if (next) {
-			INIT_DELAYED_WORK(&next->work_tdr,
-					  s_job->timeout_callback);
 			amd_sched_job_get(next);
 			schedule_delayed_work(&next->work_tdr, sched->timeout);
 		}
@@ -359,7 +357,6 @@ static void amd_sched_job_begin(struct amd_sched_job *s_job)
 	    list_first_entry_or_null(&sched->ring_mirror_list,
 				     struct amd_sched_job, node) == s_job)
 	{
-		INIT_DELAYED_WORK(&s_job->work_tdr, s_job->timeout_callback);
 		amd_sched_job_get(s_job);
 		schedule_delayed_work(&s_job->work_tdr, sched->timeout);
 	}
@@ -401,7 +398,7 @@ int amd_sched_job_init(struct amd_sched_job *job,
 		return -ENOMEM;
 
 	job->s_fence->s_job = job;
-	job->timeout_callback = timeout_cb;
+	INIT_DELAYED_WORK(&job->work_tdr, timeout_cb);
 	job->free_callback = free_cb;
 
 	if (fence)
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 69840d7..ec55b9f 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -87,7 +87,6 @@ struct amd_sched_job {
 	struct work_struct             work_free_job;
 	struct list_head			   node;
 	struct delayed_work work_tdr;
-	void (*timeout_callback) (struct work_struct *work);
 	void (*free_callback)(struct kref *refcount);
 };
 
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/8] drm/amdgpu: fix coding style in amdgpu_job_free
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
                   ` (2 preceding siblings ...)
  2016-05-25 21:04 ` [PATCH 3/8] drm/amdgpu: remove duplicated timeout callback Alex Deucher
@ 2016-05-25 21:04 ` Alex Deucher
  2016-05-25 21:04 ` [PATCH 5/8] drm/amdgpu: remove use_shed hack in job cleanup Alex Deucher
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher, Christian König

From: Christian König <christian.koenig@amd.com>

Ther should be a new line between code and decleration.
Also use amdgpu_ib_free() instead of releasing the member manually.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Monk.Liu <monk.liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 001030b..f0fa485 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -88,13 +88,14 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 
 void amdgpu_job_free(struct amdgpu_job *job)
 {
-	unsigned i;
 	struct fence *f;
+	unsigned i;
+
 	/* use sched fence if available */
-	f = (job->base.s_fence)? &job->base.s_fence->base : job->fence;
+	f = job->base.s_fence ? &job->base.s_fence->base : job->fence;
 
 	for (i = 0; i < job->num_ibs; ++i)
-		amdgpu_sa_bo_free(job->adev, &job->ibs[i].sa_bo, f);
+		amdgpu_ib_free(job->adev, &job->ibs[i], f);
 	fence_put(job->fence);
 
 	amdgpu_bo_unref(&job->uf_bo);
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/8] drm/amdgpu: remove use_shed hack in job cleanup
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
                   ` (3 preceding siblings ...)
  2016-05-25 21:04 ` [PATCH 4/8] drm/amdgpu: fix coding style in amdgpu_job_free Alex Deucher
@ 2016-05-25 21:04 ` Alex Deucher
  2016-05-25 21:04 ` [PATCH 6/8] drm/amdgpu: properly abstract scheduler timeout handling Alex Deucher
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher, Christian König

From: Christian König <christian.koenig@amd.com>

Remembering the code path in a variable to cleanup
differently is usually not a good idea at all.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Monk.Liu <monk.liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       | 13 ++++++++-----
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c |  1 -
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  1 -
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index f0fa485..be4698b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -86,7 +86,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 	return r;
 }
 
-void amdgpu_job_free(struct amdgpu_job *job)
+static void amdgpu_job_free_resources(struct amdgpu_job *job)
 {
 	struct fence *f;
 	unsigned i;
@@ -100,9 +100,6 @@ void amdgpu_job_free(struct amdgpu_job *job)
 
 	amdgpu_bo_unref(&job->uf_bo);
 	amdgpu_sync_free(&job->sync);
-
-	if (!job->base.use_sched)
-		kfree(job);
 }
 
 void amdgpu_job_free_func(struct kref *refcount)
@@ -111,6 +108,12 @@ void amdgpu_job_free_func(struct kref *refcount)
 	kfree(job);
 }
 
+void amdgpu_job_free(struct amdgpu_job *job)
+{
+	amdgpu_job_free_resources(job);
+	kfree(job);
+}
+
 int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
 		      struct amd_sched_entity *entity, void *owner,
 		      struct fence **f)
@@ -187,7 +190,7 @@ static struct fence *amdgpu_job_run(struct amd_sched_job *sched_job)
 
 err:
 	job->fence = fence;
-	amdgpu_job_free(job);
+	amdgpu_job_free_resources(job);
 	return fence;
 }
 
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index f3fd80b..e8ee90f 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -373,7 +373,6 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job)
 {
 	struct amd_sched_entity *entity = sched_job->s_entity;
 
-	sched_job->use_sched = 1;
 	fence_add_callback(&sched_job->s_fence->base,
 			   &sched_job->cb_free_job, amd_sched_free_job);
 	trace_amd_sched_job(sched_job);
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index ec55b9f..7e333fa 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -82,7 +82,6 @@ struct amd_sched_job {
 	struct amd_gpu_scheduler        *sched;
 	struct amd_sched_entity         *s_entity;
 	struct amd_sched_fence          *s_fence;
-	bool	use_sched;	/* true if the job goes to scheduler */
 	struct fence_cb                cb_free_job;
 	struct work_struct             work_free_job;
 	struct list_head			   node;
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/8] drm/amdgpu: properly abstract scheduler timeout handling
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
                   ` (4 preceding siblings ...)
  2016-05-25 21:04 ` [PATCH 5/8] drm/amdgpu: remove use_shed hack in job cleanup Alex Deucher
@ 2016-05-25 21:04 ` Alex Deucher
  2016-05-25 21:04 ` [PATCH 7/8] drm/amdgpu: move locking into the functions who need it Alex Deucher
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher, Christian König

From: Christian König <christian.koenig@amd.com>

The driver shouldn't mess with the scheduler internals.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Monk.Liu <monk.liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |  3 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       | 15 ++++++++-------
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 11 +++++++++--
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 +-
 5 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 6c5ec32..ec42dd5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -759,7 +759,6 @@ void amdgpu_job_free_func(struct kref *refcount);
 int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
 		      struct amd_sched_entity *entity, void *owner,
 		      struct fence **f);
-void amdgpu_job_timeout_func(struct work_struct *work);
 
 struct amdgpu_ring {
 	struct amdgpu_device		*adev;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 2bbeeb0..d8cac31 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -839,8 +839,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	p->job = NULL;
 
 	r = amd_sched_job_init(&job->base, &ring->sched,
-			       entity, amdgpu_job_timeout_func,
-			       amdgpu_job_free_func,
+			       entity, amdgpu_job_free_func,
 			       p->filp, &fence);
 	if (r) {
 		amdgpu_job_free(job);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index be4698b..32132f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -34,13 +34,14 @@ static void amdgpu_job_free_handler(struct work_struct *ws)
 	amd_sched_job_put(&job->base);
 }
 
-void amdgpu_job_timeout_func(struct work_struct *work)
+static void amdgpu_job_timedout(struct amd_sched_job *s_job)
 {
-	struct amdgpu_job *job = container_of(work, struct amdgpu_job, base.work_tdr.work);
+	struct amdgpu_job *job = container_of(s_job, struct amdgpu_job, base);
+
 	DRM_ERROR("ring %s timeout, last signaled seq=%u, last emitted seq=%u\n",
-				job->base.sched->name,
-				(uint32_t)atomic_read(&job->ring->fence_drv.last_seq),
-				job->ring->fence_drv.sync_seq);
+		  job->base.sched->name,
+		  atomic_read(&job->ring->fence_drv.last_seq),
+		  job->ring->fence_drv.sync_seq);
 
 	amd_sched_job_put(&job->base);
 }
@@ -126,8 +127,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
 		return -EINVAL;
 
 	r = amd_sched_job_init(&job->base, &ring->sched,
-			       entity, amdgpu_job_timeout_func,
-			       amdgpu_job_free_func, owner, &fence);
+			       entity, amdgpu_job_free_func, owner, &fence);
 	if (r)
 		return r;
 
@@ -197,4 +197,5 @@ err:
 const struct amd_sched_backend_ops amdgpu_sched_ops = {
 	.dependency = amdgpu_job_dependency,
 	.run_job = amdgpu_job_run,
+	.timedout_job = amdgpu_job_timedout,
 };
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index e8ee90f..f2ed8c5 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -362,6 +362,14 @@ static void amd_sched_job_begin(struct amd_sched_job *s_job)
 	}
 }
 
+static void amd_sched_job_timedout(struct work_struct *work)
+{
+	struct amd_sched_job *job = container_of(work, struct amd_sched_job,
+						 work_tdr.work);
+
+	job->sched->ops->timedout_job(job);
+}
+
 /**
  * Submit a job to the job queue
  *
@@ -384,7 +392,6 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job)
 int amd_sched_job_init(struct amd_sched_job *job,
 		       struct amd_gpu_scheduler *sched,
 		       struct amd_sched_entity *entity,
-		       void (*timeout_cb)(struct work_struct *work),
 		       void (*free_cb)(struct kref *refcount),
 		       void *owner, struct fence **fence)
 {
@@ -397,7 +404,7 @@ int amd_sched_job_init(struct amd_sched_job *job,
 		return -ENOMEM;
 
 	job->s_fence->s_job = job;
-	INIT_DELAYED_WORK(&job->work_tdr, timeout_cb);
+	INIT_DELAYED_WORK(&job->work_tdr, amd_sched_job_timedout);
 	job->free_callback = free_cb;
 
 	if (fence)
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 7e333fa..f0de46c 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -108,6 +108,7 @@ static inline struct amd_sched_fence *to_amd_sched_fence(struct fence *f)
 struct amd_sched_backend_ops {
 	struct fence *(*dependency)(struct amd_sched_job *sched_job);
 	struct fence *(*run_job)(struct amd_sched_job *sched_job);
+	void (*timedout_job)(struct amd_sched_job *sched_job);
 };
 
 enum amd_sched_priority {
@@ -153,7 +154,6 @@ void amd_sched_fence_signal(struct amd_sched_fence *fence);
 int amd_sched_job_init(struct amd_sched_job *job,
 		       struct amd_gpu_scheduler *sched,
 		       struct amd_sched_entity *entity,
-		       void (*timeout_cb)(struct work_struct *work),
 		       void (*free_cb)(struct kref* refcount),
 		       void *owner, struct fence **fence);
 static inline void amd_sched_job_get(struct amd_sched_job *job)
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/8] drm/amdgpu: move locking into the functions who need it
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
                   ` (5 preceding siblings ...)
  2016-05-25 21:04 ` [PATCH 6/8] drm/amdgpu: properly abstract scheduler timeout handling Alex Deucher
@ 2016-05-25 21:04 ` Alex Deucher
  2016-05-25 21:04 ` [PATCH 8/8] drm/amdgpu: fix and cleanup job destruction Alex Deucher
  2016-05-26  8:09 ` [PATCH 0/8] scheduler code cleanup Daniel Vetter
  8 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher, Christian König

From: Christian König <christian.koenig@amd.com>

Otherwise the locking becomes rather confusing.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Monk.Liu <monk.liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index f2ed8c5..cb56d90 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -333,7 +333,11 @@ static void amd_sched_job_finish(struct amd_sched_job *s_job)
 {
 	struct amd_sched_job *next;
 	struct amd_gpu_scheduler *sched = s_job->sched;
+	unsigned long flags;
 
+	/* remove job from ring_mirror_list */
+	spin_lock_irqsave(&sched->job_list_lock, flags);
+	list_del_init(&s_job->node);
 	if (sched->timeout != MAX_SCHEDULE_TIMEOUT) {
 		if (cancel_delayed_work(&s_job->work_tdr))
 			amd_sched_job_put(s_job);
@@ -347,12 +351,16 @@ static void amd_sched_job_finish(struct amd_sched_job *s_job)
 			schedule_delayed_work(&next->work_tdr, sched->timeout);
 		}
 	}
+	spin_unlock_irqrestore(&sched->job_list_lock, flags);
 }
 
 static void amd_sched_job_begin(struct amd_sched_job *s_job)
 {
 	struct amd_gpu_scheduler *sched = s_job->sched;
+	unsigned long flags;
 
+	spin_lock_irqsave(&sched->job_list_lock, flags);
+	list_add_tail(&s_job->node, &sched->ring_mirror_list);
 	if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
 	    list_first_entry_or_null(&sched->ring_mirror_list,
 				     struct amd_sched_job, node) == s_job)
@@ -360,6 +368,7 @@ static void amd_sched_job_begin(struct amd_sched_job *s_job)
 		amd_sched_job_get(s_job);
 		schedule_delayed_work(&s_job->work_tdr, sched->timeout);
 	}
+	spin_unlock_irqrestore(&sched->job_list_lock, flags);
 }
 
 static void amd_sched_job_timedout(struct work_struct *work)
@@ -457,15 +466,10 @@ static void amd_sched_process_job(struct fence *f, struct fence_cb *cb)
 	struct amd_sched_fence *s_fence =
 		container_of(cb, struct amd_sched_fence, cb);
 	struct amd_gpu_scheduler *sched = s_fence->sched;
-	unsigned long flags;
 
 	atomic_dec(&sched->hw_rq_count);
 
-	/* remove job from ring_mirror_list */
-	spin_lock_irqsave(&sched->job_list_lock, flags);
-	list_del_init(&s_fence->s_job->node);
 	amd_sched_job_finish(s_fence->s_job);
-	spin_unlock_irqrestore(&sched->job_list_lock, flags);
 
 	amd_sched_fence_signal(s_fence);
 
@@ -478,7 +482,6 @@ static int amd_sched_main(void *param)
 {
 	struct sched_param sparam = {.sched_priority = 1};
 	struct amd_gpu_scheduler *sched = (struct amd_gpu_scheduler *)param;
-	unsigned long flags;
 	int r, count;
 
 	sched_setscheduler(current, SCHED_FIFO, &sparam);
@@ -503,10 +506,7 @@ static int amd_sched_main(void *param)
 		s_fence = sched_job->s_fence;
 
 		atomic_inc(&sched->hw_rq_count);
-		spin_lock_irqsave(&sched->job_list_lock, flags);
-		list_add_tail(&sched_job->node, &sched->ring_mirror_list);
 		amd_sched_job_begin(sched_job);
-		spin_unlock_irqrestore(&sched->job_list_lock, flags);
 
 		fence = sched->ops->run_job(sched_job);
 		amd_sched_fence_scheduled(s_fence);
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/8] drm/amdgpu: fix and cleanup job destruction
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
                   ` (6 preceding siblings ...)
  2016-05-25 21:04 ` [PATCH 7/8] drm/amdgpu: move locking into the functions who need it Alex Deucher
@ 2016-05-25 21:04 ` Alex Deucher
  2016-05-26  8:09 ` [PATCH 0/8] scheduler code cleanup Daniel Vetter
  8 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2016-05-25 21:04 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher, Christian König

From: Christian König <christian.koenig@amd.com>

Remove the job reference counting and just properly destroy it from a
work item which blocks on any potential running timeout handler.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Monk.Liu <monk.liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       | 18 +++--------
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 46 +++++++++++----------------
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 25 +++------------
 5 files changed, 30 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ec42dd5..e72104b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -755,7 +755,6 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 			     struct amdgpu_job **job);
 
 void amdgpu_job_free(struct amdgpu_job *job);
-void amdgpu_job_free_func(struct kref *refcount);
 int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
 		      struct amd_sched_entity *entity, void *owner,
 		      struct fence **f);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d8cac31..a3d7d13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -839,8 +839,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	p->job = NULL;
 
 	r = amd_sched_job_init(&job->base, &ring->sched,
-			       entity, amdgpu_job_free_func,
-			       p->filp, &fence);
+			       entity, p->filp, &fence);
 	if (r) {
 		amdgpu_job_free(job);
 		return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 32132f2..34cd971 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -28,12 +28,6 @@
 #include "amdgpu.h"
 #include "amdgpu_trace.h"
 
-static void amdgpu_job_free_handler(struct work_struct *ws)
-{
-	struct amdgpu_job *job = container_of(ws, struct amdgpu_job, base.work_free_job);
-	amd_sched_job_put(&job->base);
-}
-
 static void amdgpu_job_timedout(struct amd_sched_job *s_job)
 {
 	struct amdgpu_job *job = container_of(s_job, struct amdgpu_job, base);
@@ -42,8 +36,6 @@ static void amdgpu_job_timedout(struct amd_sched_job *s_job)
 		  job->base.sched->name,
 		  atomic_read(&job->ring->fence_drv.last_seq),
 		  job->ring->fence_drv.sync_seq);
-
-	amd_sched_job_put(&job->base);
 }
 
 int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
@@ -64,7 +56,6 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
 	(*job)->vm = vm;
 	(*job)->ibs = (void *)&(*job)[1];
 	(*job)->num_ibs = num_ibs;
-	INIT_WORK(&(*job)->base.work_free_job, amdgpu_job_free_handler);
 
 	amdgpu_sync_create(&(*job)->sync);
 
@@ -103,9 +94,10 @@ static void amdgpu_job_free_resources(struct amdgpu_job *job)
 	amdgpu_sync_free(&job->sync);
 }
 
-void amdgpu_job_free_func(struct kref *refcount)
+void amdgpu_job_free_cb(struct amd_sched_job *s_job)
 {
-	struct amdgpu_job *job = container_of(refcount, struct amdgpu_job, base.refcount);
+	struct amdgpu_job *job = container_of(s_job, struct amdgpu_job, base);
+
 	kfree(job);
 }
 
@@ -126,8 +118,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
 	if (!f)
 		return -EINVAL;
 
-	r = amd_sched_job_init(&job->base, &ring->sched,
-			       entity, amdgpu_job_free_func, owner, &fence);
+	r = amd_sched_job_init(&job->base, &ring->sched, entity, owner, &fence);
 	if (r)
 		return r;
 
@@ -198,4 +189,5 @@ const struct amd_sched_backend_ops amdgpu_sched_ops = {
 	.dependency = amdgpu_job_dependency,
 	.run_job = amdgpu_job_run,
 	.timedout_job = amdgpu_job_timedout,
+	.free_job = amdgpu_job_free_cb
 };
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index cb56d90..2425172 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -319,19 +319,13 @@ static bool amd_sched_entity_in(struct amd_sched_job *sched_job)
 	return added;
 }
 
-static void amd_sched_free_job(struct fence *f, struct fence_cb *cb) {
-	struct amd_sched_job *job = container_of(cb, struct amd_sched_job,
-						 cb_free_job);
-
-	schedule_work(&job->work_free_job);
-}
-
 /* job_finish is called after hw fence signaled, and
  * the job had already been deleted from ring_mirror_list
  */
-static void amd_sched_job_finish(struct amd_sched_job *s_job)
+static void amd_sched_job_finish(struct work_struct *work)
 {
-	struct amd_sched_job *next;
+	struct amd_sched_job *s_job = container_of(work, struct amd_sched_job,
+						   finish_work);
 	struct amd_gpu_scheduler *sched = s_job->sched;
 	unsigned long flags;
 
@@ -339,19 +333,26 @@ static void amd_sched_job_finish(struct amd_sched_job *s_job)
 	spin_lock_irqsave(&sched->job_list_lock, flags);
 	list_del_init(&s_job->node);
 	if (sched->timeout != MAX_SCHEDULE_TIMEOUT) {
-		if (cancel_delayed_work(&s_job->work_tdr))
-			amd_sched_job_put(s_job);
+		struct amd_sched_job *next;
+
+		cancel_delayed_work_sync(&s_job->work_tdr);
 
 		/* queue TDR for next job */
 		next = list_first_entry_or_null(&sched->ring_mirror_list,
 						struct amd_sched_job, node);
 
-		if (next) {
-			amd_sched_job_get(next);
+		if (next)
 			schedule_delayed_work(&next->work_tdr, sched->timeout);
-		}
 	}
 	spin_unlock_irqrestore(&sched->job_list_lock, flags);
+	sched->ops->free_job(s_job);
+}
+
+static void amd_sched_job_finish_cb(struct fence *f, struct fence_cb *cb)
+{
+	struct amd_sched_job *job = container_of(cb, struct amd_sched_job,
+						 finish_cb);
+	schedule_work(&job->finish_work);
 }
 
 static void amd_sched_job_begin(struct amd_sched_job *s_job)
@@ -364,10 +365,7 @@ static void amd_sched_job_begin(struct amd_sched_job *s_job)
 	if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
 	    list_first_entry_or_null(&sched->ring_mirror_list,
 				     struct amd_sched_job, node) == s_job)
-	{
-		amd_sched_job_get(s_job);
 		schedule_delayed_work(&s_job->work_tdr, sched->timeout);
-	}
 	spin_unlock_irqrestore(&sched->job_list_lock, flags);
 }
 
@@ -390,9 +388,9 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job)
 {
 	struct amd_sched_entity *entity = sched_job->s_entity;
 
-	fence_add_callback(&sched_job->s_fence->base,
-			   &sched_job->cb_free_job, amd_sched_free_job);
 	trace_amd_sched_job(sched_job);
+	fence_add_callback(&sched_job->s_fence->base, &sched_job->finish_cb,
+			   amd_sched_job_finish_cb);
 	wait_event(entity->sched->job_scheduled,
 		   amd_sched_entity_in(sched_job));
 }
@@ -401,20 +399,17 @@ void amd_sched_entity_push_job(struct amd_sched_job *sched_job)
 int amd_sched_job_init(struct amd_sched_job *job,
 		       struct amd_gpu_scheduler *sched,
 		       struct amd_sched_entity *entity,
-		       void (*free_cb)(struct kref *refcount),
 		       void *owner, struct fence **fence)
 {
-	INIT_LIST_HEAD(&job->node);
-	kref_init(&job->refcount);
 	job->sched = sched;
 	job->s_entity = entity;
 	job->s_fence = amd_sched_fence_create(entity, owner);
 	if (!job->s_fence)
 		return -ENOMEM;
 
-	job->s_fence->s_job = job;
+	INIT_WORK(&job->finish_work, amd_sched_job_finish);
+	INIT_LIST_HEAD(&job->node);
 	INIT_DELAYED_WORK(&job->work_tdr, amd_sched_job_timedout);
-	job->free_callback = free_cb;
 
 	if (fence)
 		*fence = &job->s_fence->base;
@@ -468,9 +463,6 @@ static void amd_sched_process_job(struct fence *f, struct fence_cb *cb)
 	struct amd_gpu_scheduler *sched = s_fence->sched;
 
 	atomic_dec(&sched->hw_rq_count);
-
-	amd_sched_job_finish(s_fence->s_job);
-
 	amd_sched_fence_signal(s_fence);
 
 	trace_amd_sched_process_job(s_fence);
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index f0de46c..e63034e 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -74,19 +74,16 @@ struct amd_sched_fence {
 	struct amd_gpu_scheduler	*sched;
 	spinlock_t			lock;
 	void                            *owner;
-	struct amd_sched_job	*s_job;
 };
 
 struct amd_sched_job {
-	struct kref refcount;
 	struct amd_gpu_scheduler        *sched;
 	struct amd_sched_entity         *s_entity;
 	struct amd_sched_fence          *s_fence;
-	struct fence_cb                cb_free_job;
-	struct work_struct             work_free_job;
-	struct list_head			   node;
-	struct delayed_work work_tdr;
-	void (*free_callback)(struct kref *refcount);
+	struct fence_cb			finish_cb;
+	struct work_struct		finish_work;
+	struct list_head		node;
+	struct delayed_work		work_tdr;
 };
 
 extern const struct fence_ops amd_sched_fence_ops;
@@ -109,6 +106,7 @@ struct amd_sched_backend_ops {
 	struct fence *(*dependency)(struct amd_sched_job *sched_job);
 	struct fence *(*run_job)(struct amd_sched_job *sched_job);
 	void (*timedout_job)(struct amd_sched_job *sched_job);
+	void (*free_job)(struct amd_sched_job *sched_job);
 };
 
 enum amd_sched_priority {
@@ -154,18 +152,5 @@ void amd_sched_fence_signal(struct amd_sched_fence *fence);
 int amd_sched_job_init(struct amd_sched_job *job,
 		       struct amd_gpu_scheduler *sched,
 		       struct amd_sched_entity *entity,
-		       void (*free_cb)(struct kref* refcount),
 		       void *owner, struct fence **fence);
-static inline void amd_sched_job_get(struct amd_sched_job *job)
-{
-	if (job)
-		kref_get(&job->refcount);
-}
-
-static inline void amd_sched_job_put(struct amd_sched_job *job)
-{
-	if (job)
-		kref_put(&job->refcount, job->free_callback);
-}
-
 #endif
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/8] scheduler code cleanup
  2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
                   ` (7 preceding siblings ...)
  2016-05-25 21:04 ` [PATCH 8/8] drm/amdgpu: fix and cleanup job destruction Alex Deucher
@ 2016-05-26  8:09 ` Daniel Vetter
  8 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-05-26  8:09 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Alex Deucher, dri-devel

On Wed, May 25, 2016 at 05:04:14PM -0400, Alex Deucher wrote:
> Just some general cleanup in the GPU scheduler.
> 
> Christian König (8):

Since when has Christian forgot how git send-email works?

;-)

Cheers, Daniel

>   drm/amdgpu: fix coding style in the scheduler v2
>   drm/amdgpu: remove begin_job/finish_job
>   drm/amdgpu: remove duplicated timeout callback
>   drm/amdgpu: fix coding style in amdgpu_job_free
>   drm/amdgpu: remove use_shed hack in job cleanup
>   drm/amdgpu: properly abstract scheduler timeout handling
>   drm/amdgpu: move locking into the functions who need it
>   drm/amdgpu: fix and cleanup job destruction
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  2 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |  4 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       | 49 +++++++--------
>  drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 89 ++++++++++++++-------------
>  drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 42 ++++---------
>  drivers/gpu/drm/amd/scheduler/sched_fence.c   | 19 ++----
>  6 files changed, 86 insertions(+), 119 deletions(-)
> 
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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] 10+ messages in thread

end of thread, other threads:[~2016-05-26  8:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-25 21:04 [PATCH 0/8] scheduler code cleanup Alex Deucher
2016-05-25 21:04 ` [PATCH 1/8] drm/amdgpu: fix coding style in the scheduler v2 Alex Deucher
2016-05-25 21:04 ` [PATCH 2/8] drm/amdgpu: remove begin_job/finish_job Alex Deucher
2016-05-25 21:04 ` [PATCH 3/8] drm/amdgpu: remove duplicated timeout callback Alex Deucher
2016-05-25 21:04 ` [PATCH 4/8] drm/amdgpu: fix coding style in amdgpu_job_free Alex Deucher
2016-05-25 21:04 ` [PATCH 5/8] drm/amdgpu: remove use_shed hack in job cleanup Alex Deucher
2016-05-25 21:04 ` [PATCH 6/8] drm/amdgpu: properly abstract scheduler timeout handling Alex Deucher
2016-05-25 21:04 ` [PATCH 7/8] drm/amdgpu: move locking into the functions who need it Alex Deucher
2016-05-25 21:04 ` [PATCH 8/8] drm/amdgpu: fix and cleanup job destruction Alex Deucher
2016-05-26  8:09 ` [PATCH 0/8] scheduler code cleanup Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox