[PATCH 0/9] arm: drop dead code for non-32-bit-RGB surfaces

[PATCH 0/9] arm: drop dead code for non-32-bit-RGB surfaces

[Peter Maydell]

This patchset removes dead code (including a couple of 'template'
headers) for handling UI surfaces that are formats other than
32-bit RGB for the musicpal, tc6393xb and omap_lcdc display devices.

For a long time now the UI layer has guaranteed that the console
surface is always 32 bits per pixel RGB, so the legacy code in these
devices which was handling the possibility that the console surface
was some other format can all be deleted.

Notes:
 (1) the patch "hw/display/omap_lcdc: Drop broken bigendian ifdef"
     fixes a bug introduced in commit ea644cf343129 in 2016 in a
     previous partial start on this cleanup
 (2) the omap_lcdc changes are tested only with 'make check' as I
     don't have any test images for the OMAP1.

With these plus the other on-list patches, the only remaining
'template' header files in hw/display are pl110 (which needs it to
handle the different guest-side pixel formats) and the
milkymist-vgafb device.  Milkymist is in the 'deprecated and will be
deleted in a few releases' bucket so it's not worth doing this
cleanup for it, since it's not blocking any API transition.
(I'll take this task off the BiteSizedTasks list shortly.)

After the musicpal cleanup and the tcx patch I just posted, there
will be no more users of is_surface_bgr(), so that could then be
deleted.

thanks
-- PMM

Peter Maydell (9):
  hw/arm/musicpal: Remove dead code for non-32-bit-RGB surfaces
  hw/display/tc6393xb: Remove dead code for handling non-32bpp surfaces
  hw/display/tc6393xb: Expand out macros in template header
  hw/display/tc6393xb: Inline tc6393xb_draw_graphic32() at its callsite
  hw/display/omap_lcdc: Expand out macros in template header
  hw/display/omap_lcdc: Drop broken bigendian ifdef
  hw/display/omap_lcdc: Fix coding style issues in template header
  hw/display/omap_lcdc: Inline template header into C file
  hw/display/omap_lcdc: Delete unnecessary macro

 hw/display/omap_lcd_template.h | 169 ---------------------------------
 hw/display/tc6393xb_template.h |  72 --------------
 include/ui/console.h           |  10 --
 hw/arm/musicpal.c              |  64 +++++--------
 hw/display/omap_lcdc.c         | 129 ++++++++++++++++++++++++-
 hw/display/tc6393xb.c          |  48 ++++------
 6 files changed, 165 insertions(+), 327 deletions(-)
 delete mode 100644 hw/display/omap_lcd_template.h
 delete mode 100644 hw/display/tc6393xb_template.h

-- 
2.20.1

[PATCH 1/9] hw/arm/musicpal: Remove dead code for non-32-bit-RGB surfaces

[Peter Maydell]

For a long time now the UI layer has guaranteed that the console
surface is always 32 bits per pixel RGB. Remove the legacy dead
code from the milkymist display device which was handling the
possibility that the console surface was some other format.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/musicpal.c | 64 ++++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 40 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 6aec84aeed8..9cebece2de0 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -512,53 +512,37 @@ static uint8_t scale_lcd_color(musicpal_lcd_state *s, uint8_t col)
     }
 }
 
