Re: [PATCH v2 5/6] drm/virtio: drop fencing in virtio_gpu_resource_create_ioctl

From: Gurchetan Singh
Date: Thu Jan 31 2019 - 14:04:48 EST


On Wed, Jan 30, 2019 at 1:43 AM Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:
>
> There is no need to wait for completion here.
>
> The host will process commands in submit order, so commands can
> reference the new resource just fine even when queued up before
> completion.

Does virtio_gpu_execbuffer_ioctl also wait for completion for a host response?

>From the guest driver perspective, a fence is just implemented has a
virtio 3D resource.

https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c#L787

The DRM_IOCTL_VIRTGPU_WAIT ioctl essentially waits for the reservation
objects associated with that fence resource to become available. So
the flow is:

virtio_gpu_execbuffer_ioctl
virtio_gpu_resource_create_ioctl with fence resource
virtio_gpu_wait_ioctl with that fence resource --> associated with a
GL wait on the host side

Does this change modify this sequence of events?

>
> On the guest side there is no need to wait for completion too. Which
> btw is different from resource destroy, where we have to make sure the
> host has seen the destroy and thus doesn't use it any more before
> releasing the pages backing the resource.
>
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> ---
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 51 +---------------------------------
> 1 file changed, 1 insertion(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 431e5d767e..da06ebbb3a 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -279,10 +279,6 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
> struct virtio_gpu_object *qobj;
> struct drm_gem_object *obj;
> uint32_t handle = 0;
> - struct list_head validate_list;
> - struct ttm_validate_buffer mainbuf;
> - struct virtio_gpu_fence *fence = NULL;
> - struct ww_acquire_ctx ticket;
> struct virtio_gpu_object_params params = { 0 };
>
> if (vgdev->has_virgl_3d == false) {
> @@ -298,9 +294,6 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
> return -EINVAL;
> }
>
> - INIT_LIST_HEAD(&validate_list);
> - memset(&mainbuf, 0, sizeof(struct ttm_validate_buffer));
> -
> params.pinned = false,
> params.format = rc->format;
> params.width = rc->width;
> @@ -329,62 +322,20 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
>
> ret = virtio_gpu_object_attach(vgdev, qobj, NULL);
> } else {
> - /* use a gem reference since unref list undoes them */
> - drm_gem_object_get(&qobj->gem_base);
> - mainbuf.bo = &qobj->tbo;
> - list_add(&mainbuf.head, &validate_list);
> -
> - ret = virtio_gpu_object_list_validate(&ticket, &validate_list);
> - if (ret) {
> - DRM_DEBUG("failed to validate\n");
> - goto fail_unref;
> - }
> -
> - fence = virtio_gpu_fence_alloc(vgdev);
> - if (!fence) {
> - ret = -ENOMEM;
> - goto fail_backoff;
> - }
> -
> virtio_gpu_cmd_resource_create_3d(vgdev, qobj, &params);
> - ret = virtio_gpu_object_attach(vgdev, qobj, fence);
> - if (ret) {
> - dma_fence_put(&fence->f);
> - goto fail_backoff;
> - }
> - ttm_eu_fence_buffer_objects(&ticket, &validate_list, &fence->f);
> + ret = virtio_gpu_object_attach(vgdev, qobj, NULL);
> }
>
> ret = drm_gem_handle_create(file_priv, obj, &handle);
> if (ret) {
> -
> drm_gem_object_release(obj);
> - if (vgdev->has_virgl_3d) {
> - virtio_gpu_unref_list(&validate_list);
> - dma_fence_put(&fence->f);
> - }
> return ret;
> }
> drm_gem_object_put_unlocked(obj);
>
> rc->res_handle = qobj->hw_res_handle; /* similiar to a VM address */
> rc->bo_handle = handle;
> -
> - if (vgdev->has_virgl_3d) {
> - virtio_gpu_unref_list(&validate_list);
> - dma_fence_put(&fence->f);
> - }
> return 0;
> -fail_backoff:
> - ttm_eu_backoff_reservation(&ticket, &validate_list);
> -fail_unref:
> - if (vgdev->has_virgl_3d) {
> - virtio_gpu_unref_list(&validate_list);
> - dma_fence_put(&fence->f);
> - }
> -//fail_obj:
> -// drm_gem_object_handle_unreference_unlocked(obj);
> - return ret;
> }
>
> static int virtio_gpu_resource_info_ioctl(struct drm_device *dev, void *data,
> --
> 2.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel