On Tue, Jun 18, 2019 at 03:58:15PM +0200, Gerd Hoffmann wrote:
> Use gem reservation helpers and direct reservation_object_* calls
> instead of ttm.
>
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> ---
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 36 ++++++++++++--------------
> 1 file changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 5cffd2e54c04..6db6a6e92dde 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -107,12 +107,11 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> struct virtio_gpu_fpriv *vfpriv = drm_file->driver_priv;
> struct drm_gem_object *gobj;
> struct virtio_gpu_fence *out_fence;
> - struct virtio_gpu_object *qobj;
> int ret;
> uint32_t *bo_handles = NULL;
> void __user *user_bo_handles = NULL;
> struct list_head validate_list;
> - struct ttm_validate_buffer *buflist = NULL;
> + struct drm_gem_object **buflist = NULL;
> int i;
> struct ww_acquire_ctx ticket;
> struct sync_file *sync_file;
> @@ -157,12 +156,11 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
>
> INIT_LIST_HEAD(&validate_list);
> if (exbuf->num_bo_handles) {
> -
> bo_handles = kvmalloc_array(exbuf->num_bo_handles,
> - sizeof(uint32_t), GFP_KERNEL);
> + sizeof(uint32_t), GFP_KERNEL);
> buflist = kvmalloc_array(exbuf->num_bo_handles,
> - sizeof(struct ttm_validate_buffer),
> - GFP_KERNEL | __GFP_ZERO);
> + sizeof(struct drm_gem_object*),
> + GFP_KERNEL | __GFP_ZERO);
> if (!bo_handles || !buflist) {
> ret = -ENOMEM;
> goto out_unused_fd;
> @@ -181,19 +179,15 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> ret = -ENOENT;
> goto out_unused_fd;
> }
> -
> - qobj = gem_to_virtio_gpu_obj(gobj);
> - buflist[i].bo = &qobj->tbo;
> -
> - list_add(&buflist[i].head, &validate_list);
> + buflist[i] = gobj;
I didn't bother looking, but I guess there's some room for a
array-of-gem-id to gem_bo-pointers helper function? Would only make sense
if we can share it with panfrost/v3d maybe.
> }
> kvfree(bo_handles);
> bo_handles = NULL;
> }
>
> - ret = virtio_gpu_object_list_validate(&ticket, &validate_list);
> + ret = drm_gem_lock_reservations(buflist, exbuf->num_bo_handles, &ticket);
> if (ret)
> - goto out_free;
> + goto out_unused_fd;
>
> buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
> if (IS_ERR(buf)) {
> @@ -222,21 +216,25 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> virtio_gpu_cmd_submit(vgdev, buf, exbuf->size,
> vfpriv->ctx_id, out_fence);
>
> - ttm_eu_fence_buffer_objects(&ticket, &validate_list, &out_fence->f);
> + for (i = 0; i < exbuf->num_bo_handles; i++)
> + reservation_object_add_excl_fence(buflist[i]->resv, &out_fence->f);
Helper to unref an array of gem bo? Probably also needed by other drivers.
> + drm_gem_unlock_reservations(buflist, exbuf->num_bo_handles, &ticket);
>
> - /* fence the command bo */
> - virtio_gpu_unref_list(&validate_list);
> + for (i = 0; i < exbuf->num_bo_handles; i++)
> + if (buflist[i])
> + drm_gem_object_put_unlocked(buflist[i]);
I am left wondering who's holding references onto these buffers now? How
do you make sure they don't disappear untimely?
I think atm it's ttm making sure of that, but if you drop that completely
there needs to be something else.
> kvfree(buflist);
> return 0;
>
> out_memdup:
> kfree(buf);
> out_unresv:
> - ttm_eu_backoff_reservation(&ticket, &validate_list);
> -out_free:
> - virtio_gpu_unref_list(&validate_list);
> + drm_gem_unlock_reservations(buflist, exbuf->num_bo_handles, &ticket);
> out_unused_fd:
> kvfree(bo_handles);
> + for (i = 0; i < exbuf->num_bo_handles; i++)
> + if (buflist && buflist[i])
> + drm_gem_object_put_unlocked(buflist[i]);
> kvfree(buflist);
>
> if (out_fence_fd >= 0)
Patch itself looks correct, just put down my thoughts while reviewing it.
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
