From 7e688294c61bdcb7468a71f7bb5a9c2d42fbd2a8 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Mon, 19 Oct 2020 14:10:51 -0700 Subject: drm/msm/gpu: Convert retire/recover work to kthread_worker Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 55d16489d0f3..3f192a9df0a7 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -441,7 +441,7 @@ find_submit(struct msm_ringbuffer *ring, uint32_t fence) static void retire_submits(struct msm_gpu *gpu); -static void recover_worker(struct work_struct *work) +static void recover_worker(struct kthread_work *work) { struct msm_gpu *gpu = container_of(work, struct msm_gpu, recover_work); struct drm_device *dev = gpu->dev; @@ -544,7 +544,6 @@ static void hangcheck_handler(struct timer_list *t) { struct msm_gpu *gpu = from_timer(gpu, t, hangcheck_timer); struct drm_device *dev = gpu->dev; - struct msm_drm_private *priv = dev->dev_private; struct msm_ringbuffer *ring = gpu->funcs->active_ring(gpu); uint32_t fence = ring->memptrs->fence; @@ -561,7 +560,7 @@ static void hangcheck_handler(struct timer_list *t) DRM_DEV_ERROR(dev->dev, "%s: submitted fence: %u\n", gpu->name, ring->seqno); - queue_work(priv->wq, &gpu->recover_work); + kthread_queue_work(gpu->worker, &gpu->recover_work); } /* if still more pending work, reset the hangcheck timer: */ @@ -569,7 +568,7 @@ static void hangcheck_handler(struct timer_list *t) hangcheck_timer_reset(gpu); /* workaround for missing irq: */ - queue_work(priv->wq, &gpu->retire_work); + kthread_queue_work(gpu->worker, &gpu->retire_work); } /* @@ -728,7 +727,7 @@ static void retire_submits(struct msm_gpu *gpu) } } -static void retire_worker(struct work_struct *work) +static void retire_worker(struct kthread_work *work) { struct msm_gpu *gpu = container_of(work, struct msm_gpu, retire_work); struct drm_device *dev = gpu->dev; @@ -745,8 +744,7 @@ static void retire_worker(struct work_struct *work) /* call from irq handler to schedule work to retire bo's */ void msm_gpu_retire(struct msm_gpu *gpu) { - struct msm_drm_private *priv = gpu->dev->dev_private; - queue_work(priv->wq, &gpu->retire_work); + kthread_queue_work(gpu->worker, &gpu->retire_work); update_sw_cntrs(gpu); } @@ -869,10 +867,18 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, gpu->funcs = funcs; gpu->name = name; - INIT_LIST_HEAD(&gpu->active_list); - INIT_WORK(&gpu->retire_work, retire_worker); - INIT_WORK(&gpu->recover_work, recover_worker); + gpu->worker = kthread_create_worker(0, "%s-worker", gpu->name); + if (IS_ERR(gpu->worker)) { + ret = PTR_ERR(gpu->worker); + gpu->worker = NULL; + goto fail; + } + sched_set_fifo_low(gpu->worker->task); + + INIT_LIST_HEAD(&gpu->active_list); + kthread_init_work(&gpu->retire_work, retire_worker); + kthread_init_work(&gpu->recover_work, recover_worker); timer_setup(&gpu->hangcheck_timer, hangcheck_handler, 0); @@ -1005,4 +1011,8 @@ void msm_gpu_cleanup(struct msm_gpu *gpu) gpu->aspace->mmu->funcs->detach(gpu->aspace->mmu); msm_gem_address_space_put(gpu->aspace); } + + if (gpu->worker) { + kthread_destroy_worker(gpu->worker); + } } -- cgit From dd29bd41d4c6e1fc837a28f9a68bf344500dcced Mon Sep 17 00:00:00 2001 From: Tian Tao Date: Mon, 19 Oct 2020 14:04:22 +0800 Subject: drm/msm: Remove redundant null check clk_prepare_enable() and clk_disable_unprepare() will check NULL clock parameter, so It is not necessary to add additional checks. Signed-off-by: Tian Tao Reviewed-by: Jordan Crouse Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 3f192a9df0a7..4426f5074dfc 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -177,15 +177,12 @@ static int disable_clk(struct msm_gpu *gpu) static int enable_axi(struct msm_gpu *gpu) { - if (gpu->ebi1_clk) - clk_prepare_enable(gpu->ebi1_clk); - return 0; + return clk_prepare_enable(gpu->ebi1_clk); } static int disable_axi(struct msm_gpu *gpu) { - if (gpu->ebi1_clk) - clk_disable_unprepare(gpu->ebi1_clk); + clk_disable_unprepare(gpu->ebi1_clk); return 0; } -- cgit From 6c0e3ea25047694f97d01fb88c23ed5c4ed44da3 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 23 Oct 2020 09:51:10 -0700 Subject: drm/msm/gem: Switch over to obj->resv for locking This also converts the special msm_gem_get_vaddr_active() to expect the lock to already be held. There are two call-sites for this, one already has the lock held, so it is more straightforward to just open-code the locking for the other caller. Signed-off-by: Rob Clark Reviewed-by: Kristian H. Kristensen Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 17 +++++++---------- drivers/gpu/drm/msm/msm_gem.h | 16 +++++----------- drivers/gpu/drm/msm/msm_gem_submit.c | 8 ++++---- drivers/gpu/drm/msm/msm_gpu.c | 14 ++++++++++++-- drivers/gpu/drm/msm/msm_rd.c | 2 +- 5 files changed, 29 insertions(+), 28 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 59d7c1a7d502..6935c4437ef9 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -643,13 +643,7 @@ void *msm_gem_get_vaddr(struct drm_gem_object *obj) */ void *msm_gem_get_vaddr_active(struct drm_gem_object *obj) { - void *ret; - - msm_gem_lock(obj); - ret = get_vaddr(obj, __MSM_MADV_PURGED); - msm_gem_unlock(obj); - - return ret; + return get_vaddr(obj, __MSM_MADV_PURGED); } void msm_gem_put_vaddr_locked(struct drm_gem_object *obj) @@ -969,15 +963,20 @@ static void free_object(struct msm_gem_object *msm_obj) if (msm_obj->pages) kvfree(msm_obj->pages); + /* dma_buf_detach() grabs resv lock, so we need to unlock + * prior to drm_prime_gem_destroy + */ + msm_gem_unlock(obj); + drm_prime_gem_destroy(obj, msm_obj->sgt); } else { msm_gem_vunmap(obj); put_pages(obj); + msm_gem_unlock(obj); } drm_gem_object_release(obj); - msm_gem_unlock(obj); kfree(msm_obj); } @@ -1049,8 +1048,6 @@ static int msm_gem_new_impl(struct drm_device *dev, if (!msm_obj) return -ENOMEM; - mutex_init(&msm_obj->lock); - msm_obj->flags = flags; msm_obj->madv = MSM_MADV_WILLNEED; diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 1f4e5d871a23..f0608d96ef03 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -85,7 +85,6 @@ struct msm_gem_object { * an IOMMU. Also used for stolen/splashscreen buffer. */ struct drm_mm_node *vram_node; - struct mutex lock; /* Protects resources associated with bo */ char name[32]; /* Identifier to print for the debugfs files */ @@ -158,36 +157,31 @@ void msm_gem_describe_objects(struct list_head *list, struct seq_file *m); static inline void msm_gem_lock(struct drm_gem_object *obj) { - struct msm_gem_object *msm_obj = to_msm_bo(obj); - mutex_lock(&msm_obj->lock); + dma_resv_lock(obj->resv, NULL); } static inline bool __must_check msm_gem_trylock(struct drm_gem_object *obj) { - struct msm_gem_object *msm_obj = to_msm_bo(obj); - return mutex_trylock(&msm_obj->lock) == 1; + return dma_resv_trylock(obj->resv); } static inline int msm_gem_lock_interruptible(struct drm_gem_object *obj) { - struct msm_gem_object *msm_obj = to_msm_bo(obj); - return mutex_lock_interruptible(&msm_obj->lock); + return dma_resv_lock_interruptible(obj->resv, NULL); } static inline void msm_gem_unlock(struct drm_gem_object *obj) { - struct msm_gem_object *msm_obj = to_msm_bo(obj); - mutex_unlock(&msm_obj->lock); + dma_resv_unlock(obj->resv); } static inline bool msm_gem_is_locked(struct drm_gem_object *obj) { - struct msm_gem_object *msm_obj = to_msm_bo(obj); - return mutex_is_locked(&msm_obj->lock); + return dma_resv_is_locked(obj->resv); } static inline bool is_active(struct msm_gem_object *msm_obj) diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index aa3c7af54079..044e9bee70a2 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -221,7 +221,7 @@ static void submit_unlock_unpin_bo(struct msm_gem_submit *submit, struct msm_gem_object *msm_obj = submit->bos[i].obj; if (submit->bos[i].flags & BO_PINNED) - msm_gem_unpin_iova(&msm_obj->base, submit->aspace); + msm_gem_unpin_iova_locked(&msm_obj->base, submit->aspace); if (submit->bos[i].flags & BO_LOCKED) dma_resv_unlock(msm_obj->base.resv); @@ -324,7 +324,7 @@ static int submit_pin_objects(struct msm_gem_submit *submit) uint64_t iova; /* if locking succeeded, pin bo: */ - ret = msm_gem_get_and_pin_iova(&msm_obj->base, + ret = msm_gem_get_and_pin_iova_locked(&msm_obj->base, submit->aspace, &iova); if (ret) @@ -383,7 +383,7 @@ static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *ob /* For now, just map the entire thing. Eventually we probably * to do it page-by-page, w/ kmap() if not vmap()d.. */ - ptr = msm_gem_get_vaddr(&obj->base); + ptr = msm_gem_get_vaddr_locked(&obj->base); if (IS_ERR(ptr)) { ret = PTR_ERR(ptr); @@ -434,7 +434,7 @@ static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *ob } out: - msm_gem_put_vaddr(&obj->base); + msm_gem_put_vaddr_locked(&obj->base); return ret; } diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 4426f5074dfc..fb368b1791fe 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -323,7 +323,9 @@ static void msm_gpu_crashstate_get_bo(struct msm_gpu_state *state, if (!state_bo->data) goto out; + msm_gem_lock(&obj->base); ptr = msm_gem_get_vaddr_active(&obj->base); + msm_gem_unlock(&obj->base); if (IS_ERR(ptr)) { kvfree(state_bo->data); state_bo->data = NULL; @@ -467,14 +469,22 @@ static void recover_worker(struct kthread_work *work) put_task_struct(task); } + /* msm_rd_dump_submit() needs bo locked to dump: */ + for (i = 0; i < submit->nr_bos; i++) + msm_gem_lock(&submit->bos[i].obj->base); + if (comm && cmd) { DRM_DEV_ERROR(dev->dev, "%s: offending task: %s (%s)\n", gpu->name, comm, cmd); msm_rd_dump_submit(priv->hangrd, submit, "offending task: %s (%s)", comm, cmd); - } else + } else { msm_rd_dump_submit(priv->hangrd, submit, NULL); + } + + for (i = 0; i < submit->nr_bos; i++) + msm_gem_unlock(&submit->bos[i].obj->base); } /* Record the crash state */ @@ -779,7 +789,7 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) /* submit takes a reference to the bo and iova until retired: */ drm_gem_object_get(&msm_obj->base); - msm_gem_get_and_pin_iova(&msm_obj->base, submit->aspace, &iova); + msm_gem_get_and_pin_iova_locked(&msm_obj->base, submit->aspace, &iova); if (submit->bos[i].flags & MSM_SUBMIT_BO_WRITE) dma_resv_add_excl_fence(drm_obj->resv, submit->fence); diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c index fea30e7aa9e8..659e5cc4b40a 100644 --- a/drivers/gpu/drm/msm/msm_rd.c +++ b/drivers/gpu/drm/msm/msm_rd.c @@ -333,7 +333,7 @@ static void snapshot_buf(struct msm_rd_state *rd, rd_write_section(rd, RD_BUFFER_CONTENTS, buf, size); - msm_gem_put_vaddr(&obj->base); + msm_gem_put_vaddr_locked(&obj->base); } /* called under struct_mutex */ -- cgit From 07ddf4c30c4d7475954ee01032844f7f458f91e7 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 23 Oct 2020 09:51:12 -0700 Subject: drm/msm: Drop chatty trace It is somewhat redundant with the gpu tracepoints, and anyways not too useful to justify spamming the log when debug traces are enabled. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse Reviewed-by: Kristian H. Kristensen Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index fb368b1791fe..8e17cb7d255f 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -542,7 +542,6 @@ static void recover_worker(struct kthread_work *work) static void hangcheck_timer_reset(struct msm_gpu *gpu) { - DBG("%s", gpu->name); mod_timer(&gpu->hangcheck_timer, round_jiffies_up(jiffies + DRM_MSM_HANGCHECK_JIFFIES)); } -- cgit From 2a86efb1bf72d25c64de3f2ff529a89e76ce7705 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 23 Oct 2020 09:51:13 -0700 Subject: drm/msm: Move update_fences() Small cleanup, update_fences() is used in the hangcheck path, but also in the normal retire path. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse Reviewed-by: Kristian H. Kristensen Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 8e17cb7d255f..93965f2119db 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -262,6 +262,20 @@ int msm_gpu_hw_init(struct msm_gpu *gpu) return ret; } +static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring, + uint32_t fence) +{ + struct msm_gem_submit *submit; + + list_for_each_entry(submit, &ring->submits, node) { + if (submit->seqno > fence) + break; + + msm_update_fence(submit->ring->fctx, + submit->fence->seqno); + } +} + #ifdef CONFIG_DEV_COREDUMP static ssize_t msm_gpu_devcoredump_read(char *buffer, loff_t offset, size_t count, void *data, size_t datalen) @@ -410,20 +424,6 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu, * Hangcheck detection for locked gpu: */ -static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring, - uint32_t fence) -{ - struct msm_gem_submit *submit; - - list_for_each_entry(submit, &ring->submits, node) { - if (submit->seqno > fence) - break; - - msm_update_fence(submit->ring->fctx, - submit->fence->seqno); - } -} - static struct msm_gem_submit * find_submit(struct msm_ringbuffer *ring, uint32_t fence) { -- cgit From 77d205290aa944895cf7e8bea500b8fd95a6b05b Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 23 Oct 2020 09:51:16 -0700 Subject: drm/msm: Protect ring->submits with it's own lock One less place to rely on dev->struct_mutex. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse Reviewed-by: Kristian H. Kristensen Signed-off-by: Rob Clark --- 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(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 044e9bee70a2..24ce4c65429d 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -65,7 +65,9 @@ void msm_gem_submit_free(struct msm_gem_submit *submit) unsigned i; 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 93965f2119db..6e6b170b9e5d 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -267,6 +267,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; @@ -274,6 +275,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 @@ -429,11 +431,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; } @@ -530,8 +535,10 @@ static void recover_worker(struct kthread_work *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); } } @@ -717,7 +724,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)); @@ -726,9 +732,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; + } } } } @@ -770,7 +791,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; -- cgit From 964d2f97e74b3951973bc22d8a5547f6c0d505b4 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 23 Oct 2020 09:51:17 -0700 Subject: drm/msm: Refcount submits Before we remove dev->struct_mutex from the retire path, we have to deal with the situation of a submit retiring before the submit ioctl returns. To deal with this, ring->submits will hold a reference to the submit, which is dropped when the submit is retired. And the submit ioctl path holds it's own ref, which it drops when it is done with the submit. Also, add to submit list *after* getting/pinning bo's, to prevent badness in case the completed fence is corrupted, and retire_worker mistakenly believes the submit is done too early. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse Reviewed-by: Kristian H. Kristensen Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_drv.h | 1 - drivers/gpu/drm/msm/msm_gem.h | 13 +++++++++++++ drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++++------ drivers/gpu/drm/msm/msm_gpu.c | 21 ++++++++++++++++----- 4 files changed, 34 insertions(+), 12 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index e3a8fae8c9f6..c1212e021dd4 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -278,7 +278,6 @@ void msm_unregister_mmu(struct drm_device *dev, struct msm_mmu *mmu); bool msm_use_mmu(struct drm_device *dev); -void msm_gem_submit_free(struct msm_gem_submit *submit); int msm_ioctl_gem_submit(struct drm_device *dev, void *data, struct drm_file *file); diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index f0608d96ef03..2f289c436ddd 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -213,6 +213,7 @@ void msm_gem_free_work(struct work_struct *work); * lasts for the duration of the submit-ioctl. */ struct msm_gem_submit { + struct kref ref; struct drm_device *dev; struct msm_gpu *gpu; struct msm_gem_address_space *aspace; @@ -249,6 +250,18 @@ struct msm_gem_submit { } bos[]; }; +void __msm_gem_submit_destroy(struct kref *kref); + +static inline void msm_gem_submit_get(struct msm_gem_submit *submit) +{ + kref_get(&submit->ref); +} + +static inline void msm_gem_submit_put(struct msm_gem_submit *submit) +{ + kref_put(&submit->ref, __msm_gem_submit_destroy); +} + /* helper to determine of a buffer in submit should be dumped, used for both * devcoredump and debugfs cmdstream dumping: */ diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 24ce4c65429d..d04c349d8112 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -42,6 +42,7 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, if (!submit) return NULL; + kref_init(&submit->ref); submit->dev = dev; submit->aspace = queue->ctx->aspace; submit->gpu = gpu; @@ -60,14 +61,13 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, return submit; } -void msm_gem_submit_free(struct msm_gem_submit *submit) +void __msm_gem_submit_destroy(struct kref *kref) { + struct msm_gem_submit *submit = + container_of(kref, struct msm_gem_submit, ref); unsigned i; 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); @@ -847,8 +847,7 @@ out_pre_pm: submit_cleanup(submit); if (has_ww_ticket) ww_acquire_fini(&submit->ticket); - if (ret) - msm_gem_submit_free(submit); + msm_gem_submit_put(submit); out_unlock: if (ret && (out_fence_fd >= 0)) put_unused_fd(out_fence_fd); diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 6e6b170b9e5d..a69f6cb38838 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -718,7 +718,12 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring, pm_runtime_mark_last_busy(&gpu->pdev->dev); pm_runtime_put_autosuspend(&gpu->pdev->dev); - msm_gem_submit_free(submit); + + spin_lock(&ring->submit_lock); + list_del(&submit->node); + spin_unlock(&ring->submit_lock); + + msm_gem_submit_put(submit); } static void retire_submits(struct msm_gpu *gpu) @@ -791,10 +796,6 @@ 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); update_sw_cntrs(gpu); @@ -821,6 +822,16 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) msm_gem_active_get(drm_obj, gpu); } + /* + * ring->submits holds a ref to the submit, to deal with the case + * that a submit completes before msm_ioctl_gem_submit() returns. + */ + msm_gem_submit_get(submit); + + spin_lock(&ring->submit_lock); + list_add_tail(&submit->node, &ring->submits); + spin_unlock(&ring->submit_lock); + gpu->funcs->submit(gpu, submit); priv->lastctx = submit->queue->ctx; -- cgit From fb1a1fcbacefd839503e94a66a467809ade27b2c Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 23 Oct 2020 09:51:18 -0700 Subject: drm/msm: Remove obj->gpu It cannot be atomically updated with obj->active_count, and the only purpose is a useless WARN_ON() (which becomes a buggy WARN_ON() once retire_submits() is not serialized with incoming submits via struct_mutex) Signed-off-by: Rob Clark Reviewed-by: Kristian H. Kristensen Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 2 -- drivers/gpu/drm/msm/msm_gem.h | 1 - drivers/gpu/drm/msm/msm_gpu.c | 5 ----- 3 files changed, 8 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 084131db89d2..dd3c21bb8740 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -774,7 +774,6 @@ void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu) if (!atomic_fetch_inc(&msm_obj->active_count)) { mutex_lock(&priv->mm_lock); - msm_obj->gpu = gpu; list_del_init(&msm_obj->mm_list); list_add_tail(&msm_obj->mm_list, &gpu->active_list); mutex_unlock(&priv->mm_lock); @@ -790,7 +789,6 @@ void msm_gem_active_put(struct drm_gem_object *obj) if (!atomic_dec_return(&msm_obj->active_count)) { mutex_lock(&priv->mm_lock); - msm_obj->gpu = NULL; list_del_init(&msm_obj->mm_list); list_add_tail(&msm_obj->mm_list, &priv->inactive_list); mutex_unlock(&priv->mm_lock); diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 2f289c436ddd..f4e73c6f07bf 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -64,7 +64,6 @@ struct msm_gem_object { * */ struct list_head mm_list; - struct msm_gpu *gpu; /* non-null if active */ /* Transiently in the process of submit ioctl, objects associated * with the submit are on submit->bo_list.. this only lasts for diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index a69f6cb38838..ee1eae7a02bb 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -805,11 +805,6 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) struct drm_gem_object *drm_obj = &msm_obj->base; uint64_t iova; - /* can't happen yet.. but when we add 2d support we'll have - * to deal w/ cross-ring synchronization: - */ - WARN_ON(is_active(msm_obj) && (msm_obj->gpu != gpu)); - /* submit takes a reference to the bo and iova until retired: */ drm_gem_object_get(&msm_obj->base); msm_gem_get_and_pin_iova_locked(&msm_obj->base, submit->aspace, &iova); -- cgit From c5c1643cef7a11c1f3a5544528087618ba9a5aab Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 23 Oct 2020 09:51:19 -0700 Subject: drm/msm: Drop struct_mutex from the retire path Now that we are not relying on dev->struct_mutex to protect the ring->submits lists, drop the struct_mutex lock. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse Reviewed-by: Kristian H. Kristensen Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index ee1eae7a02bb..dbb8400f1872 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -713,7 +713,7 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring, msm_gem_active_put(&msm_obj->base); msm_gem_unpin_iova(&msm_obj->base, submit->aspace); - drm_gem_object_put_locked(&msm_obj->base); + drm_gem_object_put(&msm_obj->base); } pm_runtime_mark_last_busy(&gpu->pdev->dev); @@ -728,11 +728,8 @@ 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; int i; - WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - /* Retire the commits starting with highest priority */ for (i = 0; i < gpu->nr_rings; i++) { struct msm_ringbuffer *ring = gpu->rb[i]; @@ -762,15 +759,12 @@ static void retire_submits(struct msm_gpu *gpu) static void retire_worker(struct kthread_work *work) { struct msm_gpu *gpu = container_of(work, struct msm_gpu, retire_work); - struct drm_device *dev = gpu->dev; int i; for (i = 0; i < gpu->nr_rings; i++) update_fences(gpu, gpu->rb[i], gpu->rb[i]->memptrs->fence); - mutex_lock(&dev->struct_mutex); retire_submits(gpu); - mutex_unlock(&dev->struct_mutex); } /* call from irq handler to schedule work to retire bo's */ -- cgit From ec793cf01d1fe7f2d3eef30a0accfb3ca880abe4 Mon Sep 17 00:00:00 2001 From: Akhil P Oommen Date: Fri, 30 Oct 2020 16:17:10 +0530 Subject: drm/msm: Add support for GPU cooling Register GPU as a devfreq cooling device so that it can be passively cooled by the thermal framework. Signed-off-by: Akhil P Oommen Tested-by: Matthias Kaehlcke Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 12 ++++++++++++ drivers/gpu/drm/msm/msm_gpu.h | 2 ++ 2 files changed, 14 insertions(+) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index dbb8400f1872..dfa3b5ad2099 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -107,9 +108,18 @@ static void msm_devfreq_init(struct msm_gpu *gpu) if (IS_ERR(gpu->devfreq.devfreq)) { DRM_DEV_ERROR(&gpu->pdev->dev, "Couldn't initialize GPU devfreq\n"); gpu->devfreq.devfreq = NULL; + return; } devfreq_suspend_device(gpu->devfreq.devfreq); + + gpu->cooling = of_devfreq_cooling_register(gpu->pdev->dev.of_node, + gpu->devfreq.devfreq); + if (IS_ERR(gpu->cooling)) { + DRM_DEV_ERROR(&gpu->pdev->dev, + "Couldn't register GPU cooling device\n"); + gpu->cooling = NULL; + } } static int enable_pwrrail(struct msm_gpu *gpu) @@ -1044,4 +1054,6 @@ void msm_gpu_cleanup(struct msm_gpu *gpu) if (gpu->worker) { kthread_destroy_worker(gpu->worker); } + + devfreq_cooling_unregister(gpu->cooling); } diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index 09938ae114ec..786cfc285be0 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -155,6 +155,8 @@ struct msm_gpu { struct msm_gpu_state *crashstate; /* True if the hardware supports expanded apriv (a650 and newer) */ bool hw_apriv; + + struct thermal_cooling_device *cooling; }; static inline struct msm_gpu *dev_to_gpu(struct device *dev) -- cgit From ab5c54cb88350e224632e5b0fcd7f86ece06beb9 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Mon, 16 Nov 2020 09:48:49 -0800 Subject: drm/msm: Protect obj->active_count under obj lock Previously we only held obj lock in the _active_get() path, and relied on atomic_dec_return() to not be racy in the _active_put() path where obj lock was not held. But this is a false sense of security. Unlike obj lifetime refcnt, where you do not expect to *increase* the refcnt after the last put (which would mean that something has gone horribly wrong with the object liveness reference counting), the active_count can increase again from zero. Racing _active_put()s and _active_get()s could leave the obj on the wrong mm list. But in the retire path, immediately after the _active_put(), the _unpin_iova() would acquire obj lock. So just move the locking earlier and rely on that to protect obj->active_count. Fixes: c5c1643cef7a ("drm/msm: Drop struct_mutex from the retire path") Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 11 ++++++----- drivers/gpu/drm/msm/msm_gem.h | 5 +++-- drivers/gpu/drm/msm/msm_gpu.c | 10 ++++++---- 3 files changed, 15 insertions(+), 11 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gpu.c') diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 0319e0ad07f5..562db92aa631 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -769,7 +769,7 @@ void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu) WARN_ON(!msm_gem_is_locked(obj)); WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED); - if (!atomic_fetch_inc(&msm_obj->active_count)) { + if (msm_obj->active_count++ == 0) { mutex_lock(&priv->mm_lock); list_del_init(&msm_obj->mm_list); list_add_tail(&msm_obj->mm_list, &gpu->active_list); @@ -783,8 +783,9 @@ void msm_gem_active_put(struct drm_gem_object *obj) struct msm_drm_private *priv = obj->dev->dev_private; might_sleep(); + WARN_ON(!msm_gem_is_locked(obj)); - if (!atomic_dec_return(&msm_obj->active_count)) { + if (--msm_obj->active_count == 0) { mutex_lock(&priv->mm_lock); list_del_init(&msm_obj->mm_list); list_add_tail(&msm_obj->mm_list, &priv->inactive_list); @@ -935,15 +936,15 @@ void msm_gem_free_object(struct drm_gem_object *obj) struct drm_device *dev = obj->dev; struct msm_drm_private *priv = dev->dev_private; - /* object should not be on active list: */ - WARN_ON(is_active(msm_obj)); - mutex_lock(&priv->mm_lock); list_del(&msm_obj->mm_list); mutex_unlock(&priv->mm_lock); msm_gem_lock(obj); + /* object should not be on active list: */ + WARN_ON(is_active(msm_obj)); + put_iova(obj); if (obj->import_attach) { diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index d79e7019cc88..3355a48a023b 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -87,7 +87,7 @@ struct msm_gem_object { char name[32]; /* Identifier to print for the debugfs files */ - atomic_t active_count; + int active_count; }; #define to_msm_bo(x) container_of(x, struct msm_gem_object, base) @@ -185,7 +185,8 @@ msm_gem_is_locked(struct drm_gem_object *obj) static inline bool is_active(struct msm_gem_object *msm_obj) { - return atomic_read(&msm_obj->active_count); + WARN_ON(!msm_gem_is_locked(&msm_obj->base)); + return msm_obj->active_count; } static inline bool is_purgeable(struct msm_gem_object *msm_obj) diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index dfa3b5ad2099..ab7c167b0623 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -719,11 +719,13 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring, stats->alwayson_start, stats->alwayson_end); for (i = 0; i < submit->nr_bos; i++) { - struct msm_gem_object *msm_obj = submit->bos[i].obj; + struct drm_gem_object *obj = &submit->bos[i].obj->base; - msm_gem_active_put(&msm_obj->base); - msm_gem_unpin_iova(&msm_obj->base, submit->aspace); - drm_gem_object_put(&msm_obj->base); + msm_gem_lock(obj); + msm_gem_active_put(obj); + msm_gem_unpin_iova_locked(obj, submit->aspace); + msm_gem_unlock(obj); + drm_gem_object_put(obj); } pm_runtime_mark_last_busy(&gpu->pdev->dev); -- cgit