All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.