Re: [Qemu-devel] [PATCH 3 of 7] [UPDATE] DisplayState interface change

[Anthony Liguori <anthony@codemonkey.ws>]

Stefano Stabellini wrote:
> This patch changes the DisplayState interface adding support for
> multiple frontends at the same time (sdl and vnc) and implements most
> of the benefit of the shared_buf patch without the added complexity.
>
> Currently DisplayState is managed by sdl (or vnc) and sdl (or vnc) is
> also responsible for allocating the data and setting the depth.
> Vga.c (or another backend) will do any necessary conversion.
>
> The idea is to change it so that is vga.c (or another backend) together
> with console.c that fully manage the DisplayState interface allocating
> data and setting the depth (either 16 or 32 bit, if the guest uses a
> different resolution or is in text mode, vga.c (or another backend) is
> in charge of doing the conversion seamlessly).
>
> The other idea is that DisplayState supports *multiple* frontends
> like sdl and vnc; each of them can register some callbacks to be called
> when a display event occurs.
>
> The interesting changes are:
>
> - the new structures and related functions in console.h and console.c
>
> in particular the following functions are very helpful to manage a
> DisplaySurface:
>
> qemu_createDisplaySurface
> qemu_resizeDisplaySurface
> qemu_createDisplaySurfaceFrom
> qemu_freeDisplaySurface
>   

These names suck.  Please change them to something more in line with the 
rest of the code.

> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> ---
> diff -r f1fd23dcd333 hw/cirrus_vga.c
> --- a/hw/cirrus_vga.c	Tue Nov 25 12:28:31 2008 +0000
> +++ b/hw/cirrus_vga.c	Wed Nov 26 11:43:17 2008 +0000
> @@ -789,22 +789,16 @@
>      if (BLTUNSAFE(s))
>          return 0;
>  
> -    if (s->ds->dpy_copy) {
> -	cirrus_do_copy(s, s->cirrus_blt_dstaddr - s->start_addr,
> -		       s->cirrus_blt_srcaddr - s->start_addr,
> -		       s->cirrus_blt_width, s->cirrus_blt_height);
> -    } else {
> -	(*s->cirrus_rop) (s, s->vram_ptr +
> -                (s->cirrus_blt_dstaddr & s->cirrus_addr_mask),
> -			  s->vram_ptr +
> -                (s->cirrus_blt_srcaddr & s->cirrus_addr_mask),
> -			  s->cirrus_blt_dstpitch, s->cirrus_blt_srcpitch,
> -			  s->cirrus_blt_width, s->cirrus_blt_height);
> +    (*s->cirrus_rop) (s, s->vram_ptr +
> +            (s->cirrus_blt_dstaddr & s->cirrus_addr_mask),
> +            s->vram_ptr +
> +            (s->cirrus_blt_srcaddr & s->cirrus_addr_mask),
> +            s->cirrus_blt_dstpitch, s->cirrus_blt_srcpitch,
> +            s->cirrus_blt_width, s->cirrus_blt_height);
>   

You've eliminated the dpy_copy optimization?  This is extremely 
important for VNC.

Regards,

Anthony Liguori