[PATCH 0/2] virtgpu dummy events

[PATCH 0/2] virtgpu dummy events

[Gurchetan Singh]

From: Gurchetan Singh <gurchetansingh@chromium.org>

There was a desire to not have a virtgpu-specific implementation of
poll(..), but there wasn't any real event to return either.

Solution: Dummy event with just event code

Context:

https://lists.freedesktop.org/archives/dri-devel/2021-November/330950.html

Userspace:

crrev.com/c/3296610

This series applies to drm-misc-fixes.

Gurchetan Singh (2):
  drm/virtgpu api: define a dummy fence signaled event
  drm/virtio: use drm_poll(..) instead of virtio_gpu_poll(..)

 drivers/gpu/drm/virtio/virtgpu_drv.c   | 42 +-------------------------
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  1 -
 drivers/gpu/drm/virtio/virtgpu_ioctl.c |  2 +-
 include/uapi/drm/virtgpu_drm.h         |  7 +++++
 4 files changed, 9 insertions(+), 43 deletions(-)

-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH 1/2] drm/virtgpu api: define a dummy fence signaled event

[Gurchetan Singh]

From: Gurchetan Singh <gurchetansingh@chromium.org>

The current virtgpu implementation of poll(..) drops events
when VIRTGPU_CONTEXT_PARAM_POLL_RINGS_MASK is enabled (otherwise
it's like a normal DRM driver).

This is because paravirtualized userspaces receives responses in a
buffer of type BLOB_MEM_GUEST, not by read(..).

To be in line with other DRM drivers and avoid specialized behavior,
it is possible to define a dummy event for virtgpu.  Paravirtualized
userspace will now have to call read(..) on the DRM fd to receive the
dummy event.

Fixes: b10790434cf2 ("drm/virtgpu api: create context init feature")
Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   | 1 -
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +-
 include/uapi/drm/virtgpu_drm.h         | 7 +++++++
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index e0265fe74aa5..0a194aaad419 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -138,7 +138,6 @@ struct virtio_gpu_fence_driver {
 	spinlock_t       lock;
 };
 
-#define VIRTGPU_EVENT_FENCE_SIGNALED_INTERNAL 0x10000000
 struct virtio_gpu_fence_event {
 	struct drm_pending_event base;
 	struct drm_event event;
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 5618a1d5879c..3607646d3229 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -54,7 +54,7 @@ static int virtio_gpu_fence_event_create(struct drm_device *dev,
 	if (!e)
 		return -ENOMEM;
 
-	e->event.type = VIRTGPU_EVENT_FENCE_SIGNALED_INTERNAL;
+	e->event.type = VIRTGPU_EVENT_FENCE_SIGNALED;
 	e->event.length = sizeof(e->event);
 
 	ret = drm_event_reserve_init(dev, file, &e->base, &e->event);
diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
index a13e20cc66b4..0512fde5e697 100644
--- a/include/uapi/drm/virtgpu_drm.h
+++ b/include/uapi/drm/virtgpu_drm.h
@@ -196,6 +196,13 @@ struct drm_virtgpu_context_init {
 	__u64 ctx_set_params;
 };
 
+/*
+ * Event code that's given when VIRTGPU_CONTEXT_PARAM_POLL_RINGS_MASK is in
+ * effect.  The event size is sizeof(drm_event), since there is no additional
+ * payload.
+ */
+#define VIRTGPU_EVENT_FENCE_SIGNALED 0x90000000
+
 #define DRM_IOCTL_VIRTGPU_MAP \
 	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_MAP, struct drm_virtgpu_map)
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH 2/2] drm/virtio: use drm_poll(..) instead of virtio_gpu_poll(..)

[Gurchetan Singh]

From: Gurchetan Singh <gurchetansingh@chromium.org>

With the use of dummy events, we can drop virtgpu specific
behavior.

Fixes: cd7f5ca33585 ("drm/virtio: implement context init: add virtio_gpu_fence_event")
Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.c | 42 +---------------------------
 1 file changed, 1 insertion(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index d86e1ad4a972..5072dbb0669a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -157,36 +157,6 @@ static void virtio_gpu_config_changed(struct virtio_device *vdev)
 	schedule_work(&vgdev->config_changed_work);
 }
 
-static __poll_t virtio_gpu_poll(struct file *filp,
-				struct poll_table_struct *wait)
-{
-	struct drm_file *drm_file = filp->private_data;
-	struct virtio_gpu_fpriv *vfpriv = drm_file->driver_priv;
-	struct drm_device *dev = drm_file->minor->dev;
-	struct virtio_gpu_device *vgdev = dev->dev_private;
-	struct drm_pending_event *e = NULL;
-	__poll_t mask = 0;
-
-	if (!vgdev->has_virgl_3d || !vfpriv || !vfpriv->ring_idx_mask)
-		return drm_poll(filp, wait);
-
-	poll_wait(filp, &drm_file->event_wait, wait);
-
-	if (!list_empty(&drm_file->event_list)) {
-		spin_lock_irq(&dev->event_lock);
-		e = list_first_entry(&drm_file->event_list,
-				     struct drm_pending_event, link);
-		drm_file->event_space += e->event->length;
-		list_del(&e->link);
-		spin_unlock_irq(&dev->event_lock);
-
-		kfree(e);
-		mask |= EPOLLIN | EPOLLRDNORM;
-	}
-
-	return mask;
-}
-
 static struct virtio_device_id id_table[] = {
 	{ VIRTIO_ID_GPU, VIRTIO_DEV_ANY_ID },
 	{ 0 },
@@ -226,17 +196,7 @@ MODULE_AUTHOR("Dave Airlie <airlied@redhat.com>");
 MODULE_AUTHOR("Gerd Hoffmann <kraxel@redhat.com>");
 MODULE_AUTHOR("Alon Levy");
 
-static const struct file_operations virtio_gpu_driver_fops = {
-	.owner          = THIS_MODULE,
-	.open           = drm_open,
-	.release        = drm_release,
-	.unlocked_ioctl = drm_ioctl,
-	.compat_ioctl   = drm_compat_ioctl,
-	.poll           = virtio_gpu_poll,
-	.read           = drm_read,
-	.llseek         = noop_llseek,
-	.mmap           = drm_gem_mmap
-};
+DEFINE_DRM_GEM_FOPS(virtio_gpu_driver_fops);
 
 static const struct drm_driver driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_RENDER | DRIVER_ATOMIC,
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [PATCH 0/2] virtgpu dummy events

[Daniel Vetter]

On Mon, Nov 22, 2021 at 03:22:08PM -0800, Gurchetan Singh wrote:
> From: Gurchetan Singh <gurchetansingh@chromium.org>
> 
> There was a desire to not have a virtgpu-specific implementation of
> poll(..), but there wasn't any real event to return either.
> 
> Solution: Dummy event with just event code
> 
> Context:
> 
> https://lists.freedesktop.org/archives/dri-devel/2021-November/330950.html
> 
> Userspace:
> 
> crrev.com/c/3296610
> 
> This series applies to drm-misc-fixes.

On the series:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'm assuming someone from Google can push this to drm-misc-fixes for you?
-Daniel

> 
> Gurchetan Singh (2):
>   drm/virtgpu api: define a dummy fence signaled event
>   drm/virtio: use drm_poll(..) instead of virtio_gpu_poll(..)
> 
>  drivers/gpu/drm/virtio/virtgpu_drv.c   | 42 +-------------------------
>  drivers/gpu/drm/virtio/virtgpu_drv.h   |  1 -
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c |  2 +-
>  include/uapi/drm/virtgpu_drm.h         |  7 +++++
>  4 files changed, 9 insertions(+), 43 deletions(-)
> 
> -- 
> 2.34.0.rc2.393.gf8c9666880-goog
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/2] virtgpu dummy events

[Gerd Hoffmann]

  Hi,

> On the series:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I'm assuming someone from Google can push this to drm-misc-fixes for you?

Thanks, pushed.

take care,
  Gerd

Re: [PATCH 0/2] virtgpu dummy events

[Daniel Vetter]

On Mon, Nov 29, 2021 at 12:09:10PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > On the series:
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > I'm assuming someone from Google can push this to drm-misc-fixes for you?
> 
> Thanks, pushed.

Thanks a lot to everyone for handling this so quickly, very much
appreciated.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch