From 4645bd6f67e630f9f519cde1bececc20067dd64a Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Sun, 18 Mar 2012 12:17:51 +0100 Subject: [PATCH 8/9] qxl: make qxl_render_update async RH-Author: Alon Levy Message-id: <1332073072-27934-8-git-send-email-alevy@redhat.com> Patchwork-id: 38605 O-Subject: [PATCHv2 RHEL-6.3 qemu-kvm 7/8] qxl: make qxl_render_update async Bugzilla: 747011 RH-Acked-by: Gerd Hoffmann RH-Acked-by: Hans de Goede RH-Acked-by: Yonit Halperin 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 (cherry picked from commit 81fb6f1504fb9ef71f2382f44af34756668296e8) Conflicts: hw/qxl-render.c hw/qxl.h RHEL changes: merge conflicts because of qemu_get_ram_ptr usage instead of memory_region_get_ram_ptr, missing vram32, and added rhel6_qxl_screendump. --- 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(-) Signed-off-by: Michal Novotny --- 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 19c9449..db410a1 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 = qemu_get_ram_ptr(qxl->vga.vram_offset); + 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 899bb07..4e71fe0 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -755,6 +755,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; @@ -764,11 +769,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); @@ -777,12 +825,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 = { @@ -805,6 +857,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) @@ -1220,11 +1273,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: @@ -1673,6 +1732,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 dd47395..c45e3a9 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -17,6 +17,8 @@ enum qxl_mode { #define QXL_UNDEFINED_IO UINT32_MAX +#define QXL_NUM_DIRTY_RECTS 64 + typedef struct PCIQXLDevice { PCIDevice pci; SimpleSpiceDisplay ssd; @@ -88,6 +90,12 @@ typedef struct PCIQXLDevice { /* io bar */ uint32_t io_base; + + /* 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)) { \ @@ -129,6 +137,8 @@ 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); /* rhel6 only */ int rhel6_qxl_screendump(const char *id, const char *filename); diff --git a/ui/spice-display.h b/ui/spice-display.h index 8b11f27..0496c8e 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.7.6