| Message ID | 20230613174306.1208-1-gurchetansingh@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/virtio: conditionally allocate virtio_gpu_fence | expand |
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@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.
On Mon, Jun 19, 2023 at 3:02 PM Dmitry Osipenko < dmitry.osipenko@collabora.com> wrote: > 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@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. > Added you r-b and fixed indent in v2. > > -- > Best regards, > Dmitry > >
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) {
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@chromium.org> --- drivers/gpu/drm/virtio/virtgpu_submit.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)