Patchwork [3/3,v2] vmware_vga: Allow simple drivers to work without using the fifo

login
register
mail settings
Submitter BALATON Zoltan
Date Oct. 3, 2012, 9:31 a.m.
Message ID <alpine.GSO.2.00.1210031130550.19707@mono>
Download mbox | patch
Permalink /patch/188741/
State New
Headers show

Comments

BALATON Zoltan - Oct. 3, 2012, 9:31 a.m.
Postpone stopping the dirty log to the point where the command fifo is
configured to allow drivers which don't use the fifo to work too.
(Without this the picture rendered into the vram never got to the
screen and the DIRECT_VRAM option meant to support this case was
removed a year ago.)

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
  hw/vga.c        |    2 +-
  hw/vga_int.h    |    1 +
  hw/vmware_vga.c |   34 +++++++++++++++++++++++++---------
  3 files changed, 27 insertions(+), 10 deletions(-)

  v2: Rebase to apply to current
Paolo Bonzini - Oct. 3, 2012, 9:46 a.m.
Il 03/10/2012 11:31, BALATON Zoltan ha scritto:
> Postpone stopping the dirty log to the point where the command fifo is
> configured to allow drivers which don't use the fifo to work too.
> (Without this the picture rendered into the vram never got to the
> screen and the DIRECT_VRAM option meant to support this case was
> removed a year ago.)
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/vga.c        |    2 +-
>  hw/vga_int.h    |    1 +
>  hw/vmware_vga.c |   34 +++++++++++++++++++++++++---------
>  3 files changed, 27 insertions(+), 10 deletions(-)
> 
>  v2: Rebase to apply to current
> 
> diff --git a/hw/vga.c b/hw/vga.c
> index 80299ea..7939a9d 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -1612,7 +1612,7 @@ void vga_invalidate_scanlines(VGACommonState *s,
> int y1, int y2)
>      }
>  }
> 
> -static void vga_sync_dirty_bitmap(VGACommonState *s)
> +void vga_sync_dirty_bitmap(VGACommonState *s)
>  {
>      memory_region_sync_dirty_bitmap(&s->vram);
>  }
> diff --git a/hw/vga_int.h b/hw/vga_int.h
> index 330a32f..c01d07d 100644
> --- a/hw/vga_int.h
> +++ b/hw/vga_int.h
> @@ -196,6 +196,7 @@ MemoryRegion *vga_init_io(VGACommonState *s,
>                            const MemoryRegionPortio **vbe_ports);
>  void vga_common_reset(VGACommonState *s);
> 
> +void vga_sync_dirty_bitmap(VGACommonState *s);
>  void vga_dirty_log_start(VGACommonState *s);
>  void vga_dirty_log_stop(VGACommonState *s);
> 
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index 5e4786f..17b3140 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -820,11 +820,10 @@ static void vmsvga_value_write(void *opaque,
> uint32_t address, uint32_t value)
>          break;
> 
>      case SVGA_REG_ENABLE:
> -        s->enable = value;
> -        s->config &= !!value;
> +        s->enable = !!value;
>          s->invalidated = 1;
>          s->vga.invalidate(&s->vga);
> -        if (s->enable) {
> +        if (s->enable && s->config) {
>              vga_dirty_log_stop(&s->vga);
>          } else {
>              vga_dirty_log_start(&s->vga);
> @@ -860,15 +859,19 @@ static void vmsvga_value_write(void *opaque,
> uint32_t address, uint32_t value)
>          if (value) {
>              s->fifo = (uint32_t *) s->fifo_ptr;
>              /* Check range and alignment.  */
> -            if ((CMD(min) | CMD(max) |
> -                        CMD(next_cmd) | CMD(stop)) & 3)
> +            if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) {
>                  break;
> -            if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo)
> +            }
> +            if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *)
> s->fifo) {
>                  break;
> -            if (CMD(max) > SVGA_FIFO_SIZE)
> +            }
> +            if (CMD(max) > SVGA_FIFO_SIZE) {
>                  break;
> -            if (CMD(max) < CMD(min) + 10 * 1024)
> +            }
> +            if (CMD(max) < CMD(min) + 10 * 1024) {
>                  break;
> +            }

Please keep all coding style changes in a separate patch (so you have
one patch for coding style changes in the whole series).

