hw: goldfish_fb: Minimize the size of the update rectangle.

This patch modifies the framebuffer's update callback to compute the minimum
bounding rectangle of each framebuffer update. Before that, the code only
computed the minimum and maximum Y coordinates, assuming xmin=0 and xmax=width-1.

This should help reduce un-necessary traffic/churn for our core->UI framebuffer
protocol.

Moreover, introduce the ability to support framebuffers / display surfaces with
different depths. This is a first step towards providing 32-bit framebuffers to
the guest system to slightly speed-up emulated rendering on Gingerbread and
above.

Change-Id: I6194824ce67a3b1d6d95b5d97ddec3ef459e408e

1 files changed, 323 insertions, 92 deletions

diff --git a/hw/goldfish_fb.c b/hw/goldfish_fb.c
index 8300e7c..cc57f6a 100644
--- a/hw/goldfish_fb.c
+++ b/hw/goldfish_fb.c
@@ -14,6 +14,19 @@
 #include "goldfish_device.h"
 #include "console.h"
 
+/* These values *must* match the platform definitions found under
+ * hardware/libhardware/include/hardware/hardware.h
+ */
+enum {
+    HAL_PIXEL_FORMAT_RGBA_8888          = 1,
+    HAL_PIXEL_FORMAT_RGBX_8888          = 2,
+    HAL_PIXEL_FORMAT_RGB_888            = 3,
+    HAL_PIXEL_FORMAT_RGB_565            = 4,
+    HAL_PIXEL_FORMAT_BGRA_8888          = 5,
+    HAL_PIXEL_FORMAT_RGBA_5551          = 6,
+    HAL_PIXEL_FORMAT_RGBA_4444          = 7,
+};
+
 enum {
     FB_GET_WIDTH        = 0x00,
     FB_GET_HEIGHT       = 0x04,
@@ -32,6 +45,8 @@ enum {
 struct goldfish_fb_state {
     struct goldfish_device dev;
     DisplayState*  ds;
+    int      pixel_format;
+    int      bytes_per_pixel;
     uint32_t fb_base;
     uint32_t base_valid : 1;
     uint32_t need_update : 1;
@@ -114,6 +129,82 @@ Exit:
     return ret;
 }
 
+/* Type used to record a mapping from display surface pixel format to
+ * HAL pixel format */
+typedef struct {
+    int    pixel_format; /* HAL pixel format */
+    uint8_t bits;
+    uint8_t bytes;
+    uint32_t rmask, gmask, bmask, amask;
+} FbConfig;
+
+
+/* Return the pixel format of the current framebuffer, based on
+ * the current display surface's pixel format.
+ *
+ * Note that you should not call this function from the device initialization
+ * function, because the display surface will change format before the kernel
+ * start.
+ */
+static int goldfish_fb_get_pixel_format(struct goldfish_fb_state *s)
+{
+    if (s->pixel_format >= 0) {
+        return s->pixel_format;
+    }
+    static const FbConfig fb_configs[] = {
+        { HAL_PIXEL_FORMAT_RGB_565, 16, 2, 0xf800, 0x7e0, 0x1f, 0x0 },
+        { -1, }
+    };
+
+    /* Determine HAL pixel format value based on s->ds */
+    struct PixelFormat* pf = &s->ds->surface->pf;
+#if 0
+    printf("%s:%d: display surface,pixel format:\n", __FUNCTION__, __LINE__);
+    printf("  bits/pixel:  %d\n", pf->bits_per_pixel);
+    printf("  bytes/pixel: %d\n", pf->bytes_per_pixel);
+    printf("  depth:       %d\n", pf->depth);
+    printf("  red:         bits=%d mask=0x%x shift=%d max=0x%x\n",
+           pf->rbits, pf->rmask, pf->rshift, pf->rmax);
+    printf("  green:       bits=%d mask=0x%x shift=%d max=0x%x\n",
+           pf->gbits, pf->gmask, pf->gshift, pf->gmax);
+    printf("  blue:        bits=%d mask=0x%x shift=%d max=0x%x\n",
+           pf->bbits, pf->bmask, pf->bshift, pf->bmax);
+    printf("  alpha:       bits=%d mask=0x%x shift=%d max=0x%x\n",
+           pf->abits, pf->amask, pf->ashift, pf->amax);
+
+#endif
+    s->bytes_per_pixel = pf->bytes_per_pixel;
+    int nn;
+    for (nn = 0; fb_configs[nn].pixel_format >= 0; nn++) {
+        const FbConfig* fbc = &fb_configs[nn];
+        if (pf->bits_per_pixel == fbc->bits &&
+            pf->bytes_per_pixel == fbc->bytes &&
+            pf->rmask == fbc->rmask &&
+            pf->gmask == fbc->gmask &&
+            pf->bmask == fbc->bmask &&
+            pf->amask == fbc->amask) {
+            /* We found it */
+            s->pixel_format = fbc->pixel_format;
+            return s->pixel_format;
+        }
+    }
+    fprintf(stderr, "%s:%d: Unsupported display pixel format (depth=%d, bytespp=%d, bitspp=%d)\n",
+                __FUNCTION__, __LINE__,
+                pf->depth,
+                pf->bytes_per_pixel,
+                pf->bits_per_pixel);
+    exit(1);
+    return -1;
+}
+
+static int goldfish_fb_get_bytes_per_pixel(struct goldfish_fb_state *s)
+{
+    if (s->pixel_format < 0) {
+        (void) goldfish_fb_get_pixel_format(s);
+    }
+    return s->bytes_per_pixel;
+}
+
 static int
 pixels_to_mm(int  pixels, int dpi)
 {
@@ -135,17 +226,210 @@ static int   stats_full_updates;
 static long  stats_total_full_updates;
 #endif
 
+/* This structure is used to hold the inputs for
+ * compute_fb_update_rect_linear below.
+ * This corresponds to the source framebuffer and destination
+ * surface pixel buffers.
+ */
+typedef struct {
+    int            width;
+    int            height;
+    int            bytes_per_pixel;
+    const uint8_t* src_pixels;
+    int            src_pitch;
+    uint8_t*       dst_pixels;
+    int            dst_pitch;
+} FbUpdateState;
+
+/* This structure is used to hold the outputs for
+ * compute_fb_update_rect_linear below.
+ * This corresponds to the smalled bounding rectangle of the
+ * latest framebuffer update.
+ */
+typedef struct {
+    int xmin, ymin, xmax, ymax;
+} FbUpdateRect;
+
+/* Determine the smallest bounding rectangle of pixels which changed
+ * between the source (framebuffer) and destination (surface) pixel
+ * buffers.
+ *
+ * Return 0 if there was no change, otherwise, populate '*rect'
+ * and return 1.
+ *
+ * If 'dirty_base' is not 0, it is a physical address that will be
+ * used to speed-up the check using the VGA dirty bits. In practice
+ * this is only used if your kernel driver does not implement.
+ *
+ * This function assumes that the framebuffers are in linear memory.
+ * This may change later when we want to support larger framebuffers
+ * that exceed the max DMA aperture size though.
+ */
+static int
+compute_fb_update_rect_linear(FbUpdateState*  fbs,
+                              uint32_t        dirty_base,
+                              FbUpdateRect*   rect)
+{
+    int  yy;
+    int  width = fbs->width;
+    const uint8_t* src_line = fbs->src_pixels;
+    uint8_t*       dst_line = fbs->dst_pixels;
+    uint32_t       dirty_addr = dirty_base;
+    rect->xmin = rect->ymin = INT_MAX;
+    rect->xmax = rect->ymax = INT_MIN;
+    for (yy = 0; yy < fbs->height; yy++) {
+        int xx1, xx2;
+        /* If dirty_addr is != 0, then use it as a physical address to
+         * use the VGA dirty bits table to speed up the detection of
+         * changed pixels.
+         */
+        if (dirty_addr != 0) {
+            int  dirty = 0;
+            int  len   = fbs->src_pitch;
+
+            while (len > 0) {
+                int  len2 = TARGET_PAGE_SIZE - (dirty_addr & (TARGET_PAGE_SIZE-1));
+
+                if (len2 > len)
+                    len2 = len;
+
+                dirty |= cpu_physical_memory_get_dirty(dirty_addr, VGA_DIRTY_FLAG);
+                dirty_addr  += len2;
+                len         -= len2;
+            }
+
+            if (!dirty) { /* this line was not modified, skip to next one */
+                goto NEXT_LINE;
+            }
+        }
+
+        /* Then compute actual bounds of the changed pixels, while
+         * copying them from 'src' to 'dst'. This depends on the pixel depth.
+         */
+        switch (fbs->bytes_per_pixel) {
+        case 2:
+        {
+            const uint16_t* src = (const uint16_t*) src_line;
+            uint16_t*       dst = (uint16_t*) dst_line;
+
+            for (xx1 = 0; xx1 < width; xx1++) {
+                if (src[xx1] != dst[xx1]) {
+                    break;
+                }
+            }
+            if (xx1 == width) {
+                break;
+            }
+            for (xx2 = width-1; xx2 > xx1; xx2--) {
+                if (src[xx2] != dst[xx2]) {
+                    break;
+                }
+            }
+#if HOST_WORDS_BIGENDIAN
+            /* Convert the guest little-endian pixels into big-endian ones */
+            int xx = xx1;
+            for ( ; xx <= xx2; xx++ ) {
+                unsigned   spix = src[xx];
+                dst[xx] = (uint16_t)((spix << 8) | (spix >> 8));
+            }
+#else
+            memcpy( dst+xx1, src+xx1, (xx2-xx1+1)*2 );
+#endif
+            break;
+        }
+
+        case 3:
+        {
+            for (xx1 = 0; xx1 < width; xx1 += 1) {
+                int xx = xx1*3;
+                if (src_line[xx+0] != dst_line[xx+0] ||
+                    src_line[xx+1] != dst_line[xx+1] ||
+                    src_line[xx+2] != dst_line[xx+2]) {
+                    break;
+                }
+            }
+            if (xx1 == width) {
+                break;
+            }
+            for (xx2 = width-1; xx2 > xx1; xx2--) {
+                int xx = xx2*3;
+                if (src_line[xx+0] != dst_line[xx+0] ||
+                    src_line[xx+1] != dst_line[xx+1] ||
+                    src_line[xx+2] != dst_line[xx+2]) {
+                    break;
+                }
+            }
+            memcpy( dst_line+xx1*3, src_line+xx1*3, (xx2-xx1+1)*3 );
+            break;
+        }
+
+        case 4:
+        {
+            const uint32_t* src = (const uint32_t*) src_line;
+            uint32_t*       dst = (uint32_t*) dst_line;
+
+            for (xx1 = 0; xx1 < width; xx1++) {
+                if (src[xx1] != dst[xx1]) {
+                    break;
+                }
+            }
+            if (xx1 == width) {
+                break;
+            }
+            for (xx2 = width-1; xx2 > xx1; xx2--) {
+                if (src[xx2] != dst[xx2]) {
+                    break;
+                }
+            }
+#if HOST_WORDS_BIGENDIAN
+            /* Convert the guest little-endian pixels into big-endian ones */
+            int xx = xx1;
+            for ( ; xx <= xx2; xx++ ) {
+                uint32_t   spix = src[xx];
+                spix = (spix << 16) | (spix >> 16);
+                spix = ((spix << 8) & 0xff00ff00) | ((spix >> 8) & 0x00ff00ff);
+                dst[xx] = spix;
+            }
+#else
+            memcpy( dst+xx1, src+xx1, (xx2-xx1+1)*4 );
+#endif
+            break;
+        }
+        default:
+            return 0;
+        }
+        /* Update bounds if pixels on this line were modified */
+        if (xx1 < width) {
+            if (xx1 < rect->xmin) rect->xmin = xx1;
+            if (xx2 > rect->xmax) rect->xmax = xx2;
+            if (yy < rect->ymin) rect->ymin = yy;
+            if (yy > rect->ymax) rect->ymax = yy;
+        }
+    NEXT_LINE:
+        src_line += fbs->src_pitch;
+        dst_line += fbs->dst_pitch;
+    }
+
+    if (rect->ymin > rect->ymax) { /* nothing changed */
+        return 0;
+    }
+
+    /* Always clear the dirty VGA bits */
+    cpu_physical_memory_reset_dirty(dirty_base + rect->ymin * fbs->src_pitch,
+                                    dirty_base + (rect->ymax+1)* fbs->src_pitch,
+                                    VGA_DIRTY_FLAG);
+    return 1;
+}
+
+
 static void goldfish_fb_update_display(void *opaque)
 {
     struct goldfish_fb_state *s = (struct goldfish_fb_state *)opaque;
-    uint32_t addr;
     uint32_t base;
-
     uint8_t*  dst_line;
     uint8_t*  src_line;
-    int y_first, y_last = 0;
     int full_update = 0;
-    int    width, height, pitch;
+    int  width, height, pitch;
 
     base = s->fb_base;
     if(base == 0)
@@ -156,8 +440,6 @@ static void goldfish_fb_update_display(void *opaque)
         goldfish_device_set_irq(&s->dev, 0, 1);
     }
 
-    y_first = -1;
-    addr  = base;
     if(s->need_update) {
         full_update = 1;
         if(s->need_int) {
@@ -176,6 +458,19 @@ static void goldfish_fb_update_display(void *opaque)
     width     = s->ds->surface->width;
     height    = s->ds->surface->height;
 
+    FbUpdateState  fbs;
+    FbUpdateRect   rect;
+
+    fbs.width      = width;
+    fbs.height     = height;
+    fbs.dst_pixels = dst_line;
+    fbs.dst_pitch  = pitch;
+    fbs.bytes_per_pixel = goldfish_fb_get_bytes_per_pixel(s);
+
+    fbs.src_pixels = src_line;
+    fbs.src_pitch  = width*s->ds->surface->pf.bytes_per_pixel;
+
+
 #if STATS
     if (full_update)
         stats_full_updates += 1;
@@ -195,99 +490,29 @@ static void goldfish_fb_update_display(void *opaque)
     if (s->blank)
     {
         memset( dst_line, 0, height*pitch );
-        y_first = 0;
-        y_last  = height-1;
+        rect.xmin = 0;
+        rect.ymin = 0;
+        rect.xmax = width-1;
+        rect.ymax = height-1;
     }
-    else if (full_update)
+    else
     {
-        int  yy;
-
-        for (yy = 0; yy < height; yy++, dst_line += pitch, src_line += width*2)
-        {
-            uint16_t*  src = (uint16_t*) src_line;
-            uint16_t*  dst = (uint16_t*) dst_line;
-            int        nn;
-
-            for (nn = 0; nn < width; nn++) {
-                unsigned   spix = src[nn];
-                unsigned   dpix = dst[nn];
-#if HOST_WORDS_BIGENDIAN
-                spix = ((spix << 8) | (spix >> 8)) & 0xffff;
-#else
-                if (spix != dpix)
-                    break;
-#endif
-            }
-
-            if (nn == width)
-                continue;
-
-#if HOST_WORDS_BIGENDIAN
-            for ( ; nn < width; nn++ ) {
-                unsigned   spix = src[nn];
-                dst[nn] = (uint16_t)((spix << 8) | (spix >> 8));
-            }
-#else
-            memcpy( dst+nn, src+nn, (width-nn)*2 );
-#endif
-
-            y_first = (y_first < 0) ? yy : y_first;
-            y_last  = yy;
+        if (full_update) { /* don't use dirty-bits optimization */
+            base = 0;
         }
-    }
-    else  /* not a full update, should not happen very often with Android */
-    {
-        int  yy;
-
-        for (yy = 0; yy < height; yy++, dst_line += pitch, src_line += width*2)
-        {
-            uint16_t*  src   = (uint16_t*) src_line;
-            uint16_t*  dst   = (uint16_t*) dst_line;
-            int        len   = width*2;
-#if HOST_WORDS_BIGENDIAN
-            int        nn;
-#endif
-            int        dirty = 0;
-
-            while (len > 0) {
-                int  len2 = TARGET_PAGE_SIZE - (addr & (TARGET_PAGE_SIZE-1));
-
-                if (len2 > len)
-                    len2 = len;
-
-                dirty |= cpu_physical_memory_get_dirty(addr, VGA_DIRTY_FLAG);
-                addr  += len2;
-                len   -= len2;
-            }
-
-            if (!dirty)
-                continue;
-
-#if HOST_WORDS_BIGENDIAN
-            for (nn = 0; nn < width; nn++ ) {
-                unsigned   spix = src[nn];
-                dst[nn] = (uint16_t)((spix << 8) | (spix >> 8));
-            }
-#else
-            memcpy( dst, src, width*2 );
-#endif
-
-            y_first = (y_first < 0) ? yy : y_first;
-            y_last  = yy;
+        if (compute_fb_update_rect_linear(&fbs, base, &rect) == 0) {
+            return;
         }
     }
 
-    if (y_first < 0)
-      return;
-
-    y_last += 1;
-    //printf("goldfish_fb_update_display %d %d, base %x\n", first, last, base);
-
-    cpu_physical_memory_reset_dirty(base + y_first * width * 2,
-                                    base + y_last * width * 2,
-                                    VGA_DIRTY_FLAG);
+    rect.xmax += 1;
+    rect.ymax += 1;
+#if 0
+    printf("goldfish_fb_update_display (y:%d,h:%d,x=%d,w=%d)\n",
+           rect.ymin, rect.ymax-rect.ymin, rect.xmin, rect.xmax-rect.xmin);
+#endif
 
-    dpy_update(s->ds, 0, y_first, width, y_last-y_first);
+    dpy_update(s->ds, rect.xmin, rect.ymin, rect.xmax-rect.xmin, rect.ymax-rect.ymin);
 }
 
 static void goldfish_fb_invalidate_display(void * opaque)
@@ -408,6 +633,12 @@ void goldfish_fb_init(int id)
 
     s->dpi = 165;  /* XXX: Find better way to get actual value ! */
 
+    /* IMPORTANT: DO NOT COMPUTE s->pixel_format and s->bytes_per_pixel
+     * here because the display surface is going to change later.
+     */
+    s->bytes_per_pixel = 0;
+    s->pixel_format    = -1;
+
     goldfish_device_add(&s->dev, goldfish_fb_readfn, goldfish_fb_writefn, s);
 
     register_savevm( "goldfish_fb", 0, GOLDFISH_FB_SAVE_VERSION,