Re: çåï Re: [Xen-devel][PATCH]qemu-xen: let xenfb_guest_copy() handle depth=32 case

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 25 Oct 2010, Chun Yan Liu wrote:
> 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.

The fact that not all places follow the rule bpp == 32 -> depth == 24 is
the source of the problem.
In particular the name "depth" is used in place of "bpp" in many
variables and struct fields.


> Change xenfb_configure_fb( ) only still
> has problem.

I meant change the fifth argument from "depth" to "bpp" (because it is
really bpp).
Then it would make sense to change xenfb->depth to xenfb->bpp too for
the same reason.


> 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.

That would be a mistake, because xenfb->depth is actually storing bpp
not depth. Yet another example of how broken the current situation is!
The fix would not be changing xenfb->depth to 24, but renaming
xenfb->depth to xenfb->bpp.


> 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.
> ??

I think the reason for this particular bug would be that when you press
CTR+ALT+2 and CTRL+ALT+1, console_select is called, that calls
qemu_resize_displaysurface.
qemu_resize_displaysurface calls defaultallocator_resize_displaysurface
that creates another displaysurface without sharing the buffer.
The code path that allows us to have a shared buffer with xenfb is the "if
(xenfb->do_resize)" in xenfb_update but doesn't get involved in this
case.
A possible solution would be to set do_resize in xenfb_update if the
buffer is not shared and bpp == xenfb->depth.
Another solution would be to create a displayallocator for xenfb.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel