Pekka Paalanen
2023-Jun-27 08:18 UTC
[PATCH v3 1/8] drm: Disable the cursor plane on atomic contexts with virtualized drivers
On Mon, 26 Jun 2023 23:58:32 -0400 Zack Rusin <zack at kde.org> wrote:> From: Zack Rusin <zackr at vmware.com> > > 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 <zackr at vmware.com> > Fixes: 681e7ec73044 ("drm: Allow userspace to ask for universal plane list (v2)")Sounds good to me. Acked-by: Pekka Paalanen <pekka.paalanen at collabora.com> Thanks, pq> Cc: <stable at vger.kernel.org> # v5.4+ > 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> > Cc: Dave Airlie <airlied at redhat.com> > Cc: Gerd Hoffmann <kraxel at redhat.com> > Cc: Hans de Goede <hdegoede at redhat.com> > Cc: Gurchetan Singh <gurchetansingh at chromium.org> > Cc: Chia-I Wu <olvaffe at gmail.com> > Cc: dri-devel at lists.freedesktop.org > Cc: virtualization at lists.linux-foundation.org > Cc: spice-devel at 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 = &vbox_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 = virtio_gpu_driver_open, > .postclose = virtio_gpu_driver_postclose, > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index 8b24ecf60e3e..d3e308fdfd5b 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -1611,7 +1611,7 @@ static const struct file_operations vmwgfx_driver_fops = { > > static const struct drm_driver driver = { > .driver_features > - DRIVER_MODESET | DRIVER_RENDER | DRIVER_ATOMIC | DRIVER_GEM, > + DRIVER_MODESET | DRIVER_RENDER | DRIVER_ATOMIC | DRIVER_GEM | DRIVER_CURSOR_HOTSPOT, > .ioctls = vmw_ioctls, > .num_ioctls = ARRAY_SIZE(vmw_ioctls), > .master_set = vmw_master_set, > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index b77f2c7275b7..8303016665dd 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -104,6 +104,15 @@ enum drm_driver_feature { > * acceleration should be handled by two drivers that are connected using auxiliary bus. > */ > DRIVER_COMPUTE_ACCEL = BIT(7), > + /** > + * @DRIVER_CURSOR_HOTSPOT: > + * > + * Driver supports and requires cursor hotspot information in the > + * cursor plane (e.g. cursor plane has to actually track the mouse > + * cursor and the clients are required to set hotspot in order for > + * the cursor planes to work correctly). > + */ > + DRIVER_CURSOR_HOTSPOT = BIT(8), > > /* IMPORTANT: Below are all the legacy flags, add new ones above. */ > > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > index 966912053cb0..91cf7f452f86 100644 > --- a/include/drm/drm_file.h > +++ b/include/drm/drm_file.h > @@ -228,6 +228,18 @@ struct drm_file { > */ > bool is_master; > > + /** > + * @supports_virtualized_cursor_plane: > + * > + * This client is capable of handling the cursor plane with the > + * restrictions imposed on it by the virtualized drivers. > + * > + * This implies that the cursor plane has to behave like a cursor > + * i.e. track cursor movement. It also requires setting of the > + * hotspot properties by the client on the cursor plane. > + */ > + bool supports_virtualized_cursor_plane; > + > /** > * @master: > *-------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 833 bytes Desc: OpenPGP digital signature URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20230627/a8ecd8c3/attachment-0001.sig>