Hi, > The core issue stems from the fact that graphic stacks including X and a > number of other components along the way are terminally broken if the > framebuffer endianness doesn't match the guest endianness. We can > discuss the historical reasons for that fact if you wish and we can > discuss why it is almost unfixable separately.
"framebuffer" as in "linux framebuffer device" or as in "whatever memory area X11 renders into" ? > This is an obvious problem for qemu on ppc64 since we now support having > either BE or LE guests. > > At the moment, we have a broken patch in offb that makes the guest > sort-of limp along with the reversed framebuffer but that patch is > broken in several ways so I've reverted it upstream and X wouldn't work > anyway. Side note: tried "CONFIG_DRM_BOCHS=m"? In theory it should just work. I've never actually tested it in practice on bigendian. Not sure it helps in any way with the byteorder issue though. Probably not. Could help with the "how do we flip the framebuffer byteorder" though, as we can simply define a new register and let the driver set it. > This is purely about discussing the changes to vga.c and vga-templates.h > to be able to handle the swapping in a runtime-decided way rather than > compile time. > What do you prefer ? Let pixman handle it? Well, except that pixman can't handle byteswapped 16bpp formats. Too bad :( In any case cleaning up the whole mess is overdue and doing that beforehand should simplify the job alot. For quite some time qemu uses 32bpp internally no matter what, so the functions for converting the guest's framebuffer into anything but 32bpp should be dead code. And as long as the guest uses 32bpp too there is nothing converted anyway, we just setup pixman image in the correct format, backed by the guests vga memory. So this ... > +#ifdef TARGET_WORDS_BIGENDIAN > +static bool vga_is_be = true; > +#else > +static bool vga_is_be = false; > +#endif > + > +void vga_set_big_endian(bool be) > +{ > + vga_is_be = be; > +} > + > +static inline bool vga_is_big_endian(VGACommonState *s) > +{ > + return vga_is_be; > +} > + > +static inline bool vga_need_swap(VGACommonState *s, bool target_be) > +{ > +#ifdef HOST_WORDS_BIGENDIAN > + return !target_be; > +#else > + return target_be; > +#endif > +} > @@ -1645,11 +1690,9 @@ static void vga_draw_graphic(VGACommonState *s, int > full_update) > uint8_t *d; > uint32_t v, addr1, addr; > vga_draw_line_func *vga_draw_line; > -#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN) > - static const bool byteswap = false; > -#else > - static const bool byteswap = true; > -#endif > + bool byteswap; > + > + byteswap = vga_need_swap(s, vga_is_big_endian(s)); > > full_update |= update_basic_params(s); > > @@ -1691,7 +1734,8 @@ static void vga_draw_graphic(VGACommonState *s, int > full_update) > if (s->line_offset != s->last_line_offset || > disp_width != s->last_width || > height != s->last_height || > - s->last_depth != depth) { > + s->last_depth != depth || > + s->last_bswap != byteswap) { > if (depth == 32 || (depth == 16 && !byteswap)) { > surface = qemu_create_displaysurface_from(disp_width, > height, depth, s->line_offset, > @@ -1707,6 +1751,7 @@ static void vga_draw_graphic(VGACommonState *s, int > full_update) > s->last_height = height; > s->last_line_offset = s->line_offset; > s->last_depth = depth; > + s->last_bswap = byteswap; > full_update = 1; > } else if (is_buffer_shared(surface) && > (full_update || surface_data(surface) != s->vram_ptr ... is all you need to make things fly for the 32bpp case. cheers, Gerd