Re: [Qemu-devel] [PATCH] qxl: properly handle upright and non-shared surfaces
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 > >
Re: [Qemu-devel] [PATCH] qxl: properly handle upright and non-shared surfaces
>> -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
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 just a few nitpicks. > > Signed-off-by: Gerd Hoffmann > --- > 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 > >
[Qemu-devel] [PATCH] qxl: properly handle upright and non-shared surfaces
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 --- 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