Re: [Qemu-devel] [PATCH] sm501: Adjust endianness of pixel value in rectangle fill

[BALATON Zoltan]

On Wed, 19 Sep 2018, Marcus Comstedt wrote:
> The value from twoD_foreground (which is in host endian format) must
> be converted to the endianness of the framebuffer (currently always
> little endian) before it can be used to perform the fill operation.
>
> Signed-off-by: Marcus Comstedt <address@hidden>

Reviewed-by: BALATON Zoltan <address@hidden>

But also cc-ing Peter Maydell who reviewed endianness fixes before for 
this device model in case he has something to add. Eventually I plan to 
rewrite this to make it more optimised and maybe use pixman routines for 
these but likely I can't get to that until later (maybe in next dev 
cycle) so until then this fix would help.

Thanks for testing on big endian host and fixing this bug. Have you also 
tried KVM? That would be interesting but I think it may have problems 
currently and maybe only PR KVM would work as it need to emulate PPC440 
on server CPU but I don't know much about KVM.

Regards,
BALATON Zoltan

> ---
>
> Hi.
>
> I noticed when running AmigaOS 4.1 as a guest that the rectangle fill
> function in SM501 does not work correctly if the host is big endian
> (tested on a Talos II).  This can be observed easily by starting a
> shell in 16-bit mode, where the background turns purple instead of gray.
>
> After applying this patch rendering was ok on both big and little
> endian hosts.
>
>
>  // Marcus
>
>
> hw/display/sm501.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> index 874260a143..4a8686f0f5 100644
> --- a/hw/display/sm501.c
> +++ b/hw/display/sm501.c
> @@ -39,6 +39,7 @@
> #include "hw/i2c/i2c-ddc.h"
> #include "qemu/range.h"
> #include "ui/pixel_ops.h"
> +#include "qemu/bswap.h"
>
> /*
>  * Status: 2010/05/07
> @@ -812,9 +813,11 @@ static void sm501_2d_operation(SM501State *s)
>             FILL_RECT(1, uint8_t);
>             break;
>         case 1:
> +            color = cpu_to_le16(color);
>             FILL_RECT(2, uint16_t);
>             break;
>         case 2:
> +            color = cpu_to_le32(color);
>             FILL_RECT(4, uint32_t);
>             break;
>         }