[Qemu-devel] [PATCH v2 0/3] screendump fixups

[Qemu-devel] [PATCH v2 0/3] screendump fixups

[Gerd Hoffmann]

  Hi,

Next round, this time covering all screen dump implementations.

cheers,
  Gerd

Gerd Hoffmann (3):
  vga: simplify screendump
  Remove screendump dummy functions.
  optimize screendump for the common non-switch case

 console.c       |   12 +++++++++---
 console.h       |    2 +-
 hw/blizzard.c   |    8 ++++++--
 hw/g364fb.c     |    2 +-
 hw/jazz_led.c   |    7 +------
 hw/omap_lcdc.c  |    7 +++++--
 hw/pxa2xx_lcd.c |    7 +------
 hw/qxl.c        |    4 ++--
 hw/tcx.c        |    8 ++++----
 hw/vga.c        |   46 +++++++---------------------------------------
 hw/vmware_vga.c |    4 ++--
 11 files changed, 39 insertions(+), 68 deletions(-)

[Qemu-devel] [PATCH v2 1/3] vga: simplify screendump

[Gerd Hoffmann]

The displaychangelistener isn't needed at all, we can simply save the
image when vga_hw_update is done instead of hooking into the update
process.
---
 hw/vga.c |   36 +-----------------------------------
 1 files changed, 1 insertions(+), 35 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index c1029db..f8f30f8 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -163,8 +163,6 @@ static uint16_t expand2[256];
 static uint8_t expand4to8[16];
 
 static void vga_screen_dump(void *opaque, const char *filename);
-static const char *screen_dump_filename;
-static DisplayChangeListener *screen_dump_dcl;
 
 static void vga_update_memory_access(VGACommonState *s)
 {
@@ -2364,22 +2362,6 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion *system_memory)
 /********************************************************/
 /* vga screen dump */
 
-static void vga_save_dpy_update(DisplayState *ds,
-                                int x, int y, int w, int h)
-{
-    if (screen_dump_filename) {
-        ppm_save(screen_dump_filename, ds->surface);
-    }
-}
-
-static void vga_save_dpy_resize(DisplayState *s)
-{
-}
-
-static void vga_save_dpy_refresh(DisplayState *s)
-{
-}
-
 int ppm_save(const char *filename, struct DisplaySurface *ds)
 {
     FILE *f;
@@ -2423,29 +2405,13 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
     return 0;
 }
 
-static DisplayChangeListener* vga_screen_dump_init(DisplayState *ds)
-{
-    DisplayChangeListener *dcl;
-
-    dcl = g_malloc0(sizeof(DisplayChangeListener));
-    dcl->dpy_update = vga_save_dpy_update;
-    dcl->dpy_resize = vga_save_dpy_resize;
-    dcl->dpy_refresh = vga_save_dpy_refresh;
-    register_displaychangelistener(ds, dcl);
-    return dcl;
-}
-
 /* save the vga display in a PPM image even if no display is
    available */
 static void vga_screen_dump(void *opaque, const char *filename)
 {
     VGACommonState *s = opaque;
 
-    if (!screen_dump_dcl)
-        screen_dump_dcl = vga_screen_dump_init(s->ds);
-
-    screen_dump_filename = filename;
     vga_invalidate_display(s);
     vga_hw_update();
-    screen_dump_filename = NULL;
+    ppm_save(filename, s->ds->surface);
 }
-- 
1.7.1

[Qemu-devel] [PATCH v2 2/3] Remove screendump dummy functions.

[Gerd Hoffmann]

The code in console.c verifies whenever a screen_dump function
pointer is present before calling it, so there is no need to supply an
dummy function.  Remove them.  Also report an error to notify the user
that he didn't got a screenshot.
---
 console.c       |    2 ++
 hw/jazz_led.c   |    7 +------
 hw/pxa2xx_lcd.c |    7 +------
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/console.c b/console.c
index 135394f..153424f 100644
--- a/console.c
+++ b/console.c
@@ -184,6 +184,8 @@ void vga_hw_screen_dump(const char *filename)
     console_select(0);
     if (consoles[0] && consoles[0]->hw_screen_dump) {
         consoles[0]->hw_screen_dump(consoles[0]->hw, filename);
+    } else {
+        error_report("screen dump not implemented");
     }
 
     if (previous_active_console) {
diff --git a/hw/jazz_led.c b/hw/jazz_led.c
index 5d8040b..6486523 100644
--- a/hw/jazz_led.c
+++ b/hw/jazz_led.c
@@ -205,11 +205,6 @@ static void jazz_led_invalidate_display(void *opaque)
     s->state |= REDRAW_SEGMENTS | REDRAW_BACKGROUND;
 }
 
-static void jazz_led_screen_dump(void *opaque, const char *filename)
-{
-    printf("jazz_led_screen_dump() not implemented\n");
-}
-
 static void jazz_led_text_update(void *opaque, console_ch_t *chardata)
 {
     LedState *s = opaque;
@@ -255,7 +250,7 @@ static int jazz_led_init(SysBusDevice *dev)
 
     s->ds = graphic_console_init(jazz_led_update_display,
                                  jazz_led_invalidate_display,
-                                 jazz_led_screen_dump,
+                                 NULL,
                                  jazz_led_text_update, s);
 
     return 0;
diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c
index 9495226..fcbdfb3 100644
--- a/hw/pxa2xx_lcd.c
+++ b/hw/pxa2xx_lcd.c
@@ -899,11 +899,6 @@ static void pxa2xx_invalidate_display(void *opaque)
     s->invalidated = 1;
 }
 
