| Message ID | 20230208194817.199932-2-mcanal@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/sched: Create wrapper to add a syncobj dependency to job | expand |
Am 08.02.23 um 20:48 schrieb Maíra Canal: > In order to add a syncobj's fence as a dependency to a job, it is > necessary to call drm_syncobj_find_fence() to find the fence and then > add the dependency with drm_sched_job_add_dependency(). So, wrap these > steps in one single function, drm_sched_job_add_syncobj_dependency(). > > Signed-off-by: Maíra Canal <mcanal@igalia.com> Just one nit pick below, with that fixed Reviewed-by: Christian König <christian.koenig@amd.com> I'm pretty sure we have the exact same function now in amdgpu cause I cleaned that up just a few weeks ago to look the same as the other drivers. Would be nice to have that new function applied there as well. Thanks, Christian. > --- > drivers/gpu/drm/scheduler/sched_main.c | 29 ++++++++++++++++++++++++++ > include/drm/gpu_scheduler.h | 6 ++++++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c > index 0e4378420271..d5331b1877a3 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -53,6 +53,7 @@ > > #include <drm/drm_print.h> > #include <drm/drm_gem.h> > +#include <drm/drm_syncobj.h> > #include <drm/gpu_scheduler.h> > #include <drm/spsc_queue.h> > > @@ -718,6 +719,34 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job, > } > EXPORT_SYMBOL(drm_sched_job_add_dependency); > > +/** > + * drm_sched_job_add_syncobj_dependency - adds a syncobj's fence as a job dependency > + * @job: scheduler job to add the dependencies to > + * @file_private: drm file private pointer > + * @handle: syncobj handle to lookup > + * @point: timeline point > + * > + * This adds the fence matching the given syncobj to @job. > + * > + * Returns: > + * 0 on success, or an error on failing to expand the array. > + */ > +int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job, > + struct drm_file *file, > + u32 handle, > + u32 point) > +{ > + struct dma_fence *fence; > + int ret = 0; Please don't initialize any local return variables if it isn't necessary. This just suppresses uninitialized variables from the compiler which quite often have helped finding more wider bugs. Regards, Christian. > + > + ret = drm_syncobj_find_fence(file, handle, point, 0, &fence); > + if (ret) > + return ret; > + > + return drm_sched_job_add_dependency(job, fence); > +} > +EXPORT_SYMBOL(drm_sched_job_add_syncobj_dependency); > + > /** > * drm_sched_job_add_resv_dependencies - add all fences from the resv to the job > * @job: scheduler job to add the dependencies to > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h > index 9935d1e2ff69..4cc54f8b57b4 100644 > --- a/include/drm/gpu_scheduler.h > +++ b/include/drm/gpu_scheduler.h > @@ -48,6 +48,8 @@ struct drm_gem_object; > struct drm_gpu_scheduler; > struct drm_sched_rq; > > +struct drm_file; > + > /* These are often used as an (initial) index > * to an array, and as such should start at 0. > */ > @@ -515,6 +517,10 @@ int drm_sched_job_init(struct drm_sched_job *job, > void drm_sched_job_arm(struct drm_sched_job *job); > int drm_sched_job_add_dependency(struct drm_sched_job *job, > struct dma_fence *fence); > +int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job, > + struct drm_file *file, > + u32 handle, > + u32 point); > int drm_sched_job_add_resv_dependencies(struct drm_sched_job *job, > struct dma_resv *resv, > enum dma_resv_usage usage);
On 2023-02-08 14:54, Christian König wrote: > Am 08.02.23 um 20:48 schrieb Maíra Canal: >> In order to add a syncobj's fence as a dependency to a job, it is >> necessary to call drm_syncobj_find_fence() to find the fence and then >> add the dependency with drm_sched_job_add_dependency(). So, wrap these >> steps in one single function, drm_sched_job_add_syncobj_dependency(). >> >> Signed-off-by: Maíra Canal <mcanal@igalia.com> > Just one nit pick below, with that fixed Reviewed-by: Christian König > <christian.koenig@amd.com> > > I'm pretty sure we have the exact same function now in amdgpu cause I > cleaned that up just a few weeks ago to look the same as the other drivers. > > Would be nice to have that new function applied there as well. Hi Christian, Is that R-B for the series or just this patch?
On 2023-02-08 14:48, Maíra Canal wrote: > In order to add a syncobj's fence as a dependency to a job, it is > necessary to call drm_syncobj_find_fence() to find the fence and then > add the dependency with drm_sched_job_add_dependency(). So, wrap these > steps in one single function, drm_sched_job_add_syncobj_dependency(). Yes, that's a good change--thanks! Patch is: Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
Am 08.02.23 um 23:20 schrieb Luben Tuikov: > On 2023-02-08 14:54, Christian König wrote: >> Am 08.02.23 um 20:48 schrieb Maíra Canal: >>> In order to add a syncobj's fence as a dependency to a job, it is >>> necessary to call drm_syncobj_find_fence() to find the fence and then >>> add the dependency with drm_sched_job_add_dependency(). So, wrap these >>> steps in one single function, drm_sched_job_add_syncobj_dependency(). >>> >>> Signed-off-by: Maíra Canal <mcanal@igalia.com> >> Just one nit pick below, with that fixed Reviewed-by: Christian König >> <christian.koenig@amd.com> >> >> I'm pretty sure we have the exact same function now in amdgpu cause I >> cleaned that up just a few weeks ago to look the same as the other drivers. >> >> Would be nice to have that new function applied there as well. > Hi Christian, > > Is that R-B for the series or just this patch? Just this patch, haven't looked at the driver implementations in detail. Regards, Christian.
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 0e4378420271..d5331b1877a3 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -53,6 +53,7 @@ #include <drm/drm_print.h> #include <drm/drm_gem.h> +#include <drm/drm_syncobj.h> #include <drm/gpu_scheduler.h> #include <drm/spsc_queue.h> @@ -718,6 +719,34 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job, } EXPORT_SYMBOL(drm_sched_job_add_dependency); +/** + * drm_sched_job_add_syncobj_dependency - adds a syncobj's fence as a job dependency + * @job: scheduler job to add the dependencies to + * @file_private: drm file private pointer + * @handle: syncobj handle to lookup + * @point: timeline point + * + * This adds the fence matching the given syncobj to @job. + * + * Returns: + * 0 on success, or an error on failing to expand the array. + */ +int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job, + struct drm_file *file, + u32 handle, + u32 point) +{ + struct dma_fence *fence; + int ret = 0; + + ret = drm_syncobj_find_fence(file, handle, point, 0, &fence); + if (ret) + return ret; + + return drm_sched_job_add_dependency(job, fence); +} +EXPORT_SYMBOL(drm_sched_job_add_syncobj_dependency); + /** * drm_sched_job_add_resv_dependencies - add all fences from the resv to the job * @job: scheduler job to add the dependencies to diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 9935d1e2ff69..4cc54f8b57b4 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -48,6 +48,8 @@ struct drm_gem_object; struct drm_gpu_scheduler; struct drm_sched_rq; +struct drm_file; + /* These are often used as an (initial) index * to an array, and as such should start at 0. */ @@ -515,6 +517,10 @@ int drm_sched_job_init(struct drm_sched_job *job, void drm_sched_job_arm(struct drm_sched_job *job); int drm_sched_job_add_dependency(struct drm_sched_job *job, struct dma_fence *fence); +int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job, + struct drm_file *file, + u32 handle, + u32 point); int drm_sched_job_add_resv_dependencies(struct drm_sched_job *job, struct dma_resv *resv, enum dma_resv_usage usage);
In order to add a syncobj's fence as a dependency to a job, it is necessary to call drm_syncobj_find_fence() to find the fence and then add the dependency with drm_sched_job_add_dependency(). So, wrap these steps in one single function, drm_sched_job_add_syncobj_dependency(). Signed-off-by: Maíra Canal <mcanal@igalia.com> --- drivers/gpu/drm/scheduler/sched_main.c | 29 ++++++++++++++++++++++++++ include/drm/gpu_scheduler.h | 6 ++++++ 2 files changed, 35 insertions(+)