[v5,9/9] drm: Introduce documentation for hotspot properties

Commit Message

Zack Rusin July 19, 2023, 1:42 a.m. UTC

From: Michael Banack <[e-mail]>

To clarify the intent and reasoning behind the hotspot properties
introduce userspace documentation that goes over cursor handling
in para-virtualized environments.

The documentation is generic enough to not special case for any
specific hypervisor and should apply equally to all.

Signed-off-by: Zack Rusin <[e-mail]>
---
 Documentation/gpu/drm-kms.rst |  6 ++++
 drivers/gpu/drm/drm_plane.c   | 58 ++++++++++++++++++++++++++++++++++-
 2 files changed, 63 insertions(+), 1 deletion(-)

Comments

Javier Martinez Canillas July 19, 2023, 7:53 a.m. UTC | #1

Zack Rusin <[e-mail]> writes:

Hello Zack,

> From: Michael Banack <[e-mail]>
>
> To clarify the intent and reasoning behind the hotspot properties
> introduce userspace documentation that goes over cursor handling
> in para-virtualized environments.
>
> The documentation is generic enough to not special case for any
> specific hypervisor and should apply equally to all.
>

This is missing the S-o-B from Michael. But no need to resend I think, you
could just fixup that when applying the patch.

> Signed-off-by: Zack Rusin <[e-mail]>
> ---

I think this documentation is great and it personaly closed some conceptual
gaps for me. Thanks a lot for that.

Reviewed-by: Javier Martinez Canillas <[e-mail]>

Pekka Paalanen July 19, 2023, 8:15 a.m. UTC | #2

On Tue, 18 Jul 2023 21:42:18 -0400
Zack Rusin <[e-mail]> wrote:

> From: Michael Banack <[e-mail]>
> 
> To clarify the intent and reasoning behind the hotspot properties
> introduce userspace documentation that goes over cursor handling
> in para-virtualized environments.
> 
> The documentation is generic enough to not special case for any
> specific hypervisor and should apply equally to all.
> 
> Signed-off-by: Zack Rusin <[e-mail]>
> ---
>  Documentation/gpu/drm-kms.rst |  6 ++++
>  drivers/gpu/drm/drm_plane.c   | 58 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index c92d425cb2dd..7159b3e90a8a 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -577,6 +577,12 @@ Variable Refresh Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
>     :doc: Variable refresh properties
>  
> +Cursor Hotspot Properties
> +---------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> +   :doc: hotspot properties
> +
>  Existing KMS Properties
>  -----------------------
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 1dc00ad4c33c..f3f2eae83cca 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -230,6 +230,61 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
>  	return 0;
>  }
>  
> +/**
> + * DOC: hotspot properties
> + *
> + * HOTSPOT_X: property to set mouse hotspot x offset.
> + * HOTSPOT_Y: property to set mouse hotspot y offset.
> + *
> + * When the plane is being used as a cursor image to display a mouse pointer,
> + * the "hotspot" is the offset within the cursor image where mouse events
> + * are expected to go.
> + *
> + * Positive values move the hotspot from the top-left corner of the cursor
> + * plane towards the right and bottom.
> + *
> + * Most display drivers do not need this information because the
> + * hotspot is not actually connected to anything visible on screen.
> + * However, this is necessary for display drivers like the para-virtualized
> + * drivers (eg qxl, vbox, virtio, vmwgfx), that are attached to a user console
> + * with a mouse pointer.  Since these consoles are often being remoted over a
> + * network, they would otherwise have to wait to display the pointer movement to
> + * the user until a full network round-trip has occurred.  New mouse events have
> + * to be sent from the user's console, over the network to the virtual input
> + * devices, forwarded to the desktop for processing, and then the cursor plane's
> + * position can be updated and sent back to the user's console over the network.
> + * Instead, with the hotspot information, the console can anticipate the new
> + * location, and draw the mouse cursor there before the confirmation comes in.
> + * To do that correctly, the user's console must be able predict how the
> + * desktop will process mouse events, which normally requires the desktop's
> + * mouse topology information, ie where each CRTC sits in the mouse coordinate
> + * space.  This is typically sent to the para-virtualized drivers using some
> + * driver-specific method, and the driver then forwards it to the console by
> + * way of the virtual display device or hypervisor.
> + *
> + * The assumption is generally made that there is only one cursor plane being
> + * used this way at a time, and that the desktop is feeding all mouse devices
> + * into the same global pointer.  Para-virtualized drivers that require this
> + * should only be exposing a single cursor plane, or find some other way
> + * to coordinate with a userspace desktop that supports multiple pointers.
> + * If the hotspot properties are set, the cursor plane is therefore assumed to be
> + * used only for displaying a mouse cursor image, and the position of the combined
> + * cursor plane + offset can therefore be used for coordinating with input from a
> + * mouse device.
> + *
> + * The cursor will then be drawn either at the location of the plane in the CRTC
> + * console, or as a free-floating cursor plane on the user's console
> + * corresponding to their desktop mouse position.
> + *
> + * DRM clients which would like to work correctly on drivers which expose
> + * hotspot properties should advertise DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT.
> + * Setting this property on drivers which do not special case
> + * cursor planes will return EOPNOTSUPP, which can be used by userspace to
> + * gauge requirements of the hardware/drivers they're running on. Advertising
> + * DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT implies that the userspace client will be
> + * correctly setting the hotspot properties.
> + */

Yes! This is exactly what I was after. Thank you!

> +
>  /**
>   * drm_plane_create_hotspot_properties - creates the mouse hotspot
>   * properties and attaches them to the given cursor plane
> @@ -237,7 +292,8 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
>   * @plane: drm cursor plane
>   *
>   * This function enables the mouse hotspot property on a given
> - * cursor plane.
> + * cursor plane. Look at the documentation for hotspot properties
> + * to get a better understanding for what they're used for.

I haven't seen the rendered HTML, but is there a hyperlink from here to
the hotspot property doc? I think a link would be neat.

>   *
>   * RETURNS:
>   * Zero for success or -errno

Anyway:

Acked-by: Pekka Paalanen <[e-mail]>


Thanks,
pq

Zack Rusin July 20, 2023, 5:03 a.m. UTC | #3

On Wed, 2023-07-19 at 11:15 +0300, Pekka Paalanen wrote:
> On Tue, 18 Jul 2023 21:42:18 -0400
> Zack Rusin <[e-mail]> wrote:
> 
> > From: Michael Banack <[e-mail]>
> > 
> > To clarify the intent and reasoning behind the hotspot properties
> > introduce userspace documentation that goes over cursor handling
> > in para-virtualized environments.
> > 
> > The documentation is generic enough to not special case for any
> > specific hypervisor and should apply equally to all.
> > 
> > Signed-off-by: Zack Rusin <[e-mail]>
> > ---
> >  Documentation/gpu/drm-kms.rst |  6 ++++
> >  drivers/gpu/drm/drm_plane.c   | 58 ++++++++++++++++++++++++++++++++++-
> >  2 files changed, 63 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index c92d425cb2dd..7159b3e90a8a 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -577,6 +577,12 @@ Variable Refresh Properties
> >  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> >     :doc: Variable refresh properties
> >  
> > +Cursor Hotspot Properties
> > +---------------------------
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> > +   :doc: hotspot properties
> > +
> >  Existing KMS Properties
> >  -----------------------
> >  
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index 1dc00ad4c33c..f3f2eae83cca 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -230,6 +230,61 @@ static int create_in_format_blob(struct drm_device *dev,
> > struct drm_plane *plane
> >         return 0;
> >  }
> >  
> > +/**
> > + * DOC: hotspot properties
> > + *
> > + * HOTSPOT_X: property to set mouse hotspot x offset.
> > + * HOTSPOT_Y: property to set mouse hotspot y offset.
> > + *
> > + * When the plane is being used as a cursor image to display a mouse pointer,
> > + * the "hotspot" is the offset within the cursor image where mouse events
> > + * are expected to go.
> > + *
> > + * Positive values move the hotspot from the top-left corner of the cursor
> > + * plane towards the right and bottom.
> > + *
> > + * Most display drivers do not need this information because the
> > + * hotspot is not actually connected to anything visible on screen.
> > + * However, this is necessary for display drivers like the para-virtualized
> > + * drivers (eg qxl, vbox, virtio, vmwgfx), that are attached to a user console
> > + * with a mouse pointer.  Since these consoles are often being remoted over a
> > + * network, they would otherwise have to wait to display the pointer movement
> > to
> > + * the user until a full network round-trip has occurred.  New mouse events
> > have
> > + * to be sent from the user's console, over the network to the virtual input
> > + * devices, forwarded to the desktop for processing, and then the cursor
> > plane's
> > + * position can be updated and sent back to the user's console over the
> > network.
> > + * Instead, with the hotspot information, the console can anticipate the new
> > + * location, and draw the mouse cursor there before the confirmation comes in.
> > + * To do that correctly, the user's console must be able predict how the
> > + * desktop will process mouse events, which normally requires the desktop's
> > + * mouse topology information, ie where each CRTC sits in the mouse coordinate
> > + * space.  This is typically sent to the para-virtualized drivers using some
> > + * driver-specific method, and the driver then forwards it to the console by
> > + * way of the virtual display device or hypervisor.
> > + *
> > + * The assumption is generally made that there is only one cursor plane being
> > + * used this way at a time, and that the desktop is feeding all mouse devices
> > + * into the same global pointer.  Para-virtualized drivers that require this
> > + * should only be exposing a single cursor plane, or find some other way
> > + * to coordinate with a userspace desktop that supports multiple pointers.
> > + * If the hotspot properties are set, the cursor plane is therefore assumed to
> > be
> > + * used only for displaying a mouse cursor image, and the position of the
> > combined
> > + * cursor plane + offset can therefore be used for coordinating with input from
> > a
> > + * mouse device.
> > + *
> > + * The cursor will then be drawn either at the location of the plane in the
> > CRTC
> > + * console, or as a free-floating cursor plane on the user's console
> > + * corresponding to their desktop mouse position.
> > + *
> > + * DRM clients which would like to work correctly on drivers which expose
> > + * hotspot properties should advertise DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT.
> > + * Setting this property on drivers which do not special case
> > + * cursor planes will return EOPNOTSUPP, which can be used by userspace to
> > + * gauge requirements of the hardware/drivers they're running on. Advertising
> > + * DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT implies that the userspace client will
> > be
> > + * correctly setting the hotspot properties.
> > + */
> 
> Yes! This is exactly what I was after. Thank you!
> 
> > +
> >  /**
> >   * drm_plane_create_hotspot_properties - creates the mouse hotspot
> >   * properties and attaches them to the given cursor plane
> > @@ -237,7 +292,8 @@ static int create_in_format_blob(struct drm_device *dev,
> > struct drm_plane *plane
> >   * @plane: drm cursor plane
> >   *
> >   * This function enables the mouse hotspot property on a given
> > - * cursor plane.
> > + * cursor plane. Look at the documentation for hotspot properties
> > + * to get a better understanding for what they're used for.
> 
> I haven't seen the rendered HTML, but is there a hyperlink from here to
> the hotspot property doc? I think a link would be neat.

drm_plane_create_hotspot_properties is now a static function so it's not
generated in the docs (but to be fair, I'm also not aware of how to link to drm-
kms.rst section from within function docs because I tried that first only to realize
it's a static function and not in the generated html docs).

I'll give this series a few more hours on the list and if no one objects I'll push
it to drm-misc later today. Thanks!

z

Simon Ser July 20, 2023, 6:47 a.m. UTC | #4

On Thursday, July 20th, 2023 at 07:03, Zack Rusin <[e-mail]> wrote:

> I'll give this series a few more hours on the list and if no one objects I'll push
> it to drm-misc later today. Thanks!

Sorry, but this doesn't seem to be enough to satisfy the DRM merge
requirements. This introduces a new uAPI but is missing user-space
patches and IGT. See [1] and [2].

[1]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#requirements
[2]: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements

Javier Martinez Canillas July 20, 2023, 8:50 a.m. UTC | #5

Simon Ser <[e-mail]> writes:

Hello Simon,

> On Thursday, July 20th, 2023 at 07:03, Zack Rusin <[e-mail]> wrote:
>
>> I'll give this series a few more hours on the list and if no one objects I'll push
>> it to drm-misc later today. Thanks!
>
> Sorry, but this doesn't seem to be enough to satisfy the DRM merge
> requirements. This introduces a new uAPI but is missing user-space
> patches and IGT. See [1] and [2].
>
> [1]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#requirements
> [2]: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
>

Albert (Cc'ed) wrote IGT tests for this new uAPI and was waiting for
Zack's patches to land to post them. I believe his branch is [0] but
he can correct me if I'm wrong on that.

Zack also has mutter patches and Albert has been testing those too.

[0]: https://gitlab.freedesktop.org/aesteve/igt-gpu-tools/-/commits/modeset-cursor-hotspot-test/

Simon Ser July 20, 2023, 9:07 a.m. UTC | #6

On Thursday, July 20th, 2023 at 10:50, Javier Martinez Canillas <[e-mail]> wrote:

> > On Thursday, July 20th, 2023 at 07:03, Zack Rusin [e-mail] wrote:
> > 
> > > I'll give this series a few more hours on the list and if no one objects I'll push
> > > it to drm-misc later today. Thanks!
> > 
> > Sorry, but this doesn't seem to be enough to satisfy the DRM merge
> > requirements. This introduces a new uAPI but is missing user-space
> > patches and IGT. See 1 and 2.
> 
> 
> Albert (Cc'ed) wrote IGT tests for this new uAPI and was waiting for
> Zack's patches to land to post them. I believe his branch is [0] but
> he can correct me if I'm wrong on that.
> 
> Zack also has mutter patches and Albert has been testing those too.
> 
> [0]: https://gitlab.freedesktop.org/aesteve/igt-gpu-tools/-/commits/modeset-cursor-hotspot-test/

Ah, nice. Please do post all of these (without merging them) and
include links to them in the commit message. Posting is important
to make sure there are no gaps/mistakes in the tests and user-space
impl.

Simon Ser July 20, 2023, 9:51 a.m. UTC | #7

On Wednesday, July 19th, 2023 at 03:42, Zack Rusin <[e-mail]> wrote:

> +/**
> + * DOC: hotspot properties
> + *
> + * HOTSPOT_X: property to set mouse hotspot x offset.
> + * HOTSPOT_Y: property to set mouse hotspot y offset.
> + *
> + * When the plane is being used as a cursor image to display a mouse pointer,
> + * the "hotspot" is the offset within the cursor image where mouse events
> + * are expected to go.
> + *
> + * Positive values move the hotspot from the top-left corner of the cursor
> + * plane towards the right and bottom.
> + *
> + * Most display drivers do not need this information because the
> + * hotspot is not actually connected to anything visible on screen.
> + * However, this is necessary for display drivers like the para-virtualized
> + * drivers (eg qxl, vbox, virtio, vmwgfx), that are attached to a user console
> + * with a mouse pointer.  Since these consoles are often being remoted over a
> + * network, they would otherwise have to wait to display the pointer movement to
> + * the user until a full network round-trip has occurred.  New mouse events have
> + * to be sent from the user's console, over the network to the virtual input
> + * devices, forwarded to the desktop for processing, and then the cursor plane's
> + * position can be updated and sent back to the user's console over the network.
> + * Instead, with the hotspot information, the console can anticipate the new
> + * location, and draw the mouse cursor there before the confirmation comes in.
> + * To do that correctly, the user's console must be able predict how the
> + * desktop will process mouse events, which normally requires the desktop's
> + * mouse topology information, ie where each CRTC sits in the mouse coordinate
> + * space.  This is typically sent to the para-virtualized drivers using some
> + * driver-specific method, and the driver then forwards it to the console by
> + * way of the virtual display device or hypervisor.
> + *
> + * The assumption is generally made that there is only one cursor plane being
> + * used this way at a time, and that the desktop is feeding all mouse devices
> + * into the same global pointer.  Para-virtualized drivers that require this
> + * should only be exposing a single cursor plane, or find some other way
> + * to coordinate with a userspace desktop that supports multiple pointers.
> + * If the hotspot properties are set, the cursor plane is therefore assumed to be
> + * used only for displaying a mouse cursor image, and the position of the combined
> + * cursor plane + offset can therefore be used for coordinating with input from a
> + * mouse device.
> + *
> + * The cursor will then be drawn either at the location of the plane in the CRTC
> + * console, or as a free-floating cursor plane on the user's console
> + * corresponding to their desktop mouse position.
> + *
> + * DRM clients which would like to work correctly on drivers which expose
> + * hotspot properties should advertise DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT.

Nit: an ampersand ("&") can be used to linkify that cap.

> + * Setting this property on drivers which do not special case
> + * cursor planes will return EOPNOTSUPP, which can be used by userspace to
> + * gauge requirements of the hardware/drivers they're running on. Advertising
> + * DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT implies that the userspace client will be
> + * correctly setting the hotspot properties.

Thanks a lot for writing these docs! It's super helpful!

Reviewed-by: Simon Ser <[e-mail]>

Zack Rusin July 20, 2023, 7:28 p.m. UTC | #8

On Thu, 2023-07-20 at 09:07 +0000, Simon Ser wrote:
> !! External Email
>
> On Thursday, July 20th, 2023 at 10:50, Javier Martinez Canillas
> <[e-mail]> wrote:
>
> > > On Thursday, July 20th, 2023 at 07:03, Zack Rusin [e-mail] wrote:
> > >
> > > > I'll give this series a few more hours on the list and if no one objects
> > > > I'll push
> > > > it to drm-misc later today. Thanks!
> > >
> > > Sorry, but this doesn't seem to be enough to satisfy the DRM merge
> > > requirements. This introduces a new uAPI but is missing user-space
> > > patches and IGT. See 1 and 2.
> >
> >
> > Albert (Cc'ed) wrote IGT tests for this new uAPI and was waiting for
> > Zack's patches to land to post them. I believe his branch is [0] but
> > he can correct me if I'm wrong on that.
> >
> > Zack also has mutter patches and Albert has been testing those too.
> >
> > [0]:
> > https://gitlab.freedesktop.org/aesteve/igt-gpu-tools/-/commits/modeset-cursor-hotspot-test/
>
> Ah, nice. Please do post all of these (without merging them) and
> include links to them in the commit message. Posting is important
> to make sure there are no gaps/mistakes in the tests and user-space
> impl.

What should those links point to? Because my private mutter repository is definitely
not going to last long so I'm not sure if there's any point in putting that in a
kernel commit log. Or would you like the links to those in the cover letter?

z

Simon Ser July 20, 2023, 7:32 p.m. UTC | #9

On Thursday, July 20th, 2023 at 21:28, Zack Rusin <[e-mail]> wrote:

> On Thu, 2023-07-20 at 09:07 +0000, Simon Ser wrote:
> 
> > !! External Email
> > 
> > On Thursday, July 20th, 2023 at 10:50, Javier Martinez Canillas
> > [e-mail] wrote:
> > 
> > > > On Thursday, July 20th, 2023 at 07:03, Zack Rusin [e-mail] wrote:
> > > > 
> > > > > I'll give this series a few more hours on the list and if no one objects
> > > > > I'll push
> > > > > it to drm-misc later today. Thanks!
> > > > 
> > > > Sorry, but this doesn't seem to be enough to satisfy the DRM merge
> > > > requirements. This introduces a new uAPI but is missing user-space
> > > > patches and IGT. See 1 and 2.
> > > 
> > > Albert (Cc'ed) wrote IGT tests for this new uAPI and was waiting for
> > > Zack's patches to land to post them. I believe his branch is 0 but
> > > he can correct me if I'm wrong on that.
> > > 
> > > Zack also has mutter patches and Albert has been testing those too.
> > 
> > Ah, nice. Please do post all of these (without merging them) and
> > include links to them in the commit message. Posting is important
> > to make sure there are no gaps/mistakes in the tests and user-space
> > impl.
> 
> What should those links point to? Because my private mutter repository is definitely
> not going to last long so I'm not sure if there's any point in putting that in a
> kernel commit log. Or would you like the links to those in the cover letter?

The kernel docs say: "The userspace side must be fully reviewed and
tested to the standards of that userspace project".

So you need to open a merge request for mutter. Same for IGT.

Albert Esteve July 24, 2023, 11:04 a.m. UTC | #10

On Thu, Jul 20, 2023 at 9:32 PM Simon Ser <[e-mail]> wrote:

> On Thursday, July 20th, 2023 at 21:28, Zack Rusin <[e-mail]>
> wrote:
>
> > On Thu, 2023-07-20 at 09:07 +0000, Simon Ser wrote:
> >
> > > !! External Email
> > >
> > > On Thursday, July 20th, 2023 at 10:50, Javier Martinez Canillas
> > > [e-mail] wrote:
> > >
> > > > > On Thursday, July 20th, 2023 at 07:03, Zack Rusin [e-mail]
> wrote:
> > > > >
> > > > > > I'll give this series a few more hours on the list and if no one
> objects
> > > > > > I'll push
> > > > > > it to drm-misc later today. Thanks!
> > > > >
> > > > > Sorry, but this doesn't seem to be enough to satisfy the DRM merge
> > > > > requirements. This introduces a new uAPI but is missing user-space
> > > > > patches and IGT. See 1 and 2.
> > > >
> > > > Albert (Cc'ed) wrote IGT tests for this new uAPI and was waiting for
> > > > Zack's patches to land to post them. I believe his branch is 0 but
> > > > he can correct me if I'm wrong on that.
> > > >
> > > > Zack also has mutter patches and Albert has been testing those too.
> > >
> > > Ah, nice. Please do post all of these (without merging them) and
> > > include links to them in the commit message. Posting is important
> > > to make sure there are no gaps/mistakes in the tests and user-space
> > > impl.
> >
> > What should those links point to? Because my private mutter repository
> is definitely
> > not going to last long so I'm not sure if there's any point in putting
> that in a
> > kernel commit log. Or would you like the links to those in the cover
> letter?
>
> The kernel docs say: "The userspace side must be fully reviewed and
> tested to the standards of that userspace project".
>
> So you need to open a merge request for mutter. Same for IGT.
>
>
Hi,

Here's the link to the IGT patch:
https://lists.freedesktop.org/archives/igt-dev/2023-July/058427.html

Albert Esteve Aug. 10, 2023, 11:06 a.m. UTC | #11

On Mon, Jul 24, 2023 at 1:04 PM Albert Esteve <[e-mail]> wrote:

>
>
> On Thu, Jul 20, 2023 at 9:32 PM Simon Ser <[e-mail]> wrote:
>
>> On Thursday, July 20th, 2023 at 21:28, Zack Rusin <[e-mail]>
>> wrote:
>>
>> > On Thu, 2023-07-20 at 09:07 +0000, Simon Ser wrote:
>> >
>> > > !! External Email
>> > >
>> > > On Thursday, July 20th, 2023 at 10:50, Javier Martinez Canillas
>> > > [e-mail] wrote:
>> > >
>> > > > > On Thursday, July 20th, 2023 at 07:03, Zack Rusin
>> [e-mail] wrote:
>> > > > >
>> > > > > > I'll give this series a few more hours on the list and if no
>> one objects
>> > > > > > I'll push
>> > > > > > it to drm-misc later today. Thanks!
>> > > > >
>> > > > > Sorry, but this doesn't seem to be enough to satisfy the DRM merge
>> > > > > requirements. This introduces a new uAPI but is missing user-space
>> > > > > patches and IGT. See 1 and 2.
>> > > >
>> > > > Albert (Cc'ed) wrote IGT tests for this new uAPI and was waiting for
>> > > > Zack's patches to land to post them. I believe his branch is 0 but
>> > > > he can correct me if I'm wrong on that.
>> > > >
>> > > > Zack also has mutter patches and Albert has been testing those too.
>> > >
>> > > Ah, nice. Please do post all of these (without merging them) and
>> > > include links to them in the commit message. Posting is important
>> > > to make sure there are no gaps/mistakes in the tests and user-space
>> > > impl.
>> >
>> > What should those links point to? Because my private mutter repository
>> is definitely
>> > not going to last long so I'm not sure if there's any point in putting
>> that in a
>> > kernel commit log. Or would you like the links to those in the cover
>> letter?
>>
>> The kernel docs say: "The userspace side must be fully reviewed and
>> tested to the standards of that userspace project".
>>
>> So you need to open a merge request for mutter. Same for IGT.
>>
>>
> Hi,
>
> Here's the link to the IGT patch:
> https://lists.freedesktop.org/archives/igt-dev/2023-July/058427.html
>

Hi,

The IGT patch series already got the Reviewed-by flags.

Here's the link https://patchwork.freedesktop.org/series/121225/

@zackr Is the mutter patch posted too? If so, IIUC the next step would be
to post a new revision
of this patch with the links to the igt and mutter patches.
Is there anything else missing?

BR,
Albert

Patch

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index c92d425cb2dd..7159b3e90a8a 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -577,6 +577,12 @@  Variable Refresh Properties
 .. kernel-doc:: drivers/gpu/drm/drm_connector.c
    :doc: Variable refresh properties
 
+Cursor Hotspot Properties
+---------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: hotspot properties
+
 Existing KMS Properties
 -----------------------
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 1dc00ad4c33c..f3f2eae83cca 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -230,6 +230,61 @@  static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
 	return 0;
 }
 
+/**
+ * DOC: hotspot properties
+ *
+ * HOTSPOT_X: property to set mouse hotspot x offset.
+ * HOTSPOT_Y: property to set mouse hotspot y offset.
+ *
+ * When the plane is being used as a cursor image to display a mouse pointer,
+ * the "hotspot" is the offset within the cursor image where mouse events
+ * are expected to go.
+ *
+ * Positive values move the hotspot from the top-left corner of the cursor
+ * plane towards the right and bottom.
+ *
+ * Most display drivers do not need this information because the
+ * hotspot is not actually connected to anything visible on screen.
+ * However, this is necessary for display drivers like the para-virtualized
+ * drivers (eg qxl, vbox, virtio, vmwgfx), that are attached to a user console
+ * with a mouse pointer.  Since these consoles are often being remoted over a
+ * network, they would otherwise have to wait to display the pointer movement to
+ * the user until a full network round-trip has occurred.  New mouse events have
+ * to be sent from the user's console, over the network to the virtual input
+ * devices, forwarded to the desktop for processing, and then the cursor plane's
+ * position can be updated and sent back to the user's console over the network.
+ * Instead, with the hotspot information, the console can anticipate the new
+ * location, and draw the mouse cursor there before the confirmation comes in.
+ * To do that correctly, the user's console must be able predict how the
+ * desktop will process mouse events, which normally requires the desktop's
+ * mouse topology information, ie where each CRTC sits in the mouse coordinate
+ * space.  This is typically sent to the para-virtualized drivers using some
+ * driver-specific method, and the driver then forwards it to the console by
+ * way of the virtual display device or hypervisor.
+ *
+ * The assumption is generally made that there is only one cursor plane being
+ * used this way at a time, and that the desktop is feeding all mouse devices
+ * into the same global pointer.  Para-virtualized drivers that require this
+ * should only be exposing a single cursor plane, or find some other way
+ * to coordinate with a userspace desktop that supports multiple pointers.
+ * If the hotspot properties are set, the cursor plane is therefore assumed to be
+ * used only for displaying a mouse cursor image, and the position of the combined
+ * cursor plane + offset can therefore be used for coordinating with input from a
+ * mouse device.
+ *
+ * The cursor will then be drawn either at the location of the plane in the CRTC
+ * console, or as a free-floating cursor plane on the user's console
+ * corresponding to their desktop mouse position.
+ *
+ * DRM clients which would like to work correctly on drivers which expose
+ * hotspot properties should advertise DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT.
+ * Setting this property on drivers which do not special case
+ * cursor planes will return EOPNOTSUPP, which can be used by userspace to
+ * gauge requirements of the hardware/drivers they're running on. Advertising
+ * DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT implies that the userspace client will be
+ * correctly setting the hotspot properties.
+ */
+
 /**
  * drm_plane_create_hotspot_properties - creates the mouse hotspot
  * properties and attaches them to the given cursor plane
@@ -237,7 +292,8 @@  static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
  * @plane: drm cursor plane
  *
  * This function enables the mouse hotspot property on a given
- * cursor plane.
+ * cursor plane. Look at the documentation for hotspot properties
+ * to get a better understanding for what they're used for.
  *
  * RETURNS:
  * Zero for success or -errno