> +            vga_dirty_log_stop(&s->vga);
>          }
>          s->config = !!value;
>          break;
> @@ -949,6 +952,8 @@ static inline void vmsvga_check_size(struct
> vmsvga_state_s *s)
>  static void vmsvga_update_display(void *opaque)
>  {
>      struct vmsvga_state_s *s = opaque;
> +    bool dirty = false;
> +
>      if (!s->enable) {
>          s->vga.update(&s->vga);
>          return;
> @@ -963,13 +968,24 @@ static void vmsvga_update_display(void *opaque)
>       * Is it more efficient to look at vram VGA-dirty bits or wait
>       * for the driver to issue SVGA_CMD_UPDATE?
>       */
> -    if (s->invalidated) {
> +    if (memory_region_is_logging(&s->vga.vram)) {
> +        vga_sync_dirty_bitmap(&s->vga);
> +        dirty = memory_region_get_dirty(&s->vga.vram, 0,
> +            ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
> +            DIRTY_MEMORY_VGA);
> +    }
> +    if (s->invalidated || dirty) {
>          s->invalidated = 0;
>          memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
>                 ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
>          dpy_update(s->vga.ds, 0, 0,
>                 ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
>      }
> +    if (dirty) {
> +        memory_region_reset_dirty(&s->vga.vram, 0,
> +            ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
> +            DIRTY_MEMORY_VGA);
> +    }
>  }
> 
>  static void vmsvga_reset(DeviceState *dev)

Otherwise looks good.

Paolo

Patch

diff --git a/hw/vga.c b/hw/vga.c
index 80299ea..7939a9d 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1612,7 +1612,7 @@  void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2)
      }
  }

-static void vga_sync_dirty_bitmap(VGACommonState *s)
+void vga_sync_dirty_bitmap(VGACommonState *s)
  {
      memory_region_sync_dirty_bitmap(&s->vram);
  }
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 330a32f..c01d07d 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -196,6 +196,7 @@  MemoryRegion *vga_init_io(VGACommonState *s,
                            const MemoryRegionPortio **vbe_ports);
  void vga_common_reset(VGACommonState *s);

+void vga_sync_dirty_bitmap(VGACommonState *s);
  void vga_dirty_log_start(VGACommonState *s);
  void vga_dirty_log_stop(VGACommonState *s);

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 5e4786f..17b3140 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -820,11 +820,10 @@  static void vmsvga_value_write(void *opaque, uint32_t address, uint32_t value)
          break;

      case SVGA_REG_ENABLE:
-        s->enable = value;
-        s->config &= !!value;
+        s->enable = !!value;
          s->invalidated = 1;
          s->vga.invalidate(&s->vga);
-        if (s->enable) {
+        if (s->enable && s->config) {
              vga_dirty_log_stop(&s->vga);
          } else {
              vga_dirty_log_start(&s->vga);
@@ -860,15 +859,19 @@  static void vmsvga_value_write(void *opaque, uint32_t address, uint32_t value)
          if (value) {
              s->fifo = (uint32_t *) s->fifo_ptr;
              /* Check range and alignment.  */
-            if ((CMD(min) | CMD(max) |
-                        CMD(next_cmd) | CMD(stop)) & 3)
+            if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) {
                  break;
-            if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo)
+            }
+            if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) {
                  break;
-            if (CMD(max) > SVGA_FIFO_SIZE)
+            }
+            if (CMD(max) > SVGA_FIFO_SIZE) {
                  break;
-            if (CMD(max) < CMD(min) + 10 * 1024)
+            }
+            if (CMD(max) < CMD(min) + 10 * 1024) {
                  break;
+            }
+            vga_dirty_log_stop(&s->vga);
          }
          s->config = !!value;
          break;
@@ -949,6 +952,8 @@  static inline void vmsvga_check_size(struct vmsvga_state_s *s)
  static void vmsvga_update_display(void *opaque)
  {
      struct vmsvga_state_s *s = opaque;
+    bool dirty = false;
+
      if (!s->enable) {
          s->vga.update(&s->vga);
          return;
@@ -963,13 +968,24 @@  static void vmsvga_update_display(void *opaque)
       * Is it more efficient to look at vram VGA-dirty bits or wait
       * for the driver to issue SVGA_CMD_UPDATE?
       */
-    if (s->invalidated) {
+    if (memory_region_is_logging(&s->vga.vram)) {
+        vga_sync_dirty_bitmap(&s->vga);
+        dirty = memory_region_get_dirty(&s->vga.vram, 0,
+            ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+            DIRTY_MEMORY_VGA);
+    }
+    if (s->invalidated || dirty) {
          s->invalidated = 0;
          memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
                 ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
          dpy_update(s->vga.ds, 0, 0,
                 ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
      }
+    if (dirty) {
+        memory_region_reset_dirty(&s->vga.vram, 0,
+            ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+            DIRTY_MEMORY_VGA);
+    }
  }

  static void vmsvga_reset(DeviceState *dev)