Re: [Qemu-devel] [PATCH 2/2] qxl: add QXL_IO_UPDATE_MEM for guest S3&S4 support

[Alon Levy]

On Sun, Jun 26, 2011 at 12:59:06PM -0400, Yonit Halperin wrote:
> Sorry for the late response, wasn't available.
> I'm afraid  that (1) and (2) will indeed wakeup the worker, but will not 
> assure emptying the command ring, as it depends on the client pipe size.
> 

I actually can't figure out what wakeup does (that's what both NOTIFY and
NOTIFY_CURSOR do, see hw/qxl.c). What we did in prepare_to_sleep before was
call flush_all_qxl_commands, which reads the command and cursor rings until
they are empty (calling flush_cursor_commands and flush_display_commands), 
waiting
whenever the pipe is too large. (avoiding this wait still needs to be done, but
after ensuring suspend is correct).

More to the point this flush is done from handle_dev_destroy_surfaces, but
this is not good enough since this also destroys the surfaces, before we have
a chance to actually render the surfaces.

Perhaps RED_WORKER_MESSAGE_STOP should do a flush_all_qxl_commands?

> 
> ----- Original Message -----
> From: "Alon Levy" <address@hidden>
> To: "Gerd Hoffmann" <address@hidden>
> Cc: address@hidden, address@hidden
> Sent: Wednesday, June 22, 2011 11:57:54 AM
> Subject: Re: [Qemu-devel] [PATCH 2/2] qxl: add QXL_IO_UPDATE_MEM for guest 
> S3&S4 support
> 
> On Wed, Jun 22, 2011 at 11:13:19AM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > >>worker call.  We can add a I/O command to ask qxl to push the
> > >>release queue head to the release ring.
> > >
> > >So you suggest to replace QXL_IO_UPDATE_MEM with what, two io commands 
> > >instead
> > >of using the val parameter?
> > 
> > I'd like to (a) avoid updating the libspice-server API if possible
> > and (b) have one I/O command for each logical step.  So going into
> > S3 could look like this:
> > 
> >   (0) stop putting new commands into the rings
> >   (1) QXL_IO_NOTIFY_CMD
> >   (2) QXL_IO_NOTIFY_CURSOR
> >       qxl calls notify(), to make the worker thread empty the command
> >       rings before it processes the next dispatcher request.
> >   (3) QXl_IO_FLUSH_SURFACES (to be implemented)
> >       qxl calls stop()+start(), spice-server renders all surfaces,
> >       thereby flushing state to device memory.
> >   (4) QXL_IO_DESTROY_ALL_SURFACES
> >       zap surfaces
> >   (5) QXL_IO_FLUSH_RELEASE (to be implemented)
> >       push release queue head into the release ring, so the guest
> >       will see it and can release everything.
> > 
> > (1)+(2)+(4) exist already.
> > (3)+(5) can be done without libspice-server changes.
> > 
> > Looks workable?
> 
> yeah. Yonit?
> 
> > 
> > cheers,
> >   Gerd
> > 
> > 
>