-#define SET_LCD_PIXEL(depth, type) \
-static inline void glue(set_lcd_pixel, depth) \
-        (musicpal_lcd_state *s, int x, int y, type col) \
-{ \
-    int dx, dy; \
-    DisplaySurface *surface = qemu_console_surface(s->con); \
-    type *pixel = &((type *) surface_data(surface))[(y * 128 * 3 + x) * 3]; \
-\
-    for (dy = 0; dy < 3; dy++, pixel += 127 * 3) \
-        for (dx = 0; dx < 3; dx++, pixel++) \
-            *pixel = col; \
+static inline void set_lcd_pixel32(musicpal_lcd_state *s,
+                                   int x, int y, uint32_t col)
+{
+    int dx, dy;
+    DisplaySurface *surface = qemu_console_surface(s->con);
+    uint32_t *pixel =
+        &((uint32_t *) surface_data(surface))[(y * 128 * 3 + x) * 3];
+
+    for (dy = 0; dy < 3; dy++, pixel += 127 * 3) {
+        for (dx = 0; dx < 3; dx++, pixel++) {
+            *pixel = col;
+        }
+    }
 }
-SET_LCD_PIXEL(8, uint8_t)
-SET_LCD_PIXEL(16, uint16_t)
-SET_LCD_PIXEL(32, uint32_t)
 
 static void lcd_refresh(void *opaque)
 {
     musicpal_lcd_state *s = opaque;
-    DisplaySurface *surface = qemu_console_surface(s->con);
     int x, y, col;
 
-    switch (surface_bits_per_pixel(surface)) {
-    case 0:
-        return;
-#define LCD_REFRESH(depth, func) \
-    case depth: \
-        col = func(scale_lcd_color(s, (MP_LCD_TEXTCOLOR >> 16) & 0xff), \
-                   scale_lcd_color(s, (MP_LCD_TEXTCOLOR >> 8) & 0xff), \
-                   scale_lcd_color(s, MP_LCD_TEXTCOLOR & 0xff)); \
-        for (x = 0; x < 128; x++) { \
-            for (y = 0; y < 64; y++) { \
-                if (s->video_ram[x + (y/8)*128] & (1 << (y % 8))) { \
-                    glue(set_lcd_pixel, depth)(s, x, y, col); \
-                } else { \
-                    glue(set_lcd_pixel, depth)(s, x, y, 0); \
-                } \
-            } \
-        } \
-        break;
-    LCD_REFRESH(8, rgb_to_pixel8)
-    LCD_REFRESH(16, rgb_to_pixel16)
-    LCD_REFRESH(32, (is_surface_bgr(surface) ?
-                     rgb_to_pixel32bgr : rgb_to_pixel32))
-    default:
-        hw_error("unsupported colour depth %i\n",
-                 surface_bits_per_pixel(surface));
+    col = rgb_to_pixel32(scale_lcd_color(s, (MP_LCD_TEXTCOLOR >> 16) & 0xff),
+                         scale_lcd_color(s, (MP_LCD_TEXTCOLOR >> 8) & 0xff),
+                         scale_lcd_color(s, MP_LCD_TEXTCOLOR & 0xff));
+    for (x = 0; x < 128; x++) {
+        for (y = 0; y < 64; y++) {
+            if (s->video_ram[x + (y / 8) * 128] & (1 << (y % 8))) {
+                set_lcd_pixel32(s, x, y, col);
+            } else {
+                set_lcd_pixel32(s, x, y, 0);
+            }
+        }
     }
 
     dpy_gfx_update(s->con, 0, 0, 128*3, 64*3);
-- 
2.20.1

[PATCH 2/9] hw/display/tc6393xb: Remove dead code for handling non-32bpp surfaces

[Peter Maydell]

For a long time now the UI layer has guaranteed that the console
surface is always 32 bits per pixel RGB. Remove the legacy dead
code from the tc6393xb display device which was handling the
possibility that the console surface was some other format.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/ui/console.h  | 10 ----------
 hw/display/tc6393xb.c | 33 +--------------------------------
 2 files changed, 1 insertion(+), 42 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index d30e972d0b5..ed086f9f1ad 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -266,16 +266,6 @@ PixelFormat qemu_default_pixelformat(int bpp);
 DisplaySurface *qemu_create_displaysurface(int width, int height);
 void qemu_free_displaysurface(DisplaySurface *surface);
 
-static inline int is_surface_bgr(DisplaySurface *surface)
-{
-    if (PIXMAN_FORMAT_BPP(surface->format) == 32 &&
-        PIXMAN_FORMAT_TYPE(surface->format) == PIXMAN_TYPE_ABGR) {
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
 static inline int is_buffer_shared(DisplaySurface *surface)
 {
     return !(surface->flags & QEMU_ALLOCATED_FLAG);
diff --git a/hw/display/tc6393xb.c b/hw/display/tc6393xb.c
index 49a676d1b0e..4cddb1a99ad 100644
--- a/hw/display/tc6393xb.c
+++ b/hw/display/tc6393xb.c
@@ -410,43 +410,12 @@ static void tc6393xb_nand_writeb(TC6393xbState *s, hwaddr addr, uint32_t value)
                                         (uint32_t) addr, value & 0xff);
 }
 
-#define BITS 8
-#include "tc6393xb_template.h"
-#define BITS 15
-#include "tc6393xb_template.h"
-#define BITS 16
-#include "tc6393xb_template.h"
-#define BITS 24
-#include "tc6393xb_template.h"
 #define BITS 32
 #include "tc6393xb_template.h"
 
 static void tc6393xb_draw_graphic(TC6393xbState *s, int full_update)
 {
-    DisplaySurface *surface = qemu_console_surface(s->con);
-
-    switch (surface_bits_per_pixel(surface)) {
-        case 8:
-            tc6393xb_draw_graphic8(s);
-            break;
-        case 15:
-            tc6393xb_draw_graphic15(s);
-            break;
-        case 16:
-            tc6393xb_draw_graphic16(s);
-            break;
-        case 24:
-            tc6393xb_draw_graphic24(s);
-            break;
-        case 32:
-            tc6393xb_draw_graphic32(s);
-            break;
-        default:
-            printf("tc6393xb: unknown depth %d\n",
-                   surface_bits_per_pixel(surface));
-            return;
-    }
-
+    tc6393xb_draw_graphic32(s);
     dpy_gfx_update_full(s->con);
 }
 
-- 
2.20.1

[PATCH 3/9] hw/display/tc6393xb: Expand out macros in template header

[Peter Maydell]

Now the template header is included only for BITS==32, expand
out all the macros that depended on the BITS setting.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/tc6393xb_template.h | 35 ++++------------------------------
 1 file changed, 4 insertions(+), 31 deletions(-)

diff --git a/hw/display/tc6393xb_template.h b/hw/display/tc6393xb_template.h
index 78629c07f97..7789ffc4399 100644
--- a/hw/display/tc6393xb_template.h
+++ b/hw/display/tc6393xb_template.h
@@ -21,25 +21,7 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#if BITS == 8
-# define SET_PIXEL(addr, color)  (*(uint8_t *)addr = color)
-#elif BITS == 15 || BITS == 16
-# define SET_PIXEL(addr, color)  (*(uint16_t *)addr = color)
-#elif BITS == 24
-# define SET_PIXEL(addr, color)  \
-    do {                         \
-        addr[0] = color;         \
-        addr[1] = (color) >> 8;  \
-        addr[2] = (color) >> 16; \
-    } while (0)
-#elif BITS == 32
-# define SET_PIXEL(addr, color)  (*(uint32_t *)addr = color)
-#else
-# error unknown bit depth
-#endif
-
-
-static void glue(tc6393xb_draw_graphic, BITS)(TC6393xbState *s)
+static void tc6393xb_draw_graphic32(TC6393xbState *s)
 {
     DisplaySurface *surface = qemu_console_surface(s->con);
     int i;
@@ -49,24 +31,15 @@ static void glue(tc6393xb_draw_graphic, BITS)(TC6393xbState *s)
     data_buffer = s->vram_ptr;
     data_display = surface_data(surface);
     for(i = 0; i < s->scr_height; i++) {
-#if (BITS == 16)
-        memcpy(data_display, data_buffer, s->scr_width * 2);
-        data_buffer += s->scr_width;
-        data_display += surface_stride(surface);
-#else
         int j;
-        for (j = 0; j < s->scr_width; j++, data_display += BITS / 8, data_buffer++) {
+        for (j = 0; j < s->scr_width; j++, data_display += 4, data_buffer++) {
             uint16_t color = *data_buffer;
-            uint32_t dest_color = glue(rgb_to_pixel, BITS)(
+            uint32_t dest_color = rgb_to_pixel32(
                            ((color & 0xf800) * 0x108) >> 11,
                            ((color & 0x7e0) * 0x41) >> 9,
                            ((color & 0x1f) * 0x21) >> 2
                            );
-            SET_PIXEL(data_display, dest_color);
+            *(uint32_t *)data_display = dest_color;
         }
-#endif
     }
 }
-
-#undef BITS
-#undef SET_PIXEL
-- 
2.20.1

[PATCH 4/9] hw/display/tc6393xb: Inline tc6393xb_draw_graphic32() at its callsite

[Peter Maydell]

The function tc6393xb_draw_graphic32() is called in exactly one place,
so just inline the function body at its callsite. This allows us to
drop the template header entirely.

The code move includes a single added space after 'for' to fix
the coding style.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/tc6393xb_template.h | 45 ----------------------------------
 hw/display/tc6393xb.c          | 23 ++++++++++++++---
 2 files changed, 19 insertions(+), 49 deletions(-)
 delete mode 100644 hw/display/tc6393xb_template.h

diff --git a/hw/display/tc6393xb_template.h b/hw/display/tc6393xb_template.h
deleted file mode 100644
index 7789ffc4399..00000000000
--- a/hw/display/tc6393xb_template.h
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * Toshiba TC6393XB I/O Controller.
- * Found in Sharp Zaurus SL-6000 (tosa) or some
- * Toshiba e-Series PDAs.
- *
- * FB support code. Based on G364 fb emulator
- *
- * Copyright (c) 2007 Hervé Poussineau
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, see <http://www.gnu.org/licenses/>.
- */
-
-static void tc6393xb_draw_graphic32(TC6393xbState *s)
-{
-    DisplaySurface *surface = qemu_console_surface(s->con);
-    int i;
-    uint16_t *data_buffer;
-    uint8_t *data_display;
-
-    data_buffer = s->vram_ptr;
-    data_display = surface_data(surface);
-    for(i = 0; i < s->scr_height; i++) {
-        int j;
-        for (j = 0; j < s->scr_width; j++, data_display += 4, data_buffer++) {
-            uint16_t color = *data_buffer;
-            uint32_t dest_color = rgb_to_pixel32(
-                           ((color & 0xf800) * 0x108) >> 11,
-                           ((color & 0x7e0) * 0x41) >> 9,
-                           ((color & 0x1f) * 0x21) >> 2
-                           );
-            *(uint32_t *)data_display = dest_color;
-        }
-    }
-}
diff --git a/hw/display/tc6393xb.c b/hw/display/tc6393xb.c
index 4cddb1a99ad..1f28223c7be 100644
--- a/hw/display/tc6393xb.c
+++ b/hw/display/tc6393xb.c
@@ -410,12 +410,27 @@ static void tc6393xb_nand_writeb(TC6393xbState *s, hwaddr addr, uint32_t value)
                                         (uint32_t) addr, value & 0xff);
 }
 
-#define BITS 32
-#include "tc6393xb_template.h"
-
 static void tc6393xb_draw_graphic(TC6393xbState *s, int full_update)
 {
-    tc6393xb_draw_graphic32(s);
+    DisplaySurface *surface = qemu_console_surface(s->con);
+    int i;
+    uint16_t *data_buffer;
+    uint8_t *data_display;
+
+    data_buffer = s->vram_ptr;
+    data_display = surface_data(surface);
+    for (i = 0; i < s->scr_height; i++) {
+        int j;
+        for (j = 0; j < s->scr_width; j++, data_display += 4, data_buffer++) {
+            uint16_t color = *data_buffer;
+            uint32_t dest_color = rgb_to_pixel32(
+                           ((color & 0xf800) * 0x108) >> 11,
+                           ((color & 0x7e0) * 0x41) >> 9,
+                           ((color & 0x1f) * 0x21) >> 2
+                           );
+            *(uint32_t *)data_display = dest_color;
+        }
+    }
     dpy_gfx_update_full(s->con);
 }
 
-- 
2.20.1

[PATCH 5/9] hw/display/omap_lcdc: Expand out macros in template header

[Peter Maydell]

The omap_lcdc template header is already only included once, for
DEPTH==32, but it still has all the macro-driven parameterization
for other depths. Expand out all the macros in the header.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/omap_lcd_template.h | 67 ++++++++++++++--------------------
 1 file changed, 28 insertions(+), 39 deletions(-)

diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
index 1025ff3825d..c7c5025fb04 100644
--- a/hw/display/omap_lcd_template.h
+++ b/hw/display/omap_lcd_template.h
@@ -27,18 +27,11 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#if DEPTH == 32
-# define BPP 4
-# define PIXEL_TYPE uint32_t
-#else
-# error unsupport depth
-#endif
-
 /*
  * 2-bit colour
  */
-static void glue(draw_line2_, DEPTH)(void *opaque,
-                uint8_t *d, const uint8_t *s, int width, int deststep)
+static void draw_line2_32(void *opaque, uint8_t *d, const uint8_t *s,
+                          int width, int deststep)
 {
     uint16_t *pal = opaque;
     uint8_t v, r, g, b;
@@ -48,26 +41,26 @@ static void glue(draw_line2_, DEPTH)(void *opaque,
         r = (pal[v & 3] >> 4) & 0xf0;
         g = pal[v & 3] & 0xf0;
         b = (pal[v & 3] << 4) & 0xf0;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
-        d += BPP;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
         v >>= 2;
         r = (pal[v & 3] >> 4) & 0xf0;
         g = pal[v & 3] & 0xf0;
         b = (pal[v & 3] << 4) & 0xf0;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
-        d += BPP;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
         v >>= 2;
         r = (pal[v & 3] >> 4) & 0xf0;
         g = pal[v & 3] & 0xf0;
         b = (pal[v & 3] << 4) & 0xf0;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
-        d += BPP;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
         v >>= 2;
         r = (pal[v & 3] >> 4) & 0xf0;
         g = pal[v & 3] & 0xf0;
         b = (pal[v & 3] << 4) & 0xf0;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
-        d += BPP;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
         s ++;
         width -= 4;
     } while (width > 0);
@@ -76,8 +69,8 @@ static void glue(draw_line2_, DEPTH)(void *opaque,
 /*
  * 4-bit colour
  */
-static void glue(draw_line4_, DEPTH)(void *opaque,
-                uint8_t *d, const uint8_t *s, int width, int deststep)
+static void draw_line4_32(void *opaque, uint8_t *d, const uint8_t *s,
+                          int width, int deststep)
 {
     uint16_t *pal = opaque;
     uint8_t v, r, g, b;
@@ -87,14 +80,14 @@ static void glue(draw_line4_, DEPTH)(void *opaque,
         r = (pal[v & 0xf] >> 4) & 0xf0;
         g = pal[v & 0xf] & 0xf0;
         b = (pal[v & 0xf] << 4) & 0xf0;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
-        d += BPP;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
         v >>= 4;
         r = (pal[v & 0xf] >> 4) & 0xf0;
         g = pal[v & 0xf] & 0xf0;
         b = (pal[v & 0xf] << 4) & 0xf0;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
-        d += BPP;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
         s ++;
         width -= 2;
     } while (width > 0);
@@ -103,8 +96,8 @@ static void glue(draw_line4_, DEPTH)(void *opaque,
 /*
  * 8-bit colour
  */
-static void glue(draw_line8_, DEPTH)(void *opaque,
-                uint8_t *d, const uint8_t *s, int width, int deststep)
+static void draw_line8_32(void *opaque, uint8_t *d, const uint8_t *s,
+                          int width, int deststep)
 {
     uint16_t *pal = opaque;
     uint8_t v, r, g, b;
@@ -114,17 +107,17 @@ static void glue(draw_line8_, DEPTH)(void *opaque,
         r = (pal[v] >> 4) & 0xf0;
         g = pal[v] & 0xf0;
         b = (pal[v] << 4) & 0xf0;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
         s ++;
-        d += BPP;
+        d += 4;
     } while (-- width != 0);
 }
 
 /*
  * 12-bit colour
  */
-static void glue(draw_line12_, DEPTH)(void *opaque,
-                uint8_t *d, const uint8_t *s, int width, int deststep)
+static void draw_line12_32(void *opaque, uint8_t *d, const uint8_t *s,
+                           int width, int deststep)
 {
     uint16_t v;
     uint8_t r, g, b;
@@ -134,17 +127,17 @@ static void glue(draw_line12_, DEPTH)(void *opaque,
         r = (v >> 4) & 0xf0;
         g = v & 0xf0;
         b = (v << 4) & 0xf0;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
         s += 2;
-        d += BPP;
+        d += 4;
     } while (-- width != 0);
 }
 
 /*
  * 16-bit colour
  */
-static void glue(draw_line16_, DEPTH)(void *opaque,
-                uint8_t *d, const uint8_t *s, int width, int deststep)
+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);
@@ -157,13 +150,9 @@ static void glue(draw_line16_, DEPTH)(void *opaque,
         r = (v >> 8) & 0xf8;
         g = (v >> 3) & 0xfc;
         b = (v << 3) & 0xf8;
-        ((PIXEL_TYPE *) d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
         s += 2;
-        d += BPP;
+        d += 4;
     } while (-- width != 0);
 #endif
 }
-
-#undef DEPTH
-#undef BPP
-#undef PIXEL_TYPE
-- 
2.20.1

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

[Peter Maydell]

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

[PATCH 7/9] hw/display/omap_lcdc: Fix coding style issues in template header

[Peter Maydell]

Fix some minor coding style issues in the template header,
so checkpatch doesn't complain when we move the code.

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

diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
index 22e51d9bffb..a2f86eee3c8 100644
--- a/hw/display/omap_lcd_template.h
+++ b/hw/display/omap_lcd_template.h
@@ -61,7 +61,7 @@ static void draw_line2_32(void *opaque, uint8_t *d, const uint8_t *s,
         b = (pal[v & 3] << 4) & 0xf0;
         ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
         d += 4;
-        s ++;
+        s++;
         width -= 4;
     } while (width > 0);
 }
@@ -88,7 +88,7 @@ static void draw_line4_32(void *opaque, uint8_t *d, const uint8_t *s,
         b = (pal[v & 0xf] << 4) & 0xf0;
         ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
         d += 4;
-        s ++;
+        s++;
         width -= 2;
     } while (width > 0);
 }
@@ -108,7 +108,7 @@ static void draw_line8_32(void *opaque, uint8_t *d, const uint8_t *s,
         g = pal[v] & 0xf0;
         b = (pal[v] << 4) & 0xf0;
         ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        s ++;
+        s++;
         d += 4;
     } while (-- width != 0);
 }
-- 
2.20.1

[PATCH 8/9] hw/display/omap_lcdc: Inline template header into C file

[Peter Maydell]

We only include the template header once, so just inline it into the
source file for the device.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/omap_lcd_template.h | 154 ---------------------------------
 hw/display/omap_lcdc.c         | 127 ++++++++++++++++++++++++++-
 2 files changed, 125 insertions(+), 156 deletions(-)
 delete mode 100644 hw/display/omap_lcd_template.h

diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
deleted file mode 100644
index a2f86eee3c8..00000000000
--- a/hw/display/omap_lcd_template.h
+++ /dev/null
@@ -1,154 +0,0 @@
-/*
- * QEMU OMAP LCD Emulator templates
- *
- * Copyright (c) 2006 Andrzej Zaborowski  <balrog@zabor.org>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in
- *    the documentation and/or other materials provided with the
- *    distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
- * PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-/*
- * 2-bit colour
- */
-static void draw_line2_32(void *opaque, uint8_t *d, const uint8_t *s,
-                          int width, int deststep)
-{
-    uint16_t *pal = opaque;
-    uint8_t v, r, g, b;
-
-    do {
-        v = ldub_p((void *) s);
-        r = (pal[v & 3] >> 4) & 0xf0;
-        g = pal[v & 3] & 0xf0;
-        b = (pal[v & 3] << 4) & 0xf0;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        d += 4;
-        v >>= 2;
-        r = (pal[v & 3] >> 4) & 0xf0;
-        g = pal[v & 3] & 0xf0;
-        b = (pal[v & 3] << 4) & 0xf0;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        d += 4;
-        v >>= 2;
-        r = (pal[v & 3] >> 4) & 0xf0;
-        g = pal[v & 3] & 0xf0;
-        b = (pal[v & 3] << 4) & 0xf0;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        d += 4;
-        v >>= 2;
-        r = (pal[v & 3] >> 4) & 0xf0;
-        g = pal[v & 3] & 0xf0;
-        b = (pal[v & 3] << 4) & 0xf0;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        d += 4;
-        s++;
-        width -= 4;
-    } while (width > 0);
-}
-
-/*
- * 4-bit colour
- */
-static void draw_line4_32(void *opaque, uint8_t *d, const uint8_t *s,
-                          int width, int deststep)
-{
-    uint16_t *pal = opaque;
-    uint8_t v, r, g, b;
-
-    do {
-        v = ldub_p((void *) s);
-        r = (pal[v & 0xf] >> 4) & 0xf0;
-        g = pal[v & 0xf] & 0xf0;
-        b = (pal[v & 0xf] << 4) & 0xf0;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        d += 4;
-        v >>= 4;
-        r = (pal[v & 0xf] >> 4) & 0xf0;
-        g = pal[v & 0xf] & 0xf0;
-        b = (pal[v & 0xf] << 4) & 0xf0;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        d += 4;
-        s++;
-        width -= 2;
-    } while (width > 0);
-}
-
-/*
- * 8-bit colour
- */
-static void draw_line8_32(void *opaque, uint8_t *d, const uint8_t *s,
-                          int width, int deststep)
-{
-    uint16_t *pal = opaque;
-    uint8_t v, r, g, b;
-
-    do {
-        v = ldub_p((void *) s);
-        r = (pal[v] >> 4) & 0xf0;
-        g = pal[v] & 0xf0;
-        b = (pal[v] << 4) & 0xf0;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        s++;
-        d += 4;
-    } while (-- width != 0);
-}
-
-/*
- * 12-bit colour
- */
-static void draw_line12_32(void *opaque, uint8_t *d, const uint8_t *s,
-                           int width, int deststep)
-{
-    uint16_t v;
-    uint8_t r, g, b;
-
-    do {
-        v = lduw_le_p((void *) s);
-        r = (v >> 4) & 0xf0;
-        g = v & 0xf0;
-        b = (v << 4) & 0xf0;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        s += 2;
-        d += 4;
-    } while (-- width != 0);
-}
-
-/*
- * 16-bit colour
- */
-static void draw_line16_32(void *opaque, uint8_t *d, const uint8_t *s,
-                           int width, int deststep)
-{
-    uint16_t v;
-    uint8_t r, g, b;
-
-    do {
-        v = lduw_le_p((void *) s);
-        r = (v >> 8) & 0xf8;
-        g = (v >> 3) & 0xfc;
-        b = (v << 3) & 0xf8;
-        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
-        s += 2;
-        d += 4;
-    } while (-- width != 0);
-}
diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c
index 58e659c94ff..2db04fad2fc 100644
--- a/hw/display/omap_lcdc.c
+++ b/hw/display/omap_lcdc.c
@@ -72,8 +72,131 @@ static void omap_lcd_interrupts(struct omap_lcd_panel_s *s)
 
 #define draw_line_func drawfn
 
-#define DEPTH 32
-#include "omap_lcd_template.h"
+/*
+ * 2-bit colour
+ */
+static void draw_line2_32(void *opaque, uint8_t *d, const uint8_t *s,
+                          int width, int deststep)
+{
+    uint16_t *pal = opaque;
+    uint8_t v, r, g, b;
+
+    do {
+        v = ldub_p((void *) s);
+        r = (pal[v & 3] >> 4) & 0xf0;
+        g = pal[v & 3] & 0xf0;
+        b = (pal[v & 3] << 4) & 0xf0;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
+        v >>= 2;
+        r = (pal[v & 3] >> 4) & 0xf0;
+        g = pal[v & 3] & 0xf0;
+        b = (pal[v & 3] << 4) & 0xf0;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
+        v >>= 2;
+        r = (pal[v & 3] >> 4) & 0xf0;
+        g = pal[v & 3] & 0xf0;
+        b = (pal[v & 3] << 4) & 0xf0;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
+        v >>= 2;
+        r = (pal[v & 3] >> 4) & 0xf0;
+        g = pal[v & 3] & 0xf0;
+        b = (pal[v & 3] << 4) & 0xf0;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
+        s++;
+        width -= 4;
+    } while (width > 0);
+}
+
+/*
+ * 4-bit colour
+ */
+static void draw_line4_32(void *opaque, uint8_t *d, const uint8_t *s,
+                          int width, int deststep)
+{
+    uint16_t *pal = opaque;
+    uint8_t v, r, g, b;
+
+    do {
+        v = ldub_p((void *) s);
+        r = (pal[v & 0xf] >> 4) & 0xf0;
+        g = pal[v & 0xf] & 0xf0;
+        b = (pal[v & 0xf] << 4) & 0xf0;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
+        v >>= 4;
+        r = (pal[v & 0xf] >> 4) & 0xf0;
+        g = pal[v & 0xf] & 0xf0;
+        b = (pal[v & 0xf] << 4) & 0xf0;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        d += 4;
+        s++;
+        width -= 2;
+    } while (width > 0);
+}
+
+/*
+ * 8-bit colour
+ */
+static void draw_line8_32(void *opaque, uint8_t *d, const uint8_t *s,
+                          int width, int deststep)
+{
+    uint16_t *pal = opaque;
+    uint8_t v, r, g, b;
+
+    do {
+        v = ldub_p((void *) s);
+        r = (pal[v] >> 4) & 0xf0;
+        g = pal[v] & 0xf0;
+        b = (pal[v] << 4) & 0xf0;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        s++;
+        d += 4;
+    } while (-- width != 0);
+}
+
+/*
+ * 12-bit colour
+ */
+static void draw_line12_32(void *opaque, uint8_t *d, const uint8_t *s,
+                           int width, int deststep)
+{
+    uint16_t v;
+    uint8_t r, g, b;
+
+    do {
+        v = lduw_le_p((void *) s);
+        r = (v >> 4) & 0xf0;
+        g = v & 0xf0;
+        b = (v << 4) & 0xf0;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        s += 2;
+        d += 4;
+    } while (-- width != 0);
+}
+
+/*
+ * 16-bit colour
+ */
+static void draw_line16_32(void *opaque, uint8_t *d, const uint8_t *s,
+                           int width, int deststep)
+{
+    uint16_t v;
+    uint8_t r, g, b;
+
+    do {
+        v = lduw_le_p((void *) s);
+        r = (v >> 8) & 0xf8;
+        g = (v >> 3) & 0xfc;
+        b = (v << 3) & 0xf8;
+        ((uint32_t *) d)[0] = rgb_to_pixel32(r, g, b);
+        s += 2;
+        d += 4;
+    } while (-- width != 0);
+}
 
 static void omap_update_display(void *opaque)
 {
-- 
2.20.1

[PATCH 9/9] hw/display/omap_lcdc: Delete unnecessary macro

[Peter Maydell]

The macro draw_line_func is used only once; just expand it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/omap_lcdc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c
index 2db04fad2fc..0ba42ef637c 100644
--- a/hw/display/omap_lcdc.c
+++ b/hw/display/omap_lcdc.c
@@ -70,8 +70,6 @@ static void omap_lcd_interrupts(struct omap_lcd_panel_s *s)
     qemu_irq_lower(s->irq);
 }
 
-#define draw_line_func drawfn
-
 /*
  * 2-bit colour
  */
@@ -202,7 +200,7 @@ static void omap_update_display(void *opaque)
 {
     struct omap_lcd_panel_s *omap_lcd = (struct omap_lcd_panel_s *) opaque;
     DisplaySurface *surface;
-    draw_line_func draw_line;
+    drawfn draw_line;
     int size, height, first, last;
     int width, linesize, step, bpp, frame_offset;
     hwaddr frame_base;
-- 
2.20.1

Re: [PATCH 4/9] hw/display/tc6393xb: Inline tc6393xb_draw_graphic32() at its callsite

[Philippe Mathieu-Daudé]

On 2/15/21 11:32 AM, Peter Maydell wrote:
> The function tc6393xb_draw_graphic32() is called in exactly one place,
> so just inline the function body at its callsite. This allows us to
> drop the template header entirely.
> 
> The code move includes a single added space after 'for' to fix
> the coding style.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/display/tc6393xb_template.h | 45 ----------------------------------
>  hw/display/tc6393xb.c          | 23 ++++++++++++++---
>  2 files changed, 19 insertions(+), 49 deletions(-)
>  delete mode 100644 hw/display/tc6393xb_template.h

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

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

[Philippe Mathieu-Daudé]

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>

Re: [PATCH 7/9] hw/display/omap_lcdc: Fix coding style issues in template header

[Philippe Mathieu-Daudé]

On 2/15/21 11:32 AM, Peter Maydell wrote:
> Fix some minor coding style issues in the template header,
> so checkpatch doesn't complain when we move the code.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/display/omap_lcd_template.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

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

Re: [PATCH 8/9] hw/display/omap_lcdc: Inline template header into C file

[Philippe Mathieu-Daudé]

On 2/15/21 11:32 AM, Peter Maydell wrote:
> We only include the template header once, so just inline it into the
> source file for the device.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/display/omap_lcd_template.h | 154 ---------------------------------
>  hw/display/omap_lcdc.c         | 127 ++++++++++++++++++++++++++-
>  2 files changed, 125 insertions(+), 156 deletions(-)
>  delete mode 100644 hw/display/omap_lcd_template.h

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

Re: [PATCH 9/9] hw/display/omap_lcdc: Delete unnecessary macro

[Philippe Mathieu-Daudé]

On 2/15/21 11:32 AM, Peter Maydell wrote:
> The macro draw_line_func is used only once; just expand it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/display/omap_lcdc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

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

Re: [PATCH 0/9] arm: drop dead code for non-32-bit-RGB surfaces

[Richard Henderson]

On 2/15/21 2:32 AM, Peter Maydell wrote:
> Peter Maydell (9):
>   hw/arm/musicpal: Remove dead code for non-32-bit-RGB surfaces
>   hw/display/tc6393xb: Remove dead code for handling non-32bpp surfaces
>   hw/display/tc6393xb: Expand out macros in template header
>   hw/display/tc6393xb: Inline tc6393xb_draw_graphic32() at its callsite
>   hw/display/omap_lcdc: Expand out macros in template header
>   hw/display/omap_lcdc: Drop broken bigendian ifdef
>   hw/display/omap_lcdc: Fix coding style issues in template header
>   hw/display/omap_lcdc: Inline template header into C file
>   hw/display/omap_lcdc: Delete unnecessary macro

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~