* [PATCH 0/5] virgl: fence fd support
@ 2018-10-25 18:37 Robert Foss
2018-10-25 18:37 ` [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() Robert Foss
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Robert Foss @ 2018-10-25 18:37 UTC (permalink / raw)
To: airlied, kraxel, dri-devel, virtualization, linux-kernel,
Rob Herring, Gustavo Padovan, Emil Velikov
Cc: Robert Foss
This series implements fence support for drm/virtio and
has been tested using qemu, kmscube and the below branches.
Rob Herring solved a reference counting issue and
suggested a context check for the execbuf ioctl, his
changes have been included in the below commits to
keep the tree working at all commits.
The linux series can be found here:
https://gitlab.collabora.com/robertfoss/linux/commits/virtio_fences_v3
As for mesa, the branch can be found here:
https://gitlab.collabora.com/robertfoss/mesa/commits/virtio_fences_v3
Changes since v2:
- drm/virtio: add virtio_gpu_alloc_fence()
- Forward port and fix compilation issues
- drm/virtio: add uapi for in and out explicit fences
- Check exbuf->flags for unsupported flags
Gustavo Padovan (4):
drm/virtio: add virtio_gpu_alloc_fence()
drm/virtio: add in-fences support for explicit synchronization
drm/virtio: add out-fences support for explicit synchronization
drm/virtio: bump driver version after explicit synchronization
addition
Robert Foss (1):
drm/virtio: add uapi for in and out explicit fences
drivers/gpu/drm/virtio/virtgpu_drv.h | 22 +++--
drivers/gpu/drm/virtio/virtgpu_fence.c | 41 ++++++---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 118 +++++++++++++++++++++----
drivers/gpu/drm/virtio/virtgpu_plane.c | 46 ++++++++--
drivers/gpu/drm/virtio/virtgpu_vq.c | 16 ++--
include/uapi/drm/virtgpu_drm.h | 13 ++-
6 files changed, 201 insertions(+), 55 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() 2018-10-25 18:37 [PATCH 0/5] virgl: fence fd support Robert Foss @ 2018-10-25 18:37 ` Robert Foss 2018-10-31 9:38 ` Emil Velikov 2018-10-25 18:37 ` [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences Robert Foss ` (4 subsequent siblings) 5 siblings, 1 reply; 21+ messages in thread From: Robert Foss @ 2018-10-25 18:37 UTC (permalink / raw) To: airlied, kraxel, dri-devel, virtualization, linux-kernel, Rob Herring, Gustavo Padovan, Emil Velikov Cc: Robert Foss From: Gustavo Padovan <gustavo.padovan@collabora.com> Refactor fence creation to remove the potential allocation failure from the cmd_submit and atomic_commit paths. Now the fence should be allocated first and just after we should proceed with the rest of the execution. Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> Signed-off-by: Robert Foss <robert.foss@collabora.com> Suggested-by: Rob Herring <robh@kernel.org> --- Changes since v2: - Forward ported to upstream/master (4.20) drivers/gpu/drm/virtio/virtgpu_drv.h | 18 ++++++---- drivers/gpu/drm/virtio/virtgpu_fence.c | 41 ++++++++++++++++------- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 38 +++++++++++++++++---- drivers/gpu/drm/virtio/virtgpu_plane.c | 46 +++++++++++++++++++++++--- drivers/gpu/drm/virtio/virtgpu_vq.c | 16 ++++----- 5 files changed, 121 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 65605e207bbe..e8d2a67d8049 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -127,6 +127,7 @@ struct virtio_gpu_framebuffer { int x1, y1, x2, y2; /* dirty rect */ spinlock_t dirty_lock; uint32_t hw_res_handle; + struct virtio_gpu_fence *fence; }; #define to_virtio_gpu_framebuffer(x) \ container_of(x, struct virtio_gpu_framebuffer, base) @@ -263,7 +264,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, uint32_t resource_id, uint64_t offset, __le32 width, __le32 height, __le32 x, __le32 y, - struct virtio_gpu_fence **fence); + struct virtio_gpu_fence *fence); void virtio_gpu_cmd_resource_flush(struct virtio_gpu_device *vgdev, uint32_t resource_id, uint32_t x, uint32_t y, @@ -275,7 +276,7 @@ void virtio_gpu_cmd_set_scanout(struct virtio_gpu_device *vgdev, int virtio_gpu_object_attach(struct virtio_gpu_device *vgdev, struct virtio_gpu_object *obj, uint32_t resource_id, - struct virtio_gpu_fence **fence); + struct virtio_gpu_fence *fence); int virtio_gpu_attach_status_page(struct virtio_gpu_device *vgdev); int virtio_gpu_detach_status_page(struct virtio_gpu_device *vgdev); void virtio_gpu_cursor_ping(struct virtio_gpu_device *vgdev, @@ -299,21 +300,21 @@ void virtio_gpu_cmd_context_detach_resource(struct virtio_gpu_device *vgdev, uint32_t resource_id); void virtio_gpu_cmd_submit(struct virtio_gpu_device *vgdev, void *data, uint32_t data_size, - uint32_t ctx_id, struct virtio_gpu_fence **fence); + uint32_t ctx_id, struct virtio_gpu_fence *fence); void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev, uint32_t resource_id, uint32_t ctx_id, uint64_t offset, uint32_t level, struct virtio_gpu_box *box, - struct virtio_gpu_fence **fence); + struct virtio_gpu_fence *fence); void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev, uint32_t resource_id, uint32_t ctx_id, uint64_t offset, uint32_t level, struct virtio_gpu_box *box, - struct virtio_gpu_fence **fence); + struct virtio_gpu_fence *fence); void virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev, struct virtio_gpu_resource_create_3d *rc_3d, - struct virtio_gpu_fence **fence); + struct virtio_gpu_fence *fence); void virtio_gpu_ctrl_ack(struct virtqueue *vq); void virtio_gpu_cursor_ack(struct virtqueue *vq); void virtio_gpu_fence_ack(struct virtqueue *vq); @@ -341,9 +342,12 @@ void virtio_gpu_ttm_fini(struct virtio_gpu_device *vgdev); int virtio_gpu_mmap(struct file *filp, struct vm_area_struct *vma); /* virtio_gpu_fence.c */ +struct virtio_gpu_fence *virtio_gpu_fence_alloc( + struct virtio_gpu_device *vgdev); +void virtio_gpu_fence_cleanup(struct virtio_gpu_fence *fence); int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, struct virtio_gpu_ctrl_hdr *cmd_hdr, - struct virtio_gpu_fence **fence); + struct virtio_gpu_fence *fence); void virtio_gpu_fence_event_process(struct virtio_gpu_device *vdev, u64 last_seq); diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index 00c742a441bf..73f5afc37a32 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -67,28 +67,45 @@ static const struct dma_fence_ops virtio_fence_ops = { .timeline_value_str = virtio_timeline_value_str, }; +struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev) +{ + struct virtio_gpu_fence_driver *drv = &vgdev->fence_drv; + struct virtio_gpu_fence *fence = kzalloc(sizeof(struct virtio_gpu_fence), GFP_ATOMIC); + if (!fence) + return fence; + + fence->drv = drv; + dma_fence_init(&fence->f, &virtio_fence_ops, &drv->lock, drv->context, 0); + + return fence; +} + +void virtio_gpu_fence_cleanup(struct virtio_gpu_fence *fence) +{ + if (!fence) + return; + + if (fence->drv) + dma_fence_put(&fence->f); + else + kfree(fence); +} + int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, struct virtio_gpu_ctrl_hdr *cmd_hdr, - struct virtio_gpu_fence **fence) + struct virtio_gpu_fence *fence) { struct virtio_gpu_fence_driver *drv = &vgdev->fence_drv; unsigned long irq_flags; - *fence = kmalloc(sizeof(struct virtio_gpu_fence), GFP_ATOMIC); - if ((*fence) == NULL) - return -ENOMEM; - spin_lock_irqsave(&drv->lock, irq_flags); - (*fence)->drv = drv; - (*fence)->seq = ++drv->sync_seq; - dma_fence_init(&(*fence)->f, &virtio_fence_ops, &drv->lock, - drv->context, (*fence)->seq); - dma_fence_get(&(*fence)->f); - list_add_tail(&(*fence)->node, &drv->fences); + fence->seq = ++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->seq); return 0; } diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 7bdf6f0e58a5..d01a9ed100d1 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -168,8 +168,15 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, ret = PTR_ERR(buf); goto out_unresv; } + + fence = virtio_gpu_fence_alloc(vgdev); + if (!fence) { + kfree(buf); + ret = -ENOMEM; + goto out_unresv; + } virtio_gpu_cmd_submit(vgdev, buf, exbuf->size, - vfpriv->ctx_id, &fence); + vfpriv->ctx_id, fence); ttm_eu_fence_buffer_objects(&ticket, &validate_list, &fence->f); @@ -288,11 +295,17 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, rc_3d.nr_samples = cpu_to_le32(rc->nr_samples); rc_3d.flags = cpu_to_le32(rc->flags); + fence = virtio_gpu_fence_alloc(vgdev); + if (!fence) { + ret = -ENOMEM; + goto fail_fence; + } + virtio_gpu_cmd_resource_create_3d(vgdev, &rc_3d, NULL); - ret = virtio_gpu_object_attach(vgdev, qobj, res_id, &fence); + ret = virtio_gpu_object_attach(vgdev, qobj, res_id, fence); if (ret) { - ttm_eu_backoff_reservation(&ticket, &validate_list); - goto fail_unref; + virtio_gpu_fence_cleanup(fence); + goto fail_fence; } ttm_eu_fence_buffer_objects(&ticket, &validate_list, &fence->f); } @@ -319,6 +332,8 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, dma_fence_put(&fence->f); } return 0; +fail_fence: +ttm_eu_backoff_reservation(&ticket, &validate_list); fail_unref: if (vgdev->has_virgl_3d) { virtio_gpu_unref_list(&validate_list); @@ -383,10 +398,16 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev, goto out_unres; convert_to_hw_box(&box, &args->box); + + fence = virtio_gpu_fence_alloc(vgdev); + if (!fence) { + ret = -ENOMEM; + goto out_unres; + } virtio_gpu_cmd_transfer_from_host_3d (vgdev, qobj->hw_res_handle, vfpriv->ctx_id, offset, args->level, - &box, &fence); + &box, fence); reservation_object_add_excl_fence(qobj->tbo.resv, &fence->f); @@ -432,10 +453,15 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, (vgdev, qobj->hw_res_handle, offset, box.w, box.h, box.x, box.y, NULL); } else { + fence = virtio_gpu_fence_alloc(vgdev); + if (!fence) { + ret = -ENOMEM; + goto out_unres; + } virtio_gpu_cmd_transfer_to_host_3d (vgdev, qobj->hw_res_handle, vfpriv ? vfpriv->ctx_id : 0, offset, - args->level, &box, &fence); + args->level, &box, fence); reservation_object_add_excl_fence(qobj->tbo.resv, &fence->f); dma_fence_put(&fence->f); diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index dc5b5b2b7aab..7f06c2a0a428 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -187,6 +187,41 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, plane->state->src_h >> 16); } +static int virtio_gpu_cursor_prepare_fb(struct drm_plane *plane, + struct drm_plane_state *new_state) +{ + struct drm_device *dev = plane->dev; + struct virtio_gpu_device *vgdev = dev->dev_private; + struct virtio_gpu_framebuffer *vgfb; + struct virtio_gpu_object *bo; + + if (!new_state->fb) + return 0; + + vgfb = to_virtio_gpu_framebuffer(new_state->fb); + bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); + if (bo && bo->dumb && (plane->state->fb != new_state->fb)) { + vgfb->fence = virtio_gpu_fence_alloc(vgdev); + if (!vgfb->fence) + return -ENOMEM; + } + + return 0; +} + +static void virtio_gpu_cursor_cleanup_fb(struct drm_plane *plane, + struct drm_plane_state *old_state) +{ + struct virtio_gpu_framebuffer *vgfb; + + if (!plane->state->fb) + return; + + vgfb = to_virtio_gpu_framebuffer(plane->state->fb); + if (vgfb->fence) + virtio_gpu_fence_cleanup(vgfb->fence); +} + static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, struct drm_plane_state *old_state) { @@ -194,7 +229,6 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_output *output = NULL; struct virtio_gpu_framebuffer *vgfb; - struct virtio_gpu_fence *fence = NULL; struct virtio_gpu_object *bo = NULL; uint32_t handle; int ret = 0; @@ -220,13 +254,13 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, (vgdev, handle, 0, cpu_to_le32(plane->state->crtc_w), cpu_to_le32(plane->state->crtc_h), - 0, 0, &fence); + 0, 0, vgfb->fence); ret = virtio_gpu_object_reserve(bo, false); if (!ret) { reservation_object_add_excl_fence(bo->tbo.resv, - &fence->f); - dma_fence_put(&fence->f); - fence = NULL; + &vgfb->fence->f); + dma_fence_put(&vgfb->fence->f); + vgfb->fence = NULL; virtio_gpu_object_unreserve(bo); virtio_gpu_object_wait(bo, false); } @@ -268,6 +302,8 @@ static const struct drm_plane_helper_funcs virtio_gpu_primary_helper_funcs = { }; static const struct drm_plane_helper_funcs virtio_gpu_cursor_helper_funcs = { + .prepare_fb = virtio_gpu_cursor_prepare_fb, + .cleanup_fb = virtio_gpu_cursor_cleanup_fb, .atomic_check = virtio_gpu_plane_atomic_check, .atomic_update = virtio_gpu_cursor_plane_update, }; diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 020070d483d3..93593c496fdb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -319,7 +319,7 @@ static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev, static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev, struct virtio_gpu_vbuffer *vbuf, struct virtio_gpu_ctrl_hdr *hdr, - struct virtio_gpu_fence **fence) + struct virtio_gpu_fence *fence) { struct virtqueue *vq = vgdev->ctrlq.vq; int rc; @@ -485,7 +485,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, uint32_t resource_id, uint64_t offset, __le32 width, __le32 height, __le32 x, __le32 y, - struct virtio_gpu_fence **fence) + struct virtio_gpu_fence *fence) { struct virtio_gpu_transfer_to_host_2d *cmd_p; struct virtio_gpu_vbuffer *vbuf; @@ -509,7 +509,7 @@ virtio_gpu_cmd_resource_attach_backing(struct virtio_gpu_device *vgdev, uint32_t resource_id, struct virtio_gpu_mem_entry *ents, uint32_t nents, - struct virtio_gpu_fence **fence) + struct virtio_gpu_fence *fence) { struct virtio_gpu_resource_attach_backing *cmd_p; struct virtio_gpu_vbuffer *vbuf; @@ -764,7 +764,7 @@ void virtio_gpu_cmd_context_detach_resource(struct virtio_gpu_device *vgdev, void virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev, struct virtio_gpu_resource_create_3d *rc_3d, - struct virtio_gpu_fence **fence) + struct virtio_gpu_fence *fence) { struct virtio_gpu_resource_create_3d *cmd_p; struct virtio_gpu_vbuffer *vbuf; @@ -783,7 +783,7 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev, uint32_t resource_id, uint32_t ctx_id, uint64_t offset, uint32_t level, struct virtio_gpu_box *box, - struct virtio_gpu_fence **fence) + struct virtio_gpu_fence *fence) { struct virtio_gpu_transfer_host_3d *cmd_p; struct virtio_gpu_vbuffer *vbuf; @@ -805,7 +805,7 @@ void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev, uint32_t resource_id, uint32_t ctx_id, uint64_t offset, uint32_t level, struct virtio_gpu_box *box, - struct virtio_gpu_fence **fence) + struct virtio_gpu_fence *fence) { struct virtio_gpu_transfer_host_3d *cmd_p; struct virtio_gpu_vbuffer *vbuf; @@ -825,7 +825,7 @@ void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev, void virtio_gpu_cmd_submit(struct virtio_gpu_device *vgdev, void *data, uint32_t data_size, - uint32_t ctx_id, struct virtio_gpu_fence **fence) + uint32_t ctx_id, struct virtio_gpu_fence *fence) { struct virtio_gpu_cmd_submit *cmd_p; struct virtio_gpu_vbuffer *vbuf; @@ -846,7 +846,7 @@ void virtio_gpu_cmd_submit(struct virtio_gpu_device *vgdev, int virtio_gpu_object_attach(struct virtio_gpu_device *vgdev, struct virtio_gpu_object *obj, uint32_t resource_id, - struct virtio_gpu_fence **fence) + struct virtio_gpu_fence *fence) { struct virtio_gpu_mem_entry *ents; struct scatterlist *sg; -- 2.17.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() 2018-10-25 18:37 ` [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() Robert Foss @ 2018-10-31 9:38 ` Emil Velikov 2018-11-01 12:43 ` Robert Foss 0 siblings, 1 reply; 21+ messages in thread From: Emil Velikov @ 2018-10-31 9:38 UTC (permalink / raw) To: Robert Foss Cc: David Airlie, Gerd Hoffmann, ML dri-devel, open list:VIRTIO GPU DRIVER, Linux-Kernel@Vger. Kernel. Org, Rob Herring, Gustavo Padovan, Emil Velikov Hi Rob, On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > Refactor fence creation to remove the potential allocation failure from > the cmd_submit and atomic_commit paths. Now the fence should be allocated > first and just after we should proceed with the rest of the execution. > Commit does a bit more that what the above says: - dummy, factor out fence creation/destruction - use per virtio_gpu_framebuffer fence Personally I'd keep the two separate patches and elaborate on the latter. Obviously in that case, one will need to add 3 lines worth of virtio_gpu_fence_alloc() in virtio_gpu_cursor_plane_update which will be nuked with the next patch. Not a big deal, but it's up-to the maintainer to make the final call if it's worth splitting or not. Couple of minor nitpicks below. > struct virtio_gpu_device *vgdev = dev->dev_private; > struct virtio_gpu_output *output = NULL; > struct virtio_gpu_framebuffer *vgfb; > - struct virtio_gpu_fence *fence = NULL; > struct virtio_gpu_object *bo = NULL; > uint32_t handle; > int ret = 0; Add the virtio_gpu_fence_alloc()? And yes it will be nuked with patch 2/... > +struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev) > +{ > + struct virtio_gpu_fence_driver *drv = &vgdev->fence_drv; > + struct virtio_gpu_fence *fence = kzalloc(sizeof(struct virtio_gpu_fence), GFP_ATOMIC); > + if (!fence) > + return fence; > + > + fence->drv = drv; > + dma_fence_init(&fence->f, &virtio_fence_ops, &drv->lock, drv->context, 0); Oh no, lines over 80 col... while the original code is pretty and neat. > + > + return fence; > +} > + > +void virtio_gpu_fence_cleanup(struct virtio_gpu_fence *fence) > +{ > + if (!fence) > + return; > + > + if (fence->drv) > + dma_fence_put(&fence->f); > + else > + kfree(fence); I'm not sure if/how we reach the else case here? > +} > + > int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, > struct virtio_gpu_ctrl_hdr *cmd_hdr, > - struct virtio_gpu_fence **fence) > + struct virtio_gpu_fence *fence) > { With a follow-up commit, we can drop the no longer needed return type. Which it turns out was never checked ... > @@ -319,6 +332,8 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, > dma_fence_put(&fence->f); > } > return 0; > +fail_fence: The error labels seems to be called after what they do, not what fails. fail_backoff seems better IMHO. > +ttm_eu_backoff_reservation(&ticket, &validate_list); Indentation seems off (or my client ate it)? HTH Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() 2018-10-31 9:38 ` Emil Velikov @ 2018-11-01 12:43 ` Robert Foss 2018-11-05 6:38 ` Gerd Hoffmann 0 siblings, 1 reply; 21+ messages in thread From: Robert Foss @ 2018-11-01 12:43 UTC (permalink / raw) To: Emil Velikov Cc: David Airlie, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, open list:VIRTIO GPU DRIVER, Gustavo Padovan, Gerd Hoffmann, Emil Velikov Hey Emil, On 2018-10-31 10:38, Emil Velikov wrote: > Hi Rob, > > On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: >> >> From: Gustavo Padovan <gustavo.padovan@collabora.com> >> >> Refactor fence creation to remove the potential allocation failure from >> the cmd_submit and atomic_commit paths. Now the fence should be allocated >> first and just after we should proceed with the rest of the execution. >> > > Commit does a bit more that what the above says: > - dummy, factor out fence creation/destruction > - use per virtio_gpu_framebuffer fence > > Personally I'd keep the two separate patches and elaborate on the latter. > Obviously in that case, one will need to add 3 lines worth of > virtio_gpu_fence_alloc() in virtio_gpu_cursor_plane_update which will be nuked > with the next patch. > > Not a big deal, but it's up-to the maintainer to make the final call if it's > worth splitting or not. Agreed, I'll hold off with this change until then. > > Couple of minor nitpicks below. > >> struct virtio_gpu_device *vgdev = dev->dev_private; >> struct virtio_gpu_output *output = NULL; >> struct virtio_gpu_framebuffer *vgfb; >> - struct virtio_gpu_fence *fence = NULL; >> struct virtio_gpu_object *bo = NULL; >> uint32_t handle; >> int ret = 0; > > Add the virtio_gpu_fence_alloc()? And yes it will be nuked with patch 2/... > > > >> +struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev) >> +{ >> + struct virtio_gpu_fence_driver *drv = &vgdev->fence_drv; >> + struct virtio_gpu_fence *fence = kzalloc(sizeof(struct virtio_gpu_fence), GFP_ATOMIC); >> + if (!fence) >> + return fence; >> + >> + fence->drv = drv; >> + dma_fence_init(&fence->f, &virtio_fence_ops, &drv->lock, drv->context, 0); > Oh no, lines over 80 col... while the original code is pretty and neat. Ack > >> + >> + return fence; >> +} >> + >> +void virtio_gpu_fence_cleanup(struct virtio_gpu_fence *fence) >> +{ >> + if (!fence) >> + return; >> + >> + if (fence->drv) >> + dma_fence_put(&fence->f); >> + else >> + kfree(fence); > I'm not sure if/how we reach the else case here? That case should never be hit, and if it is that's a bug. Fixed in v4. > >> +} >> + >> int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, >> struct virtio_gpu_ctrl_hdr *cmd_hdr, >> - struct virtio_gpu_fence **fence) >> + struct virtio_gpu_fence *fence) >> { > > With a follow-up commit, we can drop the no longer needed return type. > Which it turns out was never checked ... > Fixed during drm-misc-next rebase for v4. > > >> @@ -319,6 +332,8 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, >> dma_fence_put(&fence->f); >> } >> return 0; >> +fail_fence: > > The error labels seems to be called after what they do, not what > fails. fail_backoff seems better IMHO. Agreed. Fixed in v4. > >> +ttm_eu_backoff_reservation(&ticket, &validate_list); > Indentation seems off (or my client ate it)? No, the indentation is bad here. Fixed in v4. Thanks for the feedback Emil. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() 2018-11-01 12:43 ` Robert Foss @ 2018-11-05 6:38 ` Gerd Hoffmann 0 siblings, 0 replies; 21+ messages in thread From: Gerd Hoffmann @ 2018-11-05 6:38 UTC (permalink / raw) To: Robert Foss Cc: Emil Velikov, David Airlie, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, open list:VIRTIO GPU DRIVER, Gustavo Padovan, Emil Velikov > > On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: > > > > > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > > > > > Refactor fence creation to remove the potential allocation failure from > > > the cmd_submit and atomic_commit paths. Now the fence should be allocated > > > first and just after we should proceed with the rest of the execution. > > > > > > > Commit does a bit more that what the above says: > > - dummy, factor out fence creation/destruction > > - use per virtio_gpu_framebuffer fence > > > > Personally I'd keep the two separate patches and elaborate on the latter. > > Obviously in that case, one will need to add 3 lines worth of > > virtio_gpu_fence_alloc() in virtio_gpu_cursor_plane_update which will be nuked > > with the next patch. > > > > Not a big deal, but it's up-to the maintainer to make the final call if it's > > worth splitting or not. > > Agreed, I'll hold off with this change until then. No need to split this, but a bit more verbose commit message would be good. cheers, Gerd ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-10-25 18:37 [PATCH 0/5] virgl: fence fd support Robert Foss 2018-10-25 18:37 ` [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() Robert Foss @ 2018-10-25 18:37 ` Robert Foss 2018-10-30 6:11 ` Gerd Hoffmann 2018-10-31 9:38 ` Emil Velikov 2018-10-25 18:37 ` [PATCH 3/5] drm/virtio: add in-fences support for explicit synchronization Robert Foss ` (3 subsequent siblings) 5 siblings, 2 replies; 21+ messages in thread From: Robert Foss @ 2018-10-25 18:37 UTC (permalink / raw) To: airlied, kraxel, dri-devel, virtualization, linux-kernel, Rob Herring, Gustavo Padovan, Emil Velikov Cc: Robert Foss Add a new field called fence_fd that will be used by userspace to send in-fences to the kernel and receive out-fences created by the kernel. This uapi enables virtio to take advantage of explicit synchronization of dma-bufs. There are two new flags: * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd. * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd The execbuffer IOCTL is now read-write to allow the userspace to read the out-fence. On error -1 should be returned in the fence_fd field. Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> Signed-off-by: Robert Foss <robert.foss@collabora.com> --- Changes since v2: - Since exbuf-flags is a new flag, check that unsupported flags aren't set. drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++ include/uapi/drm/virtgpu_drm.h | 13 ++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index d01a9ed100d1..1af289b28fc4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, struct ww_acquire_ctx ticket; void *buf; + exbuf->fence_fd = -1; + if (vgdev->has_virgl_3d == false) return -ENOSYS; + if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) + return -EINVAL; + INIT_LIST_HEAD(&validate_list); if (exbuf->num_bo_handles) { diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h index 9a781f0611df..91062f4ac7c5 100644 --- a/include/uapi/drm/virtgpu_drm.h +++ b/include/uapi/drm/virtgpu_drm.h @@ -47,6 +47,13 @@ extern "C" { #define DRM_VIRTGPU_WAIT 0x08 #define DRM_VIRTGPU_GET_CAPS 0x09 +#define VIRTGPU_EXECBUF_FENCE_FD_IN 0x01 +#define VIRTGPU_EXECBUF_FENCE_FD_OUT 0x02 +#define VIRTGPU_EXECBUF_FLAGS (\ + VIRTGPU_EXECBUF_FENCE_FD_IN |\ + VIRTGPU_EXECBUF_FENCE_FD_OUT |\ + 0) + struct drm_virtgpu_map { __u64 offset; /* use for mmap system call */ __u32 handle; @@ -54,12 +61,12 @@ struct drm_virtgpu_map { }; struct drm_virtgpu_execbuffer { - __u32 flags; /* for future use */ + __u32 flags; __u32 size; __u64 command; /* void* */ __u64 bo_handles; __u32 num_bo_handles; - __u32 pad; + __s32 fence_fd; }; #define VIRTGPU_PARAM_3D_FEATURES 1 /* do we have 3D features in the hw */ @@ -137,7 +144,7 @@ struct drm_virtgpu_get_caps { DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_MAP, struct drm_virtgpu_map) #define DRM_IOCTL_VIRTGPU_EXECBUFFER \ - DRM_IOW(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ + DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ struct drm_virtgpu_execbuffer) #define DRM_IOCTL_VIRTGPU_GETPARAM \ -- 2.17.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-10-25 18:37 ` [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences Robert Foss @ 2018-10-30 6:11 ` Gerd Hoffmann 2018-10-30 11:31 ` Emil Velikov 2018-10-31 9:38 ` Emil Velikov 1 sibling, 1 reply; 21+ messages in thread From: Gerd Hoffmann @ 2018-10-30 6:11 UTC (permalink / raw) To: Robert Foss Cc: airlied, dri-devel, virtualization, linux-kernel, Rob Herring, Gustavo Padovan, Emil Velikov Hi, > The execbuffer IOCTL is now read-write to allow the userspace to read the > out-fence. > #define DRM_IOCTL_VIRTGPU_EXECBUFFER \ > - DRM_IOW(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ > + DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ > struct drm_virtgpu_execbuffer) That changes the ioctl number and breaks the userspace api. cheers, Gerd ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-10-30 6:11 ` Gerd Hoffmann @ 2018-10-30 11:31 ` Emil Velikov 2018-10-30 13:52 ` Gerd Hoffmann 0 siblings, 1 reply; 21+ messages in thread From: Emil Velikov @ 2018-10-30 11:31 UTC (permalink / raw) To: Gerd Hoffmann Cc: Robert Foss, David Airlie, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, open list:VIRTIO GPU DRIVER, Gustavo Padovan, Emil Velikov HI Gerd, On Tue, 30 Oct 2018 at 06:11, Gerd Hoffmann <kraxel@redhat.com> wrote: > > Hi, > > > The execbuffer IOCTL is now read-write to allow the userspace to read the > > out-fence. > > > #define DRM_IOCTL_VIRTGPU_EXECBUFFER \ > > - DRM_IOW(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ > > + DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ > > struct drm_virtgpu_execbuffer) > > That changes the ioctl number and breaks the userspace api. > Have you looked at the drm_ioctl() implementation? AFAICT it explicitly caters for this kind of changes. -Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-10-30 11:31 ` Emil Velikov @ 2018-10-30 13:52 ` Gerd Hoffmann 2018-10-30 15:48 ` Emil Velikov 0 siblings, 1 reply; 21+ messages in thread From: Gerd Hoffmann @ 2018-10-30 13:52 UTC (permalink / raw) To: Emil Velikov Cc: Robert Foss, David Airlie, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, open list:VIRTIO GPU DRIVER, Gustavo Padovan, Emil Velikov On Tue, Oct 30, 2018 at 11:31:04AM +0000, Emil Velikov wrote: > HI Gerd, > > On Tue, 30 Oct 2018 at 06:11, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > Hi, > > > > > The execbuffer IOCTL is now read-write to allow the userspace to read the > > > out-fence. > > > > > #define DRM_IOCTL_VIRTGPU_EXECBUFFER \ > > > - DRM_IOW(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ > > > + DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ > > > struct drm_virtgpu_execbuffer) > > > > That changes the ioctl number and breaks the userspace api. > > > Have you looked at the drm_ioctl() implementation? AFAICT it > explicitly caters for this kind of changes. Looking ... The direction bits are not used to lookup the ioctl functions, so it should work indeed. Series doesn't apply to drm-misc-next and needs a rebase. cheers, Gerd ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-10-30 13:52 ` Gerd Hoffmann @ 2018-10-30 15:48 ` Emil Velikov 0 siblings, 0 replies; 21+ messages in thread From: Emil Velikov @ 2018-10-30 15:48 UTC (permalink / raw) To: Gerd Hoffmann Cc: Robert Foss, David Airlie, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, open list:VIRTIO GPU DRIVER, Gustavo Padovan, Emil Velikov On Tue, 30 Oct 2018 at 13:52, Gerd Hoffmann <kraxel@redhat.com> wrote: > > On Tue, Oct 30, 2018 at 11:31:04AM +0000, Emil Velikov wrote: > > HI Gerd, > > > > On Tue, 30 Oct 2018 at 06:11, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > > > Hi, > > > > > > > The execbuffer IOCTL is now read-write to allow the userspace to read the > > > > out-fence. > > > > > > > #define DRM_IOCTL_VIRTGPU_EXECBUFFER \ > > > > - DRM_IOW(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ > > > > + DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_EXECBUFFER,\ > > > > struct drm_virtgpu_execbuffer) > > > > > > That changes the ioctl number and breaks the userspace api. > > > > > Have you looked at the drm_ioctl() implementation? AFAICT it > > explicitly caters for this kind of changes. > > Looking ... > > The direction bits are not used to lookup the ioctl functions, > so it should work indeed. > Nice, thanks for confirming. > Series doesn't apply to drm-misc-next and needs a rebase. > Might be nicer to address that alongside any feedback. Otherwise it'll be spamming people just for the sake of rebasing. If i find some time I'll post some comments later on today. HTH Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-10-25 18:37 ` [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences Robert Foss 2018-10-30 6:11 ` Gerd Hoffmann @ 2018-10-31 9:38 ` Emil Velikov 2018-11-01 12:56 ` Robert Foss 1 sibling, 1 reply; 21+ messages in thread From: Emil Velikov @ 2018-10-31 9:38 UTC (permalink / raw) To: Robert Foss Cc: David Airlie, Gerd Hoffmann, ML dri-devel, open list:VIRTIO GPU DRIVER, Linux-Kernel@Vger. Kernel. Org, Rob Herring, Gustavo Padovan, Emil Velikov Hi Rob, On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: > > Add a new field called fence_fd that will be used by userspace to send > in-fences to the kernel and receive out-fences created by the kernel. > > This uapi enables virtio to take advantage of explicit synchronization of > dma-bufs. > > There are two new flags: > > * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd. > * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd > > The execbuffer IOCTL is now read-write to allow the userspace to read the > out-fence. > > On error -1 should be returned in the fence_fd field. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > Changes since v2: > - Since exbuf-flags is a new flag, check that unsupported > flags aren't set. > > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++ > include/uapi/drm/virtgpu_drm.h | 13 ++++++++++--- > 2 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > index d01a9ed100d1..1af289b28fc4 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, > struct ww_acquire_ctx ticket; > void *buf; > > + exbuf->fence_fd = -1; > + Move this after the sanity checking. > if (vgdev->has_virgl_3d == false) > return -ENOSYS; > > + if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) > + return -EINVAL; > + I assume this did this trigger when using old userspace? With those the patch is Reviewed-by: Emil Velikov <emil.velikov@collabora.com> Thanks Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-10-31 9:38 ` Emil Velikov @ 2018-11-01 12:56 ` Robert Foss 2018-11-02 13:34 ` Emil Velikov 0 siblings, 1 reply; 21+ messages in thread From: Robert Foss @ 2018-11-01 12:56 UTC (permalink / raw) To: Emil Velikov Cc: David Airlie, Gerd Hoffmann, ML dri-devel, open list:VIRTIO GPU DRIVER, Linux-Kernel@Vger. Kernel. Org, Rob Herring, Gustavo Padovan, Emil Velikov On 2018-10-31 10:38, Emil Velikov wrote: > Hi Rob, > > On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: >> >> Add a new field called fence_fd that will be used by userspace to send >> in-fences to the kernel and receive out-fences created by the kernel. >> >> This uapi enables virtio to take advantage of explicit synchronization of >> dma-bufs. >> >> There are two new flags: >> >> * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd. >> * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd >> >> The execbuffer IOCTL is now read-write to allow the userspace to read the >> out-fence. >> >> On error -1 should be returned in the fence_fd field. >> >> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> >> Signed-off-by: Robert Foss <robert.foss@collabora.com> >> --- >> Changes since v2: >> - Since exbuf-flags is a new flag, check that unsupported >> flags aren't set. >> >> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++ >> include/uapi/drm/virtgpu_drm.h | 13 ++++++++++--- >> 2 files changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c >> index d01a9ed100d1..1af289b28fc4 100644 >> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c >> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c >> @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, >> struct ww_acquire_ctx ticket; >> void *buf; >> >> + exbuf->fence_fd = -1; >> + > Move this after the sanity checking. Agreed. Fixed in v4 > >> if (vgdev->has_virgl_3d == false) >> return -ENOSYS; >> >> + if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) >> + return -EINVAL; >> + > I assume this did this trigger when using old userspace? No, not as far as I'm aware. This check is there to prevent userspace from polluting the bitspace of flag, so that all free bits can be used for new flags. As far as I understand this is pointed out by a drm driver development document written by danvet, which I unfortunately can't seem to find the link for at the moment. > > With those the patch is > Reviewed-by: Emil Velikov <emil.velikov@collabora.com> > > Thanks > Emil > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-11-01 12:56 ` Robert Foss @ 2018-11-02 13:34 ` Emil Velikov 2018-11-02 14:42 ` Robert Foss 0 siblings, 1 reply; 21+ messages in thread From: Emil Velikov @ 2018-11-02 13:34 UTC (permalink / raw) To: Robert Foss Cc: David Airlie, Gerd Hoffmann, ML dri-devel, open list:VIRTIO GPU DRIVER, Linux-Kernel@Vger. Kernel. Org, Rob Herring, Gustavo Padovan, Emil Velikov On Thu, 1 Nov 2018 at 12:56, Robert Foss <robert.foss@collabora.com> wrote: > On 2018-10-31 10:38, Emil Velikov wrote: > > Hi Rob, > > > > On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: > >> > >> Add a new field called fence_fd that will be used by userspace to send > >> in-fences to the kernel and receive out-fences created by the kernel. > >> > >> This uapi enables virtio to take advantage of explicit synchronization of > >> dma-bufs. > >> > >> There are two new flags: > >> > >> * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd. > >> * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd > >> > >> The execbuffer IOCTL is now read-write to allow the userspace to read the > >> out-fence. > >> > >> On error -1 should be returned in the fence_fd field. > >> > >> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > >> Signed-off-by: Robert Foss <robert.foss@collabora.com> > >> --- > >> Changes since v2: > >> - Since exbuf-flags is a new flag, check that unsupported > >> flags aren't set. > >> > >> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++ > >> include/uapi/drm/virtgpu_drm.h | 13 ++++++++++--- > >> 2 files changed, 15 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > >> index d01a9ed100d1..1af289b28fc4 100644 > >> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > >> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > >> @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, > >> struct ww_acquire_ctx ticket; > >> void *buf; > >> > >> + exbuf->fence_fd = -1; > >> + > > Move this after the sanity checking. > > Agreed. Fixed in v4 > > > > >> if (vgdev->has_virgl_3d == false) > >> return -ENOSYS; > >> > >> + if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) > >> + return -EINVAL; > >> + > > I assume this did this trigger when using old userspace? > > No, not as far as I'm aware. This check is there to prevent userspace from > polluting the bitspace of flag, so that all free bits can be used for new flags. > > As far as I understand this is pointed out by a drm driver development document > written by danvet, which I unfortunately can't seem to find the link for at the > moment. > Yes that is correct. What I was asking is: Does a kernel with this patch, work with mesa lacking the corresponding updates? I'd imagine things work just fine. -Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences 2018-11-02 13:34 ` Emil Velikov @ 2018-11-02 14:42 ` Robert Foss 0 siblings, 0 replies; 21+ messages in thread From: Robert Foss @ 2018-11-02 14:42 UTC (permalink / raw) To: Emil Velikov Cc: David Airlie, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, open list:VIRTIO GPU DRIVER, Gustavo Padovan, Gerd Hoffmann, Emil Velikov Hey Emil, On 2018-11-02 14:34, Emil Velikov wrote: > On Thu, 1 Nov 2018 at 12:56, Robert Foss <robert.foss@collabora.com> wrote: >> On 2018-10-31 10:38, Emil Velikov wrote: >>> Hi Rob, >>> >>> On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: >>>> >>>> Add a new field called fence_fd that will be used by userspace to send >>>> in-fences to the kernel and receive out-fences created by the kernel. >>>> >>>> This uapi enables virtio to take advantage of explicit synchronization of >>>> dma-bufs. >>>> >>>> There are two new flags: >>>> >>>> * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd. >>>> * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd >>>> >>>> The execbuffer IOCTL is now read-write to allow the userspace to read the >>>> out-fence. >>>> >>>> On error -1 should be returned in the fence_fd field. >>>> >>>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> >>>> Signed-off-by: Robert Foss <robert.foss@collabora.com> >>>> --- >>>> Changes since v2: >>>> - Since exbuf-flags is a new flag, check that unsupported >>>> flags aren't set. >>>> >>>> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++ >>>> include/uapi/drm/virtgpu_drm.h | 13 ++++++++++--- >>>> 2 files changed, 15 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c >>>> index d01a9ed100d1..1af289b28fc4 100644 >>>> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c >>>> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c >>>> @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, >>>> struct ww_acquire_ctx ticket; >>>> void *buf; >>>> >>>> + exbuf->fence_fd = -1; >>>> + >>> Move this after the sanity checking. >> >> Agreed. Fixed in v4 >> >>> >>>> if (vgdev->has_virgl_3d == false) >>>> return -ENOSYS; >>>> >>>> + if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) >>>> + return -EINVAL; >>>> + >>> I assume this did this trigger when using old userspace? >> >> No, not as far as I'm aware. This check is there to prevent userspace from >> polluting the bitspace of flag, so that all free bits can be used for new flags. >> >> As far as I understand this is pointed out by a drm driver development document >> written by danvet, which I unfortunately can't seem to find the link for at the >> moment. >> > Yes that is correct. What I was asking is: > > Does a kernel with this patch, work with mesa lacking the corresponding updates? > I'd imagine things work just fine. Yes it does! > > -Emil > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/5] drm/virtio: add in-fences support for explicit synchronization 2018-10-25 18:37 [PATCH 0/5] virgl: fence fd support Robert Foss 2018-10-25 18:37 ` [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() Robert Foss 2018-10-25 18:37 ` [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences Robert Foss @ 2018-10-25 18:37 ` Robert Foss 2018-10-31 9:38 ` Emil Velikov 2018-10-25 18:37 ` [PATCH 4/5] drm/virtio: add out-fences " Robert Foss ` (2 subsequent siblings) 5 siblings, 1 reply; 21+ messages in thread From: Robert Foss @ 2018-10-25 18:37 UTC (permalink / raw) To: airlied, kraxel, dri-devel, virtualization, linux-kernel, Rob Herring, Gustavo Padovan, Emil Velikov Cc: Robert Foss From: Gustavo Padovan <gustavo.padovan@collabora.com> When the execbuf call receives an in-fence it will get the dma_fence related to that fence fd and wait on it before submitting the draw call. Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> Signed-off-by: Robert Foss <robert.foss@collabora.com> Suggested-by: Rob Herring <robh@kernel.org> --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 43 ++++++++++++++++++++------ 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 1af289b28fc4..0368195966aa 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -28,6 +28,7 @@ #include <drm/drmP.h> #include <drm/virtgpu_drm.h> #include <drm/ttm/ttm_execbuf_util.h> +#include <linux/sync_file.h> #include "virtgpu_drv.h" @@ -114,6 +115,8 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, struct ttm_validate_buffer *buflist = NULL; int i; struct ww_acquire_ctx ticket; + struct dma_fence *in_fence = NULL; + int in_fence_fd = exbuf->fence_fd; void *buf; exbuf->fence_fd = -1; @@ -124,6 +127,22 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) return -EINVAL; + if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) { + in_fence = sync_file_get_fence(in_fence_fd); + if (!in_fence) + return -EINVAL; + + /* + * Wait if the fence is from a foreign context, or if the fence + * array contains any fence from a foreign context. + */ + if (!dma_fence_match_context(in_fence, vgdev->fence_drv.context)) { + ret = dma_fence_wait(in_fence, true); + if (ret) + return ret; + } + } + INIT_LIST_HEAD(&validate_list); if (exbuf->num_bo_handles) { @@ -133,26 +152,22 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, sizeof(struct ttm_validate_buffer), GFP_KERNEL | __GFP_ZERO); if (!bo_handles || !buflist) { - kvfree(bo_handles); - kvfree(buflist); - return -ENOMEM; + ret = -ENOMEM; + goto out_in_fence; } user_bo_handles = (void __user *)(uintptr_t)exbuf->bo_handles; if (copy_from_user(bo_handles, user_bo_handles, exbuf->num_bo_handles * sizeof(uint32_t))) { ret = -EFAULT; - kvfree(bo_handles); - kvfree(buflist); - return ret; + goto out_in_fence; } for (i = 0; i < exbuf->num_bo_handles; i++) { gobj = drm_gem_object_lookup(drm_file, bo_handles[i]); if (!gobj) { - kvfree(bo_handles); - kvfree(buflist); - return -ENOENT; + ret = -ENOENT; + goto out_in_fence; } qobj = gem_to_virtio_gpu_obj(gobj); @@ -161,6 +176,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, list_add(&buflist[i].head, &validate_list); } kvfree(bo_handles); + bo_handles = NULL; } ret = virtio_gpu_object_list_validate(&ticket, &validate_list); @@ -180,6 +196,12 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, ret = -ENOMEM; goto out_unresv; } + + if (in_fence) { + dma_fence_put(in_fence); + in_fence = NULL; + } + virtio_gpu_cmd_submit(vgdev, buf, exbuf->size, vfpriv->ctx_id, fence); @@ -195,7 +217,10 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, ttm_eu_backoff_reservation(&ticket, &validate_list); out_free: virtio_gpu_unref_list(&validate_list); +out_in_fence: + kvfree(bo_handles); kvfree(buflist); + dma_fence_put(in_fence); return ret; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] drm/virtio: add in-fences support for explicit synchronization 2018-10-25 18:37 ` [PATCH 3/5] drm/virtio: add in-fences support for explicit synchronization Robert Foss @ 2018-10-31 9:38 ` Emil Velikov 0 siblings, 0 replies; 21+ messages in thread From: Emil Velikov @ 2018-10-31 9:38 UTC (permalink / raw) To: Robert Foss Cc: David Airlie, Gerd Hoffmann, ML dri-devel, open list:VIRTIO GPU DRIVER, Linux-Kernel@Vger. Kernel. Org, Rob Herring, Gustavo Padovan, Emil Velikov Hi Rob, On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: > @@ -124,6 +127,22 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, > if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) > return -EINVAL; > > + if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) { > + in_fence = sync_file_get_fence(in_fence_fd); > + if (!in_fence) > + return -EINVAL; > + > + /* > + * Wait if the fence is from a foreign context, or if the fence > + * array contains any fence from a foreign context. > + */ > + if (!dma_fence_match_context(in_fence, vgdev->fence_drv.context)) { > + ret = dma_fence_wait(in_fence, true); > + if (ret) > + return ret; Aren't we missing dma_fence_put() before the return here? With that Reviewed-by: Emil Velikov <emil.velikov@collabora.com> -Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/5] drm/virtio: add out-fences support for explicit synchronization 2018-10-25 18:37 [PATCH 0/5] virgl: fence fd support Robert Foss ` (2 preceding siblings ...) 2018-10-25 18:37 ` [PATCH 3/5] drm/virtio: add in-fences support for explicit synchronization Robert Foss @ 2018-10-25 18:37 ` Robert Foss 2018-10-31 9:39 ` Emil Velikov 2018-10-25 18:37 ` [PATCH 5/5] drm/virtio: bump driver version after explicit synchronization addition Robert Foss 2018-10-31 9:45 ` [PATCH 0/5] virgl: fence fd support Emil Velikov 5 siblings, 1 reply; 21+ messages in thread From: Robert Foss @ 2018-10-25 18:37 UTC (permalink / raw) To: airlied, kraxel, dri-devel, virtualization, linux-kernel, Rob Herring, Gustavo Padovan, Emil Velikov Cc: Robert Foss From: Gustavo Padovan <gustavo.padovan@collabora.com> On the out-fence side we get fence returned by the submitted draw call and attach it to a sync_file and send the sync_file fd to userspace. On error -1 is returned to userspace. Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> Signed-off-by: Robert Foss <robert.foss@collabora.com> Suggested-by: Rob Herring <robh@kernel.org> --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 50 +++++++++++++++++++------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 0368195966aa..32e714a1c753 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -106,7 +106,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_fpriv *vfpriv = drm_file->driver_priv; struct drm_gem_object *gobj; - struct virtio_gpu_fence *fence; + struct virtio_gpu_fence *out_fence; struct virtio_gpu_object *qobj; int ret; uint32_t *bo_handles = NULL; @@ -116,7 +116,9 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, int i; struct ww_acquire_ctx ticket; struct dma_fence *in_fence = NULL; + struct sync_file *sync_file; int in_fence_fd = exbuf->fence_fd; + int out_fence_fd = -1; void *buf; exbuf->fence_fd = -1; @@ -143,6 +145,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, } } + if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_OUT) { + out_fence_fd = get_unused_fd_flags(O_CLOEXEC); + if (out_fence_fd < 0) { + ret = out_fence_fd; + goto out_in_fence; + } + } + INIT_LIST_HEAD(&validate_list); if (exbuf->num_bo_handles) { @@ -153,21 +163,21 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, GFP_KERNEL | __GFP_ZERO); if (!bo_handles || !buflist) { ret = -ENOMEM; - goto out_in_fence; + goto out_unused_fd; } user_bo_handles = (void __user *)(uintptr_t)exbuf->bo_handles; if (copy_from_user(bo_handles, user_bo_handles, exbuf->num_bo_handles * sizeof(uint32_t))) { ret = -EFAULT; - goto out_in_fence; + goto out_unused_fd; } for (i = 0; i < exbuf->num_bo_handles; i++) { gobj = drm_gem_object_lookup(drm_file, bo_handles[i]); if (!gobj) { ret = -ENOENT; - goto out_in_fence; + goto out_unused_fd; } qobj = gem_to_virtio_gpu_obj(gobj); @@ -190,11 +200,22 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, goto out_unresv; } - fence = virtio_gpu_fence_alloc(vgdev); - if (!fence) { - kfree(buf); + out_fence = virtio_gpu_fence_alloc(vgdev); + if(!out_fence) { ret = -ENOMEM; - goto out_unresv; + goto out_memdup; + } + + if (out_fence_fd >= 0) { + sync_file = sync_file_create(&out_fence->f); + if (!sync_file) { + dma_fence_put(&out_fence->f); + ret = -ENOMEM; + goto out_memdup; + } + + exbuf->fence_fd = out_fence_fd; + fd_install(out_fence_fd, sync_file->file); } if (in_fence) { @@ -203,23 +224,28 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, } virtio_gpu_cmd_submit(vgdev, buf, exbuf->size, - vfpriv->ctx_id, fence); + vfpriv->ctx_id, out_fence); - ttm_eu_fence_buffer_objects(&ticket, &validate_list, &fence->f); + ttm_eu_fence_buffer_objects(&ticket, &validate_list, &out_fence->f); /* fence the command bo */ virtio_gpu_unref_list(&validate_list); kvfree(buflist); - dma_fence_put(&fence->f); return 0; +out_memdup: + kfree(buf); out_unresv: ttm_eu_backoff_reservation(&ticket, &validate_list); out_free: virtio_gpu_unref_list(&validate_list); -out_in_fence: +out_unused_fd: kvfree(bo_handles); kvfree(buflist); + + if (out_fence_fd >= 0) + put_unused_fd(out_fence_fd); +out_in_fence: dma_fence_put(in_fence); return ret; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] drm/virtio: add out-fences support for explicit synchronization 2018-10-25 18:37 ` [PATCH 4/5] drm/virtio: add out-fences " Robert Foss @ 2018-10-31 9:39 ` Emil Velikov 0 siblings, 0 replies; 21+ messages in thread From: Emil Velikov @ 2018-10-31 9:39 UTC (permalink / raw) To: Robert Foss Cc: David Airlie, Gerd Hoffmann, ML dri-devel, open list:VIRTIO GPU DRIVER, Linux-Kernel@Vger. Kernel. Org, Rob Herring, Gustavo Padovan, Emil Velikov Hi Rob, On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > On the out-fence side we get fence returned by the submitted draw call > and attach it to a sync_file and send the sync_file fd to userspace. On > error -1 is returned to userspace. > Can we have both an IN and OUT fence at the same time? Either way, please mention that in the commit message. > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > Signed-off-by: Robert Foss <robert.foss@collabora.com> > Suggested-by: Rob Herring <robh@kernel.org> > --- > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 50 +++++++++++++++++++------- > 1 file changed, 38 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > index 0368195966aa..32e714a1c753 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > @@ -106,7 +106,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, > struct virtio_gpu_device *vgdev = dev->dev_private; > struct virtio_gpu_fpriv *vfpriv = drm_file->driver_priv; > struct drm_gem_object *gobj; > - struct virtio_gpu_fence *fence; > + struct virtio_gpu_fence *out_fence; > struct virtio_gpu_object *qobj; > int ret; > uint32_t *bo_handles = NULL; > @@ -116,7 +116,9 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, > int i; > struct ww_acquire_ctx ticket; > struct dma_fence *in_fence = NULL; > + struct sync_file *sync_file; > int in_fence_fd = exbuf->fence_fd; > + int out_fence_fd = -1; > void *buf; > > exbuf->fence_fd = -1; > @@ -143,6 +145,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, > } > } > > + if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_OUT) { > + out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > + if (out_fence_fd < 0) { > + ret = out_fence_fd; > + goto out_in_fence; > + } > + } > + If the answer to the above is "no" we want a check around here. With that the patch is Reviewed-by: Emil Velikov <emil.velikov@collabora.com> -Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/5] drm/virtio: bump driver version after explicit synchronization addition 2018-10-25 18:37 [PATCH 0/5] virgl: fence fd support Robert Foss ` (3 preceding siblings ...) 2018-10-25 18:37 ` [PATCH 4/5] drm/virtio: add out-fences " Robert Foss @ 2018-10-25 18:37 ` Robert Foss 2018-10-31 9:39 ` Emil Velikov 2018-10-31 9:45 ` [PATCH 0/5] virgl: fence fd support Emil Velikov 5 siblings, 1 reply; 21+ messages in thread From: Robert Foss @ 2018-10-25 18:37 UTC (permalink / raw) To: airlied, kraxel, dri-devel, virtualization, linux-kernel, Rob Herring, Gustavo Padovan, Emil Velikov Cc: Gustavo Padovan, Robert Foss From: Gustavo Padovan <gustavo.padovan@collabora.com> To reflect the (backward compatible) changes in the uabi we are bumping the driver's version. Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co> Signed-off-by: Robert Foss <robert.foss@collabora.com> --- drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index e8d2a67d8049..a26483b5016e 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -46,8 +46,8 @@ #define DRIVER_DATE "0" #define DRIVER_MAJOR 0 -#define DRIVER_MINOR 0 -#define DRIVER_PATCHLEVEL 1 +#define DRIVER_MINOR 1 +#define DRIVER_PATCHLEVEL 0 /* virtgpu_drm_bus.c */ int drm_virtio_init(struct drm_driver *driver, struct virtio_device *vdev); -- 2.17.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] drm/virtio: bump driver version after explicit synchronization addition 2018-10-25 18:37 ` [PATCH 5/5] drm/virtio: bump driver version after explicit synchronization addition Robert Foss @ 2018-10-31 9:39 ` Emil Velikov 0 siblings, 0 replies; 21+ messages in thread From: Emil Velikov @ 2018-10-31 9:39 UTC (permalink / raw) To: Robert Foss Cc: David Airlie, Gerd Hoffmann, ML dri-devel, open list:VIRTIO GPU DRIVER, Linux-Kernel@Vger. Kernel. Org, Rob Herring, Gustavo Padovan, Emil Velikov, Gustavo Padovan On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@collabora.com> wrote: > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > To reflect the (backward compatible) changes in the uabi we are bumping > the driver's version. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co> > Signed-off-by: Robert Foss <robert.foss@collabora.com> Not strictly required, but doesn't hurt either. Reviewed-by: Emil Velikov <emil.velikov@collabora.com> -Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] virgl: fence fd support 2018-10-25 18:37 [PATCH 0/5] virgl: fence fd support Robert Foss ` (4 preceding siblings ...) 2018-10-25 18:37 ` [PATCH 5/5] drm/virtio: bump driver version after explicit synchronization addition Robert Foss @ 2018-10-31 9:45 ` Emil Velikov 5 siblings, 0 replies; 21+ messages in thread From: Emil Velikov @ 2018-10-31 9:45 UTC (permalink / raw) To: Robert Foss Cc: David Airlie, Gerd Hoffmann, ML dri-devel, open list:VIRTIO GPU DRIVER, Linux-Kernel@Vger. Kernel. Org, Rob Herring, Gustavo Padovan, Emil Velikov Hi Rob, On Thu, 25 Oct 2018 at 19:37, Robert Foss <robert.foss@collabora.com> wrote: > > This series implements fence support for drm/virtio and > has been tested using qemu, kmscube and the below branches. > > Rob Herring solved a reference counting issue and > suggested a context check for the execbuf ioctl, his > changes have been included in the below commits to > keep the tree working at all commits. > I've put forward some mostly minor suggestions. The patches look pretty good but there's one piece missing: > > The linux series can be found here: > https://gitlab.collabora.com/robertfoss/linux/commits/virtio_fences_v3 > > As for mesa, the branch can be found here: > https://gitlab.collabora.com/robertfoss/mesa/commits/virtio_fences_v3 > Namely: This should be out for review. The kernel and userspace side of IOCTLs should happen roughly at the same time. Otherwise, there's a huge chance of merging one side of the component, while the other needs architectural changes. HTH Emil ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-11-05 6:38 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-25 18:37 [PATCH 0/5] virgl: fence fd support Robert Foss 2018-10-25 18:37 ` [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence() Robert Foss 2018-10-31 9:38 ` Emil Velikov 2018-11-01 12:43 ` Robert Foss 2018-11-05 6:38 ` Gerd Hoffmann 2018-10-25 18:37 ` [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences Robert Foss 2018-10-30 6:11 ` Gerd Hoffmann 2018-10-30 11:31 ` Emil Velikov 2018-10-30 13:52 ` Gerd Hoffmann 2018-10-30 15:48 ` Emil Velikov 2018-10-31 9:38 ` Emil Velikov 2018-11-01 12:56 ` Robert Foss 2018-11-02 13:34 ` Emil Velikov 2018-11-02 14:42 ` Robert Foss 2018-10-25 18:37 ` [PATCH 3/5] drm/virtio: add in-fences support for explicit synchronization Robert Foss 2018-10-31 9:38 ` Emil Velikov 2018-10-25 18:37 ` [PATCH 4/5] drm/virtio: add out-fences " Robert Foss 2018-10-31 9:39 ` Emil Velikov 2018-10-25 18:37 ` [PATCH 5/5] drm/virtio: bump driver version after explicit synchronization addition Robert Foss 2018-10-31 9:39 ` Emil Velikov 2018-10-31 9:45 ` [PATCH 0/5] virgl: fence fd support Emil Velikov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).