subject:"\[PATCH 0\/6\] drm\: Add mouse cursor hotspot support to atomic KMS"

On Fri, Jun 10, 2022 at 09:15:35AM +, Simon Ser wrote:
> On Friday, June 10th, 2022 at 10:41, Daniel Vetter  wrote:
> 
> > Anything I've missed? Or got completely wrong?
> 
> This plan looks good to me.
> 
> As Pekka mentionned, I'd also like to have a conversation of how far we want 
> to
> push virtualized driver features. I think KMS support is a good feature to 
> have
> to spin up a VM and have all of the basics working. However I don't think it's
> a good idea to try to plumb an ever-growing list of fancy features
> (seamless integration of guest windows into the host, HiDPI, multi-monitor,
> etc) into KMS. You'd just end up re-inventing Wayland or RDP on top of KMS.
> Instead of re-inventing these, just use RDP or waypipe or X11 forwarding
> directly.
> 
> So I think we need to draw a line somewhere, and decide e.g. that virtualized
> cursors are fine to add in KMS, but HiDPI is not.

It's getting a bit far off-topic, but google cros team has an out-of-tree
(at least I think it's not merged yet) wayland-virtio driver for exactly
this use-case. Trying to move towards something like that for fancy
virtualized setups sounds like the better approach indeed, with kms just
as the bare-bones fallback option.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-10 Thread Simon Ser

On Friday, June 10th, 2022 at 10:41, Daniel Vetter  wrote:

> Anything I've missed? Or got completely wrong?

This plan looks good to me.

As Pekka mentionned, I'd also like to have a conversation of how far we want to
push virtualized driver features. I think KMS support is a good feature to have
to spin up a VM and have all of the basics working. However I don't think it's
a good idea to try to plumb an ever-growing list of fancy features
(seamless integration of guest windows into the host, HiDPI, multi-monitor,
etc) into KMS. You'd just end up re-inventing Wayland or RDP on top of KMS.
Instead of re-inventing these, just use RDP or waypipe or X11 forwarding
directly.

So I think we need to draw a line somewhere, and decide e.g. that virtualized
cursors are fine to add in KMS, but HiDPI is not.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

On Fri, Jun 10, 2022 at 08:54:03AM +, Simon Ser wrote:
> I agree with what others have replied, just adding a few more details.
> 
> On Thursday, June 9th, 2022 at 21:39, Zack Rusin  wrote:
> 
> > virtualized drivers send drm_kms_helper_hotplug_event which sends a 
> > HOTPLUG=1
> > event with a changed preferred width/height
> 
> (Note: and the "hotplug_mode_update" property is set to 1.)
> 
> > suggested_x and suggested_y properties
> 
> These come with their own set of issues. They are poorly defined, but it seems
> like they describe a position in physical pixel coordinates. Compositors don't
> use physical pixel coordinates to organize their outputs, instead they use
> logical coordinates. For instance, a HiDPI 4k screen with a scale of 2 will
> take up 1920x1080 logical pixels. There is no way to convert physical pixel
> coordinates to logical pixel coordinates in the general case, because there's
> no "global scale factor". So suggested_x/y are incompatible with the way
> compositors work.

I dropped a request for a proper doc section that explains all the
virtualized kms driver stuff. I think we should also put in a
"limitations" part there and just spec that any kind of scaling is a no-go
on these (and that drivers better validate this is the case).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

On Fri, Jun 10, 2022 at 10:41:05AM +0200, Daniel Vetter wrote:
> Hi all,
> 
> Kinda top post because the thread is sprawling and I think we need a
> summary/restart. I think there's at least 3 issues here:
> 
> - lack of hotspot property support, which means compositors can't really
>   support hotspot with atomic. Which isn't entirely true, because you
>   totally can use atomic for the primary planes/crtcs and the legacy
>   cursor ioctls, but I understand that people might find that a bit silly :-)
> 
>   Anyway this problme is solved by the patch set here, and I think results
>   in some nice cleanups to boot.
> 
> - the fact that cursors for virtual drivers are not planes, but really
>   special things. Which just breaks the universal plane kms uapi. That
>   part isn't solved, and I do agree with Simon and Pekka that we really
>   should solve this before we unleash even more compositors onto the
>   atomic paths of virtual drivers.
> 
>   I think the simplest solution for this is:
>   1. add a new DRM_PLANE_TYPE_VIRTUAL_CURSOR, and set that for these
>   special cursor planes on all virtual drivers
>   2. add the new "I understand virtual cursors planes" setparam, filter
>   virtual cursor planes for userspace which doesn't set this (like we do
>   right now if userspace doesn't set the universal plane mode)
>   3. backport the above patches to all stable kernels
>   4. make sure the hotspot property is only set on VIRTUAL_CURSOR planes
>   and nothing else in the rebased patch series

Simon also mentioned on irc that these special planes must not expose the
CRTC_X/Y property, since that doesn't really do much at all. Or is our
understanding of how this all works for commandeered cursors wrong?

> - third issue: These special virtual display properties arent documented.
>   Aside from hotspot there's also suggested X/Y and maybe other stuff. I
>   have no idea what suggested X/Y does and what userspace should do with
>   it. I think we need a new section for virtualized drivers which:
>   - documents all the properties involved
>   - documents the new cap for enabling virtual cursor planes
>   - documents some of the key flows that compositors should implement for
> best experience
>   - documents how exactly the user experience will degrade if compositors
> pretend it's just a normal kms driver (maybe put that into each of the
> special flows that a compositor ideally supports)
>   - whatever other comments and gaps I've missed, I'm sure
> Simon/Pekka/others will chime in once the patch exists.

Great bonus would be an igt which demonstrates these flows. With the
interactive debug breakpoints to wait for resizing or whatever this should
be all neatly possible.
-Daniel

> 
> There's a bit of fixing oopsies (virtualized drivers really shouldn't have
> enabled universal planes for their cursors) and debt (all these properties
> predate the push to document stuff so we need to fix that), but I don't
> think it's too much. And I think, from reading the threads, that this
> should cover everything?
> 
> Anything I've missed? Or got completely wrong?
> 
> Cheers, Daniel
> 
> On Fri, Jun 03, 2022 at 02:14:59PM +, Simon Ser wrote:
> > Hi,
> > 
> > Please, read this thread:
> > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> > 
> > It has a lot of information about the pitfalls of cursor hotspot and
> > other things done by VM software.
> > 
> > In particular: since the driver will ignore the KMS cursor plane
> > position set by user-space, I don't think it's okay to just expose
> > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> > 
> > cc wayland-devel and Pekka for user-space feedback.
> > 
> > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  wrote:
> > 
> > > - all userspace code needs to hardcore a list of drivers which require
> > > hotspots because there's no way to query from drm "does this driver
> > > require hotspot"
> > 
> > Can you elaborate? I'm not sure I understand what you mean here.
> > 
> > Thanks,
> > 
> > Simon
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-10 Thread Pekka Paalanen

On Fri, 10 Jun 2022 10:41:05 +0200
Daniel Vetter  wrote:

> Hi all,
> 
> Kinda top post because the thread is sprawling and I think we need a
> summary/restart. I think there's at least 3 issues here:
> 
> - lack of hotspot property support, which means compositors can't really
>   support hotspot with atomic. Which isn't entirely true, because you
>   totally can use atomic for the primary planes/crtcs and the legacy
>   cursor ioctls, but I understand that people might find that a bit silly :-)
> 
>   Anyway this problme is solved by the patch set here, and I think results
>   in some nice cleanups to boot.
> 
> - the fact that cursors for virtual drivers are not planes, but really
>   special things. Which just breaks the universal plane kms uapi. That
>   part isn't solved, and I do agree with Simon and Pekka that we really
>   should solve this before we unleash even more compositors onto the
>   atomic paths of virtual drivers.
> 
>   I think the simplest solution for this is:
>   1. add a new DRM_PLANE_TYPE_VIRTUAL_CURSOR, and set that for these
>   special cursor planes on all virtual drivers
>   2. add the new "I understand virtual cursors planes" setparam, filter
>   virtual cursor planes for userspace which doesn't set this (like we do
>   right now if userspace doesn't set the universal plane mode)
>   3. backport the above patches to all stable kernels
>   4. make sure the hotspot property is only set on VIRTUAL_CURSOR planes
>   and nothing else in the rebased patch series
> 
> - third issue: These special virtual display properties arent documented.
>   Aside from hotspot there's also suggested X/Y and maybe other stuff. I
>   have no idea what suggested X/Y does and what userspace should do with
>   it. I think we need a new section for virtualized drivers which:
>   - documents all the properties involved
>   - documents the new cap for enabling virtual cursor planes
>   - documents some of the key flows that compositors should implement for
> best experience
>   - documents how exactly the user experience will degrade if compositors
> pretend it's just a normal kms driver (maybe put that into each of the
> special flows that a compositor ideally supports)
>   - whatever other comments and gaps I've missed, I'm sure
> Simon/Pekka/others will chime in once the patch exists.
> 
> There's a bit of fixing oopsies (virtualized drivers really shouldn't have
> enabled universal planes for their cursors) and debt (all these properties
> predate the push to document stuff so we need to fix that), but I don't
> think it's too much. And I think, from reading the threads, that this
> should cover everything?
> 
> Anything I've missed? Or got completely wrong?

Hi,

sounds like a good plan to me.


Thanks,
pq


pgpOdwESYywAY.pgp
Description: OpenPGP digital signature

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-10 Thread Simon Ser

I agree with what others have replied, just adding a few more details.

On Thursday, June 9th, 2022 at 21:39, Zack Rusin  wrote:

> virtualized drivers send drm_kms_helper_hotplug_event which sends a HOTPLUG=1
> event with a changed preferred width/height

(Note: and the "hotplug_mode_update" property is set to 1.)

> suggested_x and suggested_y properties

These come with their own set of issues. They are poorly defined, but it seems
like they describe a position in physical pixel coordinates. Compositors don't
use physical pixel coordinates to organize their outputs, instead they use
logical coordinates. For instance, a HiDPI 4k screen with a scale of 2 will
take up 1920x1080 logical pixels. There is no way to convert physical pixel
coordinates to logical pixel coordinates in the general case, because there's
no "global scale factor". So suggested_x/y are incompatible with the way
compositors work.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

Hi all,

Kinda top post because the thread is sprawling and I think we need a
summary/restart. I think there's at least 3 issues here:

- lack of hotspot property support, which means compositors can't really
  support hotspot with atomic. Which isn't entirely true, because you
  totally can use atomic for the primary planes/crtcs and the legacy
  cursor ioctls, but I understand that people might find that a bit silly :-)

  Anyway this problme is solved by the patch set here, and I think results
  in some nice cleanups to boot.

- the fact that cursors for virtual drivers are not planes, but really
  special things. Which just breaks the universal plane kms uapi. That
  part isn't solved, and I do agree with Simon and Pekka that we really
  should solve this before we unleash even more compositors onto the
  atomic paths of virtual drivers.

  I think the simplest solution for this is:
  1. add a new DRM_PLANE_TYPE_VIRTUAL_CURSOR, and set that for these
  special cursor planes on all virtual drivers
  2. add the new "I understand virtual cursors planes" setparam, filter
  virtual cursor planes for userspace which doesn't set this (like we do
  right now if userspace doesn't set the universal plane mode)
  3. backport the above patches to all stable kernels
  4. make sure the hotspot property is only set on VIRTUAL_CURSOR planes
  and nothing else in the rebased patch series

- third issue: These special virtual display properties arent documented.
  Aside from hotspot there's also suggested X/Y and maybe other stuff. I
  have no idea what suggested X/Y does and what userspace should do with
  it. I think we need a new section for virtualized drivers which:
  - documents all the properties involved
  - documents the new cap for enabling virtual cursor planes
  - documents some of the key flows that compositors should implement for
best experience
  - documents how exactly the user experience will degrade if compositors
pretend it's just a normal kms driver (maybe put that into each of the
special flows that a compositor ideally supports)
  - whatever other comments and gaps I've missed, I'm sure
Simon/Pekka/others will chime in once the patch exists.

There's a bit of fixing oopsies (virtualized drivers really shouldn't have
enabled universal planes for their cursors) and debt (all these properties
predate the push to document stuff so we need to fix that), but I don't
think it's too much. And I think, from reading the threads, that this
should cover everything?

Anything I've missed? Or got completely wrong?

Cheers, Daniel

On Fri, Jun 03, 2022 at 02:14:59PM +, Simon Ser wrote:
> Hi,
> 
> Please, read this thread:
> https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> 
> It has a lot of information about the pitfalls of cursor hotspot and
> other things done by VM software.
> 
> In particular: since the driver will ignore the KMS cursor plane
> position set by user-space, I don't think it's okay to just expose
> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> 
> cc wayland-devel and Pekka for user-space feedback.
> 
> On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  wrote:
> 
> > - all userspace code needs to hardcore a list of drivers which require
> > hotspots because there's no way to query from drm "does this driver
> > require hotspot"
> 
> Can you elaborate? I'm not sure I understand what you mean here.
> 
> Thanks,
> 
> Simon

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-10 Thread Jonas Ådahl

On Fri, Jun 10, 2022 at 10:49:31AM +0300, Pekka Paalanen wrote:
> On Thu, 9 Jun 2022 19:39:39 +
> Zack Rusin  wrote:
> 
> > On Wed, 2022-06-08 at 10:53 +0300, Pekka Paalanen wrote:
> > > On Tue, 7 Jun 2022 17:50:24 +
> > > Zack Rusin  wrote:
> > >   
> > > > On Tue, 2022-06-07 at 11:07 +0300, Pekka Paalanen wrote:  
> > > > > On Fri, 03 Jun 2022 14:14:59 +
> > > > > Simon Ser  wrote:
> > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > Please, read this thread:
> > > > > > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> > > > > > 
> > > > > > It has a lot of information about the pitfalls of cursor hotspot and
> > > > > > other things done by VM software.
> > > > > > 
> > > > > > In particular: since the driver will ignore the KMS cursor plane
> > > > > > position set by user-space, I don't think it's okay to just expose
> > > > > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> > > > > > 
> > > > > > cc wayland-devel and Pekka for user-space feedback.
> > > > > > 
> > > > > > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  
> > > > > > wrote:
> > > > > > 
> > > > > > > - all userspace code needs to hardcore a list of drivers which 
> > > > > > > require
> > > > > > > hotspots because there's no way to query from drm "does this 
> > > > > > > driver
> > > > > > > require hotspot"  
> > > > > > 
> > > > > > Can you elaborate? I'm not sure I understand what you mean here.
> > > > > > 
> > > > > 
> > > > > Hi Zack and everyone,
> > > > > 
> > > > > I would like to try to reboot this discussion and explain where I come
> > > > > from. Maybe I have misunderstood something.
> > > > 
> > > >  First of all thanks for restarting the discussions. I think Gerd 
> > > > did a good
> > > > job responding to individual points, so let me take a step back and 
> > > > explain the big
> > > > picture here so we can reboot.
> > > >   
> > > > > Which root problems do you want to solve exactly?
> > > > 
> > > > The problem that this patch set is solving is the relatively trivial 
> > > > problem of not
> > > > having a way of setting the hotspot in the atomic kms interface. When we
> > > > (virtualized drivers) are using the native cursor we need to know not 
> > > > only the image  
> > > 
> > > Could you clarify what is "native cursor" here?
> > > I guess it is the host window system pointer's cursor?  
> > 
> > Right, exactly. I'm a little hesitant to call it "host" because it gets 
> > tricky in
> > remote scenarios, where the host is some ESXi server but the local machine 
> > is the
> > one that's actually interacting with the guest. So it's the cursor of the 
> > machine
> > where the guest screen is displayed.
> > 
> > 
> > > > Now, where the disagreements come from is from the fact that all 
> > > > virtualized drivers
> > > > do not implement the atomic KMS cursor plane contract exactly as 
> > > > specified. In
> > > > atomic kms with universal planes the "cursor" plane can be anything so 
> > > > asking for
> > > > hotspot's for something that's not a cursor is a bit silly (but 
> > > > arguably so is
> > > > calling it a cursor plane and then complaining that people expect 
> > > > cursor in it).
> > > > 
> > > > So the argument is that we can't put hotspot data into atomic kms 
> > > > without first
> > > > rewriting all of the virtualized drivers cursor code to fix the 
> > > > underlying contract
> > > > violation that they all commit. That would likely be a lot easier sell 
> > > > if not that
> > > > gnome/kde don't put none cursor stuff in the cursor plane, so all this 
> > > > work is to
> > > > fix breakages that seem to affect 0 of our users (and I completely 
> > > > understand that
> > > > we'd still like all the drivers to be correct and unified in terms of 
> > > > their
> > > > behaviour, I'm just saying it's a hard sell without something that we 
> > > > can point to
> > > > and say "this fixes/improves things for our customers")   
> > > 
> > > What's the cost of making paravirtualized drivers honour the UAPI 
> > > contract?
> > > Can't you do that on the side of implementing these new hotspot
> > > properties, with the little addition to allowing guest userspace to be
> > > explicit about whether it supports commandeering or not?  
> > 
> > I'm reluctant here because "fixing" here seems to be a bit ephemeral as we 
> > move from
> > one solution to the next. I'm happy to write a patch that's adding a
> > DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE flag and hiding the cursor plane in 
> > virtualized
> > drivers for clients that advertise DRM_CLIENT_CAP_ATOMIC but not
> > DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE, but that doesn't solve Weston on 
> > virtualized
> > drivers.
> 
> Mind, I have not talked about hiding cursor planes. I have talked
> *only* about stopping commandeering cursor planes if guest userspace
> does not indicate it is prepared for commandeering.
> 
> I don't understand how it does not "solve on Weston

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-10 Thread Pekka Paalanen

On Thu, 9 Jun 2022 19:39:39 +
Zack Rusin  wrote:

> On Wed, 2022-06-08 at 10:53 +0300, Pekka Paalanen wrote:
> > On Tue, 7 Jun 2022 17:50:24 +
> > Zack Rusin  wrote:
> >   
> > > On Tue, 2022-06-07 at 11:07 +0300, Pekka Paalanen wrote:  
> > > > On Fri, 03 Jun 2022 14:14:59 +
> > > > Simon Ser  wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > Please, read this thread:
> > > > > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> > > > > 
> > > > > It has a lot of information about the pitfalls of cursor hotspot and
> > > > > other things done by VM software.
> > > > > 
> > > > > In particular: since the driver will ignore the KMS cursor plane
> > > > > position set by user-space, I don't think it's okay to just expose
> > > > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> > > > > 
> > > > > cc wayland-devel and Pekka for user-space feedback.
> > > > > 
> > > > > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  wrote:
> > > > > 
> > > > > > - all userspace code needs to hardcore a list of drivers which 
> > > > > > require
> > > > > > hotspots because there's no way to query from drm "does this driver
> > > > > > require hotspot"  
> > > > > 
> > > > > Can you elaborate? I'm not sure I understand what you mean here.
> > > > > 
> > > > 
> > > > Hi Zack and everyone,
> > > > 
> > > > I would like to try to reboot this discussion and explain where I come
> > > > from. Maybe I have misunderstood something.
> > > 
> > >  First of all thanks for restarting the discussions. I think Gerd 
> > > did a good
> > > job responding to individual points, so let me take a step back and 
> > > explain the big
> > > picture here so we can reboot.
> > >   
> > > > Which root problems do you want to solve exactly?
> > > 
> > > The problem that this patch set is solving is the relatively trivial 
> > > problem of not
> > > having a way of setting the hotspot in the atomic kms interface. When we
> > > (virtualized drivers) are using the native cursor we need to know not 
> > > only the image  
> > 
> > Could you clarify what is "native cursor" here?
> > I guess it is the host window system pointer's cursor?  
> 
> Right, exactly. I'm a little hesitant to call it "host" because it gets 
> tricky in
> remote scenarios, where the host is some ESXi server but the local machine is 
> the
> one that's actually interacting with the guest. So it's the cursor of the 
> machine
> where the guest screen is displayed.
> 
> 
> > > Now, where the disagreements come from is from the fact that all 
> > > virtualized drivers
> > > do not implement the atomic KMS cursor plane contract exactly as 
> > > specified. In
> > > atomic kms with universal planes the "cursor" plane can be anything so 
> > > asking for
> > > hotspot's for something that's not a cursor is a bit silly (but arguably 
> > > so is
> > > calling it a cursor plane and then complaining that people expect cursor 
> > > in it).
> > > 
> > > So the argument is that we can't put hotspot data into atomic kms without 
> > > first
> > > rewriting all of the virtualized drivers cursor code to fix the 
> > > underlying contract
> > > violation that they all commit. That would likely be a lot easier sell if 
> > > not that
> > > gnome/kde don't put none cursor stuff in the cursor plane, so all this 
> > > work is to
> > > fix breakages that seem to affect 0 of our users (and I completely 
> > > understand that
> > > we'd still like all the drivers to be correct and unified in terms of 
> > > their
> > > behaviour, I'm just saying it's a hard sell without something that we can 
> > > point to
> > > and say "this fixes/improves things for our customers")   
> > 
> > What's the cost of making paravirtualized drivers honour the UAPI contract?
> > Can't you do that on the side of implementing these new hotspot
> > properties, with the little addition to allowing guest userspace to be
> > explicit about whether it supports commandeering or not?  
> 
> I'm reluctant here because "fixing" here seems to be a bit ephemeral as we 
> move from
> one solution to the next. I'm happy to write a patch that's adding a
> DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE flag and hiding the cursor plane in 
> virtualized
> drivers for clients that advertise DRM_CLIENT_CAP_ATOMIC but not
> DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE, but that doesn't solve Weston on 
> virtualized
> drivers.

Mind, I have not talked about hiding cursor planes. I have talked
*only* about stopping commandeering cursor planes if guest userspace
does not indicate it is prepared for commandeering.

I don't understand how it does not "solve on Weston on virtualized
drivers". Can you explain what is not solved?

As far as I can see, it does solve Weston: it makes cursor plane
behaviour correct from KMS UAPI contract point of view. Anything that
is not quite optimal after that with cursor planes you can blame on
Weston not making use of additional optional features.

Curs

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-09 Thread Zack Rusin

On Wed, 2022-06-08 at 10:53 +0300, Pekka Paalanen wrote:
> On Tue, 7 Jun 2022 17:50:24 +
> Zack Rusin  wrote:
> 
> > On Tue, 2022-06-07 at 11:07 +0300, Pekka Paalanen wrote:
> > > On Fri, 03 Jun 2022 14:14:59 +
> > > Simon Ser  wrote:
> > >   
> > > > Hi,
> > > > 
> > > > Please, read this thread:
> > > > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> > > > 
> > > > It has a lot of information about the pitfalls of cursor hotspot and
> > > > other things done by VM software.
> > > > 
> > > > In particular: since the driver will ignore the KMS cursor plane
> > > > position set by user-space, I don't think it's okay to just expose
> > > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> > > > 
> > > > cc wayland-devel and Pekka for user-space feedback.
> > > > 
> > > > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  wrote:
> > > >   
> > > > > - all userspace code needs to hardcore a list of drivers which require
> > > > > hotspots because there's no way to query from drm "does this driver
> > > > > require hotspot"
> > > > 
> > > > Can you elaborate? I'm not sure I understand what you mean here.
> > > >   
> > > 
> > > Hi Zack and everyone,
> > > 
> > > I would like to try to reboot this discussion and explain where I come
> > > from. Maybe I have misunderstood something.  
> > 
> >  First of all thanks for restarting the discussions. I think Gerd did 
> > a good
> > job responding to individual points, so let me take a step back and explain 
> > the big
> > picture here so we can reboot.
> > 
> > > Which root problems do you want to solve exactly?  
> > 
> > The problem that this patch set is solving is the relatively trivial 
> > problem of not
> > having a way of setting the hotspot in the atomic kms interface. When we
> > (virtualized drivers) are using the native cursor we need to know not only 
> > the image
> 
> Could you clarify what is "native cursor" here?
> I guess it is the host window system pointer's cursor?

Right, exactly. I'm a little hesitant to call it "host" because it gets tricky 
in
remote scenarios, where the host is some ESXi server but the local machine is 
the
one that's actually interacting with the guest. So it's the cursor of the 
machine
where the guest screen is displayed.

> > Now, where the disagreements come from is from the fact that all 
> > virtualized drivers
> > do not implement the atomic KMS cursor plane contract exactly as specified. 
> > In
> > atomic kms with universal planes the "cursor" plane can be anything so 
> > asking for
> > hotspot's for something that's not a cursor is a bit silly (but arguably so 
> > is
> > calling it a cursor plane and then complaining that people expect cursor in 
> > it).
> > 
> > So the argument is that we can't put hotspot data into atomic kms without 
> > first
> > rewriting all of the virtualized drivers cursor code to fix the underlying 
> > contract
> > violation that they all commit. That would likely be a lot easier sell if 
> > not that
> > gnome/kde don't put none cursor stuff in the cursor plane, so all this work 
> > is to
> > fix breakages that seem to affect 0 of our users (and I completely 
> > understand that
> > we'd still like all the drivers to be correct and unified in terms of their
> > behaviour, I'm just saying it's a hard sell without something that we can 
> > point to
> > and say "this fixes/improves things for our customers") 
> 
> What's the cost of making paravirtualized drivers honour the UAPI contract?
> Can't you do that on the side of implementing these new hotspot
> properties, with the little addition to allowing guest userspace to be
> explicit about whether it supports commandeering or not?

I'm reluctant here because "fixing" here seems to be a bit ephemeral as we move 
from
one solution to the next. I'm happy to write a patch that's adding a
DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE flag and hiding the cursor plane in 
virtualized
drivers for clients that advertise DRM_CLIENT_CAP_ATOMIC but not
DRM_CLIENT_CAP_VIRTUAL_CURSOR_AWARE, but that doesn't solve Weston on 
virtualized
drivers.

I feel like that's a larger discussion. One that we need to have in general - 
it's
about standardising on behaviour of userspace with virtualized drivers, e.g. on
Weston even the most basic functionality of virtualized drivers which is 
resizing a
window doesn't work correctly (virtualized drivers send 
drm_kms_helper_hotplug_event
which sends a HOTPLUG=1 event with a changed preferred width/height but Weston
doesn't seem to resize the fb on them which results in Weston not resizing to 
the
new size of the window) or even considering the suggested_x and suggested_y
properties. It seems like we might need to have a wider discussion on 
standardising
those common issues on virtualized drivers because currently, I'm guessing, that
apart from Gnome and KDE most compositors are completely broken on virtualized
drivers.

I'd prefer not blocking fixing hotspots u

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-08 Thread Gerd Hoffmann

  Hi,

> > Typically there is a communication path from guest to host for pointer
> > movements (i.e. crtc_x + crtc_y updates), so the host knows where the
> > guest wants the cursor plane being placed.  So in case the pointer is
> > moved by other means (different input device, some application warping
> > the pointer, ...) the host can adapt.
> 
> Would it not be better to be explicit about it? To avoid fragile
> heuristics.

Explicit about what exactly?  Whenever pointer warps via crtc_x + crtc_y
update work or not?  Not so easy ...

> > Nevertheless behavior is not consistent here because in some cases the
> > feedback loop is not wired up end-to-end.  The spice protocol has a
> > message type for that, so pointer warps work.  The vnc protocol has not,
> > so they don't.

... for example qemu allows to enable both spice and vnc protocols.

> > It actually is a problem for multihead configurations though.  Having
> > some way to map input devices to scanouts would actually be helpful.
> > Years ago I checked how this works for touchscreens to see whenever it
> > is possible to leverage that for VMs somehow.  There wasn't some obvious
> > way, and I forgot the details meanwhile ...
> 
> Ah, that's the other way around, right? To tell guest OS which output
> an absolute input device is relative to?

Yes.

> Having a standard for naming outputs is hard it seems, and there is
> also the connector vs. monitor dilemma. I guess absolute input devices
> would usually want to be associated with the (real or virtual) monitor
> regardless of which (real or virtual) connector it is connected to.

Yes, this should be linked to the monitor not the connector.  qemu
learned to generate edid blobs a while back, so we actually have
virtual monitors and can create virtual monitor properties for them.

take care,
  Gerd

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-08 Thread Pekka Paalanen

On Tue, 7 Jun 2022 16:30:23 +0200
Gerd Hoffmann  wrote:

> > Why are pointer cursors misplaced on paravirtualized drivers?
> > 
> > It is because the paravirtualized drivers or VM viewers do *not* place
> > the cursor plane at the CRTC_X, CRTC_Y position in the guest CRTC area.
> > This is obvious: if CRTC_X, CRTC_Y were honoured, there would be no
> > misplacement.
> > 
> > Instead, the VM stack plays clever tricks with cursor planes. I have
> > understood only one of those tricks, and it goes something like this.
> > To improve hand-eye coordination, that is to reduce the hand-to-eye
> > response time a.k.a latency, the VM guest KMS driver relays the cursor
> > plane separately to the VM viewer application.  
> 
> Yes, the cursor is sent separately.
> 
> > The VM viewer application presents the cursor plane content by pushing
> > them to the host window system as the pointer cursor.  
> 
> Yes (i.e. gdk_window_set_cursor() on the host).
> 
> > This means the host window system will be autonomously moving the
> > cursor plane image around, completely disregarding what the guest KMS
> > client programmed into CRTC_X, CRTC_Y.  
> 
> Yes.
> 
> That is combined with a virtual input device sending absolute
> coordinates (i.e. tablet), so mouse clicks land at the correct place.
> And that is the point where having the hotspot information is essential
> on the host side.

Hi Gerd,

thanks for confirming.

> > Given this UAPI contract, it is very easy for userspace to make the
> > conclusion that a cursor plane is just another plane it can use for
> > whatever it wants. Weston and wlroots indeed leverage this, putting
> > also normal windows and other stuff to the cursor plane when they
> > happen to fit.  
> 
> virtual machine display devices typically offer small (64x64) cursor
> planes, so unlike the 512x512 planes I've seen offered by i915 they are
> hardly usable for anything but cursors.  Likewise additional overlay
> planes typically not offered, so the classic primary+cursor setup is
> pretty much the only reasonable option userspace has.

weston-simple-shm is 256x256, and has been demonstrated to go flying in
e.g. vmware environments:
https://oftc.irclog.whitequark.org/dri-devel/2022-06-06#30987017;

If KMS exposes planes, then userspace will try hard to make use of them
as much as possible. It's not unimaginable that there could also be
some small icon generated by the window system overlaying an
application window. That might fit a tiny cursor plane perfectly.

> > I believe the solution has two parts:
> > 
> > - The guest KMS driver needs to know whether the guest userspace is
> >   prepared for the cursor plane being commandeered. If userspace does
> >   not indicate it is prepared for it, commandeering must not happen.  
> 
> Yes.  That isn't much of a problem in practice though due to the limited
> driver/device offerings outlined above.
> 
> > - Cursor hotspot needs new KMS properties, and a KMS client must set
> >   them if the KMS client indicates it is prepared for cursor plane
> >   commandeering.  
> 
> Yes, and that is what hurts in practice and thus caused the blacklists
> being created.
> 
> > There are further problems with cursor plane commandeering. The 2020
> > email thread Simon linked to discusses the problem of pointer devices:
> > if VM guest userspace takes pointer input from multiple sources, how
> > will the VM stack know which virtual input device, if any, should drive
> > the cursor plane position?  
> 
> Typically there is a communication path from guest to host for pointer
> movements (i.e. crtc_x + crtc_y updates), so the host knows where the
> guest wants the cursor plane being placed.  So in case the pointer is
> moved by other means (different input device, some application warping
> the pointer, ...) the host can adapt.

Would it not be better to be explicit about it? To avoid fragile
heuristics.

> Nevertheless behavior is not consistent here because in some cases the
> feedback loop is not wired up end-to-end.  The spice protocol has a
> message type for that, so pointer warps work.  The vnc protocol has not,
> so they don't.
> 
> > To me the answer to this question seems it could be intimately tied to
> > the first problem: commandeering the cursor plane is allowed only if
> > guest userspace tells the guest KMS driver which input device the
> > cursor plane shall be related to. If no input device is indicated,
> > then commandeering must not happen.  
> 
> Why require an input device?  I just don't see how that would help.
> 
> For allowing the host freely move around the cursor place
> ("commandeering") I do see the point in enforcing that from a design
> point of view, although I doubt it'll buy us much in practice given we
> have broken drivers in the wild so userspace will continue to work with
> blacklists.
> 
> Having some capability to negotiate "commandeering" between kernel and
> userspace certainly makes sense, so we can get of the black lists
> long-term

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-08 Thread Pekka Paalanen

On Tue, 7 Jun 2022 17:50:24 +
Zack Rusin  wrote:

> On Tue, 2022-06-07 at 11:07 +0300, Pekka Paalanen wrote:
> > On Fri, 03 Jun 2022 14:14:59 +
> > Simon Ser  wrote:
> >   
> > > Hi,
> > > 
> > > Please, read this thread:
> > > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> > > 
> > > It has a lot of information about the pitfalls of cursor hotspot and
> > > other things done by VM software.
> > > 
> > > In particular: since the driver will ignore the KMS cursor plane
> > > position set by user-space, I don't think it's okay to just expose
> > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> > > 
> > > cc wayland-devel and Pekka for user-space feedback.
> > > 
> > > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  wrote:
> > >   
> > > > - all userspace code needs to hardcore a list of drivers which require
> > > > hotspots because there's no way to query from drm "does this driver
> > > > require hotspot"
> > > 
> > > Can you elaborate? I'm not sure I understand what you mean here.
> > >   
> > 
> > Hi Zack and everyone,
> > 
> > I would like to try to reboot this discussion and explain where I come
> > from. Maybe I have misunderstood something.  
> 
>  First of all thanks for restarting the discussions. I think Gerd did a 
> good
> job responding to individual points, so let me take a step back and explain 
> the big
> picture here so we can reboot.
> 
> > Which root problems do you want to solve exactly?  
> 
> The problem that this patch set is solving is the relatively trivial problem 
> of not
> having a way of setting the hotspot in the atomic kms interface. When we
> (virtualized drivers) are using the native cursor we need to know not only 
> the image

Could you clarify what is "native cursor" here?
I guess it is the host window system pointer's cursor?

> and position of the cursor, we need to know which point within that cursor 
> activates
> the click (going back to analogy from the previous email: cursor image with 
> arrow
> point top-left and cursor image image with arrow pointing bottom-right will 
> have
> different hotspots, likely  [0, 0] in the first case and [cursor_width,
> cursor_height] in the latter). To correctly route the mouse clicks we need to 
> be
> aware of the hotspot coordinates. Currently even though all virtualized 
> drivers are
> atomic userspace has to explicitly disable atomic kms for those drivers 
> because
> mouse clicks are offset incorrectly as soon as the cursor image changes from 
> the
> top-left pointing arrow, i.e. the hotspot isn't at [0,0]).
> 
> So we would like to be able to enable atomic kms with gnome and kde on 
> virtualized
> drivers and to do that we need a way to pass hotspot coordinates from 
> userspace to
> virtualized driver.
> 
> That seems to be pretty self-explanatory and, I think, we all agree that will 
> go
> through properties like in the attached patch set.

Right.

> Now, where the disagreements come from is from the fact that all virtualized 
> drivers
> do not implement the atomic KMS cursor plane contract exactly as specified. In
> atomic kms with universal planes the "cursor" plane can be anything so asking 
> for
> hotspot's for something that's not a cursor is a bit silly (but arguably so is
> calling it a cursor plane and then complaining that people expect cursor in 
> it).
> 
> So the argument is that we can't put hotspot data into atomic kms without 
> first
> rewriting all of the virtualized drivers cursor code to fix the underlying 
> contract
> violation that they all commit. That would likely be a lot easier sell if not 
> that
> gnome/kde don't put none cursor stuff in the cursor plane, so all this work 
> is to
> fix breakages that seem to affect 0 of our users (and I completely understand 
> that
> we'd still like all the drivers to be correct and unified in terms of their
> behaviour, I'm just saying it's a hard sell without something that we can 
> point to
> and say "this fixes/improves things for our customers") 

What's the cost of making paravirtualized drivers honour the UAPI contract?
Can't you do that on the side of implementing these new hotspot
properties, with the little addition to allowing guest userspace to be
explicit about whether it supports commandeering or not?

The deny-lists exist in guest userspace already, and they are not going
anywhere any time soon that I can see. So the delay in getting those
deny-lists side-stepped is the same delay it would take to have the
guest userspace use the new UAPI to properly say they do support
commandeering.

In your mind, how do you expect guest userspace like Mutter to drop the
deny-lists? What would make GNOME developers be willing to do that? And
why is that somehow easier or faster than a new proper UAPI to be
explicit about commandeering?

You already need patches to guest userspace to fill in the hotspot
properties, so I don't get the resistance to adding another flag to be
explicit about

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-07 Thread Zack Rusin

On Tue, 2022-06-07 at 11:07 +0300, Pekka Paalanen wrote:
> On Fri, 03 Jun 2022 14:14:59 +
> Simon Ser  wrote:
> 
> > Hi,
> > 
> > Please, read this thread:
> > https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> > 
> > It has a lot of information about the pitfalls of cursor hotspot and
> > other things done by VM software.
> > 
> > In particular: since the driver will ignore the KMS cursor plane
> > position set by user-space, I don't think it's okay to just expose
> > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> > 
> > cc wayland-devel and Pekka for user-space feedback.
> > 
> > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  wrote:
> > 
> > > - all userspace code needs to hardcore a list of drivers which require
> > > hotspots because there's no way to query from drm "does this driver
> > > require hotspot"  
> > 
> > Can you elaborate? I'm not sure I understand what you mean here.
> > 
> 
> Hi Zack and everyone,
> 
> I would like to try to reboot this discussion and explain where I come
> from. Maybe I have misunderstood something.

 First of all thanks for restarting the discussions. I think Gerd did a 
good
job responding to individual points, so let me take a step back and explain the 
big
picture here so we can reboot.

> Which root problems do you want to solve exactly?

The problem that this patch set is solving is the relatively trivial problem of 
not
having a way of setting the hotspot in the atomic kms interface. When we
(virtualized drivers) are using the native cursor we need to know not only the 
image
and position of the cursor, we need to know which point within that cursor 
activates
the click (going back to analogy from the previous email: cursor image with 
arrow
point top-left and cursor image image with arrow pointing bottom-right will have
different hotspots, likely  [0, 0] in the first case and [cursor_width,
cursor_height] in the latter). To correctly route the mouse clicks we need to be
aware of the hotspot coordinates. Currently even though all virtualized drivers 
are
atomic userspace has to explicitly disable atomic kms for those drivers because
mouse clicks are offset incorrectly as soon as the cursor image changes from the
top-left pointing arrow, i.e. the hotspot isn't at [0,0]).

So we would like to be able to enable atomic kms with gnome and kde on 
virtualized
drivers and to do that we need a way to pass hotspot coordinates from userspace 
to
virtualized driver.

That seems to be pretty self-explanatory and, I think, we all agree that will go
through properties like in the attached patch set.

Now, where the disagreements come from is from the fact that all virtualized 
drivers
do not implement the atomic KMS cursor plane contract exactly as specified. In
atomic kms with universal planes the "cursor" plane can be anything so asking 
for
hotspot's for something that's not a cursor is a bit silly (but arguably so is
calling it a cursor plane and then complaining that people expect cursor in it).

So the argument is that we can't put hotspot data into atomic kms without first
rewriting all of the virtualized drivers cursor code to fix the underlying 
contract
violation that they all commit. That would likely be a lot easier sell if not 
that
gnome/kde don't put none cursor stuff in the cursor plane, so all this work is 
to
fix breakages that seem to affect 0 of our users (and I completely understand 
that
we'd still like all the drivers to be correct and unified in terms of their
behaviour, I'm just saying it's a hard sell without something that we can point 
to
and say "this fixes/improves things for our customers") 

Finally there's a discussion on how to fix it and whether virtualized drivers 
need
to do funky stuff with the cursor. I'd like to first make sure we're on the same
page and then discuss how to fix it next, so let me finish by saying why not
"software cursor".

The easiest way to understand why we do what we do with the cursor, avoiding any
complex and weird cases lets go back to the latency issue: the round trips to 
the
guest to move the cursor are certainly noticeable when running locally, but 
with my
VMware hat on, "local" is not the case that interest us, e.g. on ESXi every VM 
is
accessed over a network so now we're dealing with remote round trips. When
multiplied by slow connections and scale at which we're operating the "software
cursors" go from "some latency" to "completely unusable". That's what I was 
alluding
to in the earlier email when I said they're broken in different ways because if
asked what would you prefer: cursor that when clicked is always incorrectly 
offset,
or choppy/unusably slow cursor - you'll get different answers, depending on who 
you
ask. Virtualization vendors tend to invest pretty heavily into protocols that
improve on vnc/rdp for remote machine access and we'd like to not lose that.

All in all, what we'd like:
 - is to be able to run at least the dominant desktops wit

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-07 Thread Gerd Hoffmann

> Why are pointer cursors misplaced on paravirtualized drivers?
> 
> It is because the paravirtualized drivers or VM viewers do *not* place
> the cursor plane at the CRTC_X, CRTC_Y position in the guest CRTC area.
> This is obvious: if CRTC_X, CRTC_Y were honoured, there would be no
> misplacement.
> 
> Instead, the VM stack plays clever tricks with cursor planes. I have
> understood only one of those tricks, and it goes something like this.
> To improve hand-eye coordination, that is to reduce the hand-to-eye
> response time a.k.a latency, the VM guest KMS driver relays the cursor
> plane separately to the VM viewer application.

Yes, the cursor is sent separately.

> The VM viewer application presents the cursor plane content by pushing
> them to the host window system as the pointer cursor.

Yes (i.e. gdk_window_set_cursor() on the host).

> This means the host window system will be autonomously moving the
> cursor plane image around, completely disregarding what the guest KMS
> client programmed into CRTC_X, CRTC_Y.

Yes.

That is combined with a virtual input device sending absolute
coordinates (i.e. tablet), so mouse clicks land at the correct place.
And that is the point where having the hotspot information is essential
on the host side.

> Given this UAPI contract, it is very easy for userspace to make the
> conclusion that a cursor plane is just another plane it can use for
> whatever it wants. Weston and wlroots indeed leverage this, putting
> also normal windows and other stuff to the cursor plane when they
> happen to fit.

virtual machine display devices typically offer small (64x64) cursor
planes, so unlike the 512x512 planes I've seen offered by i915 they are
hardly usable for anything but cursors.  Likewise additional overlay
planes typically not offered, so the classic primary+cursor setup is
pretty much the only reasonable option userspace has.

> I believe the solution has two parts:
> 
> - The guest KMS driver needs to know whether the guest userspace is
>   prepared for the cursor plane being commandeered. If userspace does
>   not indicate it is prepared for it, commandeering must not happen.

Yes.  That isn't much of a problem in practice though due to the limited
driver/device offerings outlined above.

> - Cursor hotspot needs new KMS properties, and a KMS client must set
>   them if the KMS client indicates it is prepared for cursor plane
>   commandeering.

Yes, and that is what hurts in practice and thus caused the blacklists
being created.

> There are further problems with cursor plane commandeering. The 2020
> email thread Simon linked to discusses the problem of pointer devices:
> if VM guest userspace takes pointer input from multiple sources, how
> will the VM stack know which virtual input device, if any, should drive
> the cursor plane position?

Typically there is a communication path from guest to host for pointer
movements (i.e. crtc_x + crtc_y updates), so the host knows where the
guest wants the cursor plane being placed.  So in case the pointer is
moved by other means (different input device, some application warping
the pointer, ...) the host can adapt.

Nevertheless behavior is not consistent here because in some cases the
feedback loop is not wired up end-to-end.  The spice protocol has a
message type for that, so pointer warps work.  The vnc protocol has not,
so they don't.

> To me the answer to this question seems it could be intimately tied to
> the first problem: commandeering the cursor plane is allowed only if
> guest userspace tells the guest KMS driver which input device the
> cursor plane shall be related to. If no input device is indicated,
> then commandeering must not happen.

Why require an input device?  I just don't see how that would help.

For allowing the host freely move around the cursor place
("commandeering") I do see the point in enforcing that from a design
point of view, although I doubt it'll buy us much in practice given we
have broken drivers in the wild so userspace will continue to work with
blacklists.

