Re: [Freedreno] [PATCH 06/14] drm/msm: Protect ring->submits with it's own lock

On Sun, Oct 04, 2020 at 12:21:38PM -0700, Rob Clark wrote:

From: Rob Clark robdclark@chromium.org

One less place to rely on dev->struct_mutex.

Reviewed-by: Jordan Crouse jcrouse@codeaurora.org

Signed-off-by: Rob Clark robdclark@chromium.org

drivers/gpu/drm/msm/msm_gem_submit.c | 2 ++ drivers/gpu/drm/msm/msm_gpu.c | 37 ++++++++++++++++++++++------ drivers/gpu/drm/msm/msm_ringbuffer.c | 1 + drivers/gpu/drm/msm/msm_ringbuffer.h | 6 +++++ 4 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index aa5c60a7132d..e1d1f005b3d4 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -63,7 +63,9 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, void msm_gem_submit_free(struct msm_gem_submit *submit) { dma_fence_put(submit->fence);

• spin_lock(&submit->ring->submit_lock); list_del(&submit->node);
• spin_unlock(&submit->ring->submit_lock); put_pid(submit->pid); msm_submitqueue_put(submit->queue);

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index ca8c95b32c8b..8d1e254f964a 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -270,6 +270,7 @@ static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring, { struct msm_gem_submit *submit;

• spin_lock(&ring->submit_lock); list_for_each_entry(submit, &ring->submits, node) { if (submit->seqno > fence) break;

@@ -277,6 +278,7 @@ static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring, msm_update_fence(submit->ring->fctx, submit->fence->seqno); }

• spin_unlock(&ring->submit_lock);

}

#ifdef CONFIG_DEV_COREDUMP @@ -430,11 +432,14 @@ find_submit(struct msm_ringbuffer *ring, uint32_t fence) { struct msm_gem_submit *submit;

• WARN_ON(!mutex_is_locked(&ring->gpu->dev->struct_mutex));
• list_for_each_entry(submit, &ring->submits, node)

• if (submit->seqno == fence)
  

• spin_lock(&ring->submit_lock);

• list_for_each_entry(submit, &ring->submits, node) {

• if (submit->seqno == fence) {
  

• 	spin_unlock(&ring->submit_lock);
  return submit;
  

• }
  

• }

• spin_unlock(&ring->submit_lock);

  return NULL;

} @@ -523,8 +528,10 @@ static void recover_worker(struct work_struct *work) for (i = 0; i < gpu->nr_rings; i++) { struct msm_ringbuffer *ring = gpu->rb[i];

• 	spin_lock(&ring->submit_lock);
  list_for_each_entry(submit, &ring->submits, node)
  	gpu->funcs->submit(gpu, submit);
  

• 	spin_unlock(&ring->submit_lock);
  

  } }

@@ -711,7 +718,6 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring, static void retire_submits(struct msm_gpu *gpu) { struct drm_device *dev = gpu->dev;

• struct msm_gem_submit *submit, *tmp; int i;

  WARN_ON(!mutex_is_locked(&dev->struct_mutex));

@@ -720,9 +726,24 @@ static void retire_submits(struct msm_gpu *gpu) for (i = 0; i < gpu->nr_rings; i++) { struct msm_ringbuffer *ring = gpu->rb[i];

• list_for_each_entry_safe(submit, tmp, &ring->submits, node) {
  

• 	if (dma_fence_is_signaled(submit->fence))
  

• while (true) {
  

• 	struct msm_gem_submit *submit = NULL;
  

• 	spin_lock(&ring->submit_lock);
  

• 	submit = list_first_entry_or_null(&ring->submits,
  

• 			struct msm_gem_submit, node);
  

• 	spin_unlock(&ring->submit_lock);
  

• 	/*
  

• 	 * If no submit, we are done.  If submit->fence hasn't
  

• 	 * been signalled, then later submits are not signalled
  

• 	 * either, so we are also done.
  

• 	 */
  

• 	if (submit && dma_fence_is_signaled(submit->fence)) {
  	retire_submit(gpu, ring, submit);
  

• 	} else {
  

• 		break;
  

• 	}
  

  } }

} @@ -765,7 +786,9 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)

submit->seqno = ++ring->seqno;

• spin_lock(&ring->submit_lock); list_add_tail(&submit->node, &ring->submits);

• spin_unlock(&ring->submit_lock);

  msm_rd_dump_submit(priv->rd, submit, NULL);

diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c index 1b6958e908dc..4d2a2a4abef8 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.c +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c @@ -46,6 +46,7 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu *gpu, int id, ring->memptrs_iova = memptrs_iova;

INIT_LIST_HEAD(&ring->submits);

• spin_lock_init(&ring->submit_lock); spin_lock_init(&ring->preempt_lock);

  snprintf(name, sizeof(name), "gpu-ring-%d", ring->id);

diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h index 4956d1bc5d0e..fe55d4a1aa16 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.h +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h @@ -39,7 +39,13 @@ struct msm_ringbuffer { int id; struct drm_gem_object *bo; uint32_t *start, *end, *cur, *next;

• /*

• * List of in-flight submits on this ring.  Protected by submit_lock.
  

• */
  

  struct list_head submits;
• spinlock_t submit_lock;
• uint64_t iova; uint32_t seqno; uint32_t hangcheck_fence;

-- 2.26.2

---

Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno