[PATCH 0/3] drm/scheduler: preparation for load balancing

[PATCH 0/3] drm/scheduler: preparation for load balancing

[Nayan Deshmukh]

This patch series is prepration for implementing better load balancing
in the GPU scheduler. Patch #3 is the major change which modifies the 
drm_sched_entity_init, the driver is now expected to provide a list of
potential run queue on which the jobs from this entity can be scheduled.

In future patches we will add functionality to scheduler to select the 
run queue which has the least amount of load. To avoid making
significant changes to multiple drivers in the same patch I am not yet
sending a list to drm_sched_entity_init. I will make seprate patches for
each driver for that.

Regards,
Nayan Deshmukh

Nayan Deshmukh (3):
  drm/scheduler: add a pointer to scheduler in the rq
  drm/scheduler: add counter for total jobs in scheduler
  drm/scheduler: modify args of drm_sched_entity_init

 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c     |  4 ++--
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c     |  4 ++--
 drivers/gpu/drm/etnaviv/etnaviv_drv.c     |  8 ++++----
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 31 +++++++++++++++++++++----------
 drivers/gpu/drm/v3d/v3d_drv.c             |  7 +++----
 include/drm/gpu_scheduler.h               | 17 +++++++++++++----
 11 files changed, 55 insertions(+), 36 deletions(-)

-- 
2.14.3

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

[PATCH 1/3] drm/scheduler: add a pointer to scheduler in the rq

[Nayan Deshmukh]

Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 6 ++++--
 include/drm/gpu_scheduler.h               | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 7d2560699b84..429b1328653a 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -69,11 +69,13 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb);
  *
  * Initializes a scheduler runqueue.
  */
-static void drm_sched_rq_init(struct drm_sched_rq *rq)
+static void drm_sched_rq_init(struct drm_gpu_scheduler *sched,
+			      struct drm_sched_rq *rq)
 {
 	spin_lock_init(&rq->lock);
 	INIT_LIST_HEAD(&rq->entities);
 	rq->current_entity = NULL;
+	rq->sched = sched;
 }
 
 /**
@@ -926,7 +928,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
 	sched->timeout = timeout;
 	sched->hang_limit = hang_limit;
 	for (i = DRM_SCHED_PRIORITY_MIN; i < DRM_SCHED_PRIORITY_MAX; i++)
-		drm_sched_rq_init(&sched->sched_rq[i]);
+		drm_sched_rq_init(sched, &sched->sched_rq[i]);
 
 	init_waitqueue_head(&sched->wake_up_worker);
 	init_waitqueue_head(&sched->job_scheduled);
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 4214ceb71c05..43e93d6077cf 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -93,6 +93,7 @@ struct drm_sched_entity {
  * struct drm_sched_rq - queue of entities to be scheduled.
  *
  * @lock: to modify the entities list.
+ * @sched: the scheduler to which this rq belongs to.
  * @entities: list of the entities to be scheduled.
  * @current_entity: the entity which is to be scheduled.
  *
@@ -102,6 +103,7 @@ struct drm_sched_entity {
  */
 struct drm_sched_rq {
 	spinlock_t			lock;
+	struct drm_gpu_scheduler	*sched;
 	struct list_head		entities;
 	struct drm_sched_entity		*current_entity;
 };
-- 
2.14.3

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

[PATCH 2/3] drm/scheduler: add counter for total jobs in scheduler

[Nayan Deshmukh]

Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 +++
 include/drm/gpu_scheduler.h               | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 429b1328653a..3dc1a4f07e3f 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -538,6 +538,7 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
 	trace_drm_sched_job(sched_job, entity);
 
 	first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
+	atomic_inc(&entity->sched->num_jobs);
 
 	/* first job wakes up scheduler */
 	if (first) {
@@ -818,6 +819,7 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb)
 
 	dma_fence_get(&s_fence->finished);
 	atomic_dec(&sched->hw_rq_count);
+	atomic_dec(&sched->num_jobs);
 	drm_sched_fence_finished(s_fence);
 
 	trace_drm_sched_process_job(s_fence);
@@ -935,6 +937,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
 	INIT_LIST_HEAD(&sched->ring_mirror_list);
 	spin_lock_init(&sched->job_list_lock);
 	atomic_set(&sched->hw_rq_count, 0);
+	atomic_set(&sched->num_jobs, 0);
 	atomic64_set(&sched->job_id_count, 0);
 
 	/* Each scheduler will run on a seperate kernel thread */
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 43e93d6077cf..605bd4ad2397 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -257,6 +257,7 @@ struct drm_sched_backend_ops {
  * @job_list_lock: lock to protect the ring_mirror_list.
  * @hang_limit: once the hangs by a job crosses this limit then it is marked
  *              guilty and it will be considered for scheduling further.
+ * @num_jobs: the number of jobs in queue in the scheduler
  *
  * One scheduler is implemented for each hardware ring.
  */
@@ -274,6 +275,7 @@ struct drm_gpu_scheduler {
 	struct list_head		ring_mirror_list;
 	spinlock_t			job_list_lock;
 	int				hang_limit;
+	atomic_t                        num_jobs;
 };
 
 int drm_sched_init(struct drm_gpu_scheduler *sched,
-- 
2.14.3

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

[PATCH 3/3] drm/scheduler: modify args of drm_sched_entity_init

[Nayan Deshmukh]

replace run queue by a list of run queues and remove the
sched arg as that is part of run queue itself

Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c     |  4 ++--
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c     |  4 ++--
 drivers/gpu/drm/etnaviv/etnaviv_drv.c     |  8 ++++----
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 22 ++++++++++++++--------
 drivers/gpu/drm/v3d/v3d_drv.c             |  7 +++----
 include/drm/gpu_scheduler.h               | 13 +++++++++----
 11 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 0120b24fae1b..83e3b320a793 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -90,8 +90,8 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
 		if (ring == &adev->gfx.kiq.ring)
 			continue;
 
-		r = drm_sched_entity_init(&ring->sched, &ctx->rings[i].entity,
-					  rq, &ctx->guilty);
+		r = drm_sched_entity_init(&ctx->rings[i].entity,
+					  &rq, 1, &ctx->guilty);
 		if (r)
 			goto failed;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 0246cb87d9e4..c937f6755f55 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -140,8 +140,8 @@ static int amdgpu_ttm_global_init(struct amdgpu_device *adev)
 
 	ring = adev->mman.buffer_funcs_ring;
 	rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_KERNEL];
-	r = drm_sched_entity_init(&ring->sched, &adev->mman.entity,
-				  rq, NULL);
+	r = drm_sched_entity_init(&adev->mman.entity,
+				  &rq, 1, NULL);
 	if (r) {
 		DRM_ERROR("Failed setting up TTM BO move run queue.\n");
 		goto error_entity;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index 0b46ea1c6290..1471a8c3ce24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -266,8 +266,8 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
 
 		ring = &adev->uvd.inst[j].ring;
 		rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
-		r = drm_sched_entity_init(&ring->sched, &adev->uvd.inst[j].entity,
-					  rq, NULL);
+		r = drm_sched_entity_init(&adev->uvd.inst[j].entity,
+					  &rq, 1, NULL);
 		if (r != 0) {
 			DRM_ERROR("Failed setting up UVD(%d) run queue.\n", j);
 			return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index b0dcdfd85f5b..62b2f0816695 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -190,8 +190,8 @@ int amdgpu_vce_sw_init(struct amdgpu_device *adev, unsigned long size)
 
 	ring = &adev->vce.ring[0];
 	rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
-	r = drm_sched_entity_init(&ring->sched, &adev->vce.entity,
-				  rq, NULL);
+	r = drm_sched_entity_init(&adev->vce.entity,
+				  &rq, 1, NULL);
 	if (r != 0) {
 		DRM_ERROR("Failed setting up VCE run queue.\n");
 		return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 712af5c1a5d6..00cb46965237 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2562,8 +2562,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	ring_instance %= adev->vm_manager.vm_pte_num_rings;
 	ring = adev->vm_manager.vm_pte_rings[ring_instance];
 	rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_KERNEL];
-	r = drm_sched_entity_init(&ring->sched, &vm->entity,
-				  rq, NULL);
+	r = drm_sched_entity_init(&vm->entity, &rq,
+				  1, NULL);
 	if (r)
 		return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index 8ee1c2eaaa14..da38aa2f3d13 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -429,8 +429,8 @@ static int uvd_v6_0_sw_init(void *handle)
 		struct drm_sched_rq *rq;
 		ring = &adev->uvd.inst->ring_enc[0];
 		rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
-		r = drm_sched_entity_init(&ring->sched, &adev->uvd.inst->entity_enc,
-					  rq, NULL);
+		r = drm_sched_entity_init(&adev->uvd.inst->entity_enc,
+					  &rq, 1, NULL);
 		if (r) {
 			DRM_ERROR("Failed setting up UVD ENC run queue.\n");
 			return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
index ba244d3b74db..69221430aa38 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
@@ -431,8 +431,8 @@ static int uvd_v7_0_sw_init(void *handle)
 	for (j = 0; j < adev->uvd.num_uvd_inst; j++) {
 		ring = &adev->uvd.inst[j].ring_enc[0];
 		rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
-		r = drm_sched_entity_init(&ring->sched, &adev->uvd.inst[j].entity_enc,
-					  rq, NULL);
+		r = drm_sched_entity_init(&adev->uvd.inst[j].entity_enc,
+					  &rq, 1, NULL);
 		if (r) {
 			DRM_ERROR("(%d)Failed setting up UVD ENC run queue.\n", j);
 			return r;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 45bfdf4cc107..121c53e04603 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -49,12 +49,12 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
 
 	for (i = 0; i < ETNA_MAX_PIPES; i++) {
 		struct etnaviv_gpu *gpu = priv->gpu[i];
+		struct drm_sched_rq *rq;
 
+		rq = &gpu->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
 		if (gpu) {
-			drm_sched_entity_init(&gpu->sched,
-				&ctx->sched_entity[i],
-				&gpu->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL],
-				NULL);
+			drm_sched_entity_init(&ctx->sched_entity[i],
+					      &rq, 1, NULL);
 			}
 	}
 
diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 3dc1a4f07e3f..b2dbd1c1ba69 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -162,26 +162,32 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
  * drm_sched_entity_init - Init a context entity used by scheduler when
  * submit to HW ring.
  *
- * @sched: scheduler instance
  * @entity: scheduler entity to init
- * @rq: the run queue this entity belongs
+ * @rq_list: the list of run queue on which jobs from this
+ *           entity can be submitted
+ * @num_rq_list: number of run queue in rq_list
  * @guilty: atomic_t set to 1 when a job on this queue
  *          is found to be guilty causing a timeout
  *
+ * Note: the rq_list should have atleast one element to schedule
+ *       the entity
+ *
  * Returns 0 on success or a negative error code on failure.
 */
-int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
-			  struct drm_sched_entity *entity,
-			  struct drm_sched_rq *rq,
+int drm_sched_entity_init(struct drm_sched_entity *entity,
+			  struct drm_sched_rq **rq_list,
+			  unsigned int num_rq_list,
 			  atomic_t *guilty)
 {
-	if (!(sched && entity && rq))
+	if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
 		return -EINVAL;
 
 	memset(entity, 0, sizeof(struct drm_sched_entity));
 	INIT_LIST_HEAD(&entity->list);
-	entity->rq = rq;
-	entity->sched = sched;
+	entity->rq_list = NULL;
+	entity->rq = rq_list[0];
+	entity->sched = rq_list[0]->sched;
+	entity->num_rq_list = num_rq_list;
 	entity->guilty = guilty;
 	entity->last_scheduled = NULL;
 
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 567f7d46d912..1dceba2b42fd 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -123,6 +123,7 @@ v3d_open(struct drm_device *dev, struct drm_file *file)
 {
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 	struct v3d_file_priv *v3d_priv;
+	struct drm_sched_rq *rq;
 	int i;
 
 	v3d_priv = kzalloc(sizeof(*v3d_priv), GFP_KERNEL);
@@ -132,10 +133,8 @@ v3d_open(struct drm_device *dev, struct drm_file *file)
 	v3d_priv->v3d = v3d;
 
 	for (i = 0; i < V3D_MAX_QUEUES; i++) {
-		drm_sched_entity_init(&v3d->queue[i].sched,
-				      &v3d_priv->sched_entity[i],
-				      &v3d->queue[i].sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL],
-				      NULL);
+		rq = &v3d->queue[i].sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
+		drm_sched_entity_init(&v3d_priv->sched_entity[i], &rq, 1, NULL);
 	}
 
 	file->driver_priv = v3d_priv;
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 605bd4ad2397..2b5e152d45fc 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -50,7 +50,10 @@ enum drm_sched_priority {
  *
  * @list: used to append this struct to the list of entities in the
  *        runqueue.
- * @rq: runqueue to which this entity belongs.
+ * @rq: runqueue on which this entity is currently scheduled.
+ * @rq_list: a list of run queues on which jobs from this entity can 
+ *           be scheduled
+ * @num_rq_list: number of run queues in the rq_list
  * @rq_lock: lock to modify the runqueue to which this entity belongs.
  * @sched: the scheduler instance to which this entity is enqueued.
  * @job_queue: the list of jobs of this entity.
@@ -75,6 +78,8 @@ enum drm_sched_priority {
 struct drm_sched_entity {
 	struct list_head		list;
 	struct drm_sched_rq		*rq;
+	struct drm_sched_rq		**rq_list;
+	unsigned int                    num_rq_list;
 	spinlock_t			rq_lock;
 	struct drm_gpu_scheduler	*sched;
 
@@ -284,9 +289,9 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
 		   const char *name);
 void drm_sched_fini(struct drm_gpu_scheduler *sched);
 
-int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
-			  struct drm_sched_entity *entity,
-			  struct drm_sched_rq *rq,
+int drm_sched_entity_init(struct drm_sched_entity *entity,
+			  struct drm_sched_rq **rq_list,
+			  unsigned int num_rq_list,
 			  atomic_t *guilty);
 long drm_sched_entity_flush(struct drm_gpu_scheduler *sched,
 			   struct drm_sched_entity *entity, long timeout);
-- 
2.14.3

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

Re: [PATCH 3/3] drm/scheduler: modify args of drm_sched_entity_init

[Nayan Deshmukh]

On Thu, Jul 12, 2018 at 12:07 PM Nayan Deshmukh
<nayan26deshmukh@gmail.com> wrote:
>
> replace run queue by a list of run queues and remove the
> sched arg as that is part of run queue itself
>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c   |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c   |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c   |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c     |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c     |  4 ++--
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c     |  8 ++++----
>  drivers/gpu/drm/scheduler/gpu_scheduler.c | 22 ++++++++++++++--------
>  drivers/gpu/drm/v3d/v3d_drv.c             |  7 +++----
>  include/drm/gpu_scheduler.h               | 13 +++++++++----
>  11 files changed, 44 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 0120b24fae1b..83e3b320a793 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -90,8 +90,8 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
>                 if (ring == &adev->gfx.kiq.ring)
>                         continue;
>
> -               r = drm_sched_entity_init(&ring->sched, &ctx->rings[i].entity,
> -                                         rq, &ctx->guilty);
> +               r = drm_sched_entity_init(&ctx->rings[i].entity,
> +                                         &rq, 1, &ctx->guilty);
>                 if (r)
>                         goto failed;
>         }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 0246cb87d9e4..c937f6755f55 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -140,8 +140,8 @@ static int amdgpu_ttm_global_init(struct amdgpu_device *adev)
>
>         ring = adev->mman.buffer_funcs_ring;
>         rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_KERNEL];
> -       r = drm_sched_entity_init(&ring->sched, &adev->mman.entity,
> -                                 rq, NULL);
> +       r = drm_sched_entity_init(&adev->mman.entity,
> +                                 &rq, 1, NULL);
>         if (r) {
>                 DRM_ERROR("Failed setting up TTM BO move run queue.\n");
>                 goto error_entity;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index 0b46ea1c6290..1471a8c3ce24 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -266,8 +266,8 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
>
>                 ring = &adev->uvd.inst[j].ring;
>                 rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
> -               r = drm_sched_entity_init(&ring->sched, &adev->uvd.inst[j].entity,
> -                                         rq, NULL);
> +               r = drm_sched_entity_init(&adev->uvd.inst[j].entity,
> +                                         &rq, 1, NULL);
>                 if (r != 0) {
>                         DRM_ERROR("Failed setting up UVD(%d) run queue.\n", j);
>                         return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index b0dcdfd85f5b..62b2f0816695 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -190,8 +190,8 @@ int amdgpu_vce_sw_init(struct amdgpu_device *adev, unsigned long size)
>
>         ring = &adev->vce.ring[0];
>         rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
> -       r = drm_sched_entity_init(&ring->sched, &adev->vce.entity,
> -                                 rq, NULL);
> +       r = drm_sched_entity_init(&adev->vce.entity,
> +                                 &rq, 1, NULL);
>         if (r != 0) {
>                 DRM_ERROR("Failed setting up VCE run queue.\n");
>                 return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 712af5c1a5d6..00cb46965237 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2562,8 +2562,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>         ring_instance %= adev->vm_manager.vm_pte_num_rings;
>         ring = adev->vm_manager.vm_pte_rings[ring_instance];
>         rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_KERNEL];
> -       r = drm_sched_entity_init(&ring->sched, &vm->entity,
> -                                 rq, NULL);
> +       r = drm_sched_entity_init(&vm->entity, &rq,
> +                                 1, NULL);
>         if (r)
>                 return r;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 8ee1c2eaaa14..da38aa2f3d13 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -429,8 +429,8 @@ static int uvd_v6_0_sw_init(void *handle)
>                 struct drm_sched_rq *rq;
>                 ring = &adev->uvd.inst->ring_enc[0];
>                 rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
> -               r = drm_sched_entity_init(&ring->sched, &adev->uvd.inst->entity_enc,
> -                                         rq, NULL);
> +               r = drm_sched_entity_init(&adev->uvd.inst->entity_enc,
> +                                         &rq, 1, NULL);
>                 if (r) {
>                         DRM_ERROR("Failed setting up UVD ENC run queue.\n");
>                         return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index ba244d3b74db..69221430aa38 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -431,8 +431,8 @@ static int uvd_v7_0_sw_init(void *handle)
>         for (j = 0; j < adev->uvd.num_uvd_inst; j++) {
>                 ring = &adev->uvd.inst[j].ring_enc[0];
>                 rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
> -               r = drm_sched_entity_init(&ring->sched, &adev->uvd.inst[j].entity_enc,
> -                                         rq, NULL);
> +               r = drm_sched_entity_init(&adev->uvd.inst[j].entity_enc,
> +                                         &rq, 1, NULL);
>                 if (r) {
>                         DRM_ERROR("(%d)Failed setting up UVD ENC run queue.\n", j);
>                         return r;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 45bfdf4cc107..121c53e04603 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -49,12 +49,12 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
>
>         for (i = 0; i < ETNA_MAX_PIPES; i++) {
>                 struct etnaviv_gpu *gpu = priv->gpu[i];
> +               struct drm_sched_rq *rq;
>
> +               rq = &gpu->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
>                 if (gpu) {
> -                       drm_sched_entity_init(&gpu->sched,
> -                               &ctx->sched_entity[i],
> -                               &gpu->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL],
> -                               NULL);
> +                       drm_sched_entity_init(&ctx->sched_entity[i],
> +                                             &rq, 1, NULL);
>                         }
>         }
>
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> index 3dc1a4f07e3f..b2dbd1c1ba69 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> @@ -162,26 +162,32 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
>   * drm_sched_entity_init - Init a context entity used by scheduler when
>   * submit to HW ring.
>   *
> - * @sched: scheduler instance
>   * @entity: scheduler entity to init
> - * @rq: the run queue this entity belongs
> + * @rq_list: the list of run queue on which jobs from this
> + *           entity can be submitted
> + * @num_rq_list: number of run queue in rq_list
>   * @guilty: atomic_t set to 1 when a job on this queue
>   *          is found to be guilty causing a timeout
>   *
> + * Note: the rq_list should have atleast one element to schedule
> + *       the entity
> + *
>   * Returns 0 on success or a negative error code on failure.
>  */
> -int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
> -                         struct drm_sched_entity *entity,
> -                         struct drm_sched_rq *rq,
> +int drm_sched_entity_init(struct drm_sched_entity *entity,
> +                         struct drm_sched_rq **rq_list,
> +                         unsigned int num_rq_list,
>                           atomic_t *guilty)
>  {
> -       if (!(sched && entity && rq))
> +       if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
>                 return -EINVAL;
>
>         memset(entity, 0, sizeof(struct drm_sched_entity));
>         INIT_LIST_HEAD(&entity->list);
> -       entity->rq = rq;
> -       entity->sched = sched;
> +       entity->rq_list = NULL;
> +       entity->rq = rq_list[0];
> +       entity->sched = rq_list[0]->sched;
> +       entity->num_rq_list = num_rq_list;
>         entity->guilty = guilty;
>         entity->last_scheduled = NULL;
>
> diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
> index 567f7d46d912..1dceba2b42fd 100644
> --- a/drivers/gpu/drm/v3d/v3d_drv.c
> +++ b/drivers/gpu/drm/v3d/v3d_drv.c
> @@ -123,6 +123,7 @@ v3d_open(struct drm_device *dev, struct drm_file *file)
>  {
>         struct v3d_dev *v3d = to_v3d_dev(dev);
>         struct v3d_file_priv *v3d_priv;
> +       struct drm_sched_rq *rq;
>         int i;
>
>         v3d_priv = kzalloc(sizeof(*v3d_priv), GFP_KERNEL);
> @@ -132,10 +133,8 @@ v3d_open(struct drm_device *dev, struct drm_file *file)
>         v3d_priv->v3d = v3d;
>
>         for (i = 0; i < V3D_MAX_QUEUES; i++) {
> -               drm_sched_entity_init(&v3d->queue[i].sched,
> -                                     &v3d_priv->sched_entity[i],
> -                                     &v3d->queue[i].sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL],
> -                                     NULL);
> +               rq = &v3d->queue[i].sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
> +               drm_sched_entity_init(&v3d_priv->sched_entity[i], &rq, 1, NULL);
>         }
>
>         file->driver_priv = v3d_priv;
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 605bd4ad2397..2b5e152d45fc 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -50,7 +50,10 @@ enum drm_sched_priority {
>   *
>   * @list: used to append this struct to the list of entities in the
>   *        runqueue.
> - * @rq: runqueue to which this entity belongs.
> + * @rq: runqueue on which this entity is currently scheduled.
> + * @rq_list: a list of run queues on which jobs from this entity can
Added a whitespace by mistake will remove it in the final patch         ^^
> + *           be scheduled
> + * @num_rq_list: number of run queues in the rq_list
>   * @rq_lock: lock to modify the runqueue to which this entity belongs.
>   * @sched: the scheduler instance to which this entity is enqueued.
>   * @job_queue: the list of jobs of this entity.
> @@ -75,6 +78,8 @@ enum drm_sched_priority {
>  struct drm_sched_entity {
>         struct list_head                list;
>         struct drm_sched_rq             *rq;
> +       struct drm_sched_rq             **rq_list;
> +       unsigned int                    num_rq_list;
>         spinlock_t                      rq_lock;
>         struct drm_gpu_scheduler        *sched;
>
> @@ -284,9 +289,9 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
>                    const char *name);
>  void drm_sched_fini(struct drm_gpu_scheduler *sched);
>
> -int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
> -                         struct drm_sched_entity *entity,
> -                         struct drm_sched_rq *rq,
> +int drm_sched_entity_init(struct drm_sched_entity *entity,
> +                         struct drm_sched_rq **rq_list,
> +                         unsigned int num_rq_list,
>                           atomic_t *guilty);
>  long drm_sched_entity_flush(struct drm_gpu_scheduler *sched,
>                            struct drm_sched_entity *entity, long timeout);
> --
> 2.14.3
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/scheduler: preparation for load balancing

[Christian König]

Only a few more style nit picks:

Patches #1 and #2 need a commit message. A one liner why we do this 
should be fine.

On Patch #3 you have a couple of places like this:
> -	r = drm_sched_entity_init(&ring->sched, &adev->mman.entity,
> -				  rq, NULL);
> +	r = drm_sched_entity_init(&adev->mman.entity,
> +				  &rq, 1, NULL);
If I'm not completely mistaken that could be written on one line now and 
still be shorter than 80 characters.

Those are not major issues, so patches are Reviewed-by: Christian König 
<christian.koenig@amd.com> either way.

If neither Lucas nor Eric object I'm going to pick them those patches up 
on Friday.

Thanks for the help,
Christian.

Am 12.07.2018 um 08:36 schrieb Nayan Deshmukh:
> This patch series is prepration for implementing better load balancing
> in the GPU scheduler. Patch #3 is the major change which modifies the
> drm_sched_entity_init, the driver is now expected to provide a list of
> potential run queue on which the jobs from this entity can be scheduled.
>
> In future patches we will add functionality to scheduler to select the
> run queue which has the least amount of load. To avoid making
> significant changes to multiple drivers in the same patch I am not yet
> sending a list to drm_sched_entity_init. I will make seprate patches for
> each driver for that.
>
> Regards,
> Nayan Deshmukh
>
> Nayan Deshmukh (3):
>    drm/scheduler: add a pointer to scheduler in the rq
>    drm/scheduler: add counter for total jobs in scheduler
>    drm/scheduler: modify args of drm_sched_entity_init
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c   |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c   |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c   |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c     |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c     |  4 ++--
>   drivers/gpu/drm/etnaviv/etnaviv_drv.c     |  8 ++++----
>   drivers/gpu/drm/scheduler/gpu_scheduler.c | 31 +++++++++++++++++++++----------
>   drivers/gpu/drm/v3d/v3d_drv.c             |  7 +++----
>   include/drm/gpu_scheduler.h               | 17 +++++++++++++----
>   11 files changed, 55 insertions(+), 36 deletions(-)
>

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

Re: [PATCH 1/3] drm/scheduler: add a pointer to scheduler in the rq

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 248 bytes --]

Nayan Deshmukh <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> Signed-off-by: Nayan Deshmukh <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---

Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 3/3] drm/scheduler: modify args of drm_sched_entity_init

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 2211 bytes --]

Nayan Deshmukh <nayan26deshmukh@gmail.com> writes:
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> index 3dc1a4f07e3f..b2dbd1c1ba69 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> @@ -162,26 +162,32 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
>   * drm_sched_entity_init - Init a context entity used by scheduler when
>   * submit to HW ring.
>   *
> - * @sched: scheduler instance
>   * @entity: scheduler entity to init
> - * @rq: the run queue this entity belongs
> + * @rq_list: the list of run queue on which jobs from this
> + *           entity can be submitted
> + * @num_rq_list: number of run queue in rq_list
>   * @guilty: atomic_t set to 1 when a job on this queue
>   *          is found to be guilty causing a timeout
>   *
> + * Note: the rq_list should have atleast one element to schedule
> + *       the entity
> + *
>   * Returns 0 on success or a negative error code on failure.
>  */
> -int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
> -			  struct drm_sched_entity *entity,
> -			  struct drm_sched_rq *rq,
> +int drm_sched_entity_init(struct drm_sched_entity *entity,
> +			  struct drm_sched_rq **rq_list,
> +			  unsigned int num_rq_list,
>  			  atomic_t *guilty)
>  {
> -	if (!(sched && entity && rq))
> +	if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
>  		return -EINVAL;
>  
>  	memset(entity, 0, sizeof(struct drm_sched_entity));
>  	INIT_LIST_HEAD(&entity->list);
> -	entity->rq = rq;
> -	entity->sched = sched;
> +	entity->rq_list = NULL;
> +	entity->rq = rq_list[0];
> +	entity->sched = rq_list[0]->sched;
> +	entity->num_rq_list = num_rq_list;

The API change makes sense as prep work, but I don't really like adding
the field to the struct (and changing the struct's docs for the existing
rq field) if it's going to always be NULL until a future change.

Similarly, I'd rather see patch 2 as part of a series that uses the
value.

That said, while I don't currently have a usecase for load-balancing
between entities, I may in the future, so thanks for working on this!

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 3/3] drm/scheduler: modify args of drm_sched_entity_init

[Christian König]

Am 12.07.2018 um 08:36 schrieb Nayan Deshmukh:
> replace run queue by a list of run queues and remove the
> sched arg as that is part of run queue itself
>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>

Going over this I've found one more thing which needs to be fixed:

[SNIP]
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 45bfdf4cc107..121c53e04603 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -49,12 +49,12 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
>   
>   	for (i = 0; i < ETNA_MAX_PIPES; i++) {
>   		struct etnaviv_gpu *gpu = priv->gpu[i];
> +		struct drm_sched_rq *rq;
>   
> +		rq = &gpu->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
>   		if (gpu) {

Here you use gpu right before the NULL check. It's only an address 
calculation, but still a bit ugly.

Just move the assignment under the "if (gpu) {".

> -			drm_sched_entity_init(&gpu->sched,
> -				&ctx->sched_entity[i],
> -				&gpu->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL],
> -				NULL);
> +			drm_sched_entity_init(&ctx->sched_entity[i],
> +					      &rq, 1, NULL);

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

Re: [PATCH 3/3] drm/scheduler: modify args of drm_sched_entity_init

[Nayan Deshmukh]

On Thu, Jul 12, 2018 at 11:21 PM Eric Anholt <eric@anholt.net> wrote:
>
> Nayan Deshmukh <nayan26deshmukh@gmail.com> writes:
> > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > index 3dc1a4f07e3f..b2dbd1c1ba69 100644
> > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > @@ -162,26 +162,32 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
> >   * drm_sched_entity_init - Init a context entity used by scheduler when
> >   * submit to HW ring.
> >   *
> > - * @sched: scheduler instance
> >   * @entity: scheduler entity to init
> > - * @rq: the run queue this entity belongs
> > + * @rq_list: the list of run queue on which jobs from this
> > + *           entity can be submitted
> > + * @num_rq_list: number of run queue in rq_list
> >   * @guilty: atomic_t set to 1 when a job on this queue
> >   *          is found to be guilty causing a timeout
> >   *
> > + * Note: the rq_list should have atleast one element to schedule
> > + *       the entity
> > + *
> >   * Returns 0 on success or a negative error code on failure.
> >  */
> > -int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
> > -                       struct drm_sched_entity *entity,
> > -                       struct drm_sched_rq *rq,
> > +int drm_sched_entity_init(struct drm_sched_entity *entity,
> > +                       struct drm_sched_rq **rq_list,
> > +                       unsigned int num_rq_list,
> >                         atomic_t *guilty)
> >  {
> > -     if (!(sched && entity && rq))
> > +     if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
> >               return -EINVAL;
> >
> >       memset(entity, 0, sizeof(struct drm_sched_entity));
> >       INIT_LIST_HEAD(&entity->list);
> > -     entity->rq = rq;
> > -     entity->sched = sched;
> > +     entity->rq_list = NULL;
> > +     entity->rq = rq_list[0];
> > +     entity->sched = rq_list[0]->sched;
> > +     entity->num_rq_list = num_rq_list;
>
> The API change makes sense as prep work, but I don't really like adding
> the field to the struct (and changing the struct's docs for the existing
> rq field) if it's going to always be NULL until a future change.
>
> Similarly, I'd rather see patch 2 as part of a series that uses the
> value.
I agree with you. I am fine with dropping the patch 2 for now and
modifying patch 3. I am fine either way.

What are your thoughts on this Christian?
>
> That said, while I don't currently have a usecase for load-balancing
> between entities, I may in the future, so thanks for working on this!
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/scheduler: modify args of drm_sched_entity_init

[Christian König]

Am 13.07.2018 um 09:20 schrieb Nayan Deshmukh:
> On Thu, Jul 12, 2018 at 11:21 PM Eric Anholt <eric@anholt.net> wrote:
>> Nayan Deshmukh <nayan26deshmukh@gmail.com> writes:
>>> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>> index 3dc1a4f07e3f..b2dbd1c1ba69 100644
>>> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>> @@ -162,26 +162,32 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
>>>    * drm_sched_entity_init - Init a context entity used by scheduler when
>>>    * submit to HW ring.
>>>    *
>>> - * @sched: scheduler instance
>>>    * @entity: scheduler entity to init
>>> - * @rq: the run queue this entity belongs
>>> + * @rq_list: the list of run queue on which jobs from this
>>> + *           entity can be submitted
>>> + * @num_rq_list: number of run queue in rq_list
>>>    * @guilty: atomic_t set to 1 when a job on this queue
>>>    *          is found to be guilty causing a timeout
>>>    *
>>> + * Note: the rq_list should have atleast one element to schedule
>>> + *       the entity
>>> + *
>>>    * Returns 0 on success or a negative error code on failure.
>>>   */
>>> -int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
>>> -                       struct drm_sched_entity *entity,
>>> -                       struct drm_sched_rq *rq,
>>> +int drm_sched_entity_init(struct drm_sched_entity *entity,
>>> +                       struct drm_sched_rq **rq_list,
>>> +                       unsigned int num_rq_list,
>>>                          atomic_t *guilty)
>>>   {
>>> -     if (!(sched && entity && rq))
>>> +     if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
>>>                return -EINVAL;
>>>
>>>        memset(entity, 0, sizeof(struct drm_sched_entity));
>>>        INIT_LIST_HEAD(&entity->list);
>>> -     entity->rq = rq;
>>> -     entity->sched = sched;
>>> +     entity->rq_list = NULL;
>>> +     entity->rq = rq_list[0];
>>> +     entity->sched = rq_list[0]->sched;
>>> +     entity->num_rq_list = num_rq_list;
>> The API change makes sense as prep work, but I don't really like adding
>> the field to the struct (and changing the struct's docs for the existing
>> rq field) if it's going to always be NULL until a future change.
>>
>> Similarly, I'd rather see patch 2 as part of a series that uses the
>> value.
> I agree with you. I am fine with dropping the patch 2 for now and
> modifying patch 3. I am fine either way.
>
> What are your thoughts on this Christian?

Ok with me as well. In this case just send out the two patches with the 
API change.

Christian.

>> That said, while I don't currently have a usecase for load-balancing
>> between entities, I may in the future, so thanks for working on this!

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

Re: [PATCH 2/3] drm/scheduler: add counter for total jobs in scheduler

[Zhang, Jerry (Junwei)]

On 07/12/2018 02:36 PM, Nayan Deshmukh wrote:
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> ---
>   drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 +++
>   include/drm/gpu_scheduler.h               | 2 ++
>   2 files changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> index 429b1328653a..3dc1a4f07e3f 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> @@ -538,6 +538,7 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
>   	trace_drm_sched_job(sched_job, entity);
>
>   	first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
> +	atomic_inc(&entity->sched->num_jobs);

Shall we use hw_rq_count directly or merge them together?

Regards,
Jerry
>
>   	/* first job wakes up scheduler */
>   	if (first) {
> @@ -818,6 +819,7 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb)
>
>   	dma_fence_get(&s_fence->finished);
>   	atomic_dec(&sched->hw_rq_count);
> +	atomic_dec(&sched->num_jobs);
>   	drm_sched_fence_finished(s_fence);
>
>   	trace_drm_sched_process_job(s_fence);
> @@ -935,6 +937,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
>   	INIT_LIST_HEAD(&sched->ring_mirror_list);
>   	spin_lock_init(&sched->job_list_lock);
>   	atomic_set(&sched->hw_rq_count, 0);
> +	atomic_set(&sched->num_jobs, 0);
>   	atomic64_set(&sched->job_id_count, 0);
>
>   	/* Each scheduler will run on a seperate kernel thread */
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 43e93d6077cf..605bd4ad2397 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -257,6 +257,7 @@ struct drm_sched_backend_ops {
>    * @job_list_lock: lock to protect the ring_mirror_list.
>    * @hang_limit: once the hangs by a job crosses this limit then it is marked
>    *              guilty and it will be considered for scheduling further.
> + * @num_jobs: the number of jobs in queue in the scheduler
>    *
>    * One scheduler is implemented for each hardware ring.
>    */
> @@ -274,6 +275,7 @@ struct drm_gpu_scheduler {
>   	struct list_head		ring_mirror_list;
>   	spinlock_t			job_list_lock;
>   	int				hang_limit;
> +	atomic_t                        num_jobs;
>   };
>
>   int drm_sched_init(struct drm_gpu_scheduler *sched,
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/scheduler: add counter for total jobs in scheduler

[Nayan Deshmukh]

[-- Attachment #1.1: Type: text/plain, Size: 3056 bytes --]

On Thu, Aug 2, 2018 at 10:31 AM Zhang, Jerry (Junwei) <Jerry.Zhang@amd.com>
wrote:

> On 07/12/2018 02:36 PM, Nayan Deshmukh wrote:
> > Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> > ---
> >   drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 +++
> >   include/drm/gpu_scheduler.h               | 2 ++
> >   2 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > index 429b1328653a..3dc1a4f07e3f 100644
> > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > @@ -538,6 +538,7 @@ void drm_sched_entity_push_job(struct drm_sched_job
> *sched_job,
> >       trace_drm_sched_job(sched_job, entity);
> >
> >       first = spsc_queue_push(&entity->job_queue,
> &sched_job->queue_node);
> > +     atomic_inc(&entity->sched->num_jobs);
>
> Shall we use hw_rq_count directly or merge them together?
>
hw_rq_count is the number of jobs that are currently in the hardware queue
as compared to num_jobs which is the number of jobs in the software queue.
num_jobs provides a give a better idea of the load on a scheduler that's
why I added that field and used it to decide the scheduler with the least
load.

Regards,
Nayan

>
> Regards,
> Jerry
> >
> >       /* first job wakes up scheduler */
> >       if (first) {
> > @@ -818,6 +819,7 @@ static void drm_sched_process_job(struct dma_fence
> *f, struct dma_fence_cb *cb)
> >
> >       dma_fence_get(&s_fence->finished);
> >       atomic_dec(&sched->hw_rq_count);
> > +     atomic_dec(&sched->num_jobs);
> >       drm_sched_fence_finished(s_fence);
> >
> >       trace_drm_sched_process_job(s_fence);
> > @@ -935,6 +937,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
> >       INIT_LIST_HEAD(&sched->ring_mirror_list);
> >       spin_lock_init(&sched->job_list_lock);
> >       atomic_set(&sched->hw_rq_count, 0);
> > +     atomic_set(&sched->num_jobs, 0);
> >       atomic64_set(&sched->job_id_count, 0);
> >
> >       /* Each scheduler will run on a seperate kernel thread */
> > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> > index 43e93d6077cf..605bd4ad2397 100644
> > --- a/include/drm/gpu_scheduler.h
> > +++ b/include/drm/gpu_scheduler.h
> > @@ -257,6 +257,7 @@ struct drm_sched_backend_ops {
> >    * @job_list_lock: lock to protect the ring_mirror_list.
> >    * @hang_limit: once the hangs by a job crosses this limit then it is
> marked
> >    *              guilty and it will be considered for scheduling
> further.
> > + * @num_jobs: the number of jobs in queue in the scheduler
> >    *
> >    * One scheduler is implemented for each hardware ring.
> >    */
> > @@ -274,6 +275,7 @@ struct drm_gpu_scheduler {
> >       struct list_head                ring_mirror_list;
> >       spinlock_t                      job_list_lock;
> >       int                             hang_limit;
> > +     atomic_t                        num_jobs;
> >   };
> >
> >   int drm_sched_init(struct drm_gpu_scheduler *sched,
> >
>

[-- Attachment #1.2: Type: text/html, Size: 4110 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/3] drm/scheduler: add counter for total jobs in scheduler

[Zhang, Jerry (Junwei)]

On 08/02/2018 01:50 PM, Nayan Deshmukh wrote:
>
>
> On Thu, Aug 2, 2018 at 10:31 AM Zhang, Jerry (Junwei) <Jerry.Zhang@amd.com <mailto:Jerry.Zhang@amd.com>> wrote:
>
>     On 07/12/2018 02:36 PM, Nayan Deshmukh wrote:
>      > Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com <mailto:nayan26deshmukh@gmail.com>>
>      > ---
>      >   drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 +++
>      >   include/drm/gpu_scheduler.h               | 2 ++
>      >   2 files changed, 5 insertions(+)
>      >
>      > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>      > index 429b1328653a..3dc1a4f07e3f 100644
>      > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>      > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>      > @@ -538,6 +538,7 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
>      >       trace_drm_sched_job(sched_job, entity);
>      >
>      >       first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
>      > +     atomic_inc(&entity->sched->num_jobs);
>
>     Shall we use hw_rq_count directly or merge them together?
>
> hw_rq_count is the number of jobs that are currently in the hardware queue as compared to num_jobs which is the number of jobs in the software queue. num_jobs provides a give a better idea of the load
> on a scheduler that's why I added that field and used it to decide the scheduler with the least load.

Thanks for your explanation.

Then may be more reasonable to move atomic_dec(&sched->num_jobs) after drm_sched_fence_scheduled() or just before atomic_inc(&sched->hw_rq_count).
How do you think that?

Regards,
Jerry

>
> Regards,
> Nayan
>
>
>     Regards,
>     Jerry
>      >
>      >       /* first job wakes up scheduler */
>      >       if (first) {
>      > @@ -818,6 +819,7 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb)
>      >
>      >       dma_fence_get(&s_fence->finished);
>      >       atomic_dec(&sched->hw_rq_count);
>      > +     atomic_dec(&sched->num_jobs);
>      >       drm_sched_fence_finished(s_fence);
>      >
>      >       trace_drm_sched_process_job(s_fence);
>      > @@ -935,6 +937,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
>      >       INIT_LIST_HEAD(&sched->ring_mirror_list);
>      >       spin_lock_init(&sched->job_list_lock);
>      >       atomic_set(&sched->hw_rq_count, 0);
>      > +     atomic_set(&sched->num_jobs, 0);
>      >       atomic64_set(&sched->job_id_count, 0);
>      >
>      >       /* Each scheduler will run on a seperate kernel thread */
>      > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>      > index 43e93d6077cf..605bd4ad2397 100644
>      > --- a/include/drm/gpu_scheduler.h
>      > +++ b/include/drm/gpu_scheduler.h
>      > @@ -257,6 +257,7 @@ struct drm_sched_backend_ops {
>      >    * @job_list_lock: lock to protect the ring_mirror_list.
>      >    * @hang_limit: once the hangs by a job crosses this limit then it is marked
>      >    *              guilty and it will be considered for scheduling further.
>      > + * @num_jobs: the number of jobs in queue in the scheduler
>      >    *
>      >    * One scheduler is implemented for each hardware ring.
>      >    */
>      > @@ -274,6 +275,7 @@ struct drm_gpu_scheduler {
>      >       struct list_head                ring_mirror_list;
>      >       spinlock_t                      job_list_lock;
>      >       int                             hang_limit;
>      > +     atomic_t                        num_jobs;
>      >   };
>      >
>      >   int drm_sched_init(struct drm_gpu_scheduler *sched,
>      >
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/scheduler: add counter for total jobs in scheduler

[Nayan Deshmukh]

[-- Attachment #1.1: Type: text/plain, Size: 4523 bytes --]

On Thu, Aug 2, 2018 at 11:29 AM Zhang, Jerry (Junwei) <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
wrote:

> On 08/02/2018 01:50 PM, Nayan Deshmukh wrote:
> >
> >
> > On Thu, Aug 2, 2018 at 10:31 AM Zhang, Jerry (Junwei) <
> Jerry.Zhang-5C7GfCeVMHo@public.gmane.org <mailto:Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>> wrote:
> >
> >     On 07/12/2018 02:36 PM, Nayan Deshmukh wrote:
> >      > Signed-off-by: Nayan Deshmukh <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <mailto:
> nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
> >      > ---
> >      >   drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 +++
> >      >   include/drm/gpu_scheduler.h               | 2 ++
> >      >   2 files changed, 5 insertions(+)
> >      >
> >      > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> >      > index 429b1328653a..3dc1a4f07e3f 100644
> >      > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> >      > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> >      > @@ -538,6 +538,7 @@ void drm_sched_entity_push_job(struct
> drm_sched_job *sched_job,
> >      >       trace_drm_sched_job(sched_job, entity);
> >      >
> >      >       first = spsc_queue_push(&entity->job_queue,
> &sched_job->queue_node);
> >      > +     atomic_inc(&entity->sched->num_jobs);
> >
> >     Shall we use hw_rq_count directly or merge them together?
> >
> > hw_rq_count is the number of jobs that are currently in the hardware
> queue as compared to num_jobs which is the number of jobs in the software
> queue. num_jobs provides a give a better idea of the load
> > on a scheduler that's why I added that field and used it to decide the
> scheduler with the least load.
>
> Thanks for your explanation.
>
> Then may be more reasonable to move atomic_dec(&sched->num_jobs) after
> drm_sched_fence_scheduled() or just before atomic_inc(&sched->hw_rq_count).
> How do you think that?
>
> For now num_jobs also includes jobs that have been pushed to the hardware
queue. If I shift it before atomic_inc(&sched->hw_rq_count) then I also
need to handle cases when the job was reset and update the counter
properly. I went for the easier implementation as I felt that num_jobs will
correctly represent the load on the scheduler.

But the idea that you suggested can be a potential improvement over what I
have done.

Regards,
Nayan

> Regards,
> Jerry
>
> >
> > Regards,
> > Nayan
> >
> >
> >     Regards,
> >     Jerry
> >      >
> >      >       /* first job wakes up scheduler */
> >      >       if (first) {
> >      > @@ -818,6 +819,7 @@ static void drm_sched_process_job(struct
> dma_fence *f, struct dma_fence_cb *cb)
> >      >
> >      >       dma_fence_get(&s_fence->finished);
> >      >       atomic_dec(&sched->hw_rq_count);
> >      > +     atomic_dec(&sched->num_jobs);
> >      >       drm_sched_fence_finished(s_fence);
> >      >
> >      >       trace_drm_sched_process_job(s_fence);
> >      > @@ -935,6 +937,7 @@ int drm_sched_init(struct drm_gpu_scheduler
> *sched,
> >      >       INIT_LIST_HEAD(&sched->ring_mirror_list);
> >      >       spin_lock_init(&sched->job_list_lock);
> >      >       atomic_set(&sched->hw_rq_count, 0);
> >      > +     atomic_set(&sched->num_jobs, 0);
> >      >       atomic64_set(&sched->job_id_count, 0);
> >      >
> >      >       /* Each scheduler will run on a seperate kernel thread */
> >      > diff --git a/include/drm/gpu_scheduler.h
> b/include/drm/gpu_scheduler.h
> >      > index 43e93d6077cf..605bd4ad2397 100644
> >      > --- a/include/drm/gpu_scheduler.h
> >      > +++ b/include/drm/gpu_scheduler.h
> >      > @@ -257,6 +257,7 @@ struct drm_sched_backend_ops {
> >      >    * @job_list_lock: lock to protect the ring_mirror_list.
> >      >    * @hang_limit: once the hangs by a job crosses this limit then
> it is marked
> >      >    *              guilty and it will be considered for scheduling
> further.
> >      > + * @num_jobs: the number of jobs in queue in the scheduler
> >      >    *
> >      >    * One scheduler is implemented for each hardware ring.
> >      >    */
> >      > @@ -274,6 +275,7 @@ struct drm_gpu_scheduler {
> >      >       struct list_head                ring_mirror_list;
> >      >       spinlock_t                      job_list_lock;
> >      >       int                             hang_limit;
> >      > +     atomic_t                        num_jobs;
> >      >   };
> >      >
> >      >   int drm_sched_init(struct drm_gpu_scheduler *sched,
> >      >
> >
>

[-- Attachment #1.2: Type: text/html, Size: 6328 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 2/3] drm/scheduler: add counter for total jobs in scheduler

[Zhang, Jerry (Junwei)]

On 08/02/2018 02:08 PM, Nayan Deshmukh wrote:
>
>
> On Thu, Aug 2, 2018 at 11:29 AM Zhang, Jerry (Junwei) <Jerry.Zhang@amd.com <mailto:Jerry.Zhang@amd.com>> wrote:
>
>     On 08/02/2018 01:50 PM, Nayan Deshmukh wrote:
>      >
>      >
>      > On Thu, Aug 2, 2018 at 10:31 AM Zhang, Jerry (Junwei) <Jerry.Zhang@amd.com <mailto:Jerry.Zhang@amd.com> <mailto:Jerry.Zhang@amd.com <mailto:Jerry.Zhang@amd.com>>> wrote:
>      >
>      >     On 07/12/2018 02:36 PM, Nayan Deshmukh wrote:
>      >      > Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com <mailto:nayan26deshmukh@gmail.com> <mailto:nayan26deshmukh@gmail.com <mailto:nayan26deshmukh@gmail.com>>>
>      >      > ---
>      >      >   drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 +++
>      >      >   include/drm/gpu_scheduler.h               | 2 ++
>      >      >   2 files changed, 5 insertions(+)
>      >      >
>      >      > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>      >      > index 429b1328653a..3dc1a4f07e3f 100644
>      >      > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>      >      > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>      >      > @@ -538,6 +538,7 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
>      >      >       trace_drm_sched_job(sched_job, entity);
>      >      >
>      >      >       first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
>      >      > +     atomic_inc(&entity->sched->num_jobs);
>      >
>      >     Shall we use hw_rq_count directly or merge them together?
>      >
>      > hw_rq_count is the number of jobs that are currently in the hardware queue as compared to num_jobs which is the number of jobs in the software queue. num_jobs provides a give a better idea of
>     the load
>      > on a scheduler that's why I added that field and used it to decide the scheduler with the least load.
>
>     Thanks for your explanation.
>
>     Then may be more reasonable to move atomic_dec(&sched->num_jobs) after drm_sched_fence_scheduled() or just before atomic_inc(&sched->hw_rq_count).
>     How do you think that?
>
> For now num_jobs also includes jobs that have been pushed to the hardware queue. If I shift it before atomic_inc(&sched->hw_rq_count) then I also need to handle cases when the job was reset and update
> the counter properly. I went for the easier implementation as I felt that num_jobs will correctly represent the load on the scheduler.

Thanks for your info more.
Yes, it's not a simple one deal work, just to clarify it's really meaning, a little overlap with hw_rq_count.
fine for now ;)

>
> But the idea that you suggested can be a potential improvement over what I have done.

Thanks.

Regards,
Jerry
>
> Regards,
> Nayan
>
>     Regards,
>     Jerry
>
>      >
>      > Regards,
>      > Nayan
>      >
>      >
>      >     Regards,
>      >     Jerry
>      >      >
>      >      >       /* first job wakes up scheduler */
>      >      >       if (first) {
>      >      > @@ -818,6 +819,7 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb)
>      >      >
>      >      >       dma_fence_get(&s_fence->finished);
>      >      >       atomic_dec(&sched->hw_rq_count);
>      >      > +     atomic_dec(&sched->num_jobs);
>      >      >       drm_sched_fence_finished(s_fence);
>      >      >
>      >      >       trace_drm_sched_process_job(s_fence);
>      >      > @@ -935,6 +937,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
>      >      >       INIT_LIST_HEAD(&sched->ring_mirror_list);
>      >      >       spin_lock_init(&sched->job_list_lock);
>      >      >       atomic_set(&sched->hw_rq_count, 0);
>      >      > +     atomic_set(&sched->num_jobs, 0);
>      >      >       atomic64_set(&sched->job_id_count, 0);
>      >      >
>      >      >       /* Each scheduler will run on a seperate kernel thread */
>      >      > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>      >      > index 43e93d6077cf..605bd4ad2397 100644
>      >      > --- a/include/drm/gpu_scheduler.h
>      >      > +++ b/include/drm/gpu_scheduler.h
>      >      > @@ -257,6 +257,7 @@ struct drm_sched_backend_ops {
>      >      >    * @job_list_lock: lock to protect the ring_mirror_list.
>      >      >    * @hang_limit: once the hangs by a job crosses this limit then it is marked
>      >      >    *              guilty and it will be considered for scheduling further.
>      >      > + * @num_jobs: the number of jobs in queue in the scheduler
>      >      >    *
>      >      >    * One scheduler is implemented for each hardware ring.
>      >      >    */
>      >      > @@ -274,6 +275,7 @@ struct drm_gpu_scheduler {
>      >      >       struct list_head                ring_mirror_list;
>      >      >       spinlock_t                      job_list_lock;
>      >      >       int                             hang_limit;
>      >      > +     atomic_t                        num_jobs;
>      >      >   };
>      >      >
>      >      >   int drm_sched_init(struct drm_gpu_scheduler *sched,
>      >      >
>      >
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel