From patchwork Wed Feb 22 12:26:16 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alon Levy X-Patchwork-Id: 142450 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 93B23B6F62 for ; Wed, 22 Feb 2012 23:26:49 +1100 (EST) Received: from localhost ([::1]:46714 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0BHP-0004TC-Ko for incoming@patchwork.ozlabs.org; Wed, 22 Feb 2012 07:26:43 -0500 Received: from eggs.gnu.org ([140.186.70.92]:53011) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0BHH-0004Sr-77 for qemu-devel@nongnu.org; Wed, 22 Feb 2012 07:26:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S0BHB-0002Ud-BX for qemu-devel@nongnu.org; Wed, 22 Feb 2012 07:26:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:15618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0BHB-0002UT-3P for qemu-devel@nongnu.org; Wed, 22 Feb 2012 07:26:29 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q1MCQPWD022793 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 22 Feb 2012 07:26:26 -0500 Received: from garlic.redhat.com (vpn-200-41.tlv.redhat.com [10.35.200.41]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q1MCQGm1017171 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 22 Feb 2012 07:26:20 -0500 Date: Wed, 22 Feb 2012 13:26:16 +0100 From: Alon Levy To: Gerd Hoffmann Message-ID: <20120222122616.GB607@garlic.redhat.com> Mail-Followup-To: Gerd Hoffmann , qemu-devel@nongnu.org, spice-devel@freedesktop.org, yhalperi@redhat.com, elmarco@redhat.com References: <1329860377-6284-1-git-send-email-alevy@redhat.com> <1329860377-6284-5-git-send-email-alevy@redhat.com> <4F44CD10.2040103@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4F44CD10.2040103@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: elmarco@redhat.com, yhalperi@redhat.com, qemu-devel@nongnu.org, spice-devel@freedesktop.org Subject: Re: [Qemu-devel] [RFC v4 4/9] qxl: screen_dump in vga: do a single ppm_save X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Wed, Feb 22, 2012 at 12:10:08PM +0100, Gerd Hoffmann wrote: > On 02/21/12 22:39, Alon Levy wrote: > > Using vga->screen_dump results in a number of calls to ppm_save, > > instead of a single one. > > Can you investigate why? oh, I know why. vga_screen_dump implementation: screen_dump_filename = filename; vga_invalidate_display(s); -> vga_hw_update(); screen_dump_filename = NULL; And the only user of screen_dump_filename is: 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); } } vga_save_dpy_update is called on any dpy_update, registered after the first vga_screen_dump call. Since there are a number of callers to dpy_update: vga_draw_text calls it potentially once every line, vga_draw_graphic the same. vga_draw_blank calls it once. > > > Lacking time to test all the possible users of > > vga->screen_dump, avoid the redundant calls by doing the vga_hw_update+ > > ppm_save in qxl_hw_screen_dump. > > I'd prefer to fix the root cause instead of adding workarounds. > This seems to work, only tested with -vga qxl and in vga mode: > cheers, > Gerd diff --git a/hw/vga.c b/hw/vga.c index c1029db..51f20c1 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; @@ -2389,10 +2371,12 @@ int ppm_save(const char *filename, struct DisplaySurface *ds) uint8_t r, g, b; int ret; char *linebuf, *pbuf; + static int calls; f = fopen(filename, "wb"); if (!f) return -1; + fprintf(stderr, "ppm_save %d\n", ++calls); fprintf(f, "P6\n%d %d\n%d\n", ds->width, ds->height, 255); linebuf = g_malloc(ds->width * 3); @@ -2423,29 +2407,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); }