* [Qemu-devel] [PATCH 0/2] screendump fixups @ 2012-02-23 9:22 Gerd Hoffmann 2012-02-23 9:22 ` [Qemu-devel] [PATCH 1/2] vga: simplify screendump Gerd Hoffmann 2012-02-23 9:22 ` [Qemu-devel] [PATCH 2/2] optimize screendump for the common non-switch case Gerd Hoffmann 0 siblings, 2 replies; 5+ messages in thread From: Gerd Hoffmann @ 2012-02-23 9:22 UTC (permalink / raw) To: qemu-devel; +Cc: alevy, Gerd Hoffmann Hi, After checking again I figured the displaychangelistener for screendumps isn't needed. Sorry alon. So lets just zap it. Also the console switching can be optimized further, when no switching was needed (the common case) we can just go ahead and write out the dump without a full displaysurface redraw. cheers, Gerd Gerd Hoffmann (2): vga: simplify screendump optimize screendump for the common non-switch case console.c | 10 +++++++--- console.h | 2 +- hw/qxl.c | 4 ++-- hw/vga.c | 46 +++++++--------------------------------------- hw/vmware_vga.c | 4 ++-- 5 files changed, 19 insertions(+), 47 deletions(-) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 1/2] vga: simplify screendump 2012-02-23 9:22 [Qemu-devel] [PATCH 0/2] screendump fixups Gerd Hoffmann @ 2012-02-23 9:22 ` Gerd Hoffmann 2012-02-23 10:00 ` Alon Levy 2012-02-23 9:22 ` [Qemu-devel] [PATCH 2/2] optimize screendump for the common non-switch case Gerd Hoffmann 1 sibling, 1 reply; 5+ messages in thread From: Gerd Hoffmann @ 2012-02-23 9:22 UTC (permalink / raw) To: qemu-devel; +Cc: alevy, 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vga: simplify screendump 2012-02-23 9:22 ` [Qemu-devel] [PATCH 1/2] vga: simplify screendump Gerd Hoffmann @ 2012-02-23 10:00 ` Alon Levy 0 siblings, 0 replies; 5+ messages in thread From: Alon Levy @ 2012-02-23 10:00 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel On Thu, Feb 23, 2012 at 10:22:37AM +0100, Gerd Hoffmann wrote: > 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. ACK. > --- > 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 2/2] optimize screendump for the common non-switch case 2012-02-23 9:22 [Qemu-devel] [PATCH 0/2] screendump fixups Gerd Hoffmann 2012-02-23 9:22 ` [Qemu-devel] [PATCH 1/2] vga: simplify screendump Gerd Hoffmann @ 2012-02-23 9:22 ` Gerd Hoffmann 2012-02-23 10:03 ` Alon Levy 1 sibling, 1 reply; 5+ messages in thread From: Gerd Hoffmann @ 2012-02-23 9:22 UTC (permalink / raw) To: qemu-devel; +Cc: alevy, 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/qxl.c | 4 ++-- hw/vga.c | 10 ++++++---- hw/vmware_vga.c | 4 ++-- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/console.c b/console.c index 135394f..b3bd14a 100644 --- a/console.c +++ b/console.c @@ -176,17 +176,21 @@ 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); } - 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/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/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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] optimize screendump for the common non-switch case 2012-02-23 9:22 ` [Qemu-devel] [PATCH 2/2] optimize screendump for the common non-switch case Gerd Hoffmann @ 2012-02-23 10:03 ` Alon Levy 0 siblings, 0 replies; 5+ messages in thread From: Alon Levy @ 2012-02-23 10:03 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel On Thu, Feb 23, 2012 at 10:22:38AM +0100, Gerd Hoffmann wrote: > 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. > You've missed some implementations of the vga_hw_screen_dump_ptr, see $ git grep -A3 graphic_console_init | grep screen | grep -v -w screen_dump hw/blizzard.c- blizzard_screen_dump, NULL, s); hw/g364fb.c- g364fb_screen_dump, NULL, s); hw/jazz_led.c- jazz_led_screen_dump, hw/omap_dss.c- omap_invalidate_display, omap_screen_dump, s); hw/omap_lcdc.c- omap_screen_dump, NULL, s); hw/pxa2xx_lcd.c- pxa2xx_screen_dump, NULL, s); hw/qxl.c- qxl_hw_screen_dump, qxl_hw_text_update, qxl); hw/tcx.c- tcx24_screen_dump, NULL, s); hw/tcx.c- tcx_screen_dump, NULL, s); hw/vmware_vga.c- vmsvga_screen_dump, > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > console.c | 10 +++++++--- > console.h | 2 +- > hw/qxl.c | 4 ++-- > hw/vga.c | 10 ++++++---- > hw/vmware_vga.c | 4 ++-- > 5 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/console.c b/console.c > index 135394f..b3bd14a 100644 > --- a/console.c > +++ b/console.c > @@ -176,17 +176,21 @@ 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); > } > > - 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/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/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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-02-23 10:04 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-23 9:22 [Qemu-devel] [PATCH 0/2] screendump fixups Gerd Hoffmann 2012-02-23 9:22 ` [Qemu-devel] [PATCH 1/2] vga: simplify screendump Gerd Hoffmann 2012-02-23 10:00 ` Alon Levy 2012-02-23 9:22 ` [Qemu-devel] [PATCH 2/2] optimize screendump for the common non-switch case Gerd Hoffmann 2012-02-23 10:03 ` Alon Levy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).