[v5,08/20] drm/lima: use scheduler dependency tracking
Commit Message
Nothing special going on here.
Aside reviewing the code, it seems like drm_sched_job_arm() should be
moved into lima_sched_context_queue_task and put under some mutex
together with drm_sched_push_job(). See the kerneldoc for
drm_sched_push_job().
v2: Rebase over renamed functions to add dependencies.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Qiang Yu <yuq825@gmail.com>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: lima@lists.freedesktop.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
drivers/gpu/drm/lima/lima_gem.c | 6 ++++--
drivers/gpu/drm/lima/lima_sched.c | 21 ---------------------
drivers/gpu/drm/lima/lima_sched.h | 3 ---
3 files changed, 4 insertions(+), 26 deletions(-)
Comments
On Thu, Aug 05, 2021 at 12:46:53PM +0200, Daniel Vetter wrote:
> Nothing special going on here.
>
> Aside reviewing the code, it seems like drm_sched_job_arm() should be
> moved into lima_sched_context_queue_task and put under some mutex
> together with drm_sched_push_job(). See the kerneldoc for
> drm_sched_push_job().
>
> v2: Rebase over renamed functions to add dependencies.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Qiang Yu <yuq825@gmail.com>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: lima@lists.freedesktop.org
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
Ping for an ack here please. Testing would be even better ofc.
-Daniel
> ---
> drivers/gpu/drm/lima/lima_gem.c | 6 ++++--
> drivers/gpu/drm/lima/lima_sched.c | 21 ---------------------
> drivers/gpu/drm/lima/lima_sched.h | 3 ---
> 3 files changed, 4 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
> index c528f40981bb..640acc060467 100644
> --- a/drivers/gpu/drm/lima/lima_gem.c
> +++ b/drivers/gpu/drm/lima/lima_gem.c
> @@ -267,7 +267,9 @@ static int lima_gem_sync_bo(struct lima_sched_task *task, struct lima_bo *bo,
> if (explicit)
> return 0;
>
> - return drm_gem_fence_array_add_implicit(&task->deps, &bo->base.base, write);
> + return drm_sched_job_add_implicit_dependencies(&task->base,
> + &bo->base.base,
> + write);
> }
>
> static int lima_gem_add_deps(struct drm_file *file, struct lima_submit *submit)
> @@ -285,7 +287,7 @@ static int lima_gem_add_deps(struct drm_file *file, struct lima_submit *submit)
> if (err)
> return err;
>
> - err = drm_gem_fence_array_add(&submit->task->deps, fence);
> + err = drm_sched_job_add_dependency(&submit->task->base, fence);
> if (err) {
> dma_fence_put(fence);
> return err;
> diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c
> index e968b5a8f0b0..99d5f6f1a882 100644
> --- a/drivers/gpu/drm/lima/lima_sched.c
> +++ b/drivers/gpu/drm/lima/lima_sched.c
> @@ -134,24 +134,15 @@ int lima_sched_task_init(struct lima_sched_task *task,
> task->num_bos = num_bos;
> task->vm = lima_vm_get(vm);
>
> - xa_init_flags(&task->deps, XA_FLAGS_ALLOC);
> -
> return 0;
> }
>
> void lima_sched_task_fini(struct lima_sched_task *task)
> {
> - struct dma_fence *fence;
> - unsigned long index;
> int i;
>
> drm_sched_job_cleanup(&task->base);
>
> - xa_for_each(&task->deps, index, fence) {
> - dma_fence_put(fence);
> - }
> - xa_destroy(&task->deps);
> -
> if (task->bos) {
> for (i = 0; i < task->num_bos; i++)
> drm_gem_object_put(&task->bos[i]->base.base);
> @@ -186,17 +177,6 @@ struct dma_fence *lima_sched_context_queue_task(struct lima_sched_task *task)
> return fence;
> }
>
> -static struct dma_fence *lima_sched_dependency(struct drm_sched_job *job,
> - struct drm_sched_entity *entity)
> -{
> - struct lima_sched_task *task = to_lima_task(job);
> -
> - if (!xa_empty(&task->deps))
> - return xa_erase(&task->deps, task->last_dep++);
> -
> - return NULL;
> -}
> -
> static int lima_pm_busy(struct lima_device *ldev)
> {
> int ret;
> @@ -472,7 +452,6 @@ static void lima_sched_free_job(struct drm_sched_job *job)
> }
>
> static const struct drm_sched_backend_ops lima_sched_ops = {
> - .dependency = lima_sched_dependency,
> .run_job = lima_sched_run_job,
> .timedout_job = lima_sched_timedout_job,
> .free_job = lima_sched_free_job,
> diff --git a/drivers/gpu/drm/lima/lima_sched.h b/drivers/gpu/drm/lima/lima_sched.h
> index ac70006b0e26..6a11764d87b3 100644
> --- a/drivers/gpu/drm/lima/lima_sched.h
> +++ b/drivers/gpu/drm/lima/lima_sched.h
> @@ -23,9 +23,6 @@ struct lima_sched_task {
> struct lima_vm *vm;
> void *frame;
>
> - struct xarray deps;
> - unsigned long last_dep;
> -
> struct lima_bo **bos;
> int num_bos;
>
> --
> 2.32.0
>
Thanks for the remind, indeed miss a lock. Patch is:
Reviewed-by: Qiang Yu <yuq825@gmail.com>
Regards,
Qiang
On Fri, Aug 13, 2021 at 3:28 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Thu, Aug 05, 2021 at 12:46:53PM +0200, Daniel Vetter wrote:
> > Nothing special going on here.
> >
> > Aside reviewing the code, it seems like drm_sched_job_arm() should be
> > moved into lima_sched_context_queue_task and put under some mutex
> > together with drm_sched_push_job(). See the kerneldoc for
> > drm_sched_push_job().
> >
> > v2: Rebase over renamed functions to add dependencies.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Qiang Yu <yuq825@gmail.com>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: lima@lists.freedesktop.org
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
>
> Ping for an ack here please. Testing would be even better ofc.
> -Daniel
>
> > ---
> > drivers/gpu/drm/lima/lima_gem.c | 6 ++++--
> > drivers/gpu/drm/lima/lima_sched.c | 21 ---------------------
> > drivers/gpu/drm/lima/lima_sched.h | 3 ---
> > 3 files changed, 4 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
> > index c528f40981bb..640acc060467 100644
> > --- a/drivers/gpu/drm/lima/lima_gem.c
> > +++ b/drivers/gpu/drm/lima/lima_gem.c
> > @@ -267,7 +267,9 @@ static int lima_gem_sync_bo(struct lima_sched_task *task, struct lima_bo *bo,
> > if (explicit)
> > return 0;
> >
> > - return drm_gem_fence_array_add_implicit(&task->deps, &bo->base.base, write);
> > + return drm_sched_job_add_implicit_dependencies(&task->base,
> > + &bo->base.base,
> > + write);
> > }
> >
> > static int lima_gem_add_deps(struct drm_file *file, struct lima_submit *submit)
> > @@ -285,7 +287,7 @@ static int lima_gem_add_deps(struct drm_file *file, struct lima_submit *submit)
> > if (err)
> > return err;
> >
> > - err = drm_gem_fence_array_add(&submit->task->deps, fence);
> > + err = drm_sched_job_add_dependency(&submit->task->base, fence);
> > if (err) {
> > dma_fence_put(fence);
> > return err;
> > diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c
> > index e968b5a8f0b0..99d5f6f1a882 100644
> > --- a/drivers/gpu/drm/lima/lima_sched.c
> > +++ b/drivers/gpu/drm/lima/lima_sched.c
> > @@ -134,24 +134,15 @@ int lima_sched_task_init(struct lima_sched_task *task,
> > task->num_bos = num_bos;
> > task->vm = lima_vm_get(vm);
> >
> > - xa_init_flags(&task->deps, XA_FLAGS_ALLOC);
> > -
> > return 0;
> > }
> >
> > void lima_sched_task_fini(struct lima_sched_task *task)
> > {
> > - struct dma_fence *fence;
> > - unsigned long index;
> > int i;
> >
> > drm_sched_job_cleanup(&task->base);
> >
> > - xa_for_each(&task->deps, index, fence) {
> > - dma_fence_put(fence);
> > - }
> > - xa_destroy(&task->deps);
> > -
> > if (task->bos) {
> > for (i = 0; i < task->num_bos; i++)
> > drm_gem_object_put(&task->bos[i]->base.base);
> > @@ -186,17 +177,6 @@ struct dma_fence *lima_sched_context_queue_task(struct lima_sched_task *task)
> > return fence;
> > }
> >
> > -static struct dma_fence *lima_sched_dependency(struct drm_sched_job *job,
> > - struct drm_sched_entity *entity)
> > -{
> > - struct lima_sched_task *task = to_lima_task(job);
> > -
> > - if (!xa_empty(&task->deps))
> > - return xa_erase(&task->deps, task->last_dep++);
> > -
> > - return NULL;
> > -}
> > -
> > static int lima_pm_busy(struct lima_device *ldev)
> > {
> > int ret;
> > @@ -472,7 +452,6 @@ static void lima_sched_free_job(struct drm_sched_job *job)
> > }
> >
> > static const struct drm_sched_backend_ops lima_sched_ops = {
> > - .dependency = lima_sched_dependency,
> > .run_job = lima_sched_run_job,
> > .timedout_job = lima_sched_timedout_job,
> > .free_job = lima_sched_free_job,
> > diff --git a/drivers/gpu/drm/lima/lima_sched.h b/drivers/gpu/drm/lima/lima_sched.h
> > index ac70006b0e26..6a11764d87b3 100644
> > --- a/drivers/gpu/drm/lima/lima_sched.h
> > +++ b/drivers/gpu/drm/lima/lima_sched.h
> > @@ -23,9 +23,6 @@ struct lima_sched_task {
> > struct lima_vm *vm;
> > void *frame;
> >
> > - struct xarray deps;
> > - unsigned long last_dep;
> > -
> > struct lima_bo **bos;
> > int num_bos;
> >
> > --
> > 2.32.0
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
@@ -267,7 +267,9 @@ static int lima_gem_sync_bo(struct lima_sched_task *task, struct lima_bo *bo,
if (explicit)
return 0;
- return drm_gem_fence_array_add_implicit(&task->deps, &bo->base.base, write);
+ return drm_sched_job_add_implicit_dependencies(&task->base,
+ &bo->base.base,
+ write);
}
static int lima_gem_add_deps(struct drm_file *file, struct lima_submit *submit)
@@ -285,7 +287,7 @@ static int lima_gem_add_deps(struct drm_file *file, struct lima_submit *submit)
if (err)
return err;
- err = drm_gem_fence_array_add(&submit->task->deps, fence);
+ err = drm_sched_job_add_dependency(&submit->task->base, fence);
if (err) {
dma_fence_put(fence);
return err;
@@ -134,24 +134,15 @@ int lima_sched_task_init(struct lima_sched_task *task,
task->num_bos = num_bos;
task->vm = lima_vm_get(vm);
- xa_init_flags(&task->deps, XA_FLAGS_ALLOC);
-
return 0;
}
void lima_sched_task_fini(struct lima_sched_task *task)
{
- struct dma_fence *fence;
- unsigned long index;
int i;
drm_sched_job_cleanup(&task->base);
- xa_for_each(&task->deps, index, fence) {
- dma_fence_put(fence);
- }
- xa_destroy(&task->deps);
-
if (task->bos) {
for (i = 0; i < task->num_bos; i++)
drm_gem_object_put(&task->bos[i]->base.base);
@@ -186,17 +177,6 @@ struct dma_fence *lima_sched_context_queue_task(struct lima_sched_task *task)
return fence;
}
-static struct dma_fence *lima_sched_dependency(struct drm_sched_job *job,
- struct drm_sched_entity *entity)
-{
- struct lima_sched_task *task = to_lima_task(job);
-
- if (!xa_empty(&task->deps))
- return xa_erase(&task->deps, task->last_dep++);
-
- return NULL;
-}
-
static int lima_pm_busy(struct lima_device *ldev)
{
int ret;
@@ -472,7 +452,6 @@ static void lima_sched_free_job(struct drm_sched_job *job)
}
static const struct drm_sched_backend_ops lima_sched_ops = {
- .dependency = lima_sched_dependency,
.run_job = lima_sched_run_job,
.timedout_job = lima_sched_timedout_job,
.free_job = lima_sched_free_job,
@@ -23,9 +23,6 @@ struct lima_sched_task {
struct lima_vm *vm;
void *frame;
- struct xarray deps;
- unsigned long last_dep;
-
struct lima_bo **bos;
int num_bos;