Having some capability to negotiate "commandeering" between kernel and
userspace certainly makes sense, so we can get of the black lists
long-term (although it'll probably takes a few years ...).

> I can understand if people do not want to tackle this question,
> because it probably has not been a problem yet.

On a standard guest this isn't a problem indeed because there is only
one input device and only one crtc.

It actually is a problem for multihead configurations though.  Having
some way to map input devices to scanouts would actually be helpful.
Years ago I checked how this works for touchscreens to see whenever it
is possible to leverage that for VMs somehow.  There wasn't some obvious
way, and I forgot the details meanwhile ...

take care,
  Gerd

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-07 Thread Gerd Hoffmann

  Hi,

> I don’t think I see how that fixes anything. In particular I don’t see
> a way of fixing the old user space at all. We require hotspot info,
> old user space doesn’t set it because there’s no way of setting it on
> atomic kms. Whether we expose cursor plane or not doesn’t change the
> fact that we still require the hotspot info.

Not exposing a cursor plane at all forces swcursor, which sidesteps the
hotspot issue at the expense of a rather sluggish pointer updates
because those suddenly require a round-trip to the guest.

take care,
  Gerd

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-07 Thread Pekka Paalanen

On Fri, 03 Jun 2022 14:14:59 +
Simon Ser  wrote:

> Hi,
> 
> Please, read this thread:
> https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> 
> It has a lot of information about the pitfalls of cursor hotspot and
> other things done by VM software.
> 
> In particular: since the driver will ignore the KMS cursor plane
> position set by user-space, I don't think it's okay to just expose
> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> 
> cc wayland-devel and Pekka for user-space feedback.
> 
> On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  wrote:
> 
> > - all userspace code needs to hardcore a list of drivers which require
> > hotspots because there's no way to query from drm "does this driver
> > require hotspot"  
> 
> Can you elaborate? I'm not sure I understand what you mean here.
> 

Hi Zack and everyone,

I would like to try to reboot this discussion and explain where I come
from. Maybe I have misunderstood something.

I have no fundamental objection to adding KMS properties for cursor
hotspot, but I do want to make sure the design is fully thought out and
not simply copied from legacy KMS, because atomic KMS with universal
planes is not like legacy KMS.

To my understanding from the past discussions, the fundamental reason
why you are proposing hotspot properties is that when one is running a
desktop inside a VM and looking at it through a VM viewer application,
the pointer cursor is misplaced: the hotspot is not where the end user
sees it. This you never mentioned in any of the patches nor in the
cover letter. I can only guess that this misplacement could be the
reason why some display servers have deny-listed paravirtualized
drivers. While your goal is to get paravirtualized drivers out of the
deny-lists, we must first understand why they got there in the first
place.

Why are pointer cursors misplaced on paravirtualized drivers?

It is because the paravirtualized drivers or VM viewers do *not* place
the cursor plane at the CRTC_X, CRTC_Y position in the guest CRTC area.
This is obvious: if CRTC_X, CRTC_Y were honoured, there would be no
misplacement.

Instead, the VM stack plays clever tricks with cursor planes. I have
understood only one of those tricks, and it goes something like this.
To improve hand-eye coordination, that is to reduce the hand-to-eye
response time a.k.a latency, the VM guest KMS driver relays the cursor
plane separately to the VM viewer application. The VM viewer
application presents the cursor plane content by pushing them to the
host window system as the pointer cursor. This means the host window
system will be autonomously moving the cursor plane image around,
completely disregarding what the guest KMS client programmed into
CRTC_X, CRTC_Y. When this works, it is a huge improvement in user
experience. I believe this is called "seamless pointer" or such.

When it doesn't work, the cursor is misplaced or even completely
arbitrary guest windows start flying around as if they were the pointer
cursor.

In legacy KMS, cursors have always been very special and had their own
special UAPI with even hotspot information. There paravirtualized
drivers got away with these clever tricks because no-one bothered
putting anything but actual cursor images on the (one) cursor plane.

Then comes KMS universal planes concept. All planes are assumed roughly
equal, apart from what KMS properties tell userspace about them. The
plane type primary/overlay/cursor is still kept, but only because it
helps userspace find a KMS configuration that the driver accepts at
all. Hardware may not be able to light up a CRTC without at least one
primary plane, for example. Atomic KMS requires universal planes.

The atomic KMS UAPI contract says, that a plane is positioned at
CRTC_X, CRTC_Y inside the respective CRTC. I do not know of any
documented exceptions to this. Therefore, an atomic driver that does
not show a cursor plane at the programmed CRTC_X, CRTC_Y is violating
the UAPI contract.

See 
https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-plane-properties
and how "cursor" plane type makes no exceptions, and how CRTC_X and
CRTC_Y are defined. Also note that all hardware drivers and VKMS
apparently follow the contract.

Given this UAPI contract, it is very easy for userspace to make the
conclusion that a cursor plane is just another plane it can use for
whatever it wants. Weston and wlroots indeed leverage this, putting
also normal windows and other stuff to the cursor plane when they
happen to fit.

If a paravirtualized driver commandeers the cursor plane display
position, a possible result is that arbitrary windows start flying
around unexpectedly.

Therefore we have two problems:

- paravirtualized drivers commandeering the cursor plane
- VM software not able to implement "seamless pointer" correctly

To my understanding, this patch series concerns only the latter problem
but not the former problem. I believe the solutions to both are rel

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-06 Thread Pekka Paalanen

On Fri, 3 Jun 2022 14:38:54 +
Zack Rusin  wrote:

> > On Jun 3, 2022, at 10:32 AM, Simon Ser  wrote:
> > 
> > ⚠ External Email
> > 
> > On Friday, June 3rd, 2022 at 16:27, Zack Rusin  wrote:
> >   
> >>> In particular: since the driver will ignore the KMS cursor plane
> >>> position set by user-space, I don't think it's okay to just expose
> >>> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> >>> 
> >>> cc wayland-devel and Pekka for user-space feedback.  
> >> 
> >> I think Thomas expressed our concerns and reasons why those wouldn’t
> >> work for us back then. Is there something else you’d like to add?  
> > 
> > I disagreed, and I don't understand Thomas' reasoning.
> > 
> > KMS clients will need an update to work correctly. Adding a new prop
> > without a cap leaves existing KMS clients broken. Adding a cap allows
> > both existing KMS clients and updated KMS clients to work correctly.  
> 
> I’m not sure what you mean here. They are broken right now. That’s
> what we’re fixing. That’s the reason why the virtualized drivers are
> on deny-lists for all atomic kms. So nothing needs to be updated. If

Hi Zack,

please, stop removing all email quoting level indicators, you have been
doing that a lot in your more recent replies. It makes reading the
emails really difficult, and I had to stop trying to make sense of the
latest emails.


It is really unfortunate that display servers have driver deny-lists
indeed. All the bug reports they got from being broken should have been
denied and forwarded to the respective drivers instead for breaking the
KMS UAPI. OTOH, I understand that some userspace projects do not want
to stop because of few broken but popular drivers.

> you have a kms client that was using virtualized drivers with atomic
> kms then mouse clicks never worked correctly. So, yes, clients need
> to set cursor hotspot if they want mouse to work correctly with
> virtualized drivers.

I'm very much agreeing with Simon. He has made similar questions and
comments that occurred to me.

Reading as much of the discussion between Simon and Zack as I could, it
seems every time Simon gets to the point, Zack hops to a completely
different use case to make Simon's argument no longer apply.

Like, root-window-less per-window remoting through KMS? How is that even
possible?

> >>> On Thursday, June 2nd, 2022 at 17:42, Zack Rusin z...@kde.org wrote:
> >>>   
>  - all userspace code needs to hardcore a list of drivers which require
>  hotspots because there's no way to query from drm "does this driver
>  require hotspot"  
> >>> 
> >>> Can you elaborate? I'm not sure I understand what you mean here.  
> >> 
> >> Only some drivers require informations about cursor hotspot, user space
> >> currently has no way of figuring out which ones are those, i.e. amdgpu
> >> doesn’t care about hotspots, qxl does so when running on qxl without
> >> properly set cursor hotspot atomic kms will result in a desktop where
> >> mouse clicks have incorrect coordinates.
> >> 
> >> There’s no way to differentiate between drivers that do not care about
> >> cursor hotspots and ones that absolutely require it.  
> > 
> > Only VM drivers should expose the hotspot properties. Real drivers like
> > amdgpu must not expose it.  
> 
> Yes, that’s what I said. There’s no way to differentiate between
> amdgpu that doesn’t have those properties and virtio_gpu driver from
> a kernel before hotspot properties were added.

Why would userspace want to tell the difference between a driver that
truly does not need those properties, and a driver that did not add
those properties *yet*?


Thanks,
pq


pgpVkyvtF0gw0.pgp
Description: OpenPGP digital signature

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-06 Thread Simon Ser

On Sunday, June 5th, 2022 at 20:16, Zack Rusin  wrote:

> > At any rate, I consider broken any driver which exposes a cursor plane,
> > then doesn't display it exactly at the CRTC_X/CRTC_Y
>
> But we do… The cursor is at crtc_x, crtc_y.

How do you show the cursor on the host side then? Pretty sure you use
the host X11/Wayland cursor? In which case nothing guarantees that the
host cursor position matches CRTC_X/CRTC_Y.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-05 Thread Zack Rusin


> On Jun 5, 2022, at 12:26 PM, Simon Ser  wrote:
> 
> --- Original Message ---
> On Sunday, June 5th, 2022 at 17:47, Zack Rusin  wrote:
> 
 Also, let me point this out because it also seems to be a fundamental
 misunderstanding, user space implementing software cursor doesn’t fix
 anything. Just leaves everything broken in different ways. The reason
 virtualized drivers went away from software cursors is because it makes it
 impossible to make it work with a bunch of interesting and desirable
 scenarios, e.g. unity mode where the guest might have a terminal and 
 browser
 open and then the virtual machine software creates windows out of those, so
 you don’t have the entire desktop of the guest but instead native looking
 windows with the apps. In that case guest has no way of knowing when the
 cursor leaves the window, so to make seemless cursors work you’d need to
 implement irc or backdoors that send that information from the host to the
 guest, then have virtualized drivers create some sort of uevent, to send to
 the userspace to let it know to hide the cursor because it actually left 
 the
 window. That’s a trivial scenario and there’s a lot more with mice that are
 remoted themselves, guests with singular or maybe many apps exported,
 possibly overlapping on the host but all within a desktop in the guest, 
 etc.
>>> 
>>> Are you saying that the current broken behavior is better than software
>>> cursors?
>> 
>> They’re broken in very different ways. You’re asking me which bugs do
>> I prefer. Ultimately the current lack of hotspots leaves mouse unusable
>> but I don’t see any reason to trade that for another set of bugs instead
>> of just fixing it (either via fallback to legacy or using the new hotspot
>> api).
> 
> Software cursors aren't a bug. They're a fallback.

They work well, or usually well enough, on native but not with virtual 
machines. They break a lot of features. The nature of virtualization is that 
the guest doesn’t explicitly get access to a lot of host side info. If you move 
to software cursor you automatically lose all that info (unless, as I mentioned 
before, you rewrite hypervisors to push all that info to guests again, either 
via backdoors, irq or register accesses). Software cursor just doesn’t work 
with modern hypervisors well enough to be a reasonable replacement. 

>>> New user-space supports setting the hotspot prop, and is aware that it 
>>> can't
>>> set the cursor plane position, so the cursor plane can be exposed again 
>>> when
>>> the cap is enabled.
>> 
>> But we still use cursor plane position. Hotspots are offsets from
>> cursor plane positions. Both are required.
> 
> No, VM drivers don't need and disregard the cursor position AFAIK. They
> replace it with the host cursor's position.
 
 Iirc they all use it.
>>> 
>>> What do they use it for?
>>> 
>>> The whole point of this discussion is to be able to display the guest's 
>>> cursor
>>> image on the host's cursor, so that latency is reduced?
>> 
>> 
>> Ah, I think I see now where the confusion is coming from. No, it’s
>> definitely not. This has nothing to do with latency. By default
>> position of a mouse cursor doesn’t tell us where the point that is
>> actually used as an activation of a click is, e.g. a mouse cursor image
>> with an arrow pointing to the top-left and a mouse cursor image pointing
>> to the bottom right - both at the same position, as you can imagine it will
>> become impossible to use the desktop if the click defaults to the top left,
>> especially as the number of cursor images increases, you need information
>> about which point within the cursor image activates the click, that’s the
>> hotspot. You need to know the position of the image and you need to know
>> which point within that image is responsible for actually activating the
>> events.
> 
> Yeah, that's what a hotspot is.
> 
> By "the whole point of this discussion", I meant that the whole point
> for VM drivers to expose a hardware cursor is to improve latency (and
> provide other related features).
> 
> At any rate, I consider broken any driver which exposes a cursor plane,
> then doesn't display it exactly at the CRTC_X/CRTC_Y

But we do… The cursor is at crtc_x, crtc_y.

> or misbehaves if it's missing hotspot info.

That doesn’t follow at all. How can they not behave incorrectly if the 
information is missing?

Maybe we can refocus the discussion because it seems like we’re moving in 
circles. Your proposal for a feature cap and removal of the cursor plane 
doesn’t fix anything:
- current user space which doesn’t fallback to legacy kms with virtualized 
driver is completely broken and obviously will always remain broken on all 
already released kernels
- making the same software fallback to software cursor breaks a massive amount 
of interactions in VMs
so what are you proposing?

z

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-05 Thread Simon Ser







--- Original Message ---
On Sunday, June 5th, 2022 at 17:47, Zack Rusin  wrote:

> > > Also, let me point this out because it also seems to be a fundamental
> > > misunderstanding, user space implementing software cursor doesn’t fix
> > > anything. Just leaves everything broken in different ways. The reason
> > > virtualized drivers went away from software cursors is because it makes it
> > > impossible to make it work with a bunch of interesting and desirable
> > > scenarios, e.g. unity mode where the guest might have a terminal and 
> > > browser
> > > open and then the virtual machine software creates windows out of those, 
> > > so
> > > you don’t have the entire desktop of the guest but instead native looking
> > > windows with the apps. In that case guest has no way of knowing when the
> > > cursor leaves the window, so to make seemless cursors work you’d need to
> > > implement irc or backdoors that send that information from the host to the
> > > guest, then have virtualized drivers create some sort of uevent, to send 
> > > to
> > > the userspace to let it know to hide the cursor because it actually left 
> > > the
> > > window. That’s a trivial scenario and there’s a lot more with mice that 
> > > are
> > > remoted themselves, guests with singular or maybe many apps exported,
> > > possibly overlapping on the host but all within a desktop in the guest, 
> > > etc.
> >
> > Are you saying that the current broken behavior is better than software
> > cursors?
>
> They’re broken in very different ways. You’re asking me which bugs do
> I prefer. Ultimately the current lack of hotspots leaves mouse unusable
> but I don’t see any reason to trade that for another set of bugs instead
> of just fixing it (either via fallback to legacy or using the new hotspot
> api).

Software cursors aren't a bug. They're a fallback.

> > > > > > New user-space supports setting the hotspot prop, and is aware that 
> > > > > > it can't
> > > > > > set the cursor plane position, so the cursor plane can be exposed 
> > > > > > again when
> > > > > > the cap is enabled.
> > > > >
> > > > > But we still use cursor plane position. Hotspots are offsets from
> > > > > cursor plane positions. Both are required.
> > > >
> > > > No, VM drivers don't need and disregard the cursor position AFAIK. They
> > > > replace it with the host cursor's position.
> > >
> > > Iirc they all use it.
> >
> > What do they use it for?
> >
> > The whole point of this discussion is to be able to display the guest's 
> > cursor
> > image on the host's cursor, so that latency is reduced?
>
>
> Ah, I think I see now where the confusion is coming from. No, it’s
> definitely not. This has nothing to do with latency. By default
> position of a mouse cursor doesn’t tell us where the point that is
> actually used as an activation of a click is, e.g. a mouse cursor image
> with an arrow pointing to the top-left and a mouse cursor image pointing
> to the bottom right - both at the same position, as you can imagine it will
> become impossible to use the desktop if the click defaults to the top left,
> especially as the number of cursor images increases, you need information
> about which point within the cursor image activates the click, that’s the
> hotspot. You need to know the position of the image and you need to know
> which point within that image is responsible for actually activating the
> events.

Yeah, that's what a hotspot is.

By "the whole point of this discussion", I meant that the whole point
for VM drivers to expose a hardware cursor is to improve latency (and
provide other related features).

At any rate, I consider broken any driver which exposes a cursor plane,
then doesn't display it exactly at the CRTC_X/CRTC_Y or misbehaves if
it's missing hotspot info.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-05 Thread Zack Rusin



> On Jun 5, 2022, at 3:30 AM, Simon Ser  wrote:
> 
> ⚠ External Email
> 
> On Friday, June 3rd, 2022 at 20:31, Zack Rusin  wrote:
> 
>>> My proposal was:
>>> 
>>> - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). 
>>> Only
>>> user-space which supports the hotspot props will enable it.
>>> - By default, don't expose a cursor plane, because current user-space 
>>> doesn't
>>> support it (Weston might put a window in the cursor plane, and nobody 
>>> can
>>> report hotspot).
>>> - If the KMS client enables the cap, advertise the cursor
>>> plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y 
>>> properties
>>> since the driver will pick the position.
>>> 
>>> That way both old and new user-space are fixed.
>> 
>> I don’t think I see how that fixes anything. In particular I don’t see a 
>> way
>> of fixing the old user space at all. We require hotspot info, old user 
>> space
>> doesn’t set it because there’s no way of setting it on atomic kms.
> 
> Old atomic user-space is fixed by removing the cursor plane. Then old
> atomic user-space will fallback to drawing the cursor itself, e.g. via
> OpenGL.
 
 But it’s not fixed, because the driver is still on a deny-list and
 nothing implements this. You’re saying you could potentially “fix” by
 implementing client side cursor handling in all kms clients? That’s a
 hard sell if the user space can just put the virtualized driver on
 deny-lists and fallback to use old kms which supports hotspots.
>>> 
>>> What deny-list are you referring to?
>>> 
>>> All compositors I know of implement a fallback when no cursor plane is
>>> usable.
>> 
>> I put the links in the first email in the
>> series:
>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.gnome.org%2FGNOME%2Fmutter%2F-%2Fblob%2Fmain%2Fsrc%2Fbackends%2Fnative%2Fmeta-kms-impl-device-atomic.c%23L1188&data=05%7C01%7Czackr%40vmware.com%7C2b0ab6c67e7d4bfb618a08da46c5394f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637900110157712044%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=wggJaNScF0ziIG%2BfSdSUKBVZGoNjtm4Q8amS7SbJa%2FY%3D&reserved=0
>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Finvent.kde.org%2Fplasma%2Fkwin%2F-%2Fblob%2Fmaster%2Fsrc%2Fbackends%2Fdrm%2Fdrm_gpu.cpp%23L156&data=05%7C01%7Czackr%40vmware.com%7C2b0ab6c67e7d4bfb618a08da46c5394f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637900110157712044%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2BCTEJ9XtlI9kuKJJZVMNodtkZSkRIv8RN9jSRAM1pqM%3D&reserved=0
> 
> I was not aware of these lists. Having to work-around drivers in user-space is
> really not great.
> 
> But regardless, that doesn't really change anything. With my proposal:
> 
> - Old user-space with hacky deny-lists (Mutter, KWin) still goes through the
>  legacy uAPI.
> - Old user-space without the hacky deny-lists (Weston, wlroots) uses software
>  cursors and is not broken anymore.
> - New user-space can report hotspot and is not broken.

Yea, that doesn’t work, but I think below I stopped where the issue is.

>> Also, let me point this out because it also seems to be a fundamental
>> misunderstanding, user space implementing software cursor doesn’t fix
>> anything. Just leaves everything broken in different ways. The reason
>> virtualized drivers went away from software cursors is because it makes it
>> impossible to make it work with a bunch of interesting and desirable
>> scenarios, e.g. unity mode where the guest might have a terminal and browser
>> open and then the virtual machine software creates windows out of those, so
>> you don’t have the entire desktop of the guest but instead native looking
>> windows with the apps. In that case guest has no way of knowing when the
>> cursor leaves the window, so to make seemless cursors work you’d need to
>> implement irc or backdoors that send that information from the host to the
>> guest, then have virtualized drivers create some sort of uevent, to send to
>> the userspace to let it know to hide the cursor because it actually left the
>> window. That’s a trivial scenario and there’s a lot more with mice that are
>> remoted themselves, guests with singular or maybe many apps exported,
>> possibly overlapping on the host but all within a desktop in the guest, etc.
> 
> Are you saying that the current broken behavior is better than software
> cursors?

They’re broken in very different ways. You’re asking me which bugs do I prefer. 
Ultimately the current lack of hotspots leaves mouse unusable but I don’t see 
any reason to trade that for another set of bugs instead of just fixing it 
(either via fallback to legacy or using the new hotspot api).

> New user-space supports setting the hotspot prop, and is aware that

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-05 Thread Simon Ser

On Saturday, June 4th, 2022 at 23:19, Hans de Goede  wrote:

> >>> My proposal was:
> >>>
> >>> - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). 
> >>> Only
> >>> user-space which supports the hotspot props will enable it.
> >>> - By default, don't expose a cursor plane, because current user-space 
> >>> doesn't
> >>> support it (Weston might put a window in the cursor plane, and nobody can
> >>> report hotspot).
> >>> - If the KMS client enables the cap, advertise the cursor
> >>> plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
> >>> since the driver will pick the position.
> >>>
> >>> That way both old and new user-space are fixed.
> >>
> >> I don’t think I see how that fixes anything. In particular I don’t see a 
> >> way
> >> of fixing the old user space at all. We require hotspot info, old user 
> >> space
> >> doesn’t set it because there’s no way of setting it on atomic kms.
> >
> > Old atomic user-space is fixed by removing the cursor plane. Then old
> > atomic user-space will fallback to drawing the cursor itself, e.g. via
> > OpenGL.
>
> That is just a terrible idea, the current situation is that userspace has a
> hardcoded list of drivers which need hotspots, so it uses the old non-atomic
> APIs on that "hw" since the atomic APIs don't support hotspots.

*Some* user-space does this (Mutter, KWin). There is also user-space which
doesn't (Weston, wlroots).

> The downsides I see to your proposal are:
>
> 1. Falling back to sw cursor rendering instead of using the old APIs would
> be a pretty significant regression in user experience. I know that in theory
> sw rendering can be made to not flicker, but in practice it tends to be a
> flickery mess.

Old Mutter/KWin with the deny-lists cannot be updated and will still use the
legacy uAPI. New Muter/KWin with support for atomic hotspot will not need a
deny-list anymore. No breakage here.

> 2. It does not help anything since userspace is still hardcoded to use
> the old, hotspot aware non-atomic API anyways. So it would only make things
> worse since hiding the cursorplane for userspace which does not set the CAP
> would mean the the old non-atomic API also stops working. Or this would add
> extra complications in the kernel to still keep the old APIs working.

The cursor plane would only be hidden when user-space has enabled
DRM_CAP_UNIVERSAL_PLANES. IOW, user-space only supporting the legacy uAPI will
still work as it does today.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-05 Thread Simon Ser

On Friday, June 3rd, 2022 at 20:31, Zack Rusin  wrote:

> > > > > > My proposal was:
> > > > > >
> > > > > > - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better 
> > > > > > name). Only
> > > > > > user-space which supports the hotspot props will enable it.
> > > > > > - By default, don't expose a cursor plane, because current 
> > > > > > user-space doesn't
> > > > > > support it (Weston might put a window in the cursor plane, and 
> > > > > > nobody can
> > > > > > report hotspot).
> > > > > > - If the KMS client enables the cap, advertise the cursor
> > > > > > plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y 
> > > > > > properties
> > > > > > since the driver will pick the position.
> > > > > >
> > > > > > That way both old and new user-space are fixed.
> > > > >
> > > > > I don’t think I see how that fixes anything. In particular I don’t 
> > > > > see a way
> > > > > of fixing the old user space at all. We require hotspot info, old 
> > > > > user space
> > > > > doesn’t set it because there’s no way of setting it on atomic kms.
> > > >
> > > > Old atomic user-space is fixed by removing the cursor plane. Then old
> > > > atomic user-space will fallback to drawing the cursor itself, e.g. via
> > > > OpenGL.
> > >
> > > But it’s not fixed, because the driver is still on a deny-list and
> > > nothing implements this. You’re saying you could potentially “fix” by
> > > implementing client side cursor handling in all kms clients? That’s a
> > > hard sell if the user space can just put the virtualized driver on
> > > deny-lists and fallback to use old kms which supports hotspots.
> >
> > What deny-list are you referring to?
> >
> > All compositors I know of implement a fallback when no cursor plane is
> > usable.
>
> I put the links in the first email in the
> series:
> https://gitlab.gnome.org/GNOME/mutter/-/blob/main/src/backends/native/meta-kms-impl-device-atomic.c#L1188
> https://invent.kde.org/plasma/kwin/-/blob/master/src/backends/drm/drm_gpu.cpp#L156

I was not aware of these lists. Having to work-around drivers in user-space is
really not great.

But regardless, that doesn't really change anything. With my proposal:

- Old user-space with hacky deny-lists (Mutter, KWin) still goes through the
  legacy uAPI.
- Old user-space without the hacky deny-lists (Weston, wlroots) uses software
  cursors and is not broken anymore.
- New user-space can report hotspot and is not broken.

> Also, let me point this out because it also seems to be a fundamental
> misunderstanding, user space implementing software cursor doesn’t fix
> anything. Just leaves everything broken in different ways. The reason
> virtualized drivers went away from software cursors is because it makes it
> impossible to make it work with a bunch of interesting and desirable
> scenarios, e.g. unity mode where the guest might have a terminal and browser
> open and then the virtual machine software creates windows out of those, so
> you don’t have the entire desktop of the guest but instead native looking
> windows with the apps. In that case guest has no way of knowing when the
> cursor leaves the window, so to make seemless cursors work you’d need to
> implement irc or backdoors that send that information from the host to the
> guest, then have virtualized drivers create some sort of uevent, to send to
> the userspace to let it know to hide the cursor because it actually left the
> window. That’s a trivial scenario and there’s a lot more with mice that are
> remoted themselves, guests with singular or maybe many apps exported,
> possibly overlapping on the host but all within a desktop in the guest, etc.

Are you saying that the current broken behavior is better than software
cursors?

> > > > New user-space supports setting the hotspot prop, and is aware that it 
> > > > can't
> > > > set the cursor plane position, so the cursor plane can be exposed again 
> > > > when
> > > > the cap is enabled.
> > >
> > > But we still use cursor plane position. Hotspots are offsets from
> > > cursor plane positions. Both are required.
> >
> > No, VM drivers don't need and disregard the cursor position AFAIK. They
> > replace it with the host cursor's position.
>
> Iirc they all use it.

What do they use it for?

The whole point of this discussion is to be able to display the guest's cursor
image on the host's cursor, so that latency is reduced? When the VM software
does this, why does it need to use CRTC_X/CRTC_Y, since these are thrown away
by the host?

> I guess we could repurpose src_x|y to mean mouse hotspots and write patches
> for userspace to use that instead of explicit properties

That sounds like a terrible idea.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

2022-06-04 Thread Hans de Goede

Hi,

On 6/3/22 17:22, Simon Ser wrote:
> On Friday, June 3rd, 2022 at 17:17, Zack Rusin  wrote:
> 
>>
>>> On Jun 3, 2022, at 10:56 AM, Simon Ser  wrote:
>>> ⚠ External Email
>>>
>>> On Friday, June 3rd, 2022 at 16:38, Zack Rusin  wrote:
>>>
> On Jun 3, 2022, at 10:32 AM, Simon Ser  wrote:
>
> ⚠ External Email
>
> On Friday, June 3rd, 2022 at 16:27, Zack Rusin  wrote:
>
>>> In particular: since the driver will ignore the KMS cursor plane
>>> position set by user-space, I don't think it's okay to just expose
>>> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
>>>
>>> cc wayland-devel and Pekka for user-space feedback.
>>
>> I think Thomas expressed our concerns and reasons why those wouldn’t
>> work for us back then. Is there something else you’d like to add?
>
> I disagreed, and I don't understand Thomas' reasoning.
>
> KMS clients will need an update to work correctly. Adding a new prop
> without a cap leaves existing KMS clients broken. Adding a cap allows
> both existing KMS clients and updated KMS clients to work correctly.

 I’m not sure what you mean here. They are broken right now. That’s what 
 we’re
 fixing. That’s the reason why the virtualized drivers are on deny-lists for
 all atomic kms. So nothing needs to be updated. If you have a kms client 
 that
 was using virtualized drivers with atomic kms then mouse clicks never 
 worked
 correctly.

 So, yes, clients need to set cursor hotspot if they want mouse to work
 correctly with virtualized drivers.
>>>
>>> My proposal was:
>>>
>>> - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). Only
>>> user-space which supports the hotspot props will enable it.
>>> - By default, don't expose a cursor plane, because current user-space 
>>> doesn't
>>> support it (Weston might put a window in the cursor plane, and nobody can
>>> report hotspot).
>>> - If the KMS client enables the cap, advertise the cursor
>>> plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
>>> since the driver will pick the position.
>>>
>>> That way both old and new user-space are fixed.
>>
>> I don’t think I see how that fixes anything. In particular I don’t see a way
>> of fixing the old user space at all. We require hotspot info, old user space
>> doesn’t set it because there’s no way of setting it on atomic kms.
> 
> Old atomic user-space is fixed by removing the cursor plane. Then old
> atomic user-space will fallback to drawing the cursor itself, e.g. via
> OpenGL.

That is just a terrible idea, the current situation is that userspace has a
hardcoded list of drivers which need hotspots, so it uses the old non-atomic
APIs on that "hw" since the atomic APIs don't support hotspots.

The downsides I see to your proposal are:

1. Falling back to sw cursor rendering instead of using the old APIs would
be a pretty significant regression in user experience. I know that in theory
sw rendering can be made to not flicker, but in practice it tends to be a
flickery mess.

2. It does not help anything since userspace is still hardcoded to use
the old, hotspot aware non-atomic API anyways. So it would only make things
worse since hiding the cursorplane for userspace which does not set the CAP
would mean the the old non-atomic API also stops working. Or this would add
extra complications in the kernel to still keep the old APIs working.

I do think that a CAP might be a good idea, but the disabling of the
cursor plane based on the CAP does not sound like a good idea to me.

Regards,

Hans

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

On Jun 3, 2022, at 11:49 AM, Simon Ser
mailto:cont...@emersion.fr>> wrote:

⚠ External Email

On Friday, June 3rd, 2022 at 17:32, Zack Rusin
mailto:za...@vmware.com>> wrote:

On Jun 3, 2022, at 11:22 AM, Simon Ser
mailto:cont...@emersion.fr>> wrote:
⚠ External Email

On Friday, June 3rd, 2022 at 17:17, Zack Rusin
mailto:za...@vmware.com>> wrote:

On Jun 3, 2022, at 10:56 AM, Simon Ser
mailto:cont...@emersion.fr>> wrote:
⚠ External Email

On Friday, June 3rd, 2022 at 16:38, Zack Rusin
mailto:za...@vmware.com>> wrote:

On Jun 3, 2022, at 10:32 AM, Simon Ser
mailto:cont...@emersion.fr>> wrote:

⚠ External Email

On Friday, June 3rd, 2022 at 16:27, Zack Rusin
mailto:za...@vmware.com>> wrote:

In particular: since the driver will ignore the KMS cursor plane
position set by user-space, I don't think it's okay to just expose
without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).

cc wayland-devel and Pekka for user-space feedback.

I think Thomas expressed our concerns and reasons why those wouldn’t
work for us back then. Is there something else you’d like to add?

I disagreed, and I don't understand Thomas' reasoning.

KMS clients will need an update to work correctly. Adding a new prop
without a cap leaves existing KMS clients broken. Adding a cap allows
both existing KMS clients and updated KMS clients to work correctly.

I’m not sure what you mean here. They are broken right now. That’s what we’re
fixing. That’s the reason why the virtualized drivers are on deny-lists for
all atomic kms. So nothing needs to be updated. If you have a kms client that
was using virtualized drivers with atomic kms then mouse clicks never worked
correctly.

So, yes, clients need to set cursor hotspot if they want mouse to work
correctly with virtualized drivers.

My proposal was:

- Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). Only
user-space which supports the hotspot props will enable it.
- By default, don't expose a cursor plane, because current user-space doesn't
support it (Weston might put a window in the cursor plane, and nobody can
report hotspot).
- If the KMS client enables the cap, advertise the cursor
plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
since the driver will pick the position.

That way both old and new user-space are fixed.

I don’t think I see how that fixes anything. In particular I don’t see a way
of fixing the old user space at all. We require hotspot info, old user space
doesn’t set it because there’s no way of setting it on atomic kms.

Old atomic user-space is fixed by removing the cursor plane. Then old
atomic user-space will fallback to drawing the cursor itself, e.g. via
OpenGL.

But it’s not fixed, because the driver is still on a deny-list and
nothing implements this. You’re saying you could potentially “fix” by
implementing client side cursor handling in all kms clients? That’s a
hard sell if the user space can just put the virtualized driver on
deny-lists and fallback to use old kms which supports hotspots.

What deny-list are you referring to?

All compositors I know of implement a fallback when no cursor plane is
usable.

I put the links in the first email in the series:
https://gitlab.gnome.org/GNOME/mutter/-/blob/main/src/backends/native/meta-kms-impl-device-atomic.c#L1188
https://invent.kde.org/plasma/kwin/-/blob/master/src/backends/drm/drm_gpu.cpp#L156

Also, let me point this out because it also seems to be a fundamental
misunderstanding, user space implementing software cursor doesn’t fix anything.
Just leaves everything broken in different ways. The reason virtualized drivers
went away from software cursors is because it makes it impossible to make it
work with a bunch of interesting and desirable scenarios, e.g. unity mode where
the guest might have a terminal and browser open and then the virtual machine
software creates windows out of those, so you don’t have the entire desktop of
the guest but instead native looking windows with the apps. In that case guest
has no way of knowing when the cursor leaves the window, so to make seemless
cursors work you’d need to implement irc or backdoors that send that
information from the host to the guest, then have virtualized drivers create
some sort of uevent, to send to the userspace to let it know to hide the cursor
because it actually left the window. That’s a trivial scenario and there’s a
lot more with mice that are remoted themselves, guests with singular or maybe
many apps exported, possibly overlapping on the host but all within a desktop
in the guest, etc.

New user-space supports setting the hotspot prop, and is aware that it can't
set the cursor plane position, so the cursor plane can be exposed again when
the cap is enabled.

But we still use cursor plane position. Hotspots are offsets from
cursor plane positions. Both are required.

No, VM drivers don't need and disregard the cursor position AFAIK. They
replace it with the host cursor's p

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS



On Friday, June 3rd, 2022 at 17:32, Zack Rusin  wrote:

> > On Jun 3, 2022, at 11:22 AM, Simon Ser  wrote:
> > ⚠ External Email
> >
> > On Friday, June 3rd, 2022 at 17:17, Zack Rusin  wrote:
> >
> >
> > >
> > >
> > > > On Jun 3, 2022, at 10:56 AM, Simon Ser  wrote:
> > > > ⚠ External Email
> > > >
> > > > On Friday, June 3rd, 2022 at 16:38, Zack Rusin  wrote:
> > > >
> > > >
> > > > > > On Jun 3, 2022, at 10:32 AM, Simon Ser  wrote:
> > > > > >
> > > > > > ⚠ External Email
> > > > > >
> > > > > > On Friday, June 3rd, 2022 at 16:27, Zack Rusin  
> > > > > > wrote:
> > > > > >
> > > > > >
> > > > > > > > In particular: since the driver will ignore the KMS cursor plane
> > > > > > > > position set by user-space, I don't think it's okay to just 
> > > > > > > > expose
> > > > > > > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> > > > > > > >
> > > > > > > > cc wayland-devel and Pekka for user-space feedback.
> > > > > > >
> > > > > > >
> > > > > > > I think Thomas expressed our concerns and reasons why those 
> > > > > > > wouldn’t
> > > > > > > work for us back then. Is there something else you’d like to add?
> > > > > >
> > > > > >
> > > > > > I disagreed, and I don't understand Thomas' reasoning.
> > > > > >
> > > > > > KMS clients will need an update to work correctly. Adding a new prop
> > > > > > without a cap leaves existing KMS clients broken. Adding a cap 
> > > > > > allows
> > > > > > both existing KMS clients and updated KMS clients to work correctly.
> > > > >
> > > > >
> > > > > I’m not sure what you mean here. They are broken right now. That’s 
> > > > > what we’re
> > > > > fixing. That’s the reason why the virtualized drivers are on 
> > > > > deny-lists for
> > > > > all atomic kms. So nothing needs to be updated. If you have a kms 
> > > > > client that
> > > > > was using virtualized drivers with atomic kms then mouse clicks never 
> > > > > worked
> > > > > correctly.
> > > > >
> > > > > So, yes, clients need to set cursor hotspot if they want mouse to work
> > > > > correctly with virtualized drivers.
> > > >
> > > >
> > > > My proposal was:
> > > >
> > > > - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). 
> > > > Only
> > > > user-space which supports the hotspot props will enable it.
> > > > - By default, don't expose a cursor plane, because current user-space 
> > > > doesn't
> > > > support it (Weston might put a window in the cursor plane, and nobody 
> > > > can
> > > > report hotspot).
> > > > - If the KMS client enables the cap, advertise the cursor
> > > > plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y 
> > > > properties
> > > > since the driver will pick the position.
> > > >
> > > > That way both old and new user-space are fixed.
> > >
> > >
> > > I don’t think I see how that fixes anything. In particular I don’t see a 
> > > way
> > > of fixing the old user space at all. We require hotspot info, old user 
> > > space
> > > doesn’t set it because there’s no way of setting it on atomic kms.
> >
> >
> > Old atomic user-space is fixed by removing the cursor plane. Then old
> > atomic user-space will fallback to drawing the cursor itself, e.g. via
> > OpenGL.
>
> But it’s not fixed, because the driver is still on a deny-list and
> nothing implements this. You’re saying you could potentially “fix” by
> implementing client side cursor handling in all kms clients? That’s a
> hard sell if the user space can just put the virtualized driver on
> deny-lists and fallback to use old kms which supports hotspots.

What deny-list are you referring to?

All compositors I know of implement a fallback when no cursor plane is
usable.

> > New user-space supports setting the hotspot prop, and is aware that it can't
> > set the cursor plane position, so the cursor plane can be exposed again when
> > the cap is enabled.
>
> But we still use cursor plane position. Hotspots are offsets from
> cursor plane positions. Both are required.

No, VM drivers don't need and disregard the cursor position AFAIK. They
replace it with the host cursor's position.

This is what breaks Weston, breaks uAPI expectations, and IMHO is
unacceptable without KMS client opt-in.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS



On Jun 3, 2022, at 11:22 AM, Simon Ser 
mailto:cont...@emersion.fr>> wrote:

⚠ External Email

On Friday, June 3rd, 2022 at 17:17, Zack Rusin 
mailto:za...@vmware.com>> wrote:


On Jun 3, 2022, at 10:56 AM, Simon Ser 
mailto:cont...@emersion.fr>> wrote:
⚠ External Email

On Friday, June 3rd, 2022 at 16:38, Zack Rusin 
mailto:za...@vmware.com>> wrote:

On Jun 3, 2022, at 10:32 AM, Simon Ser 
mailto:cont...@emersion.fr>> wrote:

⚠ External Email

On Friday, June 3rd, 2022 at 16:27, Zack Rusin 
mailto:za...@vmware.com>> wrote:

In particular: since the driver will ignore the KMS cursor plane
position set by user-space, I don't think it's okay to just expose
without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).

cc wayland-devel and Pekka for user-space feedback.

I think Thomas expressed our concerns and reasons why those wouldn’t
work for us back then. Is there something else you’d like to add?

I disagreed, and I don't understand Thomas' reasoning.

KMS clients will need an update to work correctly. Adding a new prop
without a cap leaves existing KMS clients broken. Adding a cap allows
both existing KMS clients and updated KMS clients to work correctly.

I’m not sure what you mean here. They are broken right now. That’s what we’re
fixing. That’s the reason why the virtualized drivers are on deny-lists for
all atomic kms. So nothing needs to be updated. If you have a kms client that
was using virtualized drivers with atomic kms then mouse clicks never worked
correctly.

So, yes, clients need to set cursor hotspot if they want mouse to work
correctly with virtualized drivers.

My proposal was:

- Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). Only
user-space which supports the hotspot props will enable it.
- By default, don't expose a cursor plane, because current user-space doesn't
support it (Weston might put a window in the cursor plane, and nobody can
report hotspot).
- If the KMS client enables the cap, advertise the cursor
plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
since the driver will pick the position.

That way both old and new user-space are fixed.

I don’t think I see how that fixes anything. In particular I don’t see a way
of fixing the old user space at all. We require hotspot info, old user space
doesn’t set it because there’s no way of setting it on atomic kms.

Old atomic user-space is fixed by removing the cursor plane. Then old
atomic user-space will fallback to drawing the cursor itself, e.g. via
OpenGL.

But it’s not fixed, because the driver is still on a deny-list and nothing 
implements this. You’re saying you could potentially “fix” by implementing 
client side cursor handling in all kms clients? That’s a hard sell if the user 
space can just put the virtualized driver on deny-lists and fallback to use old 
kms which supports hotspots.

New user-space supports setting the hotspot prop, and is aware that it can't
set the cursor plane position, so the cursor plane can be exposed again when
the cap is enabled.

But we still use cursor plane position. Hotspots are offsets from cursor plane 
positions. Both are required.

z

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

On Friday, June 3rd, 2022 at 17:17, Zack Rusin  wrote:

>
> > On Jun 3, 2022, at 10:56 AM, Simon Ser  wrote:
> > ⚠ External Email
> >
> > On Friday, June 3rd, 2022 at 16:38, Zack Rusin  wrote:
> >
> > > > On Jun 3, 2022, at 10:32 AM, Simon Ser  wrote:
> > > >
> > > > ⚠ External Email
> > > >
> > > > On Friday, June 3rd, 2022 at 16:27, Zack Rusin  wrote:
> > > >
> > > > > > In particular: since the driver will ignore the KMS cursor plane
> > > > > > position set by user-space, I don't think it's okay to just expose
> > > > > > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> > > > > >
> > > > > > cc wayland-devel and Pekka for user-space feedback.
> > > > >
> > > > > I think Thomas expressed our concerns and reasons why those wouldn’t
> > > > > work for us back then. Is there something else you’d like to add?
> > > >
> > > > I disagreed, and I don't understand Thomas' reasoning.
> > > >
> > > > KMS clients will need an update to work correctly. Adding a new prop
> > > > without a cap leaves existing KMS clients broken. Adding a cap allows
> > > > both existing KMS clients and updated KMS clients to work correctly.
> > >
> > > I’m not sure what you mean here. They are broken right now. That’s what 
> > > we’re
> > > fixing. That’s the reason why the virtualized drivers are on deny-lists 
> > > for
> > > all atomic kms. So nothing needs to be updated. If you have a kms client 
> > > that
> > > was using virtualized drivers with atomic kms then mouse clicks never 
> > > worked
> > > correctly.
> > >
> > > So, yes, clients need to set cursor hotspot if they want mouse to work
> > > correctly with virtualized drivers.
> >
> > My proposal was:
> >
> > - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). Only
> > user-space which supports the hotspot props will enable it.
> > - By default, don't expose a cursor plane, because current user-space 
> > doesn't
> > support it (Weston might put a window in the cursor plane, and nobody can
> > report hotspot).
> > - If the KMS client enables the cap, advertise the cursor
> > plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
> > since the driver will pick the position.
> >
> > That way both old and new user-space are fixed.
>
> I don’t think I see how that fixes anything. In particular I don’t see a way
> of fixing the old user space at all. We require hotspot info, old user space
> doesn’t set it because there’s no way of setting it on atomic kms.

