diff mbox series

[v2,5/8] drm/vboxvideo: Use the hotspot properties from cursor planes

Message ID 20220712033246.1148476-6-zack@kde.org (mailing list archive)
State New, archived
Headers show
Series Fix cursor planes with virtualized drivers | expand

Commit Message

Zack Rusin July 12, 2022, 3:32 a.m. UTC
From: Zack Rusin <zackr@vmware.com>

Atomic modesetting got support for mouse hotspots via the hotspot
properties. Port the legacy kms hotspot handling to the new properties
on cursor planes.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/vboxvideo/vbox_mode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Pekka Paalanen July 12, 2022, 7:56 a.m. UTC | #1
On Mon, 11 Jul 2022 23:32:43 -0400
Zack Rusin <zack@kde.org> wrote:

> From: Zack Rusin <zackr@vmware.com>
> 
> Atomic modesetting got support for mouse hotspots via the hotspot
> properties. Port the legacy kms hotspot handling to the new properties
> on cursor planes.
> 
> Signed-off-by: Zack Rusin <zackr@vmware.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  drivers/gpu/drm/vboxvideo/vbox_mode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> index fa0d73ce07bc..ca3134adb104 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> @@ -429,8 +429,8 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
>  	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
>  		VBOX_MOUSE_POINTER_ALPHA;
>  	hgsmi_update_pointer_shape(vbox->guest_pool, flags,
> -				   min_t(u32, max(fb->hot_x, 0), width),
> -				   min_t(u32, max(fb->hot_y, 0), height),
> +				   min_t(u32, max(new_state->hotspot_x, 0), width),
> +				   min_t(u32, max(new_state->hotspot_y, 0), height),
>  				   width, height, vbox->cursor_data, data_size);
>  
>  	mutex_unlock(&vbox->hw_mutex);

Hi,

this looks like silent clamping of the hotspot coordinates.

Should the DRM core perhaps already ensure that the hotspot must reside
inside the plane (fb) boundaries and reject the atomic (TEST_ONLY)
commit when it's outside?

Or if this restriction is not universal, maybe this driver should
reject invalid hotspots rather than silently mangle them?

Also, if supports_virtual_cursor_plane is false, should there not be
somewhere code that will ignore the values set to the atomic hotspot
properties?

When one KMS client switches to another, the first one being hotspot
aware and the latter not, and both atomic, then the latter KMS client
who doesn't know to drive the hotspot will inherit potentially invalid
hotspot from the previous KMS client. Does something prevent that from
being a problem?

The old KMS client may have left the virtual cursor plane visible, and
the new KMS client doesn't even know the virtual cursor plane exists
because it doesn't set the DRM client cap. Something should probably
ensure the virtual cursor plane gets disabled, right?


Thanks,
pq
Zack Rusin July 13, 2022, 3:35 a.m. UTC | #2
On Tue, 2022-07-12 at 10:56 +0300, Pekka Paalanen wrote:
> On Mon, 11 Jul 2022 23:32:43 -0400
> Zack Rusin <zack@kde.org> wrote:
> 
> > From: Zack Rusin <zackr@vmware.com>
> > 
> > Atomic modesetting got support for mouse hotspots via the hotspot
> > properties. Port the legacy kms hotspot handling to the new properties
> > on cursor planes.
> > 
> > Signed-off-by: Zack Rusin <zackr@vmware.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > ---
> >  drivers/gpu/drm/vboxvideo/vbox_mode.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > index fa0d73ce07bc..ca3134adb104 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > @@ -429,8 +429,8 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
> >  	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
> >  		VBOX_MOUSE_POINTER_ALPHA;
> >  	hgsmi_update_pointer_shape(vbox->guest_pool, flags,
> > -				   min_t(u32, max(fb->hot_x, 0), width),
> > -				   min_t(u32, max(fb->hot_y, 0), height),
> > +				   min_t(u32, max(new_state->hotspot_x, 0), width),
> > +				   min_t(u32, max(new_state->hotspot_y, 0), height),
> >  				   width, height, vbox->cursor_data, data_size);
> >  
> >  	mutex_unlock(&vbox->hw_mutex);
> 
> Hi,
> 
> this looks like silent clamping of the hotspot coordinates.
> 
> Should the DRM core perhaps already ensure that the hotspot must reside
> inside the plane (fb) boundaries and reject the atomic (TEST_ONLY)
> commit when it's outside?
> 
> Or if this restriction is not universal, maybe this driver should
> reject invalid hotspots rather than silently mangle them?

TBH, I'm not really sure why vboxvideo is clamping those values. I didn't want to
introduce any regressions by changing it here, but the hotspot code never specified
that the hotspot offsets have to be positive or even within the plane. In a quick
search I couldn't find real world cursors that were doing anything like that though
so I just left it.

> Also, if supports_virtual_cursor_plane is false, should there not be
> somewhere code that will ignore the values set to the atomic hotspot
> properties?

Hmm, good question. I'm not sure if there's a case where that could be possible:
without supports_virtual_cursor we either won't have a cursor plane or the client
won't be atomic and will only be allowed to use the old legacy kms ioctl's, i.e.
drmModeSetCursor2.

> When one KMS client switches to another, the first one being hotspot
> aware and the latter not, and both atomic, then the latter KMS client
> who doesn't know to drive the hotspot will inherit potentially invalid
> hotspot from the previous KMS client. Does something prevent that from
> being a problem?

That switch will result in plane state reset, ending in
__drm_atomic_helper_plane_state_reset which will reset both hotspot properties (set
them to 0).

> The old KMS client may have left the virtual cursor plane visible, and
> the new KMS client doesn't even know the virtual cursor plane exists
> because it doesn't set the DRM client cap. Something should probably
> ensure the virtual cursor plane gets disabled, right?

Hah, that's also a good question. I *think* the same code to above would be ran,
i.e. plane reset which should result in the plane disappearing and the new client
not being able to drive it anymore. At least when running gnome-shell, switching to
weston and then switching to gnome-shell things look ok, but I haven't tried running
such clients at the same time.

z
Pekka Paalanen July 13, 2022, 7:20 a.m. UTC | #3
On Wed, 13 Jul 2022 03:35:57 +0000
Zack Rusin <zackr@vmware.com> wrote:

> On Tue, 2022-07-12 at 10:56 +0300, Pekka Paalanen wrote:
> > On Mon, 11 Jul 2022 23:32:43 -0400
> > Zack Rusin <zack@kde.org> wrote:
> >   
> > > From: Zack Rusin <zackr@vmware.com>
> > > 
> > > Atomic modesetting got support for mouse hotspots via the hotspot
> > > properties. Port the legacy kms hotspot handling to the new properties
> > > on cursor planes.
> > > 
> > > Signed-off-by: Zack Rusin <zackr@vmware.com>
> > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > ---
> > >  drivers/gpu/drm/vboxvideo/vbox_mode.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > index fa0d73ce07bc..ca3134adb104 100644
> > > --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > @@ -429,8 +429,8 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
> > >  	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
> > >  		VBOX_MOUSE_POINTER_ALPHA;
> > >  	hgsmi_update_pointer_shape(vbox->guest_pool, flags,
> > > -				   min_t(u32, max(fb->hot_x, 0), width),
> > > -				   min_t(u32, max(fb->hot_y, 0), height),
> > > +				   min_t(u32, max(new_state->hotspot_x, 0), width),
> > > +				   min_t(u32, max(new_state->hotspot_y, 0), height),
> > >  				   width, height, vbox->cursor_data, data_size);
> > >  
> > >  	mutex_unlock(&vbox->hw_mutex);  
> > 
> > Hi,
> > 
> > this looks like silent clamping of the hotspot coordinates.
> > 
> > Should the DRM core perhaps already ensure that the hotspot must reside
> > inside the plane (fb) boundaries and reject the atomic (TEST_ONLY)
> > commit when it's outside?
> > 
> > Or if this restriction is not universal, maybe this driver should
> > reject invalid hotspots rather than silently mangle them?  
> 
> TBH, I'm not really sure why vboxvideo is clamping those values. I didn't want to
> introduce any regressions by changing it here, but the hotspot code never specified
> that the hotspot offsets have to be positive or even within the plane. In a quick
> search I couldn't find real world cursors that were doing anything like that though
> so I just left it.
> 
> > Also, if supports_virtual_cursor_plane is false, should there not be
> > somewhere code that will ignore the values set to the atomic hotspot
> > properties?  
> 
> Hmm, good question. I'm not sure if there's a case where that could be possible:
> without supports_virtual_cursor we either won't have a cursor plane or the client
> won't be atomic and will only be allowed to use the old legacy kms ioctl's, i.e.
> drmModeSetCursor2.
> 
> > When one KMS client switches to another, the first one being hotspot
> > aware and the latter not, and both atomic, then the latter KMS client
> > who doesn't know to drive the hotspot will inherit potentially invalid
> > hotspot from the previous KMS client. Does something prevent that from
> > being a problem?  
> 
> That switch will result in plane state reset, ending in
> __drm_atomic_helper_plane_state_reset which will reset both hotspot properties (set
> them to 0).

