[Spice-devel] [RFC PATCH v2 00/20] Monitor ID rework

[Gerd Hoffmann]

On Fri, Aug 24, 2018 at 03:38:07PM +0200, Lukáš Hrázký wrote:
> On Fri, 2018-08-24 at 14:25 +0200, Gerd Hoffmann wrote:
> > On Fri, Aug 24, 2018 at 11:12:43AM +0200, Lukáš Hrázký wrote:
> > > On Thu, 2018-08-23 at 22:42 +0200, Gerd Hoffmann wrote:
> > > >   Hi,
> > > > 
> > > > > "we only support" seems to just state the use cases before adding
> > > > > vGPU but we are trying to support vGPU cases.
> > > > > If even we decide that for vGPU cards we always have monitor_id == 0
> > > > 
> > > > Yes, we want this for sure.  One channel per display.
> > > 
> > > For sure? This deserves a justification.
> > 
> > That is the way modern display architectures (including wayland) are
> > working.  One framebuffer per display.  Not one huge framebuffer
> > covering all heads, then defining rectangles for each display, like qxl
> > handles multihead on linux (with xorg).
> 
> Ok, that makes some sense. You do however need some synchronization
> mechanism between the different framebuffers? (Imagine a video playing
> across two displays)

The linux kernel kms drivers support atomic page flips for both
displays, and wayland actually uses that.

> > And the qxl way of doing multihead on linux starts to cause problems:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1611141
> 
> Tbh. the cause of the bug may not be that related (unless you
> know/suspect more that there is in the bug), but ok, I acknowledge
> that.

Well, it is.  Wayland wants a second primary surface for the second
head, which is the fundamental problem here.

There are some smaller issues too, like the kernel kms driver not
catching that error so the qxl emulation raises an error irq.  And it
seems the vdagent doesn't support wayland.

> > > Really? Can you describe how the guest could read that? Thanks.
> > 
> > It's QXLRom->id (see /usr/include/spice-1/spice/qxl_dev.h).
> 
> That's from the driver, right?

Yes.

> How to get it in the userspace?

Hmm, not sure the kernel driver exports it somehow.  Should not be that
hard to place it in some sysfs file.  That doesn't help non-qxl devices
though.

> > I'd rather try to get rid of monitor_id and just not support setups
> > with both qxl multihead (linux/xorg style) and multiple display devices
> > at the same time.
> 
> Maybe, but that's the way it works in Xorg,

Well.  It is the way the xorg qxl driver handles it.  If you use the
modesetting driver instead (which works simliar to wayland) you loose
multihead support.

> which we still need to support and the vGPU solutions are also (AFAIK)
> Xorg only atm. We could still just split Xorg's large framebuffer into
> single monitor ones, I suppose?

Whenever there is a large framebuffer in the first place depends on the
driver.

> > xrandr output id doesn't look like a clever choice to me.
> > What do you do in windows guests?
> > How do you handle wayland?
> 
> Yes, I don't know :( No clue about windows at all, I did some
> investigation about wayland and found out each compositor handles
> multi-monitor itself, which to me indicates that for multi-monitor
> under wayland we'd need to add support for each of the compositors?

Didn't look at wayland multihead in detail yet.  Compositors talk to the
kernel's kms drivers and evdev drivers, so ideally we'd get things going
with only that, without requiring qxl/spice specific tweaks in
compositors.

> If you have more insight here, I'll be very grateful.

Well, I think the mouse event handling could use a redesign.  Right now
there is a single uinput device, which gets re-created on each display
configuration change.  I'd suggest to create one uinput device per
display channel instead.  Then you can configure xorg accordingly, using
"xinput --map-to-output ... " (or X11 library calls effectively doing
the same).

Advantage #1:  You can create the uinput device once and be done with it.
Advantage #2:  You don't need to know the xrandr layout for mouse
               event routing.
Advantage #3:  I expect this is easier to make work with wayland,
               because there is physical hardware which works that way
               too (touchscreens).  But, as mentioned above, didn't
               check in detail yet.

> > Also: why handle this in the server in the first place?  Isn't it
> > sufficient if the guest agent knows which output is which channel id?
> 
> The idea here was to send an ID that the agent would understand, which
> needs to be a guest context ID, and the only such ID is the xrandr
> output ID (on xorg).

Or the agent learns to map channel ids to display output.

Keeping this in the agent has the advantage that differences between
xork/wayland/windows are easier to handle because the agent is
guest-specific anyway.

> At this moment, the agent has no idea about channel_ids,

I think this one should be solved.

So, qemu knows which channel id belongs to which device (and head, in
case devices have one channel per head like virtio).  qemu could pass
that information to spice-server, so vdagent can query it
from spice-server.

Then vdagent can scan /dev/dri/card*, query the outputs of each device
(using libdrm), lookup the device in sysfs, build a channel-id -> output
mapping from that.

> > If you must change the protocol for that (and I'm not convinced yet it
> > is actually needed) I'd try to fix this for real and cover both stream
> > display channels and classic display channels.  Otherwise it will be
> > just band-aid again and I wouldn't surprised if it breaks again rather
> > sooner than later.
> 
> But that's what we're doing, fixing it for both streaming and classic
> display...

Well, not really.  You try to fix it for streaming, and hope that the
existing logic continues to work well enough for the classic display,
right?  Does it work properly if you plug in two emulated display
devices?

cheers,
  Gerd