Old atomic user-space is fixed by removing the cursor plane. Then old
atomic user-space will fallback to drawing the cursor itself, e.g. via
OpenGL.

New user-space supports setting the hotspot prop, and is aware that it can't
set the cursor plane position, so the cursor plane can be exposed again when
the cap is enabled.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS



On Jun 3, 2022, at 10:56 AM, Simon Ser 
mailto:cont...@emersion.fr>> wrote:

⚠ External Email

On Friday, June 3rd, 2022 at 16:38, Zack Rusin 
mailto:za...@vmware.com>> wrote:

On Jun 3, 2022, at 10:32 AM, Simon Ser 
mailto:cont...@emersion.fr>> wrote:

⚠ External Email

On Friday, June 3rd, 2022 at 16:27, Zack Rusin 
mailto:za...@vmware.com>> wrote:

In particular: since the driver will ignore the KMS cursor plane
position set by user-space, I don't think it's okay to just expose
without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).

cc wayland-devel and Pekka for user-space feedback.

I think Thomas expressed our concerns and reasons why those wouldn’t
work for us back then. Is there something else you’d like to add?

I disagreed, and I don't understand Thomas' reasoning.

KMS clients will need an update to work correctly. Adding a new prop
without a cap leaves existing KMS clients broken. Adding a cap allows
both existing KMS clients and updated KMS clients to work correctly.

I’m not sure what you mean here. They are broken right now. That’s what we’re
fixing. That’s the reason why the virtualized drivers are on deny-lists for
all atomic kms. So nothing needs to be updated. If you have a kms client that
was using virtualized drivers with atomic kms then mouse clicks never worked
correctly.

So, yes, clients need to set cursor hotspot if they want mouse to work
correctly with virtualized drivers.

My proposal was:

- Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). Only
user-space which supports the hotspot props will enable it.
- By default, don't expose a cursor plane, because current user-space doesn't
support it (Weston might put a window in the cursor plane, and nobody can
report hotspot).
- If the KMS client enables the cap, advertise the cursor
plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
since the driver will pick the position.

That way both old and new user-space are fixed.

I don’t think I see how that fixes anything. In particular I don’t see a way of 
fixing the old user space at all. We require hotspot info, old user space 
doesn’t set it because there’s no way of setting it on atomic kms. Whether we 
expose cursor plane or not doesn’t change the fact that we still require the 
hotspot info. I don’t know… Unless the small-print in your proposal is “rewrite 
mouse cursor support in all hypervisors” (which I don’t see how you could make 
work without hotspot info) then I don’t think this solves anything, old or new.

Or did you have some magical way of extracting the hotspot data without the kms 
clients providing it? e.g. in your proposal in virtgpu_plane.c how is 
output->cursor.hot_x and output->cursor.hot_y set without a cursor plane and 
with it?

z

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

On Friday, June 3rd, 2022 at 16:38, Zack Rusin  wrote:

> > On Jun 3, 2022, at 10:32 AM, Simon Ser  wrote:
> >
> > ⚠ External Email
> >
> > On Friday, June 3rd, 2022 at 16:27, Zack Rusin  wrote:
> >
> >>> In particular: since the driver will ignore the KMS cursor plane
> >>> position set by user-space, I don't think it's okay to just expose
> >>> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> >>>
> >>> cc wayland-devel and Pekka for user-space feedback.
> >>
> >> I think Thomas expressed our concerns and reasons why those wouldn’t
> >> work for us back then. Is there something else you’d like to add?
> >
> > I disagreed, and I don't understand Thomas' reasoning.
> >
> > KMS clients will need an update to work correctly. Adding a new prop
> > without a cap leaves existing KMS clients broken. Adding a cap allows
> > both existing KMS clients and updated KMS clients to work correctly.
>
> I’m not sure what you mean here. They are broken right now. That’s what we’re
> fixing. That’s the reason why the virtualized drivers are on deny-lists for
> all atomic kms. So nothing needs to be updated. If you have a kms client that
> was using virtualized drivers with atomic kms then mouse clicks never worked
> correctly.
>
> So, yes, clients need to set cursor hotspot if they want mouse to work
> correctly with virtualized drivers.

My proposal was:

- Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). Only
  user-space which supports the hotspot props will enable it.
- By default, don't expose a cursor plane, because current user-space doesn't
  support it (Weston might put a window in the cursor plane, and nobody can
  report hotspot).
- If the KMS client enables the cap, advertise the cursor
  plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
  since the driver will pick the position.

That way both old and new user-space are fixed.

> >>> On Thursday, June 2nd, 2022 at 17:42, Zack Rusin z...@kde.org wrote:
> >>>
>  - all userspace code needs to hardcore a list of drivers which require
>  hotspots because there's no way to query from drm "does this driver
>  require hotspot"
> >>>
> >>> Can you elaborate? I'm not sure I understand what you mean here.
> >>
> >> Only some drivers require informations about cursor hotspot, user space
> >> currently has no way of figuring out which ones are those, i.e. amdgpu
> >> doesn’t care about hotspots, qxl does so when running on qxl without
> >> properly set cursor hotspot atomic kms will result in a desktop where
> >> mouse clicks have incorrect coordinates.
> >>
> >> There’s no way to differentiate between drivers that do not care about
> >> cursor hotspots and ones that absolutely require it.
> >
> > Only VM drivers should expose the hotspot properties. Real drivers like
> > amdgpu must not expose it.
>
> Yes, that’s what I said. There’s no way to differentiate between amdgpu that
> doesn’t have those properties and virtio_gpu driver from a kernel before
> hotspot properties were added.

See cap proposal above.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS



> On Jun 3, 2022, at 6:28 AM, Gerd Hoffmann  wrote:
> 
> ⚠ External Email
> 
>  Hi,
> 
>> the legacy kms to atomic. This left virtualized drivers, all which
>> are atomic, in a weird spot because all userspace compositors put
>> those drivers on deny-lists for atomic kms due to the fact that mouse
>> clicks were incorrectly routed, e.g:
> 
>> - all userspace code needs to hardcore a list of drivers which require
>> hotspots because there's no way to query from drm "does this driver
>> require hotspot"
> 
> So drivers move from one list to another.  Not ideal, but also not worse
> than it was before, so:

Yea, that problem was always there because we never had a way of checking 
whether a particular driver requires hotspot info. Unfortunately I don’t think 
this can not be solved in the kernel anymore. Technically the hotspot 
properties solve it: does the driver expose hotspot properties on cursor 
planes? If yes, then it requires hotspot info. But we can’t differentiate 
between:
- driver that doesn’t require hotspot info
- driver that requires hotspot data but we’re running on a kernel before those 
properties were implemented
We’d have to backport those properties to every kernel between now and when 
atomic kms was implemented. Same with any other check we could add to the 
kernel. Likely libdrm would be the place for this atm but I’m not sure in what 
form.

z

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS



> On Jun 3, 2022, at 10:32 AM, Simon Ser  wrote:
> 
> ⚠ External Email
> 
> On Friday, June 3rd, 2022 at 16:27, Zack Rusin  wrote:
> 
>>> In particular: since the driver will ignore the KMS cursor plane
>>> position set by user-space, I don't think it's okay to just expose
>>> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
>>> 
>>> cc wayland-devel and Pekka for user-space feedback.
>> 
>> I think Thomas expressed our concerns and reasons why those wouldn’t
>> work for us back then. Is there something else you’d like to add?
> 
> I disagreed, and I don't understand Thomas' reasoning.
> 
> KMS clients will need an update to work correctly. Adding a new prop
> without a cap leaves existing KMS clients broken. Adding a cap allows
> both existing KMS clients and updated KMS clients to work correctly.

I’m not sure what you mean here. They are broken right now. That’s what we’re 
fixing. That’s the reason why the virtualized drivers are on deny-lists for all 
atomic kms. So nothing needs to be updated. If you have a kms client that was 
using virtualized drivers with atomic kms then mouse clicks never worked 
correctly.
So, yes, clients need to set cursor hotspot if they want mouse to work 
correctly with virtualized drivers.


>>> On Thursday, June 2nd, 2022 at 17:42, Zack Rusin z...@kde.org wrote:
>>> 
 - all userspace code needs to hardcore a list of drivers which require
 hotspots because there's no way to query from drm "does this driver
 require hotspot"
>>> 
>>> Can you elaborate? I'm not sure I understand what you mean here.
>> 
>> Only some drivers require informations about cursor hotspot, user space
>> currently has no way of figuring out which ones are those, i.e. amdgpu
>> doesn’t care about hotspots, qxl does so when running on qxl without
>> properly set cursor hotspot atomic kms will result in a desktop where
>> mouse clicks have incorrect coordinates.
>> 
>> There’s no way to differentiate between drivers that do not care about
>> cursor hotspots and ones that absolutely require it.
> 
> Only VM drivers should expose the hotspot properties. Real drivers like
> amdgpu must not expose it.

Yes, that’s what I said. There’s no way to differentiate between amdgpu that 
doesn’t have those properties and virtio_gpu driver from a kernel before 
hotspot properties were added.

z

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

On Friday, June 3rd, 2022 at 16:27, Zack Rusin  wrote:

> > In particular: since the driver will ignore the KMS cursor plane
> > position set by user-space, I don't think it's okay to just expose
> > without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> >
> > cc wayland-devel and Pekka for user-space feedback.
>
> I think Thomas expressed our concerns and reasons why those wouldn’t
> work for us back then. Is there something else you’d like to add?

I disagreed, and I don't understand Thomas' reasoning.

KMS clients will need an update to work correctly. Adding a new prop
without a cap leaves existing KMS clients broken. Adding a cap allows
both existing KMS clients and updated KMS clients to work correctly.

> > On Thursday, June 2nd, 2022 at 17:42, Zack Rusin z...@kde.org wrote:
> >
> > > - all userspace code needs to hardcore a list of drivers which require
> > > hotspots because there's no way to query from drm "does this driver
> > > require hotspot"
> >
> > Can you elaborate? I'm not sure I understand what you mean here.
>
> Only some drivers require informations about cursor hotspot, user space
> currently has no way of figuring out which ones are those, i.e. amdgpu
> doesn’t care about hotspots, qxl does so when running on qxl without
> properly set cursor hotspot atomic kms will result in a desktop where
> mouse clicks have incorrect coordinates.
>
> There’s no way to differentiate between drivers that do not care about
> cursor hotspots and ones that absolutely require it.

Only VM drivers should expose the hotspot properties. Real drivers like
amdgpu must not expose it.

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

> On Jun 3, 2022, at 10:14 AM, Simon Ser  wrote:
> 
> Hi,
> 
> Please, read this thread:
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-March%2Fthread.html%23259615&data=05%7C01%7Czackr%40vmware.com%7C05141cb812004e947f0408da456b76e2%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637898625140237028%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2BZUG0OFC5SXC8%2Bm3D93AliT0VNJHbc1AEwnhVAnw8WQ%3D&reserved=0
> 
> It has a lot of information about the pitfalls of cursor hotspot and
> other things done by VM software.
> 
> In particular: since the driver will ignore the KMS cursor plane
> position set by user-space, I don't think it's okay to just expose
> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> 
> cc wayland-devel and Pekka for user-space feedback.

I think Thomas expressed our concerns and reasons why those wouldn’t work for 
us back then. Is there something else you’d like to add?

> On Thursday, June 2nd, 2022 at 17:42, Zack Rusin  wrote:
> 
>> - all userspace code needs to hardcore a list of drivers which require
>> hotspots because there's no way to query from drm "does this driver
>> require hotspot"
> 
> Can you elaborate? I'm not sure I understand what you mean here.

Only some drivers require informations about cursor hotspot, user space 
currently has no way of figuring out which ones are those, i.e. amdgpu doesn’t 
care about hotspots, qxl does so when running on qxl without properly set 
cursor hotspot atomic kms will result in a desktop where mouse clicks have 
incorrect coordinates. 
There’s no way to differentiate between drivers that do not care about cursor 
hotspots and ones that absolutely require it.

z

Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS