Re: [Qemu-devel] [PATCH] vmware_vga: Cleanup and allow simple drivers to work without the fifo
On Wed, Aug 22, 2012 at 05:23:49PM +0200, Jan Kiszka wrote: On 2012-08-22 12:19, BALATON Zoltan wrote: On Wed, 22 Aug 2012, Jan Kiszka wrote: This is a rather big patch. I strongly suspect you can break it up into smaller pieces that address separate aspects one-by-one. Also, it is definitely to heavy for qemu-trivial. Despite its size the changes included are fairly simple but I can try to break it up. I've sent it to qemu-trivial because it may meet the Do not fall under an actively maintained subsystem. But that doesn't make it trivial, rather harder (if no one reviewed it). category as I've found no maintainer for this part. Should I send the revisions only to qemu-devel then? Yes, that's more appropriate IMHO. Yes, thanks. I don't know how this device is supposed to work and the patch isn't trivial, so I can't apply this to qemu-trivial. Stefan
Re: [Qemu-devel] [PATCH] vmware_vga: Cleanup and allow simple drivers to work without the fifo
On 2012-08-21 23:33, BALATON Zoltan wrote:
Detailed changes: Removing info available elsewhere from vmsvga_state.
Fix mixup between depth and bits per pixel. Return a value for FB_SIZE
even before enabled (according to the documentation, drivers should
read this value before enabling the device). 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 but the
DIRECT_VRAM option meant to support this case was removed a year ago.)
This is a rather big patch. I strongly suspect you can break it up into
smaller pieces that address separate aspects one-by-one. Also, it is
definitely to heavy for qemu-trivial.
Jan
Signed-off-by: BALATON Zoltan bala...@eik.bme.hu
---
console.h | 20 +
hw/vga.c|2 +-
hw/vga_int.h|1 +
hw/vmware_vga.c | 243
++-
4 files changed, 137 insertions(+), 129 deletions(-)
diff --git a/console.h b/console.h
index 4334db5..00baf5b 100644
--- a/console.h
+++ b/console.h
@@ -328,6 +328,26 @@ static inline int
ds_get_bytes_per_pixel(DisplayState *ds)
return ds-surface-pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds-surface-pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds-surface-pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds-surface-pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds-surface-pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include curses.h
typedef chtype console_ch_t;
diff --git a/hw/vga.c b/hw/vga.c
index f82ced8..7e6dc41 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1611,7 +1611,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 8938093..16d53ef 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -195,6 +195,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 f5e4f44..2b77766 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -32,16 +32,19 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include vga_int.h
+#include vga_int.h
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
+/* -*- The members marked are now unused and could be removed but they are
+ * contained in the VMState thus need special handling. Maybe they
could
+ * be removed the next time a new machine type is added.
+ */
int invalidated;
-int depth;
-int bypp;
+int depth; /* -*- */
int enable;
int config;
struct {
@@ -58,11 +61,8 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
-int fb_size;
+int fb_size; /* -*- */
MemoryRegion fifo_ram;
uint8_t *fifo_ptr;
@@ -298,40 +298,33 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w s-width) {
+if (x + w ds_get_width(s-vga.ds)) {
fprintf(stderr, %s: update width too large x: %d, w: %d\n,
-__FUNCTION__, x, w);
-x = MIN(x, s-width);
-w = s-width - x;
+__func__, x, w);
+x = MIN(x, ds_get_width(s-vga.ds));
+w = ds_get_width(s-vga.ds) - x;
}
-if (y + h s-height) {
+if (y + h ds_get_height(s-vga.ds)) {
fprintf(stderr, %s: update height too large y: %d, h: %d\n,
-__FUNCTION__, y, h);
-y = MIN(y, s-height);
-h = s-height - y;
+__func__, y, h);
+y = MIN(y, ds_get_height(s-vga.ds));
+h = ds_get_height(s-vga.ds) - y;
}
-line = h;
-bypl = s-bypp * s-width;
-width = s-bypp * w;
-start = s-bypp * x + bypl * y;
+bypl = ds_get_linesize(s-vga.ds);
+width = ds_get_bytes_per_pixel(s-vga.ds) * w;
+start = ds_get_bytes_per_pixel(s-vga.ds) * x + bypl * y;
src = s-vga.vram_ptr + start;
dst = ds_get_data(s-vga.ds) + start;
-for (; line 0; line --, src += bypl, dst += bypl)
+for (line = h; line 0; line--, src += bypl, dst += bypl)
Re: [Qemu-devel] [PATCH] vmware_vga: Cleanup and allow simple drivers to work without the fifo
On Wed, 22 Aug 2012, Jan Kiszka wrote: This is a rather big patch. I strongly suspect you can break it up into smaller pieces that address separate aspects one-by-one. Also, it is definitely to heavy for qemu-trivial. Despite its size the changes included are fairly simple but I can try to break it up. I've sent it to qemu-trivial because it may meet the Do not fall under an actively maintained subsystem. category as I've found no maintainer for this part. Should I send the revisions only to qemu-devel then? Thanks, BALATON Zoltan
Re: [Qemu-devel] [PATCH] vmware_vga: Cleanup and allow simple drivers to work without the fifo
On 2012-08-22 12:19, BALATON Zoltan wrote: On Wed, 22 Aug 2012, Jan Kiszka wrote: This is a rather big patch. I strongly suspect you can break it up into smaller pieces that address separate aspects one-by-one. Also, it is definitely to heavy for qemu-trivial. Despite its size the changes included are fairly simple but I can try to break it up. I've sent it to qemu-trivial because it may meet the Do not fall under an actively maintained subsystem. But that doesn't make it trivial, rather harder (if no one reviewed it). category as I've found no maintainer for this part. Should I send the revisions only to qemu-devel then? Yes, that's more appropriate IMHO. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] vmware_vga: Cleanup and allow simple drivers to work without the fifo
Detailed changes: Removing info available elsewhere from vmsvga_state.
Fix mixup between depth and bits per pixel. Return a value for FB_SIZE
even before enabled (according to the documentation, drivers should
read this value before enabling the device). 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 but the
DIRECT_VRAM option meant to support this case was removed a year ago.)
Signed-off-by: BALATON Zoltan bala...@eik.bme.hu
---
console.h | 20 +
hw/vga.c|2 +-
hw/vga_int.h|1 +
hw/vmware_vga.c | 243 ++-
4 files changed, 137 insertions(+), 129 deletions(-)
diff --git a/console.h b/console.h
index 4334db5..00baf5b 100644
--- a/console.h
+++ b/console.h
@@ -328,6 +328,26 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds)
return ds-surface-pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds-surface-pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds-surface-pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds-surface-pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds-surface-pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include curses.h
typedef chtype console_ch_t;
diff --git a/hw/vga.c b/hw/vga.c
index f82ced8..7e6dc41 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1611,7 +1611,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 8938093..16d53ef 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -195,6 +195,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 f5e4f44..2b77766 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -32,16 +32,19 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include vga_int.h
+#include vga_int.h
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
+/* -*- The members marked are now unused and could be removed but they are
+ * contained in the VMState thus need special handling. Maybe they could
+ * be removed the next time a new machine type is added.
+ */
int invalidated;
-int depth;
-int bypp;
+int depth; /* -*- */
int enable;
int config;
struct {
@@ -58,11 +61,8 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
-int fb_size;
+int fb_size; /* -*- */
MemoryRegion fifo_ram;
uint8_t *fifo_ptr;
@@ -298,40 +298,33 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w s-width) {
+if (x + w ds_get_width(s-vga.ds)) {
fprintf(stderr, %s: update width too large x: %d, w: %d\n,
-__FUNCTION__, x, w);
-x = MIN(x, s-width);
-w = s-width - x;
+__func__, x, w);
+x = MIN(x, ds_get_width(s-vga.ds));
+w = ds_get_width(s-vga.ds) - x;
}
-if (y + h s-height) {
+if (y + h ds_get_height(s-vga.ds)) {
fprintf(stderr, %s: update height too large y: %d, h: %d\n,
-__FUNCTION__, y, h);
-y = MIN(y, s-height);
-h = s-height - y;
+__func__, y, h);
+y = MIN(y, ds_get_height(s-vga.ds));
+h = ds_get_height(s-vga.ds) - y;
}
-line = h;
-bypl = s-bypp * s-width;
-width = s-bypp * w;
-start = s-bypp * x + bypl * y;
+bypl = ds_get_linesize(s-vga.ds);
+width = ds_get_bytes_per_pixel(s-vga.ds) * w;
+start = ds_get_bytes_per_pixel(s-vga.ds) * x + bypl * y;
src = s-vga.vram_ptr + start;
dst = ds_get_data(s-vga.ds) + start;
-for (; line 0; line --, src += bypl, dst += bypl)
+for (line = h; line 0; line--, src += bypl, dst += bypl) {
memcpy(dst, src, width);
+}
dpy_update(s-vga.ds, x, y, w, h);
}
-static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
-{
-memcpy(ds_get_data(s-vga.ds), s-vga.vram_ptr,
- s-bypp * s-width * s-height);
-dpy_update(s-vga.ds, 0, 0, s-width, s-height);
-}
-
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
