From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54748) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RzXAM-0006UW-Q0 for qemu-devel@nongnu.org; Mon, 20 Feb 2012 12:36:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RzXAL-0001Fa-20 for qemu-devel@nongnu.org; Mon, 20 Feb 2012 12:36:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56872) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RzXAK-0001FW-PM for qemu-devel@nongnu.org; Mon, 20 Feb 2012 12:36:45 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q1KHahoq014007 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 20 Feb 2012 12:36:43 -0500 Date: Mon, 20 Feb 2012 19:36:39 +0200 From: Alon Levy Message-ID: <20120220173639.GJ23926@garlic.redhat.com> References: <1329686886-6853-1-git-send-email-alevy@redhat.com> <1329686886-6853-8-git-send-email-alevy@redhat.com> <4F423155.1010706@redhat.com> <20120220123823.GG23926@garlic.redhat.com> <4F42481B.3070605@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F42481B.3070605@redhat.com> Subject: Re: [Qemu-devel] [RFC 7/7] qxl: add allocator List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org, elmarco@redhat.com On Mon, Feb 20, 2012 at 02:18:19PM +0100, Gerd Hoffmann wrote: > Hi, > > > I'll send a series that works with vnc+spice and sdl+spice (didn't test > > sdl+vnc+spice), and screendumps at the same time. > > > >> > >> Setting the QEMU_ALLOCATED_FLAG flag sounds hackish too. > > QEMU_ALLOCATED_FLAG is just a flag which is used by the > defaultallocator_free_displaysurface function to figure whenever the > displaysurface memory was allocated by qemu_alloc_display or not. > > I think vga.c shouldn't look at it in the first place, and faking it > because vga.c looks at it is even worse. > > I'm also not sure it is the right approach to to have qxl register a > display allocator in the first place. The only other place doing this > is sdl, which is a user interface. None of the gfx card emulations in > the tree do that ... TBH I'm having problems without doing it with my own Allocator. In particular calling ppm_save from spice_server thread seems to be a problem. I can remove the hackish use of QEMU_ALLOCATED_FLAG by not checking for it in vga.c in the first place, but I don't know what other code it affects (well, I do - every machine using vga, but I'm not testing all of these). Also reallocating the displaysurface seems to be the wrong thing, and that's what we do whenever we check in qxl_render_update is_shared_buffer. > > > Actually I think the right thing is to move/copy the 24bit->32bit convertion > > from vga.c to pflib.c, what do you think? > > Agree, although that easily gets a patch series of its own when you > collect optimized format conversion functions to move them over to pflib ... Yes, I think I'll defer that to later. > > BTW: qxl insisting on a shared displaysurface isn't very clean too, it > better should be able to fallback to just copying/converting for the > non-shared case. I'll try to get that working, although it seems to require having some timer/bh to do the ppm_save. > > cheers, > Gerd > >