[Qemu-devel] [PATCH 0/7] Make vga devices use test_and_set

[Qemu-devel] [PATCH 0/7] Make vga devices use test_and_set

[Juan Quintela]

Hi

While I was working with memory.c, I was removing last user of
memory_region_test_and_clear().  But searching on the tree, I can see
that vga devices look like good users.  Comments?

Later, Juan.

Juan Quintela (7):
  vmware_vga: port to test_and_clear instead of test/reset
  vga: port to test_and_clear instead of test/reset
  tcx: port to test_and_clear instead of test/reset
  sm501: port to test_and_clear instead of test/reset
  g364fb: port to test_and_clear instead of test/reset
  framebufer: port to test_and_clear instead of test/reset
  exynox4210: port to test_and_clear instead of test/reset

 hw/display/exynos4210_fimd.c |  5 +--
 hw/display/framebuffer.c     |  6 ++--
 hw/display/g364fb.c          | 38 ++++++++---------------
 hw/display/sm501.c           | 18 ++---------
 hw/display/tcx.c             | 72 +++++++++++---------------------------------
 hw/display/vga.c             | 20 +++---------
 hw/display/vmware_vga.c      |  7 +----
 7 files changed, 41 insertions(+), 125 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/7] vmware_vga: port to test_and_clear instead of test/reset

[Juan Quintela]

From: Juan Quintela <quintela@trasno.org>

I am assuming that we are testing all the device pages on the loop, that is right?

Signed-off-by: Juan Quintela <quintela@trasno.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/display/vmware_vga.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index a6a8cdc..9852a79 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -1082,7 +1082,7 @@ static void vmsvga_update_display(void *opaque)
      */
     if (memory_region_is_logging(&s->vga.vram)) {
         vga_sync_dirty_bitmap(&s->vga);
-        dirty = memory_region_get_dirty(&s->vga.vram, 0,
+        dirty = memory_region_test_and_clear_dirty(&s->vga.vram, 0,
             surface_stride(surface) * surface_height(surface),
             DIRTY_MEMORY_VGA);
     }
@@ -1091,11 +1091,6 @@ static void vmsvga_update_display(void *opaque)
         dpy_gfx_update(s->vga.con, 0, 0,
                    surface_width(surface), surface_height(surface));
     }
-    if (dirty) {
-        memory_region_reset_dirty(&s->vga.vram, 0,
-            surface_stride(surface) * surface_height(surface),
-            DIRTY_MEMORY_VGA);
-    }
 }

 static void vmsvga_reset(DeviceState *dev)
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/7] vga: port to test_and_clear instead of test/reset

[Juan Quintela]

From: Juan Quintela <quintela@trasno.org>

Now page_min/max are not needed anymore.

Signed-off-by: Juan Quintela <quintela@trasno.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/display/vga.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 7b91d9c..d411c5c 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1640,7 +1640,7 @@ static void vga_draw_graphic(VGACommonState *s, int full_update)
     DisplaySurface *surface = qemu_console_surface(s->con);
     int y1, y, update, linesize, y_start, double_scan, mask, depth;
     int width, height, shift_control, line_offset, bwidth, bits;
-    ram_addr_t page0, page1, page_min, page_max;
+    ram_addr_t page0, page1;
     int disp_width, multi_scan, multi_run;
     uint8_t *d;
     uint32_t v, addr1, addr;
@@ -1784,8 +1784,6 @@ static void vga_draw_graphic(VGACommonState *s, int full_update)
     addr1 = (s->start_addr * 4);
     bwidth = (width * bits + 7) / 8;
     y_start = -1;
-    page_min = -1;
-    page_max = 0;
     d = surface_data(surface);
     linesize = surface_stride(surface);
     y1 = 0;
@@ -1803,17 +1801,14 @@ static void vga_draw_graphic(VGACommonState *s, int full_update)
         update = full_update;
         page0 = addr;
         page1 = addr + bwidth - 1;
-        update |= memory_region_get_dirty(&s->vram, page0, page1 - page0,
-                                          DIRTY_MEMORY_VGA);
+        update |= memory_region_test_and_clear_dirty(&s->vram, page0,
+                                                     page1 - page0,
+                                                     DIRTY_MEMORY_VGA);
         /* explicit invalidation for the hardware cursor */
         update |= (s->invalidated_y_table[y >> 5] >> (y & 0x1f)) & 1;
         if (update) {
             if (y_start < 0)
                 y_start = y;
-            if (page0 < page_min)
-                page_min = page0;
-            if (page1 > page_max)
-                page_max = page1;
             if (!(is_buffer_shared(surface))) {
                 vga_draw_line(s, d, s->vram_ptr + addr, width);
                 if (s->cursor_draw_line)
@@ -1846,13 +1841,6 @@ static void vga_draw_graphic(VGACommonState *s, int full_update)
         dpy_gfx_update(s->con, 0, y_start,
                        disp_width, y - y_start);
     }
-    /* reset modified pages */
-    if (page_max >= page_min) {
-        memory_region_reset_dirty(&s->vram,
-                                  page_min,
-                                  page_max - page_min,
-                                  DIRTY_MEMORY_VGA);
-    }
     memset(s->invalidated_y_table, 0, ((height + 31) >> 5) * 4);
 }

-- 
1.8.3.1

[Qemu-devel] [PATCH 3/7] tcx: port to test_and_clear instead of test/reset

[Juan Quintela]

From: Juan Quintela <quintela@trasno.org>

Now page_min/max are not needed anymore.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/display/tcx.c | 72 ++++++++++++++------------------------------------------
 1 file changed, 18 insertions(+), 54 deletions(-)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 24876d3..d863a55 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -178,45 +178,31 @@ static inline void tcx24_draw_line32(TCXState *s1, uint8_t *d,
     }
 }

-static inline int check_dirty(TCXState *s, ram_addr_t page, ram_addr_t page24,
-                              ram_addr_t cpage)
+static inline int test_and_clear_dirty(TCXState *s, ram_addr_t page,
+                                       ram_addr_t page24,
+                                       ram_addr_t cpage)
 {
     int ret;

-    ret = memory_region_get_dirty(&s->vram_mem, page, TARGET_PAGE_SIZE,
-                                  DIRTY_MEMORY_VGA);
-    ret |= memory_region_get_dirty(&s->vram_mem, page24, TARGET_PAGE_SIZE * 4,
-                                   DIRTY_MEMORY_VGA);
-    ret |= memory_region_get_dirty(&s->vram_mem, cpage, TARGET_PAGE_SIZE * 4,
-                                   DIRTY_MEMORY_VGA);
+    ret = memory_region_test_and_clear_dirty(&s->vram_mem, page,
+                                             TARGET_PAGE_SIZE,
+                                             DIRTY_MEMORY_VGA);
+    ret |= memory_region_test_and_clear_dirty(&s->vram_mem, page24,
+                                              TARGET_PAGE_SIZE * 4,
+                                              DIRTY_MEMORY_VGA);
+    ret |= memory_region_test_and_clear_dirty(&s->vram_mem, cpage,
+                                              TARGET_PAGE_SIZE * 4,
+                                              DIRTY_MEMORY_VGA);
     return ret;
 }

-static inline void reset_dirty(TCXState *ts, ram_addr_t page_min,
-                               ram_addr_t page_max, ram_addr_t page24,
-                              ram_addr_t cpage)
-{
-    memory_region_reset_dirty(&ts->vram_mem,
-                              page_min,
-                              (page_max - page_min) + TARGET_PAGE_SIZE,
-                              DIRTY_MEMORY_VGA);
-    memory_region_reset_dirty(&ts->vram_mem,
-                              page24 + page_min * 4,
-                              (page_max - page_min) * 4 + TARGET_PAGE_SIZE,
-                              DIRTY_MEMORY_VGA);
-    memory_region_reset_dirty(&ts->vram_mem,
-                              cpage + page_min * 4,
-                              (page_max - page_min) * 4 + TARGET_PAGE_SIZE,
-                              DIRTY_MEMORY_VGA);
-}
-
 /* Fixed line length 1024 allows us to do nice tricks not possible on
    VGA... */
 static void tcx_update_display(void *opaque)
 {
     TCXState *ts = opaque;
     DisplaySurface *surface = qemu_console_surface(ts->con);
-    ram_addr_t page, page_min, page_max;
+    ram_addr_t page;
     int y, y_start, dd, ds;
     uint8_t *d, *s;
     void (*f)(TCXState *s1, uint8_t *dst, const uint8_t *src, int width);
@@ -227,8 +213,6 @@ static void tcx_update_display(void *opaque)

     page = 0;
     y_start = -1;
-    page_min = -1;
-    page_max = 0;
     d = surface_data(surface);
     s = ts->vram;
     dd = surface_stride(surface);
@@ -251,14 +235,11 @@ static void tcx_update_display(void *opaque)
     }

     for(y = 0; y < ts->height; y += 4, page += TARGET_PAGE_SIZE) {
-        if (memory_region_get_dirty(&ts->vram_mem, page, TARGET_PAGE_SIZE,
-                                    DIRTY_MEMORY_VGA)) {
+        if (memory_region_test_and_clear_dirty(&ts->vram_mem, page,
+                                               TARGET_PAGE_SIZE,
+                                               DIRTY_MEMORY_VGA)) {
             if (y_start < 0)
                 y_start = y;
-            if (page < page_min)
-                page_min = page;
-            if (page > page_max)
-                page_max = page;
             f(ts, d, s, ts->width);
             d += dd;
             s += ds;
@@ -287,20 +268,13 @@ static void tcx_update_display(void *opaque)
         dpy_gfx_update(ts->con, 0, y_start,
                        ts->width, y - y_start);
     }
-    /* reset modified pages */
-    if (page_max >= page_min) {
-        memory_region_reset_dirty(&ts->vram_mem,
-                                  page_min,
-                                  (page_max - page_min) + TARGET_PAGE_SIZE,
-                                  DIRTY_MEMORY_VGA);
-    }
 }

 static void tcx24_update_display(void *opaque)
 {
     TCXState *ts = opaque;
     DisplaySurface *surface = qemu_console_surface(ts->con);
-    ram_addr_t page, page_min, page_max, cpage, page24;
+    ram_addr_t page, cpage, page24;
     int y, y_start, dd, ds;
     uint8_t *d, *s;
     uint32_t *cptr, *s24;
@@ -313,8 +287,6 @@ static void tcx24_update_display(void *opaque)
     page24 = ts->vram24_offset;
     cpage = ts->cplane_offset;
     y_start = -1;
-    page_min = -1;
-    page_max = 0;
     d = surface_data(surface);
     s = ts->vram;
     s24 = ts->vram24;
@@ -324,13 +296,9 @@ static void tcx24_update_display(void *opaque)

     for(y = 0; y < ts->height; y += 4, page += TARGET_PAGE_SIZE,
             page24 += TARGET_PAGE_SIZE, cpage += TARGET_PAGE_SIZE) {
-        if (check_dirty(ts, page, page24, cpage)) {
+        if (test_and_clear_dirty(ts, page, page24, cpage)) {
             if (y_start < 0)
                 y_start = y;
-            if (page < page_min)
-                page_min = page;
-            if (page > page_max)
-                page_max = page;
             tcx24_draw_line32(ts, d, s, ts->width, cptr, s24);
             d += dd;
             s += ds;
@@ -369,10 +337,6 @@ static void tcx24_update_display(void *opaque)
         dpy_gfx_update(ts->con, 0, y_start,
                        ts->width, y - y_start);
     }
-    /* reset modified pages */
-    if (page_max >= page_min) {
-        reset_dirty(ts, page_min, page_max, page24, cpage);
-    }
 }

 static void tcx_invalidate_display(void *opaque)
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/7] sm501: port to test_and_clear instead of test/reset

[Juan Quintela]

From: Juan Quintela <quintela@trasno.org>

Now page_min/max are not needed anymore.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/display/sm501.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index c75d6ac..2b2bfac 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1270,8 +1270,6 @@ static void sm501_draw_crt(SM501State * s)
     draw_hwc_line_func * draw_hwc_line = NULL;
     int full_update = 0;
     int y_start = -1;
-    ram_addr_t page_min = ~0l;
-    ram_addr_t page_max = 0l;
     ram_addr_t offset = 0;

     /* choose draw_line function */
@@ -1328,8 +1326,9 @@ static void sm501_draw_crt(SM501State * s)
         ram_addr_t page1 = offset + width * src_bpp - 1;

 	/* check dirty flags for each line */
-        update = memory_region_get_dirty(&s->local_mem_region, page0,
-                                         page1 - page0, DIRTY_MEMORY_VGA);
+        update = memory_region_test_and_clear_dirty(&s->local_mem_region, page0,
+                                                    page1 - page0,
+                                                    DIRTY_MEMORY_VGA);

 	/* draw line and change status */
 	if (update) {
@@ -1346,10 +1345,6 @@ static void sm501_draw_crt(SM501State * s)

 	    if (y_start < 0)
 		y_start = y;
-	    if (page0 < page_min)
-		page_min = page0;
-	    if (page1 > page_max)
-		page_max = page1;
 	} else {
 	    if (y_start >= 0) {
 		/* flush to display */
@@ -1365,13 +1360,6 @@ static void sm501_draw_crt(SM501State * s)
     /* complete flush to display */
     if (y_start >= 0)
         dpy_gfx_update(s->con, 0, y_start, width, y - y_start);
-
-    /* clear dirty flags */
-    if (page_min != ~0l) {
-	memory_region_reset_dirty(&s->local_mem_region,
-                                  page_min, page_max + TARGET_PAGE_SIZE,
-                                  DIRTY_MEMORY_VGA);
-    }
 }

 static void sm501_update_display(void *opaque)
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/7] g364fb: port to test_and_clear instead of test/reset

[Juan Quintela]

From: Juan Quintela <quintela@trasno.org>

Now page_min/max are not needed anymore.  In this case, we need to
know if something has changed to refresh the display. Introduce
"dirty" bool for that.

Signed-off-by: Juan Quintela <quintela@trasno.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/display/g364fb.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
index 7082171..8306294 100644
--- a/hw/display/g364fb.c
+++ b/hw/display/g364fb.c
@@ -60,19 +60,11 @@ typedef struct G364State {

 #define G364_PAGE_SIZE 4096

-static inline int check_dirty(G364State *s, ram_addr_t page)
+static inline bool test_and_clear_dirty(G364State *s, ram_addr_t page)
 {
-    return memory_region_get_dirty(&s->mem_vram, page, G364_PAGE_SIZE,
-                                   DIRTY_MEMORY_VGA);
-}
-
-static inline void reset_dirty(G364State *s,
-                               ram_addr_t page_min, ram_addr_t page_max)
-{
-    memory_region_reset_dirty(&s->mem_vram,
-                              page_min,
-                              page_max + G364_PAGE_SIZE - page_min - 1,
-                              DIRTY_MEMORY_VGA);
+    return memory_region_test_and_clear_dirty(&s->mem_vram, page,
+                                              G364_PAGE_SIZE,
+                                              DIRTY_MEMORY_VGA);
 }

 static void g364fb_draw_graphic8(G364State *s)
@@ -81,12 +73,13 @@ static void g364fb_draw_graphic8(G364State *s)
     int i, w;
     uint8_t *vram;
     uint8_t *data_display, *dd;
-    ram_addr_t page, page_min, page_max;
+    ram_addr_t page;
     int x, y;
     int xmin, xmax;
     int ymin, ymax;
     int xcursor, ycursor;
     unsigned int (*rgb_to_pixel)(unsigned int r, unsigned int g, unsigned int b);
+    bool dirty;

     switch (surface_bits_per_pixel(surface)) {
         case 8:
@@ -112,9 +105,7 @@ static void g364fb_draw_graphic8(G364State *s)
     }

     page = 0;
-    page_min = (ram_addr_t)-1;
-    page_max = 0;
-
+    dirty = false;
     x = y = 0;
     xmin = s->width;
     xmax = 0;
@@ -132,12 +123,10 @@ static void g364fb_draw_graphic8(G364State *s)
     /* XXX: out of range in vram? */
     data_display = dd = surface_data(surface);
     while (y < s->height) {
-        if (check_dirty(s, page)) {
+        if (test_and_clear_dirty(s, page)) {
+            dirty = true;
             if (y < ymin)
                 ymin = ymax = y;
-            if (page_min == (ram_addr_t)-1)
-                page_min = page;
-            page_max = page;
             if (x < xmin)
                 xmin = x;
             for (i = 0; i < G364_PAGE_SIZE; i++) {
@@ -194,10 +183,8 @@ static void g364fb_draw_graphic8(G364State *s)
                 ymax = y;
         } else {
             int dy;
-            if (page_min != (ram_addr_t)-1) {
-                reset_dirty(s, page_min, page_max);
-                page_min = (ram_addr_t)-1;
-                page_max = 0;
+            if (dirty) {
+                dirty = false;
                 dpy_gfx_update(s->con, xmin, ymin,
                                xmax - xmin + 1, ymax - ymin + 1);
                 xmin = s->width;
@@ -217,9 +204,8 @@ static void g364fb_draw_graphic8(G364State *s)
     }

 done:
-    if (page_min != (ram_addr_t)-1) {
+    if (dirty) {
         dpy_gfx_update(s->con, xmin, ymin, xmax - xmin + 1, ymax - ymin + 1);
-        reset_dirty(s, page_min, page_max);
     }
 }

-- 
1.8.3.1

[Qemu-devel] [PATCH 6/7] framebufer: port to test_and_clear instead of test/reset

[Juan Quintela]

From: Juan Quintela <quintela@trasno.org>

Signed-off-by: Juan Quintela <quintela@trasno.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/display/framebuffer.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/display/framebuffer.c b/hw/display/framebuffer.c
index 4546e42..5369362 100644
--- a/hw/display/framebuffer.c
+++ b/hw/display/framebuffer.c
@@ -88,8 +88,8 @@ void framebuffer_update_display(
     dest += i * dest_row_pitch;

     for (; i < rows; i++) {
-        dirty = memory_region_get_dirty(mem, addr, src_width,
-                                             DIRTY_MEMORY_VGA);
+        dirty = memory_region_test_and_clear_dirty(mem, addr, src_width,
+                                                   DIRTY_MEMORY_VGA);
         if (dirty || invalidate) {
             fn(opaque, dest, src, cols, dest_col_pitch);
             if (first == -1)
@@ -104,8 +104,6 @@ void framebuffer_update_display(
     if (first < 0) {
         goto out;
     }
-    memory_region_reset_dirty(mem, mem_section.offset_within_region, src_len,
-                              DIRTY_MEMORY_VGA);
     *first_row = first;
     *last_row = last;
 out:
-- 
1.8.3.1

[Qemu-devel] [PATCH 7/7] exynox4210: port to test_and_clear instead of test/reset

[Juan Quintela]

From: Juan Quintela <quintela@trasno.org>

Signed-off-by: Juan Quintela <quintela@trasno.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/display/exynos4210_fimd.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 65cca1d..de2ba8b 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1286,7 +1286,7 @@ static void exynos4210_fimd_update(void *opaque)
             fb_line_addr = w->mem_section.offset_within_region;

             for (line = 0; line < scrn_height; line++) {
-                is_dirty = memory_region_get_dirty(w->mem_section.mr,
+                is_dirty = memory_region_test_and_clear_dirty(w->mem_section.mr,
                             fb_line_addr, scrn_width, DIRTY_MEMORY_VGA);

                 if (s->invalidate || is_dirty) {
@@ -1302,9 +1302,6 @@ static void exynos4210_fimd_update(void *opaque)
                 fb_line_addr += inc_size;
                 is_dirty = false;
             }
-            memory_region_reset_dirty(w->mem_section.mr,
-                w->mem_section.offset_within_region,
-                w->fb_len, DIRTY_MEMORY_VGA);
             blend = true;
         }
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/7] vmware_vga: port to test_and_clear instead of test/reset

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

On 10/11/2013 08:20 AM, Juan Quintela wrote:
> From: Juan Quintela <quintela@trasno.org>
> 
> I am assuming that we are testing all the device pages on the loop, that is right?
> 
> Signed-off-by: Juan Quintela <quintela@trasno.org>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Double S-o-B throughout the series looks a bit odd.  Might be worth
using just a preferred address?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 1/7] vmware_vga: port to test_and_clear instead of test/reset

[Juan Quintela]

Eric Blake <eblake@redhat.com> wrote:
> On 10/11/2013 08:20 AM, Juan Quintela wrote:
>> From: Juan Quintela <quintela@trasno.org>
>> 
>> I am assuming that we are testing all the device pages on the loop,
>> that is right?
>> 
>> Signed-off-by: Juan Quintela <quintela@trasno.org>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> Double S-o-B throughout the series looks a bit odd.  Might be worth
> using just a preferred address?

Fixed locally just after send.
Setup is that repositories by default get my local address,  and then I
switched them to my redhat one for the official ones.  Clearly,  my
memory failed me here.  Sorry :-(