dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Zack Rusin <zackr@vmware.com>
To: "ppaalanen@gmail.com" <ppaalanen@gmail.com>
Cc: "hdegoede@redhat.com" <hdegoede@redhat.com>,
	"airlied@linux.ie" <airlied@linux.ie>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"gurchetansingh@chromium.org" <gurchetansingh@chromium.org>,
	Martin Krastev <krastevm@vmware.com>,
	"kraxel@redhat.com" <kraxel@redhat.com>,
	"tzimmermann@suse.de" <tzimmermann@suse.de>,
	"wayland-devel@lists.freedesktop.org"
	<wayland-devel@lists.freedesktop.org>,
	Maaz Mombasawala <mombasawalam@vmware.com>
Subject: Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
Date: Tue, 7 Jun 2022 17:50:24 +0000	[thread overview]
Message-ID: <197201d5da2c504502b8c2225d6766338b18d732.camel@vmware.com> (raw)
In-Reply-To: <20220607110707.02eccda5@eldfell>

On Tue, 2022-06-07 at 11:07 +0300, Pekka Paalanen wrote:
> On Fri, 03 Jun 2022 14:14:59 +0000
> Simon Ser <contact@emersion.fr> 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 <zack@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.
> > 
> 
> Hi Zack and everyone,
> 
> I would like to try to reboot this discussion and explain where I come
> from. Maybe I have misunderstood something.

<snip> 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 with atomic kms
what we need:
 - we need the hotspot data,
what we want to avoid:
 - fallbacks to software cursors
 - large additions to user-space

I hope this clarifies things a bit.

z

  parent reply	other threads:[~2022-06-07 17:50 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-02 15:42 [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS Zack Rusin
2022-06-02 15:42 ` [PATCH 1/6] drm/atomic: Add support for mouse hotspots Zack Rusin
2022-06-02 15:42 ` [PATCH 2/6] drm/vmwgfx: Create mouse hotspot properties on cursor planes Zack Rusin
2022-06-03 13:11   ` Martin Krastev (VMware)
2022-06-02 15:42 ` [PATCH 3/6] drm/qxl: " Zack Rusin
2022-06-02 22:05   ` kernel test robot
2022-06-02 23:26   ` kernel test robot
2022-06-02 15:42 ` [PATCH 4/6] drm/vboxvideo: " Zack Rusin
2022-06-02 15:42 ` [PATCH 5/6] drm/virtio: " Zack Rusin
2022-06-02 15:42 ` [PATCH 6/6] drm: Remove legacy cursor hotspot code Zack Rusin
2022-06-03 10:28 ` [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS Gerd Hoffmann
2022-06-03 14:43   ` Zack Rusin
2022-06-03 14:14 ` Simon Ser
2022-06-03 14:27   ` Zack Rusin
2022-06-03 14:32     ` Simon Ser
2022-06-03 14:38       ` Zack Rusin
2022-06-03 14:56         ` Simon Ser
2022-06-03 15:17           ` Zack Rusin
2022-06-03 15:22             ` Simon Ser
2022-06-03 15:32               ` Zack Rusin
2022-06-03 15:49                 ` Simon Ser
2022-06-03 18:31                   ` Zack Rusin
2022-06-05  7:30                     ` Simon Ser
2022-06-05 15:47                       ` Zack Rusin
2022-06-05 16:26                         ` Simon Ser
2022-06-05 18:16                           ` Zack Rusin
2022-06-06  8:11                             ` Simon Ser
2022-06-04 21:19               ` Hans de Goede
2022-06-05  7:34                 ` Simon Ser
2022-06-07 11:25             ` Gerd Hoffmann
2022-06-06  9:13         ` Pekka Paalanen
2022-06-07  8:07   ` Pekka Paalanen
2022-06-07 14:30     ` Gerd Hoffmann
2022-06-08  7:53       ` Pekka Paalanen
2022-06-08 14:52         ` Gerd Hoffmann
2022-06-07 17:50     ` Zack Rusin [this message]
2022-06-08  7:53       ` Pekka Paalanen
2022-06-09 19:39         ` Zack Rusin
2022-06-10  7:49           ` Pekka Paalanen
2022-06-10  8:22             ` Jonas Ådahl
2022-06-10  8:54           ` Simon Ser
2022-06-10  9:01             ` Daniel Vetter
2022-06-10  8:41   ` Daniel Vetter
2022-06-10  8:56     ` Pekka Paalanen
2022-06-10  8:59     ` Daniel Vetter
2022-06-10 12:03       ` Gerd Hoffmann
2022-06-10 14:24       ` Zack Rusin
2022-06-13  7:33         ` Pekka Paalanen
2022-06-13 13:14           ` Zack Rusin
2022-06-13 14:25             ` Pekka Paalanen
2022-06-13 14:54               ` Zack Rusin
2022-06-14  7:36                 ` Pekka Paalanen
2022-06-14 14:40                   ` Zack Rusin
2022-06-14 14:54                     ` Daniel Stone
2023-06-09 15:20       ` Albert Esteve
2023-06-21  7:10         ` Javier Martinez Canillas
2023-06-22  4:29           ` Zack Rusin
2023-06-22  6:20             ` Javier Martinez Canillas
2022-06-10  9:15     ` Simon Ser
2022-06-10  9:49       ` Daniel Vetter
2022-06-10 12:36         ` Gerd Hoffmann
2022-06-10 12:53           ` Simon Ser
2022-06-11 15:34             ` Hans de Goede
2022-06-13  7:45               ` Pekka Paalanen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=197201d5da2c504502b8c2225d6766338b18d732.camel@vmware.com \
    --to=zackr@vmware.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gurchetansingh@chromium.org \
    --cc=hdegoede@redhat.com \
    --cc=krastevm@vmware.com \
    --cc=kraxel@redhat.com \
    --cc=mombasawalam@vmware.com \
    --cc=ppaalanen@gmail.com \
    --cc=tzimmermann@suse.de \
    --cc=wayland-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).