framebuffer: set DIRTY_MEMORY_VGA on RAM that is used for the framebuffer

The MemoryRegionSection contains enough information to access the
RAM region underlying the framebuffer, and can be cached inside the
display device.

By doing this, the new framebuffer_update_memory_section function can
enable dirty memory logging on the relevant RAM region.  The function
must be called whenever the stride or base of the framebuffer changes;
a simple way to cover these cases is to call it on every full frame
invalidation, which is a rare case.

framebuffer_update_display now works entirely on a MemoryRegionSection,
without going through cpu_physical_memory_map/unmap.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

6 files changed, 137 insertions, 51 deletions

diff --git a/hw/display/framebuffer.c b/hw/display/framebuffer.c
index 2cabced..7f075ce 100644
--- a/hw/display/framebuffer.c
+++ b/hw/display/framebuffer.c
@@ -21,12 +21,40 @@
 #include "ui/console.h"
 #include "framebuffer.h"
 
+void framebuffer_update_memory_section(
+    MemoryRegionSection *mem_section,
+    MemoryRegion *root,
+    hwaddr base,
+    unsigned rows,
+    unsigned src_width)
+{
+    hwaddr src_len = (hwaddr)rows * src_width;
+
+    if (mem_section->mr) {
+        memory_region_set_log(mem_section->mr, false, DIRTY_MEMORY_VGA);
+        memory_region_unref(mem_section->mr);
+        mem_section->mr = NULL;
+    }
+
+    *mem_section = memory_region_find(root, base, src_len);
+    if (!mem_section->mr) {
+        return;
+    }
+
+    if (int128_get64(mem_section->size) < src_len ||
+            !memory_region_is_ram(mem_section->mr)) {
+        memory_region_unref(mem_section->mr);
+        mem_section->mr = NULL;
+        return;
+    }
+
+    memory_region_set_log(mem_section->mr, true, DIRTY_MEMORY_VGA);
+}
+
 /* Render an image from a shared memory framebuffer.  */
-   
 void framebuffer_update_display(
     DisplaySurface *ds,
-    MemoryRegion *address_space,
-    hwaddr base,
+    MemoryRegionSection *mem_section,
     int cols, /* Width in pixels.  */
     int rows, /* Height in pixels.  */
     int src_width, /* Length of source line, in bytes.  */
@@ -41,51 +69,33 @@ void framebuffer_update_display(
     hwaddr src_len;
     uint8_t *dest;
     uint8_t *src;
-    uint8_t *src_base;
     int first, last = 0;
     int dirty;
     int i;
     ram_addr_t addr;
-    MemoryRegionSection mem_section;
     MemoryRegion *mem;
 
     i = *first_row;
     *first_row = -1;
     src_len = src_width * rows;
 
-    mem_section = memory_region_find(address_space, base, src_len);
-    mem = mem_section.mr;
-    if (int128_get64(mem_section.size) != src_len ||
-            !memory_region_is_ram(mem_section.mr)) {
-        goto out;
+    mem = mem_section->mr;
+    if (!mem) {
+        return;
     }
-    assert(mem);
-    assert(mem_section.offset_within_address_space == base);
-
     memory_region_sync_dirty_bitmap(mem);
-    if (!memory_region_is_logging(mem, DIRTY_MEMORY_VGA)) {
-        invalidate = true;
-    }
 
-    src_base = cpu_physical_memory_map(base, &src_len, 0);
-    /* If we can't map the framebuffer then bail.  We could try harder,
-       but it's not really worth it as dirty flag tracking will probably
-       already have failed above.  */
-    if (!src_base)
-        goto out;
-    if (src_len != src_width * rows) {
-        cpu_physical_memory_unmap(src_base, src_len, 0, 0);
-        goto out;
-    }
-    src = src_base;
+    addr = mem_section->offset_within_region;
+    src = memory_region_get_ram_ptr(mem) + addr;
+
     dest = surface_data(ds);
-    if (dest_col_pitch < 0)
+    if (dest_col_pitch < 0) {
         dest -= dest_col_pitch * (cols - 1);
+    }
     if (dest_row_pitch < 0) {
         dest -= dest_row_pitch * (rows - 1);
     }
     first = -1;
-    addr = mem_section.offset_within_region;
 
     addr += i * src_width;
     src += i * src_width;
@@ -104,14 +114,11 @@ void framebuffer_update_display(
         src += src_width;
         dest += dest_row_pitch;
     }
-    cpu_physical_memory_unmap(src_base, src_len, 0, 0);
     if (first < 0) {
-        goto out;
+        return;
     }
-    memory_region_reset_dirty(mem, mem_section.offset_within_region, src_len,
+    memory_region_reset_dirty(mem, mem_section->offset_within_region, src_len,
                               DIRTY_MEMORY_VGA);
     *first_row = first;
     *last_row = last;
-out:
-    memory_region_unref(mem);
 }
diff --git a/hw/display/framebuffer.h b/hw/display/framebuffer.h
index 6eae035..38fa0dc 100644
--- a/hw/display/framebuffer.h
+++ b/hw/display/framebuffer.h
@@ -7,10 +7,50 @@
 
 typedef void (*drawfn)(void *, uint8_t *, const uint8_t *, int, int);
 
+/* framebuffer_update_memory_section: Update framebuffer
+ * #MemoryRegionSection, for example if the framebuffer is switched to
+ * a different memory area.
+ *
+ * @mem_section: Output #MemoryRegionSection, to be passed to
+ * framebuffer_update_display().
+ * @root: #MemoryRegion within which the framebuffer lies
+ * @base: Base address of the framebuffer within @root.
+ * @rows: Height of the screen.
+ * @src_width: Number of bytes in framebuffer memory between two rows.
+ */
+void framebuffer_update_memory_section(
+    MemoryRegionSection *mem_section,
+    MemoryRegion *root,
+    hwaddr base,
+    unsigned rows,
+    unsigned src_width);
+
+/* framebuffer_update_display: Draw the framebuffer on a surface.
+ *
+ * @ds: #DisplaySurface to draw to.
+ * @mem_section: #MemoryRegionSection provided by
+ * framebuffer_update_memory_section().
+ * @cols: Width the screen.
+ * @rows: Height of the screen.
+ * @src_width: Number of bytes in framebuffer memory between two rows.
+ * @dest_row_pitch: Number of bytes in the surface data between two rows.
+ * Negative if the framebuffer is stored in the opposite order (e.g.
+ * bottom-to-top) compared to the framebuffer.
+ * @dest_col_pitch: Number of bytes in the surface data between two pixels.
+ * Negative if the framebuffer is stored in the opposite order (e.g.
+ * right-to-left) compared to the framebuffer.
+ * @invalidate: True if the function should redraw the whole screen
+ * without checking the DIRTY_MEMORY_VGA dirty bitmap.
+ * @fn: Drawing function to be called for each row that has to be drawn.
+ * @opaque: Opaque pointer passed to @fn.
+ * @first_row: Pointer to an integer, receives the number of the first row
+ * that was drawn (either the first dirty row, or 0 if @invalidate is true).
+ * @last_row: Pointer to an integer, receives the number of the last row that
+ * was drawn (either the last dirty row, or @rows-1 if @invalidate is true).
+ */
 void framebuffer_update_display(
     DisplaySurface *ds,
-    MemoryRegion *address_space,
-    hwaddr base,
+    MemoryRegionSection *mem_section,
     int cols,
     int rows,
     int src_width,
diff --git a/hw/display/milkymist-vgafb.c b/hw/display/milkymist-vgafb.c
index 9b35e76..ab3074f 100644
--- a/hw/display/milkymist-vgafb.c
+++ b/hw/display/milkymist-vgafb.c
@@ -71,6 +71,7 @@ struct MilkymistVgafbState {
     SysBusDevice parent_obj;
 
     MemoryRegion regs_region;
+    MemoryRegionSection fbsection;
     QemuConsole *con;
 
     int invalidate;
@@ -91,6 +92,7 @@ static void vgafb_update_display(void *opaque)
     MilkymistVgafbState *s = opaque;
     SysBusDevice *sbd;
     DisplaySurface *surface = qemu_console_surface(s->con);
+    int src_width;
     int first = 0;
     int last = 0;
     drawfn fn;
@@ -129,11 +131,18 @@ static void vgafb_update_display(void *opaque)
         break;
     }
 
-    framebuffer_update_display(surface, sysbus_address_space(sbd),
-                               s->regs[R_BASEADDRESS] + s->fb_offset,
+    src_width = s->regs[R_HRES] * 2;
+    if (s->invalidate) {
+        framebuffer_update_memory_section(&s->fbsection,
+                                          sysbus_address_space(sbd),
+                                          s->regs[R_BASEADDRESS] + s->fb_offset,
+                                          s->regs[R_VRES], src_width);
+    }
+
+    framebuffer_update_display(surface, &s->fbsection,
                                s->regs[R_HRES],
                                s->regs[R_VRES],
-                               s->regs[R_HRES] * 2,
+                               src_width,
                                dest_width,
                                0,
                                s->invalidate,
diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c
index fda81ba..a7c6cd7 100644
--- a/hw/display/omap_lcdc.c
+++ b/hw/display/omap_lcdc.c
@@ -25,6 +25,7 @@
 struct omap_lcd_panel_s {
     MemoryRegion *sysmem;
     MemoryRegion iomem;
+    MemoryRegionSection fbsection;
     qemu_irq irq;
     QemuConsole *con;
 
@@ -215,12 +216,19 @@ static void omap_update_display(void *opaque)
 
     step = width * bpp >> 3;
     linesize = surface_stride(surface);
-    framebuffer_update_display(surface, omap_lcd->sysmem,
-                               frame_base, width, height,
+    if (omap_lcd->invalidate) {
+        framebuffer_update_memory_section(&omap_lcd->fbsection,
+                                          omap_lcd->sysmem, frame_base,
+                                          height, step);
+    }
+
+    framebuffer_update_display(surface, &omap_lcd->fbsection,
+                               width, height,
                                step, linesize, 0,
                                omap_lcd->invalidate,
                                draw_line, omap_lcd->palette,
                                &first, &last);
+
     if (first >= 0) {
         dpy_gfx_update(omap_lcd->con, 0, first, width, last - first + 1);
     }
diff --git a/hw/display/pl110.c b/hw/display/pl110.c
index c574cf1..ef1a7b1 100644
--- a/hw/display/pl110.c
+++ b/hw/display/pl110.c
@@ -46,6 +46,7 @@ typedef struct PL110State {
     SysBusDevice parent_obj;
 
     MemoryRegion iomem;
+    MemoryRegionSection fbsection;
     QemuConsole *con;
 
     int version;
@@ -238,12 +239,20 @@ static void pl110_update_display(void *opaque)
     }
     dest_width *= s->cols;
     first = 0;
-    framebuffer_update_display(surface, sysbus_address_space(sbd),
-                               s->upbase, s->cols, s->rows,
+    if (s->invalidate) {
+        framebuffer_update_memory_section(&s->fbsection,
+                                          sysbus_address_space(sbd),
+                                          s->upbase,
+                                          s->rows, src_width);
+    }
+
+    framebuffer_update_display(surface, &s->fbsection,
+                               s->cols, s->rows,
                                src_width, dest_width, 0,
                                s->invalidate,
                                fn, s->palette,
                                &first, &last);
+
     if (first >= 0) {
         dpy_gfx_update(s->con, 0, first, s->cols, last - first + 1);
     }
diff --git a/hw/display/pxa2xx_lcd.c b/hw/display/pxa2xx_lcd.c
index ac3c018..494700d 100644
--- a/hw/display/pxa2xx_lcd.c
+++ b/hw/display/pxa2xx_lcd.c
@@ -35,6 +35,7 @@ struct DMAChannel {
 struct PXA2xxLCDState {
     MemoryRegion *sysmem;
     MemoryRegion iomem;
+    MemoryRegionSection fbsection;
     qemu_irq irq;
     int irqlevel;
 
@@ -687,8 +688,11 @@ static void pxa2xx_lcdc_dma0_redraw_rot0(PXA2xxLCDState *s,
 
     dest_width = s->xres * s->dest_width;
     *miny = 0;
-    framebuffer_update_display(surface, s->sysmem,
-                               addr, s->xres, s->yres,
+    if (s->invalidated) {
+        framebuffer_update_memory_section(&s->fbsection, s->sysmem,
+                                          addr, s->yres, src_width);
+    }
+    framebuffer_update_display(surface, &s->fbsection, s->xres, s->yres,
                                src_width, dest_width, s->dest_width,
                                s->invalidated,
                                fn, s->dma_ch[0].palette, miny, maxy);
@@ -715,8 +719,11 @@ static void pxa2xx_lcdc_dma0_redraw_rot90(PXA2xxLCDState *s,
 
     dest_width = s->yres * s->dest_width;
     *miny = 0;
-    framebuffer_update_display(surface, s->sysmem,
-                               addr, s->xres, s->yres,
+    if (s->invalidated) {
+        framebuffer_update_memory_section(&s->fbsection, s->sysmem,
+                                          addr, s->yres, src_width);
+    }
+    framebuffer_update_display(surface, &s->fbsection, s->xres, s->yres,
                                src_width, s->dest_width, -dest_width,
                                s->invalidated,
                                fn, s->dma_ch[0].palette,
@@ -747,8 +754,11 @@ static void pxa2xx_lcdc_dma0_redraw_rot180(PXA2xxLCDState *s,
 
     dest_width = s->xres * s->dest_width;
     *miny = 0;
-    framebuffer_update_display(surface, s->sysmem,
-                               addr, s->xres, s->yres,
+    if (s->invalidated) {
+        framebuffer_update_memory_section(&s->fbsection, s->sysmem,
+                                          addr, s->yres, src_width);
+    }
+    framebuffer_update_display(surface, &s->fbsection, s->xres, s->yres,
                                src_width, -dest_width, -s->dest_width,
                                s->invalidated,
                                fn, s->dma_ch[0].palette, miny, maxy);
@@ -778,8 +788,11 @@ static void pxa2xx_lcdc_dma0_redraw_rot270(PXA2xxLCDState *s,
 
     dest_width = s->yres * s->dest_width;
     *miny = 0;
-    framebuffer_update_display(surface, s->sysmem,
-                               addr, s->xres, s->yres,
+    if (s->invalidated) {
+        framebuffer_update_memory_section(&s->fbsection, s->sysmem,
+                                          addr, s->yres, src_width);
+    }
+    framebuffer_update_display(surface, &s->fbsection, s->xres, s->yres,
                                src_width, -s->dest_width, dest_width,
                                s->invalidated,
                                fn, s->dma_ch[0].palette,