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

[Lukáš Hrázký]

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)

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

> > > Except when the device is
> > > qxl, because qxl has a channel-id field somewhere (in qxl rom IIRC)
> > > which the guest can read.
> > 
> > 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? How to get it in the userspace?

> > > And it is not limited to vGPU.  Try place two emulated display devices
> > > into one guest (not using qxl).  You'll face the very same issue.
> > > 
> > > Placing both channel_id and monitor_id into the messages isn't going to
> > > solve this.
> > 
> > No, the way I solve this is by having a hash table on the server, which
> > maps (channel_id, monitor_id) -> guest_output_id.
> > 
> > The reason for using (channel_id, monitor_id) is that it is the only
> > unambiguous unique identifier of a monitor on the server.
> 
> 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, 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? I don't know enough about this.

> > The guest_output_id is sent to the server from the streaming agent and
> > it is the xrandr output id.
> 
> 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?

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

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

At this moment, the agent has no idea about channel_ids, even for QXL
(and for other devices I think there is not even a way for the agent to
have it).

> > Therefore we obviously have it only for
> > streamed monitors. For other, old-style monitors, we still fall back to
> > channel_id + monitor_id.
> 
> 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...

Cheers,
Lukas

> cheers,
>   Gerd
>