dri-devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/5] *** Delay context create cmd ***
@ 2020-02-13 23:18 Gurchetan Singh
  2020-02-13 23:18 ` [PATCH 1/5] drm/virtio: use consistent names for drm_files Gurchetan Singh
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Gurchetan Singh @ 2020-02-13 23:18 UTC (permalink / raw)
  To: dri-devel; +Cc: Gurchetan Singh, kraxel, jbates

Let's delay enqueuing the context creation command until the first 3D
ioctl, as we add more context types.  This series is based on kraxel's
"[PATCH v3 0/4] drm/virtio: rework batching" patches.

Gurchetan Singh (5):
  drm/virtio: use consistent names for drm_files
  drm/virtio: factor out context create hyercall
  drm/virtio: track whether or not a context has been initiated
  drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl
  drm/virtio: add virtio_gpu_context_type

 drivers/gpu/drm/virtio/virtgpu_drv.h   |  1 +
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 55 +++++++++++++++++++++-----
 drivers/gpu/drm/virtio/virtgpu_kms.c   | 25 +++---------
 3 files changed, 52 insertions(+), 29 deletions(-)

-- 
2.25.0.265.gbab2e86ba0-goog

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] drm/virtio: use consistent names for drm_files
  2020-02-13 23:18 [PATCH v2 0/5] *** Delay context create cmd *** Gurchetan Singh
@ 2020-02-13 23:18 ` Gurchetan Singh
  2020-02-13 23:18 ` [PATCH 2/5] drm/virtio: factor out context create hyercall Gurchetan Singh
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Gurchetan Singh @ 2020-02-13 23:18 UTC (permalink / raw)
  To: dri-devel; +Cc: Gurchetan Singh, kraxel, jbates

Minor cleanup, change:

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

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 467649733d24..b0edf1ca095f 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;
@@ -177,7 +177,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;
@@ -200,7 +200,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;
@@ -251,7 +251,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;
@@ -264,13 +264,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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/5] drm/virtio: factor out context create hyercall
  2020-02-13 23:18 [PATCH v2 0/5] *** Delay context create cmd *** Gurchetan Singh
  2020-02-13 23:18 ` [PATCH 1/5] drm/virtio: use consistent names for drm_files Gurchetan Singh
@ 2020-02-13 23:18 ` Gurchetan Singh
  2020-02-13 23:18 ` [PATCH 3/5] drm/virtio: track whether or not a context has been initiated Gurchetan Singh
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Gurchetan Singh @ 2020-02-13 23:18 UTC (permalink / raw)
  To: dri-devel; +Cc: Gurchetan Singh, kraxel, jbates

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)

Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  2 ++
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 16 ++++++++++++++++
 drivers/gpu/drm/virtio/virtgpu_kms.c   | 25 ++++++-------------------
 3 files changed, 24 insertions(+), 19 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 b0edf1ca095f..896c3f419a6d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -33,6 +33,22 @@
 
 #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];
+
+	/* can't create contexts without 3d renderer */
+	if (!vgdev->has_virgl_3d)
+		return;
+
+	get_task_comm(dbgname, current);
+	virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
+				      strlen(dbgname), dbgname);
+}
+
 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 8fd7acef960f..282558576527 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -52,18 +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);
-	return handle;
-}
-
 static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev,
 				      uint32_t ctx_id)
 {
@@ -257,8 +245,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)
@@ -269,15 +256,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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/5] drm/virtio: track whether or not a context has been initiated
  2020-02-13 23:18 [PATCH v2 0/5] *** Delay context create cmd *** Gurchetan Singh
  2020-02-13 23:18 ` [PATCH 1/5] drm/virtio: use consistent names for drm_files Gurchetan Singh
  2020-02-13 23:18 ` [PATCH 2/5] drm/virtio: factor out context create hyercall Gurchetan Singh
@ 2020-02-13 23:18 ` Gurchetan Singh
  2020-02-14 19:27   ` Chia-I Wu
  2020-02-13 23:18 ` [PATCH 4/5] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl Gurchetan Singh
  2020-02-13 23:18 ` [PATCH 5/5] drm/virtio: add virtio_gpu_context_type Gurchetan Singh
  4 siblings, 1 reply; 9+ messages in thread
From: Gurchetan Singh @ 2020-02-13 23:18 UTC (permalink / raw)
  To: dri-devel; +Cc: Gurchetan Singh, kraxel, jbates

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

v2: Fix possible race (@olv)

Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   | 1 +
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 3 +++
 drivers/gpu/drm/virtio/virtgpu_kms.c   | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 72c1d9b59dfe..ca505984f8ab 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -209,6 +209,7 @@ struct virtio_gpu_device {
 
 struct virtio_gpu_fpriv {
 	uint32_t ctx_id;
+	atomic_t context_initiated;
 };
 
 /* virtio_ioctl.c */
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 896c3f419a6d..a98884462944 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -44,6 +44,9 @@ void virtio_gpu_create_context(struct drm_device *dev,
 	if (!vgdev->has_virgl_3d)
 		return;
 
+	if (!atomic_add_unless(&vfpriv->context_initiated, 1, 1))
+		return;
+
 	get_task_comm(dbgname, current);
 	virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
 				      strlen(dbgname), dbgname);
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 282558576527..25248bad3fc4 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -263,6 +263,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
 	}
 
 	vfpriv->ctx_id = handle + 1;
+	atomic_set(&vfpriv->context_initiated, 0);
 	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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/5] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl
  2020-02-13 23:18 [PATCH v2 0/5] *** Delay context create cmd *** Gurchetan Singh
                   ` (2 preceding siblings ...)
  2020-02-13 23:18 ` [PATCH 3/5] drm/virtio: track whether or not a context has been initiated Gurchetan Singh
@ 2020-02-13 23:18 ` Gurchetan Singh
  2020-02-13 23:18 ` [PATCH 5/5] drm/virtio: add virtio_gpu_context_type Gurchetan Singh
  4 siblings, 0 replies; 9+ messages in thread
From: Gurchetan Singh @ 2020-02-13 23:18 UTC (permalink / raw)
  To: dri-devel; +Cc: Gurchetan Singh, kraxel, jbates

For old userspace, initialization will still be implicit.

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

v2: staticify virtio_gpu_create_context

Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   | 2 --
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 9 +++++++--
 drivers/gpu/drm/virtio/virtgpu_kms.c   | 1 -
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index ca505984f8ab..4de0741da454 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -215,8 +215,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 a98884462944..d2f17778bdc4 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;
@@ -93,6 +93,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;
 
@@ -243,6 +244,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 			return -EINVAL;
 	}
 
+	virtio_gpu_create_context(dev, file);
 	params.format = rc->format;
 	params.width = rc->width;
 	params.height = rc->height;
@@ -316,6 +318,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;
@@ -363,6 +366,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;
@@ -463,6 +467,7 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev,
 	spin_unlock(&vgdev->display_info_lock);
 
 	/* not in cache - need to talk to hw */
+	virtio_gpu_create_context(dev, file);
 	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 25248bad3fc4..f84f2cb7546a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -265,7 +265,6 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
 	vfpriv->ctx_id = handle + 1;
 	atomic_set(&vfpriv->context_initiated, 0);
 	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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/5] drm/virtio: add virtio_gpu_context_type
  2020-02-13 23:18 [PATCH v2 0/5] *** Delay context create cmd *** Gurchetan Singh
                   ` (3 preceding siblings ...)
  2020-02-13 23:18 ` [PATCH 4/5] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl Gurchetan Singh
@ 2020-02-13 23:18 ` Gurchetan Singh
  4 siblings, 0 replies; 9+ messages in thread
From: Gurchetan Singh @ 2020-02-13 23:18 UTC (permalink / raw)
  To: dri-devel; +Cc: Gurchetan Singh, kraxel, jbates

We'll have to do something like this eventually, and this
conveys we want a Virgl context by default.

Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index d2f17778bdc4..f3be29d3f103 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -33,8 +33,14 @@
 
 #include "virtgpu_drv.h"
 
+/* TODO: add more context types */
+enum virtio_gpu_context_type {
+	virtio_gpu_virgl_context,
+};
+
 static void virtio_gpu_create_context(struct drm_device *dev,
-				      struct drm_file *file)
+				      struct drm_file *file,
+				      enum virtio_gpu_context_type type)
 {
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
@@ -47,6 +53,11 @@ static void virtio_gpu_create_context(struct drm_device *dev,
 	if (!atomic_add_unless(&vfpriv->context_initiated, 1, 1))
 		return;
 
+	if (type != virtio_gpu_virgl_context) {
+		DRM_ERROR("Unsupported context type: %u\n", type);
+		return;
+	}
+
 	get_task_comm(dbgname, current);
 	virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
 				      strlen(dbgname), dbgname);
@@ -93,7 +104,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
 
 	exbuf->fence_fd = -1;
 
-	virtio_gpu_create_context(dev, file);
+	virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context);
 	if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) {
 		struct dma_fence *in_fence;
 
@@ -244,7 +255,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 			return -EINVAL;
 	}
 
-	virtio_gpu_create_context(dev, file);
+	virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context);
 	params.format = rc->format;
 	params.width = rc->width;
 	params.height = rc->height;
@@ -318,7 +329,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);
+	virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context);
 	objs = virtio_gpu_array_from_handles(file, &args->bo_handle, 1);
 	if (objs == NULL)
 		return -ENOENT;
@@ -366,7 +377,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);
+		virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context);
 		ret = virtio_gpu_array_lock_resv(objs);
 		if (ret != 0)
 			goto err_put_free;
@@ -467,7 +478,7 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev,
 	spin_unlock(&vgdev->display_info_lock);
 
 	/* not in cache - need to talk to hw */
-	virtio_gpu_create_context(dev, file);
+	virtio_gpu_create_context(dev, file, virtio_gpu_virgl_context);
 	virtio_gpu_cmd_get_capset(vgdev, found_valid, args->cap_set_ver,
 				  &cache_ent);
 
-- 
2.25.0.265.gbab2e86ba0-goog

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] drm/virtio: track whether or not a context has been initiated
  2020-02-13 23:18 ` [PATCH 3/5] drm/virtio: track whether or not a context has been initiated Gurchetan Singh
