[PATCH 6/9] hw/display/omap_lcdc: Drop broken bigendian ifdef

Peter Maydell posted 9 patches 4 years, 9 months ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Jan Kiszka <jan.kiszka@web.de>, Peter Maydell <peter.maydell@linaro.org>, Andrzej Zaborowski <balrogg@gmail.com>
[PATCH 6/9] hw/display/omap_lcdc: Drop broken bigendian ifdef
Posted by Peter Maydell 4 years, 9 months ago
The draw_line16_32() function in the omap_lcdc template header
includes an ifdef for the case where HOST_WORDS_BIGENDIAN matches
TARGET_WORDS_BIGENDIAN.  This is trying to optimise for "source
bitmap and destination bitmap format match", but it is broken,
because in this function the formats don't match: the source is
16-bit colour and the destination is 32-bit colour, so a memcpy()
will produce corrupted graphics output.  Drop the bogus ifdef.

This bug was introduced in commit ea644cf343129, when we dropped
support for DEPTH values other than 32 from the template header.
The old #if line was
  #if DEPTH == 16 && defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
and this was mistakenly changed to
  #if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
rather than deleting the #if as now having an always-false condition.

Fixes: ea644cf343129
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/omap_lcd_template.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
index c7c5025fb04..22e51d9bffb 100644
--- a/hw/display/omap_lcd_template.h
+++ b/hw/display/omap_lcd_template.h
@@ -139,9 +139,6 @@ static void draw_line12_32(void *opaque, uint8_t *d, const uint8_t *s,
 static void draw_line16_32(void *opaque, uint8_t *d, const uint8_t *s,
                            int width, int deststep)
 {
-#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
-    memcpy(d, s, width * 2);
-#else
     uint16_t v;
     uint8_t r, g, b;
 
@@ -154,5 +151,4 @@ static void draw_line16_32(void *opaque, uint8_t *d, const uint8_t *s,
         s += 2;
         d += 4;
     } while (-- width != 0);
-#endif
 }
-- 
2.20.1


Re: [PATCH 6/9] hw/display/omap_lcdc: Drop broken bigendian ifdef
Posted by Philippe Mathieu-Daudé 4 years, 9 months ago
On 2/15/21 11:32 AM, Peter Maydell wrote:
> The draw_line16_32() function in the omap_lcdc template header
> includes an ifdef for the case where HOST_WORDS_BIGENDIAN matches
> TARGET_WORDS_BIGENDIAN.  This is trying to optimise for "source
> bitmap and destination bitmap format match", but it is broken,
> because in this function the formats don't match: the source is
> 16-bit colour and the destination is 32-bit colour, so a memcpy()
> will produce corrupted graphics output.  Drop the bogus ifdef.
> 
> This bug was introduced in commit ea644cf343129, when we dropped
> support for DEPTH values other than 32 from the template header.
> The old #if line was
>   #if DEPTH == 16 && defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
> and this was mistakenly changed to
>   #if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
> rather than deleting the #if as now having an always-false condition.
> 
> Fixes: ea644cf343129
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/display/omap_lcd_template.h | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>