|
Dear Stefano,
Thanks for you comments. Something to clarify:
1. (when bpp == 32, depth should be 24), I think not all places follow that rule. Change xenfb_configure_fb( ) only still has problem.
In my testing, the initialization process: fb_connect -> xenfb_configure_fb(), it sets xenfb->depth to fb_page->depth, it's 32. The graphic display after VM is started is fine.
But if we change xenfb->depth to 24, the graphic display after VM is started is corrupted.
2. About the second comment, if (xenfb->depth == bpp), buffer should be shared. Actually, in our testing, do CTRL+ALT+2, then CTRL+ALT+1, in xenfb_guest_copy( ), xenfb->depth=32, bpp=32, buffer is NOT shared. If we do not add the if(xenfb->depth == bpp ) part, the screen can not be restored.
Thanks,
Chunyan
>>> Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 2010/10/21 22:32 >>>
On Wed, 20 Oct 2010, Chun Yan Liu wrote:
> In hw/xenfb.c, xenfb_guest_copy() function doesn't handle xenfb->depth=32 case. When xenfb->depth=32, it will report error
> and won't copy data from the guest framebuffer region into QEMU's displaysurface, thus when enter CTRL+ALT+2 to qemu
> monitor console then CTRL+ALT+1 back to guest X window, the screen cannot be restored.
>
> This patch adds two things:
> 1. when xenfb->depth equals destination bpp, do not need data translation, simply copy data from guest framebuffer into
> QEMU's displaysurface.
> 2. when xenfb->depth and destination bpp differs, add processing for xenfb->depth=32 case.
>
The source of the confusion is that in the xenfb protocol depth actually
means bpp (when bpp == 32, depth should be 24).
It would be good to rename xenfb->depth to xenfb->bpp or at least add
a comment in xenfb_configure_fb to explain that depth is actually
bpp in that case.
> Signed-off by Chunyan Liu <cyliu@xxxxxxxxxx>
> ??
> diff -r e4f337bb97f7 tools/ioemu-qemu-xen/hw/xenfb.c
> --- a/hw/xenfb.c??Wed Oct 20 19:39:28 2010 +0800
> +++ b/hw/xenfb.c??Wed Oct 20 21:42:37 2010 +0800
> @@ -612,6 +612,12 @@
> ???????? uint8_t *data = "">> ??
> ???????? if (!is_buffer_shared(xenfb->c.ds->surface)) {
> +?????????????? if (xenfb->depth == bpp) {
> +?????????????????????????????? for (line = y; line < (y+h); line++) {
> +?????????????????????????????????????????????? memcpy (data + (line * linesize) + (x * bpp / 8), xenfb->pixels + xenfb->offset + (line *
> xenfb->row_stride) + (x * xenfb->depth / 8), w * xenfb->depth / 8);
> +?????????????????????????????? }
> +?????????????? }
> +?????????????? else{
This is not needed because if (xenfb->depth == bpp) the buffer should be
shared.
See xenfb_update and the call to qemu_create_displaysurface_from.
|
Home