-static void pxa2xx_screen_dump(void *opaque, const char *filename)
-{
-    /* TODO */
-}
-
 static void pxa2xx_lcdc_orientation(void *opaque, int angle)
 {
     PXA2xxLCDState *s = (PXA2xxLCDState *) opaque;
@@ -1009,7 +1004,7 @@ PXA2xxLCDState *pxa2xx_lcdc_init(MemoryRegion *sysmem,
 
     s->ds = graphic_console_init(pxa2xx_update_display,
                                  pxa2xx_invalidate_display,
-                                 pxa2xx_screen_dump, NULL, s);
+                                 NULL, NULL, s);
 
     switch (ds_get_bits_per_pixel(s->ds)) {
     case 0:
-- 
1.7.1

[Qemu-devel] [PATCH v2 3/3] optimize screendump for the common non-switch case

[Gerd Hoffmann]

switch console only if needed, also pass down whenever the console was
switched or not because a displaysurface redraw is only needed in case
the console was switched.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 console.c       |   10 +++++++---
 console.h       |    2 +-
 hw/blizzard.c   |    8 ++++++--
 hw/g364fb.c     |    2 +-
 hw/omap_lcdc.c  |    7 +++++--
 hw/qxl.c        |    4 ++--
 hw/tcx.c        |    8 ++++----
 hw/vga.c        |   10 ++++++----
 hw/vmware_vga.c |    4 ++--
 9 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/console.c b/console.c
index 153424f..da3dd07 100644
--- a/console.c
+++ b/console.c
@@ -176,19 +176,23 @@ void vga_hw_invalidate(void)
 void vga_hw_screen_dump(const char *filename)
 {
     TextConsole *previous_active_console;
+    bool cswitch;
 
     previous_active_console = active_console;
+    cswitch = previous_active_console && previous_active_console->index != 0;
 
     /* There is currently no way of specifying which screen we want to dump,
        so always dump the first one.  */
-    console_select(0);
+    if (cswitch) {
+        console_select(0);
+    }
     if (consoles[0] && consoles[0]->hw_screen_dump) {
-        consoles[0]->hw_screen_dump(consoles[0]->hw, filename);
+        consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch);
     } else {
         error_report("screen dump not implemented");
     }
 
-    if (previous_active_console) {
+    if (cswitch) {
         console_select(previous_active_console->index);
     }
 }
diff --git a/console.h b/console.h
index 6ba0d5d..1fc8ea3 100644
--- a/console.h
+++ b/console.h
@@ -340,7 +340,7 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
 
 typedef void (*vga_hw_update_ptr)(void *);
 typedef void (*vga_hw_invalidate_ptr)(void *);
-typedef void (*vga_hw_screen_dump_ptr)(void *, const char *);
+typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch);
 typedef void (*vga_hw_text_update_ptr)(void *, console_ch_t *);
 
 DisplayState *graphic_console_init(vga_hw_update_ptr update,
diff --git a/hw/blizzard.c b/hw/blizzard.c
index b2c1b22..c7d844d 100644
--- a/hw/blizzard.c
+++ b/hw/blizzard.c
@@ -932,10 +932,14 @@ static void blizzard_update_display(void *opaque)
     s->my[1] = 0;
 }
 
-static void blizzard_screen_dump(void *opaque, const char *filename) {
+static void blizzard_screen_dump(void *opaque, const char *filename,
+                                 bool cswitch)
+{
     BlizzardState *s = (BlizzardState *) opaque;
 
-    blizzard_update_display(opaque);
+    if (cswitch) {
+        blizzard_update_display(opaque);
+    }
     if (s && ds_get_data(s->state))
         ppm_save(filename, s->state->surface);
 }
diff --git a/hw/g364fb.c b/hw/g364fb.c
index 9c63bdd..3a0b68f 100644
--- a/hw/g364fb.c
+++ b/hw/g364fb.c
@@ -289,7 +289,7 @@ static void g364fb_reset(G364State *s)
     g364fb_invalidate_display(s);
 }
 
-static void g364fb_screen_dump(void *opaque, const char *filename)
+static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch)
 {
     G364State *s = opaque;
     int y, x;
diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
index f265306..f172093 100644
--- a/hw/omap_lcdc.c
+++ b/hw/omap_lcdc.c
@@ -264,9 +264,12 @@ static int ppm_save(const char *filename, uint8_t *data,
     return 0;
 }
 
-static void omap_screen_dump(void *opaque, const char *filename) {
+static void omap_screen_dump(void *opaque, const char *filename, bool cswitch)
+{
     struct omap_lcd_panel_s *omap_lcd = opaque;
-    omap_update_display(opaque);
+    if (cswitch) {
+        omap_update_display(opaque);
+    }
     if (omap_lcd && ds_get_data(omap_lcd->state))
         ppm_save(filename, ds_get_data(omap_lcd->state),
                 omap_lcd->width, omap_lcd->height,
diff --git a/hw/qxl.c b/hw/qxl.c
index e38bb29..f4c68ab 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1438,7 +1438,7 @@ static void qxl_hw_invalidate(void *opaque)
     vga->invalidate(vga);
 }
 
-static void qxl_hw_screen_dump(void *opaque, const char *filename)
+static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
 {
     PCIQXLDevice *qxl = opaque;
     VGACommonState *vga = &qxl->vga;
@@ -1450,7 +1450,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename)
         ppm_save(filename, qxl->ssd.ds->surface);
         break;
     case QXL_MODE_VGA:
-        vga->screen_dump(vga, filename);
+        vga->screen_dump(vga, filename, cswitch);
         break;
     default:
         break;
diff --git a/hw/tcx.c b/hw/tcx.c
index 2b66d86..ac7dcb4 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -56,8 +56,8 @@ typedef struct TCXState {
     uint8_t dac_index, dac_state;
 } TCXState;
 
-static void tcx_screen_dump(void *opaque, const char *filename);
-static void tcx24_screen_dump(void *opaque, const char *filename);
+static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch);
+static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch);
 
 static void tcx_set_dirty(TCXState *s)
 {
@@ -574,7 +574,7 @@ static int tcx_init1(SysBusDevice *dev)
     return 0;
 }
 
-static void tcx_screen_dump(void *opaque, const char *filename)
+static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
 {
     TCXState *s = opaque;
     FILE *f;
@@ -601,7 +601,7 @@ static void tcx_screen_dump(void *opaque, const char *filename)
     return;
 }
 
-static void tcx24_screen_dump(void *opaque, const char *filename)
+static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch)
 {
     TCXState *s = opaque;
     FILE *f;
diff --git a/hw/vga.c b/hw/vga.c
index f8f30f8..5994f43 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -162,7 +162,7 @@ static uint32_t expand4[256];
 static uint16_t expand2[256];
 static uint8_t expand4to8[16];
 
-static void vga_screen_dump(void *opaque, const char *filename);
+static void vga_screen_dump(void *opaque, const char *filename, bool cswitch);
 
 static void vga_update_memory_access(VGACommonState *s)
 {
@@ -2407,11 +2407,13 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
 
 /* save the vga display in a PPM image even if no display is
    available */
-static void vga_screen_dump(void *opaque, const char *filename)
+static void vga_screen_dump(void *opaque, const char *filename, bool cswitch)
 {
     VGACommonState *s = opaque;
 
-    vga_invalidate_display(s);
-    vga_hw_update();
+    if (cswitch) {
+        vga_invalidate_display(s);
+        vga_hw_update();
+    }
     ppm_save(filename, s->ds->surface);
 }
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index f8afa3c..142d9f4 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1003,11 +1003,11 @@ static void vmsvga_invalidate_display(void *opaque)
 
 /* save the vga display in a PPM image even if no display is
    available */
-static void vmsvga_screen_dump(void *opaque, const char *filename)
+static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch)
 {
     struct vmsvga_state_s *s = opaque;
     if (!s->enable) {
-        s->vga.screen_dump(&s->vga, filename);
+        s->vga.screen_dump(&s->vga, filename, cswitch);
         return;
     }
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH v2 0/3] screendump fixups

[Alon Levy]

On Fri, Feb 24, 2012 at 12:43:42PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> Next round, this time covering all screen dump implementations.
> 

Reviewed-by: Alon Levy <alevy@redhat.com>

Also compiled-by :)

> cheers,
>   Gerd
> 
> Gerd Hoffmann (3):
>   vga: simplify screendump
>   Remove screendump dummy functions.
>   optimize screendump for the common non-switch case
> 
>  console.c       |   12 +++++++++---
>  console.h       |    2 +-
>  hw/blizzard.c   |    8 ++++++--
>  hw/g364fb.c     |    2 +-
>  hw/jazz_led.c   |    7 +------
>  hw/omap_lcdc.c  |    7 +++++--
>  hw/pxa2xx_lcd.c |    7 +------
>  hw/qxl.c        |    4 ++--
>  hw/tcx.c        |    8 ++++----
>  hw/vga.c        |   46 +++++++---------------------------------------
>  hw/vmware_vga.c |    4 ++--
>  11 files changed, 39 insertions(+), 68 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 0/3] screendump fixups

[Anthony Liguori]

On 02/24/2012 05:43 AM, Gerd Hoffmann wrote:
>    Hi,
>
> Next round, this time covering all screen dump implementations.

Applied.  THanks.

Regards,

Anthony Liguori

>
> cheers,
>    Gerd
>
> Gerd Hoffmann (3):
>    vga: simplify screendump
>    Remove screendump dummy functions.
>    optimize screendump for the common non-switch case
>
>   console.c       |   12 +++++++++---
>   console.h       |    2 +-
>   hw/blizzard.c   |    8 ++++++--
>   hw/g364fb.c     |    2 +-
>   hw/jazz_led.c   |    7 +------
>   hw/omap_lcdc.c  |    7 +++++--
>   hw/pxa2xx_lcd.c |    7 +------
>   hw/qxl.c        |    4 ++--
>   hw/tcx.c        |    8 ++++----
>   hw/vga.c        |   46 +++++++---------------------------------------
>   hw/vmware_vga.c |    4 ++--
>   11 files changed, 39 insertions(+), 68 deletions(-)
>
>
>