Re: [Qemu-devel] [PATCH] qxl: properly handle upright and non-shared surfaces

2012-02-28 Thread Gerd Hoffmann 
 -dprint(qxl, 1, %s: stride %d, [%d, %d, %d, %d]\n, __func__,
 +dprint(qxl, 2, %s: stride %d, [%d, %d, %d, %d]\n, __func__,
 
 You know 2 is used right now for high frequency stuff, like
 interface_get_command? I think this should be lower. Anyway, not a big
 deal.

/me used '1' for important but infrequent stuff, basically all init ops,
mode switching etc.  This can happen quite alot in case you are running
with SDL.  Maybe we need not just 1+2 but 1+2+3 levels, with 3 for the
really frequent stuff which will flood the logfiles.

Or even better just turn them all into tracepoints ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qxl: properly handle upright and non-shared surfaces

2012-02-28 Thread Alon Levy 
On Tue, Feb 28, 2012 at 09:29:38AM +0100, Gerd Hoffmann wrote:
  -dprint(qxl, 1, %s: stride %d, [%d, %d, %d, %d]\n, __func__,
  +dprint(qxl, 2, %s: stride %d, [%d, %d, %d, %d]\n, __func__,
  
  You know 2 is used right now for high frequency stuff, like
  interface_get_command? I think this should be lower. Anyway, not a big
  deal.
 
 /me used '1' for important but infrequent stuff, basically all init ops,
 mode switching etc.  This can happen quite alot in case you are running
 with SDL.  Maybe we need not just 1+2 but 1+2+3 levels, with 3 for the
 really frequent stuff which will flood the logfiles.
 
 Or even better just turn them all into tracepoints ...
 

Yeah, I started working on this but didn't finish. I'll try to do it.

 cheers,
   Gerd

[Qemu-devel] [PATCH] qxl: properly handle upright and non-shared surfaces

2012-02-27 Thread Gerd Hoffmann 
Although qxl creates a shared displaysurface when the qxl surface is
upright and doesn't need to be flipped there is no guarantee that the
surface doesn't become unshared for some reason.  Rename qxl_flip to
qxl_blit and fix it to handle both flip and non-flip cases.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/qxl-render.c |   20 +---
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index f323a4d..25857f6 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -21,25 +21,31 @@
 
 #include qxl.h
 
-static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect)
+static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
 {
 uint8_t *src;
 uint8_t *dst = qxl-vga.ds-surface-data;
 int len, i;
 
-if (qxl-guest_primary.qxl_stride  0) {
+if (is_buffer_shared(qxl-vga.ds-surface)) {
 return;
 }
 if (!qxl-guest_primary.data) {
 dprint(qxl, 1, %s: initializing guest_primary.data\n, __func__);
 qxl-guest_primary.data = memory_region_get_ram_ptr(qxl-vga.vram);
 }
-dprint(qxl, 1, %s: stride %d, [%d, %d, %d, %d]\n, __func__,
+dprint(qxl, 2, %s: stride %d, [%d, %d, %d, %d]\n, __func__,
 qxl-guest_primary.qxl_stride,
 rect-left, rect-right, rect-top, rect-bottom);
 src = qxl-guest_primary.data;
-src += (qxl-guest_primary.surface.height - rect-top - 1) *
-qxl-guest_primary.abs_stride;
+if (qxl-guest_primary.qxl_stride  0) {
+/* qxl surface is upside down, walk src scanlines
+ * in reverse order to flip it */
+src += (qxl-guest_primary.surface.height - rect-top - 1) *
+qxl-guest_primary.abs_stride;
+} else {
+src += rect-top * qxl-guest_primary.abs_stride;
+}
 dst += rect-top  * qxl-guest_primary.abs_stride;
 src += rect-left * qxl-guest_primary.bytes_pp;
 dst += rect-left * qxl-guest_primary.bytes_pp;
@@ -48,7 +54,7 @@ static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect)
 for (i = rect-top; i  rect-bottom; i++) {
 memcpy(dst, src, len);
 dst += qxl-guest_primary.abs_stride;
-src -= qxl-guest_primary.abs_stride;
+src += qxl-guest_primary.qxl_stride;
 }
 }
 
@@ -132,7 +138,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice 
*qxl)
 if (qemu_spice_rect_is_empty(qxl-dirty+i)) {
 break;
 }
-qxl_flip(qxl, qxl-dirty+i);
+qxl_blit(qxl, qxl-dirty+i);
 dpy_update(vga-ds,
qxl-dirty[i].left, qxl-dirty[i].top,
qxl-dirty[i].right - qxl-dirty[i].left,
-- 
1.7.1

Re: [Qemu-devel] [PATCH] qxl: properly handle upright and non-shared surfaces

2012-02-27 Thread Alon Levy 
On Mon, Feb 27, 2012 at 11:10:10AM +0100, Gerd Hoffmann wrote:
 Although qxl creates a shared displaysurface when the qxl surface is
 upright and doesn't need to be flipped there is no guarantee that the
 surface doesn't become unshared for some reason.  Rename qxl_flip to
 qxl_blit and fix it to handle both flip and non-flip cases.

Reviewed-by: Alon Levy al...@redhat.com

just a few nitpicks.

 
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
  hw/qxl-render.c |   20 +---
  1 files changed, 13 insertions(+), 7 deletions(-)
 
 diff --git a/hw/qxl-render.c b/hw/qxl-render.c
 index f323a4d..25857f6 100644
 --- a/hw/qxl-render.c
 +++ b/hw/qxl-render.c
 @@ -21,25 +21,31 @@
  
  #include qxl.h
  
 -static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect)
 +static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
  {
  uint8_t *src;
  uint8_t *dst = qxl-vga.ds-surface-data;
  int len, i;
  
 -if (qxl-guest_primary.qxl_stride  0) {
 +if (is_buffer_shared(qxl-vga.ds-surface)) {

ok, /me regrets not liking this predicate and using qxl_stride  0
instead.

  return;
  }
  if (!qxl-guest_primary.data) {
  dprint(qxl, 1, %s: initializing guest_primary.data\n, __func__);
  qxl-guest_primary.data = memory_region_get_ram_ptr(qxl-vga.vram);
  }
 -dprint(qxl, 1, %s: stride %d, [%d, %d, %d, %d]\n, __func__,
 +dprint(qxl, 2, %s: stride %d, [%d, %d, %d, %d]\n, __func__,

You know 2 is used right now for high frequency stuff, like
interface_get_command? I think this should be lower. Anyway, not a big
deal.

  qxl-guest_primary.qxl_stride,
  rect-left, rect-right, rect-top, rect-bottom);
  src = qxl-guest_primary.data;
 -src += (qxl-guest_primary.surface.height - rect-top - 1) *
 -qxl-guest_primary.abs_stride;
 +if (qxl-guest_primary.qxl_stride  0) {
 +/* qxl surface is upside down, walk src scanlines
 + * in reverse order to flip it */

This passed checkpatch.pl? I demand equal mistreatment! :)

 +src += (qxl-guest_primary.surface.height - rect-top - 1) *
 +qxl-guest_primary.abs_stride;
 +} else {
 +src += rect-top * qxl-guest_primary.abs_stride;
 +}
  dst += rect-top  * qxl-guest_primary.abs_stride;
  src += rect-left * qxl-guest_primary.bytes_pp;
  dst += rect-left * qxl-guest_primary.bytes_pp;
 @@ -48,7 +54,7 @@ static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect)
  for (i = rect-top; i  rect-bottom; i++) {
  memcpy(dst, src, len);
  dst += qxl-guest_primary.abs_stride;
 -src -= qxl-guest_primary.abs_stride;
 +src += qxl-guest_primary.qxl_stride;
  }
  }
  
 @@ -132,7 +138,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice 
 *qxl)
  if (qemu_spice_rect_is_empty(qxl-dirty+i)) {
  break;
  }
 -qxl_flip(qxl, qxl-dirty+i);
 +qxl_blit(qxl, qxl-dirty+i);
  dpy_update(vga-ds,
 qxl-dirty[i].left, qxl-dirty[i].top,
 qxl-dirty[i].right - qxl-dirty[i].left,
 -- 
 1.7.1