diff mbox series

[v5,08/20] drm/lima: use scheduler dependency tracking

Message ID 20210805104705.862416-9-daniel.vetter@ffwll.ch
State Accepted
Commit c79a4487f33bdac125dc682c5584f3e13f5461f5
Headers show
Series None | expand

Commit Message

Daniel Vetter Aug. 5, 2021, 10:46 a.m. UTC 
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

Daniel Vetter Aug. 12, 2021, 7:28 p.m. UTC | #1 
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
Qiang Yu Aug. 14, 2021, 2:45 a.m. UTC | #2 
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
diff mbox series

Patch

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;