[PATCH 1/3] drm/virtio: set seqno for dma-fence

[Chia-I Wu]

(Add missing CCs)


On Mon, Apr 29, 2019 at 3:08 PM Chia-I Wu <olvaffe at gmail.com> wrote:
>
> This is motivated by having meaningful ftrace events, but it also
> fixes use cases where dma_fence_is_later is called, such as in
> sync_file_merge.
>
> In other drivers, fence creation and cmdbuf submission normally
> happen atomically,
>
>   mutex_lock();
>   fence = dma_fence_create(..., ++timeline->seqno);
>   submit_cmdbuf();
>   mutex_unlock();
>
> and have no such issue.  But in our driver, because most ioctls
> queue commands into ctrlq, we do not want to grab a lock.  Instead,
> we set seqno to 0 when a fence is created, and update it when the
> command is finally queued and the seqno is known.
>
> Signed-off-by: Chia-I Wu <olvaffe at gmail.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h   |  1 -
>  drivers/gpu/drm/virtio/virtgpu_fence.c | 17 ++++++++++-------
>  2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 491dec0712b3..90461feeafdb 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -102,7 +102,6 @@ struct virtio_gpu_fence {
>         struct dma_fence f;
>         struct virtio_gpu_fence_driver *drv;
>         struct list_head node;
> -       uint64_t seq;
>  };
>  #define to_virtio_fence(x) \
>         container_of(x, struct virtio_gpu_fence, f)
> diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c
> index 87d1966192f4..72b4f7561432 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_fence.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c
> @@ -40,16 +40,14 @@ bool virtio_fence_signaled(struct dma_fence *f)
>  {
>         struct virtio_gpu_fence *fence = to_virtio_fence(f);
>
> -       if (atomic64_read(&fence->drv->last_seq) >= fence->seq)
> +       if (atomic64_read(&fence->drv->last_seq) >= fence->f.seqno)
>                 return true;
>         return false;
>  }
>
>  static void virtio_fence_value_str(struct dma_fence *f, char *str, int size)
>  {
> -       struct virtio_gpu_fence *fence = to_virtio_fence(f);
> -
> -       snprintf(str, size, "%llu", fence->seq);
> +       snprintf(str, size, "%llu", f->seqno);
>  }
>
>  static void virtio_timeline_value_str(struct dma_fence *f, char *str, int size)
> @@ -76,6 +74,11 @@ struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev)
>                 return fence;
>
>         fence->drv = drv;
> +
> +       /* This only partially initializes the fence because the seqno is
> +        * unknown yet.  The fence must not be used outside of the driver
> +        * until virtio_gpu_fence_emit is called.
> +        */
>         dma_fence_init(&fence->f, &virtio_fence_ops, &drv->lock, drv->context, 0);
>
>         return fence;
> @@ -89,13 +92,13 @@ int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev,
>         unsigned long irq_flags;
>
>         spin_lock_irqsave(&drv->lock, irq_flags);
> -       fence->seq = ++drv->sync_seq;
> +       fence->f.seqno = ++drv->sync_seq;
>         dma_fence_get(&fence->f);
>         list_add_tail(&fence->node, &drv->fences);
>         spin_unlock_irqrestore(&drv->lock, irq_flags);
>
>         cmd_hdr->flags |= cpu_to_le32(VIRTIO_GPU_FLAG_FENCE);
> -       cmd_hdr->fence_id = cpu_to_le64(fence->seq);
> +       cmd_hdr->fence_id = cpu_to_le64(fence->f.seqno);
>         return 0;
>  }
>
> @@ -109,7 +112,7 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev,
>         spin_lock_irqsave(&drv->lock, irq_flags);
>         atomic64_set(&vgdev->fence_drv.last_seq, last_seq);
>         list_for_each_entry_safe(fence, tmp, &drv->fences, node) {
> -               if (last_seq < fence->seq)
> +               if (last_seq < fence->f.seqno)
>                         continue;
>                 dma_fence_signal_locked(&fence->f);
>                 list_del(&fence->node);
> --
> 2.21.0.593.g511ec345e18-goog
>