[PATCH] drm/virtio: conditionally allocate virtio_gpu_fence

[Dmitry Osipenko]

On 6/13/23 20:43, Gurchetan Singh wrote:
> We don't want to create a fence for every command submission.  It's
> only necessary when userspace provides a waitable token for submission.
> This could be:
> 
> 1) bo_handles, to be used with VIRTGPU_WAIT
> 2) out_fence_fd, to be used with dma_fence apis
> 3) a ring_idx provided with VIRTGPU_CONTEXT_PARAM_POLL_RINGS_MASK
>    + DRM event API
> 4) syncobjs in the future
> 
> The use case for just submitting a command to the host, and expecting
> no response.  For example, gfxstream has GFXSTREAM_CONTEXT_PING that
> just wakes up the host side worker threads.  There's also
> CROSS_DOMAIN_CMD_SEND which just sends data to the Wayland server.
> 
> This prevents the need to signal the automatically created
> virtio_gpu_fence.
> 
> Signed-off-by: Gurchetan Singh <gurchetansingh at chromium.org>
> ---
>  drivers/gpu/drm/virtio/virtgpu_submit.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_submit.c b/drivers/gpu/drm/virtio/virtgpu_submit.c
> index cf3c04b16a7a..add106c06ab2 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_submit.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_submit.c
> @@ -168,9 +168,13 @@ static int virtio_gpu_init_submit(struct virtio_gpu_submit *submit,
>  
>  	memset(submit, 0, sizeof(*submit));
>  
> -	out_fence = virtio_gpu_fence_alloc(vgdev, fence_ctx, ring_idx);
> -	if (!out_fence)
> -		return -ENOMEM;
> +	if ((exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_OUT) ||
> +		((exbuf->flags & VIRTGPU_EXECBUF_RING_IDX) &&
> +		(vfpriv->ring_idx_mask & BIT_ULL(ring_idx))) ||
> +		exbuf->num_bo_handles)
> +		out_fence = virtio_gpu_fence_alloc(vgdev, fence_ctx, ring_idx);
> +	else
> +		out_fence = NULL;
>  
>  	err = virtio_gpu_fence_event_create(dev, file, out_fence, ring_idx);
>  	if (err) {

Looks okay, code indentation may be improved a tad to make it more eye-friendly:

+	if ((exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_OUT) ||
+	   ((exbuf->flags & VIRTGPU_EXECBUF_RING_IDX) && (vfpriv->ring_idx_mask & BIT_ULL(ring_idx))) ||
+	     exbuf->num_bo_handles)
+		out_fence = virtio_gpu_fence_alloc(vgdev, fence_ctx, ring_idx);
+	else
+		out_fence = NULL;

Checkpatch will complain about this variant, but the complaint can be ignored in this case.

-- 
Best regards,
Dmitry