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-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 Daniel Vetter
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 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

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


pgpOrIrUOEj1g.pgp
Description: OpenPGP digital signature


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

2022-06-10 Thread Daniel Vetter
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 Daniel Vetter
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

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: [ANNOUNCE] Wayland 1.21.0 release schedule

2022-06-10 Thread Olivier Fourdan
Hi Simon,

On Tue, 24 May 2022 at 18:26, Simon Ser  wrote:

> On Wednesday, May 18th, 2022 at 10:39, Olivier Fourdan 
> wrote:
>
> > Maybe we could consider
> https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/188 ?
> >
> > I understand this is a huge change in the core of libwayland, but
> > that would help addressing old and real issues.
>
> I've added it to the milestone. I'm not sure we'll be able to merge it
> before the release, because it's quite large and reviewer time is
> scarce. We'll see. At any rate, even if it doesn't make it, numbers are
> cheap, I don't mind doing another release after that MR lands.
>

I understand and completely agree, it's not a small change and a pretty
risky one as well!

Cheers
Olivier


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

2022-06-10 Thread Daniel Vetter
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 Gerd Hoffmann
  Hi,

> >   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?

Depends.  In some cases the pointer position is a one-way host->guest
ticket (via tablet device).  In some cases the other direction works too
and the guest can warp the mouse pointer to another place on the host
display.  The guest can't easily figure whenever warp works or not
because that depends on host-side configuration the guest has no insight
to.

take care,
  Gerd



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

2022-06-10 Thread Gerd Hoffmann
  Hi,

> > 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.

What is the problem with HiDPI?  qemu generates standard edid blobs,
there should be no need to special-case virtualized drivers in any way.

What is the problem with multi-monitor?  That isn't much different than
physical multi-monitor either.

One little thing though:  On physical hardware you just don't know which
monitor is left and which is right until the user tells you.  In case of
a virtual multi-monitor setup we know how the two windows for the two
virtual monitors are arranged on the host and can pass that as hint to
the guest (not sure whenever *that* is the purpose of the
suggested_{x,y} properties).

> 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.

virtio-gpu got the ability to attach uuids to objects, to allow them
being identified on the host side.  So it could be that wayland-virtio
still uses kms for framebuffers (disclaimer: don't know how
wayland-virtio works in detail).  But, yes, all the scanout + cursor
handling would be out of the way, virtio-gpu would "only" handle fast
buffer sharing.

take care,
  Gerd



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 14:36, Gerd Hoffmann  wrote:

> Hi,
>
> > > 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.
>
>
> What is the problem with HiDPI? qemu generates standard edid blobs,
> there should be no need to special-case virtualized drivers in any way.
>
> What is the problem with multi-monitor? That isn't much different than
> physical multi-monitor either.
>
> One little thing though: On physical hardware you just don't know which
> monitor is left and which is right until the user tells you. In case of
> a virtual multi-monitor setup we know how the two windows for the two
> virtual monitors are arranged on the host and can pass that as hint to
> the guest (not sure whenever that is the purpose of the
> suggested_{x,y} properties).

The problem with suggested_x/y is described here:
https://lore.kernel.org/dri-devel/20220610123629.fgu2em3fto53f...@sirius.home.kraxel.org/T/#m119cfbbf736e43831c3105f0c91bd790da2d58fb

HiDPI would need a way to propagate the scale factor back-and-forth:
the VM viewer needs to advertise the preferred scale to the guest
compositor, and the guest compositor needs to indicate the scale it
renders with to the VM viewer.

Sounds familiar? Yup, that's exactly the Wayland protocol. Do we really
want to replicate the Wayland protocol in KMS? I'm not so sure.

> > 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.
>
> virtio-gpu got the ability to attach uuids to objects, to allow them
> being identified on the host side. So it could be that wayland-virtio
> still uses kms for framebuffers (disclaimer: don't know how
> wayland-virtio works in detail). But, yes, all the scanout + cursor
> handling would be out of the way, virtio-gpu would "only" handle fast
> buffer sharing.

wayland-virtio is not used with KMS. wayland-virtio proxies the Wayland
protocol between the host and the guest, so the guest doesn't use KMS
in that case.


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

2022-06-10 Thread Zack Rusin
On Fri, 2022-06-10 at 10:59 +0200, Daniel Vetter wrote:
> ⚠ External Email
> 
> 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?

Yes, that's the part I don't understand. I don't think I see how the CRTC_X|Y
properties aren't used.

I think the confusion might stem from the fact that it appears that the
hypervisors/hosts are moving that plane, which is not the case. We never move 
the
plane itself, we redirect the mouse focus/movement, that's what's reducing the
latency but don't touch drm internals. I can't speak for every virtualized 
stack,
but what is happening on ours is that we set the image that's on the cursor 
plane as
the cursor on the machine that's running the guest. So when you move the mouse
across the screen as you enter the VM window the cursor plane isn't touched, 
but the
local machines cursor changes to what's inside the cursor plane. When the mouse 
is
clicked the mouse device in the guest generates the event with the proper
coordinates (hence we need the hotspot to route that event correctly). That's 
when
the guest reacts just like it would normally on native if a mouse event was
received.

The actual cursor plane might not be visible while this is happening but even 
when
it's not visible it's still at whatever crtc_x|y the guest thinks it is. 
crtc_x|y
are still only driven by the guests mouse device.

We control the mouse device and visibility of the cursor plane itself based on
what's happening in the system but I don't think that's that different from a 
native
system.

This is easy to see by running the "weston-simple-damage --width=64 
--height=64" if
you click on that window and move the cursor to the very edge of the virtualized
window you'll notice that it's coming out outside of the window, that's because 
it's
the local machines cursor, but if you stop the press the window will jump to
wherever it was because the real guest plane is still at crtc_x|y (and because 
in
weston that window doesn't react to mouse move events like a cursor would it 
never
sets crtc_x|y to the mouse directed coordinates). 

The problem stems from the fact that the cursor plane has something that's not a
cursor and doesn't react to mouse events like a cursor would. So the flag (or 
new
plane) that we'd be adding is simply making user-space give the following 
promise:

"I understand that what's inside the cursor plane needs to react and deal with 
mouse
events like a mouse cursor would, i.e. it should be a mouse cursor"


z