From: Rob Clark robdclark@chromium.org
The first six paches are various cleanups and simplifications. The next two get rid of redundant vma lookups in the submit and retire paths. Following that, fenced vma lets us indicate a fence value following which the vma is no longer used, which is needed because otherwise userspace could observe the signaled fence prior to retire_submits() finishing. (With userspace allocated GPU addresses userspace is tracking when a buffer is no longer used and it's vma can be deleted.) And finally the last patch adds the new uabi for user- space allocated iova.
v2: Fix inuse check in last patch, split GEM_WARN_ON() conversion to separate patch, and nits v3: Fix nested fb prepares in 06/10, return -EBUSY if trying to set iova on buffer that already has one in 10/10 v4: Fix race condition in 09/10
Rob Clark (10): drm/msm/gem: Move prototypes drm/msm/gpu: Drop duplicate fence counter drm/msm/gem: Convert some missed GEM_WARN_ON()s drm/msm/gem: Split out inuse helper drm/msm/gem: Drop PAGE_SHIFT for address space mm drm/msm: Drop msm_gem_iova() drm/msm/gem: Rework vma lookup and pin drm/msm/gem: Split vma lookup and pin drm/msm/gem: Add fenced vma unpin drm/msm: Add a way for userspace to allocate GPU iova
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 14 ++- drivers/gpu/drm/msm/msm_drv.c | 21 ++++ drivers/gpu/drm/msm/msm_drv.h | 23 ---- drivers/gpu/drm/msm/msm_fb.c | 20 +++- drivers/gpu/drm/msm/msm_fence.c | 6 +- drivers/gpu/drm/msm/msm_fence.h | 3 + drivers/gpu/drm/msm/msm_gem.c | 151 ++++++++++++++---------- drivers/gpu/drm/msm/msm_gem.h | 52 +++++++- drivers/gpu/drm/msm/msm_gem_submit.c | 32 ++--- drivers/gpu/drm/msm/msm_gem_vma.c | 61 +++++++--- drivers/gpu/drm/msm/msm_gpu.c | 8 +- drivers/gpu/drm/msm/msm_gpu.h | 2 +- drivers/gpu/drm/msm/msm_ringbuffer.c | 13 +- drivers/gpu/drm/msm/msm_ringbuffer.h | 1 - include/uapi/drm/msm_drm.h | 3 + 18 files changed, 277 insertions(+), 139 deletions(-)
From: Rob Clark robdclark@chromium.org
These belong more cleanly in the gem header.
Signed-off-by: Rob Clark robdclark@chromium.org Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- drivers/gpu/drm/msm/msm_drv.h | 23 ----------------------- drivers/gpu/drm/msm/msm_gem.h | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index d661debb50f1..000fe649a0ab 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -250,29 +250,6 @@ void msm_atomic_state_free(struct drm_atomic_state *state); int msm_crtc_enable_vblank(struct drm_crtc *crtc); void msm_crtc_disable_vblank(struct drm_crtc *crtc);
-int msm_gem_init_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma, int npages, - u64 range_start, u64 range_end); -void msm_gem_purge_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma); -void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma); -int msm_gem_map_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma, int prot, - struct sg_table *sgt, int npages); -void msm_gem_close_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma); - - -struct msm_gem_address_space * -msm_gem_address_space_get(struct msm_gem_address_space *aspace); - -void msm_gem_address_space_put(struct msm_gem_address_space *aspace); - -struct msm_gem_address_space * -msm_gem_address_space_create(struct msm_mmu *mmu, const char *name, - u64 va_start, u64 size); - int msm_register_mmu(struct drm_device *dev, struct msm_mmu *mmu); void msm_unregister_mmu(struct drm_device *dev, struct msm_mmu *mmu);
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 58e11c282928..947ff7d9b471 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -40,6 +40,15 @@ struct msm_gem_address_space { int faults; };
+struct msm_gem_address_space * +msm_gem_address_space_get(struct msm_gem_address_space *aspace); + +void msm_gem_address_space_put(struct msm_gem_address_space *aspace); + +struct msm_gem_address_space * +msm_gem_address_space_create(struct msm_mmu *mmu, const char *name, + u64 va_start, u64 size); + struct msm_gem_vma { struct drm_mm_node node; uint64_t iova; @@ -49,6 +58,19 @@ struct msm_gem_vma { int inuse; };
+int msm_gem_init_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma, int npages, + u64 range_start, u64 range_end); +void msm_gem_purge_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma); +void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma); +int msm_gem_map_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma, int prot, + struct sg_table *sgt, int npages); +void msm_gem_close_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma); + struct msm_gem_object { struct drm_gem_object base;
From: Rob Clark robdclark@chromium.org
The ring seqno counter duplicates the fence-context last_fence counter. They end up getting incremented in lock-step, on the same scheduler thread, but the split just makes things less obvious.
Signed-off-by: Rob Clark robdclark@chromium.org --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 ++-- drivers/gpu/drm/msm/msm_gpu.c | 8 ++++---- drivers/gpu/drm/msm/msm_gpu.h | 2 +- drivers/gpu/drm/msm/msm_ringbuffer.h | 1 - 6 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index 407f50a15faa..d31aa87c6c8d 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -1235,7 +1235,7 @@ static void a5xx_fault_detect_irq(struct msm_gpu *gpu) return;
DRM_DEV_ERROR(dev->dev, "gpu fault ring %d fence %x status %8.8X rb %4.4x/%4.4x ib1 %16.16llX/%4.4x ib2 %16.16llX/%4.4x\n", - ring ? ring->id : -1, ring ? ring->seqno : 0, + ring ? ring->id : -1, ring ? ring->fctx->last_fence : 0, gpu_read(gpu, REG_A5XX_RBBM_STATUS), gpu_read(gpu, REG_A5XX_CP_RB_RPTR), gpu_read(gpu, REG_A5XX_CP_RB_WPTR), diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 83c31b2ad865..17de46fc4bf2 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -1390,7 +1390,7 @@ static void a6xx_fault_detect_irq(struct msm_gpu *gpu)
DRM_DEV_ERROR(&gpu->pdev->dev, "gpu fault ring %d fence %x status %8.8X rb %4.4x/%4.4x ib1 %16.16llX/%4.4x ib2 %16.16llX/%4.4x\n", - ring ? ring->id : -1, ring ? ring->seqno : 0, + ring ? ring->id : -1, ring ? ring->fctx->last_fence : 0, gpu_read(gpu, REG_A6XX_RBBM_STATUS), gpu_read(gpu, REG_A6XX_CP_RB_RPTR), gpu_read(gpu, REG_A6XX_CP_RB_WPTR), diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 45f2c6084aa7..6385ab06632f 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -578,7 +578,7 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
state->ring[i].fence = gpu->rb[i]->memptrs->fence; state->ring[i].iova = gpu->rb[i]->iova; - state->ring[i].seqno = gpu->rb[i]->seqno; + state->ring[i].seqno = gpu->rb[i]->fctx->last_fence; state->ring[i].rptr = get_rptr(adreno_gpu, gpu->rb[i]); state->ring[i].wptr = get_wptr(gpu->rb[i]);
@@ -828,7 +828,7 @@ void adreno_dump_info(struct msm_gpu *gpu)
printk("rb %d: fence: %d/%d\n", i, ring->memptrs->fence, - ring->seqno); + ring->fctx->last_fence);
printk("rptr: %d\n", get_rptr(adreno_gpu, ring)); printk("rb wptr: %d\n", get_wptr(ring)); diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 747b89aa9d13..9480bdf875db 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -534,7 +534,7 @@ static void hangcheck_handler(struct timer_list *t) if (fence != ring->hangcheck_fence) { /* some progress has been made.. ya! */ ring->hangcheck_fence = fence; - } else if (fence_before(fence, ring->seqno)) { + } else if (fence_before(fence, ring->fctx->last_fence)) { /* no progress and not done.. hung! */ ring->hangcheck_fence = fence; DRM_DEV_ERROR(dev->dev, "%s: hangcheck detected gpu lockup rb %d!\n", @@ -542,13 +542,13 @@ static void hangcheck_handler(struct timer_list *t) DRM_DEV_ERROR(dev->dev, "%s: completed fence: %u\n", gpu->name, fence); DRM_DEV_ERROR(dev->dev, "%s: submitted fence: %u\n", - gpu->name, ring->seqno); + gpu->name, ring->fctx->last_fence);
kthread_queue_work(gpu->worker, &gpu->recover_work); }
/* if still more pending work, reset the hangcheck timer: */ - if (fence_after(ring->seqno, ring->hangcheck_fence)) + if (fence_after(ring->fctx->last_fence, ring->hangcheck_fence)) hangcheck_timer_reset(gpu);
/* workaround for missing irq: */ @@ -770,7 +770,7 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
msm_gpu_hw_init(gpu);
- submit->seqno = ++ring->seqno; + submit->seqno = submit->hw_fence->seqno;
msm_rd_dump_submit(priv->rd, submit, NULL);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index 2c0203fd6ce3..e47a42b1244a 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -291,7 +291,7 @@ static inline bool msm_gpu_active(struct msm_gpu *gpu) for (i = 0; i < gpu->nr_rings; i++) { struct msm_ringbuffer *ring = gpu->rb[i];
- if (fence_after(ring->seqno, ring->memptrs->fence)) + if (fence_after(ring->fctx->last_fence, ring->memptrs->fence)) return true; }
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h index d8c63df4e9ca..2a5045abe46e 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.h +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h @@ -59,7 +59,6 @@ struct msm_ringbuffer { spinlock_t submit_lock;
uint64_t iova; - uint32_t seqno; uint32_t hangcheck_fence; struct msm_rbmemptrs *memptrs; uint64_t memptrs_iova;
From: Rob Clark robdclark@chromium.org
Signed-off-by: Rob Clark robdclark@chromium.org --- drivers/gpu/drm/msm/msm_gem_vma.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index f914ddbaea89..64906594fc65 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -44,7 +44,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, unsigned size = vma->node.size << PAGE_SHIFT;
/* Print a message if we try to purge a vma in use */ - if (WARN_ON(vma->inuse > 0)) + if (GEM_WARN_ON(vma->inuse > 0)) return;
/* Don't do anything if the memory isn't mapped */ @@ -61,7 +61,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) { - if (!WARN_ON(!vma->iova)) + if (!GEM_WARN_ON(!vma->iova)) vma->inuse--; }
@@ -73,7 +73,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, unsigned size = npages << PAGE_SHIFT; int ret = 0;
- if (WARN_ON(!vma->iova)) + if (GEM_WARN_ON(!vma->iova)) return -EINVAL;
/* Increase the usage counter */ @@ -100,7 +100,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, void msm_gem_close_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) { - if (WARN_ON(vma->inuse > 0 || vma->mapped)) + if (GEM_WARN_ON(vma->inuse > 0 || vma->mapped)) return;
spin_lock(&aspace->lock); @@ -120,7 +120,7 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, { int ret;
- if (WARN_ON(vma->iova)) + if (GEM_WARN_ON(vma->iova)) return -EBUSY;
spin_lock(&aspace->lock);
On 12/04/2022 00:58, Rob Clark wrote:
From: Rob Clark robdclark@chromium.org
Signed-off-by: Rob Clark robdclark@chromium.org
Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org
drivers/gpu/drm/msm/msm_gem_vma.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index f914ddbaea89..64906594fc65 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -44,7 +44,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, unsigned size = vma->node.size << PAGE_SHIFT;
/* Print a message if we try to purge a vma in use */
- if (WARN_ON(vma->inuse > 0))
if (GEM_WARN_ON(vma->inuse > 0)) return;
/* Don't do anything if the memory isn't mapped */
@@ -61,7 +61,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) {
- if (!WARN_ON(!vma->iova))
- if (!GEM_WARN_ON(!vma->iova)) vma->inuse--; }
@@ -73,7 +73,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, unsigned size = npages << PAGE_SHIFT; int ret = 0;
- if (WARN_ON(!vma->iova))
if (GEM_WARN_ON(!vma->iova)) return -EINVAL;
/* Increase the usage counter */
@@ -100,7 +100,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, void msm_gem_close_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) {
- if (WARN_ON(vma->inuse > 0 || vma->mapped))
if (GEM_WARN_ON(vma->inuse > 0 || vma->mapped)) return;
spin_lock(&aspace->lock);
@@ -120,7 +120,7 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, { int ret;
- if (WARN_ON(vma->iova))
if (GEM_WARN_ON(vma->iova)) return -EBUSY;
spin_lock(&aspace->lock);
From: Rob Clark robdclark@chromium.org
Prep for a following patch, where it gets a bit more complicated.
Signed-off-by: Rob Clark robdclark@chromium.org --- drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/msm/msm_gem.h | 1 + drivers/gpu/drm/msm/msm_gem_vma.c | 9 +++++++-- 3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index a4f61972667b..f96d1dc72021 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -938,7 +938,7 @@ void msm_gem_describe(struct drm_gem_object *obj, struct seq_file *m, name, comm ? ":" : "", comm ? comm : "", vma->aspace, vma->iova, vma->mapped ? "mapped" : "unmapped", - vma->inuse); + msm_gem_vma_inuse(vma)); kfree(comm); }
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 947ff7d9b471..1b7f0f0b88bf 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -61,6 +61,7 @@ struct msm_gem_vma { int msm_gem_init_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int npages, u64 range_start, u64 range_end); +bool msm_gem_vma_inuse(struct msm_gem_vma *vma); void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma); void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index 64906594fc65..dc2ae097805e 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -37,6 +37,11 @@ msm_gem_address_space_get(struct msm_gem_address_space *aspace) return aspace; }
+bool msm_gem_vma_inuse(struct msm_gem_vma *vma) +{ + return !!vma->inuse; +} + /* Actually unmap memory for the vma */ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) @@ -44,7 +49,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, unsigned size = vma->node.size << PAGE_SHIFT;
/* Print a message if we try to purge a vma in use */ - if (GEM_WARN_ON(vma->inuse > 0)) + if (GEM_WARN_ON(msm_gem_vma_inuse(vma))) return;
/* Don't do anything if the memory isn't mapped */ @@ -100,7 +105,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, void msm_gem_close_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) { - if (GEM_WARN_ON(vma->inuse > 0 || vma->mapped)) + if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped)) return;
spin_lock(&aspace->lock);
On 12/04/2022 00:58, Rob Clark wrote:
From: Rob Clark robdclark@chromium.org
Prep for a following patch, where it gets a bit more complicated.
Signed-off-by: Rob Clark robdclark@chromium.org
Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org
drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/msm/msm_gem.h | 1 + drivers/gpu/drm/msm/msm_gem_vma.c | 9 +++++++-- 3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index a4f61972667b..f96d1dc72021 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -938,7 +938,7 @@ void msm_gem_describe(struct drm_gem_object *obj, struct seq_file *m, name, comm ? ":" : "", comm ? comm : "", vma->aspace, vma->iova, vma->mapped ? "mapped" : "unmapped",
vma->inuse);
msm_gem_vma_inuse(vma)); kfree(comm);diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 947ff7d9b471..1b7f0f0b88bf 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -61,6 +61,7 @@ struct msm_gem_vma { int msm_gem_init_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int npages, u64 range_start, u64 range_end); +bool msm_gem_vma_inuse(struct msm_gem_vma *vma); void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma); void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index 64906594fc65..dc2ae097805e 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -37,6 +37,11 @@ msm_gem_address_space_get(struct msm_gem_address_space *aspace) return aspace; }
+bool msm_gem_vma_inuse(struct msm_gem_vma *vma) +{
- return !!vma->inuse;
+}
- /* Actually unmap memory for the vma */ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma)
@@ -44,7 +49,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, unsigned size = vma->node.size << PAGE_SHIFT;
/* Print a message if we try to purge a vma in use */
- if (GEM_WARN_ON(vma->inuse > 0))
if (GEM_WARN_ON(msm_gem_vma_inuse(vma))) return;
/* Don't do anything if the memory isn't mapped */
@@ -100,7 +105,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, void msm_gem_close_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) {
- if (GEM_WARN_ON(vma->inuse > 0 || vma->mapped))
if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped)) return;
spin_lock(&aspace->lock);
From: Rob Clark robdclark@chromium.org
Get rid of all the unnecessary conversion between address/size and page offsets. It just confuses things.
Signed-off-by: Rob Clark robdclark@chromium.org Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- drivers/gpu/drm/msm/msm_gem.c | 5 ++--- drivers/gpu/drm/msm/msm_gem.h | 4 ++-- drivers/gpu/drm/msm/msm_gem_vma.c | 16 ++++++++-------- 4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 3e325e2a2b1b..9f76f5b15759 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -1172,7 +1172,7 @@ static int a6xx_gmu_memory_alloc(struct a6xx_gmu *gmu, struct a6xx_gmu_bo *bo, return PTR_ERR(bo->obj);
ret = msm_gem_get_and_pin_iova_range(bo->obj, gmu->aspace, &bo->iova, - range_start >> PAGE_SHIFT, range_end >> PAGE_SHIFT); + range_start, range_end); if (ret) { drm_gem_object_put(bo->obj); return ret; diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index f96d1dc72021..f4b68bb28a4d 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -392,7 +392,7 @@ static int get_iova_locked(struct drm_gem_object *obj, if (IS_ERR(vma)) return PTR_ERR(vma);
- ret = msm_gem_init_vma(aspace, vma, obj->size >> PAGE_SHIFT, + ret = msm_gem_init_vma(aspace, vma, obj->size, range_start, range_end); if (ret) { del_vma(vma); @@ -434,8 +434,7 @@ static int msm_gem_pin_iova(struct drm_gem_object *obj, if (IS_ERR(pages)) return PTR_ERR(pages);
- ret = msm_gem_map_vma(aspace, vma, prot, - msm_obj->sgt, obj->size >> PAGE_SHIFT); + ret = msm_gem_map_vma(aspace, vma, prot, msm_obj->sgt, obj->size);
if (!ret) msm_obj->pin_count++; diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 1b7f0f0b88bf..090c3b1a6d9a 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -59,7 +59,7 @@ struct msm_gem_vma { };
int msm_gem_init_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma, int npages, + struct msm_gem_vma *vma, int size, u64 range_start, u64 range_end); bool msm_gem_vma_inuse(struct msm_gem_vma *vma); void msm_gem_purge_vma(struct msm_gem_address_space *aspace, @@ -68,7 +68,7 @@ void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma); int msm_gem_map_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int prot, - struct sg_table *sgt, int npages); + struct sg_table *sgt, int size); void msm_gem_close_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma);
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index dc2ae097805e..4949899f1fc7 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -46,7 +46,7 @@ bool msm_gem_vma_inuse(struct msm_gem_vma *vma) void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) { - unsigned size = vma->node.size << PAGE_SHIFT; + unsigned size = vma->node.size;
/* Print a message if we try to purge a vma in use */ if (GEM_WARN_ON(msm_gem_vma_inuse(vma))) @@ -73,9 +73,8 @@ void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, int msm_gem_map_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int prot, - struct sg_table *sgt, int npages) + struct sg_table *sgt, int size) { - unsigned size = npages << PAGE_SHIFT; int ret = 0;
if (GEM_WARN_ON(!vma->iova)) @@ -120,7 +119,7 @@ void msm_gem_close_vma(struct msm_gem_address_space *aspace,
/* Initialize a new vma and allocate an iova for it */ int msm_gem_init_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma, int npages, + struct msm_gem_vma *vma, int size, u64 range_start, u64 range_end) { int ret; @@ -129,14 +128,15 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, return -EBUSY;
spin_lock(&aspace->lock); - ret = drm_mm_insert_node_in_range(&aspace->mm, &vma->node, npages, 0, - 0, range_start, range_end, 0); + ret = drm_mm_insert_node_in_range(&aspace->mm, &vma->node, + size, PAGE_SIZE, 0, + range_start, range_end, 0); spin_unlock(&aspace->lock);
if (ret) return ret;
- vma->iova = vma->node.start << PAGE_SHIFT; + vma->iova = vma->node.start; vma->mapped = false;
kref_get(&aspace->kref); @@ -161,7 +161,7 @@ msm_gem_address_space_create(struct msm_mmu *mmu, const char *name, aspace->name = name; aspace->mmu = mmu;
- drm_mm_init(&aspace->mm, va_start >> PAGE_SHIFT, size >> PAGE_SHIFT); + drm_mm_init(&aspace->mm, va_start, size);
kref_init(&aspace->kref);
From: Rob Clark robdclark@chromium.org
There was only a single user, which could just as easily stash the iova when pinning.
v2: fix prepare->prepare->cleanup->cleanup sequences
Signed-off-by: Rob Clark robdclark@chromium.org Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- drivers/gpu/drm/msm/msm_fb.c | 20 ++++++++++++++------ drivers/gpu/drm/msm/msm_gem.c | 16 ---------------- drivers/gpu/drm/msm/msm_gem.h | 2 -- 3 files changed, 14 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c index 7137492fe78e..362775ae50af 100644 --- a/drivers/gpu/drm/msm/msm_fb.c +++ b/drivers/gpu/drm/msm/msm_fb.c @@ -21,6 +21,10 @@ struct msm_framebuffer {
/* Count of # of attached planes which need dirtyfb: */ refcount_t dirtyfb; + + /* Framebuffer per-plane address, if pinned, else zero: */ + uint64_t iova[DRM_FORMAT_MAX_PLANES]; + atomic_t prepare_count; }; #define to_msm_framebuffer(x) container_of(x, struct msm_framebuffer, base)
@@ -76,14 +80,16 @@ int msm_framebuffer_prepare(struct drm_framebuffer *fb, { struct msm_framebuffer *msm_fb = to_msm_framebuffer(fb); int ret, i, n = fb->format->num_planes; - uint64_t iova;
if (needs_dirtyfb) refcount_inc(&msm_fb->dirtyfb);
+ atomic_inc(&msm_fb->prepare_count); + for (i = 0; i < n; i++) { - ret = msm_gem_get_and_pin_iova(fb->obj[i], aspace, &iova); - drm_dbg_state(fb->dev, "FB[%u]: iova[%d]: %08llx (%d)", fb->base.id, i, iova, ret); + ret = msm_gem_get_and_pin_iova(fb->obj[i], aspace, &msm_fb->iova[i]); + drm_dbg_state(fb->dev, "FB[%u]: iova[%d]: %08llx (%d)", + fb->base.id, i, msm_fb->iova[i], ret); if (ret) return ret; } @@ -103,14 +109,16 @@ void msm_framebuffer_cleanup(struct drm_framebuffer *fb,
for (i = 0; i < n; i++) msm_gem_unpin_iova(fb->obj[i], aspace); + + if (!atomic_dec_return(&msm_fb->prepare_count)) + memset(msm_fb->iova, 0, sizeof(msm_fb->iova)); }
uint32_t msm_framebuffer_iova(struct drm_framebuffer *fb, struct msm_gem_address_space *aspace, int plane) { - if (!fb->obj[plane]) - return 0; - return msm_gem_iova(fb->obj[plane], aspace) + fb->offsets[plane]; + struct msm_framebuffer *msm_fb = to_msm_framebuffer(fb); + return msm_fb->iova[plane]; }
struct drm_gem_object *msm_framebuffer_bo(struct drm_framebuffer *fb, int plane) diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index f4b68bb28a4d..deafae6feaa8 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -509,22 +509,6 @@ int msm_gem_get_iova(struct drm_gem_object *obj, return ret; }
-/* get iova without taking a reference, used in places where you have - * already done a 'msm_gem_get_and_pin_iova' or 'msm_gem_get_iova' - */ -uint64_t msm_gem_iova(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace) -{ - struct msm_gem_vma *vma; - - msm_gem_lock(obj); - vma = lookup_vma(obj, aspace); - msm_gem_unlock(obj); - GEM_WARN_ON(!vma); - - return vma ? vma->iova : 0; -} - /* * Locked variant of msm_gem_unpin_iova() */ diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 090c3b1a6d9a..772de010a669 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -142,8 +142,6 @@ int msm_gem_get_and_pin_iova_locked(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova); int msm_gem_get_and_pin_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova); -uint64_t msm_gem_iova(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace); void msm_gem_unpin_iova_locked(struct drm_gem_object *obj, struct msm_gem_address_space *aspace); void msm_gem_unpin_iova(struct drm_gem_object *obj,
From: Rob Clark robdclark@chromium.org
Combines duplicate vma lookup in the get_and_pin path.
Signed-off-by: Rob Clark robdclark@chromium.org Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/gpu/drm/msm/msm_gem.c | 50 ++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index deafae6feaa8..218744a490a4 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -376,39 +376,40 @@ put_iova_vmas(struct drm_gem_object *obj) } }
-static int get_iova_locked(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace, uint64_t *iova, +static struct msm_gem_vma *get_vma_locked(struct drm_gem_object *obj, + struct msm_gem_address_space *aspace, u64 range_start, u64 range_end) { struct msm_gem_vma *vma; - int ret = 0;
GEM_WARN_ON(!msm_gem_is_locked(obj));
vma = lookup_vma(obj, aspace);
if (!vma) { + int ret; + vma = add_vma(obj, aspace); if (IS_ERR(vma)) - return PTR_ERR(vma); + return vma;
ret = msm_gem_init_vma(aspace, vma, obj->size, range_start, range_end); if (ret) { del_vma(vma); - return ret; + return ERR_PTR(ret); } + } else { + GEM_WARN_ON(vma->iova < range_start); + GEM_WARN_ON((vma->iova + obj->size) > range_end); }
- *iova = vma->iova; - return 0; + return vma; }
-static int msm_gem_pin_iova(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace) +static int msm_gem_pin_iova(struct drm_gem_object *obj, struct msm_gem_vma *vma) { struct msm_gem_object *msm_obj = to_msm_bo(obj); - struct msm_gem_vma *vma; struct page **pages; int ret, prot = IOMMU_READ;
@@ -426,15 +427,11 @@ static int msm_gem_pin_iova(struct drm_gem_object *obj, if (GEM_WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED)) return -EBUSY;
- vma = lookup_vma(obj, aspace); - if (GEM_WARN_ON(!vma)) - return -EINVAL; - pages = get_pages(obj); if (IS_ERR(pages)) return PTR_ERR(pages);
- ret = msm_gem_map_vma(aspace, vma, prot, msm_obj->sgt, obj->size); + ret = msm_gem_map_vma(vma->aspace, vma, prot, msm_obj->sgt, obj->size);
if (!ret) msm_obj->pin_count++; @@ -446,19 +443,18 @@ static int get_and_pin_iova_range_locked(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova, u64 range_start, u64 range_end) { - u64 local; + struct msm_gem_vma *vma; int ret;
GEM_WARN_ON(!msm_gem_is_locked(obj));
- ret = get_iova_locked(obj, aspace, &local, - range_start, range_end); - - if (!ret) - ret = msm_gem_pin_iova(obj, aspace); + vma = get_vma_locked(obj, aspace, range_start, range_end); + if (IS_ERR(vma)) + return PTR_ERR(vma);
+ ret = msm_gem_pin_iova(obj, vma); if (!ret) - *iova = local; + *iova = vma->iova;
return ret; } @@ -500,10 +496,16 @@ int msm_gem_get_and_pin_iova(struct drm_gem_object *obj, int msm_gem_get_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova) { - int ret; + struct msm_gem_vma *vma; + int ret = 0;
msm_gem_lock(obj); - ret = get_iova_locked(obj, aspace, iova, 0, U64_MAX); + vma = get_vma_locked(obj, aspace, 0, U64_MAX); + if (IS_ERR(vma)) { + ret = PTR_ERR(vma); + } else { + *iova = vma->iova; + } msm_gem_unlock(obj);
return ret;
From: Rob Clark robdclark@chromium.org
This way we only lookup vma once per object per submit, for both the submit and retire path.
Signed-off-by: Rob Clark robdclark@chromium.org --- drivers/gpu/drm/msm/msm_gem.c | 60 +++++++++++++--------------- drivers/gpu/drm/msm/msm_gem.h | 9 +++-- drivers/gpu/drm/msm/msm_gem_submit.c | 17 +++++--- 3 files changed, 44 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 218744a490a4..e8107a22c33a 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -407,7 +407,7 @@ static struct msm_gem_vma *get_vma_locked(struct drm_gem_object *obj, return vma; }
-static int msm_gem_pin_iova(struct drm_gem_object *obj, struct msm_gem_vma *vma) +int msm_gem_pin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vma) { struct msm_gem_object *msm_obj = to_msm_bo(obj); struct page **pages; @@ -439,6 +439,26 @@ static int msm_gem_pin_iova(struct drm_gem_object *obj, struct msm_gem_vma *vma) return ret; }
+void msm_gem_unpin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vma) +{ + struct msm_gem_object *msm_obj = to_msm_bo(obj); + + GEM_WARN_ON(!msm_gem_is_locked(obj)); + + msm_gem_unmap_vma(vma->aspace, vma); + + msm_obj->pin_count--; + GEM_WARN_ON(msm_obj->pin_count < 0); + + update_inactive(msm_obj); +} + +struct msm_gem_vma *msm_gem_get_vma_locked(struct drm_gem_object *obj, + struct msm_gem_address_space *aspace) +{ + return get_vma_locked(obj, aspace, 0, U64_MAX); +} + static int get_and_pin_iova_range_locked(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova, u64 range_start, u64 range_end) @@ -452,7 +472,7 @@ static int get_and_pin_iova_range_locked(struct drm_gem_object *obj, if (IS_ERR(vma)) return PTR_ERR(vma);
- ret = msm_gem_pin_iova(obj, vma); + ret = msm_gem_pin_vma_locked(obj, vma); if (!ret) *iova = vma->iova;
@@ -476,12 +496,6 @@ int msm_gem_get_and_pin_iova_range(struct drm_gem_object *obj, return ret; }
-int msm_gem_get_and_pin_iova_locked(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace, uint64_t *iova) -{ - return get_and_pin_iova_range_locked(obj, aspace, iova, 0, U64_MAX); -} - /* get iova and pin it. Should have a matching put */ int msm_gem_get_and_pin_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova) @@ -511,29 +525,6 @@ int msm_gem_get_iova(struct drm_gem_object *obj, return ret; }
-/* - * Locked variant of msm_gem_unpin_iova() - */ -void msm_gem_unpin_iova_locked(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace) -{ - struct msm_gem_object *msm_obj = to_msm_bo(obj); - struct msm_gem_vma *vma; - - GEM_WARN_ON(!msm_gem_is_locked(obj)); - - vma = lookup_vma(obj, aspace); - - if (!GEM_WARN_ON(!vma)) { - msm_gem_unmap_vma(aspace, vma); - - msm_obj->pin_count--; - GEM_WARN_ON(msm_obj->pin_count < 0); - - update_inactive(msm_obj); - } -} - /* * Unpin a iova by updating the reference counts. The memory isn't actually * purged until something else (shrinker, mm_notifier, destroy, etc) decides @@ -542,8 +533,13 @@ void msm_gem_unpin_iova_locked(struct drm_gem_object *obj, void msm_gem_unpin_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace) { + struct msm_gem_vma *vma; + msm_gem_lock(obj); - msm_gem_unpin_iova_locked(obj, aspace); + vma = lookup_vma(obj, aspace); + if (!GEM_WARN_ON(!vma)) { + msm_gem_unpin_vma_locked(obj, vma); + } msm_gem_unlock(obj); }
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 772de010a669..f98264cf130d 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -133,17 +133,17 @@ struct msm_gem_object { #define to_msm_bo(x) container_of(x, struct msm_gem_object, base)
uint64_t msm_gem_mmap_offset(struct drm_gem_object *obj); +int msm_gem_pin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vma); +void msm_gem_unpin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vma); +struct msm_gem_vma *msm_gem_get_vma_locked(struct drm_gem_object *obj, + struct msm_gem_address_space *aspace); int msm_gem_get_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova); int msm_gem_get_and_pin_iova_range(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova, u64 range_start, u64 range_end); -int msm_gem_get_and_pin_iova_locked(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace, uint64_t *iova); int msm_gem_get_and_pin_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova); -void msm_gem_unpin_iova_locked(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace); void msm_gem_unpin_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace); struct page **msm_gem_get_pages(struct drm_gem_object *obj); @@ -369,6 +369,7 @@ struct msm_gem_submit { uint32_t handle; }; uint64_t iova; + struct msm_gem_vma *vma; } bos[]; };
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index c6d60c8d286d..91da05af40ee 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -232,7 +232,7 @@ static void submit_cleanup_bo(struct msm_gem_submit *submit, int i, unsigned flags = submit->bos[i].flags & cleanup_flags;
if (flags & BO_PINNED) - msm_gem_unpin_iova_locked(obj, submit->aspace); + msm_gem_unpin_vma_locked(obj, submit->bos[i].vma);
if (flags & BO_ACTIVE) msm_gem_active_put(obj); @@ -365,21 +365,26 @@ static int submit_pin_objects(struct msm_gem_submit *submit)
for (i = 0; i < submit->nr_bos; i++) { struct drm_gem_object *obj = &submit->bos[i].obj->base; - uint64_t iova; + struct msm_gem_vma *vma;
/* if locking succeeded, pin bo: */ - ret = msm_gem_get_and_pin_iova_locked(obj, - submit->aspace, &iova); + vma = msm_gem_get_vma_locked(obj, submit->aspace); + if (IS_ERR(vma)) { + ret = PTR_ERR(vma); + break; + }
+ ret = msm_gem_pin_vma_locked(obj, vma); if (ret) break;
submit->bos[i].flags |= BO_PINNED; + submit->bos[i].vma = vma;
- if (iova == submit->bos[i].iova) { + if (vma->iova == submit->bos[i].iova) { submit->bos[i].flags |= BO_VALID; } else { - submit->bos[i].iova = iova; + submit->bos[i].iova = vma->iova; /* iova changed, so address in cmdstream is not valid: */ submit->bos[i].flags &= ~BO_VALID; submit->valid = false;
From: Rob Clark robdclark@chromium.org
With userspace allocated iova (next patch), we can have a race condition where userspace observes the fence completion and deletes the vma before retire_submit() gets around to unpinning the vma. To handle this, add a fenced unpin which drops the refcount but tracks the fence, and update msm_gem_vma_inuse() to check any previously unsignaled fences.
v2: Fix inuse underflow (duplicate unpin) v3: Fix msm_job_run() vs submit_cleanup() race condition
Signed-off-by: Rob Clark robdclark@chromium.org --- drivers/gpu/drm/msm/msm_fence.c | 6 ++++-- drivers/gpu/drm/msm/msm_fence.h | 3 +++ drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/msm/msm_gem.h | 14 +++++++++++-- drivers/gpu/drm/msm/msm_gem_submit.c | 15 +++++++------- drivers/gpu/drm/msm/msm_gem_vma.c | 30 +++++++++++++++++++++++++--- drivers/gpu/drm/msm/msm_ringbuffer.c | 13 +++++++++++- 7 files changed, 66 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c index f2cece542c3f..3df255402a33 100644 --- a/drivers/gpu/drm/msm/msm_fence.c +++ b/drivers/gpu/drm/msm/msm_fence.c @@ -15,6 +15,7 @@ msm_fence_context_alloc(struct drm_device *dev, volatile uint32_t *fenceptr, const char *name) { struct msm_fence_context *fctx; + static int index = 0;
fctx = kzalloc(sizeof(*fctx), GFP_KERNEL); if (!fctx) @@ -23,6 +24,7 @@ msm_fence_context_alloc(struct drm_device *dev, volatile uint32_t *fenceptr, fctx->dev = dev; strncpy(fctx->name, name, sizeof(fctx->name)); fctx->context = dma_fence_context_alloc(1); + fctx->index = index++; fctx->fenceptr = fenceptr; spin_lock_init(&fctx->spinlock);
@@ -34,7 +36,7 @@ void msm_fence_context_free(struct msm_fence_context *fctx) kfree(fctx); }
-static inline bool fence_completed(struct msm_fence_context *fctx, uint32_t fence) +bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence) { /* * Note: Check completed_fence first, as fenceptr is in a write-combine @@ -76,7 +78,7 @@ static const char *msm_fence_get_timeline_name(struct dma_fence *fence) static bool msm_fence_signaled(struct dma_fence *fence) { struct msm_fence *f = to_msm_fence(fence); - return fence_completed(f->fctx, f->base.seqno); + return msm_fence_completed(f->fctx, f->base.seqno); }
static const struct dma_fence_ops msm_fence_ops = { diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h index 17ee3822b423..7f1798c54cd1 100644 --- a/drivers/gpu/drm/msm/msm_fence.h +++ b/drivers/gpu/drm/msm/msm_fence.h @@ -21,6 +21,8 @@ struct msm_fence_context { char name[32]; /** context: see dma_fence_context_alloc() */ unsigned context; + /** index: similar to context, but local to msm_fence_context's */ + unsigned index;
/** * last_fence: @@ -56,6 +58,7 @@ struct msm_fence_context * msm_fence_context_alloc(struct drm_device *dev, volatile uint32_t *fenceptr, const char *name); void msm_fence_context_free(struct msm_fence_context *fctx);
+bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence); void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence);
struct dma_fence * msm_fence_alloc(struct msm_fence_context *fctx); diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index e8107a22c33a..bf4af17e2f1e 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -445,7 +445,7 @@ void msm_gem_unpin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vm
GEM_WARN_ON(!msm_gem_is_locked(obj));
- msm_gem_unmap_vma(vma->aspace, vma); + msm_gem_unpin_vma(vma);
msm_obj->pin_count--; GEM_WARN_ON(msm_obj->pin_count < 0); diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index f98264cf130d..580b6eb95edd 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -49,6 +49,8 @@ struct msm_gem_address_space * msm_gem_address_space_create(struct msm_mmu *mmu, const char *name, u64 va_start, u64 size);
+struct msm_fence_context; + struct msm_gem_vma { struct drm_mm_node node; uint64_t iova; @@ -56,6 +58,9 @@ struct msm_gem_vma { struct list_head list; /* node in msm_gem_object::vmas */ bool mapped; int inuse; + uint32_t fence_mask; + uint32_t fence[MSM_GPU_MAX_RINGS]; + struct msm_fence_context *fctx[MSM_GPU_MAX_RINGS]; };
int msm_gem_init_vma(struct msm_gem_address_space *aspace, @@ -64,8 +69,8 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, bool msm_gem_vma_inuse(struct msm_gem_vma *vma); void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma); -void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma); +void msm_gem_unpin_vma(struct msm_gem_vma *vma); +void msm_gem_unpin_vma_fenced(struct msm_gem_vma *vma, struct msm_fence_context *fctx); int msm_gem_map_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int prot, struct sg_table *sgt, int size); @@ -363,6 +368,11 @@ struct msm_gem_submit { struct drm_msm_gem_submit_reloc *relocs; } *cmd; /* array of size nr_cmds */ struct { +/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */ +#define BO_VALID 0x8000 /* is current addr in cmdstream correct/valid? */ +#define BO_LOCKED 0x4000 /* obj lock is held */ +#define BO_ACTIVE 0x2000 /* active refcnt is held */ +#define BO_PINNED 0x1000 /* obj is pinned and on active list */ uint32_t flags; union { struct msm_gem_object *obj; diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 91da05af40ee..23b68bc945f6 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -21,12 +21,6 @@ * Cmdstream submission: */
-/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */ -#define BO_VALID 0x8000 /* is current addr in cmdstream correct/valid? */ -#define BO_LOCKED 0x4000 /* obj lock is held */ -#define BO_ACTIVE 0x2000 /* active refcnt is held */ -#define BO_PINNED 0x1000 /* obj is pinned and on active list */ - static struct msm_gem_submit *submit_create(struct drm_device *dev, struct msm_gpu *gpu, struct msm_gpu_submitqueue *queue, uint32_t nr_bos, @@ -231,6 +225,13 @@ static void submit_cleanup_bo(struct msm_gem_submit *submit, int i, struct drm_gem_object *obj = &submit->bos[i].obj->base; unsigned flags = submit->bos[i].flags & cleanup_flags;
+ /* + * Clear flags bit before dropping lock, so that the msm_job_run() + * path isn't racing with submit_cleanup() (ie. the read/modify/ + * write is protected by the obj lock in all paths) + */ + submit->bos[i].flags &= ~cleanup_flags; + if (flags & BO_PINNED) msm_gem_unpin_vma_locked(obj, submit->bos[i].vma);
@@ -239,8 +240,6 @@ static void submit_cleanup_bo(struct msm_gem_submit *submit, int i,
if (flags & BO_LOCKED) dma_resv_unlock(obj->resv); - - submit->bos[i].flags &= ~cleanup_flags; }
static void submit_unlock_unpin_bo(struct msm_gem_submit *submit, int i) diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index 4949899f1fc7..0cd6770faf41 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -5,6 +5,7 @@ */
#include "msm_drv.h" +#include "msm_fence.h" #include "msm_gem.h" #include "msm_mmu.h"
@@ -39,7 +40,19 @@ msm_gem_address_space_get(struct msm_gem_address_space *aspace)
bool msm_gem_vma_inuse(struct msm_gem_vma *vma) { - return !!vma->inuse; + if (vma->inuse > 0) + return true; + + while (vma->fence_mask) { + unsigned idx = ffs(vma->fence_mask) - 1; + + if (!msm_fence_completed(vma->fctx[idx], vma->fence[idx])) + return true; + + vma->fence_mask &= ~BIT(idx); + } + + return false; }
/* Actually unmap memory for the vma */ @@ -63,13 +76,24 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, }
/* Remove reference counts for the mapping */ -void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, - struct msm_gem_vma *vma) +void msm_gem_unpin_vma(struct msm_gem_vma *vma) { + if (GEM_WARN_ON(!vma->inuse)) + return; if (!GEM_WARN_ON(!vma->iova)) vma->inuse--; }
+/* Replace pin reference with fence: */ +void msm_gem_unpin_vma_fenced(struct msm_gem_vma *vma, struct msm_fence_context *fctx) +{ + vma->fctx[fctx->index] = fctx; + vma->fence[fctx->index] = fctx->last_fence; + vma->fence_mask |= BIT(fctx->index); + msm_gem_unpin_vma(vma); +} + +/* Map and pin vma: */ int msm_gem_map_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int prot, diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c index 367a6aaa3a20..eb67fce5bc62 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.c +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c @@ -14,9 +14,20 @@ module_param(num_hw_submissions, uint, 0600); static struct dma_fence *msm_job_run(struct drm_sched_job *job) { struct msm_gem_submit *submit = to_msm_submit(job); + struct msm_fence_context *fctx = submit->ring->fctx; struct msm_gpu *gpu = submit->gpu; + int i;
- submit->hw_fence = msm_fence_alloc(submit->ring->fctx); + submit->hw_fence = msm_fence_alloc(fctx); + + for (i = 0; i < submit->nr_bos; i++) { + struct drm_gem_object *obj = &submit->bos[i].obj->base; + + msm_gem_lock(obj); + msm_gem_unpin_vma_fenced(submit->bos[i].vma, fctx); + submit->bos[i].flags &= ~BO_PINNED; + msm_gem_unlock(obj); + }
pm_runtime_get_sync(&gpu->pdev->dev);
From: Rob Clark robdclark@chromium.org
The motivation at this point is mainly native userspace mesa driver in a VM guest. The one remaining synchronous "hotpath" is buffer allocation, because guest needs to wait to know the bo's iova before it can start emitting cmdstream/state that references the new bo. By allocating the iova in the guest userspace, we no longer need to wait for a response from the host, but can just rely on the allocation request being processed before the cmdstream submission. Allocation failures (OoM, etc) would just be treated as context-lost (ie. GL_GUILTY_CONTEXT_RESET) or subsequent allocations (or readpix, etc) can raise GL_OUT_OF_MEMORY.
v2: Fix inuse check v3: Change mismatched iova case to -EBUSY
Signed-off-by: Rob Clark robdclark@chromium.org Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 10 ++++++ drivers/gpu/drm/msm/msm_drv.c | 21 +++++++++++ drivers/gpu/drm/msm/msm_gem.c | 48 +++++++++++++++++++++++++ drivers/gpu/drm/msm/msm_gem.h | 8 +++++ drivers/gpu/drm/msm/msm_gem_vma.c | 2 ++ include/uapi/drm/msm_drm.h | 3 ++ 6 files changed, 92 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 6385ab06632f..4caae0229518 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -281,6 +281,16 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx, case MSM_PARAM_SUSPENDS: *value = gpu->suspend_count; return 0; + case MSM_PARAM_VA_START: + if (ctx->aspace == gpu->aspace) + return -EINVAL; + *value = ctx->aspace->va_start; + return 0; + case MSM_PARAM_VA_SIZE: + if (ctx->aspace == gpu->aspace) + return -EINVAL; + *value = ctx->aspace->va_size; + return 0; default: DBG("%s: invalid param: %u", gpu->name, param); return -EINVAL; diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index d618953d33ea..34e2169308b4 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -722,6 +722,23 @@ static int msm_ioctl_gem_info_iova(struct drm_device *dev, return msm_gem_get_iova(obj, ctx->aspace, iova); }
+static int msm_ioctl_gem_info_set_iova(struct drm_device *dev, + struct drm_file *file, struct drm_gem_object *obj, + uint64_t iova) +{ + struct msm_drm_private *priv = dev->dev_private; + struct msm_file_private *ctx = file->driver_priv; + + if (!priv->gpu) + return -EINVAL; + + /* Only supported if per-process address space is supported: */ + if (priv->gpu->aspace == ctx->aspace) + return -EOPNOTSUPP; + + return msm_gem_set_iova(obj, ctx->aspace, iova); +} + static int msm_ioctl_gem_info(struct drm_device *dev, void *data, struct drm_file *file) { @@ -736,6 +753,7 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, switch (args->info) { case MSM_INFO_GET_OFFSET: case MSM_INFO_GET_IOVA: + case MSM_INFO_SET_IOVA: /* value returned as immediate, not pointer, so len==0: */ if (args->len) return -EINVAL; @@ -760,6 +778,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, case MSM_INFO_GET_IOVA: ret = msm_ioctl_gem_info_iova(dev, file, obj, &args->value); break; + case MSM_INFO_SET_IOVA: + ret = msm_ioctl_gem_info_set_iova(dev, file, obj, args->value); + break; case MSM_INFO_SET_NAME: /* length check should leave room for terminating null: */ if (args->len >= sizeof(msm_obj->name)) { diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index bf4af17e2f1e..3ee30b8a76bd 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -525,6 +525,54 @@ int msm_gem_get_iova(struct drm_gem_object *obj, return ret; }
+static int clear_iova(struct drm_gem_object *obj, + struct msm_gem_address_space *aspace) +{ + struct msm_gem_vma *vma = lookup_vma(obj, aspace); + + if (!vma) + return 0; + + if (msm_gem_vma_inuse(vma)) + return -EBUSY; + + msm_gem_purge_vma(vma->aspace, vma); + msm_gem_close_vma(vma->aspace, vma); + del_vma(vma); + + return 0; +} + +/* + * Get the requested iova but don't pin it. Fails if the requested iova is + * not available. Doesn't need a put because iovas are currently valid for + * the life of the object. + * + * Setting an iova of zero will clear the vma. + */ +int msm_gem_set_iova(struct drm_gem_object *obj, + struct msm_gem_address_space *aspace, uint64_t iova) +{ + int ret = 0; + + msm_gem_lock(obj); + if (!iova) { + ret = clear_iova(obj, aspace); + } else { + struct msm_gem_vma *vma; + vma = get_vma_locked(obj, aspace, iova, iova + obj->size); + if (IS_ERR(vma)) { + ret = PTR_ERR(vma); + } else if (GEM_WARN_ON(vma->iova != iova)) { + clear_iova(obj, aspace); + ret = -EBUSY; + } + } + msm_gem_unlock(obj); + + return ret; +} + /* * Unpin a iova by updating the reference counts. The memory isn't actually * purged until something else (shrinker, mm_notifier, destroy, etc) decides diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 580b6eb95edd..c75d3b879a53 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -38,6 +38,12 @@ struct msm_gem_address_space {
/* @faults: the number of GPU hangs associated with this address space */ int faults; + + /** @va_start: lowest possible address to allocate */ + uint64_t va_start; + + /** @va_size: the size of the address space (in bytes) */ + uint64_t va_size; };
struct msm_gem_address_space * @@ -144,6 +150,8 @@ struct msm_gem_vma *msm_gem_get_vma_locked(struct drm_gem_object *obj, struct msm_gem_address_space *aspace); int msm_gem_get_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova); +int msm_gem_set_iova(struct drm_gem_object *obj, + struct msm_gem_address_space *aspace, uint64_t iova); int msm_gem_get_and_pin_iova_range(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova, u64 range_start, u64 range_end); diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index 0cd6770faf41..3c1dc9241831 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -184,6 +184,8 @@ msm_gem_address_space_create(struct msm_mmu *mmu, const char *name, spin_lock_init(&aspace->lock); aspace->name = name; aspace->mmu = mmu; + aspace->va_start = va_start; + aspace->va_size = size;
drm_mm_init(&aspace->mm, va_start, size);
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h index 794ad1948497..3c7b097c4e3d 100644 --- a/include/uapi/drm/msm_drm.h +++ b/include/uapi/drm/msm_drm.h @@ -84,6 +84,8 @@ struct drm_msm_timespec { #define MSM_PARAM_SYSPROF 0x0b /* WO: 1 preserves perfcntrs, 2 also disables suspend */ #define MSM_PARAM_COMM 0x0c /* WO: override for task->comm */ #define MSM_PARAM_CMDLINE 0x0d /* WO: override for task cmdline */ +#define MSM_PARAM_VA_START 0x0e /* RO: start of valid GPU iova range */ +#define MSM_PARAM_VA_SIZE 0x0f /* RO: size of valid GPU iova range (bytes) */
/* For backwards compat. The original support for preemption was based on * a single ring per priority level so # of priority levels equals the # @@ -135,6 +137,7 @@ struct drm_msm_gem_new { #define MSM_INFO_GET_IOVA 0x01 /* get iova, returned by value */ #define MSM_INFO_SET_NAME 0x02 /* set the debug name (by pointer) */ #define MSM_INFO_GET_NAME 0x03 /* get debug name, returned by pointer */ +#define MSM_INFO_SET_IOVA 0x04 /* set the iova, passed by value */
struct drm_msm_gem_info { __u32 handle; /* in */
dri-devel@lists.freedesktop.org
-
Dmitry Baryshkov -
Rob Clark