On Mon, 26 Jun 2023 23:58:32 -0400
Zack Rusin wrote:
> From: Zack Rusin
>
> Cursor planes on virtualized drivers have special meaning and require
> that the clients handle them in specific ways, e.g. the cursor plane
> should react to the mouse movement the way a mouse cursor would be
> expected to and the client is required to set hotspot properties on it
> in order for the mouse events to be routed correctly.
>
> This breaks the contract as specified by the "universal planes". Fix it
> by disabling the cursor planes on virtualized drivers while adding
> a foundation on top of which it's possible to special case mouse cursor
> planes for clients that want it.
>
> Disabling the cursor planes makes some kms compositors which were broken,
> e.g. Weston, fallback to software cursor which works fine or at least
> better than currently while having no effect on others, e.g. gnome-shell
> or kwin, which put virtualized drivers on a deny-list when running in
> atomic context to make them fallback to legacy kms and avoid this issue.
>
> Signed-off-by: Zack Rusin
> Fixes: 681e7ec73044 ("drm: Allow userspace to ask for universal plane list
> (v2)")
Sounds good to me.
Acked-by: Pekka Paalanen
Thanks,
pq
> Cc: # v5.4+
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Dave Airlie
> Cc: Gerd Hoffmann
> Cc: Hans de Goede
> Cc: Gurchetan Singh
> Cc: Chia-I Wu
> Cc: dri-de...@lists.freedesktop.org
> Cc: virtualizat...@lists.linux-foundation.org
> Cc: spice-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/drm_plane.c | 13 +
> drivers/gpu/drm/qxl/qxl_drv.c| 2 +-
> drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
> include/drm/drm_drv.h| 9 +
> include/drm/drm_file.h | 12
> 7 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 24e7998d1731..a4a39f4834e2 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -678,6 +678,19 @@ int drm_mode_getplane_res(struct drm_device *dev, void
> *data,
> !file_priv->universal_planes)
> continue;
>
> + /*
> + * If we're running on a virtualized driver then,
> + * unless userspace advertizes support for the
> + * virtualized cursor plane, disable cursor planes
> + * because they'll be broken due to missing cursor
> + * hotspot info.
> + */
> + if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> + drm_core_check_feature(dev, DRIVER_CURSOR_HOTSPOT) &&
> + file_priv->atomic &&
> + !file_priv->supports_virtualized_cursor_plane)
> + continue;
> +
> if (drm_lease_held(file_priv, plane->base.id)) {
> if (count < plane_resp->count_planes &&
> put_user(plane->base.id, plane_ptr + count))
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index b30ede1cf62d..91930e84a9cd 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -283,7 +283,7 @@ static const struct drm_ioctl_desc qxl_ioctls[] = {
> };
>
> static struct drm_driver qxl_driver = {
> - .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC |
> DRIVER_CURSOR_HOTSPOT,
>
> .dumb_create = qxl_mode_dumb_create,
> .dumb_map_offset = drm_gem_ttm_dumb_map_offset,
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index 4fee15c97c34..8ecd0863fad7 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -172,7 +172,7 @@ DEFINE_DRM_GEM_FOPS(vbox_fops);
>
> static const struct drm_driver driver = {
> .driver_features =
> - DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
> + DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC | DRIVER_CURSOR_HOTSPOT,
>
> .fops = _fops,
> .name = DRIVER_NAME,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
> b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index a7ec5a3770da..8f4bb8a4e952 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -176,7 +176,7 @@ static const struct drm_driver driver = {
>* If KMS is disabled DRIVER_MODESET and DRIVER_ATOMIC are masked
>* out via drm_device::driver_features:
>*/
> - .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_RENDER |
> DRIVER_ATOMIC,
> + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_RENDER |
> DRIVER_ATOMIC | DRIVER_CURSOR_HOTSPOT,
> .open =
