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

[Zack Rusin <zackr@vmware.com>]

On Wed, 2023-06-28 at 10:54 +0300, Pekka Paalanen wrote:
> On Wed, 28 Jun 2023 10:41:06 +0300
> Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Wed, 28 Jun 2023 01:21:27 -0400
> > Zack Rusin <zack@kde.org> wrote:
> > 
> > > From: Zack Rusin <zackr@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@vmware.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Reviewed-by: Javier Martinez Canillas <javierm@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:

/**
 * @hotspot_x_property: property to set mouse hotspot x offset.
 *
 * 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).
 *
 * 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.
 *
 * 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).
 * /

z