@ 2020-02-14 19:27   ` Chia-I Wu
  2020-02-15  2:29     ` Gurchetan Singh
  0 siblings, 1 reply; 9+ messages in thread
From: Chia-I Wu @ 2020-02-14 19:27 UTC (permalink / raw)
  To: Gurchetan Singh; +Cc: Gerd Hoffmann, ML dri-devel, jbates

On Thu, Feb 13, 2020 at 3:18 PM Gurchetan Singh
<gurchetansingh@chromium.org> wrote:
>
> Use an atomic variable to track whether a context has been
> initiated.
>
> v2: Fix possible race (@olv)
>
> Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 1 +
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 3 +++
>  drivers/gpu/drm/virtio/virtgpu_kms.c   | 1 +
>  3 files changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 72c1d9b59dfe..ca505984f8ab 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -209,6 +209,7 @@ struct virtio_gpu_device {
>
>  struct virtio_gpu_fpriv {
>         uint32_t ctx_id;
> +       atomic_t context_initiated;
>  };
>
>  /* virtio_ioctl.c */
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 896c3f419a6d..a98884462944 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -44,6 +44,9 @@ void virtio_gpu_create_context(struct drm_device *dev,
>         if (!vgdev->has_virgl_3d)
>                 return;
>
> +       if (!atomic_add_unless(&vfpriv->context_initiated, 1, 1))
> +               return;
> +
How does this work?  When thread A and B enter this function at the
same time, and thread B returns early, it is possible that thread B
queues other commands before thread A has the chance to queue
virtio_gpu_cmd_context_create.

>         get_task_comm(dbgname, current);
>         virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
>                                       strlen(dbgname), dbgname);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 282558576527..25248bad3fc4 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -263,6 +263,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
>         }
>
>         vfpriv->ctx_id = handle + 1;
> +       atomic_set(&vfpriv->context_initiated, 0);
>         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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] drm/virtio: track whether or not a context has been initiated
  2020-02-14 19:27   ` Chia-I Wu
@ 2020-02-15  2:29     ` Gurchetan Singh
  2020-02-15  4:29       ` Chia-I Wu
  0 siblings, 1 reply; 9+ messages in thread
From: Gurchetan Singh @ 2020-02-15  2:29 UTC (permalink / raw)
  To: Chia-I Wu; +Cc: Gerd Hoffmann, ML dri-devel, jbates

On Fri, Feb 14, 2020 at 11:27 AM Chia-I Wu <olvaffe@gmail.com> wrote:
>
> On Thu, Feb 13, 2020 at 3:18 PM Gurchetan Singh
> <gurchetansingh@chromium.org> wrote:
> >
> > Use an atomic variable to track whether a context has been
> > initiated.
> >
> > v2: Fix possible race (@olv)
> >
> > Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> > ---
> >  drivers/gpu/drm/virtio/virtgpu_drv.h   | 1 +
> >  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 3 +++
> >  drivers/gpu/drm/virtio/virtgpu_kms.c   | 1 +
> >  3 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > index 72c1d9b59dfe..ca505984f8ab 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > @@ -209,6 +209,7 @@ struct virtio_gpu_device {
> >
> >  struct virtio_gpu_fpriv {
> >         uint32_t ctx_id;
> > +       atomic_t context_initiated;
> >  };
> >
> >  /* virtio_ioctl.c */
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > index 896c3f419a6d..a98884462944 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > @@ -44,6 +44,9 @@ void virtio_gpu_create_context(struct drm_device *dev,
> >         if (!vgdev->has_virgl_3d)
> >                 return;
> >
> > +       if (!atomic_add_unless(&vfpriv->context_initiated, 1, 1))
> > +               return;
> > +
> How does this work?  When thread A and B enter this function at the
> same time, and thread B returns early, it is possible that thread B
> queues other commands before thread A has the chance to queue
> virtio_gpu_cmd_context_create.

Good catch, I'll add a spinlock in v3.

>
> >         get_task_comm(dbgname, current);
> >         virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
> >                                       strlen(dbgname), dbgname);
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > index 282558576527..25248bad3fc4 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > @@ -263,6 +263,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
> >         }
> >
> >         vfpriv->ctx_id = handle + 1;
> > +       atomic_set(&vfpriv->context_initiated, 0);
> >         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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] drm/virtio: track whether or not a context has been initiated
  2020-02-15  2:29     ` Gurchetan Singh
@ 2020-02-15  4:29       ` Chia-I Wu
  0 siblings, 0 replies; 9+ messages in thread
From: Chia-I Wu @ 2020-02-15  4:29 UTC (permalink / raw)
  To: Gurchetan Singh; +Cc: Gerd Hoffmann, ML dri-devel, jbates

On Fri, Feb 14, 2020 at 6:29 PM Gurchetan Singh
<gurchetansingh@chromium.org> wrote:
>
> On Fri, Feb 14, 2020 at 11:27 AM Chia-I Wu <olvaffe@gmail.com> wrote:
> >
> > On Thu, Feb 13, 2020 at 3:18 PM Gurchetan Singh
> > <gurchetansingh@chromium.org> wrote:
> > >
> > > Use an atomic variable to track whether a context has been
> > > initiated.
> > >
> > > v2: Fix possible race (@olv)
> > >
> > > Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> > > ---
> > >  drivers/gpu/drm/virtio/virtgpu_drv.h   | 1 +
> > >  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 3 +++
> > >  drivers/gpu/drm/virtio/virtgpu_kms.c   | 1 +
> > >  3 files changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > > index 72c1d9b59dfe..ca505984f8ab 100644
> > > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> > > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > > @@ -209,6 +209,7 @@ struct virtio_gpu_device {
> > >
> > >  struct virtio_gpu_fpriv {
> > >         uint32_t ctx_id;
> > > +       atomic_t context_initiated;
> > >  };
> > >
> > >  /* virtio_ioctl.c */
> > > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > > index 896c3f419a6d..a98884462944 100644
> > > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > > @@ -44,6 +44,9 @@ void virtio_gpu_create_context(struct drm_device *dev,
> > >         if (!vgdev->has_virgl_3d)
> > >                 return;
> > >
> > > +       if (!atomic_add_unless(&vfpriv->context_initiated, 1, 1))
> > > +               return;
> > > +
> > How does this work?  When thread A and B enter this function at the
> > same time, and thread B returns early, it is possible that thread B
> > queues other commands before thread A has the chance to queue
> > virtio_gpu_cmd_context_create.
>
> Good catch, I'll add a spinlock in v3.
virtio_gpu_cmd_context_create can sleep.  You will need a mutex (or
figure out another way to do the lazy initialization).

>
> >
> > >         get_task_comm(dbgname, current);
> > >         virtio_gpu_cmd_context_create(vgdev, vfpriv->ctx_id,
> > >                                       strlen(dbgname), dbgname);
> > > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > > index 282558576527..25248bad3fc4 100644
> > > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> > > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > > @@ -263,6 +263,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
> > >         }
> > >
> > >         vfpriv->ctx_id = handle + 1;
> > > +       atomic_set(&vfpriv->context_initiated, 0);
> > >         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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 23:18 [PATCH v2 0/5] *** Delay context create cmd *** Gurchetan Singh
2020-02-13 23:18 ` [PATCH 1/5] drm/virtio: use consistent names for drm_files Gurchetan Singh
2020-02-13 23:18 ` [PATCH 2/5] drm/virtio: factor out context create hyercall Gurchetan Singh
2020-02-13 23:18 ` [PATCH 3/5] drm/virtio: track whether or not a context has been initiated Gurchetan Singh
2020-02-14 19:27   ` Chia-I Wu
2020-02-15  2:29     ` Gurchetan Singh
2020-02-15  4:29       ` Chia-I Wu
2020-02-13 23:18 ` [PATCH 4/5] drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl Gurchetan Singh
2020-02-13 23:18 ` [PATCH 5/5] drm/virtio: add virtio_gpu_context_type Gurchetan Singh

dri-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dri-devel/0 dri-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dri-devel dri-devel/ https://lore.kernel.org/dri-devel \
		dri-devel@lists.freedesktop.org
	public-inbox-index dri-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.dri-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git