| Message ID | 20201004192152.3298573-7-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm: de-struct_mutex-ification | expand |
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
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;