| Message ID | 20230712124704.333004-6-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] drm/radeon: switch over to drm_exec | expand |
On 12/07/2023 13:47, Christian König wrote: > Just a straightforward conversion without any optimization. > > Only compile tested for now. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Cc: Rob Herring <robh@kernel.org> > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Steven Price <steven.price@arm.com> > Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> > --- > drivers/gpu/drm/panfrost/Kconfig | 1 + > drivers/gpu/drm/panfrost/panfrost_job.c | 12 +++++++----- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/Kconfig b/drivers/gpu/drm/panfrost/Kconfig > index e6403a9d66ad..e86a1a2fd8e1 100644 > --- a/drivers/gpu/drm/panfrost/Kconfig > +++ b/drivers/gpu/drm/panfrost/Kconfig > @@ -7,6 +7,7 @@ config DRM_PANFROST > depends on !GENERIC_ATOMIC64 # for IOMMU_IO_PGTABLE_LPAE > depends on MMU > select DRM_SCHED > + select DRM_EXEC > select IOMMU_SUPPORT > select IOMMU_IO_PGTABLE_LPAE > select DRM_GEM_SHMEM_HELPER > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index dbc597ab46fb..8b9206e910b5 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -8,6 +8,7 @@ > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/dma-resv.h> > +#include <drm/drm_exec.h> > #include <drm/gpu_scheduler.h> > #include <drm/panfrost_drm.h> > > @@ -275,13 +276,14 @@ static void panfrost_attach_object_fences(struct drm_gem_object **bos, > int panfrost_job_push(struct panfrost_job *job) > { > struct panfrost_device *pfdev = job->pfdev; > - struct ww_acquire_ctx acquire_ctx; > + struct drm_exec exec; > int ret = 0; > > - ret = drm_gem_lock_reservations(job->bos, job->bo_count, > - &acquire_ctx); > + drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT); > + drm_exec_until_all_locked(&exec) > + ret = drm_exec_prepare_array(&exec, job->bos, job->bo_count, 1); This loop bothers me - the return value is ignored until drm_exec_until_all_locked() decides we can exit. It also reads badly without the braces and with the "if" unindented below. The documentation ("a typical usage pattern") suggests this should be something like: drm_exec_until_all_locked(&exec) { ret = drm_exec_prepare_array(...); drm_exec_retry_on_contention(&exec); if (ret) goto unlock; } Although I'm not sure if that's actually different because if there's no contention that drm_exec_until_all_locked() looks like it should exit. Perhaps it's just the name drm_exec_until_all_locked() which perhaps should be drm_exec_until_not_contended()...? Anyway I gave it a quick spin on a Firefly-RK3288 and didn't see any problems, but I don't think I've got any tests which would create contention on the BOs. Steve > if (ret) > - return ret; > + goto unlock; > > mutex_lock(&pfdev->sched_lock); > drm_sched_job_arm(&job->base); > @@ -305,7 +307,7 @@ int panfrost_job_push(struct panfrost_job *job) > job->render_done_fence); > > unlock: > - drm_gem_unlock_reservations(job->bos, job->bo_count, &acquire_ctx); > + drm_exec_fini(&exec); > > return ret; > }
diff --git a/drivers/gpu/drm/panfrost/Kconfig b/drivers/gpu/drm/panfrost/Kconfig index e6403a9d66ad..e86a1a2fd8e1 100644 --- a/drivers/gpu/drm/panfrost/Kconfig +++ b/drivers/gpu/drm/panfrost/Kconfig @@ -7,6 +7,7 @@ config DRM_PANFROST depends on !GENERIC_ATOMIC64 # for IOMMU_IO_PGTABLE_LPAE depends on MMU select DRM_SCHED + select DRM_EXEC select IOMMU_SUPPORT select IOMMU_IO_PGTABLE_LPAE select DRM_GEM_SHMEM_HELPER diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index dbc597ab46fb..8b9206e910b5 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -8,6 +8,7 @@ #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/dma-resv.h> +#include <drm/drm_exec.h> #include <drm/gpu_scheduler.h> #include <drm/panfrost_drm.h> @@ -275,13 +276,14 @@ static void panfrost_attach_object_fences(struct drm_gem_object **bos, int panfrost_job_push(struct panfrost_job *job) { struct panfrost_device *pfdev = job->pfdev; - struct ww_acquire_ctx acquire_ctx; + struct drm_exec exec; int ret = 0; - ret = drm_gem_lock_reservations(job->bos, job->bo_count, - &acquire_ctx); + drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT); + drm_exec_until_all_locked(&exec) + ret = drm_exec_prepare_array(&exec, job->bos, job->bo_count, 1); if (ret) - return ret; + goto unlock; mutex_lock(&pfdev->sched_lock); drm_sched_job_arm(&job->base); @@ -305,7 +307,7 @@ int panfrost_job_push(struct panfrost_job *job) job->render_done_fence); unlock: - drm_gem_unlock_reservations(job->bos, job->bo_count, &acquire_ctx); + drm_exec_fini(&exec); return ret; }
Just a straightforward conversion without any optimization. Only compile tested for now. Signed-off-by: Christian König <christian.koenig@amd.com> Cc: Rob Herring <robh@kernel.org> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> Cc: Steven Price <steven.price@arm.com> Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> --- drivers/gpu/drm/panfrost/Kconfig | 1 + drivers/gpu/drm/panfrost/panfrost_job.c | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-)