[PATCH 1/4 v6] drm/virtio: use consistent names for drm_files

[PATCH 1/4 v6] drm/virtio: use consistent names for drm_files

[Gurchetan Singh]

Minor cleanup, change:

- file_priv--> file,
- drm_file --> file.

Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index bbc31aef51f1..baad7e1c9505 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -34,12 +34,12 @@
 #include "virtgpu_drv.h"
 
 static int virtio_gpu_map_ioctl(struct drm_device *dev, void *data,
-				struct drm_file *file_priv)
+				struct drm_file *file)
 {
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct drm_virtgpu_map *virtio_gpu_map = data;
 
-	return virtio_gpu_mode_dumb_mmap(file_priv, vgdev->ddev,
+	return virtio_gpu_mode_dumb_mmap(file, vgdev->ddev,
 					 virtio_gpu_map->handle,
 					 &virtio_gpu_map->offset);
 }
@@ -51,11 +51,11 @@ static int virtio_gpu_map_ioctl(struct drm_device *dev, void *data,
  * VIRTIO_GPUReleaseInfo struct (first XXX bytes)
  */
 static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
-				 struct drm_file *drm_file)
+				 struct drm_file *file)
 {
 	struct drm_virtgpu_execbuffer *exbuf = data;
 	struct virtio_gpu_device *vgdev = dev->dev_private;
-	struct virtio_gpu_fpriv *vfpriv = drm_file->driver_priv;
+	struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
 	struct virtio_gpu_fence *out_fence;
 	int ret;
 	uint32_t *bo_handles = NULL;
@@ -116,7 +116,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
 			goto out_unused_fd;
 		}
 
-		buflist = virtio_gpu_array_from_handles(drm_file, bo_handles,
+		buflist = virtio_gpu_array_from_handles(file, bo_handles,
 							exbuf->num_bo_handles);
 		if (!buflist) {
 			ret = -ENOENT;
@@ -178,7 +178,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
 }
 
 static int virtio_gpu_getparam_ioctl(struct drm_device *dev, void *data,
-				     struct drm_file *file_priv)
+				     struct drm_file *file)
 {
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct drm_virtgpu_getparam *param = data;
@@ -201,7 +201,7 @@ static int virtio_gpu_getparam_ioctl(struct drm_device *dev, void *data,
 }
 
 static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
-					    struct drm_file *file_priv)
+					    struct drm_file *file)
 {
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct drm_virtgpu_resource_create *rc = data;
@@ -252,7 +252,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 		return ret;
 	obj = &qobj->base.base;
 
-	ret = drm_gem_handle_create(file_priv, obj, &handle);
+	ret = drm_gem_handle_create(file, obj, &handle);
 	if (ret) {
 		drm_gem_object_release(obj);
 		return ret;
@@ -265,13 +265,13 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 }
 
 static int virtio_gpu_resource_info_ioctl(struct drm_device *dev, void *data,
-					  struct drm_file *file_priv)
+					  struct drm_file *file)
 {
 	struct drm_virtgpu_resource_info *ri = data;
 	struct drm_gem_object *gobj = NULL;
 	struct virtio_gpu_object *qobj = NULL;
 
-	gobj = drm_gem_object_lookup(file_priv, ri->bo_handle);
+	gobj = drm_gem_object_lookup(file, ri->bo_handle);
 	if (gobj == NULL)
 		return -ENOENT;
 
-- 
2.25.0.265.gbab2e86ba0-goog

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

[PATCH 2/4 v6] drm/virtio: factor out context create hypercall

[Gurchetan Singh]

We currently create an OpenGL context when opening the DRM fd
if 3D is available.

We may need other context types (VK,..) in the future, and the plan
is to have explicit initialization for that.

For explicit initialization to work, we need to factor out
virtio_gpu_create_context from driver initialization.

v2: Move context handle initialization too (olv)
v6: Remove redundant 3D check (emil.velikov)

Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  2 ++
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 13 +++++++++++++
 drivers/gpu/drm/virtio/virtgpu_kms.c   | 26 ++++++--------------------
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 2f6c4ccbfd14..72c1d9b59dfe 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -214,6 +214,8 @@ struct virtio_gpu_fpriv {
 /* virtio_ioctl.c */
 #define DRM_VIRTIO_NUM_IOCTLS 10
 extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
+void virtio_gpu_create_context(struct drm_device *dev,
+			       struct drm_file *file);
 
 /* virtio_kms.c */
 int virtio_gpu_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index baad7e1c9505..00ef9fd3fbf6 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -33,6 +33,19 @@
 
 #include "virtgpu_drv.h"
 
+void virtio_gpu_create_context(struct drm_device *dev,
+			       struct drm_file *file)
+{
+	struct virtio_gpu_device *vgdev = dev->dev_private;
+	struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
+	char dbgname[TASK_COMM_LEN];
+
+	get_task_comm(dbgname, current);
+	virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
+				      strlen(dbgname), dbgname);
+	virtio_gpu_notify(vgdev);
+}
+
 static int virtio_gpu_map_ioctl(struct drm_device *dev, void *data,
 				struct drm_file *file)
 {
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index ad3b673f5796..f7e3712502ca 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -52,19 +52,6 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
 		      events_clear, &events_clear);
 }
 
