On Tue, Nov 16, 2021 at 7:43 AM Daniel Vetter daniel@ffwll.ch wrote:
On Mon, Nov 15, 2021 at 07:26:14PM +0000, Kasireddy, Vivek wrote:
Hi Daniel, Greg,
If it is the same or a similar crash reported here:
https://lists.freedesktop.org/archives/dri-devel/2021-November/330018.html
and here:
https://lists.freedesktop.org/archives/dri-devel/2021-November/330212.html
then the fix is already merged:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
Yeah but that still leaves the problem of why exaxtly virtgpu is
reinventing drm_poll here?
Can you please replace it with drm_poll like all other drivers, including the ones that have private events?
Hi Daniel,
Allow me to explain the use case a bit. It's for when virtgpu KMS is not used, but a special Wayland compositor does wayland passthrough instead:
https://www.youtube.com/watch?v=WwrXqDERFm8https://www.youtube.com/watch?v=E...
This technique has gained much popularity in the virtualized laptop space, where it offers better performance/user experience than virtgpu KMS. The relevant paravirtualized userspace is "Sommelier":
https://chromium.googlesource.com/chromiumos/platform2/+/main/vm_tools/somme... https://chromium.googlesource.com/chromiumos/platform2/+/main/vm_tools/somme...
Previously, we were using the out-of-tree virtio-wl device and there were many discussions on how we could get this upstream:
https://lists.freedesktop.org/archives/dri-devel/2017-December/160309.html https://lists.oasis-open.org/archives/virtio-dev/202002/msg00005.html
Extending virtgpu was deemed the least intrusive option:
https://www.spinics.net/lists/kvm/msg159206.html
We ultimately settled on the context type abstraction and used virtio_gpu_poll to tell the guest "hey, we have a Wayland event". The host response is actually in a buffer of type BLOB_MEM_GUEST.
It is possible to use drm_poll(..), but that would have to be accompanied by a drm_read(..). You'll need to define a dummy VIRTGPU_EVENT_FENCE_SIGNALED in the uapi too.
That's originally how I did it, but some pointed out that's unnecessary since the host response is in the BLOB_MEM_GUEST buffer and virtgpu event is a dummy event. So we decided just to modify virtio_gpu_poll(..) to have the desired semantics in that case.
For the regular virtio-gpu KMS path, things remain unchanged.
There are of course other ways to do it (perhaps polling a dma_fence), but that was the cleanest way we could find.
It's not rare for virtio to "special things" (see virtio_dma_buf_ops, virtio_dma_ops), since they are in fake devices.
We're open to other ideas, but hopefully that answers some of your questions.
Thanks, Daniel
Thanks, Vivek
On Sat, Nov 13, 2021 at 03:51:48PM +0100, Greg KH wrote:
On Tue, Sep 21, 2021 at 04:20:23PM -0700, Gurchetan Singh wrote:
Similar to DRM_VMW_EVENT_FENCE_SIGNALED. Sends a pollable event to the DRM file descriptor when a fence on a specific ring is signaled.
One difference is the event is not exposed via the UAPI -- this is because host responses are on a shared memory buffer of type BLOB_MEM_GUEST [this is the common way to receive responses with virtgpu]. As such, there is no context specific read(..) implementation either -- just a poll(..) implementation.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org Acked-by: Nicholas Verne nverne@chromium.org
drivers/gpu/drm/virtio/virtgpu_drv.c | 43
+++++++++++++++++++++++++-
drivers/gpu/drm/virtio/virtgpu_drv.h | 7 +++++ drivers/gpu/drm/virtio/virtgpu_fence.c | 10 ++++++ drivers/gpu/drm/virtio/virtgpu_ioctl.c | 34 ++++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-)
This commit seems to cause a crash in a virtual drm gpu driver for Android. I have reverted this, and the next commit in the series
from
Linus's tree and all is good again.
Any ideas?
Well no, but also this patch looks very questionable of hand-rolling drm_poll. Yes you can do driver private events like DRM_VMW_EVENT_FENCE_SIGNALED, that's fine. But you really should not
need
to hand-roll the poll callback. vmwgfx (which generally is a very old driver which has lots of custom stuff, so not a great example) doesn't
do
that either.
So that part should go no matter what I think.
-Daniel
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch