* [PATCH 1/4] drm/scheduler: rework entity creation
@ 2019-12-06 17:33 Nirmoy Das
2019-12-06 17:33 ` [PATCH 2/4] drm/amdgpu: replace vm_pte's run-queue list with drm gpu scheds list Nirmoy Das
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Nirmoy Das @ 2019-12-06 17:33 UTC (permalink / raw)
To: alexander.deucher, kenny.ho, christian.koenig; +Cc: nirmoy.das, amd-gfx
Entity currently keeps a copy of run_queue list and modify it in
drm_sched_entity_set_priority(). Entities shouldn't modify run_queue
list. Use drm_gpu_scheduler list instead of drm_sched_rq list
in drm_sched_entity struct. In this way we can select a runqueue based
on entity/ctx's priority for a drm scheduler.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 7 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 7 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 7 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++--
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 7 ++-
drivers/gpu/drm/lima/lima_sched.c | 5 +-
drivers/gpu/drm/panfrost/panfrost_job.c | 8 ++-
drivers/gpu/drm/scheduler/sched_entity.c | 74 ++++++++++--------------
drivers/gpu/drm/v3d/v3d_drv.c | 8 ++-
include/drm/gpu_scheduler.h | 8 ++-
11 files changed, 78 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index a0d3d7b756eb..1d6850af9908 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -122,7 +122,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
- struct drm_sched_rq *rqs[AMDGPU_MAX_RINGS];
+ struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
unsigned num_rings = 0;
unsigned num_rqs = 0;
@@ -181,12 +181,13 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
if (!rings[j]->adev)
continue;
- rqs[num_rqs++] = &rings[j]->sched.sched_rq[priority];
+ sched_list[num_rqs++] = &rings[j]->sched;
}
for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
r = drm_sched_entity_init(&ctx->entities[i][j].entity,
- rqs, num_rqs, &ctx->guilty);
+ priority, sched_list,
+ num_rqs, &ctx->guilty);
if (r)
goto error_cleanup_entities;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 19ffe00d9072..2b6e35893918 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1957,11 +1957,13 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
if (enable) {
struct amdgpu_ring *ring;
- struct drm_sched_rq *rq;
+ struct drm_gpu_scheduler *sched;
ring = adev->mman.buffer_funcs_ring;
- rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_KERNEL];
- r = drm_sched_entity_init(&adev->mman.entity, &rq, 1, NULL);
+ sched = &ring->sched;
+ r = drm_sched_entity_init(&adev->mman.entity,
+ DRM_SCHED_PRIORITY_KERNEL, &sched,
+ 1, NULL);
if (r) {
DRM_ERROR("Failed setting up TTM BO move entity (%d)\n",
r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index e324bfe6c58f..a1a110f5513d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -330,12 +330,13 @@ int amdgpu_uvd_sw_fini(struct amdgpu_device *adev)
int amdgpu_uvd_entity_init(struct amdgpu_device *adev)
{
struct amdgpu_ring *ring;
- struct drm_sched_rq *rq;
+ struct drm_gpu_scheduler *sched;
int r;
ring = &adev->uvd.inst[0].ring;
- rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
- r = drm_sched_entity_init(&adev->uvd.entity, &rq, 1, NULL);
+ sched = &ring->sched;
+ r = drm_sched_entity_init(&adev->uvd.entity, DRM_SCHED_PRIORITY_NORMAL,
+ &sched, 1, NULL);
if (r) {
DRM_ERROR("Failed setting up UVD kernel entity.\n");
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 46b590af2fd2..ceb0dbf685f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -240,12 +240,13 @@ int amdgpu_vce_sw_fini(struct amdgpu_device *adev)
int amdgpu_vce_entity_init(struct amdgpu_device *adev)
{
struct amdgpu_ring *ring;
- struct drm_sched_rq *rq;
+ struct drm_gpu_scheduler *sched;
int r;
ring = &adev->vce.ring[0];
- rq = &ring->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
- r = drm_sched_entity_init(&adev->vce.entity, &rq, 1, NULL);
+ sched = &ring->sched;
+ r = drm_sched_entity_init(&adev->vce.entity, DRM_SCHED_PRIORITY_NORMAL,
+ &sched, 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 a94c4faa5af1..5e78db30c722 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2687,6 +2687,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
{
struct amdgpu_bo_param bp;
struct amdgpu_bo *root;
+ struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
int r, i;
vm->va = RB_ROOT_CACHED;
@@ -2700,14 +2701,19 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
spin_lock_init(&vm->invalidated_lock);
INIT_LIST_HEAD(&vm->freed);
+ for (i = 0; i < adev->vm_manager.vm_pte_num_rqs; i++)
+ sched_list[i] = adev->vm_manager.vm_pte_rqs[i]->sched;
+
/* create scheduler entities for page table updates */
- r = drm_sched_entity_init(&vm->direct, adev->vm_manager.vm_pte_rqs,
- adev->vm_manager.vm_pte_num_rqs, NULL);
+ r = drm_sched_entity_init(&vm->direct, DRM_SCHED_PRIORITY_NORMAL,
+ sched_list, adev->vm_manager.vm_pte_num_rqs,
+ NULL);
if (r)
return r;
- r = drm_sched_entity_init(&vm->delayed, adev->vm_manager.vm_pte_rqs,
- adev->vm_manager.vm_pte_num_rqs, NULL);
+ r = drm_sched_entity_init(&vm->delayed, DRM_SCHED_PRIORITY_NORMAL,
+ sched_list, adev->vm_manager.vm_pte_num_rqs,
+ NULL);
if (r)
goto error_free_direct;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 1f9c01be40d7..76ecdf8bd31c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -65,12 +65,13 @@ 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;
+ struct drm_gpu_scheduler *sched;
if (gpu) {
- rq = &gpu->sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
+ sched = &gpu->sched;
drm_sched_entity_init(&ctx->sched_entity[i],
- &rq, 1, NULL);
+ DRM_SCHED_PRIORITY_NORMAL, &sched,
+ 1, NULL);
}
}
diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c
index f522c5f99729..fc8362e4149b 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -159,9 +159,10 @@ int lima_sched_context_init(struct lima_sched_pipe *pipe,
struct lima_sched_context *context,
atomic_t *guilty)
{
- struct drm_sched_rq *rq = pipe->base.sched_rq + DRM_SCHED_PRIORITY_NORMAL;
+ struct drm_gpu_scheduler *sched = &pipe->base;
- return drm_sched_entity_init(&context->base, &rq, 1, guilty);
+ return drm_sched_entity_init(&context->base, DRM_SCHED_PRIORITY_NORMAL,
+ &sched, 1, guilty);
}
void lima_sched_context_fini(struct lima_sched_pipe *pipe,
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
index d411eb6c8eb9..4f9ae5a12090 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -542,12 +542,14 @@ int panfrost_job_open(struct panfrost_file_priv *panfrost_priv)
{
struct panfrost_device *pfdev = panfrost_priv->pfdev;
struct panfrost_job_slot *js = pfdev->js;
- struct drm_sched_rq *rq;
+ struct drm_gpu_scheduler *sched;
int ret, i;
for (i = 0; i < NUM_JOB_SLOTS; i++) {
- rq = &js->queue[i].sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
- ret = drm_sched_entity_init(&panfrost_priv->sched_entity[i], &rq, 1, NULL);
+ sched = &js->queue[i].sched;
+ ret = drm_sched_entity_init(&panfrost_priv->sched_entity[i],
+ DRM_SCHED_PRIORITY_NORMAL, &sched,
+ 1, NULL, DRM_SCHED_PRIORITY_NORMAL);
if (WARN_ON(ret))
return ret;
}
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index 461a7a8129f4..f9b6ce29c58f 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -38,9 +38,10 @@
* submit to HW ring.
*
* @entity: scheduler entity to init
- * @rq_list: the list of run queue on which jobs from this
+ * @priority: priority of the entity
+ * @sched_list: the list of drm scheds on which jobs from this
* entity can be submitted
- * @num_rq_list: number of run queue in rq_list
+ * @num_sched_list: number of drm sched in sched_list
* @guilty: atomic_t set to 1 when a job on this queue
* is found to be guilty causing a timeout
*
@@ -50,32 +51,35 @@
* Returns 0 on success or a negative error code on failure.
*/
int drm_sched_entity_init(struct drm_sched_entity *entity,
- struct drm_sched_rq **rq_list,
- unsigned int num_rq_list,
+ enum drm_sched_priority priority,
+ struct drm_gpu_scheduler **sched_list,
+ unsigned int num_sched_list,
atomic_t *guilty)
{
int i;
- if (!(entity && rq_list && (num_rq_list == 0 || rq_list[0])))
+ if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
return -EINVAL;
memset(entity, 0, sizeof(struct drm_sched_entity));
INIT_LIST_HEAD(&entity->list);
entity->rq = NULL;
entity->guilty = guilty;
- entity->num_rq_list = num_rq_list;
- entity->rq_list = kcalloc(num_rq_list, sizeof(struct drm_sched_rq *),
- GFP_KERNEL);
- if (!entity->rq_list)
+ entity->num_sched_list = num_sched_list;
+ entity->priority = priority;
+ entity->sched_list = kcalloc(num_sched_list,
+ sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
+
+ if(!entity->sched_list)
return -ENOMEM;
init_completion(&entity->entity_idle);
- for (i = 0; i < num_rq_list; ++i)
- entity->rq_list[i] = rq_list[i];
+ for (i = 0; i < num_sched_list; i++)
+ entity->sched_list[i] = sched_list[i];
- if (num_rq_list)
- entity->rq = rq_list[0];
+ if (num_sched_list)
+ entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
entity->last_scheduled = NULL;
@@ -139,10 +143,10 @@ drm_sched_entity_get_free_sched(struct drm_sched_entity *entity)
unsigned int min_jobs = UINT_MAX, num_jobs;
int i;
- for (i = 0; i < entity->num_rq_list; ++i) {
- struct drm_gpu_scheduler *sched = entity->rq_list[i]->sched;
+ for (i = 0; i < entity->num_sched_list; ++i) {
+ struct drm_gpu_scheduler *sched = entity->sched_list[i];
- if (!entity->rq_list[i]->sched->ready) {
+ if (!entity->sched_list[i]->ready) {
DRM_WARN("sched%s is not ready, skipping", sched->name);
continue;
}
@@ -150,7 +154,7 @@ drm_sched_entity_get_free_sched(struct drm_sched_entity *entity)
num_jobs = atomic_read(&sched->num_jobs);
if (num_jobs < min_jobs) {
min_jobs = num_jobs;
- rq = entity->rq_list[i];
+ rq = &entity->sched_list[i]->sched_rq[entity->priority];
}
}
@@ -308,7 +312,7 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
- kfree(entity->rq_list);
+ kfree(entity->sched_list);
}
EXPORT_SYMBOL(drm_sched_entity_fini);
@@ -353,15 +357,6 @@ static void drm_sched_entity_wakeup(struct dma_fence *f,
drm_sched_wakeup(entity->rq->sched);
}
-/**
- * drm_sched_entity_set_rq_priority - helper for drm_sched_entity_set_priority
- */
-static void drm_sched_entity_set_rq_priority(struct drm_sched_rq **rq,
- enum drm_sched_priority priority)
-{
- *rq = &(*rq)->sched->sched_rq[priority];
-}
-
/**
* drm_sched_entity_set_priority - Sets priority of the entity
*
@@ -373,19 +368,8 @@ static void drm_sched_entity_set_rq_priority(struct drm_sched_rq **rq,
void drm_sched_entity_set_priority(struct drm_sched_entity *entity,
enum drm_sched_priority priority)
{
- unsigned int i;
-
spin_lock(&entity->rq_lock);
-
- for (i = 0; i < entity->num_rq_list; ++i)
- drm_sched_entity_set_rq_priority(&entity->rq_list[i], priority);
-
- if (entity->rq) {
- drm_sched_rq_remove_entity(entity->rq, entity);
- drm_sched_entity_set_rq_priority(&entity->rq, priority);
- drm_sched_rq_add_entity(entity->rq, entity);
- }
-
+ entity->priority = priority;
spin_unlock(&entity->rq_lock);
}
EXPORT_SYMBOL(drm_sched_entity_set_priority);
@@ -490,20 +474,20 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity)
struct dma_fence *fence;
struct drm_sched_rq *rq;
- if (spsc_queue_count(&entity->job_queue) || entity->num_rq_list <= 1)
+ if (spsc_queue_count(&entity->job_queue) || entity->num_sched_list <= 1)
return;
fence = READ_ONCE(entity->last_scheduled);
if (fence && !dma_fence_is_signaled(fence))
return;
+ spin_lock(&entity->rq_lock);
rq = drm_sched_entity_get_free_sched(entity);
- if (rq == entity->rq)
- return;
+ if (rq != entity->rq) {
+ drm_sched_rq_remove_entity(entity->rq, entity);
+ entity->rq = rq;
+ }
- spin_lock(&entity->rq_lock);
- drm_sched_rq_remove_entity(entity->rq, entity);
- entity->rq = rq;
spin_unlock(&entity->rq_lock);
}
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 1a07462b4528..eaa8e9682373 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -140,7 +140,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;
+ struct drm_gpu_scheduler *sched;
int i;
v3d_priv = kzalloc(sizeof(*v3d_priv), GFP_KERNEL);
@@ -150,8 +150,10 @@ v3d_open(struct drm_device *dev, struct drm_file *file)
v3d_priv->v3d = v3d;
for (i = 0; i < V3D_MAX_QUEUES; i++) {
- rq = &v3d->queue[i].sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
- drm_sched_entity_init(&v3d_priv->sched_entity[i], &rq, 1, NULL);
+ sched = &v3d->queue[i].sched;
+ drm_sched_entity_init(&v3d_priv->sched_entity[i],
+ DRM_SCHED_PRIORITY_NORMAL, &sched,
+ 1, NULL);
}
file->driver_priv = v3d_priv;
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 684692a8ed76..96a1a1b7526e 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -81,8 +81,9 @@ 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;
+ unsigned int num_sched_list;
+ struct drm_gpu_scheduler **sched_list;
+ enum drm_sched_priority priority;
spinlock_t rq_lock;
struct spsc_queue job_queue;
@@ -312,7 +313,8 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
struct drm_sched_entity *entity);
int drm_sched_entity_init(struct drm_sched_entity *entity,
- struct drm_sched_rq **rq_list,
+ enum drm_sched_priority priority,
+ struct drm_gpu_scheduler **sched_list,
unsigned int num_rq_list,
atomic_t *guilty);
long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout);
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/4] drm/amdgpu: replace vm_pte's run-queue list with drm gpu scheds list
2019-12-06 17:33 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
@ 2019-12-06 17:33 ` Nirmoy Das
2019-12-06 19:37 ` Christian König
2019-12-06 17:33 ` [PATCH 3/4] drm/amdgpu: allocate entities on demand Nirmoy Das
2019-12-06 17:33 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
2 siblings, 1 reply; 25+ messages in thread
From: Nirmoy Das @ 2019-12-06 17:33 UTC (permalink / raw)
To: alexander.deucher, kenny.ho, christian.koenig; +Cc: nirmoy.das, amd-gfx
drm_sched_entity_init() takes drm gpu scheduler list instead of
drm_sched_rq list. This makes conversion of drm_sched_rq list
to drm gpu scheduler list unnecessary
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 ++++-------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++--
drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 8 +++-----
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 8 +++-----
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 8 +++-----
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 5 ++---
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 8 +++-----
drivers/gpu/drm/amd/amdgpu/si_dma.c | 8 +++-----
9 files changed, 24 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f85007382093..cf4953c4e2cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2779,7 +2779,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
adev->mman.buffer_funcs = NULL;
adev->mman.buffer_funcs_ring = NULL;
adev->vm_manager.vm_pte_funcs = NULL;
- adev->vm_manager.vm_pte_num_rqs = 0;
+ adev->vm_manager.vm_pte_num_scheds = 0;
adev->gmc.gmc_funcs = NULL;
adev->fence_context = dma_fence_context_alloc(AMDGPU_MAX_RINGS);
bitmap_zero(adev->gfx.pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 5e78db30c722..0e1ed8ef2ce7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2687,7 +2687,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
{
struct amdgpu_bo_param bp;
struct amdgpu_bo *root;
- struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
int r, i;
vm->va = RB_ROOT_CACHED;
@@ -2701,19 +2700,17 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
spin_lock_init(&vm->invalidated_lock);
INIT_LIST_HEAD(&vm->freed);
- for (i = 0; i < adev->vm_manager.vm_pte_num_rqs; i++)
- sched_list[i] = adev->vm_manager.vm_pte_rqs[i]->sched;
/* create scheduler entities for page table updates */
r = drm_sched_entity_init(&vm->direct, DRM_SCHED_PRIORITY_NORMAL,
- sched_list, adev->vm_manager.vm_pte_num_rqs,
- NULL);
+ adev->vm_manager.vm_pte_scheds,
+ adev->vm_manager.vm_pte_num_scheds, NULL);
if (r)
return r;
r = drm_sched_entity_init(&vm->delayed, DRM_SCHED_PRIORITY_NORMAL,
- sched_list, adev->vm_manager.vm_pte_num_rqs,
- NULL);
+ adev->vm_manager.vm_pte_scheds,
+ adev->vm_manager.vm_pte_num_scheds, NULL);
if (r)
goto error_free_direct;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 76fcf853035c..5eaba8645a43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -322,8 +322,8 @@ struct amdgpu_vm_manager {
u64 vram_base_offset;
/* vm pte handling */
const struct amdgpu_vm_pte_funcs *vm_pte_funcs;
- struct drm_sched_rq *vm_pte_rqs[AMDGPU_MAX_RINGS];
- unsigned vm_pte_num_rqs;
+ struct drm_gpu_scheduler *vm_pte_scheds[AMDGPU_MAX_RINGS];
+ unsigned vm_pte_num_scheds;
struct amdgpu_ring *page_fault;
/* partial resident texture handling */
diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
index 82cdb8f57bfd..1f22a8d0f7f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
@@ -1373,16 +1373,14 @@ static const struct amdgpu_vm_pte_funcs cik_sdma_vm_pte_funcs = {
static void cik_sdma_set_vm_pte_funcs(struct amdgpu_device *adev)
{
- struct drm_gpu_scheduler *sched;
unsigned i;
adev->vm_manager.vm_pte_funcs = &cik_sdma_vm_pte_funcs;
for (i = 0; i < adev->sdma.num_instances; i++) {
- sched = &adev->sdma.instance[i].ring.sched;
- adev->vm_manager.vm_pte_rqs[i] =
- &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
+ adev->vm_manager.vm_pte_scheds[i] =
+ &adev->sdma.instance[i].ring.sched;
}
- adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
+ adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
}
const struct amdgpu_ip_block_version cik_sdma_ip_block =
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
index 89e8c74a40f4..606b621145a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
@@ -1261,16 +1261,14 @@ static const struct amdgpu_vm_pte_funcs sdma_v2_4_vm_pte_funcs = {
static void sdma_v2_4_set_vm_pte_funcs(struct amdgpu_device *adev)
{
- struct drm_gpu_scheduler *sched;
unsigned i;
adev->vm_manager.vm_pte_funcs = &sdma_v2_4_vm_pte_funcs;
for (i = 0; i < adev->sdma.num_instances; i++) {
- sched = &adev->sdma.instance[i].ring.sched;
- adev->vm_manager.vm_pte_rqs[i] =
- &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
+ adev->vm_manager.vm_pte_scheds[i] =
+ &adev->sdma.instance[i].ring.sched;
}
- adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
+ adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
}
const struct amdgpu_ip_block_version sdma_v2_4_ip_block =
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index 011fd12c41fe..a559573ec8fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -1699,16 +1699,14 @@ static const struct amdgpu_vm_pte_funcs sdma_v3_0_vm_pte_funcs = {
static void sdma_v3_0_set_vm_pte_funcs(struct amdgpu_device *adev)
{
- struct drm_gpu_scheduler *sched;
unsigned i;
adev->vm_manager.vm_pte_funcs = &sdma_v3_0_vm_pte_funcs;
for (i = 0; i < adev->sdma.num_instances; i++) {
- sched = &adev->sdma.instance[i].ring.sched;
- adev->vm_manager.vm_pte_rqs[i] =
- &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
+ adev->vm_manager.vm_pte_scheds[i] =
+ &adev->sdma.instance[i].ring.sched;
}
- adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
+ adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
}
const struct amdgpu_ip_block_version sdma_v3_0_ip_block =
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 350b2c99fefc..bd9ed33bab43 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -2411,10 +2411,9 @@ static void sdma_v4_0_set_vm_pte_funcs(struct amdgpu_device *adev)
sched = &adev->sdma.instance[i].page.sched;
else
sched = &adev->sdma.instance[i].ring.sched;
- adev->vm_manager.vm_pte_rqs[i] =
- &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
+ adev->vm_manager.vm_pte_scheds[i] = sched;
}
- adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
+ adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
}
const struct amdgpu_ip_block_version sdma_v4_0_ip_block = {
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 64c53eed7fac..63f667cfe3f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1723,17 +1723,15 @@ static const struct amdgpu_vm_pte_funcs sdma_v5_0_vm_pte_funcs = {
static void sdma_v5_0_set_vm_pte_funcs(struct amdgpu_device *adev)
{
- struct drm_gpu_scheduler *sched;
unsigned i;
if (adev->vm_manager.vm_pte_funcs == NULL) {
adev->vm_manager.vm_pte_funcs = &sdma_v5_0_vm_pte_funcs;
for (i = 0; i < adev->sdma.num_instances; i++) {
- sched = &adev->sdma.instance[i].ring.sched;
- adev->vm_manager.vm_pte_rqs[i] =
- &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
+ adev->vm_manager.vm_pte_scheds[i] =
+ &adev->sdma.instance[i].ring.sched;
}
- adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
+ adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
}
}
diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
index 122df0732f0c..9ad85eddf9c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
+++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
@@ -835,16 +835,14 @@ static const struct amdgpu_vm_pte_funcs si_dma_vm_pte_funcs = {
static void si_dma_set_vm_pte_funcs(struct amdgpu_device *adev)
{
- struct drm_gpu_scheduler *sched;
unsigned i;
adev->vm_manager.vm_pte_funcs = &si_dma_vm_pte_funcs;
for (i = 0; i < adev->sdma.num_instances; i++) {
- sched = &adev->sdma.instance[i].ring.sched;
- adev->vm_manager.vm_pte_rqs[i] =
- &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
+ adev->vm_manager.vm_pte_scheds[i] =
+ &adev->sdma.instance[i].ring.sched;
}
- adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
+ adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
}
const struct amdgpu_ip_block_version si_dma_ip_block =
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/4] drm/amdgpu: allocate entities on demand
2019-12-06 17:33 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
2019-12-06 17:33 ` [PATCH 2/4] drm/amdgpu: replace vm_pte's run-queue list with drm gpu scheds list Nirmoy Das
@ 2019-12-06 17:33 ` Nirmoy Das
2019-12-06 19:40 ` Christian König
2019-12-06 17:33 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
2 siblings, 1 reply; 25+ messages in thread
From: Nirmoy Das @ 2019-12-06 17:33 UTC (permalink / raw)
To: alexander.deucher, kenny.ho, christian.koenig; +Cc: nirmoy.das, amd-gfx
Currently we pre-allocate entities for all the HW IPs on
context creation and some of which are might never be used.
This patch tries to resolve entity wastage by creating entities
for a HW IP only when it is required.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 176 +++++++++++++-----------
1 file changed, 97 insertions(+), 79 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 1d6850af9908..c7643af8827f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -68,13 +68,99 @@ static int amdgpu_ctx_priority_permit(struct drm_file *filp,
return -EACCES;
}
+static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip)
+{
+ struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
+ struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
+ struct amdgpu_device *adev = ctx->adev;
+ unsigned num_rings = 0;
+ unsigned num_scheds = 0;
+ unsigned i, j;
+ int r = 0;
+
+ switch (hw_ip) {
+ case AMDGPU_HW_IP_GFX:
+ rings[0] = &adev->gfx.gfx_ring[0];
+ num_rings = 1;
+ break;
+ case AMDGPU_HW_IP_COMPUTE:
+ for (i = 0; i < adev->gfx.num_compute_rings; ++i)
+ rings[i] = &adev->gfx.compute_ring[i];
+ num_rings = adev->gfx.num_compute_rings;
+ break;
+ case AMDGPU_HW_IP_DMA:
+ for (i = 0; i < adev->sdma.num_instances; ++i)
+ rings[i] = &adev->sdma.instance[i].ring;
+ num_rings = adev->sdma.num_instances;
+ break;
+ case AMDGPU_HW_IP_UVD:
+ rings[0] = &adev->uvd.inst[0].ring;
+ num_rings = 1;
+ break;
+ case AMDGPU_HW_IP_VCE:
+ rings[0] = &adev->vce.ring[0];
+ num_rings = 1;
+ break;
+ case AMDGPU_HW_IP_UVD_ENC:
+ rings[0] = &adev->uvd.inst[0].ring_enc[0];
+ num_rings = 1;
+ break;
+ case AMDGPU_HW_IP_VCN_DEC:
+ for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
+ if (adev->vcn.harvest_config & (1 << i))
+ continue;
+ rings[num_rings++] = &adev->vcn.inst[i].ring_dec;
+ }
+ break;
+ case AMDGPU_HW_IP_VCN_ENC:
+ for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
+ if (adev->vcn.harvest_config & (1 << i))
+ continue;
+ for (j = 0; j < adev->vcn.num_enc_rings; ++j)
+ rings[num_rings++] = &adev->vcn.inst[i].ring_enc[j];
+ }
+ break;
+ case AMDGPU_HW_IP_VCN_JPEG:
+ for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
+ if (adev->vcn.harvest_config & (1 << i))
+ continue;
+ rings[num_rings++] = &adev->jpeg.inst[i].ring_dec;
+ }
+ break;
+ }
+
+ for (i = 0; i < num_rings; ++i) {
+ if (!rings[i]->adev)
+ continue;
+
+ sched_list[num_scheds++] = &rings[i]->sched;
+ }
+
+ for (i = 0; i < amdgpu_ctx_num_entities[hw_ip]; ++i)
+ r = drm_sched_entity_init(&ctx->entities[hw_ip][i].entity,
+ ctx->init_priority, sched_list, num_scheds, &ctx->guilty);
+ if (r)
+ goto error_cleanup_entities;
+
+ for (i = 0; i < amdgpu_ctx_num_entities[hw_ip]; ++i)
+ ctx->entities[hw_ip][i].sequence = 1;
+
+ return 0;
+
+error_cleanup_entities:
+ for (i = 0; i < amdgpu_ctx_num_entities[hw_ip]; ++i)
+ drm_sched_entity_destroy(&ctx->entities[hw_ip][i].entity);
+
+ return r;
+}
+
static int amdgpu_ctx_init(struct amdgpu_device *adev,
enum drm_sched_priority priority,
struct drm_file *filp,
struct amdgpu_ctx *ctx)
{
unsigned num_entities = amdgpu_ctx_total_num_entities();
- unsigned i, j, k;
+ unsigned i;
int r;
if (priority < 0 || priority >= DRM_SCHED_PRIORITY_MAX)
@@ -103,7 +189,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
for (i = 0; i < num_entities; ++i) {
struct amdgpu_ctx_entity *entity = &ctx->entities[0][i];
- entity->sequence = 1;
+ entity->sequence = -1;
entity->fences = &ctx->fences[amdgpu_sched_jobs * i];
}
for (i = 1; i < AMDGPU_HW_IP_NUM; ++i)
@@ -120,85 +206,8 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
ctx->init_priority = priority;
ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
- for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
- struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
- struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
- unsigned num_rings = 0;
- unsigned num_rqs = 0;
-
- switch (i) {
- case AMDGPU_HW_IP_GFX:
- rings[0] = &adev->gfx.gfx_ring[0];
- num_rings = 1;
- break;
- case AMDGPU_HW_IP_COMPUTE:
- for (j = 0; j < adev->gfx.num_compute_rings; ++j)
- rings[j] = &adev->gfx.compute_ring[j];
- num_rings = adev->gfx.num_compute_rings;
- break;
- case AMDGPU_HW_IP_DMA:
- for (j = 0; j < adev->sdma.num_instances; ++j)
- rings[j] = &adev->sdma.instance[j].ring;
- num_rings = adev->sdma.num_instances;
- break;
- case AMDGPU_HW_IP_UVD:
- rings[0] = &adev->uvd.inst[0].ring;
- num_rings = 1;
- break;
- case AMDGPU_HW_IP_VCE:
- rings[0] = &adev->vce.ring[0];
- num_rings = 1;
- break;
- case AMDGPU_HW_IP_UVD_ENC:
- rings[0] = &adev->uvd.inst[0].ring_enc[0];
- num_rings = 1;
- break;
- case AMDGPU_HW_IP_VCN_DEC:
- for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
- if (adev->vcn.harvest_config & (1 << j))
- continue;
- rings[num_rings++] = &adev->vcn.inst[j].ring_dec;
- }
- break;
- case AMDGPU_HW_IP_VCN_ENC:
- for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
- if (adev->vcn.harvest_config & (1 << j))
- continue;
- for (k = 0; k < adev->vcn.num_enc_rings; ++k)
- rings[num_rings++] = &adev->vcn.inst[j].ring_enc[k];
- }
- break;
- case AMDGPU_HW_IP_VCN_JPEG:
- for (j = 0; j < adev->jpeg.num_jpeg_inst; ++j) {
- if (adev->vcn.harvest_config & (1 << j))
- continue;
- rings[num_rings++] = &adev->jpeg.inst[j].ring_dec;
- }
- break;
- }
-
- for (j = 0; j < num_rings; ++j) {
- if (!rings[j]->adev)
- continue;
-
- sched_list[num_rqs++] = &rings[j]->sched;
- }
-
- for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
- r = drm_sched_entity_init(&ctx->entities[i][j].entity,
- priority, sched_list,
- num_rqs, &ctx->guilty);
- if (r)
- goto error_cleanup_entities;
- }
-
return 0;
-error_cleanup_entities:
- for (i = 0; i < num_entities; ++i)
- drm_sched_entity_destroy(&ctx->entities[0][i].entity);
- kfree(ctx->entities[0]);
-
error_free_fences:
kfree(ctx->fences);
ctx->fences = NULL;
@@ -229,6 +238,8 @@ static void amdgpu_ctx_fini(struct kref *ref)
int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 hw_ip, u32 instance,
u32 ring, struct drm_sched_entity **entity)
{
+ int r;
+
if (hw_ip >= AMDGPU_HW_IP_NUM) {
DRM_ERROR("unknown HW IP type: %d\n", hw_ip);
return -EINVAL;
@@ -245,6 +256,13 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 hw_ip, u32 instance,
return -EINVAL;
}
+ if (ctx->entities[hw_ip][ring].sequence == -1) {
+ r = amdgpu_ctx_init_entity(ctx, hw_ip);
+
+ if (r)
+ return r;
+ }
+
*entity = &ctx->entities[hw_ip][ring].entity;
return 0;
}
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-06 17:33 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
2019-12-06 17:33 ` [PATCH 2/4] drm/amdgpu: replace vm_pte's run-queue list with drm gpu scheds list Nirmoy Das
2019-12-06 17:33 ` [PATCH 3/4] drm/amdgpu: allocate entities on demand Nirmoy Das
@ 2019-12-06 17:33 ` Nirmoy Das
2019-12-06 19:41 ` Christian König
2 siblings, 1 reply; 25+ messages in thread
From: Nirmoy Das @ 2019-12-06 17:33 UTC (permalink / raw)
To: alexander.deucher, kenny.ho, christian.koenig; +Cc: nirmoy.das, amd-gfx
entity should not keep copy and maintain sched list for
itself.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 11 ++++++++---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
drivers/gpu/drm/scheduler/sched_entity.c | 12 +-----------
3 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index c7643af8827f..1939b414d23b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -71,7 +71,6 @@ static int amdgpu_ctx_priority_permit(struct drm_file *filp,
static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip)
{
struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
- struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
struct amdgpu_device *adev = ctx->adev;
unsigned num_rings = 0;
unsigned num_scheds = 0;
@@ -129,16 +128,21 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip)
break;
}
+ ctx->sched_list = kcalloc(num_rings,
+ sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
+ if (ctx->sched_list == NULL )
+ return -ENOMEM;
+
for (i = 0; i < num_rings; ++i) {
if (!rings[i]->adev)
continue;
- sched_list[num_scheds++] = &rings[i]->sched;
+ ctx->sched_list[num_scheds++] = &rings[i]->sched;
}
for (i = 0; i < amdgpu_ctx_num_entities[hw_ip]; ++i)
r = drm_sched_entity_init(&ctx->entities[hw_ip][i].entity,
- ctx->init_priority, sched_list, num_scheds, &ctx->guilty);
+ ctx->init_priority, ctx->sched_list, num_scheds, &ctx->guilty);
if (r)
goto error_cleanup_entities;
@@ -228,6 +232,7 @@ static void amdgpu_ctx_fini(struct kref *ref)
for (j = 0; j < amdgpu_sched_jobs; ++j)
dma_fence_put(ctx->entities[0][i].fences[j]);
kfree(ctx->fences);
+ kfree(ctx->sched_list);
kfree(ctx->entities[0]);
mutex_destroy(&ctx->lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index da808633732b..9fd02cc47352 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -44,6 +44,7 @@ struct amdgpu_ctx {
spinlock_t ring_lock;
struct dma_fence **fences;
struct amdgpu_ctx_entity *entities[AMDGPU_HW_IP_NUM];
+ struct drm_gpu_scheduler **sched_list;
bool preamble_presented;
enum drm_sched_priority init_priority;
enum drm_sched_priority override_priority;
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index f9b6ce29c58f..c84a9a66f7f0 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -56,8 +56,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
unsigned int num_sched_list,
atomic_t *guilty)
{
- int i;
-
if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
return -EINVAL;
@@ -67,17 +65,10 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
entity->guilty = guilty;
entity->num_sched_list = num_sched_list;
entity->priority = priority;
- entity->sched_list = kcalloc(num_sched_list,
- sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
-
- if(!entity->sched_list)
- return -ENOMEM;
+ entity->sched_list = sched_list;
init_completion(&entity->entity_idle);
- for (i = 0; i < num_sched_list; i++)
- entity->sched_list[i] = sched_list[i];
-
if (num_sched_list)
entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
@@ -312,7 +303,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
- kfree(entity->sched_list);
}
EXPORT_SYMBOL(drm_sched_entity_fini);
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 2/4] drm/amdgpu: replace vm_pte's run-queue list with drm gpu scheds list
2019-12-06 17:33 ` [PATCH 2/4] drm/amdgpu: replace vm_pte's run-queue list with drm gpu scheds list Nirmoy Das
@ 2019-12-06 19:37 ` Christian König
0 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2019-12-06 19:37 UTC (permalink / raw)
To: Nirmoy Das, alexander.deucher, kenny.ho, christian.koenig
Cc: nirmoy.das, amd-gfx
Am 06.12.19 um 18:33 schrieb Nirmoy Das:
> drm_sched_entity_init() takes drm gpu scheduler list instead of
> drm_sched_rq list. This makes conversion of drm_sched_rq list
> to drm gpu scheduler list unnecessary
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 ++++-------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++--
> drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 8 +++-----
> drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 8 +++-----
> drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 8 +++-----
> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 5 ++---
> drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 8 +++-----
> drivers/gpu/drm/amd/amdgpu/si_dma.c | 8 +++-----
> 9 files changed, 24 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index f85007382093..cf4953c4e2cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2779,7 +2779,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> adev->mman.buffer_funcs = NULL;
> adev->mman.buffer_funcs_ring = NULL;
> adev->vm_manager.vm_pte_funcs = NULL;
> - adev->vm_manager.vm_pte_num_rqs = 0;
> + adev->vm_manager.vm_pte_num_scheds = 0;
> adev->gmc.gmc_funcs = NULL;
> adev->fence_context = dma_fence_context_alloc(AMDGPU_MAX_RINGS);
> bitmap_zero(adev->gfx.pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 5e78db30c722..0e1ed8ef2ce7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2687,7 +2687,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> {
> struct amdgpu_bo_param bp;
> struct amdgpu_bo *root;
> - struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
> int r, i;
>
> vm->va = RB_ROOT_CACHED;
> @@ -2701,19 +2700,17 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> spin_lock_init(&vm->invalidated_lock);
> INIT_LIST_HEAD(&vm->freed);
>
> - for (i = 0; i < adev->vm_manager.vm_pte_num_rqs; i++)
> - sched_list[i] = adev->vm_manager.vm_pte_rqs[i]->sched;
>
> /* create scheduler entities for page table updates */
> r = drm_sched_entity_init(&vm->direct, DRM_SCHED_PRIORITY_NORMAL,
> - sched_list, adev->vm_manager.vm_pte_num_rqs,
> - NULL);
> + adev->vm_manager.vm_pte_scheds,
> + adev->vm_manager.vm_pte_num_scheds, NULL);
> if (r)
> return r;
>
> r = drm_sched_entity_init(&vm->delayed, DRM_SCHED_PRIORITY_NORMAL,
> - sched_list, adev->vm_manager.vm_pte_num_rqs,
> - NULL);
> + adev->vm_manager.vm_pte_scheds,
> + adev->vm_manager.vm_pte_num_scheds, NULL);
> if (r)
> goto error_free_direct;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 76fcf853035c..5eaba8645a43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -322,8 +322,8 @@ struct amdgpu_vm_manager {
> u64 vram_base_offset;
> /* vm pte handling */
> const struct amdgpu_vm_pte_funcs *vm_pte_funcs;
> - struct drm_sched_rq *vm_pte_rqs[AMDGPU_MAX_RINGS];
> - unsigned vm_pte_num_rqs;
> + struct drm_gpu_scheduler *vm_pte_scheds[AMDGPU_MAX_RINGS];
> + unsigned vm_pte_num_scheds;
> struct amdgpu_ring *page_fault;
>
> /* partial resident texture handling */
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 82cdb8f57bfd..1f22a8d0f7f3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -1373,16 +1373,14 @@ static const struct amdgpu_vm_pte_funcs cik_sdma_vm_pte_funcs = {
>
> static void cik_sdma_set_vm_pte_funcs(struct amdgpu_device *adev)
> {
> - struct drm_gpu_scheduler *sched;
> unsigned i;
>
> adev->vm_manager.vm_pte_funcs = &cik_sdma_vm_pte_funcs;
> for (i = 0; i < adev->sdma.num_instances; i++) {
> - sched = &adev->sdma.instance[i].ring.sched;
> - adev->vm_manager.vm_pte_rqs[i] =
> - &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
> + adev->vm_manager.vm_pte_scheds[i] =
> + &adev->sdma.instance[i].ring.sched;
> }
> - adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
> + adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> }
>
> const struct amdgpu_ip_block_version cik_sdma_ip_block =
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index 89e8c74a40f4..606b621145a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -1261,16 +1261,14 @@ static const struct amdgpu_vm_pte_funcs sdma_v2_4_vm_pte_funcs = {
>
> static void sdma_v2_4_set_vm_pte_funcs(struct amdgpu_device *adev)
> {
> - struct drm_gpu_scheduler *sched;
> unsigned i;
>
> adev->vm_manager.vm_pte_funcs = &sdma_v2_4_vm_pte_funcs;
> for (i = 0; i < adev->sdma.num_instances; i++) {
> - sched = &adev->sdma.instance[i].ring.sched;
> - adev->vm_manager.vm_pte_rqs[i] =
> - &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
> + adev->vm_manager.vm_pte_scheds[i] =
> + &adev->sdma.instance[i].ring.sched;
> }
> - adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
> + adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> }
>
> const struct amdgpu_ip_block_version sdma_v2_4_ip_block =
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 011fd12c41fe..a559573ec8fd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -1699,16 +1699,14 @@ static const struct amdgpu_vm_pte_funcs sdma_v3_0_vm_pte_funcs = {
>
> static void sdma_v3_0_set_vm_pte_funcs(struct amdgpu_device *adev)
> {
> - struct drm_gpu_scheduler *sched;
> unsigned i;
>
> adev->vm_manager.vm_pte_funcs = &sdma_v3_0_vm_pte_funcs;
> for (i = 0; i < adev->sdma.num_instances; i++) {
> - sched = &adev->sdma.instance[i].ring.sched;
> - adev->vm_manager.vm_pte_rqs[i] =
> - &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
> + adev->vm_manager.vm_pte_scheds[i] =
> + &adev->sdma.instance[i].ring.sched;
> }
> - adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
> + adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> }
>
> const struct amdgpu_ip_block_version sdma_v3_0_ip_block =
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 350b2c99fefc..bd9ed33bab43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -2411,10 +2411,9 @@ static void sdma_v4_0_set_vm_pte_funcs(struct amdgpu_device *adev)
> sched = &adev->sdma.instance[i].page.sched;
> else
> sched = &adev->sdma.instance[i].ring.sched;
> - adev->vm_manager.vm_pte_rqs[i] =
> - &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
> + adev->vm_manager.vm_pte_scheds[i] = sched;
> }
> - adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
> + adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> }
>
> const struct amdgpu_ip_block_version sdma_v4_0_ip_block = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index 64c53eed7fac..63f667cfe3f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -1723,17 +1723,15 @@ static const struct amdgpu_vm_pte_funcs sdma_v5_0_vm_pte_funcs = {
>
> static void sdma_v5_0_set_vm_pte_funcs(struct amdgpu_device *adev)
> {
> - struct drm_gpu_scheduler *sched;
> unsigned i;
>
> if (adev->vm_manager.vm_pte_funcs == NULL) {
> adev->vm_manager.vm_pte_funcs = &sdma_v5_0_vm_pte_funcs;
> for (i = 0; i < adev->sdma.num_instances; i++) {
> - sched = &adev->sdma.instance[i].ring.sched;
> - adev->vm_manager.vm_pte_rqs[i] =
> - &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
> + adev->vm_manager.vm_pte_scheds[i] =
> + &adev->sdma.instance[i].ring.sched;
> }
> - adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
> + adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> }
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index 122df0732f0c..9ad85eddf9c4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -835,16 +835,14 @@ static const struct amdgpu_vm_pte_funcs si_dma_vm_pte_funcs = {
>
> static void si_dma_set_vm_pte_funcs(struct amdgpu_device *adev)
> {
> - struct drm_gpu_scheduler *sched;
> unsigned i;
>
> adev->vm_manager.vm_pte_funcs = &si_dma_vm_pte_funcs;
> for (i = 0; i < adev->sdma.num_instances; i++) {
> - sched = &adev->sdma.instance[i].ring.sched;
> - adev->vm_manager.vm_pte_rqs[i] =
> - &sched->sched_rq[DRM_SCHED_PRIORITY_KERNEL];
> + adev->vm_manager.vm_pte_scheds[i] =
> + &adev->sdma.instance[i].ring.sched;
> }
> - adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
> + adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> }
>
> const struct amdgpu_ip_block_version si_dma_ip_block =
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] drm/amdgpu: allocate entities on demand
2019-12-06 17:33 ` [PATCH 3/4] drm/amdgpu: allocate entities on demand Nirmoy Das
@ 2019-12-06 19:40 ` Christian König
0 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2019-12-06 19:40 UTC (permalink / raw)
To: Nirmoy Das, alexander.deucher, kenny.ho, christian.koenig
Cc: nirmoy.das, amd-gfx
Am 06.12.19 um 18:33 schrieb Nirmoy Das:
> Currently we pre-allocate entities for all the HW IPs on
> context creation and some of which are might never be used.
>
> This patch tries to resolve entity wastage by creating entities
> for a HW IP only when it is required.
Please delay that until we have fully cleaned up the scheduler
initialization.
Christian.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 176 +++++++++++++-----------
> 1 file changed, 97 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 1d6850af9908..c7643af8827f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -68,13 +68,99 @@ static int amdgpu_ctx_priority_permit(struct drm_file *filp,
> return -EACCES;
> }
>
> +static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip)
> +{
> + struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
> + struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
> + struct amdgpu_device *adev = ctx->adev;
> + unsigned num_rings = 0;
> + unsigned num_scheds = 0;
> + unsigned i, j;
> + int r = 0;
> +
> + switch (hw_ip) {
> + case AMDGPU_HW_IP_GFX:
> + rings[0] = &adev->gfx.gfx_ring[0];
> + num_rings = 1;
> + break;
> + case AMDGPU_HW_IP_COMPUTE:
> + for (i = 0; i < adev->gfx.num_compute_rings; ++i)
> + rings[i] = &adev->gfx.compute_ring[i];
> + num_rings = adev->gfx.num_compute_rings;
> + break;
> + case AMDGPU_HW_IP_DMA:
> + for (i = 0; i < adev->sdma.num_instances; ++i)
> + rings[i] = &adev->sdma.instance[i].ring;
> + num_rings = adev->sdma.num_instances;
> + break;
> + case AMDGPU_HW_IP_UVD:
> + rings[0] = &adev->uvd.inst[0].ring;
> + num_rings = 1;
> + break;
> + case AMDGPU_HW_IP_VCE:
> + rings[0] = &adev->vce.ring[0];
> + num_rings = 1;
> + break;
> + case AMDGPU_HW_IP_UVD_ENC:
> + rings[0] = &adev->uvd.inst[0].ring_enc[0];
> + num_rings = 1;
> + break;
> + case AMDGPU_HW_IP_VCN_DEC:
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + if (adev->vcn.harvest_config & (1 << i))
> + continue;
> + rings[num_rings++] = &adev->vcn.inst[i].ring_dec;
> + }
> + break;
> + case AMDGPU_HW_IP_VCN_ENC:
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + if (adev->vcn.harvest_config & (1 << i))
> + continue;
> + for (j = 0; j < adev->vcn.num_enc_rings; ++j)
> + rings[num_rings++] = &adev->vcn.inst[i].ring_enc[j];
> + }
> + break;
> + case AMDGPU_HW_IP_VCN_JPEG:
> + for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
> + if (adev->vcn.harvest_config & (1 << i))
> + continue;
> + rings[num_rings++] = &adev->jpeg.inst[i].ring_dec;
> + }
> + break;
> + }
> +
> + for (i = 0; i < num_rings; ++i) {
> + if (!rings[i]->adev)
> + continue;
> +
> + sched_list[num_scheds++] = &rings[i]->sched;
> + }
> +
> + for (i = 0; i < amdgpu_ctx_num_entities[hw_ip]; ++i)
> + r = drm_sched_entity_init(&ctx->entities[hw_ip][i].entity,
> + ctx->init_priority, sched_list, num_scheds, &ctx->guilty);
> + if (r)
> + goto error_cleanup_entities;
> +
> + for (i = 0; i < amdgpu_ctx_num_entities[hw_ip]; ++i)
> + ctx->entities[hw_ip][i].sequence = 1;
> +
> + return 0;
> +
> +error_cleanup_entities:
> + for (i = 0; i < amdgpu_ctx_num_entities[hw_ip]; ++i)
> + drm_sched_entity_destroy(&ctx->entities[hw_ip][i].entity);
> +
> + return r;
> +}
> +
> static int amdgpu_ctx_init(struct amdgpu_device *adev,
> enum drm_sched_priority priority,
> struct drm_file *filp,
> struct amdgpu_ctx *ctx)
> {
> unsigned num_entities = amdgpu_ctx_total_num_entities();
> - unsigned i, j, k;
> + unsigned i;
> int r;
>
> if (priority < 0 || priority >= DRM_SCHED_PRIORITY_MAX)
> @@ -103,7 +189,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
> for (i = 0; i < num_entities; ++i) {
> struct amdgpu_ctx_entity *entity = &ctx->entities[0][i];
>
> - entity->sequence = 1;
> + entity->sequence = -1;
> entity->fences = &ctx->fences[amdgpu_sched_jobs * i];
> }
> for (i = 1; i < AMDGPU_HW_IP_NUM; ++i)
> @@ -120,85 +206,8 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
> ctx->init_priority = priority;
> ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
>
> - for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
> - struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
> - struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
> - unsigned num_rings = 0;
> - unsigned num_rqs = 0;
> -
> - switch (i) {
> - case AMDGPU_HW_IP_GFX:
> - rings[0] = &adev->gfx.gfx_ring[0];
> - num_rings = 1;
> - break;
> - case AMDGPU_HW_IP_COMPUTE:
> - for (j = 0; j < adev->gfx.num_compute_rings; ++j)
> - rings[j] = &adev->gfx.compute_ring[j];
> - num_rings = adev->gfx.num_compute_rings;
> - break;
> - case AMDGPU_HW_IP_DMA:
> - for (j = 0; j < adev->sdma.num_instances; ++j)
> - rings[j] = &adev->sdma.instance[j].ring;
> - num_rings = adev->sdma.num_instances;
> - break;
> - case AMDGPU_HW_IP_UVD:
> - rings[0] = &adev->uvd.inst[0].ring;
> - num_rings = 1;
> - break;
> - case AMDGPU_HW_IP_VCE:
> - rings[0] = &adev->vce.ring[0];
> - num_rings = 1;
> - break;
> - case AMDGPU_HW_IP_UVD_ENC:
> - rings[0] = &adev->uvd.inst[0].ring_enc[0];
> - num_rings = 1;
> - break;
> - case AMDGPU_HW_IP_VCN_DEC:
> - for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
> - if (adev->vcn.harvest_config & (1 << j))
> - continue;
> - rings[num_rings++] = &adev->vcn.inst[j].ring_dec;
> - }
> - break;
> - case AMDGPU_HW_IP_VCN_ENC:
> - for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
> - if (adev->vcn.harvest_config & (1 << j))
> - continue;
> - for (k = 0; k < adev->vcn.num_enc_rings; ++k)
> - rings[num_rings++] = &adev->vcn.inst[j].ring_enc[k];
> - }
> - break;
> - case AMDGPU_HW_IP_VCN_JPEG:
> - for (j = 0; j < adev->jpeg.num_jpeg_inst; ++j) {
> - if (adev->vcn.harvest_config & (1 << j))
> - continue;
> - rings[num_rings++] = &adev->jpeg.inst[j].ring_dec;
> - }
> - break;
> - }
> -
> - for (j = 0; j < num_rings; ++j) {
> - if (!rings[j]->adev)
> - continue;
> -
> - sched_list[num_rqs++] = &rings[j]->sched;
> - }
> -
> - for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
> - r = drm_sched_entity_init(&ctx->entities[i][j].entity,
> - priority, sched_list,
> - num_rqs, &ctx->guilty);
> - if (r)
> - goto error_cleanup_entities;
> - }
> -
> return 0;
>
> -error_cleanup_entities:
> - for (i = 0; i < num_entities; ++i)
> - drm_sched_entity_destroy(&ctx->entities[0][i].entity);
> - kfree(ctx->entities[0]);
> -
> error_free_fences:
> kfree(ctx->fences);
> ctx->fences = NULL;
> @@ -229,6 +238,8 @@ static void amdgpu_ctx_fini(struct kref *ref)
> int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 hw_ip, u32 instance,
> u32 ring, struct drm_sched_entity **entity)
> {
> + int r;
> +
> if (hw_ip >= AMDGPU_HW_IP_NUM) {
> DRM_ERROR("unknown HW IP type: %d\n", hw_ip);
> return -EINVAL;
> @@ -245,6 +256,13 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 hw_ip, u32 instance,
> return -EINVAL;
> }
>
> + if (ctx->entities[hw_ip][ring].sequence == -1) {
> + r = amdgpu_ctx_init_entity(ctx, hw_ip);
> +
> + if (r)
> + return r;
> + }
> +
> *entity = &ctx->entities[hw_ip][ring].entity;
> return 0;
> }
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-06 17:33 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
@ 2019-12-06 19:41 ` Christian König
2019-12-08 19:57 ` Nirmoy
0 siblings, 1 reply; 25+ messages in thread
From: Christian König @ 2019-12-06 19:41 UTC (permalink / raw)
To: Nirmoy Das, alexander.deucher, kenny.ho, christian.koenig
Cc: nirmoy.das, amd-gfx
Am 06.12.19 um 18:33 schrieb Nirmoy Das:
> entity should not keep copy and maintain sched list for
> itself.
That is a good step, but we need to take this further.
The sched_list is static for the whole device and we shouldn't allocate
it inside the context at all.
Christian.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 11 ++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> drivers/gpu/drm/scheduler/sched_entity.c | 12 +-----------
> 3 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index c7643af8827f..1939b414d23b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -71,7 +71,6 @@ static int amdgpu_ctx_priority_permit(struct drm_file *filp,
> static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip)
> {
> struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
> - struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
> struct amdgpu_device *adev = ctx->adev;
> unsigned num_rings = 0;
> unsigned num_scheds = 0;
> @@ -129,16 +128,21 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip)
> break;
> }
>
> + ctx->sched_list = kcalloc(num_rings,
> + sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
> + if (ctx->sched_list == NULL )
> + return -ENOMEM;
> +
> for (i = 0; i < num_rings; ++i) {
> if (!rings[i]->adev)
> continue;
>
> - sched_list[num_scheds++] = &rings[i]->sched;
> + ctx->sched_list[num_scheds++] = &rings[i]->sched;
> }
>
> for (i = 0; i < amdgpu_ctx_num_entities[hw_ip]; ++i)
> r = drm_sched_entity_init(&ctx->entities[hw_ip][i].entity,
> - ctx->init_priority, sched_list, num_scheds, &ctx->guilty);
> + ctx->init_priority, ctx->sched_list, num_scheds, &ctx->guilty);
> if (r)
> goto error_cleanup_entities;
>
> @@ -228,6 +232,7 @@ static void amdgpu_ctx_fini(struct kref *ref)
> for (j = 0; j < amdgpu_sched_jobs; ++j)
> dma_fence_put(ctx->entities[0][i].fences[j]);
> kfree(ctx->fences);
> + kfree(ctx->sched_list);
> kfree(ctx->entities[0]);
>
> mutex_destroy(&ctx->lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index da808633732b..9fd02cc47352 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -44,6 +44,7 @@ struct amdgpu_ctx {
> spinlock_t ring_lock;
> struct dma_fence **fences;
> struct amdgpu_ctx_entity *entities[AMDGPU_HW_IP_NUM];
> + struct drm_gpu_scheduler **sched_list;
> bool preamble_presented;
> enum drm_sched_priority init_priority;
> enum drm_sched_priority override_priority;
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index f9b6ce29c58f..c84a9a66f7f0 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -56,8 +56,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
> unsigned int num_sched_list,
> atomic_t *guilty)
> {
> - int i;
> -
> if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
> return -EINVAL;
>
> @@ -67,17 +65,10 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
> entity->guilty = guilty;
> entity->num_sched_list = num_sched_list;
> entity->priority = priority;
> - entity->sched_list = kcalloc(num_sched_list,
> - sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
> -
> - if(!entity->sched_list)
> - return -ENOMEM;
> + entity->sched_list = sched_list;
>
> init_completion(&entity->entity_idle);
>
> - for (i = 0; i < num_sched_list; i++)
> - entity->sched_list[i] = sched_list[i];
> -
> if (num_sched_list)
> entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
>
> @@ -312,7 +303,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
>
> dma_fence_put(entity->last_scheduled);
> entity->last_scheduled = NULL;
> - kfree(entity->sched_list);
> }
> EXPORT_SYMBOL(drm_sched_entity_fini);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-06 19:41 ` Christian König
@ 2019-12-08 19:57 ` Nirmoy
2019-12-09 12:20 ` Christian König
0 siblings, 1 reply; 25+ messages in thread
From: Nirmoy @ 2019-12-08 19:57 UTC (permalink / raw)
To: christian.koenig, Nirmoy Das, alexander.deucher, kenny.ho
Cc: nirmoy.das, amd-gfx
On 12/6/19 8:41 PM, Christian König wrote:
> Am 06.12.19 um 18:33 schrieb Nirmoy Das:
>> entity should not keep copy and maintain sched list for
>> itself.
>
> That is a good step, but we need to take this further.
How about something like ?
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 0ae0a2715b0d..a71ee084b47a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -269,8 +269,10 @@ struct amdgpu_gfx {
bool me_fw_write_wait;
bool cp_fw_write_wait;
struct amdgpu_ring gfx_ring[AMDGPU_MAX_GFX_RINGS];
+ struct drm_gpu_scheduler *gfx_sched_list[AMDGPU_MAX_GFX_RINGS];
unsigned num_gfx_rings;
struct amdgpu_ring compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
+ struct drm_gpu_scheduler
*compute_sched_list[AMDGPU_MAX_COMPUTE_RINGS];
unsigned num_compute_rings;
struct amdgpu_irq_src eop_irq;
struct amdgpu_irq_src priv_reg_irq;
Regards,
Nirmoy
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-08 19:57 ` Nirmoy
@ 2019-12-09 12:20 ` Christian König
2019-12-09 13:56 ` Nirmoy
0 siblings, 1 reply; 25+ messages in thread
From: Christian König @ 2019-12-09 12:20 UTC (permalink / raw)
To: Nirmoy, Nirmoy Das, alexander.deucher, kenny.ho; +Cc: nirmoy.das, amd-gfx
Yes, you need to do this for the SDMA as well but in general that looks
like the idea I had in mind as well.
I would do it like this:
1. Change the special case when you only get one scheduler for an entity
to drop the pointer to the scheduler list.
This way we always use the same scheduler for the entity and can
pass in the array on the stack.
2. Change all callers which use more than one scheduler in the list to
pass in pointers which are not allocated on the stack.
This obviously also means that we build the list of schedulers for
each type only once during device init and not for each context init.
3. Make the scheduler list const and drop the kcalloc()/kfree() from the
entity code.
Regards,
Christian.
Am 08.12.19 um 20:57 schrieb Nirmoy:
>
> On 12/6/19 8:41 PM, Christian König wrote:
>> Am 06.12.19 um 18:33 schrieb Nirmoy Das:
>>> entity should not keep copy and maintain sched list for
>>> itself.
>>
>> That is a good step, but we need to take this further.
>
> How about something like ?
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 0ae0a2715b0d..a71ee084b47a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -269,8 +269,10 @@ struct amdgpu_gfx {
> bool me_fw_write_wait;
> bool cp_fw_write_wait;
> struct amdgpu_ring gfx_ring[AMDGPU_MAX_GFX_RINGS];
> + struct drm_gpu_scheduler *gfx_sched_list[AMDGPU_MAX_GFX_RINGS];
> unsigned num_gfx_rings;
> struct amdgpu_ring compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
> + struct drm_gpu_scheduler
> *compute_sched_list[AMDGPU_MAX_COMPUTE_RINGS];
> unsigned num_compute_rings;
> struct amdgpu_irq_src eop_irq;
> struct amdgpu_irq_src priv_reg_irq;
>
>
> Regards,
>
> Nirmoy
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-09 12:20 ` Christian König
@ 2019-12-09 13:56 ` Nirmoy
2019-12-09 14:09 ` Nirmoy
0 siblings, 1 reply; 25+ messages in thread
From: Nirmoy @ 2019-12-09 13:56 UTC (permalink / raw)
To: Christian König, Nirmoy Das, alexander.deucher, kenny.ho
Cc: nirmoy.das, amd-gfx
Hi Christian,
I got a different idea, a bit more simple let me know what do you think
about it:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 50bab33cba39..8de4de4f7a43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -870,6 +870,7 @@ struct amdgpu_device {
u64 fence_context;
unsigned num_rings;
struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
+ struct drm_gpu_scheduler *rings_sched_list[AMDGPU_MAX_RINGS];
bool ib_pool_ready;
struct amdgpu_sa_manager ring_tmp_bo;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 1d6850af9908..52b3a5d85a1d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -122,9 +122,8 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
- struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS]
+ struct drm_gpu_scheduler **sched_list;
unsigned num_rings = 0;
- unsigned num_rqs = 0;
switch (i) {
case AMDGPU_HW_IP_GFX:
@@ -177,17 +176,11 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
break;
}
- for (j = 0; j < num_rings; ++j) {
- if (!rings[j]->adev)
- continue;
-
- sched_list[num_rqs++] = &rings[j]->sched;
- }
-
+ sched_list= adev->rings_sched_list+rings[0]->idx;
for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
r =
drm_sched_entity_init(&ctx->entities[i][j].entity,
priority, sched_list,
- num_rqs, &ctx->guilty);
+ num_rings, &ctx->guilty);
if (r)
goto error_cleanup_entities;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 377fe20bce23..e8cfa357e445 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -480,6 +480,8 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring
*ring,
ring->name);
return r;
}
+
+ adev->rings_sched_list[ring->idx] = &ring->sched;
}
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index bd9ed33bab43..bfe36199ffed 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1744,8 +1744,11 @@ static int sdma_v4_0_sw_init(void *handle)
AMDGPU_SDMA_IRQ_INSTANCE0 + i);
if (r)
return r;
+ }
+
+ if (adev->sdma.has_page_queue) {
+ for (i = 0; i < adev->sdma.num_instances; i++) {
- if (adev->sdma.has_page_queue) {
ring = &adev->sdma.instance[i].page;
ring->ring_obj = NULL;
ring->use_doorbell = true;
It relies on contiguous ring initialization that's why I had to change
sdma_v4_0.c so that we do ring_init(sdma0, sdma1, page0, page1}
instead of ring_init{sdma0, page0, sdma1, page1}
Regards,
Nirmoy
On 12/9/19 1:20 PM, Christian König wrote:
> Yes, you need to do this for the SDMA as well but in general that
> looks like the idea I had in mind as well.
>
> I would do it like this:
>
> 1. Change the special case when you only get one scheduler for an
> entity to drop the pointer to the scheduler list.
> This way we always use the same scheduler for the entity and can
> pass in the array on the stack.
>
> 2. Change all callers which use more than one scheduler in the list to
> pass in pointers which are not allocated on the stack.
> This obviously also means that we build the list of schedulers for
> each type only once during device init and not for each context init.
>
> 3. Make the scheduler list const and drop the kcalloc()/kfree() from
> the entity code.
>
> Regards,
> Christian.
>
> Am 08.12.19 um 20:57 schrieb Nirmoy:
>>
>> On 12/6/19 8:41 PM, Christian König wrote:
>>> Am 06.12.19 um 18:33 schrieb Nirmoy Das:
>>>> entity should not keep copy and maintain sched list for
>>>> itself.
>>>
>>> That is a good step, but we need to take this further.
>>
>> How about something like ?
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> index 0ae0a2715b0d..a71ee084b47a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> @@ -269,8 +269,10 @@ struct amdgpu_gfx {
>> bool me_fw_write_wait;
>> bool cp_fw_write_wait;
>> struct amdgpu_ring gfx_ring[AMDGPU_MAX_GFX_RINGS];
>> + struct drm_gpu_scheduler *gfx_sched_list[AMDGPU_MAX_GFX_RINGS];
>> unsigned num_gfx_rings;
>> struct amdgpu_ring compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
>> + struct drm_gpu_scheduler
>> *compute_sched_list[AMDGPU_MAX_COMPUTE_RINGS];
>> unsigned num_compute_rings;
>> struct amdgpu_irq_src eop_irq;
>> struct amdgpu_irq_src priv_reg_irq;
>>
>>
>> Regards,
>>
>> Nirmoy
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-09 13:56 ` Nirmoy
@ 2019-12-09 14:09 ` Nirmoy
2019-12-09 15:37 ` Christian König
0 siblings, 1 reply; 25+ messages in thread
From: Nirmoy @ 2019-12-09 14:09 UTC (permalink / raw)
To: Christian König, alexander.deucher, kenny.ho; +Cc: nirmoy.das, amd-gfx
I can see one issue with this. I am ignoring/removing changes from
commit 2a84e48e9712ea8591a10dd59d59ccab3d54efd6 drm/amdgpu: Only add rqs
for initialized rings.
I wonder if we can handle that differently.
Regards,
Nirmoy
On 12/9/19 2:56 PM, Nirmoy wrote:
> Hi Christian,
>
> I got a different idea, a bit more simple let me know what do you
> think about it:
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 50bab33cba39..8de4de4f7a43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -870,6 +870,7 @@ struct amdgpu_device {
> u64 fence_context;
> unsigned num_rings;
> struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
> + struct drm_gpu_scheduler *rings_sched_list[AMDGPU_MAX_RINGS];
> bool ib_pool_ready;
> struct amdgpu_sa_manager ring_tmp_bo;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 1d6850af9908..52b3a5d85a1d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -122,9 +122,8 @@ static int amdgpu_ctx_init(struct amdgpu_device
> *adev,
>
> for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
> struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
> - struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS]
> + struct drm_gpu_scheduler **sched_list;
> unsigned num_rings = 0;
> - unsigned num_rqs = 0;
>
> switch (i) {
> case AMDGPU_HW_IP_GFX:
> @@ -177,17 +176,11 @@ static int amdgpu_ctx_init(struct amdgpu_device
> *adev,
> break;
> }
>
> - for (j = 0; j < num_rings; ++j) {
> - if (!rings[j]->adev)
> - continue;
> -
> - sched_list[num_rqs++] = &rings[j]->sched;
> - }
> -
> + sched_list= adev->rings_sched_list+rings[0]->idx;
> for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
> r =
> drm_sched_entity_init(&ctx->entities[i][j].entity,
> priority, sched_list,
> - num_rqs, &ctx->guilty);
> + num_rings,
> &ctx->guilty);
> if (r)
> goto error_cleanup_entities;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 377fe20bce23..e8cfa357e445 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -480,6 +480,8 @@ int amdgpu_fence_driver_init_ring(struct
> amdgpu_ring *ring,
> ring->name);
> return r;
> }
> +
> + adev->rings_sched_list[ring->idx] = &ring->sched;
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index bd9ed33bab43..bfe36199ffed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -1744,8 +1744,11 @@ static int sdma_v4_0_sw_init(void *handle)
> AMDGPU_SDMA_IRQ_INSTANCE0 + i);
> if (r)
> return r;
> + }
> +
> + if (adev->sdma.has_page_queue) {
> + for (i = 0; i < adev->sdma.num_instances; i++) {
>
> - if (adev->sdma.has_page_queue) {
> ring = &adev->sdma.instance[i].page;
> ring->ring_obj = NULL;
> ring->use_doorbell = true;
>
> It relies on contiguous ring initialization that's why I had to
> change sdma_v4_0.c so that we do ring_init(sdma0, sdma1, page0, page1}
>
> instead of ring_init{sdma0, page0, sdma1, page1}
>
>
> Regards,
>
> Nirmoy
>
> On 12/9/19 1:20 PM, Christian König wrote:
>> Yes, you need to do this for the SDMA as well but in general that
>> looks like the idea I had in mind as well.
>>
>> I would do it like this:
>>
>> 1. Change the special case when you only get one scheduler for an
>> entity to drop the pointer to the scheduler list.
>> This way we always use the same scheduler for the entity and can
>> pass in the array on the stack.
>>
>> 2. Change all callers which use more than one scheduler in the list
>> to pass in pointers which are not allocated on the stack.
>> This obviously also means that we build the list of schedulers
>> for each type only once during device init and not for each context
>> init.
>>
>> 3. Make the scheduler list const and drop the kcalloc()/kfree() from
>> the entity code.
>>
>> Regards,
>> Christian.
>>
>> Am 08.12.19 um 20:57 schrieb Nirmoy:
>>>
>>> On 12/6/19 8:41 PM, Christian König wrote:
>>>> Am 06.12.19 um 18:33 schrieb Nirmoy Das:
>>>>> entity should not keep copy and maintain sched list for
>>>>> itself.
>>>>
>>>> That is a good step, but we need to take this further.
>>>
>>> How about something like ?
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> index 0ae0a2715b0d..a71ee084b47a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> @@ -269,8 +269,10 @@ struct amdgpu_gfx {
>>> bool me_fw_write_wait;
>>> bool cp_fw_write_wait;
>>> struct amdgpu_ring gfx_ring[AMDGPU_MAX_GFX_RINGS];
>>> + struct drm_gpu_scheduler *gfx_sched_list[AMDGPU_MAX_GFX_RINGS];
>>> unsigned num_gfx_rings;
>>> struct amdgpu_ring compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
>>> + struct drm_gpu_scheduler
>>> *compute_sched_list[AMDGPU_MAX_COMPUTE_RINGS];
>>> unsigned num_compute_rings;
>>> struct amdgpu_irq_src eop_irq;
>>> struct amdgpu_irq_src priv_reg_irq;
>>>
>>>
>>> Regards,
>>>
>>> Nirmoy
>>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-09 14:09 ` Nirmoy
@ 2019-12-09 15:37 ` Christian König
0 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2019-12-09 15:37 UTC (permalink / raw)
To: Nirmoy, alexander.deucher, kenny.ho; +Cc: nirmoy.das, amd-gfx
Yeah, that won't work very well.
During bring-up we also often have the case that we can't correctly
initialize all engines, e.g. only the first SDMA comes up etc.
So better stick with the initial approach of constructing the scheduler
array for each engine type which needs it.
Regards,
Christian.
Am 09.12.19 um 15:09 schrieb Nirmoy:
> I can see one issue with this. I am ignoring/removing changes from
>
> commit 2a84e48e9712ea8591a10dd59d59ccab3d54efd6 drm/amdgpu: Only add
> rqs for initialized rings.
>
> I wonder if we can handle that differently.
>
> Regards,
>
> Nirmoy
>
> On 12/9/19 2:56 PM, Nirmoy wrote:
>> Hi Christian,
>>
>> I got a different idea, a bit more simple let me know what do you
>> think about it:
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 50bab33cba39..8de4de4f7a43 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -870,6 +870,7 @@ struct amdgpu_device {
>> u64 fence_context;
>> unsigned num_rings;
>> struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
>> + struct drm_gpu_scheduler *rings_sched_list[AMDGPU_MAX_RINGS];
>> bool ib_pool_ready;
>> struct amdgpu_sa_manager ring_tmp_bo;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index 1d6850af9908..52b3a5d85a1d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -122,9 +122,8 @@ static int amdgpu_ctx_init(struct amdgpu_device
>> *adev,
>>
>> for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
>> struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
>> - struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS]
>> + struct drm_gpu_scheduler **sched_list;
>> unsigned num_rings = 0;
>> - unsigned num_rqs = 0;
>>
>> switch (i) {
>> case AMDGPU_HW_IP_GFX:
>> @@ -177,17 +176,11 @@ static int amdgpu_ctx_init(struct amdgpu_device
>> *adev,
>> break;
>> }
>>
>> - for (j = 0; j < num_rings; ++j) {
>> - if (!rings[j]->adev)
>> - continue;
>> -
>> - sched_list[num_rqs++] = &rings[j]->sched;
>> - }
>> -
>> + sched_list= adev->rings_sched_list+rings[0]->idx;
>> for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
>> r =
>> drm_sched_entity_init(&ctx->entities[i][j].entity,
>> priority, sched_list,
>> - num_rqs,
>> &ctx->guilty);
>> + num_rings,
>> &ctx->guilty);
>> if (r)
>> goto error_cleanup_entities;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> index 377fe20bce23..e8cfa357e445 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> @@ -480,6 +480,8 @@ int amdgpu_fence_driver_init_ring(struct
>> amdgpu_ring *ring,
>> ring->name);
>> return r;
>> }
>> +
>> + adev->rings_sched_list[ring->idx] = &ring->sched;
>> }
>>
>> return 0;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> index bd9ed33bab43..bfe36199ffed 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> @@ -1744,8 +1744,11 @@ static int sdma_v4_0_sw_init(void *handle)
>> AMDGPU_SDMA_IRQ_INSTANCE0 + i);
>> if (r)
>> return r;
>> + }
>> +
>> + if (adev->sdma.has_page_queue) {
>> + for (i = 0; i < adev->sdma.num_instances; i++) {
>>
>> - if (adev->sdma.has_page_queue) {
>> ring = &adev->sdma.instance[i].page;
>> ring->ring_obj = NULL;
>> ring->use_doorbell = true;
>>
>> It relies on contiguous ring initialization that's why I had to
>> change sdma_v4_0.c so that we do ring_init(sdma0, sdma1, page0, page1}
>>
>> instead of ring_init{sdma0, page0, sdma1, page1}
>>
>>
>> Regards,
>>
>> Nirmoy
>>
>> On 12/9/19 1:20 PM, Christian König wrote:
>>> Yes, you need to do this for the SDMA as well but in general that
>>> looks like the idea I had in mind as well.
>>>
>>> I would do it like this:
>>>
>>> 1. Change the special case when you only get one scheduler for an
>>> entity to drop the pointer to the scheduler list.
>>> This way we always use the same scheduler for the entity and can
>>> pass in the array on the stack.
>>>
>>> 2. Change all callers which use more than one scheduler in the list
>>> to pass in pointers which are not allocated on the stack.
>>> This obviously also means that we build the list of schedulers
>>> for each type only once during device init and not for each context
>>> init.
>>>
>>> 3. Make the scheduler list const and drop the kcalloc()/kfree() from
>>> the entity code.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 08.12.19 um 20:57 schrieb Nirmoy:
>>>>
>>>> On 12/6/19 8:41 PM, Christian König wrote:
>>>>> Am 06.12.19 um 18:33 schrieb Nirmoy Das:
>>>>>> entity should not keep copy and maintain sched list for
>>>>>> itself.
>>>>>
>>>>> That is a good step, but we need to take this further.
>>>>
>>>> How about something like ?
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>>> index 0ae0a2715b0d..a71ee084b47a 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>>> @@ -269,8 +269,10 @@ struct amdgpu_gfx {
>>>> bool me_fw_write_wait;
>>>> bool cp_fw_write_wait;
>>>> struct amdgpu_ring gfx_ring[AMDGPU_MAX_GFX_RINGS];
>>>> + struct drm_gpu_scheduler
>>>> *gfx_sched_list[AMDGPU_MAX_GFX_RINGS];
>>>> unsigned num_gfx_rings;
>>>> struct amdgpu_ring compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
>>>> + struct drm_gpu_scheduler
>>>> *compute_sched_list[AMDGPU_MAX_COMPUTE_RINGS];
>>>> unsigned num_compute_rings;
>>>> struct amdgpu_irq_src eop_irq;
>>>> struct amdgpu_irq_src priv_reg_irq;
>>>>
>>>>
>>>> Regards,
>>>>
>>>> Nirmoy
>>>>
>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-11 15:42 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
@ 2019-12-11 15:42 ` Nirmoy Das
0 siblings, 0 replies; 25+ messages in thread
From: Nirmoy Das @ 2019-12-11 15:42 UTC (permalink / raw)
To: dri-devel
Cc: alexander.deucher, kenny.ho, nirmoy.das, christian.koenig, amd-gfx
entity should not keep copy and maintain sched list for
itself.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/scheduler/sched_entity.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index f9b6ce29c58f..2e3a058fc239 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -56,8 +56,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
unsigned int num_sched_list,
atomic_t *guilty)
{
- int i;
-
if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
return -EINVAL;
@@ -67,22 +65,14 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
entity->guilty = guilty;
entity->num_sched_list = num_sched_list;
entity->priority = priority;
- entity->sched_list = kcalloc(num_sched_list,
- sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
+ entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
+ entity->last_scheduled = NULL;
- if(!entity->sched_list)
- return -ENOMEM;
+ if(num_sched_list)
+ entity->rq = &sched_list[0]->sched_rq[entity->priority];
init_completion(&entity->entity_idle);
- for (i = 0; i < num_sched_list; i++)
- entity->sched_list[i] = sched_list[i];
-
- if (num_sched_list)
- entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
-
- entity->last_scheduled = NULL;
-
spin_lock_init(&entity->rq_lock);
spsc_queue_init(&entity->job_queue);
@@ -312,7 +302,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
- kfree(entity->sched_list);
}
EXPORT_SYMBOL(drm_sched_entity_fini);
--
2.24.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
@ 2019-12-11 15:42 ` Nirmoy Das
0 siblings, 0 replies; 25+ messages in thread
From: Nirmoy Das @ 2019-12-11 15:42 UTC (permalink / raw)
To: dri-devel
Cc: alexander.deucher, kenny.ho, nirmoy.das, christian.koenig, amd-gfx
entity should not keep copy and maintain sched list for
itself.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/scheduler/sched_entity.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index f9b6ce29c58f..2e3a058fc239 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -56,8 +56,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
unsigned int num_sched_list,
atomic_t *guilty)
{
- int i;
-
if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
return -EINVAL;
@@ -67,22 +65,14 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
entity->guilty = guilty;
entity->num_sched_list = num_sched_list;
entity->priority = priority;
- entity->sched_list = kcalloc(num_sched_list,
- sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
+ entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
+ entity->last_scheduled = NULL;
- if(!entity->sched_list)
- return -ENOMEM;
+ if(num_sched_list)
+ entity->rq = &sched_list[0]->sched_rq[entity->priority];
init_completion(&entity->entity_idle);
- for (i = 0; i < num_sched_list; i++)
- entity->sched_list[i] = sched_list[i];
-
- if (num_sched_list)
- entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
-
- entity->last_scheduled = NULL;
-
spin_lock_init(&entity->rq_lock);
spsc_queue_init(&entity->job_queue);
@@ -312,7 +302,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
- kfree(entity->sched_list);
}
EXPORT_SYMBOL(drm_sched_entity_fini);
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-11 14:24 [PATCH 1/4 v2] drm/scheduler: rework entity creation Nirmoy Das
@ 2019-12-11 14:24 ` Nirmoy Das
0 siblings, 0 replies; 25+ messages in thread
From: Nirmoy Das @ 2019-12-11 14:24 UTC (permalink / raw)
To: alexander.deucher, kenny.ho, christian.koenig; +Cc: nirmoy.das, amd-gfx
entity should not keep copy and maintain sched list for
itself.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/scheduler/sched_entity.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index f9b6ce29c58f..2e3a058fc239 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -56,8 +56,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
unsigned int num_sched_list,
atomic_t *guilty)
{
- int i;
-
if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
return -EINVAL;
@@ -67,22 +65,14 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
entity->guilty = guilty;
entity->num_sched_list = num_sched_list;
entity->priority = priority;
- entity->sched_list = kcalloc(num_sched_list,
- sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
+ entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
+ entity->last_scheduled = NULL;
- if(!entity->sched_list)
- return -ENOMEM;
+ if(num_sched_list)
+ entity->rq = &sched_list[0]->sched_rq[entity->priority];
init_completion(&entity->entity_idle);
- for (i = 0; i < num_sched_list; i++)
- entity->sched_list[i] = sched_list[i];
-
- if (num_sched_list)
- entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
-
- entity->last_scheduled = NULL;
-
spin_lock_init(&entity->rq_lock);
spsc_queue_init(&entity->job_queue);
@@ -312,7 +302,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
- kfree(entity->sched_list);
}
EXPORT_SYMBOL(drm_sched_entity_fini);
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 18:17 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
@ 2019-12-11 12:25 ` Christian König
0 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2019-12-11 12:25 UTC (permalink / raw)
To: Nirmoy Das, alexander.deucher, kenny.ho, christian.koenig
Cc: nirmoy.das, amd-gfx
Am 10.12.19 um 19:17 schrieb Nirmoy Das:
> entity should not keep copy and maintain sched list for
> itself.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/scheduler/sched_entity.c | 19 ++++---------------
> 1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index f9b6ce29c58f..2e3a058fc239 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -56,8 +56,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
> unsigned int num_sched_list,
> atomic_t *guilty)
> {
> - int i;
> -
> if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
> return -EINVAL;
>
> @@ -67,22 +65,14 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
> entity->guilty = guilty;
> entity->num_sched_list = num_sched_list;
> entity->priority = priority;
> - entity->sched_list = kcalloc(num_sched_list,
> - sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
> + entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
> + entity->last_scheduled = NULL;
>
> - if(!entity->sched_list)
> - return -ENOMEM;
> + if(num_sched_list)
> + entity->rq = &sched_list[0]->sched_rq[entity->priority];
>
> init_completion(&entity->entity_idle);
>
> - for (i = 0; i < num_sched_list; i++)
> - entity->sched_list[i] = sched_list[i];
> -
> - if (num_sched_list)
> - entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
> -
> - entity->last_scheduled = NULL;
> -
> spin_lock_init(&entity->rq_lock);
> spsc_queue_init(&entity->job_queue);
>
> @@ -312,7 +302,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
>
> dma_fence_put(entity->last_scheduled);
> entity->last_scheduled = NULL;
> - kfree(entity->sched_list);
> }
> EXPORT_SYMBOL(drm_sched_entity_fini);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 17:32 ` Christian König
@ 2019-12-10 18:38 ` Nirmoy
0 siblings, 0 replies; 25+ messages in thread
From: Nirmoy @ 2019-12-10 18:38 UTC (permalink / raw)
To: Christian König, alexander.deucher, kenny.ho; +Cc: nirmoy.das, amd-gfx
On 12/10/19 6:32 PM, Christian König wrote:
>
>>>> Maybe make this "num_sched_list > 1 ? sched_list : NULL" to avoid
>>>> accidentally dereferencing a stale pointer to the stack.
>>> Do you mean "num_sched_list >= 1 ? sched_list : NULL"
>
> No, the entity->sched_list field should be NULL when num_sched_list==1.
>
> When num_sched_list==1 the entity->sched_list shouldn't be used and we
> can use a dummy list on the stack as parameter. But we should set the
> pointer to NULL in this case just to make sure that nobody is
> dereferencing it.
Okay I understand now.
>
>>>>
>>>>> - if(!entity->sched_list)
>>>>> - return -ENOMEM;
>>>>> init_completion(&entity->entity_idle);
>>>>> -
>>>>> - for (i = 0; i < num_sched_list; i++)
>>>>> - entity->sched_list[i] = sched_list[i];
>>>>> -
>>>>> if (num_sched_list)
>>>>
>>>> That check can actually be dropped as well. We return -EINVAL when
>>>> the num_sched_list is zero.
>>>
>>> This one took me some time to understand. So we don't really return
>>> -EINVAL if num_sched_list == 0
>>>
>>> if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
>>> return -EINVAL;
>>>
>>> This is coming from below patch. Are we suppose to tolerate IPs with
>>> uninitialized sched so that ctx creation dosn't return error ?
>
> Yes, exactly. That's intentionally this way.
>
> GPU reset sometimes resulted in schedulers being disabled because we
> couldn't re-init them. In this case we had entities with an empty
> scheduler list.
>
> That can't happen any more after you make the scheduler arrays
> constant, but I would stick with that behavior.
So I will keep the check.
>
> Regards,
> Christian.
>
>
Regards,
Nirmoy
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 18:17 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
@ 2019-12-10 18:17 ` Nirmoy Das
2019-12-11 12:25 ` Christian König
0 siblings, 1 reply; 25+ messages in thread
From: Nirmoy Das @ 2019-12-10 18:17 UTC (permalink / raw)
To: alexander.deucher, kenny.ho, christian.koenig; +Cc: nirmoy.das, amd-gfx
entity should not keep copy and maintain sched list for
itself.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/scheduler/sched_entity.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index f9b6ce29c58f..2e3a058fc239 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -56,8 +56,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
unsigned int num_sched_list,
atomic_t *guilty)
{
- int i;
-
if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
return -EINVAL;
@@ -67,22 +65,14 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
entity->guilty = guilty;
entity->num_sched_list = num_sched_list;
entity->priority = priority;
- entity->sched_list = kcalloc(num_sched_list,
- sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
+ entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
+ entity->last_scheduled = NULL;
- if(!entity->sched_list)
- return -ENOMEM;
+ if(num_sched_list)
+ entity->rq = &sched_list[0]->sched_rq[entity->priority];
init_completion(&entity->entity_idle);
- for (i = 0; i < num_sched_list; i++)
- entity->sched_list[i] = sched_list[i];
-
- if (num_sched_list)
- entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
-
- entity->last_scheduled = NULL;
-
spin_lock_init(&entity->rq_lock);
spsc_queue_init(&entity->job_queue);
@@ -312,7 +302,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
- kfree(entity->sched_list);
}
EXPORT_SYMBOL(drm_sched_entity_fini);
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 15:08 ` Nirmoy
@ 2019-12-10 17:32 ` Christian König
2019-12-10 18:38 ` Nirmoy
0 siblings, 1 reply; 25+ messages in thread
From: Christian König @ 2019-12-10 17:32 UTC (permalink / raw)
To: Nirmoy, alexander.deucher, kenny.ho; +Cc: nirmoy.das, amd-gfx
Am 10.12.19 um 16:08 schrieb Nirmoy:
> I think amdgpu_ctx_init() should check for num_scheds and not call
> drm_sched_entity_init()
>
> if its zero.
Ah, that's where that came from. No that is intentionally this way, but
see below.
>
> On 12/10/19 3:47 PM, Nirmoy wrote:
>>
>> On 12/10/19 2:00 PM, Christian König wrote:
>>> Am 10.12.19 um 13:53 schrieb Nirmoy Das:
>>>> entity should not keep copy and maintain sched list for
>>>> itself.
>>>>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>> ---
>>>> drivers/gpu/drm/scheduler/sched_entity.c | 10 +---------
>>>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>>>> b/drivers/gpu/drm/scheduler/sched_entity.c
>>>> index f9b6ce29c58f..a5f729f421f8 100644
>>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>>> @@ -67,17 +67,10 @@ int drm_sched_entity_init(struct
>>>> drm_sched_entity *entity,
>>>> entity->guilty = guilty;
>>>> entity->num_sched_list = num_sched_list;
>>>> entity->priority = priority;
>>>> - entity->sched_list = kcalloc(num_sched_list,
>>>> - sizeof(struct drm_gpu_scheduler *),
>>>> GFP_KERNEL);
>>>> + entity->sched_list = sched_list;
>>>
>>> Maybe make this "num_sched_list > 1 ? sched_list : NULL" to avoid
>>> accidentally dereferencing a stale pointer to the stack.
>> Do you mean "num_sched_list >= 1 ? sched_list : NULL"
No, the entity->sched_list field should be NULL when num_sched_list==1.
When num_sched_list==1 the entity->sched_list shouldn't be used and we
can use a dummy list on the stack as parameter. But we should set the
pointer to NULL in this case just to make sure that nobody is
dereferencing it.
>>>
>>>> - if(!entity->sched_list)
>>>> - return -ENOMEM;
>>>> init_completion(&entity->entity_idle);
>>>> -
>>>> - for (i = 0; i < num_sched_list; i++)
>>>> - entity->sched_list[i] = sched_list[i];
>>>> -
>>>> if (num_sched_list)
>>>
>>> That check can actually be dropped as well. We return -EINVAL when
>>> the num_sched_list is zero.
>>
>> This one took me some time to understand. So we don't really return
>> -EINVAL if num_sched_list == 0
>>
>> if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
>> return -EINVAL;
>>
>> This is coming from below patch. Are we suppose to tolerate IPs with
>> uninitialized sched so that ctx creation dosn't return error ?
Yes, exactly. That's intentionally this way.
GPU reset sometimes resulted in schedulers being disabled because we
couldn't re-init them. In this case we had entities with an empty
scheduler list.
That can't happen any more after you make the scheduler arrays constant,
but I would stick with that behavior.
Regards,
Christian.
>>
>> commit 1decbf6bb0b4dc56c9da6c5e57b994ebfc2be3aa
>> Author: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>> Date: Wed Jan 30 02:53:19 2019 +0100
>>
>> drm/sched: Fix entities with 0 rqs.
>>
>> Some blocks in amdgpu can have 0 rqs.
>>
>> Job creation already fails with -ENOENT when entity->rq is NULL,
>> so jobs cannot be pushed. Without a rq there is no scheduler to
>> pop jobs, and rq selection already does the right thing with a
>> list of length 0.
>>
>> So the operations we need to fix are:
>> - Creation, do not set rq to rq_list[0] if the list can have
>> length 0.
>> - Do not flush any jobs when there is no rq.
>> - On entity destruction handle the rq = NULL case.
>> - on set_priority, do not try to change the rq if it is NULL.
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>> entity->rq =
>>>> &entity->sched_list[0]->sched_rq[entity->priority];
>>>> @@ -312,7 +305,6 @@ void drm_sched_entity_fini(struct
>>>> drm_sched_entity *entity)
>>>> dma_fence_put(entity->last_scheduled);
>>>> entity->last_scheduled = NULL;
>>>> - kfree(entity->sched_list);
>>>> }
>>>> EXPORT_SYMBOL(drm_sched_entity_fini);
>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 14:47 ` Nirmoy
@ 2019-12-10 15:08 ` Nirmoy
2019-12-10 17:32 ` Christian König
0 siblings, 1 reply; 25+ messages in thread
From: Nirmoy @ 2019-12-10 15:08 UTC (permalink / raw)
To: Christian König, alexander.deucher, kenny.ho; +Cc: nirmoy.das, amd-gfx
I think amdgpu_ctx_init() should check for num_scheds and not call
drm_sched_entity_init()
if its zero.
On 12/10/19 3:47 PM, Nirmoy wrote:
>
> On 12/10/19 2:00 PM, Christian König wrote:
>> Am 10.12.19 um 13:53 schrieb Nirmoy Das:
>>> entity should not keep copy and maintain sched list for
>>> itself.
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>> ---
>>> drivers/gpu/drm/scheduler/sched_entity.c | 10 +---------
>>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>>> b/drivers/gpu/drm/scheduler/sched_entity.c
>>> index f9b6ce29c58f..a5f729f421f8 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>> @@ -67,17 +67,10 @@ int drm_sched_entity_init(struct
>>> drm_sched_entity *entity,
>>> entity->guilty = guilty;
>>> entity->num_sched_list = num_sched_list;
>>> entity->priority = priority;
>>> - entity->sched_list = kcalloc(num_sched_list,
>>> - sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
>>> + entity->sched_list = sched_list;
>>
>> Maybe make this "num_sched_list > 1 ? sched_list : NULL" to avoid
>> accidentally dereferencing a stale pointer to the stack.
> Do you mean "num_sched_list >= 1 ? sched_list : NULL"
>>
>>> - if(!entity->sched_list)
>>> - return -ENOMEM;
>>> init_completion(&entity->entity_idle);
>>> -
>>> - for (i = 0; i < num_sched_list; i++)
>>> - entity->sched_list[i] = sched_list[i];
>>> -
>>> if (num_sched_list)
>>
>> That check can actually be dropped as well. We return -EINVAL when
>> the num_sched_list is zero.
>
> This one took me some time to understand. So we don't really return
> -EINVAL if num_sched_list == 0
>
> if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
> return -EINVAL;
>
> This is coming from below patch. Are we suppose to tolerate IPs with
> uninitialized sched so that ctx creation dosn't return error ?
>
> commit 1decbf6bb0b4dc56c9da6c5e57b994ebfc2be3aa
> Author: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> Date: Wed Jan 30 02:53:19 2019 +0100
>
> drm/sched: Fix entities with 0 rqs.
>
> Some blocks in amdgpu can have 0 rqs.
>
> Job creation already fails with -ENOENT when entity->rq is NULL,
> so jobs cannot be pushed. Without a rq there is no scheduler to
> pop jobs, and rq selection already does the right thing with a
> list of length 0.
>
> So the operations we need to fix are:
> - Creation, do not set rq to rq_list[0] if the list can have
> length 0.
> - Do not flush any jobs when there is no rq.
> - On entity destruction handle the rq = NULL case.
> - on set_priority, do not try to change the rq if it is NULL.
>
>>
>> Regards,
>> Christian.
>>
>>> entity->rq =
>>> &entity->sched_list[0]->sched_rq[entity->priority];
>>> @@ -312,7 +305,6 @@ void drm_sched_entity_fini(struct
>>> drm_sched_entity *entity)
>>> dma_fence_put(entity->last_scheduled);
>>> entity->last_scheduled = NULL;
>>> - kfree(entity->sched_list);
>>> }
>>> EXPORT_SYMBOL(drm_sched_entity_fini);
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 13:00 ` Christian König
2019-12-10 13:02 ` Christian König
@ 2019-12-10 14:47 ` Nirmoy
2019-12-10 15:08 ` Nirmoy
1 sibling, 1 reply; 25+ messages in thread
From: Nirmoy @ 2019-12-10 14:47 UTC (permalink / raw)
To: Christian König, Nirmoy Das, alexander.deucher, kenny.ho
Cc: nirmoy.das, amd-gfx
On 12/10/19 2:00 PM, Christian König wrote:
> Am 10.12.19 um 13:53 schrieb Nirmoy Das:
>> entity should not keep copy and maintain sched list for
>> itself.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>> drivers/gpu/drm/scheduler/sched_entity.c | 10 +---------
>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>> b/drivers/gpu/drm/scheduler/sched_entity.c
>> index f9b6ce29c58f..a5f729f421f8 100644
>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>> @@ -67,17 +67,10 @@ int drm_sched_entity_init(struct drm_sched_entity
>> *entity,
>> entity->guilty = guilty;
>> entity->num_sched_list = num_sched_list;
>> entity->priority = priority;
>> - entity->sched_list = kcalloc(num_sched_list,
>> - sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
>> + entity->sched_list = sched_list;
>
> Maybe make this "num_sched_list > 1 ? sched_list : NULL" to avoid
> accidentally dereferencing a stale pointer to the stack.
Do you mean "num_sched_list >= 1 ? sched_list : NULL"
>
>> - if(!entity->sched_list)
>> - return -ENOMEM;
>> init_completion(&entity->entity_idle);
>> -
>> - for (i = 0; i < num_sched_list; i++)
>> - entity->sched_list[i] = sched_list[i];
>> -
>> if (num_sched_list)
>
> That check can actually be dropped as well. We return -EINVAL when the
> num_sched_list is zero.
This one took me some time to understand. So we don't really return
-EINVAL if num_sched_list == 0
if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
return -EINVAL;
This is coming from below patch. Are we suppose to tolerate IPs with
uninitialized sched so that ctx creation dosn't return error ?
commit 1decbf6bb0b4dc56c9da6c5e57b994ebfc2be3aa
Author: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Date: Wed Jan 30 02:53:19 2019 +0100
drm/sched: Fix entities with 0 rqs.
Some blocks in amdgpu can have 0 rqs.
Job creation already fails with -ENOENT when entity->rq is NULL,
so jobs cannot be pushed. Without a rq there is no scheduler to
pop jobs, and rq selection already does the right thing with a
list of length 0.
So the operations we need to fix are:
- Creation, do not set rq to rq_list[0] if the list can have
length 0.
- Do not flush any jobs when there is no rq.
- On entity destruction handle the rq = NULL case.
- on set_priority, do not try to change the rq if it is NULL.
>
> Regards,
> Christian.
>
>> entity->rq =
>> &entity->sched_list[0]->sched_rq[entity->priority];
>> @@ -312,7 +305,6 @@ void drm_sched_entity_fini(struct
>> drm_sched_entity *entity)
>> dma_fence_put(entity->last_scheduled);
>> entity->last_scheduled = NULL;
>> - kfree(entity->sched_list);
>> }
>> EXPORT_SYMBOL(drm_sched_entity_fini);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 13:00 ` Christian König
@ 2019-12-10 13:02 ` Christian König
2019-12-10 14:47 ` Nirmoy
1 sibling, 0 replies; 25+ messages in thread
From: Christian König @ 2019-12-10 13:02 UTC (permalink / raw)
To: Christian König, Nirmoy Das, alexander.deucher, kenny.ho
Cc: nirmoy.das, amd-gfx
Am 10.12.19 um 14:00 schrieb Christian König:
> Am 10.12.19 um 13:53 schrieb Nirmoy Das:
>> entity should not keep copy and maintain sched list for
>> itself.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>> drivers/gpu/drm/scheduler/sched_entity.c | 10 +---------
>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>> b/drivers/gpu/drm/scheduler/sched_entity.c
>> index f9b6ce29c58f..a5f729f421f8 100644
>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>> @@ -67,17 +67,10 @@ int drm_sched_entity_init(struct drm_sched_entity
>> *entity,
>> entity->guilty = guilty;
>> entity->num_sched_list = num_sched_list;
>> entity->priority = priority;
>> - entity->sched_list = kcalloc(num_sched_list,
>> - sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
>> + entity->sched_list = sched_list;
>
> Maybe make this "num_sched_list > 1 ? sched_list : NULL" to avoid
> accidentally dereferencing a stale pointer to the stack.
>
>> - if(!entity->sched_list)
>> - return -ENOMEM;
>> init_completion(&entity->entity_idle);
>> -
>> - for (i = 0; i < num_sched_list; i++)
>> - entity->sched_list[i] = sched_list[i];
>> -
>> if (num_sched_list)
>
> That check can actually be dropped as well. We return -EINVAL when the
> num_sched_list is zero.
>
> Regards,
> Christian.
>
>> entity->rq =
>> &entity->sched_list[0]->sched_rq[entity->priority];
Forgot to note that this should then probably use
"sched_list[0]->sched_rq[entity->priority]" directly when we change the
assignment above.
Christian.
>> @@ -312,7 +305,6 @@ void drm_sched_entity_fini(struct
>> drm_sched_entity *entity)
>> dma_fence_put(entity->last_scheduled);
>> entity->last_scheduled = NULL;
>> - kfree(entity->sched_list);
>> }
>> EXPORT_SYMBOL(drm_sched_entity_fini);
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 12:53 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
@ 2019-12-10 13:00 ` Christian König
2019-12-10 13:02 ` Christian König
2019-12-10 14:47 ` Nirmoy
0 siblings, 2 replies; 25+ messages in thread
From: Christian König @ 2019-12-10 13:00 UTC (permalink / raw)
To: Nirmoy Das, alexander.deucher, kenny.ho; +Cc: nirmoy.das, amd-gfx
Am 10.12.19 um 13:53 schrieb Nirmoy Das:
> entity should not keep copy and maintain sched list for
> itself.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
> drivers/gpu/drm/scheduler/sched_entity.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index f9b6ce29c58f..a5f729f421f8 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -67,17 +67,10 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
> entity->guilty = guilty;
> entity->num_sched_list = num_sched_list;
> entity->priority = priority;
> - entity->sched_list = kcalloc(num_sched_list,
> - sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
> + entity->sched_list = sched_list;
Maybe make this "num_sched_list > 1 ? sched_list : NULL" to avoid
accidentally dereferencing a stale pointer to the stack.
>
> - if(!entity->sched_list)
> - return -ENOMEM;
>
> init_completion(&entity->entity_idle);
> -
> - for (i = 0; i < num_sched_list; i++)
> - entity->sched_list[i] = sched_list[i];
> -
> if (num_sched_list)
That check can actually be dropped as well. We return -EINVAL when the
num_sched_list is zero.
Regards,
Christian.
> entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
>
> @@ -312,7 +305,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
>
> dma_fence_put(entity->last_scheduled);
> entity->last_scheduled = NULL;
> - kfree(entity->sched_list);
> }
> EXPORT_SYMBOL(drm_sched_entity_fini);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-10 12:52 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
@ 2019-12-10 12:53 ` Nirmoy Das
2019-12-10 13:00 ` Christian König
0 siblings, 1 reply; 25+ messages in thread
From: Nirmoy Das @ 2019-12-10 12:53 UTC (permalink / raw)
To: alexander.deucher, kenny.ho, christian.koenig; +Cc: nirmoy.das, amd-gfx
entity should not keep copy and maintain sched list for
itself.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/scheduler/sched_entity.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index f9b6ce29c58f..a5f729f421f8 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -67,17 +67,10 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
entity->guilty = guilty;
entity->num_sched_list = num_sched_list;
entity->priority = priority;
- entity->sched_list = kcalloc(num_sched_list,
- sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
+ entity->sched_list = sched_list;
- if(!entity->sched_list)
- return -ENOMEM;
init_completion(&entity->entity_idle);
-
- for (i = 0; i < num_sched_list; i++)
- entity->sched_list[i] = sched_list[i];
-
if (num_sched_list)
entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
@@ -312,7 +305,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
- kfree(entity->sched_list);
}
EXPORT_SYMBOL(drm_sched_entity_fini);
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/4] drm/scheduler: do not keep a copy of sched list
2019-12-09 21:53 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
@ 2019-12-09 21:53 ` Nirmoy Das
0 siblings, 0 replies; 25+ messages in thread
From: Nirmoy Das @ 2019-12-09 21:53 UTC (permalink / raw)
To: alexander.deucher, kenny.ho, christian.koenig; +Cc: nirmoy.das, amd-gfx
entity should not keep copy and maintain sched list for
itself.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/scheduler/sched_entity.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index f9b6ce29c58f..a5f729f421f8 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -67,17 +67,10 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
entity->guilty = guilty;
entity->num_sched_list = num_sched_list;
entity->priority = priority;
- entity->sched_list = kcalloc(num_sched_list,
- sizeof(struct drm_gpu_scheduler *), GFP_KERNEL);
+ entity->sched_list = sched_list;
- if(!entity->sched_list)
- return -ENOMEM;
init_completion(&entity->entity_idle);
-
- for (i = 0; i < num_sched_list; i++)
- entity->sched_list[i] = sched_list[i];
-
if (num_sched_list)
entity->rq = &entity->sched_list[0]->sched_rq[entity->priority];
@@ -312,7 +305,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
- kfree(entity->sched_list);
}
EXPORT_SYMBOL(drm_sched_entity_fini);
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
end of thread, other threads:[~2019-12-12 8:55 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-06 17:33 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
2019-12-06 17:33 ` [PATCH 2/4] drm/amdgpu: replace vm_pte's run-queue list with drm gpu scheds list Nirmoy Das
2019-12-06 19:37 ` Christian König
2019-12-06 17:33 ` [PATCH 3/4] drm/amdgpu: allocate entities on demand Nirmoy Das
2019-12-06 19:40 ` Christian König
2019-12-06 17:33 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
2019-12-06 19:41 ` Christian König
2019-12-08 19:57 ` Nirmoy
2019-12-09 12:20 ` Christian König
2019-12-09 13:56 ` Nirmoy
2019-12-09 14:09 ` Nirmoy
2019-12-09 15:37 ` Christian König
2019-12-09 21:53 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
2019-12-09 21:53 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
2019-12-10 12:52 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
2019-12-10 12:53 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
2019-12-10 13:00 ` Christian König
2019-12-10 13:02 ` Christian König
2019-12-10 14:47 ` Nirmoy
2019-12-10 15:08 ` Nirmoy
2019-12-10 17:32 ` Christian König
2019-12-10 18:38 ` Nirmoy
2019-12-10 18:17 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
2019-12-10 18:17 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
2019-12-11 12:25 ` Christian König
2019-12-11 14:24 [PATCH 1/4 v2] drm/scheduler: rework entity creation Nirmoy Das
2019-12-11 14:24 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
2019-12-11 15:42 [PATCH 1/4] drm/scheduler: rework entity creation Nirmoy Das
2019-12-11 15:42 ` [PATCH 4/4] drm/scheduler: do not keep a copy of sched list Nirmoy Das
2019-12-11 15:42 ` Nirmoy Das
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.