DisplayState interface change
PATCH 1/1 DisplayState interface change
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_createDisplaySurfaceqemu_resizeDisplaySurfaceqemu_createDisplaySurfaceFromqemu_freeDisplaySurface
- console_select and qemu_console_resize in console.c this two functions manage multiple consoles on a single host display
- moving code around in hw/vga.c
as for the shared_buf patch this is necessary to be able to handle a dynamic DisplaySurface bpp
- changes to vga_draw_graphic in hw/vga.c
this is the place where the DisplaySurface buffer is shared with the videoram, when possible;
Compared to the last version the only changes are:
- do not use qemu_reallocz, use qemu_realloc instead
- do not use is_active_console, use is_graphic_console instead. is_graphic_console is an already existing function in console.c, there is no need to implement a new function that does the same thing and adds another dependency upon QEMUConsole.
---
console.c +144 -27
@@ -1044,12 +1044,15 @@if (index >=MAX_CONSOLES )return ; + active_console->g_width =ds_get_width (active_console->ds); + active_console->g_height =ds_get_height (active_console->ds); s = consoles[index];if (s) { +DisplayState *ds = s->ds; active_console = s; -if (s->console_type !=TEXT_CONSOLE && s->g_width && s->g_height - && (s->g_width !=ds_get_width (s->ds) || s->g_height !=ds_get_height (s->ds))) -dpy_resize (s->ds, s->g_width, s->g_height); + ds->surface =qemu_resizeDisplaySurface (ds->surface, s->g_width, + s->g_height,32 ,4 * s->g_width); +dpy_resize (s->ds);vga_hw_invalidate (); } }
@@ -1157,16 +1160,6 @@static void text_console_invalidate (void *opaque) {TextConsole *s = (TextConsole *) opaque; - -if (s->g_width !=ds_get_width (s->ds) || s->g_height !=ds_get_height (s->ds)) { -if (s->console_type ==TEXT_CONSOLE_FIXED_SIZE ) -dpy_resize (s->ds, s->g_width, s->g_height); -else { - s->g_width =ds_get_width (s->ds); - s->g_height =ds_get_height (s->ds); -text_console_resize (s); - } - }console_refresh (s); }
@@ -1346,13 +1339,12 @@void qemu_console_resize (QEMUConsole *console,int width,int height) { -if (console->g_width != width || console->g_height != height - || !ds_get_data (console->ds)) { - console->g_width = width; - console->g_height = height; -if (active_console == console) { -dpy_resize (console->ds, width, height); - } + console->g_width = width; + console->g_height = height; +if (active_console == console) { +DisplayState *ds = console->ds; + ds->surface =qemu_resizeDisplaySurface (ds->surface, width, height,32 ,4 * width); +dpy_resize (console->ds); } }
@@ -1360,12 +1352,137 @@ int dst_x, int dst_y, int w, int h) { if (active_console == console) { - if (console->ds->dpy_copy) - console->ds->dpy_copy(console->ds, - src_x, src_y, dst_x, dst_y, w, h); - else { -/* TODO */ - console->ds->dpy_update (console->ds, dst_x, dst_y, w, h); - } + dpy_copy(console->ds, src_x, src_y, dst_x, dst_y, w, h); } } + +static PixelFormat qemu_default_pixelformat (int bpp) +{ +PixelFormat pf; + +memset (&pf,0x00 ,sizeof (PixelFormat)); + + pf.bits_per_pixel = bpp; + pf.bytes_per_pixel = bpp /8 ; + pf.depth = bpp ==32 ?24 : bpp; + +switch (bpp) { +case 8 : + pf.rmask =0x000000E0 ; + pf.gmask =0x0000001C ; + pf.bmask =0x00000003 ; + pf.rmax =7 ; + pf.gmax =7 ; + pf.bmax =3 ; + pf.rshift =5 ; + pf.gshift =2 ; + pf.bshift =0 ; +break ; +case 16 : + pf.rmask =0x0000F800 ; + pf.gmask =0x000007E0 ; + pf.bmask =0x0000001F ; + pf.rmax =31 ; + pf.gmax =63 ; + pf.bmax =31 ; + pf.rshift =11 ; + pf.gshift =5 ; + pf.bshift =0 ; +break ; +case 24 : +case 32 : + pf.rmask =0x00FF0000 ; + pf.gmask =0x0000FF00 ; + pf.bmask =0x000000FF ; + pf.rmax =255 ; + pf.gmax =255 ; + pf.bmax =255 ; + pf.rshift =16 ; + pf.gshift =8 ; + pf.bshift =0 ; +break ; +default : +break ; + } +return pf; +} + +DisplaySurface *qemu_createDisplaySurface (int width,int height,int bpp,int linesize) +{ +DisplaySurface *surface = (DisplaySurface *)qemu_mallocz (sizeof (DisplaySurface)); +if (surface ==NULL ) { +fprintf (stderr,"qemu_createDisplaySurface: malloc failed\n" ); +exit (1 ); + } + + surface->width = width; + surface->height = height; + surface->linesize = linesize; + surface->pf =qemu_default_pixelformat (bpp); +#ifdef WORDS_BIGENDIAN + surface->flags =QEMU_ALLOCATED_FLAG |QEMU_BIG_ENDIAN_FLAG ; +#else + surface->flags =QEMU_ALLOCATED_FLAG ; +#endif + surface->data = (uint8_t *)qemu_mallocz (surface->linesize * surface->height); +if (surface->data ==NULL ) { +fprintf (stderr,"qemu_createDisplaySurface: malloc failed\n" ); +exit (1 ); + } + +return surface; +} + +DisplaySurface *qemu_resizeDisplaySurface (DisplaySurface *surface, +int width,int height,int bpp,int linesize) +{ + surface->width = width; + surface->height = height; + surface->linesize = linesize; + surface->pf =qemu_default_pixelformat (bpp); +if (surface->flags &QEMU_ALLOCATED_FLAG ) + surface->data = (uint8_t *)qemu_realloc (surface->data, surface->linesize * surface->height); +else + surface->data = (uint8_t *)qemu_malloc (surface->linesize * surface->height); +if (surface->data ==NULL ) { +fprintf (stderr,"qemu_createDisplaySurface: malloc failed\n" ); +exit (1 ); + } +#ifdef WORDS_BIGENDIAN + surface->flags =QEMU_ALLOCATED_FLAG |QEMU_BIG_ENDIAN_FLAG ; +#else + surface->flags =QEMU_ALLOCATED_FLAG ; +#endif + +return surface; +} + +DisplaySurface *qemu_createDisplaySurfaceFrom (int width,int height,int bpp, +int linesize,uint8_t *data) +{ +DisplaySurface *surface = (DisplaySurface *)qemu_mallocz (sizeof (DisplaySurface)); +if (surface ==NULL ) { +fprintf (stderr,"qemu_createDisplaySurface: malloc failed\n" ); +exit (1 ); + } + + surface->width = width; + surface->height = height; + surface->linesize = linesize; + surface->pf =qemu_default_pixelformat (bpp); +#ifdef WORDS_BIGENDIAN + surface->flags =QEMU_BIG_ENDIAN_FLAG ; +#endif + surface->data = data; + +return surface; +} + +void qemu_freeDisplaySurface (DisplaySurface *surface) +{ +if (surface ==NULL ) +return ; +if (surface->flags &QEMU_ALLOCATED_FLAG ) +qemu_free (surface->data); +qemu_free (surface); +}
console.h +118 -20
188 lines not on this page · raw message
curses.c +20 -18
@@ -97,13 +97,13 @@ } } -static void curses_resize (DisplayState *ds,int w,int h) +static void curses_resize (DisplayState *ds) { -if (w == gwidth && h == gheight) +if (ds_get_width (ds) == gwidth &&ds_get_height (ds) == gheight)return ; - gwidth = w; - gheight = h; + gwidth =ds_get_width (ds); + gheight =ds_get_height (ds);curses_calc_pad (); }
@@ -169,8 +169,8 @@clear ();refresh ();curses_calc_pad (); - ds->width =FONT_WIDTH * width; - ds->height =FONT_HEIGHT * height; + ds->surface->width =FONT_WIDTH * width; + ds->surface->height =FONT_HEIGHT * height;vga_hw_invalidate (); invalidate =0 ; }
@@ -197,8 +197,8 @@refresh ();curses_calc_pad ();curses_update (ds,0 ,0 , width, height); - ds->width =FONT_WIDTH * width; - ds->height =FONT_HEIGHT * height; + ds->surface->width =FONT_WIDTH * width; + ds->surface->height =FONT_HEIGHT * height; continue; }#endif
@@ -338,6 +338,7 @@void curses_display_init (DisplayState *ds,int full_screen) { +DisplayChangeListener *dcl;#ifndef _WIN32if (!isatty (1 )) {fprintf (stderr,"We need a terminal output\n" );
@@ -357,18 +358,19 @@#endif #endif - ds->data = (void *) screen; - ds->linesize =0 ; - ds->depth =0 ; - ds->width =640 ; - ds->height =400 ; - ds->dpy_update = curses_update; - ds->dpy_resize = curses_resize; - ds->dpy_refresh = curses_refresh; - ds->dpy_text_cursor = curses_cursor_position; + dcl = (DisplayChangeListener *)qemu_mallocz (sizeof (DisplayChangeListener)); +if (!dcl) +exit (1 ); + dcl->dpy_update = curses_update; + dcl->dpy_resize = curses_resize; + dcl->dpy_refresh = curses_refresh; + dcl->dpy_text_cursor = curses_cursor_position; +register_displaychangelistener (ds, dcl); +qemu_freeDisplaySurface (ds->surface); + ds->surface =qemu_createDisplaySurfaceFrom (80 ,25 ,0 ,0 , (uint8_t *) screen); invalidate =1 ;/* Standard VGA initial text mode dimensions */ -curses_resize (ds,80 ,25 ); +curses_resize (ds); }
hw/cirrus_vga.c +9 -15
@@ -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); -cirrus_invalidate_region (s, s->cirrus_blt_dstaddr, - s->cirrus_blt_dstpitch, s->cirrus_blt_width, - s->cirrus_blt_height); - } +cirrus_invalidate_region (s, s->cirrus_blt_dstaddr, + s->cirrus_blt_dstpitch, s->cirrus_blt_width, + s->cirrus_blt_height);return 1 ; }
hw/nseries.c +2 -1
@@ -1361,7 +1361,8 @@/* FIXME: We shouldn't really be doing this here. The LCD controller will set the size once configured, so this just sets an initial size until the guest activates the display. */ -dpy_resize (ds,800 ,480 ); + ds->surface =qemu_resizeDisplaySurface (ds->surface,800 ,480 ,32 ,4 *800 ); +dpy_resize (ds); }static struct arm_boot_info n800_binfo = {
hw/palm.c +2 -1
@@ -278,7 +278,8 @@/* FIXME: We shouldn't really be doing this here. The LCD controller will set the size once configured, so this just sets an initial size until the guest activates the display. */ -dpy_resize (ds,320 ,320 ); + ds->surface =qemu_resizeDisplaySurface (ds->surface,320 ,320 ,32 ,4 *320 ); +dpy_resize (ds); }QEMUMachine palmte_machine = {
hw/vga.c +68 -34
212 lines not on this page · raw message
hw/vga_int.h +2 -0
@@ -155,9 +155,11 @@uint32_t line_compare; \uint32_t start_addr; \uint32_t plane_updated; \ +uint32_t last_line_offset; \uint8_t last_cw, last_ch; \uint32_t last_width, last_height;/* in chars or pixels */ \uint32_t last_scr_width, last_scr_height;/* in pixels */ \ +uint32_t last_depth;/* in bits */ \uint8_t cursor_start, cursor_end; \uint32_t cursor_offset; \ unsigned int (*rgb_to_pixel)(unsigned int r, \
qemu-common.h +3 -0
@@ -146,6 +146,9 @@typedef struct AudioState AudioState ;typedef struct BlockDriverState BlockDriverState ;typedef struct DisplayState DisplayState ; +typedef struct DisplayChangeListener DisplayChangeListener ; +typedef struct DisplaySurface DisplaySurface ; +typedef struct PixelFormat PixelFormat ;typedef struct TextConsole TextConsole ;typedef TextConsole QEMUConsole ;typedef struct CharDriverState CharDriverState ;
sdl.c +49 -37
154 lines not on this page · raw message
vl.c +60 -37
172 lines not on this page · raw message
vnc.c +34 -73
222 lines not on this page · raw message
Quoted 22 lines · Stefano Stabellini
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_bufpatch 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 withconsole.cthat 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.handconsole.c
Quoted 8 lines
in particular the following functions are very helpful to manage a DisplaySurface:
qemu_createDisplaySurfaceqemu_resizeDisplaySurfaceqemu_createDisplaySurfaceFromqemu_freeDisplaySurface
These names suck. Please change them to something more in line with the rest of the code.
Quoted 18 lines
Signed-off-by Stefano Stabellini <[email protected]>---
diff -rf1fd23dcd333hw/cirrus_vga.chw/cirrus_vga.c +0
-8@@ -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 +
hw/cirrus_vga.c line 798
Quoted 10 lines
hw/cirrus_vga.c +6
-3- (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
Quoted 4 lines · Anthony Liguori
Quoted 3 lines · Stefano Stabellini
in particular the following functions are very helpful to manage a DisplaySurface:
Quoted 8 lines
Quoted 5 lines
qemu_createDisplaySurfaceqemu_resizeDisplaySurfaceqemu_createDisplaySurfaceFromqemu_freeDisplaySurfaceThese names suck. Please change them to something more in line with the rest of the code.
Are the capital letters that sucks, or the names?
qemu_create_displaysurfaceqemu_resize_displaysurfaceqemu_create_displaysurface_fromqemu_free_displaysurface
Are these any better?
Quoted 20 lines
Quoted 20 lines
diff -r
f1fd23dcd333hw/cirrus_vga.chw/cirrus_vga.c Wed Nov 26 11:43:17 2008 +0000 renamed from hw/cirrus_vga.c Tue Nov 25 12:28:31 2008 +0000 +2
-11@@ -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),
Quoted 9 lines
hw/cirrus_vga.c Wed Nov 26 11:43:17 2008 +0000 line 798
Quoted 5 lines
hw/cirrus_vga.c Wed Nov 26 11:43:17 2008 +0000 renamed from hw/cirrus_vga.c Tue Nov 25 12:28:31 2008 +0000 +4
-0+ 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_copyoptimization? This is extremely important for VNC.
My approach has been too radical.
I need to remove the memmove from vnc.c, but I should be able to keep the dpy_copy optimization.