2023-11-21 00:40:31

by Rob Clark

[permalink] [raw]
Subject: [PATCH v2 4/7] drm/msm/gem: Split out submit_unpin_objects() helper

From: Rob Clark <[email protected]>

Untangle unpinning from unlock/unref loop. The unpin only happens in
error paths so it is easier to decouple from the normal unlock path.

Since we never have an intermediate state where a subset of buffers
are pinned (ie. we never bail out of the pin or unpin loops) we can
replace the bo state flag bit with a global flag in the submit.

Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/msm/msm_gem.h | 6 +++---
drivers/gpu/drm/msm/msm_gem_submit.c | 22 +++++++++++++++++-----
drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index c36c1c1fa222..af884ced7a0d 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -270,8 +270,9 @@ struct msm_gem_submit {
int fence_id; /* key into queue->fence_idr */
struct msm_gpu_submitqueue *queue;
struct pid *pid; /* submitting process */
- bool fault_dumped; /* Limit devcoredump dumping to one per submit */
- bool in_rb; /* "sudo" mode, copy cmds into RB */
+ bool bos_pinned : 1;
+ bool fault_dumped:1;/* Limit devcoredump dumping to one per submit */
+ bool in_rb : 1; /* "sudo" mode, copy cmds into RB */
struct msm_ringbuffer *ring;
unsigned int nr_cmds;
unsigned int nr_bos;
@@ -288,7 +289,6 @@ struct msm_gem_submit {
struct {
/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */
#define BO_LOCKED 0x4000 /* obj lock is held */
-#define BO_PINNED 0x2000 /* obj (pages) is pinned and on active list */
uint32_t flags;
union {
struct drm_gem_object *obj;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 786b48a55309..d001bf286606 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -265,9 +265,6 @@ static void submit_cleanup_bo(struct msm_gem_submit *submit, int i,
*/
submit->bos[i].flags &= ~cleanup_flags;

- if (flags & BO_PINNED)
- msm_gem_unpin_locked(obj);
-
if (flags & BO_LOCKED)
dma_resv_unlock(obj->resv);
}
@@ -407,13 +404,28 @@ static int submit_pin_objects(struct msm_gem_submit *submit)
mutex_lock(&priv->lru.lock);
for (i = 0; i < submit->nr_bos; i++) {
msm_gem_pin_obj_locked(submit->bos[i].obj);
- submit->bos[i].flags |= BO_PINNED;
}
mutex_unlock(&priv->lru.lock);

+ submit->bos_pinned = true;
+
return ret;
}

+static void submit_unpin_objects(struct msm_gem_submit *submit)
+{
+ if (!submit->bos_pinned)
+ return;
+
+ for (int i = 0; i < submit->nr_bos; i++) {
+ struct drm_gem_object *obj = submit->bos[i].obj;
+
+ msm_gem_unpin_locked(obj);
+ }
+
+ submit->bos_pinned = false;
+}
+
static void submit_attach_object_fences(struct msm_gem_submit *submit)
{
int i;
@@ -525,7 +537,7 @@ static void submit_cleanup(struct msm_gem_submit *submit, bool error)
unsigned i;

if (error)
- cleanup_flags |= BO_PINNED;
+ submit_unpin_objects(submit);

for (i = 0; i < submit->nr_bos; i++) {
struct drm_gem_object *obj = submit->bos[i].obj;
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c
index 929df7243792..bd125ca4d230 100644
--- a/drivers/gpu/drm/msm/msm_ringbuffer.c
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
@@ -29,9 +29,10 @@ static struct dma_fence *msm_job_run(struct drm_sched_job *job)
struct drm_gem_object *obj = submit->bos[i].obj;

msm_gem_unpin_active(obj);
- submit->bos[i].flags &= ~BO_PINNED;
}

+ submit->bos_pinned = false;
+
mutex_unlock(&priv->lru.lock);

msm_gpu_submit(gpu, submit);
--
2.42.0


2023-12-03 11:31:46

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v2 4/7] drm/msm/gem: Split out submit_unpin_objects() helper

On 21/11/2023 02:38, Rob Clark wrote:
> From: Rob Clark <[email protected]>
>
> Untangle unpinning from unlock/unref loop. The unpin only happens in
> error paths so it is easier to decouple from the normal unlock path.
>
> Since we never have an intermediate state where a subset of buffers
> are pinned (ie. we never bail out of the pin or unpin loops) we can
> replace the bo state flag bit with a global flag in the submit.
>
> Signed-off-by: Rob Clark <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_gem.h | 6 +++---
> drivers/gpu/drm/msm/msm_gem_submit.c | 22 +++++++++++++++++-----
> drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
> 3 files changed, 22 insertions(+), 9 deletions(-)

Reviewed-by: Dmitry Baryshkov <[email protected]>

--
With best wishes
Dmitry