It will?

To my knowledge, KMS has never reset anything when one KMS client
switches to the next, so that's new.

What triggers it?

Only if you actually switch to fbdev/fbcon instead of another userspace
KMS client, the fbdev code might reset some things, but not all.

> > The old KMS client may have left the virtual cursor plane visible, and
> > the new KMS client doesn't even know the virtual cursor plane exists
> > because it doesn't set the DRM client cap. Something should probably
> > ensure the virtual cursor plane gets disabled, right?  
> 
> Hah, that's also a good question. I *think* the same code to above would be ran,
> i.e. plane reset which should result in the plane disappearing and the new client
> not being able to drive it anymore. At least when running gnome-shell, switching to
> weston and then switching to gnome-shell things look ok, but I haven't tried running
> such clients at the same time.

That's an interesting experiment, but how is "at the same time"
different from what you tested?

As i mentioned above, if you switch to fbcon in between, then you are
not switching from one userspace KMS client to the next.


Thanks,
pq
Zack Rusin July 13, 2022, 1:35 p.m. UTC | #4
On Wed, 2022-07-13 at 10:20 +0300, Pekka Paalanen wrote:
> On Wed, 13 Jul 2022 03:35:57 +0000
> Zack Rusin <zackr@vmware.com> wrote:
> 
> > On Tue, 2022-07-12 at 10:56 +0300, Pekka Paalanen wrote:
> > > On Mon, 11 Jul 2022 23:32:43 -0400
> > > Zack Rusin <zack@kde.org> wrote:
> > >   
> > > > From: Zack Rusin <zackr@vmware.com>
> > > > 
> > > > Atomic modesetting got support for mouse hotspots via the hotspot
> > > > properties. Port the legacy kms hotspot handling to the new properties
> > > > on cursor planes.
> > > > 
> > > > Signed-off-by: Zack Rusin <zackr@vmware.com>
> > > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > > Cc: David Airlie <airlied@linux.ie>
> > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > ---
> > > >  drivers/gpu/drm/vboxvideo/vbox_mode.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > > index fa0d73ce07bc..ca3134adb104 100644
> > > > --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > > +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > > @@ -429,8 +429,8 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
> > > >  	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
> > > >  		VBOX_MOUSE_POINTER_ALPHA;
> > > >  	hgsmi_update_pointer_shape(vbox->guest_pool, flags,
> > > > -				   min_t(u32, max(fb->hot_x, 0), width),
> > > > -				   min_t(u32, max(fb->hot_y, 0), height),
> > > > +				   min_t(u32, max(new_state->hotspot_x, 0), width),
> > > > +				   min_t(u32, max(new_state->hotspot_y, 0), height),
> > > >  				   width, height, vbox->cursor_data, data_size);
> > > >  
> > > >  	mutex_unlock(&vbox->hw_mutex);  
> > > 
> > > Hi,
> > > 
> > > this looks like silent clamping of the hotspot coordinates.
> > > 
> > > Should the DRM core perhaps already ensure that the hotspot must reside
> > > inside the plane (fb) boundaries and reject the atomic (TEST_ONLY)
> > > commit when it's outside?
> > > 
> > > Or if this restriction is not universal, maybe this driver should
> > > reject invalid hotspots rather than silently mangle them?  
> > 
> > TBH, I'm not really sure why vboxvideo is clamping those values. I didn't want to
> > introduce any regressions by changing it here, but the hotspot code never specified
> > that the hotspot offsets have to be positive or even within the plane. In a quick
> > search I couldn't find real world cursors that were doing anything like that though
> > so I just left it.
> > 
> > > Also, if supports_virtual_cursor_plane is false, should there not be
> > > somewhere code that will ignore the values set to the atomic hotspot
> > > properties?  
> > 
> > Hmm, good question. I'm not sure if there's a case where that could be possible:
> > without supports_virtual_cursor we either won't have a cursor plane or the client
> > won't be atomic and will only be allowed to use the old legacy kms ioctl's, i.e.
> > drmModeSetCursor2.
> > 
> > > When one KMS client switches to another, the first one being hotspot
> > > aware and the latter not, and both atomic, then the latter KMS client
> > > who doesn't know to drive the hotspot will inherit potentially invalid
> > > hotspot from the previous KMS client. Does something prevent that from
> > > being a problem?  
> > 
> > That switch will result in plane state reset, ending in
> > __drm_atomic_helper_plane_state_reset which will reset both hotspot properties (set
> > them to 0).
> 
> It will?
> 
> To my knowledge, KMS has never reset anything when one KMS client
> switches to the next, so that's new.
> 
> What triggers it?
> 
> Only if you actually switch to fbdev/fbcon instead of another userspace
> KMS client, the fbdev code might reset some things, but not all.
> 
> > > The old KMS client may have left the virtual cursor plane visible, and
> > > the new KMS client doesn't even know the virtual cursor plane exists
> > > because it doesn't set the DRM client cap. Something should probably
> > > ensure the virtual cursor plane gets disabled, right?  
> > 
> > Hah, that's also a good question. I *think* the same code to above would be ran,
> > i.e. plane reset which should result in the plane disappearing and the new client
> > not being able to drive it anymore. At least when running gnome-shell, switching to
> > weston and then switching to gnome-shell things look ok, but I haven't tried running
> > such clients at the same time.
> 
> That's an interesting experiment, but how is "at the same time"
> different from what you tested?
> 
> As i mentioned above, if you switch to fbcon in between, then you are
> not switching from one userspace KMS client to the next.

FWIW, running weston inside gnome-shell as a window also works fine. And running 
weston-simple-damage --width=60 --height=60 which, currently would make that client
pop up in the cursor plane, while running weston in a window inside gnome-shell also
works. I'm not sure what are the paths clients are taking in those cases so I don't
want to speculate but I'd be happy to try any other experiments or cases you think
might break.

z
Pekka Paalanen July 14, 2022, 7:38 a.m. UTC | #5
On Wed, 13 Jul 2022 13:35:56 +0000
Zack Rusin <zackr@vmware.com> wrote:

> On Wed, 2022-07-13 at 10:20 +0300, Pekka Paalanen wrote:
> > On Wed, 13 Jul 2022 03:35:57 +0000
> > Zack Rusin <zackr@vmware.com> wrote:
> >   
> > > On Tue, 2022-07-12 at 10:56 +0300, Pekka Paalanen wrote:  
> > > > On Mon, 11 Jul 2022 23:32:43 -0400
> > > > Zack Rusin <zack@kde.org> wrote:
> > > >     
> > > > > From: Zack Rusin <zackr@vmware.com>
> > > > > 
> > > > > Atomic modesetting got support for mouse hotspots via the hotspot
> > > > > properties. Port the legacy kms hotspot handling to the new properties
> > > > > on cursor planes.
> > > > > 
> > > > > Signed-off-by: Zack Rusin <zackr@vmware.com>
> > > > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > > > Cc: David Airlie <airlied@linux.ie>
> > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > ---
> > > > >  drivers/gpu/drm/vboxvideo/vbox_mode.c | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > > > index fa0d73ce07bc..ca3134adb104 100644
> > > > > --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > > > +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > > > > @@ -429,8 +429,8 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
> > > > >  	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
> > > > >  		VBOX_MOUSE_POINTER_ALPHA;
> > > > >  	hgsmi_update_pointer_shape(vbox->guest_pool, flags,
> > > > > -				   min_t(u32, max(fb->hot_x, 0), width),
> > > > > -				   min_t(u32, max(fb->hot_y, 0), height),
> > > > > +				   min_t(u32, max(new_state->hotspot_x, 0), width),
> > > > > +				   min_t(u32, max(new_state->hotspot_y, 0), height),
> > > > >  				   width, height, vbox->cursor_data, data_size);
> > > > >  
> > > > >  	mutex_unlock(&vbox->hw_mutex);    
> > > > 
> > > > Hi,
> > > > 
> > > > this looks like silent clamping of the hotspot coordinates.
> > > > 
> > > > Should the DRM core perhaps already ensure that the hotspot must reside
> > > > inside the plane (fb) boundaries and reject the atomic (TEST_ONLY)
> > > > commit when it's outside?
> > > > 
> > > > Or if this restriction is not universal, maybe this driver should
> > > > reject invalid hotspots rather than silently mangle them?    
> > > 
> > > TBH, I'm not really sure why vboxvideo is clamping those values. I didn't want to
> > > introduce any regressions by changing it here, but the hotspot code never specified
> > > that the hotspot offsets have to be positive or even within the plane. In a quick
> > > search I couldn't find real world cursors that were doing anything like that though
> > > so I just left it.
> > >   
> > > > Also, if supports_virtual_cursor_plane is false, should there not be
> > > > somewhere code that will ignore the values set to the atomic hotspot
> > > > properties?    
> > > 
> > > Hmm, good question. I'm not sure if there's a case where that could be possible:
> > > without supports_virtual_cursor we either won't have a cursor plane or the client
> > > won't be atomic and will only be allowed to use the old legacy kms ioctl's, i.e.
> > > drmModeSetCursor2.
> > >   
> > > > When one KMS client switches to another, the first one being hotspot
> > > > aware and the latter not, and both atomic, then the latter KMS client
> > > > who doesn't know to drive the hotspot will inherit potentially invalid
> > > > hotspot from the previous KMS client. Does something prevent that from
> > > > being a problem?    
> > > 
> > > That switch will result in plane state reset, ending in
> > > __drm_atomic_helper_plane_state_reset which will reset both hotspot properties (set
> > > them to 0).  
> > 
> > It will?
> > 
> > To my knowledge, KMS has never reset anything when one KMS client
> > switches to the next, so that's new.
> > 
> > What triggers it?
> > 
> > Only if you actually switch to fbdev/fbcon instead of another userspace
> > KMS client, the fbdev code might reset some things, but not all.
> >   
> > > > The old KMS client may have left the virtual cursor plane visible, and
> > > > the new KMS client doesn't even know the virtual cursor plane exists
> > > > because it doesn't set the DRM client cap. Something should probably
> > > > ensure the virtual cursor plane gets disabled, right?    
> > > 
> > > Hah, that's also a good question. I *think* the same code to above would be ran,
> > > i.e. plane reset which should result in the plane disappearing and the new client
> > > not being able to drive it anymore. At least when running gnome-shell, switching to
> > > weston and then switching to gnome-shell things look ok, but I haven't tried running
> > > such clients at the same time.  
> > 
> > That's an interesting experiment, but how is "at the same time"
> > different from what you tested?
> > 
> > As i mentioned above, if you switch to fbcon in between, then you are
> > not switching from one userspace KMS client to the next.  
> 
> FWIW, running weston inside gnome-shell as a window also works fine.

