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

Tue Jun 14 07:36:42 UTC 2022

On Mon, 13 Jun 2022 14:54:57 +0000
Zack Rusin <zackr at vmware.com> wrote:

> On Mon, 2022-06-13 at 17:25 +0300, Pekka Paalanen wrote:
> > On Mon, 13 Jun 2022 13:14:48 +0000
> > Zack Rusin <zackr at vmware.com> wrote:
> >   
> > > On Mon, 2022-06-13 at 10:33 +0300, Pekka Paalanen wrote:  
> > > > On Fri, 10 Jun 2022 14:24:01 +0000
> > > > Zack Rusin <zackr at 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.    
> > > 
> > > But we don't ignore them, we absolutely need them to set the mouse cursor. This is a
> > > two way process, I think Hans mentioned that, mouse might be "seamless", i.e. it's
> > > the host routing clicks to the guest, or it might be "guest locked", also known as
> > > "gaming mouse", in which case it's fully guest routed/controlled. To have any idea
> > > where the cursor is we absolutely require the crtc_x|y.  
> > 
> > Ok, so seamless mouse mode ignores CRTC_X/Y. This is the commandeered
> > mode.
> > 
> > The normal non-commandeered mode, or what you seem to call "guest
> > locked" which I guess also includes grabbing the mouse into the VM
> > viewer window in the host/viewer system, requires CRTC_X/Y. That's
> > clear.
> > 
> > In other words, the VM stack is switching between seamless pointer,
> > normal pointer, and normal pointer with a grab on the VM viewer winsys,
> > right?
> > 
> > This only means that virtual cursor planes do need CRTC_X/Y properties.
> > That's fine.
> > 
> > The VM stack is still breaking the KMS UAPI contract if the VM stack
> > enters seamless pointer mode without an explicit approval from the guest
> > userspace. You can't say it's ok to do seamless pointer if you
> > *sometimes* also do normal pointer instead, that's not enough.  
> 
> I don't think I ever said that.

I did read that from your replies to the claims that you are ignoring
CRTC_X/Y. It's the impression I got.

Saying that you don't ignore CRTC_X/Y but also implying that you are
not always using them as the cursor image position did not make sense
to me.

> In general I'm not making any philosophical
> argument, all I'm saying is that "virtualized drivers require hotspot
> info". Simon and you are saying that we can't get it until we fix
> other issues with virtualized drivers and I'm simply pointing out
> that your solutions do not work. When we started I did mention that
> this is a lot bigger issue, that's been present for years and will be
> hard to solve, which is why we're off to crazy town right now talking
> about essentially forking kms for virtualized drivers.

The reason I am saying that you need to fix other issues with
virtualized drivers at the same time is because those other issues are
the sole and only reason why the driver would ever need hotspot info.

Hotspot info must not be necessary for correct KMS operation, yet you
seem to insist it is. You assume that cursor plane commandeering is ok
to do. But it is not ok without adding the KMS UAPI that would allow
it: a way for guest userspace to explicitly say that commandeering will
be ok.

If and only if guest userspace says commandeering is ok, then you can
require hotspot info. On the other hand, you cannot retrofit hotspot
info by saying that if a driver exposes hotspot properties then all KMS
clients must set them. That would be a kernel regression by definition:
KMS clients that used to abide the KMS UAPI contract are suddenly
breaking the contract because the kernel changed the contract.

Therefore I very much disagree that virtualized drivers need hotspot
info. They do not strictly need hotspot info for correct operation,
they need it only for making the user experience more smooth, which is
an optional optimization. That optimization may be very important in
practise, but it's still an optimization and is generally not expected
by KMS clients.

I have not understood yet why our proposed solutions do not work. You
keep saying they don't work, but how do they not work?

It is exactly that forking of KMS that I do not want. Hence we need
explicit approval from KMS clients that doing those tricks you want to
do will be ok. If you do those tricks without explicit KMS client
approval, then you have forked KMS: we have two kinds of drivers, they
behave differently, and there is no way for userspace to know which way
they behave, aside for special-casing by driver name, which would be a
maintenance nightmare.

I would not want virtualized drivers to do any of those tricks to begin
with, at all, because that's not what KMS is designed for. But I have
already given in to the idea of the use case (seamless pointer mode)
that requires hotspot info, so I am not fighting against that.

> I thought the solution consisting of an addition of 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 was simple enough and enough to get weston using
> software cursors while having a clear path for gnome-shell and kwin to use atomic
> with virtualized drivers without rewriting them. If we can't agree on that then, at
> this point, it might be better if we just schedule an irc and/or video conference
> with all interested parties to figure this out.

I thought we already agreed on that. I think it's a fine plan from
userspace perspective, and it's along the lines of what Daniel Vetter
wrote which I agreed to.

I don't care if there is cursor plane not being exposed or yes being
exposed. I only care that if a cursor plane is exposed, and it is used
by a KMS client, and the KMS client does not explicitly indicate that
commandeering is ok, then commandeering cannot happen.

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20220614/a9206879/attachment-0001.sig>