Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Thu, 9 Jun 2022 19:39:39 + Zack Rusin wrote: > On Wed, 2022-06-08 at 10:53 +0300, Pekka Paalanen wrote: > > On Tue, 7 Jun 2022 17:50:24 + > > Zack Rusin wrote: > > > > > On Tue, 2022-06-07 at 11:07 +0300, Pekka Paalanen wrote: > > > > On Fri, 03 Jun 2022 14:14:59 + > > > > Simon Ser wrote: > > > > > > > > > Hi, > > > > > > > > > > Please, read this thread: > > > > > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615 > > > > > > > > > > It has a lot of information about the pitfalls of cursor hotspot and > > > > > other things done by VM software. > > > > > > > > > > In particular: since the driver will ignore the KMS cursor plane > > > > > position set by user-space, I don't think it's okay to just expose > > > > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP). > > > > > > > > > > cc wayland-devel and Pekka for user-space feedback. > > > > > > > > > > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin wrote: > > > > > > > > > > > - all userspace code needs to hardcore a list of drivers which > > > > > > require > > > > > > hotspots because there's no way to query from drm "does this driver > > > > > > require hotspot" > > > > > > > > > > Can you elaborate? I'm not sure I understand what you mean here. > > > > > > > > > > > > > Hi Zack and everyone, > > > > > > > > I would like to try to reboot this discussion and explain where I come > > > > from. Maybe I have misunderstood something. > > > > > > First of all thanks for restarting the discussions. I think Gerd > > > did a good > > > job responding to individual points, so let me take a step back and > > > explain the big > > > picture here so we can reboot. > > > > > > > Which root problems do you want to solve exactly? > > > > > > The problem that this patch set is solving is the relatively trivial > > > problem of not > > > having a way of setting the hotspot in the atomic kms interface. When we > > > (virtualized drivers) are using the native cursor we need to know not > > > only the image > > > > Could you clarify what is "native cursor" here? > > I guess it is the host window system pointer's cursor? > > Right, exactly. I'm a little hesitant to call it "host" because it gets > tricky in > remote scenarios, where the host is some ESXi server but the local machine is > the > one that's actually interacting with the guest. So it's the cursor of the > machine > where the guest screen is displayed. > > > > > Now, where the disagreements come from is from the fact that all > > > virtualized drivers > > > do not implement the atomic KMS cursor plane contract exactly as > > > specified. In > > > atomic kms with universal planes the "cursor" plane can be anything so > > > asking for > > > hotspot's for something that's not a cursor is a bit silly (but arguably > > > so is > > > calling it a cursor plane and then complaining that people expect cursor > > > in it). > > > > > > So the argument is that we can't put hotspot data into atomic kms without > > > first > > > rewriting all of the virtualized drivers cursor code to fix the > > > underlying contract > > > violation that they all commit. That would likely be a lot easier sell if > > > not that > > > gnome/kde don't put none cursor stuff in the cursor plane, so all this > > > work is to > > > fix breakages that seem to affect 0 of our users (and I completely > > > understand that > > > we'd still like all the drivers to be correct and unified in terms of > > > their > > > behaviour, I'm just saying it's a hard sell without something that we can > > > point to > > > and say "this fixes/improves things for our customers") > > > > What's the cost of making paravirtualized drivers honour the UAPI contract? > > Can't you do that on the side of implementing these new hotspot > > properties, with the little addition to allowing guest userspace to be > > explicit about whether it supports commandeering or not? > > I'm reluctant here because "fixing" here seems to be a bit ephemeral as we > move from > one solution to the next. I'm happy to write a patch that's adding a > DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE flag and hiding the cursor plane in > virtualized > drivers for clients that advertise DRM_CLIENT_CAP_ATOMIC but not > DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE, but that doesn't solve Weston on > virtualized > drivers. Mind, I have not talked about hiding cursor planes. I have talked *only* about stopping commandeering cursor planes if guest userspace does not indicate it is prepared for commandeering. I don't understand how it does not "solve on Weston on virtualized drivers". Can you explain what is not solved? As far as I can see, it does solve Weston: it makes cursor plane behaviour correct from KMS UAPI contract point of view. Anything that is not quite optimal after that with cursor planes you can blame on Weston not making use of additional optional features. Curs
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Fri, Jun 10, 2022 at 10:49:31AM +0300, Pekka Paalanen wrote: > On Thu, 9 Jun 2022 19:39:39 + > Zack Rusin wrote: > > > On Wed, 2022-06-08 at 10:53 +0300, Pekka Paalanen wrote: > > > On Tue, 7 Jun 2022 17:50:24 + > > > Zack Rusin wrote: > > > > > > > On Tue, 2022-06-07 at 11:07 +0300, Pekka Paalanen wrote: > > > > > On Fri, 03 Jun 2022 14:14:59 + > > > > > Simon Ser wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > Please, read this thread: > > > > > > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615 > > > > > > > > > > > > It has a lot of information about the pitfalls of cursor hotspot and > > > > > > other things done by VM software. > > > > > > > > > > > > In particular: since the driver will ignore the KMS cursor plane > > > > > > position set by user-space, I don't think it's okay to just expose > > > > > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP). > > > > > > > > > > > > cc wayland-devel and Pekka for user-space feedback. > > > > > > > > > > > > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin > > > > > > wrote: > > > > > > > > > > > > > - all userspace code needs to hardcore a list of drivers which > > > > > > > require > > > > > > > hotspots because there's no way to query from drm "does this > > > > > > > driver > > > > > > > require hotspot" > > > > > > > > > > > > Can you elaborate? I'm not sure I understand what you mean here. > > > > > > > > > > > > > > > > Hi Zack and everyone, > > > > > > > > > > I would like to try to reboot this discussion and explain where I come > > > > > from. Maybe I have misunderstood something. > > > > > > > > First of all thanks for restarting the discussions. I think Gerd > > > > did a good > > > > job responding to individual points, so let me take a step back and > > > > explain the big > > > > picture here so we can reboot. > > > > > > > > > Which root problems do you want to solve exactly? > > > > > > > > The problem that this patch set is solving is the relatively trivial > > > > problem of not > > > > having a way of setting the hotspot in the atomic kms interface. When we > > > > (virtualized drivers) are using the native cursor we need to know not > > > > only the image > > > > > > Could you clarify what is "native cursor" here? > > > I guess it is the host window system pointer's cursor? > > > > Right, exactly. I'm a little hesitant to call it "host" because it gets > > tricky in > > remote scenarios, where the host is some ESXi server but the local machine > > is the > > one that's actually interacting with the guest. So it's the cursor of the > > machine > > where the guest screen is displayed. > > > > > > > > Now, where the disagreements come from is from the fact that all > > > > virtualized drivers > > > > do not implement the atomic KMS cursor plane contract exactly as > > > > specified. In > > > > atomic kms with universal planes the "cursor" plane can be anything so > > > > asking for > > > > hotspot's for something that's not a cursor is a bit silly (but > > > > arguably so is > > > > calling it a cursor plane and then complaining that people expect > > > > cursor in it). > > > > > > > > So the argument is that we can't put hotspot data into atomic kms > > > > without first > > > > rewriting all of the virtualized drivers cursor code to fix the > > > > underlying contract > > > > violation that they all commit. That would likely be a lot easier sell > > > > if not that > > > > gnome/kde don't put none cursor stuff in the cursor plane, so all this > > > > work is to > > > > fix breakages that seem to affect 0 of our users (and I completely > > > > understand that > > > > we'd still like all the drivers to be correct and unified in terms of > > > > their > > > > behaviour, I'm just saying it's a hard sell without something that we > > > > can point to > > > > and say "this fixes/improves things for our customers") > > > > > > What's the cost of making paravirtualized drivers honour the UAPI > > > contract? > > > Can't you do that on the side of implementing these new hotspot > > > properties, with the little addition to allowing guest userspace to be > > > explicit about whether it supports commandeering or not? > > > > I'm reluctant here because "fixing" here seems to be a bit ephemeral as we > > move from > > one solution to the next. I'm happy to write a patch that's adding a > > DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE flag and hiding the cursor plane in > > virtualized > > drivers for clients that advertise DRM_CLIENT_CAP_ATOMIC but not > > DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE, but that doesn't solve Weston on > > virtualized > > drivers. > > Mind, I have not talked about hiding cursor planes. I have talked > *only* about stopping commandeering cursor planes if guest userspace > does not indicate it is prepared for commandeering. > > I don't understand how it does not "solve on Weston
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
Hi all, Kinda top post because the thread is sprawling and I think we need a summary/restart. I think there's at least 3 issues here: - lack of hotspot property support, which means compositors can't really support hotspot with atomic. Which isn't entirely true, because you totally can use atomic for the primary planes/crtcs and the legacy cursor ioctls, but I understand that people might find that a bit silly :-) Anyway this problme is solved by the patch set here, and I think results in some nice cleanups to boot. - the fact that cursors for virtual drivers are not planes, but really special things. Which just breaks the universal plane kms uapi. That part isn't solved, and I do agree with Simon and Pekka that we really should solve this before we unleash even more compositors onto the atomic paths of virtual drivers. I think the simplest solution for this is: 1. add a new DRM_PLANE_TYPE_VIRTUAL_CURSOR, and set that for these special cursor planes on all virtual drivers 2. add the new "I understand virtual cursors planes" setparam, filter virtual cursor planes for userspace which doesn't set this (like we do right now if userspace doesn't set the universal plane mode) 3. backport the above patches to all stable kernels 4. make sure the hotspot property is only set on VIRTUAL_CURSOR planes and nothing else in the rebased patch series - third issue: These special virtual display properties arent documented. Aside from hotspot there's also suggested X/Y and maybe other stuff. I have no idea what suggested X/Y does and what userspace should do with it. I think we need a new section for virtualized drivers which: - documents all the properties involved - documents the new cap for enabling virtual cursor planes - documents some of the key flows that compositors should implement for best experience - documents how exactly the user experience will degrade if compositors pretend it's just a normal kms driver (maybe put that into each of the special flows that a compositor ideally supports) - whatever other comments and gaps I've missed, I'm sure Simon/Pekka/others will chime in once the patch exists. There's a bit of fixing oopsies (virtualized drivers really shouldn't have enabled universal planes for their cursors) and debt (all these properties predate the push to document stuff so we need to fix that), but I don't think it's too much. And I think, from reading the threads, that this should cover everything? Anything I've missed? Or got completely wrong? Cheers, Daniel On Fri, Jun 03, 2022 at 02:14:59PM +, Simon Ser wrote: > Hi, > > Please, read this thread: > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615 > > It has a lot of information about the pitfalls of cursor hotspot and > other things done by VM software. > > In particular: since the driver will ignore the KMS cursor plane > position set by user-space, I don't think it's okay to just expose > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP). > > cc wayland-devel and Pekka for user-space feedback. > > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin wrote: > > > - all userspace code needs to hardcore a list of drivers which require > > hotspots because there's no way to query from drm "does this driver > > require hotspot" > > Can you elaborate? I'm not sure I understand what you mean here. > > Thanks, > > Simon -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
I agree with what others have replied, just adding a few more details. On Thursday, June 9th, 2022 at 21:39, Zack Rusin wrote: > virtualized drivers send drm_kms_helper_hotplug_event which sends a HOTPLUG=1 > event with a changed preferred width/height (Note: and the "hotplug_mode_update" property is set to 1.) > suggested_x and suggested_y properties These come with their own set of issues. They are poorly defined, but it seems like they describe a position in physical pixel coordinates. Compositors don't use physical pixel coordinates to organize their outputs, instead they use logical coordinates. For instance, a HiDPI 4k screen with a scale of 2 will take up 1920x1080 logical pixels. There is no way to convert physical pixel coordinates to logical pixel coordinates in the general case, because there's no "global scale factor". So suggested_x/y are incompatible with the way compositors work.
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Fri, 10 Jun 2022 10:41:05 +0200 Daniel Vetter wrote: > Hi all, > > Kinda top post because the thread is sprawling and I think we need a > summary/restart. I think there's at least 3 issues here: > > - lack of hotspot property support, which means compositors can't really > support hotspot with atomic. Which isn't entirely true, because you > totally can use atomic for the primary planes/crtcs and the legacy > cursor ioctls, but I understand that people might find that a bit silly :-) > > Anyway this problme is solved by the patch set here, and I think results > in some nice cleanups to boot. > > - the fact that cursors for virtual drivers are not planes, but really > special things. Which just breaks the universal plane kms uapi. That > part isn't solved, and I do agree with Simon and Pekka that we really > should solve this before we unleash even more compositors onto the > atomic paths of virtual drivers. > > I think the simplest solution for this is: > 1. add a new DRM_PLANE_TYPE_VIRTUAL_CURSOR, and set that for these > special cursor planes on all virtual drivers > 2. add the new "I understand virtual cursors planes" setparam, filter > virtual cursor planes for userspace which doesn't set this (like we do > right now if userspace doesn't set the universal plane mode) > 3. backport the above patches to all stable kernels > 4. make sure the hotspot property is only set on VIRTUAL_CURSOR planes > and nothing else in the rebased patch series > > - third issue: These special virtual display properties arent documented. > Aside from hotspot there's also suggested X/Y and maybe other stuff. I > have no idea what suggested X/Y does and what userspace should do with > it. I think we need a new section for virtualized drivers which: > - documents all the properties involved > - documents the new cap for enabling virtual cursor planes > - documents some of the key flows that compositors should implement for > best experience > - documents how exactly the user experience will degrade if compositors > pretend it's just a normal kms driver (maybe put that into each of the > special flows that a compositor ideally supports) > - whatever other comments and gaps I've missed, I'm sure > Simon/Pekka/others will chime in once the patch exists. > > There's a bit of fixing oopsies (virtualized drivers really shouldn't have > enabled universal planes for their cursors) and debt (all these properties > predate the push to document stuff so we need to fix that), but I don't > think it's too much. And I think, from reading the threads, that this > should cover everything? > > Anything I've missed? Or got completely wrong? Hi, sounds like a good plan to me. Thanks, pq pgpOrIrUOEj1g.pgp Description: OpenPGP digital signature
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Fri, Jun 10, 2022 at 10:41:05AM +0200, Daniel Vetter wrote: > Hi all, > > Kinda top post because the thread is sprawling and I think we need a > summary/restart. I think there's at least 3 issues here: > > - lack of hotspot property support, which means compositors can't really > support hotspot with atomic. Which isn't entirely true, because you > totally can use atomic for the primary planes/crtcs and the legacy > cursor ioctls, but I understand that people might find that a bit silly :-) > > Anyway this problme is solved by the patch set here, and I think results > in some nice cleanups to boot. > > - the fact that cursors for virtual drivers are not planes, but really > special things. Which just breaks the universal plane kms uapi. That > part isn't solved, and I do agree with Simon and Pekka that we really > should solve this before we unleash even more compositors onto the > atomic paths of virtual drivers. > > I think the simplest solution for this is: > 1. add a new DRM_PLANE_TYPE_VIRTUAL_CURSOR, and set that for these > special cursor planes on all virtual drivers > 2. add the new "I understand virtual cursors planes" setparam, filter > virtual cursor planes for userspace which doesn't set this (like we do > right now if userspace doesn't set the universal plane mode) > 3. backport the above patches to all stable kernels > 4. make sure the hotspot property is only set on VIRTUAL_CURSOR planes > and nothing else in the rebased patch series Simon also mentioned on irc that these special planes must not expose the CRTC_X/Y property, since that doesn't really do much at all. Or is our understanding of how this all works for commandeered cursors wrong? > - third issue: These special virtual display properties arent documented. > Aside from hotspot there's also suggested X/Y and maybe other stuff. I > have no idea what suggested X/Y does and what userspace should do with > it. I think we need a new section for virtualized drivers which: > - documents all the properties involved > - documents the new cap for enabling virtual cursor planes > - documents some of the key flows that compositors should implement for > best experience > - documents how exactly the user experience will degrade if compositors > pretend it's just a normal kms driver (maybe put that into each of the > special flows that a compositor ideally supports) > - whatever other comments and gaps I've missed, I'm sure > Simon/Pekka/others will chime in once the patch exists. Great bonus would be an igt which demonstrates these flows. With the interactive debug breakpoints to wait for resizing or whatever this should be all neatly possible. -Daniel > > There's a bit of fixing oopsies (virtualized drivers really shouldn't have > enabled universal planes for their cursors) and debt (all these properties > predate the push to document stuff so we need to fix that), but I don't > think it's too much. And I think, from reading the threads, that this > should cover everything? > > Anything I've missed? Or got completely wrong? > > Cheers, Daniel > > On Fri, Jun 03, 2022 at 02:14:59PM +, Simon Ser wrote: > > Hi, > > > > Please, read this thread: > > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615 > > > > It has a lot of information about the pitfalls of cursor hotspot and > > other things done by VM software. > > > > In particular: since the driver will ignore the KMS cursor plane > > position set by user-space, I don't think it's okay to just expose > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP). > > > > cc wayland-devel and Pekka for user-space feedback. > > > > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin wrote: > > > > > - all userspace code needs to hardcore a list of drivers which require > > > hotspots because there's no way to query from drm "does this driver > > > require hotspot" > > > > Can you elaborate? I'm not sure I understand what you mean here. > > > > Thanks, > > > > Simon > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Fri, Jun 10, 2022 at 08:54:03AM +, Simon Ser wrote: > I agree with what others have replied, just adding a few more details. > > On Thursday, June 9th, 2022 at 21:39, Zack Rusin wrote: > > > virtualized drivers send drm_kms_helper_hotplug_event which sends a > > HOTPLUG=1 > > event with a changed preferred width/height > > (Note: and the "hotplug_mode_update" property is set to 1.) > > > suggested_x and suggested_y properties > > These come with their own set of issues. They are poorly defined, but it seems > like they describe a position in physical pixel coordinates. Compositors don't > use physical pixel coordinates to organize their outputs, instead they use > logical coordinates. For instance, a HiDPI 4k screen with a scale of 2 will > take up 1920x1080 logical pixels. There is no way to convert physical pixel > coordinates to logical pixel coordinates in the general case, because there's > no "global scale factor". So suggested_x/y are incompatible with the way > compositors work. I dropped a request for a proper doc section that explains all the virtualized kms driver stuff. I think we should also put in a "limitations" part there and just spec that any kind of scaling is a no-go on these (and that drivers better validate this is the case). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Friday, June 10th, 2022 at 10:41, Daniel Vetter wrote: > Anything I've missed? Or got completely wrong? This plan looks good to me. As Pekka mentionned, I'd also like to have a conversation of how far we want to push virtualized driver features. I think KMS support is a good feature to have to spin up a VM and have all of the basics working. However I don't think it's a good idea to try to plumb an ever-growing list of fancy features (seamless integration of guest windows into the host, HiDPI, multi-monitor, etc) into KMS. You'd just end up re-inventing Wayland or RDP on top of KMS. Instead of re-inventing these, just use RDP or waypipe or X11 forwarding directly. So I think we need to draw a line somewhere, and decide e.g. that virtualized cursors are fine to add in KMS, but HiDPI is not.
Re: [ANNOUNCE] Wayland 1.21.0 release schedule
Hi Simon, On Tue, 24 May 2022 at 18:26, Simon Ser wrote: > On Wednesday, May 18th, 2022 at 10:39, Olivier Fourdan > wrote: > > > Maybe we could consider > https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/188 ? > > > > I understand this is a huge change in the core of libwayland, but > > that would help addressing old and real issues. > > I've added it to the milestone. I'm not sure we'll be able to merge it > before the release, because it's quite large and reviewer time is > scarce. We'll see. At any rate, even if it doesn't make it, numbers are > cheap, I don't mind doing another release after that MR lands. > I understand and completely agree, it's not a small change and a pretty risky one as well! Cheers Olivier
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Fri, Jun 10, 2022 at 09:15:35AM +, Simon Ser wrote: > On Friday, June 10th, 2022 at 10:41, Daniel Vetter wrote: > > > Anything I've missed? Or got completely wrong? > > This plan looks good to me. > > As Pekka mentionned, I'd also like to have a conversation of how far we want > to > push virtualized driver features. I think KMS support is a good feature to > have > to spin up a VM and have all of the basics working. However I don't think it's > a good idea to try to plumb an ever-growing list of fancy features > (seamless integration of guest windows into the host, HiDPI, multi-monitor, > etc) into KMS. You'd just end up re-inventing Wayland or RDP on top of KMS. > Instead of re-inventing these, just use RDP or waypipe or X11 forwarding > directly. > > So I think we need to draw a line somewhere, and decide e.g. that virtualized > cursors are fine to add in KMS, but HiDPI is not. It's getting a bit far off-topic, but google cros team has an out-of-tree (at least I think it's not merged yet) wayland-virtio driver for exactly this use-case. Trying to move towards something like that for fancy virtualized setups sounds like the better approach indeed, with kms just as the bare-bones fallback option. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
Hi, > > 4. make sure the hotspot property is only set on VIRTUAL_CURSOR planes > > and nothing else in the rebased patch series > > Simon also mentioned on irc that these special planes must not expose the > CRTC_X/Y property, since that doesn't really do much at all. Or is our > understanding of how this all works for commandeered cursors wrong? Depends. In some cases the pointer position is a one-way host->guest ticket (via tablet device). In some cases the other direction works too and the guest can warp the mouse pointer to another place on the host display. The guest can't easily figure whenever warp works or not because that depends on host-side configuration the guest has no insight to. take care, Gerd
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
Hi, > > As Pekka mentionned, I'd also like to have a conversation of how far we > > want to > > push virtualized driver features. I think KMS support is a good feature to > > have > > to spin up a VM and have all of the basics working. However I don't think > > it's > > a good idea to try to plumb an ever-growing list of fancy features > > (seamless integration of guest windows into the host, HiDPI, multi-monitor, > > etc) into KMS. You'd just end up re-inventing Wayland or RDP on top of KMS. > > Instead of re-inventing these, just use RDP or waypipe or X11 forwarding > > directly. > > So I think we need to draw a line somewhere, and decide e.g. that > > virtualized > > cursors are fine to add in KMS, but HiDPI is not. What is the problem with HiDPI? qemu generates standard edid blobs, there should be no need to special-case virtualized drivers in any way. What is the problem with multi-monitor? That isn't much different than physical multi-monitor either. One little thing though: On physical hardware you just don't know which monitor is left and which is right until the user tells you. In case of a virtual multi-monitor setup we know how the two windows for the two virtual monitors are arranged on the host and can pass that as hint to the guest (not sure whenever *that* is the purpose of the suggested_{x,y} properties). > It's getting a bit far off-topic, but google cros team has an out-of-tree > (at least I think it's not merged yet) wayland-virtio driver for exactly > this use-case. Trying to move towards something like that for fancy > virtualized setups sounds like the better approach indeed, with kms just > as the bare-bones fallback option. virtio-gpu got the ability to attach uuids to objects, to allow them being identified on the host side. So it could be that wayland-virtio still uses kms for framebuffers (disclaimer: don't know how wayland-virtio works in detail). But, yes, all the scanout + cursor handling would be out of the way, virtio-gpu would "only" handle fast buffer sharing. take care, Gerd
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Friday, June 10th, 2022 at 14:36, Gerd Hoffmann wrote: > Hi, > > > > As Pekka mentionned, I'd also like to have a conversation of how far we > > > want to > > > push virtualized driver features. I think KMS support is a good feature > > > to have > > > to spin up a VM and have all of the basics working. However I don't think > > > it's > > > a good idea to try to plumb an ever-growing list of fancy features > > > (seamless integration of guest windows into the host, HiDPI, > > > multi-monitor, > > > etc) into KMS. You'd just end up re-inventing Wayland or RDP on top of > > > KMS. > > > Instead of re-inventing these, just use RDP or waypipe or X11 forwarding > > > directly. > > > > So I think we need to draw a line somewhere, and decide e.g. that > > > virtualized > > > cursors are fine to add in KMS, but HiDPI is not. > > > What is the problem with HiDPI? qemu generates standard edid blobs, > there should be no need to special-case virtualized drivers in any way. > > What is the problem with multi-monitor? That isn't much different than > physical multi-monitor either. > > One little thing though: On physical hardware you just don't know which > monitor is left and which is right until the user tells you. In case of > a virtual multi-monitor setup we know how the two windows for the two > virtual monitors are arranged on the host and can pass that as hint to > the guest (not sure whenever that is the purpose of the > suggested_{x,y} properties). The problem with suggested_x/y is described here: https://lore.kernel.org/dri-devel/20220610123629.fgu2em3fto53f...@sirius.home.kraxel.org/T/#m119cfbbf736e43831c3105f0c91bd790da2d58fb HiDPI would need a way to propagate the scale factor back-and-forth: the VM viewer needs to advertise the preferred scale to the guest compositor, and the guest compositor needs to indicate the scale it renders with to the VM viewer. Sounds familiar? Yup, that's exactly the Wayland protocol. Do we really want to replicate the Wayland protocol in KMS? I'm not so sure. > > It's getting a bit far off-topic, but google cros team has an out-of-tree > > (at least I think it's not merged yet) wayland-virtio driver for exactly > > this use-case. Trying to move towards something like that for fancy > > virtualized setups sounds like the better approach indeed, with kms just > > as the bare-bones fallback option. > > virtio-gpu got the ability to attach uuids to objects, to allow them > being identified on the host side. So it could be that wayland-virtio > still uses kms for framebuffers (disclaimer: don't know how > wayland-virtio works in detail). But, yes, all the scanout + cursor > handling would be out of the way, virtio-gpu would "only" handle fast > buffer sharing. wayland-virtio is not used with KMS. wayland-virtio proxies the Wayland protocol between the host and the guest, so the guest doesn't use KMS in that case.
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Fri, 2022-06-10 at 10:59 +0200, Daniel Vetter wrote: > ⚠ External Email > > On Fri, Jun 10, 2022 at 10:41:05AM +0200, Daniel Vetter wrote: > > Hi all, > > > > Kinda top post because the thread is sprawling and I think we need a > > summary/restart. I think there's at least 3 issues here: > > > > - lack of hotspot property support, which means compositors can't really > > support hotspot with atomic. Which isn't entirely true, because you > > totally can use atomic for the primary planes/crtcs and the legacy > > cursor ioctls, but I understand that people might find that a bit silly > > :-) > > > > Anyway this problme is solved by the patch set here, and I think results > > in some nice cleanups to boot. > > > > - the fact that cursors for virtual drivers are not planes, but really > > special things. Which just breaks the universal plane kms uapi. That > > part isn't solved, and I do agree with Simon and Pekka that we really > > should solve this before we unleash even more compositors onto the > > atomic paths of virtual drivers. > > > > I think the simplest solution for this is: > > 1. add a new DRM_PLANE_TYPE_VIRTUAL_CURSOR, and set that for these > > special cursor planes on all virtual drivers > > 2. add the new "I understand virtual cursors planes" setparam, filter > > virtual cursor planes for userspace which doesn't set this (like we do > > right now if userspace doesn't set the universal plane mode) > > 3. backport the above patches to all stable kernels > > 4. make sure the hotspot property is only set on VIRTUAL_CURSOR planes > > and nothing else in the rebased patch series > > Simon also mentioned on irc that these special planes must not expose the > CRTC_X/Y property, since that doesn't really do much at all. Or is our > understanding of how this all works for commandeered cursors wrong? Yes, that's the part I don't understand. I don't think I see how the CRTC_X|Y properties aren't used. I think the confusion might stem from the fact that it appears that the hypervisors/hosts are moving that plane, which is not the case. We never move the plane itself, we redirect the mouse focus/movement, that's what's reducing the latency but don't touch drm internals. I can't speak for every virtualized stack, but what is happening on ours is that we set the image that's on the cursor plane as the cursor on the machine that's running the guest. So when you move the mouse across the screen as you enter the VM window the cursor plane isn't touched, but the local machines cursor changes to what's inside the cursor plane. When the mouse is clicked the mouse device in the guest generates the event with the proper coordinates (hence we need the hotspot to route that event correctly). That's when the guest reacts just like it would normally on native if a mouse event was received. The actual cursor plane might not be visible while this is happening but even when it's not visible it's still at whatever crtc_x|y the guest thinks it is. crtc_x|y are still only driven by the guests mouse device. We control the mouse device and visibility of the cursor plane itself based on what's happening in the system but I don't think that's that different from a native system. This is easy to see by running the "weston-simple-damage --width=64 --height=64" if you click on that window and move the cursor to the very edge of the virtualized window you'll notice that it's coming out outside of the window, that's because it's the local machines cursor, but if you stop the press the window will jump to wherever it was because the real guest plane is still at crtc_x|y (and because in weston that window doesn't react to mouse move events like a cursor would it never sets crtc_x|y to the mouse directed coordinates). The problem stems from the fact that the cursor plane has something that's not a cursor and doesn't react to mouse events like a cursor would. So the flag (or new plane) that we'd be adding is simply making user-space give the following promise: "I understand that what's inside the cursor plane needs to react and deal with mouse events like a mouse cursor would, i.e. it should be a mouse cursor" z