[PATCH v4 2/8] drm/atomic: Add support for mouse hotspots

[Pekka Paalanen]

On Wed, 28 Jun 2023 19:54:49 +0000
Zack Rusin <zackr at vmware.com> wrote:

> On Wed, 2023-06-28 at 10:54 +0300, Pekka Paalanen wrote:
> > On Wed, 28 Jun 2023 10:41:06 +0300
> > Pekka Paalanen <ppaalanen at gmail.com> wrote:
> >   
> > > On Wed, 28 Jun 2023 01:21:27 -0400
> > > Zack Rusin <zack at kde.org> wrote:
> > >   
> > > > From: Zack Rusin <zackr at vmware.com>
> > > > 
> > > > Atomic modesetting code lacked support for specifying mouse cursor
> > > > hotspots. The legacy kms DRM_IOCTL_MODE_CURSOR2 had support for setting
> > > > the hotspot but the functionality was not implemented in the new atomic
> > > > paths.
> > > > 
> > > > Due to the lack of hotspots in the atomic paths userspace compositors
> > > > completely disable atomic modesetting for drivers that require it (i.e.
> > > > all paravirtualized drivers).
> > > > 
> > > > This change adds hotspot properties to the atomic codepaths throughtout
> > > > the DRM core and will allow enabling atomic modesetting for virtualized
> > > > drivers in the userspace.
> > > > 
> > > > Signed-off-by: Zack Rusin <zackr at vmware.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > > Cc: Maxime Ripard <mripard at kernel.org>
> > > > Cc: Thomas Zimmermann <tzimmermann at suse.de>
> > > > Cc: David Airlie <airlied at linux.ie>
> > > > Cc: Daniel Vetter <daniel at ffwll.ch>
> > > > Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>    
> > > 
> > > Hi Zack,
> > > 
> > > I still do not see any UAPI docs for the new properties in this patch?  
> > 
> > If you are wondering what there could be to write about, maybe this can
> > give a good mindset:
> > 
> > Let's assume that I am a Wayland compositor developer who has never used
> > "hotspots" with KMS UAPI before. As I have never tested anything in a
> > VM, I have no idea why the kernel would ever want to know about cursor
> > hotspots. Display hardware never does anything with that, it just puts
> > the cursor plane where I tell it to and composes a single image to be
> > sent to the sink. The virtual driver VKMS does the same. To me, a
> > cursor plane is just another universal plane that may have weird
> > stacking order, pixel format, and size limitations.
> > 
> > Ideally the doc for HOTSPOT_X and HOTSPOT_Y documents not only their
> > possible existence and allowed/expected values, but also the reasons
> > to have them and what they are used for, and that if the properties
> > are exposed they are mandatory to program in order to use the plane.  
> 
> Instead of resending the entire series maybe I can draft a possible doc below and
> see if we like it (once we're ok with I'll send out v5 which hopefully will be
> good). How about:

Hi Zack,

cool!

> 
> /**
>  * @hotspot_x_property: property to set mouse hotspot x offset.

Hmm, this does not look like the style of
https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#plane-composition-properties

I suspect it's a .rst file somewhere.

It is important to use the userspace visible concepts and names, like
the property name being "HOTSPOT_X", not hotspot_x_property. After all,
"HOTSPOT_X" is the string that userspace must use to find this
property. That's the UAPI.

>  *
>  * Hotspot is the point within the cursor image that's activating
>  * the click e.g. imagine an arrow cursor pointing to bottom right -
>  * the origin coordinate for that image would be top left
>  * but the point which would be propagating the click would be
>  * the bottom right cursor position (crtc_x, crtc_y) + hotspot
>  * coordinates which for bottom right facing arrow would probably
>  * be (cursor_width, cursor_height).

Is it really required that the hotspot coordinates fall inside the
cursor plane? Will the atomic commit be rejected otherwise?

Are they given with respect to the cursor plane top-left corner,
positive directions being right/down? Is the unit in CRTC pixels or FB
pixels? The example does give an indirect answer, but my personal taste
would like it to be more explicit.

>  *
>  * This information is only relevant for drivers working on top
>  * of para-virtualized hardware. The reason for that is that
>  * the hotspot is fairly encapsulated in the system but imagine having
>  * multiple windows with virtual machines running on servers
>  * across the globe, as you move the mouse across the screen
>  * and the cursor moves over those multiple windows you wouldn't
>  * want to be sending those mouse events to those machines, so virtual
>  * machine monitors implement an optimization where unless the mouse
>  * is locked to the VM window (e.g. via a click) instead of propagating
>  * those mouse events to those VM's they change the image of the native
>  * cursor to what's inside the mouse cursor plane and do not interact
>  * with the VM window until mouse is clicked in it.

Surely the mouse events are sent to those machines across the globe
regardless?

The point I believe you want to make is that in addition that, a
virtual machine viewer application independently moves the cursor image
on the viewer window to avoid the roundtrip latency across the globe
between mouse movement and cursor movement.

Why is the locking you mention relevant? Wouldn't you do this
optimization always if there is any cursor plane image set?

Or if you literally do mean that no input is sent to the VM at all
until the pointer is locked to that window, then why bother using the
guest cursor image without locking?

I suppose different viewers could do different things, so maybe it's
not necessary to go into those details. Just the idea of the viewer
independently moving the cursor image to reduce hand-eye latency is
important, right?

>  *
>  * In order for that click to correctly and seamlessly propagate
>  * between the native and virtual machine, not only the cursor image
>  * but also the hotspot information has to match between them.
>  *
>  * Make sure to set this property on the cursor plane if you'd like
>  * your application to behave correctly when running on
>  * para-virtualized drivers (qxl, vbox, virtio or vmwgfx).
>  * /

I think you could be more strict here. If these properties exist, then
userspace must set them appropriately and use the cursor plane only for
an actual input controlled cursor image. I might even leave the driver
list out here, because they are mentioned at
DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT doc, and userspace should not base
anything on "if driver is X or Y".

This doc should also link to DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT.

The question of which input device corresponds to which cursor plane
might be good to answer too. I presume the VM runner is configured to
expose exactly one of each, so there can be only one association?

Btw. for my own curiosity, what happens if there are two simultaneous
viewer instances open to the same VM/guest instance? Will they fight
over controlling the same pointer?


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/dri-devel/attachments/20230629/5d29d656/attachment.sig>