-static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
-				      uint32_t nlen, const char *name)
-{
-	int handle = ida_alloc(&vgdev->ctx_id_ida, GFP_KERNEL);
-
-	if (handle < 0)
-		return handle;
-	handle += 1;
-	virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
-	virtio_gpu_notify(vgdev);
-	return handle;
-}
-
 static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev,
 				      uint32_t ctx_id)
 {
@@ -260,8 +247,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
 {
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct virtio_gpu_fpriv *vfpriv;
-	int id;
-	char dbgname[TASK_COMM_LEN];
+	int handle;
 
 	/* can't create contexts without 3d renderer */
 	if (!vgdev->has_virgl_3d)
@@ -272,15 +258,15 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
 	if (!vfpriv)
 		return -ENOMEM;
 
-	get_task_comm(dbgname, current);
-	id = virtio_gpu_context_create(vgdev, strlen(dbgname), dbgname);
-	if (id < 0) {
+	handle = ida_alloc(&vgdev->ctx_id_ida, GFP_KERNEL);
+	if (handle < 0) {
 		kfree(vfpriv);
-		return id;
+		return handle;
 	}
 
-	vfpriv->ctx_id = id;
+	vfpriv->ctx_id = handle + 1;
 	file->driver_priv = vfpriv;
+	virtio_gpu_create_context(dev, file);
 	return 0;
 }
 
-- 
2.25.0.265.gbab2e86ba0-goog

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

[PATCH 3/4 v6] drm/virtio: track whether or not a context has been initiated

[Gurchetan Singh]

Use an boolean variable to track whether a context has been
initiated.

v5: Fix possible race and sleep via mutex (olv)

Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   | 2 ++
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 8 ++++++++
 drivers/gpu/drm/virtio/virtgpu_kms.c   | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 72c1d9b59dfe..76b7b7c30e10 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -209,6 +209,8 @@ struct virtio_gpu_device {
 
 struct virtio_gpu_fpriv {
 	uint32_t ctx_id;
+	bool context_created;
+	struct mutex context_lock;
 };
 
 /* virtio_ioctl.c */
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 00ef9fd3fbf6..ec38cf5573aa 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -40,10 +40,18 @@ void virtio_gpu_create_context(struct drm_device *dev,
 	struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
 	char dbgname[TASK_COMM_LEN];
 
+	mutex_lock(&vfpriv->context_lock);
+	if (vfpriv->context_created)
+		goto out_unlock;
+
 	get_task_comm(dbgname, current);
 	virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
 				      strlen(dbgname), dbgname);
 	virtio_gpu_notify(vgdev);
+	vfpriv->context_created = true;
+
+out_unlock:
+	mutex_unlock(&vfpriv->context_lock);
 }
 
 static int virtio_gpu_map_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index f7e3712502ca..424729cb81d1 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -258,6 +258,8 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
 	if (!vfpriv)
 		return -ENOMEM;
 
+	mutex_init(&vfpriv->context_lock);
+
 	handle = ida_alloc(&vgdev->ctx_id_ida, GFP_KERNEL);
 	if (handle < 0) {
 		kfree(vfpriv);
@@ -281,6 +283,7 @@ void virtio_gpu_driver_postclose(struct drm_device *dev, struct drm_file *file)
 	vfpriv = file->driver_priv;
 
 	virtio_gpu_context_destroy(vgdev, vfpriv->ctx_id);
+	mutex_destroy(&vfpriv->context_lock);
 	kfree(vfpriv);
 	file->driver_priv = NULL;
 }
-- 
2.25.0.265.gbab2e86ba0-goog

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

[PATCH 4/4 v6] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl

[Gurchetan Singh]

For old userspace, initialization will still be implicit.

For backwards compatibility, enqueue virtio_gpu_cmd_context_create after
the first 3D ioctl.

v3: staticify virtio_gpu_create_context
    remove notify to batch vm-exit
v6: Remove nested 3D checks (emil.velikov):
      - unify 3D check in resource create
      - VIRTIO_GPU_CMD_GET_CAPSET is listed as a 2D ioctl, added a
        3D check there.

Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  2 --
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 32 +++++++++++++++-----------
 drivers/gpu/drm/virtio/virtgpu_kms.c   |  1 -
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 76b7b7c30e10..95a7443baaba 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -216,8 +216,6 @@ struct virtio_gpu_fpriv {
 /* virtio_ioctl.c */
 #define DRM_VIRTIO_NUM_IOCTLS 10
 extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
-void virtio_gpu_create_context(struct drm_device *dev,
-			       struct drm_file *file);
 
 /* virtio_kms.c */
 int virtio_gpu_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index ec38cf5573aa..c36faa572caa 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -33,8 +33,8 @@
 
 #include "virtgpu_drv.h"
 
-void virtio_gpu_create_context(struct drm_device *dev,
-			       struct drm_file *file)
+static void virtio_gpu_create_context(struct drm_device *dev,
+				      struct drm_file *file)
 {
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
@@ -95,6 +95,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
 
 	exbuf->fence_fd = -1;
 
+	virtio_gpu_create_context(dev, file);
 	if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) {
 		struct dma_fence *in_fence;
 
@@ -233,7 +234,17 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 	uint32_t handle = 0;
 	struct virtio_gpu_object_params params = { 0 };
 
-	if (vgdev->has_virgl_3d == false) {
+	if (vgdev->has_virgl_3d) {
+		virtio_gpu_create_context(dev, file);
+		params.virgl = true;
+		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;
+	} else {
 		if (rc->depth > 1)
 			return -EINVAL;
 		if (rc->nr_samples > 1)
@@ -250,16 +261,6 @@ 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.virgl = true;
-		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;
@@ -319,6 +320,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev,
 	if (vgdev->has_virgl_3d == false)
 		return -ENOSYS;
 
+	virtio_gpu_create_context(dev, file);
 	objs = virtio_gpu_array_from_handles(file, &args->bo_handle, 1);
 	if (objs == NULL)
 		return -ENOENT;
@@ -367,6 +369,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data,
 			 args->box.w, args->box.h, args->box.x, args->box.y,
 			 objs, NULL);
 	} else {
+		virtio_gpu_create_context(dev, file);
 		ret = virtio_gpu_array_lock_resv(objs);
 		if (ret != 0)
 			goto err_put_free;
@@ -466,6 +469,9 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev,
 	}
 	spin_unlock(&vgdev->display_info_lock);
 
+	if (vgdev->has_virgl_3d)
+		virtio_gpu_create_context(dev, file);
+
 	/* not in cache - need to talk to hw */
 	virtio_gpu_cmd_get_capset(vgdev, found_valid, args->cap_set_ver,
 				  &cache_ent);
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 424729cb81d1..023a030ca7b9 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -268,7 +268,6 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
 
 	vfpriv->ctx_id = handle + 1;
 	file->driver_priv = vfpriv;
-	virtio_gpu_create_context(dev, file);
 	return 0;
 }
 
-- 
2.25.0.265.gbab2e86ba0-goog

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

Re: [PATCH 4/4 v6] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl

[Chia-I Wu]

On Wed, Feb 19, 2020 at 2:34 PM Gurchetan Singh
<gurchetansingh@chromium.org> wrote:
>
> For old userspace, initialization will still be implicit.
>
> For backwards compatibility, enqueue virtio_gpu_cmd_context_create after
> the first 3D ioctl.
>
> v3: staticify virtio_gpu_create_context
>     remove notify to batch vm-exit
> v6: Remove nested 3D checks (emil.velikov):
>       - unify 3D check in resource create
>       - VIRTIO_GPU_CMD_GET_CAPSET is listed as a 2D ioctl, added a
>         3D check there.
I missed this change.  Why do we need a context to get capsets?  Is
this a workaround or something?

>
> Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h   |  2 --
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 32 +++++++++++++++-----------
>  drivers/gpu/drm/virtio/virtgpu_kms.c   |  1 -
>  3 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 76b7b7c30e10..95a7443baaba 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -216,8 +216,6 @@ struct virtio_gpu_fpriv {
>  /* virtio_ioctl.c */
>  #define DRM_VIRTIO_NUM_IOCTLS 10
>  extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
> -void virtio_gpu_create_context(struct drm_device *dev,
> -                              struct drm_file *file);
>
>  /* virtio_kms.c */
>  int virtio_gpu_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index ec38cf5573aa..c36faa572caa 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -33,8 +33,8 @@
>
>  #include "virtgpu_drv.h"
>
> -void virtio_gpu_create_context(struct drm_device *dev,
> -                              struct drm_file *file)
> +static void virtio_gpu_create_context(struct drm_device *dev,
> +                                     struct drm_file *file)
>  {
>         struct virtio_gpu_device *vgdev = dev->dev_private;
>         struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
> @@ -95,6 +95,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
>
>         exbuf->fence_fd = -1;
>
> +       virtio_gpu_create_context(dev, file);
>         if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) {
>                 struct dma_fence *in_fence;
>
> @@ -233,7 +234,17 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
>         uint32_t handle = 0;
>         struct virtio_gpu_object_params params = { 0 };
>
> -       if (vgdev->has_virgl_3d == false) {
> +       if (vgdev->has_virgl_3d) {
> +               virtio_gpu_create_context(dev, file);
> +               params.virgl = true;
> +               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;
> +       } else {
>                 if (rc->depth > 1)
>                         return -EINVAL;
>                 if (rc->nr_samples > 1)
> @@ -250,16 +261,6 @@ 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.virgl = true;
> -               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;
> @@ -319,6 +320,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev,
>         if (vgdev->has_virgl_3d == false)
>                 return -ENOSYS;
>
> +       virtio_gpu_create_context(dev, file);
>         objs = virtio_gpu_array_from_handles(file, &args->bo_handle, 1);
>         if (objs == NULL)
>                 return -ENOENT;
> @@ -367,6 +369,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data,
>                          args->box.w, args->box.h, args->box.x, args->box.y,
>                          objs, NULL);
>         } else {
> +               virtio_gpu_create_context(dev, file);
>                 ret = virtio_gpu_array_lock_resv(objs);
>                 if (ret != 0)
>                         goto err_put_free;
> @@ -466,6 +469,9 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev,
>         }
>         spin_unlock(&vgdev->display_info_lock);
>
> +       if (vgdev->has_virgl_3d)
> +               virtio_gpu_create_context(dev, file);
> +
>         /* not in cache - need to talk to hw */
>         virtio_gpu_cmd_get_capset(vgdev, found_valid, args->cap_set_ver,
>                                   &cache_ent);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 424729cb81d1..023a030ca7b9 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -268,7 +268,6 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
>
>         vfpriv->ctx_id = handle + 1;
>         file->driver_priv = vfpriv;
> -       virtio_gpu_create_context(dev, file);
>         return 0;
>  }
>
> --
> 2.25.0.265.gbab2e86ba0-goog
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4 v6] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl

[Gurchetan Singh]

On Fri, Feb 21, 2020 at 3:06 PM Chia-I Wu <olvaffe@gmail.com> wrote:
>
> On Wed, Feb 19, 2020 at 2:34 PM Gurchetan Singh
> <gurchetansingh@chromium.org> wrote:
> >
> > For old userspace, initialization will still be implicit.
> >
> > For backwards compatibility, enqueue virtio_gpu_cmd_context_create after
> > the first 3D ioctl.
> >
> > v3: staticify virtio_gpu_create_context
> >     remove notify to batch vm-exit
> > v6: Remove nested 3D checks (emil.velikov):
> >       - unify 3D check in resource create
> >       - VIRTIO_GPU_CMD_GET_CAPSET is listed as a 2D ioctl, added a
> >         3D check there.
> I missed this change.  Why do we need a context to get capsets?  Is
> this a workaround or something?

No, don't need a context to get a capset.  The patch tries to create a
context when a 3D userspace first talks to the host, not when a 3D
userspace first needs a context ID.  If the latter is preferred, we
can do that too.

>
> >
> > Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> > Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> > Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> > ---
> >  drivers/gpu/drm/virtio/virtgpu_drv.h   |  2 --
> >  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 32 +++++++++++++++-----------
> >  drivers/gpu/drm/virtio/virtgpu_kms.c   |  1 -
> >  3 files changed, 19 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > index 76b7b7c30e10..95a7443baaba 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > @@ -216,8 +216,6 @@ struct virtio_gpu_fpriv {
> >  /* virtio_ioctl.c */
> >  #define DRM_VIRTIO_NUM_IOCTLS 10
> >  extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
> > -void virtio_gpu_create_context(struct drm_device *dev,
> > -                              struct drm_file *file);
> >
> >  /* virtio_kms.c */
> >  int virtio_gpu_init(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > index ec38cf5573aa..c36faa572caa 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > @@ -33,8 +33,8 @@
> >
> >  #include "virtgpu_drv.h"
> >
> > -void virtio_gpu_create_context(struct drm_device *dev,
> > -                              struct drm_file *file)
> > +static void virtio_gpu_create_context(struct drm_device *dev,
> > +                                     struct drm_file *file)
> >  {
> >         struct virtio_gpu_device *vgdev = dev->dev_private;
> >         struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
> > @@ -95,6 +95,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> >
> >         exbuf->fence_fd = -1;
> >
> > +       virtio_gpu_create_context(dev, file);
> >         if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) {
> >                 struct dma_fence *in_fence;
> >
> > @@ -233,7 +234,17 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
> >         uint32_t handle = 0;
> >         struct virtio_gpu_object_params params = { 0 };
> >
> > -       if (vgdev->has_virgl_3d == false) {
> > +       if (vgdev->has_virgl_3d) {
> > +               virtio_gpu_create_context(dev, file);
> > +               params.virgl = true;
> > +               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;
> > +       } else {
> >                 if (rc->depth > 1)
> >                         return -EINVAL;
> >                 if (rc->nr_samples > 1)
> > @@ -250,16 +261,6 @@ 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.virgl = true;
> > -               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;
> > @@ -319,6 +320,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev,
> >         if (vgdev->has_virgl_3d == false)
> >                 return -ENOSYS;
> >
> > +       virtio_gpu_create_context(dev, file);
> >         objs = virtio_gpu_array_from_handles(file, &args->bo_handle, 1);
> >         if (objs == NULL)
> >                 return -ENOENT;
> > @@ -367,6 +369,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data,
> >                          args->box.w, args->box.h, args->box.x, args->box.y,
> >                          objs, NULL);
> >         } else {
> > +               virtio_gpu_create_context(dev, file);
> >                 ret = virtio_gpu_array_lock_resv(objs);
> >                 if (ret != 0)
> >                         goto err_put_free;
> > @@ -466,6 +469,9 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev,
> >         }
> >         spin_unlock(&vgdev->display_info_lock);
> >
> > +       if (vgdev->has_virgl_3d)
> > +               virtio_gpu_create_context(dev, file);
> > +
> >         /* not in cache - need to talk to hw */
> >         virtio_gpu_cmd_get_capset(vgdev, found_valid, args->cap_set_ver,
> >                                   &cache_ent);
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > index 424729cb81d1..023a030ca7b9 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > @@ -268,7 +268,6 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
> >
> >         vfpriv->ctx_id = handle + 1;
> >         file->driver_priv = vfpriv;
> > -       virtio_gpu_create_context(dev, file);
> >         return 0;
> >  }
> >
> > --
> > 2.25.0.265.gbab2e86ba0-goog
> >
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4 v6] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl

[Gerd Hoffmann]

On Fri, Feb 21, 2020 at 04:54:02PM -0800, Gurchetan Singh wrote:
> On Fri, Feb 21, 2020 at 3:06 PM Chia-I Wu <olvaffe@gmail.com> wrote:
> >
> > On Wed, Feb 19, 2020 at 2:34 PM Gurchetan Singh
> > <gurchetansingh@chromium.org> wrote:
> > >
> > > For old userspace, initialization will still be implicit.
> > >
> > > For backwards compatibility, enqueue virtio_gpu_cmd_context_create after
> > > the first 3D ioctl.
> > >
> > > v3: staticify virtio_gpu_create_context
> > >     remove notify to batch vm-exit
> > > v6: Remove nested 3D checks (emil.velikov):
> > >       - unify 3D check in resource create
> > >       - VIRTIO_GPU_CMD_GET_CAPSET is listed as a 2D ioctl, added a
> > >         3D check there.
> > I missed this change.  Why do we need a context to get capsets?  Is
> > this a workaround or something?
> 
> No, don't need a context to get a capset.  The patch tries to create a
> context when a 3D userspace first talks to the host, not when a 3D
> userspace first needs a context ID.  If the latter is preferred, we
> can do that too.

I think it makes sense to exclude the capset ioctls here.

Possibly they are used for non-3d-related capabilities at some
point in the future.

Also userspace gets capsets while checking device capabilities
and possibly does so before deciding how to drive the device.

cheers,
  Gerd

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

Re: [PATCH 4/4 v6] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl

[Emil Velikov]

On Mon, 24 Feb 2020 at 11:06, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Fri, Feb 21, 2020 at 04:54:02PM -0800, Gurchetan Singh wrote:
> > On Fri, Feb 21, 2020 at 3:06 PM Chia-I Wu <olvaffe@gmail.com> wrote:
> > >
> > > On Wed, Feb 19, 2020 at 2:34 PM Gurchetan Singh
> > > <gurchetansingh@chromium.org> wrote:
> > > >
> > > > For old userspace, initialization will still be implicit.
> > > >
> > > > For backwards compatibility, enqueue virtio_gpu_cmd_context_create after
> > > > the first 3D ioctl.
> > > >
> > > > v3: staticify virtio_gpu_create_context
> > > >     remove notify to batch vm-exit
> > > > v6: Remove nested 3D checks (emil.velikov):
> > > >       - unify 3D check in resource create
> > > >       - VIRTIO_GPU_CMD_GET_CAPSET is listed as a 2D ioctl, added a
> > > >         3D check there.
> > > I missed this change.  Why do we need a context to get capsets?  Is
> > > this a workaround or something?
> >
> > No, don't need a context to get a capset.  The patch tries to create a
> > context when a 3D userspace first talks to the host, not when a 3D
> > userspace first needs a context ID.  If the latter is preferred, we
> > can do that too.
>
> I think it makes sense to exclude the capset ioctls here.
>
> Possibly they are used for non-3d-related capabilities at some
> point in the future.
>
> Also userspace gets capsets while checking device capabilities
> and possibly does so before deciding how to drive the device.
>
Virglrenderer creates the internal/base GL* context during
virgl_renderer_init().
Based upon which the caps are populated.

Personally I don't have a preference for/against dropping the
virtio_gpu_create_context().
Yet it does seems safe to omit.

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

Re: [PATCH 4/4 v6] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl

[Chia-I Wu]

On Mon, Feb 24, 2020 at 5:24 AM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Mon, 24 Feb 2020 at 11:06, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Fri, Feb 21, 2020 at 04:54:02PM -0800, Gurchetan Singh wrote:
> > > On Fri, Feb 21, 2020 at 3:06 PM Chia-I Wu <olvaffe@gmail.com> wrote:
> > > >
> > > > On Wed, Feb 19, 2020 at 2:34 PM Gurchetan Singh
> > > > <gurchetansingh@chromium.org> wrote:
> > > > >
> > > > > For old userspace, initialization will still be implicit.
> > > > >
> > > > > For backwards compatibility, enqueue virtio_gpu_cmd_context_create after
> > > > > the first 3D ioctl.
> > > > >
> > > > > v3: staticify virtio_gpu_create_context
> > > > >     remove notify to batch vm-exit
> > > > > v6: Remove nested 3D checks (emil.velikov):
> > > > >       - unify 3D check in resource create
> > > > >       - VIRTIO_GPU_CMD_GET_CAPSET is listed as a 2D ioctl, added a
> > > > >         3D check there.
> > > > I missed this change.  Why do we need a context to get capsets?  Is
> > > > this a workaround or something?
> > >
> > > No, don't need a context to get a capset.  The patch tries to create a
> > > context when a 3D userspace first talks to the host, not when a 3D
> > > userspace first needs a context ID.  If the latter is preferred, we
> > > can do that too.
> >
> > I think it makes sense to exclude the capset ioctls here.
> >
> > Possibly they are used for non-3d-related capabilities at some
> > point in the future.
> >
> > Also userspace gets capsets while checking device capabilities
> > and possibly does so before deciding how to drive the device.
> >
> Virglrenderer creates the internal/base GL* context during
> virgl_renderer_init().
> Based upon which the caps are populated.
>
> Personally I don't have a preference for/against dropping the
> virtio_gpu_create_context().
> Yet it does seems safe to omit.
Yeah, it should be safe to omit.  The userspace might want to decide
the "context type" based on the capsets.  It should also be better to
omit.


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