On 20 January 2014 19:34, Michael Walle <mich...@walle.cc> wrote: > In commit fc97bb5ba3e7239c0b6d24095df6784868dfebbf the lduw_raw() call was > eliminated. But we are reading from the target buffer a 16-bit value, which > is in big-endian format. Therefore, swap the bytes if we are building for a > little-endian host.
Paolo, can you remember why you included this change in that commit? It purports to just be moving the display devices around but it seems to have included the introduction of this bug, and also a removal of a lduw_raw() call from (what is now) hw/display/blizzard_template.h which I suspect is also wrong... > Cc: Paolo Bonzini <pbonz...@redhat.com> > Signed-off-by: Michael Walle <mich...@walle.cc> > --- > hw/display/milkymist-vgafb_template.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/display/milkymist-vgafb_template.h > b/hw/display/milkymist-vgafb_template.h > index e0036e1..3f25484 100644 > --- a/hw/display/milkymist-vgafb_template.h > +++ b/hw/display/milkymist-vgafb_template.h > @@ -62,6 +62,7 @@ static void glue(draw_line_, BITS)(void *opaque, uint8_t > *d, const uint8_t *s, > > while (width--) { > memcpy(&rgb565, s, sizeof(rgb565)); > + rgb565 = be16_to_cpu(rgb565); If we know the framebuffer is always bigendian (regardless of the target CPU endianness) then rather than memcpy and then byteswap we might as well just rgb565 = lduw_be_p(s); I think. > r = ((rgb565 >> 11) & 0x1f) << 3; > g = ((rgb565 >> 5) & 0x3f) << 2; > b = ((rgb565 >> 0) & 0x1f) << 3; thanks -- PMM