From c26805e29312fee136008a57c70a2f5f140ba706 Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Fri, 24 Feb 2012 23:19:31 +0200 Subject: [PATCH 418/434] qxl: make qxl_render_update async RHBZ# 747011 Removes the last user of QXL_SYNC when using update drivers that use the _ASYNC io ports. The last user is qxl_render_update, it is called both by qxl_hw_update which is the vga_hw_update_ptr passed to graphic_console_init, and by qxl_hw_screen_dump. At the same time the QXLRect area being passed to the red_worker thread is passed as a copy, as part of the QXLCookie. The implementation uses interface_update_area_complete with a bh to make sure dpy_update and qxl_flip are called from the io thread, otherwise the vga->ds->surface.data can change under our feet. With this patch sdl+spice works fine. But spice by itself doesn't produce the expected screendumps unless repeated a few times, due to ppm_save being called before update_area (rendering done in spice server thread) having a chance to complete. Fixed by next patch, but see commit message for problem introduced by it. Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- hw/qxl-render.c | 96 +++++++++++++++++++++++++++++++++++++--------------- hw/qxl.c | 69 ++++++++++++++++++++++++++++++++++--- hw/qxl.h | 10 ++++++ ui/spice-display.h | 6 ++++ 4 files changed, 150 insertions(+), 31 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index 4c22166..4857838 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -82,17 +82,25 @@ void qxl_render_resize(PCIQXLDevice *qxl) } } -void qxl_render_update(PCIQXLDevice *qxl) +static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area) +{ + area->left = 0; + area->right = qxl->guest_primary.surface.width; + area->top = 0; + area->bottom = qxl->guest_primary.surface.height; +} + +static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl) { VGACommonState *vga = &qxl->vga; - QXLRect dirty[32], update; - int i, redraw = 0; + int i; DisplaySurface *surface = vga->ds->surface; if (qxl->guest_primary.resized) { qxl->guest_primary.resized = 0; - qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram); + qxl_set_rect_to_surface(qxl, &qxl->dirty[0]); + qxl->num_dirty_rects = 1; dprint(qxl, 1, "%s: %dx%d, stride %d, bpp %d, depth %d\n", __FUNCTION__, qxl->guest_primary.surface.width, @@ -103,9 +111,9 @@ void qxl_render_update(PCIQXLDevice *qxl) } if (surface->width != qxl->guest_primary.surface.width || surface->height != qxl->guest_primary.surface.height) { - dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n", - __func__); if (qxl->guest_primary.qxl_stride > 0) { + dprint(qxl, 1, "%s: using guest_primary for displaysurface\n", + __func__); qemu_free_displaysurface(vga->ds); qemu_create_displaysurface_from(qxl->guest_primary.surface.width, qxl->guest_primary.surface.height, @@ -113,36 +121,70 @@ void qxl_render_update(PCIQXLDevice *qxl) qxl->guest_primary.abs_stride, qxl->guest_primary.data); } else { + dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n", + __func__); qemu_resize_displaysurface(vga->ds, qxl->guest_primary.surface.width, qxl->guest_primary.surface.height); } } - update.left = 0; - update.right = qxl->guest_primary.surface.width; - update.top = 0; - update.bottom = qxl->guest_primary.surface.height; - - memset(dirty, 0, sizeof(dirty)); - if (runstate_is_running() && qxl->guest_primary.commands) { - qxl->guest_primary.commands = 0; - qxl_spice_update_area(qxl, 0, &update, - dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL); - } - if (redraw) { - memset(dirty, 0, sizeof(dirty)); - dirty[0] = update; - } - for (i = 0; i < ARRAY_SIZE(dirty); i++) { - if (qemu_spice_rect_is_empty(dirty+i)) { + for (i = 0; i < qxl->num_dirty_rects; i++) { + if (qemu_spice_rect_is_empty(qxl->dirty+i)) { break; } - qxl_flip(qxl, dirty+i); + qxl_flip(qxl, qxl->dirty+i); dpy_update(vga->ds, - dirty[i].left, dirty[i].top, - dirty[i].right - dirty[i].left, - dirty[i].bottom - dirty[i].top); + qxl->dirty[i].left, qxl->dirty[i].top, + qxl->dirty[i].right - qxl->dirty[i].left, + qxl->dirty[i].bottom - qxl->dirty[i].top); + } + qxl->num_dirty_rects = 0; +} + +/* + * use ssd.lock to protect render_update_cookie_num. + * qxl_render_update is called by io thread or vcpu thread, and the completion + * callbacks are called by spice_server thread, defering to bh called from the + * io thread. + */ +void qxl_render_update(PCIQXLDevice *qxl) +{ + QXLCookie *cookie; + + qemu_mutex_lock(&qxl->ssd.lock); + + if (!runstate_is_running() || !qxl->guest_primary.commands) { + qxl_render_update_area_unlocked(qxl); + qemu_mutex_unlock(&qxl->ssd.lock); + return; } + + qxl->guest_primary.commands = 0; + qxl->render_update_cookie_num++; + qemu_mutex_unlock(&qxl->ssd.lock); + cookie = qxl_cookie_new(QXL_COOKIE_TYPE_RENDER_UPDATE_AREA, + 0); + qxl_set_rect_to_surface(qxl, &cookie->u.render.area); + qxl_spice_update_area(qxl, 0, &cookie->u.render.area, NULL, + 0, 1 /* clear_dirty_region */, QXL_ASYNC, cookie); +} + +void qxl_render_update_area_bh(void *opaque) +{ + PCIQXLDevice *qxl = opaque; + + qemu_mutex_lock(&qxl->ssd.lock); + qxl_render_update_area_unlocked(qxl); + qemu_mutex_unlock(&qxl->ssd.lock); +} + +void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie) +{ + qemu_mutex_lock(&qxl->ssd.lock); + qemu_bh_schedule(qxl->update_area_bh); + qxl->render_update_cookie_num--; + qemu_mutex_unlock(&qxl->ssd.lock); + g_free(cookie); } static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor) diff --git a/hw/qxl.c b/hw/qxl.c index e6e65d9..73be115 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -750,6 +750,11 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie) __func__, current_async, cookie->io); } switch (current_async) { + case QXL_IO_MEMSLOT_ADD_ASYNC: + case QXL_IO_DESTROY_PRIMARY_ASYNC: + case QXL_IO_UPDATE_AREA_ASYNC: + case QXL_IO_FLUSH_SURFACES_ASYNC: + break; case QXL_IO_CREATE_PRIMARY_ASYNC: qxl_create_guest_primary_complete(qxl); break; @@ -759,11 +764,54 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie) case QXL_IO_DESTROY_SURFACE_ASYNC: qxl_spice_destroy_surface_wait_complete(qxl, cookie->u.surface_id); break; + default: + fprintf(stderr, "qxl: %s: unexpected current_async %d\n", __func__, + current_async); } qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD); } /* called from spice server thread context only */ +static void interface_update_area_complete(QXLInstance *sin, + uint32_t surface_id, + QXLRect *dirty, uint32_t num_updated_rects) +{ + PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); + int i; + int qxl_i; + + qemu_mutex_lock(&qxl->ssd.lock); + if (surface_id != 0 || !qxl->render_update_cookie_num) { + qemu_mutex_unlock(&qxl->ssd.lock); + return; + } + if (qxl->num_dirty_rects + num_updated_rects > QXL_NUM_DIRTY_RECTS) { + /* + * overflow - treat this as a full update. Not expected to be common. + */ + dprint(qxl, 1, "%s: overflow of dirty rects\n", __func__); + qxl->guest_primary.resized = 1; + } + if (qxl->guest_primary.resized) { + /* + * Don't bother copying or scheduling the bh since we will flip + * the whole area anyway on completion of the update_area async call + */ + qemu_mutex_unlock(&qxl->ssd.lock); + return; + } + qxl_i = qxl->num_dirty_rects; + for (i = 0; i < num_updated_rects; i++) { + qxl->dirty[qxl_i++] = dirty[i]; + } + qxl->num_dirty_rects += num_updated_rects; + dprint(qxl, 1, "%s: scheduling update_area_bh, #dirty %d\n", + __func__, qxl->num_dirty_rects); + qemu_bh_schedule(qxl->update_area_bh); + qemu_mutex_unlock(&qxl->ssd.lock); +} + +/* called from spice server thread context only */ static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token) { PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); @@ -772,12 +820,16 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token) switch (cookie->type) { case QXL_COOKIE_TYPE_IO: interface_async_complete_io(qxl, cookie); + g_free(cookie); + break; + case QXL_COOKIE_TYPE_RENDER_UPDATE_AREA: + qxl_render_update_area_done(qxl, cookie); break; default: fprintf(stderr, "qxl: %s: unexpected cookie type %d\n", __func__, cookie->type); + g_free(cookie); } - g_free(cookie); } static const QXLInterface qxl_interface = { @@ -800,6 +852,7 @@ static const QXLInterface qxl_interface = { .notify_update = interface_notify_update, .flush_resources = interface_flush_resources, .async_complete = interface_async_complete, + .update_area_complete = interface_update_area_complete, }; static void qxl_enter_vga_mode(PCIQXLDevice *d) @@ -1216,11 +1269,17 @@ async_common: switch (io_port) { case QXL_IO_UPDATE_AREA: { + QXLCookie *cookie = NULL; QXLRect update = d->ram->update_area; + + if (async == QXL_ASYNC) { + cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_UPDATE_AREA_ASYNC); + cookie->u.area = update; + } qxl_spice_update_area(d, d->ram->update_surface, - &update, NULL, 0, 0, async, - qxl_cookie_new(QXL_COOKIE_TYPE_IO, - QXL_IO_UPDATE_AREA_ASYNC)); + cookie ? &cookie->u.area : &update, + NULL, 0, 0, async, cookie); break; } case QXL_IO_NOTIFY_CMD: @@ -1652,6 +1711,8 @@ static int qxl_init_common(PCIQXLDevice *qxl) init_pipe_signaling(qxl); qxl_reset_state(qxl); + qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl); + return 0; } diff --git a/hw/qxl.h b/hw/qxl.h index 1443925..86e415b 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -18,6 +18,8 @@ enum qxl_mode { #define QXL_UNDEFINED_IO UINT32_MAX +#define QXL_NUM_DIRTY_RECTS 64 + typedef struct PCIQXLDevice { PCIDevice pci; SimpleSpiceDisplay ssd; @@ -93,6 +95,12 @@ typedef struct PCIQXLDevice { /* user-friendly properties (in megabytes) */ uint32_t ram_size_mb; uint32_t vram_size_mb; + + /* qxl_render_update state */ + int render_update_cookie_num; + int num_dirty_rects; + QXLRect dirty[QXL_NUM_DIRTY_RECTS]; + QEMUBH *update_area_bh; } PCIQXLDevice; #define PANIC_ON(x) if ((x)) { \ @@ -134,3 +142,5 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext); void qxl_render_resize(PCIQXLDevice *qxl); void qxl_render_update(PCIQXLDevice *qxl); void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext); +void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie); +void qxl_render_update_area_bh(void *opaque); diff --git a/ui/spice-display.h b/ui/spice-display.h index 8a010cb..12e50b6 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -50,6 +50,7 @@ typedef enum qxl_async_io { enum { QXL_COOKIE_TYPE_IO, + QXL_COOKIE_TYPE_RENDER_UPDATE_AREA, }; typedef struct QXLCookie { @@ -57,6 +58,11 @@ typedef struct QXLCookie { uint64_t io; union { uint32_t surface_id; + QXLRect area; + struct { + QXLRect area; + int redraw; + } render; } u; } QXLCookie; -- 1.7.10