Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5862 bytes --]

On Fri, 10 Jun 2022 14:24:01 +0000
Zack Rusin <zackr@vmware.com> wrote:

> 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.

Sorry Zack, while I'm sure that is technically correct and very detaily
accurate, it's totally not any different to what we have been talking
about all along.

We care about how things look like to the end user, and not what
property values the guest KMS driver might have for each plane. The KMS
UAPI contract is about how things look to the end user, not just what
values might be stored in a KMS driver (and then ignored).

It doesn't matter if the CRTC_X/Y properties remain at what the guest
userspace set them to, if you are going to hide the "real" virtual
cursor plane and instead upload the cursor image to the host window
system to be composited independently. You are breaking the UAPI
contract. What the end user sees is *not* what the guest OS programmed.
That's the whole point.

What you described is the very definition of cursor plane commandeering
as I defined it: showing the cursor image not where the guest OS put it.

I never assumed that you would actually reflect host cursor position in
the guest KMS cursor plane CRTC_X/Y. You just ignore those values
completely in the VM stack levels closer to the end user's eyes in
seamless mouse mode. The end effect is the same.

> 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"

Correct.

Without such explicit promise from guest userspace you cannot activate
seamless mouse mode at all.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]