The documentation of drm_sched_job_init and drm_sched_entity_push_job has been clarified. Both functions should be called under a shared lock, to avoid jobs getting pushed into the scheduler queue in a different order than their sched_fence seqnos, which will confuse checks that are looking at the seqnos to infer information about completion order. Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx> --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 4 ++-- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 2 +- drivers/gpu/drm/etnaviv/etnaviv_sched.c | 24 ++++++++++++++------ 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 46ecd3e66ac9..983e67f19e45 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -388,9 +388,9 @@ static void submit_cleanup(struct kref *kref) dma_fence_put(submit->in_fence); if (submit->out_fence) { /* first remove from IDR, so fence can not be found anymore */ - mutex_lock(&submit->gpu->fence_idr_lock); + mutex_lock(&submit->gpu->fence_lock); idr_remove(&submit->gpu->fence_idr, submit->out_fence_id); - mutex_unlock(&submit->gpu->fence_idr_lock); + mutex_unlock(&submit->gpu->fence_lock); dma_fence_put(submit->out_fence); } kfree(submit->pmrs); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 8a88799bf79b..94b8315dc9bc 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1743,7 +1743,7 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev) gpu->dev = &pdev->dev; mutex_init(&gpu->lock); - mutex_init(&gpu->fence_idr_lock); + mutex_init(&gpu->fence_lock); /* Map registers: */ gpu->mmio = etnaviv_ioremap(pdev, NULL, dev_name(gpu->dev)); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h index 3c3005501846..6f8dd8faea81 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h @@ -129,7 +129,7 @@ struct etnaviv_gpu { u32 idle_mask; /* Fencing support */ - struct mutex fence_idr_lock; + struct mutex fence_lock; struct idr fence_idr; u32 next_fence; u32 active_fence; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c index 6cf0775dbcd7..f13283abe369 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c @@ -127,28 +127,38 @@ static const struct drm_sched_backend_ops etnaviv_sched_ops = { int etnaviv_sched_push_job(struct drm_sched_entity *sched_entity, struct etnaviv_gem_submit *submit) { - int ret; + int ret = 0; + + /* + * Hold the fence lock across the whole operation to avoid jobs being + * pushed out of order with regard to their sched fence seqnos as + * allocated in drm_sched_job_init. + */ + mutex_lock(&submit->gpu->fence_lock); ret = drm_sched_job_init(&submit->sched_job, &submit->gpu->sched, sched_entity, submit->cmdbuf.ctx); if (ret) - return ret; + goto out_unlock; submit->out_fence = dma_fence_get(&submit->sched_job.s_fence->finished); - mutex_lock(&submit->gpu->fence_idr_lock); submit->out_fence_id = idr_alloc_cyclic(&submit->gpu->fence_idr, submit->out_fence, 0, INT_MAX, GFP_KERNEL); - mutex_unlock(&submit->gpu->fence_idr_lock); - if (submit->out_fence_id < 0) - return -ENOMEM; + if (submit->out_fence_id < 0) { + ret = -ENOMEM; + goto out_unlock; + } /* the scheduler holds on to the job now */ kref_get(&submit->refcount); drm_sched_entity_push_job(&submit->sched_job, sched_entity); - return 0; +out_unlock: + mutex_unlock(&submit->gpu->fence_lock); + + return ret; } int etnaviv_sched_init(struct etnaviv_gpu *gpu) -- 2.17.0 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel