[PATCH v2 0/6] drm/virtio: ttm improvements

[PATCH v2 0/6] drm/virtio: ttm improvements

[Gerd Hoffmann]

changes in v2:
 - some cleanup patches are reviewed and merged already, dropped them.
 - more verbose commit messages.

Gerd Hoffmann (6):
  drm/virtio: move virtio_gpu_object_{attach,detach} calls.
  drm/virtio: use struct to pass params to virtio_gpu_object_create()
  drm/virtio: params struct for virtio_gpu_cmd_create_resource()
  drm/virtio: params struct for virtio_gpu_cmd_create_resource_3d()
  drm/virtio: drop fencing in virtio_gpu_resource_create_ioctl
  drm/virtio: move virtio_gpu_cmd_create_resource call into
    virtio_gpu_object_create

 drivers/gpu/drm/virtio/virtgpu_drv.h    | 34 +++++++++---
 drivers/gpu/drm/virtio/virtgpu_gem.c    | 35 +++++-------
 drivers/gpu/drm/virtio/virtgpu_ioctl.c  | 97 +++++++--------------------------
 drivers/gpu/drm/virtio/virtgpu_object.c | 30 +++++-----
 drivers/gpu/drm/virtio/virtgpu_ttm.c    | 92 ++++++++-----------------------
 drivers/gpu/drm/virtio/virtgpu_vq.c     | 30 ++++++----
 6 files changed, 117 insertions(+), 201 deletions(-)

-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/6] drm/virtio: move virtio_gpu_object_{attach, detach} calls.

[Gerd Hoffmann]

Drop the dummy ttm backend implementation, add a real one for
TTM_PL_FLAG_TT objects.  The bin/unbind callbacks will call
virtio_gpu_object_{attach,detach}, to update the object state
on the host side, instead of invoking those calls from the
move_notify() callback.

With that in place the move and move_notify callbacks are not
needed any more, so drop them.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_ttm.c | 92 ++++++++++--------------------------
 1 file changed, 24 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c b/drivers/gpu/drm/virtio/virtgpu_ttm.c
index 4bfbf25fab..77407976c7 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
@@ -194,42 +194,45 @@ static void virtio_gpu_ttm_io_mem_free(struct ttm_bo_device *bdev,
  */
 struct virtio_gpu_ttm_tt {
 	struct ttm_dma_tt		ttm;
-	struct virtio_gpu_device	*vgdev;
-	u64				offset;
+	struct virtio_gpu_object        *obj;
 };
 
-static int virtio_gpu_ttm_backend_bind(struct ttm_tt *ttm,
-				       struct ttm_mem_reg *bo_mem)
+static int virtio_gpu_ttm_tt_bind(struct ttm_tt *ttm,
+				  struct ttm_mem_reg *bo_mem)
 {
-	struct virtio_gpu_ttm_tt *gtt = (void *)ttm;
+	struct virtio_gpu_ttm_tt *gtt =
+		container_of(ttm, struct virtio_gpu_ttm_tt, ttm.ttm);
+	struct virtio_gpu_device *vgdev =
+		virtio_gpu_get_vgdev(gtt->obj->tbo.bdev);
 
-	gtt->offset = (unsigned long)(bo_mem->start << PAGE_SHIFT);
-	if (!ttm->num_pages)
-		WARN(1, "nothing to bind %lu pages for mreg %p back %p!\n",
-		     ttm->num_pages, bo_mem, ttm);
-
-	/* Not implemented */
+	virtio_gpu_object_attach(vgdev, gtt->obj, NULL);
 	return 0;
 }
 
-static int virtio_gpu_ttm_backend_unbind(struct ttm_tt *ttm)
+static int virtio_gpu_ttm_tt_unbind(struct ttm_tt *ttm)
 {
-	/* Not implemented */
+	struct virtio_gpu_ttm_tt *gtt =
+		container_of(ttm, struct virtio_gpu_ttm_tt, ttm.ttm);
+	struct virtio_gpu_device *vgdev =
+		virtio_gpu_get_vgdev(gtt->obj->tbo.bdev);
+
+	virtio_gpu_object_detach(vgdev, gtt->obj);
 	return 0;
 }
 
-static void virtio_gpu_ttm_backend_destroy(struct ttm_tt *ttm)
+static void virtio_gpu_ttm_tt_destroy(struct ttm_tt *ttm)
 {
-	struct virtio_gpu_ttm_tt *gtt = (void *)ttm;
+	struct virtio_gpu_ttm_tt *gtt =
+		container_of(ttm, struct virtio_gpu_ttm_tt, ttm.ttm);
 
 	ttm_dma_tt_fini(&gtt->ttm);
 	kfree(gtt);
 }
 
-static struct ttm_backend_func virtio_gpu_backend_func = {
-	.bind = &virtio_gpu_ttm_backend_bind,
-	.unbind = &virtio_gpu_ttm_backend_unbind,
-	.destroy = &virtio_gpu_ttm_backend_destroy,
+static struct ttm_backend_func virtio_gpu_tt_func = {
+	.bind = &virtio_gpu_ttm_tt_bind,
+	.unbind = &virtio_gpu_ttm_tt_unbind,
+	.destroy = &virtio_gpu_ttm_tt_destroy,
 };
 
 static struct ttm_tt *virtio_gpu_ttm_tt_create(struct ttm_buffer_object *bo,
@@ -242,8 +245,8 @@ static struct ttm_tt *virtio_gpu_ttm_tt_create(struct ttm_buffer_object *bo,
 	gtt = kzalloc(sizeof(struct virtio_gpu_ttm_tt), GFP_KERNEL);
 	if (gtt == NULL)
 		return NULL;
-	gtt->ttm.ttm.func = &virtio_gpu_backend_func;
-	gtt->vgdev = vgdev;
+	gtt->ttm.ttm.func = &virtio_gpu_tt_func;
+	gtt->obj = container_of(bo, struct virtio_gpu_object, tbo);
 	if (ttm_dma_tt_init(&gtt->ttm, bo, page_flags)) {
 		kfree(gtt);
 		return NULL;
@@ -251,51 +254,6 @@ static struct ttm_tt *virtio_gpu_ttm_tt_create(struct ttm_buffer_object *bo,
 	return &gtt->ttm.ttm;
 }
 
-static void virtio_gpu_move_null(struct ttm_buffer_object *bo,
-				 struct ttm_mem_reg *new_mem)
-{
-	struct ttm_mem_reg *old_mem = &bo->mem;
-
-	BUG_ON(old_mem->mm_node != NULL);
-	*old_mem = *new_mem;
-	new_mem->mm_node = NULL;
-}
-
-static int virtio_gpu_bo_move(struct ttm_buffer_object *bo, bool evict,
-			      struct ttm_operation_ctx *ctx,
-			      struct ttm_mem_reg *new_mem)
-{
-	int ret;
-
-	ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
-	if (ret)
-		return ret;
-
-	virtio_gpu_move_null(bo, new_mem);
-	return 0;
-}
-
-static void virtio_gpu_bo_move_notify(struct ttm_buffer_object *tbo,
-				      bool evict,
-				      struct ttm_mem_reg *new_mem)
-{
-	struct virtio_gpu_object *bo;
-	struct virtio_gpu_device *vgdev;
-
-	bo = container_of(tbo, struct virtio_gpu_object, tbo);
-	vgdev = (struct virtio_gpu_device *)bo->gem_base.dev->dev_private;
-
-	if (!new_mem || (new_mem->placement & TTM_PL_FLAG_SYSTEM)) {
-		if (bo->hw_res_handle)
-			virtio_gpu_object_detach(vgdev, bo);
-
-	} else if (new_mem->placement & TTM_PL_FLAG_TT) {
-		if (bo->hw_res_handle) {
-			virtio_gpu_object_attach(vgdev, bo, NULL);
-		}
-	}
-}
-
 static void virtio_gpu_bo_swap_notify(struct ttm_buffer_object *tbo)
 {
 	struct virtio_gpu_object *bo;
@@ -314,11 +272,9 @@ static struct ttm_bo_driver virtio_gpu_bo_driver = {
 	.init_mem_type = &virtio_gpu_init_mem_type,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = &virtio_gpu_evict_flags,
-	.move = &virtio_gpu_bo_move,
 	.verify_access = &virtio_gpu_verify_access,
 	.io_mem_reserve = &virtio_gpu_ttm_io_mem_reserve,
 	.io_mem_free = &virtio_gpu_ttm_io_mem_free,
-	.move_notify = &virtio_gpu_bo_move_notify,
 	.swap_notify = &virtio_gpu_bo_swap_notify,
 };
 
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/6] drm/virtio: move virtio_gpu_object_{attach, detach} calls.

[Noralf Trønnes]

Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Drop the dummy ttm backend implementation, add a real one for
> TTM_PL_FLAG_TT objects.  The bin/unbind callbacks will call
> virtio_gpu_object_{attach,detach}, to update the object state
> on the host side, instead of invoking those calls from the
> move_notify() callback.
> 
> With that in place the move and move_notify callbacks are not
> needed any more, so drop them.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---

Acked-by: Noralf Trønnes <noralf@tronnes.org>

[PATCH v2 2/6] drm/virtio: use struct to pass params to virtio_gpu_object_create()

[Gerd Hoffmann]

Create virtio_gpu_object_params, use that to pass object parameters to
virtio_gpu_object_create.  This is just the first step, followup patches
will add more parameters to the struct.  The plan is to use the struct
for all object parameters.

Also drop unused "kernel" parameter for virtio_gpu_alloc_object(), it is
unused and always false.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h    | 15 ++++++++++-----
 drivers/gpu/drm/virtio/virtgpu_gem.c    | 17 ++++++++++-------
 drivers/gpu/drm/virtio/virtgpu_ioctl.c  | 11 ++++++-----
 drivers/gpu/drm/virtio/virtgpu_object.c | 22 +++++++++-------------
 4 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index d577cb76f5..40928980a2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -50,6 +50,11 @@
 #define DRIVER_MINOR 1
 #define DRIVER_PATCHLEVEL 0
 
+struct virtio_gpu_object_params {
+	unsigned long size;
+	bool pinned;
+};
+
 struct virtio_gpu_object {
 	struct drm_gem_object gem_base;
 	uint32_t hw_res_handle;
@@ -217,16 +222,16 @@ int virtio_gpu_gem_init(struct virtio_gpu_device *vgdev);
 void virtio_gpu_gem_fini(struct virtio_gpu_device *vgdev);
 int virtio_gpu_gem_create(struct drm_file *file,
 			  struct drm_device *dev,
-			  uint64_t size,
+			  struct virtio_gpu_object_params *params,
 			  struct drm_gem_object **obj_p,
 			  uint32_t *handle_p);
 int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
 			       struct drm_file *file);
 void virtio_gpu_gem_object_close(struct drm_gem_object *obj,
 				 struct drm_file *file);
-struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
-						  size_t size, bool kernel,
-						  bool pinned);
+struct virtio_gpu_object*
+virtio_gpu_alloc_object(struct drm_device *dev,
+			struct virtio_gpu_object_params *params);
 int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 				struct drm_device *dev,
 				struct drm_mode_create_dumb *args);
@@ -342,7 +347,7 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vdev,
 
 /* virtio_gpu_object */
 int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
-			     unsigned long size, bool kernel, bool pinned,
+			     struct virtio_gpu_object_params *params,
 			     struct virtio_gpu_object **bo_ptr);
 void virtio_gpu_object_kunmap(struct virtio_gpu_object *bo);
 int virtio_gpu_object_kmap(struct virtio_gpu_object *bo);
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index f065863939..b5f2d94ce5 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -34,15 +34,15 @@ void virtio_gpu_gem_free_object(struct drm_gem_object *gem_obj)
 		virtio_gpu_object_unref(&obj);
 }
 
-struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
-						  size_t size, bool kernel,
-						  bool pinned)
+struct virtio_gpu_object*
+virtio_gpu_alloc_object(struct drm_device *dev,
+			struct virtio_gpu_object_params *params)
 {
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct virtio_gpu_object *obj;
 	int ret;
 
-	ret = virtio_gpu_object_create(vgdev, size, kernel, pinned, &obj);
+	ret = virtio_gpu_object_create(vgdev, params, &obj);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -51,7 +51,7 @@ struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
 
 int virtio_gpu_gem_create(struct drm_file *file,
 			  struct drm_device *dev,
-			  uint64_t size,
+			  struct virtio_gpu_object_params *params,
 			  struct drm_gem_object **obj_p,
 			  uint32_t *handle_p)
 {
@@ -59,7 +59,7 @@ int virtio_gpu_gem_create(struct drm_file *file,
 	int ret;
 	u32 handle;
 
-	obj = virtio_gpu_alloc_object(dev, size, false, false);
+	obj = virtio_gpu_alloc_object(dev, params);
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
@@ -85,6 +85,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct drm_gem_object *gobj;
 	struct virtio_gpu_object *obj;
+	struct virtio_gpu_object_params params = { 0 };
 	int ret;
 	uint32_t pitch;
 	uint32_t format;
@@ -96,7 +97,9 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 	args->size = pitch * args->height;
 	args->size = ALIGN(args->size, PAGE_SIZE);
 
-	ret = virtio_gpu_gem_create(file_priv, dev, args->size, &gobj,
+	params.pinned = false,
+	params.size = args->size;
+	ret = virtio_gpu_gem_create(file_priv, dev, &params, &gobj,
 				    &args->handle);
 	if (ret)
 		goto fail;
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 14ce8188c0..fa7b958ca2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -279,12 +279,12 @@ 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;
-	uint32_t size;
 	struct list_head validate_list;
 	struct ttm_validate_buffer mainbuf;
 	struct virtio_gpu_fence *fence = NULL;
 	struct ww_acquire_ctx ticket;
 	struct virtio_gpu_resource_create_3d rc_3d;
+	struct virtio_gpu_object_params params = { 0 };
 
 	if (vgdev->has_virgl_3d == false) {
 		if (rc->depth > 1)
@@ -302,13 +302,14 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 	INIT_LIST_HEAD(&validate_list);
 	memset(&mainbuf, 0, sizeof(struct ttm_validate_buffer));
 
-	size = rc->size;
+	params.pinned = false,
+	params.size = rc->size;
 
 	/* allocate a single page size object */
-	if (size == 0)
-		size = PAGE_SIZE;
+	if (params.size == 0)
+		params.size = PAGE_SIZE;
 
-	qobj = virtio_gpu_alloc_object(dev, size, false, false);
+	qobj = virtio_gpu_alloc_object(dev, &params);
 	if (IS_ERR(qobj))
 		return PTR_ERR(qobj);
 	obj = &qobj->gem_base;
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index f39a183d59..62367e3f80 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -79,21 +79,16 @@ static void virtio_gpu_init_ttm_placement(struct virtio_gpu_object *vgbo,
 }
 
 int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
-			     unsigned long size, bool kernel, bool pinned,
+			     struct virtio_gpu_object_params *params,
 			     struct virtio_gpu_object **bo_ptr)
 {
 	struct virtio_gpu_object *bo;
-	enum ttm_bo_type type;
 	size_t acc_size;
 	int ret;
 
-	if (kernel)
-		type = ttm_bo_type_kernel;
-	else
-		type = ttm_bo_type_device;
 	*bo_ptr = NULL;
 
-	acc_size = ttm_bo_dma_acc_size(&vgdev->mman.bdev, size,
+	acc_size = ttm_bo_dma_acc_size(&vgdev->mman.bdev, params->size,
 				       sizeof(struct virtio_gpu_object));
 
 	bo = kzalloc(sizeof(struct virtio_gpu_object), GFP_KERNEL);
@@ -104,19 +99,20 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
 		kfree(bo);
 		return ret;
 	}
-	size = roundup(size, PAGE_SIZE);
-	ret = drm_gem_object_init(vgdev->ddev, &bo->gem_base, size);
+	params->size = roundup(params->size, PAGE_SIZE);
+	ret = drm_gem_object_init(vgdev->ddev, &bo->gem_base, params->size);
 	if (ret != 0) {
 		virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
 		kfree(bo);
 		return ret;
 	}
 	bo->dumb = false;
-	virtio_gpu_init_ttm_placement(bo, pinned);
+	virtio_gpu_init_ttm_placement(bo, params->pinned);
 
-	ret = ttm_bo_init(&vgdev->mman.bdev, &bo->tbo, size, type,
-			  &bo->placement, 0, !kernel, acc_size,
-			  NULL, NULL, &virtio_gpu_ttm_bo_destroy);
+	ret = ttm_bo_init(&vgdev->mman.bdev, &bo->tbo, params->size,
+			  ttm_bo_type_device, &bo->placement, 0,
+			  true, acc_size, NULL, NULL,
+			  &virtio_gpu_ttm_bo_destroy);
 	/* ttm_bo_init failure will call the destroy */
 	if (ret != 0)
 		return ret;
-- 
2.9.3

Re: [PATCH v2 2/6] drm/virtio: use struct to pass params to virtio_gpu_object_create()

[Noralf Trønnes]

Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Create virtio_gpu_object_params, use that to pass object parameters to
> virtio_gpu_object_create.  This is just the first step, followup patches
> will add more parameters to the struct.  The plan is to use the struct
> for all object parameters.
> 
> Also drop unused "kernel" parameter for virtio_gpu_alloc_object(), it is
> unused and always false.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h    | 15 ++++++++++-----
>  drivers/gpu/drm/virtio/virtgpu_gem.c    | 17 ++++++++++-------
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c  | 11 ++++++-----
>  drivers/gpu/drm/virtio/virtgpu_object.c | 22 +++++++++-------------
>  4 files changed, 35 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index d577cb76f5..40928980a2 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -50,6 +50,11 @@
>  #define DRIVER_MINOR 1
>  #define DRIVER_PATCHLEVEL 0
>  
> +struct virtio_gpu_object_params {
> +	unsigned long size;
> +	bool pinned;
> +};
> +
>  struct virtio_gpu_object {
>  	struct drm_gem_object gem_base;
>  	uint32_t hw_res_handle;
> @@ -217,16 +222,16 @@ int virtio_gpu_gem_init(struct virtio_gpu_device *vgdev);
>  void virtio_gpu_gem_fini(struct virtio_gpu_device *vgdev);
>  int virtio_gpu_gem_create(struct drm_file *file,
>  			  struct drm_device *dev,
> -			  uint64_t size,
> +			  struct virtio_gpu_object_params *params,
>  			  struct drm_gem_object **obj_p,
>  			  uint32_t *handle_p);
>  int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
>  			       struct drm_file *file);
>  void virtio_gpu_gem_object_close(struct drm_gem_object *obj,
>  				 struct drm_file *file);
> -struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
> -						  size_t size, bool kernel,
> -						  bool pinned);
> +struct virtio_gpu_object*
> +virtio_gpu_alloc_object(struct drm_device *dev,
> +			struct virtio_gpu_object_params *params);
>  int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
>  				struct drm_device *dev,
>  				struct drm_mode_create_dumb *args);
> @@ -342,7 +347,7 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vdev,
>  
>  /* virtio_gpu_object */
>  int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
> -			     unsigned long size, bool kernel, bool pinned,
> +			     struct virtio_gpu_object_params *params,
>  			     struct virtio_gpu_object **bo_ptr);
>  void virtio_gpu_object_kunmap(struct virtio_gpu_object *bo);
>  int virtio_gpu_object_kmap(struct virtio_gpu_object *bo);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index f065863939..b5f2d94ce5 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -34,15 +34,15 @@ void virtio_gpu_gem_free_object(struct drm_gem_object *gem_obj)
>  		virtio_gpu_object_unref(&obj);
>  }
>  
> -struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
> -						  size_t size, bool kernel,
> -						  bool pinned)
> +struct virtio_gpu_object*
> +virtio_gpu_alloc_object(struct drm_device *dev,
> +			struct virtio_gpu_object_params *params)
>  {
>  	struct virtio_gpu_device *vgdev = dev->dev_private;
>  	struct virtio_gpu_object *obj;
>  	int ret;
>  
> -	ret = virtio_gpu_object_create(vgdev, size, kernel, pinned, &obj);
> +	ret = virtio_gpu_object_create(vgdev, params, &obj);
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> @@ -51,7 +51,7 @@ struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
>  
>  int virtio_gpu_gem_create(struct drm_file *file,
>  			  struct drm_device *dev,
> -			  uint64_t size,
> +			  struct virtio_gpu_object_params *params,
>  			  struct drm_gem_object **obj_p,
>  			  uint32_t *handle_p)
>  {
> @@ -59,7 +59,7 @@ int virtio_gpu_gem_create(struct drm_file *file,
>  	int ret;
>  	u32 handle;
>  
> -	obj = virtio_gpu_alloc_object(dev, size, false, false);
> +	obj = virtio_gpu_alloc_object(dev, params);
>  	if (IS_ERR(obj))
>  		return PTR_ERR(obj);
>  
> @@ -85,6 +85,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
>  	struct virtio_gpu_device *vgdev = dev->dev_private;
>  	struct drm_gem_object *gobj;
>  	struct virtio_gpu_object *obj;
> +	struct virtio_gpu_object_params params = { 0 };
>  	int ret;
>  	uint32_t pitch;
>  	uint32_t format;
> @@ -96,7 +97,9 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
>  	args->size = pitch * args->height;
>  	args->size = ALIGN(args->size, PAGE_SIZE);
>  
> -	ret = virtio_gpu_gem_create(file_priv, dev, args->size, &gobj,
> +	params.pinned = false,

You have a comma here, but assigning to false isn't really necessary
since the struct is zeroed. Same goes for the same assignment further down.

With this fixed in some way:

Acked-by: Noralf Trønnes <noralf@tronnes.org>

> +	params.size = args->size;
> +	ret = virtio_gpu_gem_create(file_priv, dev, &params, &gobj,
>  				    &args->handle);
>  	if (ret)
>  		goto fail;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 14ce8188c0..fa7b958ca2 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -279,12 +279,12 @@ 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;
> -	uint32_t size;
>  	struct list_head validate_list;
>  	struct ttm_validate_buffer mainbuf;
>  	struct virtio_gpu_fence *fence = NULL;
>  	struct ww_acquire_ctx ticket;
>  	struct virtio_gpu_resource_create_3d rc_3d;
> +	struct virtio_gpu_object_params params = { 0 };
>  
>  	if (vgdev->has_virgl_3d == false) {
>  		if (rc->depth > 1)
> @@ -302,13 +302,14 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
>  	INIT_LIST_HEAD(&validate_list);
>  	memset(&mainbuf, 0, sizeof(struct ttm_validate_buffer));
>  
> -	size = rc->size;
> +	params.pinned = false,
> +	params.size = rc->size;
>  
>  	/* allocate a single page size object */
> -	if (size == 0)
> -		size = PAGE_SIZE;
> +	if (params.size == 0)
> +		params.size = PAGE_SIZE;
>  
> -	qobj = virtio_gpu_alloc_object(dev, size, false, false);
> +	qobj = virtio_gpu_alloc_object(dev, &params);
>  	if (IS_ERR(qobj))
>  		return PTR_ERR(qobj);
>  	obj = &qobj->gem_base;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
> index f39a183d59..62367e3f80 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -79,21 +79,16 @@ static void virtio_gpu_init_ttm_placement(struct virtio_gpu_object *vgbo,
>  }
>  
>  int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
> -			     unsigned long size, bool kernel, bool pinned,
> +			     struct virtio_gpu_object_params *params,
>  			     struct virtio_gpu_object **bo_ptr)
>  {
>  	struct virtio_gpu_object *bo;
> -	enum ttm_bo_type type;
>  	size_t acc_size;
>  	int ret;
>  
> -	if (kernel)
> -		type = ttm_bo_type_kernel;
> -	else
> -		type = ttm_bo_type_device;
>  	*bo_ptr = NULL;
>  
> -	acc_size = ttm_bo_dma_acc_size(&vgdev->mman.bdev, size,
> +	acc_size = ttm_bo_dma_acc_size(&vgdev->mman.bdev, params->size,
>  				       sizeof(struct virtio_gpu_object));
>  
>  	bo = kzalloc(sizeof(struct virtio_gpu_object), GFP_KERNEL);
> @@ -104,19 +99,20 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
>  		kfree(bo);
>  		return ret;
>  	}
> -	size = roundup(size, PAGE_SIZE);
> -	ret = drm_gem_object_init(vgdev->ddev, &bo->gem_base, size);
> +	params->size = roundup(params->size, PAGE_SIZE);
> +	ret = drm_gem_object_init(vgdev->ddev, &bo->gem_base, params->size);
>  	if (ret != 0) {
>  		virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
>  		kfree(bo);
>  		return ret;
>  	}
>  	bo->dumb = false;
> -	virtio_gpu_init_ttm_placement(bo, pinned);
> +	virtio_gpu_init_ttm_placement(bo, params->pinned);
>  
> -	ret = ttm_bo_init(&vgdev->mman.bdev, &bo->tbo, size, type,
> -			  &bo->placement, 0, !kernel, acc_size,
> -			  NULL, NULL, &virtio_gpu_ttm_bo_destroy);
> +	ret = ttm_bo_init(&vgdev->mman.bdev, &bo->tbo, params->size,
> +			  ttm_bo_type_device, &bo->placement, 0,
> +			  true, acc_size, NULL, NULL,
> +			  &virtio_gpu_ttm_bo_destroy);
>  	/* ttm_bo_init failure will call the destroy */
>  	if (ret != 0)
>  		return ret;
> 

Re: [PATCH v2 2/6] drm/virtio: use struct to pass params to virtio_gpu_object_create()

[Gerd Hoffmann]

> > -	ret = virtio_gpu_gem_create(file_priv, dev, args->size, &gobj,
> > +	params.pinned = false,
> 
> You have a comma here, but assigning to false isn't really necessary
> since the struct is zeroed. Same goes for the same assignment further down.

Hmm, yes, but it likewise isn't used, so I think I can just scratch it
altogether.

It's also wrong, virtio-gpu objects don't move around, so they are all
pinned.  Not that this bug changes much in practice given virtio-gpu
supports a single kind of storage only, so there is no opportunity for
ttm to try move objects from one to another.  I'll fix it nevertheless
in v3.

cheers,
  Gerd

[PATCH v2 3/6] drm/virtio: params struct for virtio_gpu_cmd_create_resource()

[Gerd Hoffmann]

Add format, width and height fields to the virtio_gpu_object_params
struct.  With that in place we can use the parameter struct for
virtio_gpu_cmd_create_resource() calls too.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  7 ++++---
 drivers/gpu/drm/virtio/virtgpu_gem.c   |  8 ++++----
 drivers/gpu/drm/virtio/virtgpu_ioctl.c |  6 ++++--
 drivers/gpu/drm/virtio/virtgpu_vq.c    | 10 ++++------
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 40928980a2..a40215c10e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -51,6 +51,9 @@
 #define DRIVER_PATCHLEVEL 0
 
 struct virtio_gpu_object_params {
+	uint32_t format;
+	uint32_t width;
+	uint32_t height;
 	unsigned long size;
 	bool pinned;
 };
@@ -248,9 +251,7 @@ int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
 void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev);
 void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
 				    struct virtio_gpu_object *bo,
-				    uint32_t format,
-				    uint32_t width,
-				    uint32_t height);
+				    struct virtio_gpu_object_params *params);
 void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
 				   uint32_t resource_id);
 void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index b5f2d94ce5..3a63ffcd4b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -88,7 +88,6 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 	struct virtio_gpu_object_params params = { 0 };
 	int ret;
 	uint32_t pitch;
-	uint32_t format;
 
 	if (args->bpp != 32)
 		return -EINVAL;
@@ -98,16 +97,17 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 	args->size = ALIGN(args->size, PAGE_SIZE);
 
 	params.pinned = false,
+	params.format = virtio_gpu_translate_format(DRM_FORMAT_HOST_XRGB8888);
+	params.width = args->width;
+	params.height = args->height;
 	params.size = args->size;
 	ret = virtio_gpu_gem_create(file_priv, dev, &params, &gobj,
 				    &args->handle);
 	if (ret)
 		goto fail;
 
-	format = virtio_gpu_translate_format(DRM_FORMAT_HOST_XRGB8888);
 	obj = gem_to_virtio_gpu_obj(gobj);
-	virtio_gpu_cmd_create_resource(vgdev, obj, format,
-				       args->width, args->height);
+	virtio_gpu_cmd_create_resource(vgdev, obj, &params);
 
 	/* attach the object to the resource */
 	ret = virtio_gpu_object_attach(vgdev, obj, NULL);
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index fa7b958ca2..84c2216fd4 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -303,6 +303,9 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 	memset(&mainbuf, 0, sizeof(struct ttm_validate_buffer));
 
 	params.pinned = false,
+	params.format = rc->format;
+	params.width = rc->width;
+	params.height = rc->height;
 	params.size = rc->size;
 
 	/* allocate a single page size object */
@@ -315,8 +318,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 	obj = &qobj->gem_base;
 
 	if (!vgdev->has_virgl_3d) {
-		virtio_gpu_cmd_create_resource(vgdev, qobj, rc->format,
-					       rc->width, rc->height);
+		virtio_gpu_cmd_create_resource(vgdev, qobj, &params);
 
 		ret = virtio_gpu_object_attach(vgdev, qobj, NULL);
 	} else {
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 6bc2008b0d..363b8b8577 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -376,9 +376,7 @@ static int virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
 /* create a basic resource */
 void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
 				    struct virtio_gpu_object *bo,
-				    uint32_t format,
-				    uint32_t width,
-				    uint32_t height)
+				    struct virtio_gpu_object_params *params)
 {
 	struct virtio_gpu_resource_create_2d *cmd_p;
 	struct virtio_gpu_vbuffer *vbuf;
@@ -388,9 +386,9 @@ void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
 
 	cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_RESOURCE_CREATE_2D);
 	cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle);
-	cmd_p->format = cpu_to_le32(format);
-	cmd_p->width = cpu_to_le32(width);
-	cmd_p->height = cpu_to_le32(height);
+	cmd_p->format = cpu_to_le32(params->format);
+	cmd_p->width = cpu_to_le32(params->width);
+	cmd_p->height = cpu_to_le32(params->height);
 
 	virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
 	bo->created = true;
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/6] drm/virtio: params struct for virtio_gpu_cmd_create_resource()

[Noralf Trønnes]

Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Add format, width and height fields to the virtio_gpu_object_params
> struct.  With that in place we can use the parameter struct for
> virtio_gpu_cmd_create_resource() calls too.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---

Acked-by: Noralf Trønnes <noralf@tronnes.org>

[PATCH v2 4/6] drm/virtio: params struct for virtio_gpu_cmd_create_resource_3d()

[Gerd Hoffmann]

Add 3d resource parameters to virtio_gpu_object_params struct.  With
that in place we can use it for virtio_gpu_cmd_resource_create_3d()
calls.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   | 10 +++++++++-
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 25 ++++++++++---------------
 drivers/gpu/drm/virtio/virtgpu_vq.c    | 16 +++++++++++++---
 3 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index a40215c10e..3265e62725 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -56,6 +56,14 @@ struct virtio_gpu_object_params {
 	uint32_t height;
 	unsigned long size;
 	bool pinned;
+	/* 3d */
+	uint32_t target;
+	uint32_t bind;
+	uint32_t depth;
+	uint32_t array_size;
+	uint32_t last_level;
+	uint32_t nr_samples;
+	uint32_t flags;
 };
 
 struct virtio_gpu_object {
@@ -310,7 +318,7 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
 void
 virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev,
 				  struct virtio_gpu_object *bo,
-				  struct virtio_gpu_resource_create_3d *rc_3d);
+				  struct virtio_gpu_object_params *params);
 void virtio_gpu_ctrl_ack(struct virtqueue *vq);
 void virtio_gpu_cursor_ack(struct virtqueue *vq);
 void virtio_gpu_fence_ack(struct virtqueue *vq);
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 84c2216fd4..431e5d767e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -283,7 +283,6 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 	struct ttm_validate_buffer mainbuf;
 	struct virtio_gpu_fence *fence = NULL;
 	struct ww_acquire_ctx ticket;
-	struct virtio_gpu_resource_create_3d rc_3d;
 	struct virtio_gpu_object_params params = { 0 };
 
 	if (vgdev->has_virgl_3d == false) {
@@ -307,7 +306,15 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 	params.width = rc->width;
 	params.height = rc->height;
 	params.size = rc->size;
-
+	if (vgdev->has_virgl_3d) {
+		params.target = rc->target;
+		params.bind = rc->bind;
+		params.depth = rc->depth;
+		params.array_size = rc->array_size;
+		params.last_level = rc->last_level;
+		params.nr_samples = rc->nr_samples;
+		params.flags = rc->flags;
+	}
 	/* allocate a single page size object */
 	if (params.size == 0)
 		params.size = PAGE_SIZE;
@@ -333,25 +340,13 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 			goto fail_unref;
 		}
 
-		rc_3d.resource_id = cpu_to_le32(qobj->hw_res_handle);
-		rc_3d.target = cpu_to_le32(rc->target);
-		rc_3d.format = cpu_to_le32(rc->format);
-		rc_3d.bind = cpu_to_le32(rc->bind);
-		rc_3d.width = cpu_to_le32(rc->width);
-		rc_3d.height = cpu_to_le32(rc->height);
-		rc_3d.depth = cpu_to_le32(rc->depth);
-		rc_3d.array_size = cpu_to_le32(rc->array_size);
-		rc_3d.last_level = cpu_to_le32(rc->last_level);
-		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_backoff;
 		}
 
-		virtio_gpu_cmd_resource_create_3d(vgdev, qobj, &rc_3d);
+		virtio_gpu_cmd_resource_create_3d(vgdev, qobj, &params);
 		ret = virtio_gpu_object_attach(vgdev, qobj, fence);
 		if (ret) {
 			dma_fence_put(&fence->f);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 363b8b8577..ca93ec6ca3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -826,7 +826,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_object *bo,
-				  struct virtio_gpu_resource_create_3d *rc_3d)
+				  struct virtio_gpu_object_params *params)
 {
 	struct virtio_gpu_resource_create_3d *cmd_p;
 	struct virtio_gpu_vbuffer *vbuf;
@@ -834,9 +834,19 @@ virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev,
 	cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
 	memset(cmd_p, 0, sizeof(*cmd_p));
 
-	*cmd_p = *rc_3d;
 	cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_RESOURCE_CREATE_3D);
-	cmd_p->hdr.flags = 0;
+	cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle);
+	cmd_p->format = cpu_to_le32(params->format);
+	cmd_p->width = cpu_to_le32(params->width);
+	cmd_p->height = cpu_to_le32(params->height);
+
+	cmd_p->target = cpu_to_le32(params->target);
+	cmd_p->bind = cpu_to_le32(params->bind);
+	cmd_p->depth = cpu_to_le32(params->depth);
+	cmd_p->array_size = cpu_to_le32(params->array_size);
+	cmd_p->last_level = cpu_to_le32(params->last_level);
+	cmd_p->nr_samples = cpu_to_le32(params->nr_samples);
+	cmd_p->flags = cpu_to_le32(params->flags);
 
 	virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
 	bo->created = true;
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/6] drm/virtio: params struct for virtio_gpu_cmd_create_resource_3d()

[Noralf Trønnes]

Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Add 3d resource parameters to virtio_gpu_object_params struct.  With
> that in place we can use it for virtio_gpu_cmd_resource_create_3d()
> calls.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---

You don't remove the struct virtio_gpu_resource_create_3d definition,
but it looks like there's no users left?

Noralf.

>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 10 +++++++++-
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 25 ++++++++++---------------
>  drivers/gpu/drm/virtio/virtgpu_vq.c    | 16 +++++++++++++---
>  3 files changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index a40215c10e..3265e62725 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -56,6 +56,14 @@ struct virtio_gpu_object_params {
>  	uint32_t height;
>  	unsigned long size;
>  	bool pinned;
> +	/* 3d */
> +	uint32_t target;
> +	uint32_t bind;
> +	uint32_t depth;
> +	uint32_t array_size;
> +	uint32_t last_level;
> +	uint32_t nr_samples;
> +	uint32_t flags;
>  };
>  
>  struct virtio_gpu_object {
> @@ -310,7 +318,7 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
>  void
>  virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev,
>  				  struct virtio_gpu_object *bo,
> -				  struct virtio_gpu_resource_create_3d *rc_3d);
> +				  struct virtio_gpu_object_params *params);
>  void virtio_gpu_ctrl_ack(struct virtqueue *vq);
>  void virtio_gpu_cursor_ack(struct virtqueue *vq);
>  void virtio_gpu_fence_ack(struct virtqueue *vq);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 84c2216fd4..431e5d767e 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -283,7 +283,6 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
>  	struct ttm_validate_buffer mainbuf;
>  	struct virtio_gpu_fence *fence = NULL;
>  	struct ww_acquire_ctx ticket;
> -	struct virtio_gpu_resource_create_3d rc_3d;
>  	struct virtio_gpu_object_params params = { 0 };
>  
>  	if (vgdev->has_virgl_3d == false) {
> @@ -307,7 +306,15 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
>  	params.width = rc->width;
>  	params.height = rc->height;
>  	params.size = rc->size;
> -
> +	if (vgdev->has_virgl_3d) {
> +		params.target = rc->target;
> +		params.bind = rc->bind;
> +		params.depth = rc->depth;
> +		params.array_size = rc->array_size;
> +		params.last_level = rc->last_level;
> +		params.nr_samples = rc->nr_samples;
> +		params.flags = rc->flags;
> +	}
>  	/* allocate a single page size object */
>  	if (params.size == 0)
>  		params.size = PAGE_SIZE;
> @@ -333,25 +340,13 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
>  			goto fail_unref;
>  		}
>  
> -		rc_3d.resource_id = cpu_to_le32(qobj->hw_res_handle);
> -		rc_3d.target = cpu_to_le32(rc->target);
> -		rc_3d.format = cpu_to_le32(rc->format);
> -		rc_3d.bind = cpu_to_le32(rc->bind);
> -		rc_3d.width = cpu_to_le32(rc->width);
> -		rc_3d.height = cpu_to_le32(rc->height);
> -		rc_3d.depth = cpu_to_le32(rc->depth);
> -		rc_3d.array_size = cpu_to_le32(rc->array_size);
> -		rc_3d.last_level = cpu_to_le32(rc->last_level);
> -		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_backoff;
>  		}
>  
> -		virtio_gpu_cmd_resource_create_3d(vgdev, qobj, &rc_3d);
> +		virtio_gpu_cmd_resource_create_3d(vgdev, qobj, &params);
>  		ret = virtio_gpu_object_attach(vgdev, qobj, fence);
>  		if (ret) {
>  			dma_fence_put(&fence->f);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 363b8b8577..ca93ec6ca3 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -826,7 +826,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_object *bo,
> -				  struct virtio_gpu_resource_create_3d *rc_3d)
> +				  struct virtio_gpu_object_params *params)
>  {
>  	struct virtio_gpu_resource_create_3d *cmd_p;
>  	struct virtio_gpu_vbuffer *vbuf;
> @@ -834,9 +834,19 @@ virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev,
>  	cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
>  	memset(cmd_p, 0, sizeof(*cmd_p));
>  
> -	*cmd_p = *rc_3d;
>  	cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_RESOURCE_CREATE_3D);
> -	cmd_p->hdr.flags = 0;
> +	cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle);
> +	cmd_p->format = cpu_to_le32(params->format);
> +	cmd_p->width = cpu_to_le32(params->width);
> +	cmd_p->height = cpu_to_le32(params->height);
> +
> +	cmd_p->target = cpu_to_le32(params->target);
> +	cmd_p->bind = cpu_to_le32(params->bind);
> +	cmd_p->depth = cpu_to_le32(params->depth);
> +	cmd_p->array_size = cpu_to_le32(params->array_size);
> +	cmd_p->last_level = cpu_to_le32(params->last_level);
> +	cmd_p->nr_samples = cpu_to_le32(params->nr_samples);
> +	cmd_p->flags = cpu_to_le32(params->flags);
>  
>  	virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
>  	bo->created = true;
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/6] drm/virtio: params struct for virtio_gpu_cmd_create_resource_3d()

[Gerd Hoffmann]

On Thu, Jan 31, 2019 at 11:47:38AM +0100, Noralf Trønnes wrote:
> 
> 
> Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> > Add 3d resource parameters to virtio_gpu_object_params struct.  With
> > that in place we can use it for virtio_gpu_cmd_resource_create_3d()
> > calls.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> 
> You don't remove the struct virtio_gpu_resource_create_3d definition,
> but it looks like there's no users left?

virtio_gpu_cmd_resource_create_3d() still uses it (and has to, it is
part of the guest <-> host protocol).

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/6] drm/virtio: params struct for virtio_gpu_cmd_create_resource_3d()

[Noralf Trønnes]

Den 01.02.2019 09.01, skrev Gerd Hoffmann:
> On Thu, Jan 31, 2019 at 11:47:38AM +0100, Noralf Trønnes wrote:
>>
>>
>> Den 30.01.2019 10.43, skrev Gerd Hoffmann:
>>> Add 3d resource parameters to virtio_gpu_object_params struct.  With
>>> that in place we can use it for virtio_gpu_cmd_resource_create_3d()
>>> calls.
>>>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>
>> You don't remove the struct virtio_gpu_resource_create_3d definition,
>> but it looks like there's no users left?
> 
> virtio_gpu_cmd_resource_create_3d() still uses it (and has to, it is
> part of the guest <-> host protocol).
> 

Acked-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Gerd Hoffmann]

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.

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@redhat.com>
---
 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

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

[Noralf Trønnes]

Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> 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.
> 
> 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@redhat.com>
> ---

Acked-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Gurchetan Singh]

On Wed, Jan 30, 2019 at 1:43 AM Gerd Hoffmann <kraxel@redhat.com> 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@redhat.com>
> ---
>  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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Gerd Hoffmann]

On Thu, Jan 31, 2019 at 11:04:31AM -0800, Gurchetan Singh wrote:
> On Wed, Jan 30, 2019 at 1:43 AM Gerd Hoffmann <kraxel@redhat.com> 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?

No.

But you pass in a list of objects (drm_virtgpu_execbuffer->bo_handles)
used.  They will all get reserved, so you can use DRM_IOCTL_VIRTGPU_WAIT
on any of these objects to wait for completion.

Recently the driver got support for returning a fence fd for the
execbuffer, which you can also use to wait for completion in case your
kernel is new enough.

> 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

Oh.  /me looks surprised.
Wasn't aware that userspace is doing *that*.

> Does this change modify this sequence of events?

Yes.  DRM_IOCTL_VIRTGPU_WAIT will not wait any more.  Guess I have to
scratch the patch then ...

cheers,
  Gerd

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

[Gerd Hoffmann]

  Hi,

> 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

After a long break (sidetracked with other stuff) I've finally sent v3
which should address this.

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 6/6] drm/virtio: move virtio_gpu_cmd_create_resource call into virtio_gpu_object_create

[Gerd Hoffmann]

Specifically call virtio_gpu_object_create() before ttm_bo_init(), so
the object is already created when ttm calls the
virtio_gpu_ttm_tt_bind() callback (which in turn calls
virtio_gpu_object_attach()).

With that in place virtio_gpu_object_attach() will never be called with
an object which is not yet created, so the extra
virtio_gpu_object_attach() calls done after
virtio_gpu_cmd_create_resource() is not needed any more.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h    |  2 ++
 drivers/gpu/drm/virtio/virtgpu_gem.c    | 12 +-----------
 drivers/gpu/drm/virtio/virtgpu_ioctl.c  | 10 +---------
 drivers/gpu/drm/virtio/virtgpu_object.c | 10 ++++++++--
 drivers/gpu/drm/virtio/virtgpu_vq.c     |  4 ++--
 5 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 3265e62725..52f3950f82 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -56,7 +56,9 @@ struct virtio_gpu_object_params {
 	uint32_t height;
 	unsigned long size;
 	bool pinned;
+	bool dumb;
 	/* 3d */
+	bool virgl;
 	uint32_t target;
 	uint32_t bind;
 	uint32_t depth;
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 3a63ffcd4b..b5d7df17ac 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -82,9 +82,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 				struct drm_device *dev,
 				struct drm_mode_create_dumb *args)
 {
-	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct drm_gem_object *gobj;
-	struct virtio_gpu_object *obj;
 	struct virtio_gpu_object_params params = { 0 };
 	int ret;
 	uint32_t pitch;
@@ -101,20 +99,12 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 	params.width = args->width;
 	params.height = args->height;
 	params.size = args->size;
+	params.dumb = true;
 	ret = virtio_gpu_gem_create(file_priv, dev, &params, &gobj,
 				    &args->handle);
 	if (ret)
 		goto fail;
 
-	obj = gem_to_virtio_gpu_obj(gobj);
-	virtio_gpu_cmd_create_resource(vgdev, obj, &params);
-
-	/* attach the object to the resource */
-	ret = virtio_gpu_object_attach(vgdev, obj, NULL);
-	if (ret)
-		goto fail;
-
-	obj->dumb = true;
 	args->pitch = pitch;
 	return ret;
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index da06ebbb3a..3a1c447098 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -300,6 +300,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 	params.height = rc->height;
 	params.size = rc->size;
 	if (vgdev->has_virgl_3d) {
+		params.virgl = true;
 		params.target = rc->target;
 		params.bind = rc->bind;
 		params.depth = rc->depth;
@@ -317,15 +318,6 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 		return PTR_ERR(qobj);
 	obj = &qobj->gem_base;
 
-	if (!vgdev->has_virgl_3d) {
-		virtio_gpu_cmd_create_resource(vgdev, qobj, &params);
-
-		ret = virtio_gpu_object_attach(vgdev, qobj, NULL);
-	} else {
-		virtio_gpu_cmd_resource_create_3d(vgdev, qobj, &params);
-		ret = virtio_gpu_object_attach(vgdev, qobj, NULL);
-	}
-
 	ret = drm_gem_handle_create(file_priv, obj, &handle);
 	if (ret) {
 		drm_gem_object_release(obj);
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 62367e3f80..94da9e68d2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -106,9 +106,15 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
 		kfree(bo);
 		return ret;
 	}
-	bo->dumb = false;
+	bo->dumb = params->dumb;
+
+	if (params->virgl) {
+		virtio_gpu_cmd_resource_create_3d(vgdev, bo, params);
+	} else {
+		virtio_gpu_cmd_create_resource(vgdev, bo, params);
+	}
+
 	virtio_gpu_init_ttm_placement(bo, params->pinned);
-
 	ret = ttm_bo_init(&vgdev->mman.bdev, &bo->tbo, params->size,
 			  ttm_bo_type_device, &bo->placement, 0,
 			  true, acc_size, NULL, NULL,
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index ca93ec6ca3..292663c192 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -932,8 +932,8 @@ int virtio_gpu_object_attach(struct virtio_gpu_device *vgdev,
 	struct scatterlist *sg;
 	int si, nents;
 
-	if (!obj->created)
-		return 0;
+	if (WARN_ON_ONCE(!obj->created))
+		return -EINVAL;
 
 	if (!obj->pages) {
 		int ret;
-- 
2.9.3

Re: [PATCH v2 6/6] drm/virtio: move virtio_gpu_cmd_create_resource call into virtio_gpu_object_create

[Noralf Trønnes]

Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Specifically call virtio_gpu_object_create() before ttm_bo_init(), so
> the object is already created when ttm calls the
> virtio_gpu_ttm_tt_bind() callback (which in turn calls
> virtio_gpu_object_attach()).
> 
> With that in place virtio_gpu_object_attach() will never be called with
> an object which is not yet created, so the extra
> virtio_gpu_object_attach() calls done after
> virtio_gpu_cmd_create_resource() is not needed any more.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---

Acked-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel