On Mon, Oct 01, 2018 at 06:01:33PM +0530, Sharat Masetty wrote: > Track the GPU fences created at submit time with idr instead of the ring > the sequence number. This helps with easily changing the underlying > fence to something we don't truly own, like the scheduler fence. > > Also move the GPU fence allocation to msm_gpu_submit() and have > the function return the fence. This suits well when integrating with the > GPU scheduler. > > Additionally remove the non-interruptible wait variant from msm_wait_fence() > as it is not used.
So basically this is just propping up the msm_wait_fence ioctl a bit more? At what point should we just deprecate that bad boy and move on with our lives? > Signed-off-by: Sharat Masetty <[email protected]> > --- > drivers/gpu/drm/msm/msm_drv.c | 3 +-- > drivers/gpu/drm/msm/msm_fence.c | 30 ++++++++++++++---------------- > drivers/gpu/drm/msm/msm_fence.h | 5 +++-- > drivers/gpu/drm/msm/msm_gem.h | 1 + > drivers/gpu/drm/msm/msm_gem_submit.c | 26 ++++++++++++++++++-------- > drivers/gpu/drm/msm/msm_gpu.c | 10 ++++++++-- > drivers/gpu/drm/msm/msm_gpu.h | 4 ++-- > drivers/gpu/drm/msm/msm_ringbuffer.c | 5 +++++ > drivers/gpu/drm/msm/msm_ringbuffer.h | 1 + > 9 files changed, 53 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index 021a0b6..8eaa1bd 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -746,8 +746,7 @@ static int msm_ioctl_wait_fence(struct drm_device *dev, > void *data, > if (!queue) > return -ENOENT; > > - ret = msm_wait_fence(gpu->rb[queue->prio]->fctx, args->fence, &timeout, > - true); > + ret = msm_wait_fence(gpu->rb[queue->prio], args->fence, &timeout); > > msm_submitqueue_put(queue); > return ret; > diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c > index 349c12f..0e7912b 100644 > --- a/drivers/gpu/drm/msm/msm_fence.c > +++ b/drivers/gpu/drm/msm/msm_fence.c > @@ -50,41 +50,39 @@ static inline bool fence_completed(struct > msm_fence_context *fctx, uint32_t fenc > } > > /* legacy path for WAIT_FENCE ioctl: */ > -int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence, > - ktime_t *timeout, bool interruptible) > +int msm_wait_fence(struct msm_ringbuffer *ring, uint32_t fence_id, > + ktime_t *timeout) > { > + struct dma_fence *fence; > int ret; > > - if (fence > fctx->last_fence) { > - DRM_ERROR("%s: waiting on invalid fence: %u (of %u)\n", > - fctx->name, fence, fctx->last_fence); > - return -EINVAL; > + rcu_read_lock(); > + fence = idr_find(&ring->fence_idr, fence_id); > + > + if (!fence || !dma_fence_get_rcu(fence)) { > + rcu_read_unlock(); > + return 0; > } > + rcu_read_unlock(); > > if (!timeout) { > /* no-wait: */ > - ret = fence_completed(fctx, fence) ? 0 : -EBUSY; > + ret = dma_fence_is_signaled(fence) ? 0 : -EBUSY; > } else { > unsigned long remaining_jiffies = timeout_to_jiffies(timeout); > > - if (interruptible) > - ret = wait_event_interruptible_timeout(fctx->event, > - fence_completed(fctx, fence), > - remaining_jiffies); > - else > - ret = wait_event_timeout(fctx->event, > - fence_completed(fctx, fence), > - remaining_jiffies); > + ret = dma_fence_wait_timeout(fence, true, remaining_jiffies); > > if (ret == 0) { > DBG("timeout waiting for fence: %u (completed: %u)", > - fence, fctx->completed_fence); > + fence_id, ring->memptrs->fence); > ret = -ETIMEDOUT; > } else if (ret != -ERESTARTSYS) { > ret = 0; > } > } > > + dma_fence_put(fence); > return ret; > } > > diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h > index b9fe059..a8133f7 100644 > --- a/drivers/gpu/drm/msm/msm_fence.h > +++ b/drivers/gpu/drm/msm/msm_fence.h > @@ -19,6 +19,7 @@ > #define __MSM_FENCE_H__ > > #include "msm_drv.h" > +#include "msm_ringbuffer.h" > > struct msm_fence_context { > struct drm_device *dev; > @@ -35,8 +36,8 @@ struct msm_fence_context * msm_fence_context_alloc(struct > drm_device *dev, > const char *name); > void msm_fence_context_free(struct msm_fence_context *fctx); > > -int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence, > - ktime_t *timeout, bool interruptible); > +int msm_wait_fence(struct msm_ringbuffer *ring, uint32_t fence_id, > + ktime_t *timeout); > 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.h b/drivers/gpu/drm/msm/msm_gem.h > index c5d9bd3..287f974 100644 > --- a/drivers/gpu/drm/msm/msm_gem.h > +++ b/drivers/gpu/drm/msm/msm_gem.h > @@ -143,6 +143,7 @@ struct msm_gem_submit { > struct ww_acquire_ctx ticket; > uint32_t seqno; /* Sequence number of the submit on the ring */ > struct dma_fence *fence; > + int out_fence_id; > struct msm_gpu_submitqueue *queue; > struct pid *pid; /* submitting process */ > bool valid; /* true if no cmdstream patching needed */ > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > b/drivers/gpu/drm/msm/msm_gem_submit.c > index 7bd83e0..00e6347 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -48,6 +48,7 @@ static struct msm_gem_submit *submit_create(struct > drm_device *dev, > submit->dev = dev; > submit->gpu = gpu; > submit->fence = NULL; > + submit->out_fence_id = -1; > submit->pid = get_pid(task_pid(current)); > submit->cmd = (void *)&submit->bos[nr_bos]; > submit->queue = queue; > @@ -66,6 +67,8 @@ static struct msm_gem_submit *submit_create(struct > drm_device *dev, > > void msm_gem_submit_free(struct msm_gem_submit *submit) > { > + idr_remove(&submit->ring->fence_idr, submit->out_fence_id); > + > dma_fence_put(submit->fence); > list_del(&submit->node); > put_pid(submit->pid); > @@ -557,26 +560,33 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > *data, > > submit->nr_cmds = i; > > - submit->fence = msm_fence_alloc(ring->fctx); > + msm_gpu_submit(gpu, submit, ctx); > if (IS_ERR(submit->fence)) { > ret = PTR_ERR(submit->fence); > submit->fence = NULL; > goto out; > } > > + /* > + * No protection should be okay here since this is protected by the big > + * GPU lock. > + */ > + submit->out_fence_id = idr_alloc_cyclic(&ring->fence_idr, submit->fence, > + 0, INT_MAX, GFP_KERNEL); > + > + if (submit->out_fence_id < 0) { > + ret = -ENOMEM; > + goto out; > + } > + > + args->fence = submit->out_fence_id; > + > if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) { > sync_file = sync_file_create(submit->fence); > if (!sync_file) { > ret = -ENOMEM; > goto out; > } > - } > - > - msm_gpu_submit(gpu, submit, ctx); > - > - args->fence = submit->fence->seqno; > - > - if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) { > fd_install(out_fence_fd, sync_file->file); > args->fence_fd = out_fence_fd; > } > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c > index 1c09acf..eb67172 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.c > +++ b/drivers/gpu/drm/msm/msm_gpu.c > @@ -602,8 +602,8 @@ void msm_gpu_retire(struct msm_gpu *gpu) > } > > /* add bo's to gpu's ring, and kick gpu: */ > -void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, > - struct msm_file_private *ctx) > +struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu, > + struct msm_gem_submit *submit, struct msm_file_private *ctx) > { > struct drm_device *dev = gpu->dev; > struct msm_drm_private *priv = dev->dev_private; > @@ -612,6 +612,10 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct > msm_gem_submit *submit, > > WARN_ON(!mutex_is_locked(&dev->struct_mutex)); > > + submit->fence = msm_fence_alloc(ring->fctx); > + if (IS_ERR(submit->fence)) > + return submit->fence; > + > pm_runtime_get_sync(&gpu->pdev->dev); > > msm_gpu_hw_init(gpu); > @@ -648,6 +652,8 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct > msm_gem_submit *submit, > priv->lastctx = ctx; > > hangcheck_timer_reset(gpu); > + > + return submit->fence; > } > > /* > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h > index b824117..b562b25 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.h > +++ b/drivers/gpu/drm/msm/msm_gpu.h > @@ -235,8 +235,8 @@ int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint32_t > *activetime, > uint32_t *totaltime, uint32_t ncntrs, uint32_t *cntrs); > > void msm_gpu_retire(struct msm_gpu *gpu); > -void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, > - struct msm_file_private *ctx); > +struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu, > + struct msm_gem_submit *submit, struct msm_file_private *ctx); > > int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, > struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs, > diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c > b/drivers/gpu/drm/msm/msm_ringbuffer.c > index 6f5295b..734f2b8 100644 > --- a/drivers/gpu/drm/msm/msm_ringbuffer.c > +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c > @@ -59,6 +59,8 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu > *gpu, int id, > > ring->fctx = msm_fence_context_alloc(gpu->dev, name); > > + idr_init(&ring->fence_idr); > + > return ring; > > fail: > @@ -78,5 +80,8 @@ void msm_ringbuffer_destroy(struct msm_ringbuffer *ring) > msm_gem_put_vaddr(ring->bo); > drm_gem_object_put_unlocked(ring->bo); > } > + > + idr_destroy(&ring->fence_idr); > + > kfree(ring); > } > diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h > b/drivers/gpu/drm/msm/msm_ringbuffer.h > index cffce09..b74a0a9 100644 > --- a/drivers/gpu/drm/msm/msm_ringbuffer.h > +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h > @@ -40,6 +40,7 @@ struct msm_ringbuffer { > struct msm_rbmemptrs *memptrs; > uint64_t memptrs_iova; > struct msm_fence_context *fctx; > + struct idr fence_idr; > spinlock_t lock; > }; > > -- > 1.9.1 > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
