Skip to contentSkip to conversation

vnc and vga improvements

PATCH In review Stefano Stabellini 4 participants 7 messages Aug 28, 2008 – Sep 7, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 vnc and vga improvements

Hi all,
This is a three patch series coming from xen-unstable to improve vnc and vga performances.

The first patch implements dynamic colour depth changes in vnc.c:
this way the vnc server can change its own internal colour depth at run time to follow any guest resolution change.

The second patch implements the WMVi vnc extension in the qemu vnc server, so that we can also notify a vnc client when we change internal colour depth and offload any possible colour conversion to the client.

The third patch implements sharing of the display pixel buffer between vnc.c and vga.c, in order to save a lot of memcpy's.

The idea is that vnc.c (and in the near future sdl.c too) strictly follows the guest display resolution and notifies the client of any change. As a consequence we can save two colour conversions: one between vga and vnc, another one between the vnc server and the vnc client.

For the moment this is all, but I'll start working on the sdl shared buffer in the next days.

Cheers,

Stefano Stabellini

#1 lore raw
Quoted 13 lines · Stefano Stabellini

Hi all,
This is a three patch series coming from xen-unstable to improve vnc and vga performances.

The first patch implements dynamic colour depth changes in vnc.c:
this way the vnc server can change its own internal colour depth at run time to follow any guest resolution change.

The second patch implements the WMVi vnc extension in the qemu vnc server, so that we can also notify a vnc client when we change internal colour depth and offload any possible colour conversion to the client.

Quoted 8 lines

The third patch implements sharing of the display pixel buffer between vnc.c and vga.c, in order to save a lot of memcpy's.

The idea is that vnc.c (and in the near future sdl.c too) strictly follows the guest display resolution and notifies the client of any change. As a consequence we can save two colour conversions: one between vga and vnc, another one between the vnc server and the vnc client.

I have mixed feelings about this. On the one hand, reducing the copying is a good thing. On the other hand, we pretty much make it impossible to ever support multiple clients.

I'm going to be travelling tomorrow but I'll look through these patches in more detail over the weekend.

Regards,

Anthony Liguori

Quoted 9 lines

For the moment this is all, but I'll start working on the sdl shared buffer in the next days.

Cheers,

Stefano Stabellini

#2 lore raw
Quoted 10 lines · Anthony Liguori
Quoted 5 lines · Stefano Stabellini

The idea is that vnc.c (and in the near future sdl.c too) strictly follows the guest display resolution and notifies the client of any change. As a consequence we can save two colour conversions: one between vga and vnc, another one between the vnc server and the vnc client.

I have mixed feelings about this. On the one hand, reducing the copying is a good thing. On the other hand, we pretty much make it impossible to ever support multiple clients.

Do you mean multiple vnc clients? Or do you mean sdl and vnc at the same time?
Either way I don't think my patches prevent any of these scenarios.

Quoted 3 lines

I'm going to be travelling tomorrow but I'll look through these patches in more detail over the weekend.

Thanks.

#3 lore raw
In reply to Anthony Liguori
Quoted 23 lines · Anthony Liguori
Quoted 13 lines · Stefano Stabellini

Hi all,
This is a three patch series coming from xen-unstable to improve vnc and vga performances.

The first patch implements dynamic colour depth changes in vnc.c:
this way the vnc server can change its own internal colour depth at run time to follow any guest resolution change.

The second patch implements the WMVi vnc extension in the qemu vnc server, so that we can also notify a vnc client when we change internal colour depth and offload any possible colour conversion to the client.

Quoted 8 lines

The third patch implements sharing of the display pixel buffer between vnc.c and vga.c, in order to save a lot of memcpy's.

The idea is that vnc.c (and in the near future sdl.c too) strictly follows the guest display resolution and notifies the client of any change. As a consequence we can save two colour conversions: one between vga and vnc, another one between the vnc server and the vnc client.

Quoted 3 lines

I have mixed feelings about this. On the one hand, reducing the copying is a good thing. On the other hand, we pretty much make it impossible to ever support multiple clients.

Getting rid of the color space conversions is a good thing, even in case we can't drop the separate buffers and the memcpys. Right now the vnc server runs at 32bit unconditionally. Having it run at 16bpp in case the guest uses 16bpp is still a win because we (a) have to handle less memory and (b) can to a straight copy instead of a pixel conversion.

And maybe the buffer sharing can be implemented in a way that we can turn it on and off at runtime? So the common case of at most one client can run with buffer sharing, otherwise we copy to per-client buffers? I'm not a vnc protocol expert though.

Looking at the patches, it seems buffer sharing is optional already (usage depending on the color depth right now it seems).

cheers,
Gerd

#4 lore raw
Quoted 2 lines · Gerd Hoffmann, le Fri 29 Aug 2008 11:51:35 +0200, a écrit :

Looking at the patches, it seems buffer sharing is optional already (usage depending on the color depth right now it seems).

Yes, and because of other things, like text mode :)

Samuel

#5 lore raw
In reply to Gerd Hoffmann
Quoted 8 lines · Gerd Hoffmann

And maybe the buffer sharing can be implemented in a way that we can turn it on and off at runtime? So the common case of at most one client can run with buffer sharing, otherwise we copy to per-client buffers? I'm not a vnc protocol expert though.

Looking at the patches, it seems buffer sharing is optional already (usage depending on the color depth right now it seems).

Yes, it is optional already: every time the guest changes colour depth vga.c calls dpy_resize_shared (if implemented); dpy_resize_shared checks the new colour depth and if vnc can support it, then switches the vnc server internel colour depth to the new one and set shared_buf to 1. If the vnc server cannot support the new colour depth (for example the vnc protocol doesn't support 24 bit depth with 24 bpp, only 24 bit depth with 32 bpp) then set shared_buf to 0 and we are back to the old colour conversion code.

#6 lore raw
In reply to Gerd Hoffmann
Quoted 9 lines · Gerd Hoffmann
Quoted 6 lines · Anthony Liguori

Stefano Stabellini wrote:

I have mixed feelings about this. On the one hand, reducing the copying is a good thing. On the other hand, we pretty much make it impossible to ever support multiple clients.

Quoted 6 lines

Getting rid of the color space conversions is a good thing, even in case we can't drop the separate buffers and the memcpys. Right now the vnc server runs at 32bit unconditionally. Having it run at 16bpp in case the guest uses 16bpp is still a win because we (a) have to handle less memory and (b) can to a straight copy instead of a pixel conversion.

It's not as big of a win as it seems as there is only one client that can handle this ATM :-)

Quoted 4 lines

And maybe the buffer sharing can be implemented in a way that we can turn it on and off at runtime? So the common case of at most one client can run with buffer sharing, otherwise we copy to per-client buffers?

Yes, I would like this. I haven't yet looked closely at the patches, but if this is the case, I would be inclined to apply them.

Regards,

Anthony Liguori

Quoted 12 lines

I'm not a vnc protocol expert though.

Looking at the patches, it seems buffer sharing is optional already (usage depending on the color depth right now it seems).

cheers,
Gerd