Re: [Qemu-devel] [PULL v4 05/12] milkymist-vgafb: swap pixel data in source buffer

[Peter Maydell]

On 20 January 2014 19:34, Michael Walle <address@hidden> 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 <address@hidden>
> Signed-off-by: Michael Walle <address@hidden>
> ---
>  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