Re: [PATCH v3 8/8] drm: Introduce DRM_CLIENT_CAP_VIRTUALIZED_CURSOR_PLANE

[Pekka Paalanen <ppaalanen@gmail.com>]

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

On Mon, 26 Jun 2023 23:58:39 -0400
Zack Rusin <zack@kde.org> wrote:

> From: Zack Rusin <zackr@vmware.com>
> 
> Virtualized drivers place additional restrictions on the cursor plane
> which breaks the contract of universal planes. To allow atomic
> modesettings with virtualized drivers the clients need to advertise
> that they're capable of dealing with those extra restrictions.
> 
> To do that introduce DRM_CLIENT_CAP_VIRTUALIZED_CURSOR_PLANE which
> lets DRM know that the client is aware of and capable of dealing with
> the extra restrictions on the virtual cursor plane.
> 
> Setting this option to true makes DRM expose the cursor plane on
> virtualized drivers. The userspace is expected to set the hotspots
> and handle mouse events on that plane.
> 
> 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>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_ioctl.c |  9 +++++++++
>  include/uapi/drm/drm.h      | 26 ++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 8e9afe7af19c..6fd17ff14656 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -361,6 +361,15 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>  			return -EINVAL;
>  		file_priv->writeback_connectors = req->value;
>  		break;
> +	case DRM_CLIENT_CAP_VIRTUALIZED_CURSOR_PLANE:
> +		if (!drm_core_check_feature(dev, DRIVER_CURSOR_HOTSPOT))
> +			return -EOPNOTSUPP;
> +		if (!file_priv->atomic)
> +			return -EINVAL;
> +		if (req->value > 1)
> +			return -EINVAL;
> +		file_priv->supports_virtualized_cursor_plane = req->value;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index a87bbbbca2d4..057ef2a16d31 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -836,6 +836,32 @@ struct drm_get_cap {
>   */
>  #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS	5
>  
> +/**
> + * DRM_CLIENT_CAP_VIRTUALIZED_CURSOR_PLANE
> + *
> + * Drivers for para-virtualized hardware (e.g. vmwgfx, qxl, virtio and
> + * virtualbox) have additional restrictions for cursor planes (thus
> + * making cursor planes on those drivers not truly universal,) e.g.
> + * they need cursor planes to act like one would expect from a mouse
> + * cursor and have correctly set hotspot properties.
> + * If this client cap is not set the DRM core will hide cursor plane on
> + * those virtualized drivers because not setting it implies that the
> + * client is not capable of dealing with those extra restictions.
> + * Clients which do set cursor hotspot and treat the cursor plane
> + * like a mouse cursor should set this property.
> + * The client must enable &DRM_CLIENT_CAP_ATOMIC first.
> + *
> + * Setting this property on drivers which do not special case
> + * cursor planes (i.e. non-virtualized drivers) will return
> + * EOPNOTSUPP, which can be used by userspace to gauge
> + * requirements of the hardware/drivers they're running on.
> + *
> + * This capability is always supported for atomic-capable virtualized
> + * drivers starting from kernel version 6.5.
> + */
> +#define DRM_CLIENT_CAP_VIRTUALIZED_CURSOR_PLANE	6

All this sounds really good!

Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>

The only nitpick I can come up with is maybe naming it to
DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT to distil the essence of the
semantics in the name. I find the word "virtualized" having too many
possible meanings in this context. In any case, the doc makes it very
clear what this is.

I think this feature does not need to be limited to virtualized
drivers. If your display hardware system implements gaze tracking, it
could show a hardware assisted gaze cursor with this.


Thanks,
pq

> +
> +
>  /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
>  struct drm_set_client_cap {
>  	__u64 capability;


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