[PATCH 3/6] hw/display/ati: Access host memory as little-endian

Philippe Mathieu-Daudé posted 6 patches 1 month, 3 weeks ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Max Filippov <jcmvbkbc@gmail.com>, Jason Wang <jasowang@redhat.com>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
There is a newer version of this series
[PATCH 3/6] hw/display/ati: Access host memory as little-endian
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
ati_2d.c is part of the ATI_VGA component, being built with
the hw/display/ati.c file. Commit 339534d4025 ("ati-vga: Fix
indexed access to video memory") made access to host memory
using little-endian order. Assume the same order is used for
the 2D component.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/display/ati_2d.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 309bb5ccb6c..72fde6b8008 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -221,7 +221,7 @@ void ati_2d_blt(ATIVGAState *s)
             for (y = 0; y < s->regs.dst_height; y++) {
                 i = dst_x * bypp + (dst_y + y) * dst_pitch;
                 for (x = 0; x < s->regs.dst_width; x++, i += bypp) {
-                    stn_he_p(&dst_bits[i], bypp, filler);
+                    stn_le_p(&dst_bits[i], bypp, filler);
                 }
             }
         }
-- 
2.52.0


Re: [PATCH 3/6] hw/display/ati: Access host memory as little-endian
Posted by BALATON Zoltan 1 month, 3 weeks ago
On Thu, 18 Dec 2025, Philippe Mathieu-Daudé wrote:
> ati_2d.c is part of the ATI_VGA component, being built with
> the hw/display/ati.c file. Commit 339534d4025 ("ati-vga: Fix
> indexed access to video memory") made access to host memory
> using little-endian order. Assume the same order is used for
> the 2D component.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/display/ati_2d.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
> index 309bb5ccb6c..72fde6b8008 100644
> --- a/hw/display/ati_2d.c
> +++ b/hw/display/ati_2d.c
> @@ -221,7 +221,7 @@ void ati_2d_blt(ATIVGAState *s)
>             for (y = 0; y < s->regs.dst_height; y++) {
>                 i = dst_x * bypp + (dst_y + y) * dst_pitch;
>                 for (x = 0; x < s->regs.dst_width; x++, i += bypp) {
> -                    stn_he_p(&dst_bits[i], bypp, filler);
> +                    stn_le_p(&dst_bits[i], bypp, filler);

I don't think this is correct as it would swap filler and thus break 
colors. This should just write the value as it is to dst_bits[i] location 
so if you want to remove stn_he_n then maybe inline it and make it a 
switch that casts to the right size and assigns the value. The stn_he_p is 
just used here as a short form instead of that switch.

Regards,
BALATON Zoltan

>                 }
>             }
>         }
>