Of course it does. :-)

It won't be using DRM or KMS at all in that case. Weston acts as a
Wayland or X11 client in that case (while also being a Wayland
compositor towards its own clients).

> And running 
> weston-simple-damage --width=60 --height=60 which, currently would make that client
> pop up in the cursor plane, while running weston in a window inside gnome-shell also
> works. I'm not sure what are the paths clients are taking in those cases so I don't
> want to speculate but I'd be happy to try any other experiments or cases you think
> might break.

Right, I do believe that case is solved. The switching case is
different though.

The test for the switching case would be something like this:

- patch Mutter to set CAP_VIRTUAL_CURSOR, drop the blacklisting, set
  hotspot properties correctly, and make sure it uses the virtual
  cursor plane when possible

- launch GNOME on one VT

- launch Weston on another VT, make sure it uses drm-backend.so and not
  e.g. wayland-backend.so (the logs tell you, defaulting to stdout or
  stderr, I never remember which)

- switch to GNOME VT, check the cursor plane is used, switch to Weston VT

- observe if you have a dead cursor sprite on screen left over from GNOME

- Make sure Mutter or GDM did not "sanitize" KMS state before releasing
  DRM master for Weston

The last part is important, because the kernel must not rely on
userspace to sanitize KMS state on its way out. E.g. forced session
switching will not give the chance even, and I suppose most KMS clients
also don't bother. Sanitizing KMS on switch-out is also generally
avoided to avoid unnecessary modesets which take time and cause more
flicker, even if you're not aiming for seamless handover.

Since Weston never sees the virtual cursor plane even exists, there is
no way Weston could reset it. That's why it is important for the kernel
to reset the virtual cursor plane in this case. It is the fact that the
kernel does not expose the cursor plane at all to Weston that makes
this situation unprecedented, needing something completely new in the
kernel.

Maybe it's easier to patch some other compositor like Sway than Mutter
to use the new virtual cursor stuff, and then test between the two
compositor instances, one with the CAP and one without, and see how
VT-switching between them works.



Thanks,
pq
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index fa0d73ce07bc..ca3134adb104 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -429,8 +429,8 @@  static void vbox_cursor_atomic_update(struct drm_plane *plane,
 	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
 		VBOX_MOUSE_POINTER_ALPHA;
 	hgsmi_update_pointer_shape(vbox->guest_pool, flags,
-				   min_t(u32, max(fb->hot_x, 0), width),
-				   min_t(u32, max(fb->hot_y, 0), height),
+				   min_t(u32, max(new_state->hotspot_x, 0), width),
+				   min_t(u32, max(new_state->hotspot_y, 0), height),
 				   width, height, vbox->cursor_data, data_size);
 
 	mutex_unlock(&vbox->hw_mutex);