Here is the current patch set. I'd appreciate feedback on whether this is horribly offensive. If not, I'll submit the qemu patch upstream, and the spice-protocol and xf86-video-qxl patches here. With these three patches, a qemu -spice deferred-fps=nn should make the qemu/xf86-video-qxl driver operate in the deferred frames mode. That mode should be dramatically better for network bandwidth than the normal mode. Anyone going across any sort of WAN link should notice a substantial difference with this mode. Cheers, Jeremy p.s. Alon, Thunderbird makes it particularly hard to manually inline patches; I hunted around for an option, but could not find one. I chose to get this off my chest now, rather than wait until I could dust off my mutt-fu. Sorry :-(.
diff --git a/hw/qxl.c b/hw/qxl.c index b66b414..355d4b6 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -28,6 +28,7 @@ #include "trace.h" #include "hw/qxl.h" +#include "qmp-commands.h" /* * NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as @@ -321,6 +322,7 @@ static void init_qxl_rom(PCIQXLDevice *d) uint32_t num_pages; uint32_t fb; int i, n; + SpiceInfo *info; memset(rom, 0, d->rom_size); @@ -365,9 +367,14 @@ static void init_qxl_rom(PCIQXLDevice *d) rom->num_pages = cpu_to_le32(num_pages); rom->ram_header_offset = cpu_to_le32(d->vga.vram_size - ram_header_size); + info = qmp_query_spice(NULL); + rom->deferred_fps = info->deferred_fps; + qapi_free_SpiceInfo(info); + d->shadow_rom = *rom; d->rom = rom; d->modes = modes; + } static void init_qxl_ram(PCIQXLDevice *d) @@ -1978,6 +1985,10 @@ static int qxl_init_common(PCIQXLDevice *qxl) pci_device_rev = QXL_REVISION_STABLE_V12; io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1); break; + case 5: /* qxl-5 */ + pci_device_rev = QXL_REVISION_STABLE_V14; + io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1); + break; default: error_report("Invalid revision %d for qxl device (max %d)", qxl->revision, QXL_DEFAULT_REVISION); diff --git a/hw/qxl.h b/hw/qxl.h index 36f1a25..098fc49 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -132,7 +132,7 @@ typedef struct PCIQXLDevice { } \ } while (0) -#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V12 +#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V14 /* qxl.c */ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id); diff --git a/qemu-options.hx b/qemu-options.hx index 06dd565..123cc94 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -919,6 +919,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice, " [,streaming-video=[off|all|filter]][,disable-copy-paste]\n" " [,agent-mouse=[on|off]][,playback-compression=[on|off]]\n" " [,seamless-migration=[on|off]]\n" + " [,deferred-fps=<fps>]\n" " enable spice\n" " at least one of {port, tls-port} is mandatory\n", QEMU_ARCH_ALL) @@ -1006,6 +1007,12 @@ Enable/disable audio stream compression (using celt 0.5.1). Default is on. @item seamless-migration=[on|off] Enable/disable spice seamless migration. Default is off. +@item deferred-fps +If set to a number greater than 0, a guest running the xf86-video-spice +driver will only send screen updates at the give frame rate. This should +improve network bandwidth consumption substantially, at the cost of +some fidelity. + @end table ETEXI diff --git a/ui/spice-core.c b/ui/spice-core.c index bcc4199..5994934 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -493,9 +493,12 @@ static QemuOptsList qemu_spice_opts = { },{ .name = "playback-compression", .type = QEMU_OPT_BOOL, - }, { + },{ .name = "seamless-migration", .type = QEMU_OPT_BOOL, + },{ + .name = "deferred-fps", + .type = QEMU_OPT_NUMBER, }, { /* end of list */ } }, @@ -553,6 +556,8 @@ SpiceInfo *qmp_query_spice(Error **errp) info->has_channels = true; info->channels = qmp_query_spice_channels(); + info->deferred_fps = qemu_opt_get_number(opts, "deferred-fps", 0); + return info; }
diff --git a/configure.ac b/configure.ac index 7a3808a..22c6008 100644 --- a/configure.ac +++ b/configure.ac @@ -1,8 +1,8 @@ AC_PREREQ([2.57]) m4_define([SPICE_MAJOR], 0) -m4_define([SPICE_MINOR], 12) -m4_define([SPICE_MICRO], 5) +m4_define([SPICE_MINOR], 14) +m4_define([SPICE_MICRO], 0) AC_INIT(spice-protocol, [SPICE_MAJOR.SPICE_MINOR.SPICE_MICRO], [], spice-protocol) diff --git a/spice/qxl_dev.h b/spice/qxl_dev.h index a3e91a5..3f0db09 100644 --- a/spice/qxl_dev.h +++ b/spice/qxl_dev.h @@ -49,6 +49,7 @@ enum { QXL_REVISION_STABLE_V06=0x02, QXL_REVISION_STABLE_V10=0x03, QXL_REVISION_STABLE_V12=0x04, + QXL_REVISION_STABLE_V14=0x05, }; #define QXL_DEVICE_ID_DEVEL 0x01ff @@ -164,6 +165,8 @@ typedef struct SPICE_ATTR_PACKED QXLRom { uint16_t padding; QXLURect heads[64]; } client_monitors_config; + /* appended for qxl-5 */ + uint32_t deferred_fps; } QXLRom; #define CLIENT_MONITORS_CONFIG_CRC32_POLY 0xedb88320
diff --git a/configure.ac b/configure.ac index 48904a2..f69ebcc 100644 --- a/configure.ac +++ b/configure.ac @@ -91,21 +91,21 @@ AC_ARG_ENABLE(xspice, enable_xspice=no fi ]) - -if test "x$enable_xspice" = "xyes"; then - PKG_CHECK_MODULES([SPICE], [spice-server >= 0.6.3], - [ - AC_SUBST(SPICE_CFLAGS) - AC_SUBST(SPICE_LIBS) - ], -) -else +if test "x$enable_xspice" = x; then enable_xspice=no fi + +PKG_CHECK_MODULES([SPICE], [spice-server >= 0.6.3], +[ + AC_SUBST(SPICE_CFLAGS) + AC_SUBST(SPICE_LIBS) +], +) + AM_CONDITIONAL(BUILD_XSPICE, test "x$enable_xspice" = "xyes") AM_CONDITIONAL(BUILD_QXL, test "x$enable_qxl" = "xyes") -PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.12.0]) +PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.14.0]) # AC_CHECK_FILE is not supported when cross compiling if test "$cross_compiling" = "no" ; then diff --git a/src/Makefile.am b/src/Makefile.am index 8632297..eea19c1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -32,6 +32,7 @@ AM_CFLAGS = $(SPICE_PROTOCOL_CFLAGS) $(XORG_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNF if BUILD_QXL qxl_drv_la_LTLIBRARIES = qxl_drv.la qxl_drv_la_LDFLAGS = -module -avoid-version +qxl_drv_la_CFLAGS = $(AM_CFLAGS) $(SPICE_CFLAGS) qxl_drv_ladir = @moduledir@/drivers qxl_drv_la_LIBADD = uxa/libuxa.la @@ -56,6 +57,7 @@ qxl_drv_la_SOURCES = \ qxl_uxa.c \ qxl_ums_mode.c \ qxl_io.c \ + dfps.c \ compat-api.h endif diff --git a/src/dfps.c b/src/dfps.c index 6ac29f9..b57519b 100644 --- a/src/dfps.c +++ b/src/dfps.c @@ -53,6 +53,44 @@ struct dfps_info_t GCPtr pgc; }; +typedef struct SpiceTimer { + OsTimerPtr xorg_timer; + SpiceTimerFunc func; + void *opaque; // also stored in xorg_timer, but needed for timer_start +} Timer; + +static CARD32 xorg_timer_callback( + OsTimerPtr xorg_timer, + CARD32 time, + pointer arg) +{ + SpiceTimer *timer = (SpiceTimer*)arg; + + timer->func(timer->opaque); + return 0; // if non zero xorg does a TimerSet, we don't want that. +} + +static SpiceTimer* timer_add(SpiceTimerFunc func, void *opaque) +{ + SpiceTimer *timer = calloc(sizeof(SpiceTimer), 1); + + timer->xorg_timer = TimerSet(NULL, 0, 1e9 /* TODO: infinity? */, xorg_timer_callback, timer); + timer->func = func; + timer->opaque = opaque; + return timer; +} + +static void timer_start(SpiceTimer *timer, uint32_t ms) +{ + TimerSet(timer->xorg_timer, 0 /* flags */, ms, xorg_timer_callback, timer); +} + +void dfps_start_ticker(qxl_screen_t *qxl) +{ + qxl->frames_timer = timer_add(dfps_ticker, qxl); + timer_start(qxl->frames_timer, 1000 / qxl->deferred_fps); +} + void dfps_ticker(void *opaque) { qxl_screen_t *qxl = (qxl_screen_t *) opaque; @@ -68,7 +106,7 @@ void dfps_ticker(void *opaque) RegionUninit(&info->updated_region); RegionInit(&info->updated_region, NULL, 0); } - qxl->core->timer_start(qxl->frames_timer, 1000 / qxl->deferred_fps); + timer_start(qxl->frames_timer, 1000 / qxl->deferred_fps); } diff --git a/src/dfps.h b/src/dfps.h index ea38a46..3f3336a 100644 --- a/src/dfps.h +++ b/src/dfps.h @@ -24,6 +24,7 @@ typedef struct dfps_info_t dfps_info_t; +void dfps_start_ticker(qxl_screen_t *qxl); void dfps_ticker(void *opaque); void dfps_set_uxa_functions(qxl_screen_t *qxl, ScreenPtr screen); diff --git a/src/qxl.h b/src/qxl.h index c26ea8f..a49f020 100644 --- a/src/qxl.h +++ b/src/qxl.h @@ -26,9 +26,7 @@ #include <stdint.h> #include <spice/qxl_dev.h> -#ifdef XSPICE #include <spice.h> -#endif #include "compiler.h" #include "xf86.h" @@ -105,6 +103,7 @@ enum { OPTION_ENABLE_FALLBACK_CACHE, OPTION_ENABLE_SURFACES, OPTION_NUM_HEADS, + OPTION_SPICE_DEFERRED_FPS, #ifdef XSPICE OPTION_SPICE_PORT, OPTION_SPICE_TLS_PORT, @@ -128,7 +127,6 @@ enum { OPTION_SPICE_TLS_CIPHERS, OPTION_SPICE_CACERT_FILE, OPTION_SPICE_DH_FILE, - OPTION_SPICE_DEFERRED_FPS, OPTION_SPICE_EXIT_ON_DISCONNECT, OPTION_SPICE_PLAYBACK_FIFO_DIR, #endif @@ -275,12 +273,13 @@ struct _qxl_screen_t int enable_fallback_cache; int enable_surfaces; + SpiceCoreInterface *core; + SpiceTimer * frames_timer; + #ifdef XSPICE /* XSpice specific */ struct QXLRom shadow_rom; /* Parameter RAM */ SpiceServer * spice_server; - SpiceCoreInterface *core; - SpiceTimer * frames_timer; QXLWorker * worker; int worker_running; @@ -307,11 +306,10 @@ struct _qxl_screen_t uint8_t *data, *flipped; } guest_primary; - uint32_t deferred_fps; - char playback_fifo_dir[PATH_MAX]; #endif /* XSPICE */ + uint32_t deferred_fps; struct xorg_list ums_bos; struct qxl_bo_funcs *bo_funcs; }; diff --git a/src/qxl_driver.c b/src/qxl_driver.c index f1f6592..d225e82 100644 --- a/src/qxl_driver.c +++ b/src/qxl_driver.c @@ -47,8 +47,8 @@ #include "qxl_option_helpers.h" #include <spice/protocol.h> -#ifdef XSPICE #include "spiceqxl_driver.h" +#include "spiceqxl_spice_server.h" #include "spiceqxl_main_loop.h" #include "spiceqxl_display.h" #include "spiceqxl_inputs.h" @@ -56,7 +56,6 @@ #include "spiceqxl_spice_server.h" #include "dfps.h" #include "spiceqxl_audio.h" -#endif /* XSPICE */ extern void compat_init_scrn (ScrnInfoPtr); @@ -78,6 +77,8 @@ const OptionInfoRec DefaultOptions[] = "EnableSurfaces", OPTV_BOOLEAN, { 1 }, FALSE }, { OPTION_NUM_HEADS, "NumHeads", OPTV_INTEGER, { 4 }, FALSE }, + { OPTION_SPICE_DEFERRED_FPS, + "SpiceDeferredFPS", OPTV_INTEGER, { 0 }, FALSE}, #ifdef XSPICE { OPTION_SPICE_PORT, "SpicePort", OPTV_INTEGER, {5900}, FALSE }, @@ -124,8 +125,6 @@ const OptionInfoRec DefaultOptions[] = "SpiceCacertFile", OPTV_STRING, {0}, FALSE}, { OPTION_SPICE_DH_FILE, "SpiceDhFile", OPTV_STRING, {0}, FALSE}, - { OPTION_SPICE_DEFERRED_FPS, - "SpiceDeferredFPS", OPTV_INTEGER, {0}, FALSE}, { OPTION_SPICE_EXIT_ON_DISCONNECT, "SpiceExitOnDisconnect", OPTV_BOOLEAN, {0}, FALSE}, { OPTION_SPICE_PLAYBACK_FIFO_DIR, @@ -543,9 +542,7 @@ qxl_resize_primary_to_virtual (qxl_screen_t *qxl) { PixmapPtr root = pScreen->GetScreenPixmap (pScreen); -#ifdef XSPICE if (qxl->deferred_fps <= 0) -#endif { qxl_surface_t *surf; @@ -606,9 +603,7 @@ qxl_create_screen_resources (ScreenPtr pScreen) pPixmap = pScreen->GetScreenPixmap (pScreen); -#ifdef XSPICE if (qxl->deferred_fps <= 0) -#endif { set_screen_pixmap_header (pScreen); @@ -640,11 +635,6 @@ spiceqxl_screen_init (ScrnInfoPtr pScrn, qxl_screen_t *qxl) qxl_add_spice_playback_interface (qxl); qxl->worker->start (qxl->worker); qxl->worker_running = TRUE; - if (qxl->deferred_fps) - { - qxl->frames_timer = qxl->core->timer_add(dfps_ticker, qxl); - qxl->core->timer_start(qxl->frames_timer, 1000 / qxl->deferred_fps); - } } qxl->spice_server = qxl->spice_server; } @@ -675,7 +665,7 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL) VisualPtr visual; CHECK_POINT (); - + assert (qxl->pScrn == pScrn); if (!qxl_map_memory (qxl, pScrn->scrnIndex)) @@ -802,6 +792,11 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL) /* bounds" */ xf86RandR12SetTransformSupport (pScreen, TRUE); + if (qxl->deferred_fps) + { + dfps_start_ticker(qxl); + } + return TRUE; out: @@ -854,9 +849,7 @@ qxl_leave_vt (VT_FUNC_ARGS_DECL) pScrn->EnableDisableFBAccess (XF86_SCRN_ARG (pScrn), FALSE); -#ifdef XSPICE if (qxl->deferred_fps <= 0) -#endif qxl->vt_surfaces = qxl_surface_cache_evacuate_all (qxl->surface_cache); ioport_write (qxl, QXL_IO_RESET, 0); @@ -977,14 +970,6 @@ qxl_pre_init_common(ScrnInfoPtr pScrn) qxl->num_heads = get_int_option (qxl->options, OPTION_NUM_HEADS, "QXL_NUM_HEADS"); -#ifdef XSPICE - qxl->deferred_fps = get_int_option(qxl->options, OPTION_SPICE_DEFERRED_FPS, "XSPICE_DEFERRED_FPS"); - if (qxl->deferred_fps > 0) - xf86DrvMsg(scrnIndex, X_INFO, "Deferred FPS: %d\n", qxl->deferred_fps); - else - xf86DrvMsg(scrnIndex, X_INFO, "Deferred Frames: Disabled\n"); -#endif - xf86DrvMsg (scrnIndex, X_INFO, "Offscreen Surfaces: %s\n", qxl->enable_surfaces ? "Enabled" : "Disabled"); xf86DrvMsg (scrnIndex, X_INFO, "Image Cache: %s\n", @@ -1047,6 +1032,7 @@ qxl_pre_init (ScrnInfoPtr pScrn, int flags) { ErrorF ("Ignoring monitor config, device revision < 4\n"); } + #endif /* XSPICE */ pScrn->monitor = pScrn->confScreen->monitor; @@ -1074,6 +1060,17 @@ qxl_pre_init (ScrnInfoPtr pScrn, int flags) xspice_init_qxl_ram (qxl); /* initialize the rings */ #endif + if (qxl->pci->revision >= 5) + qxl->deferred_fps = qxl->rom->deferred_fps; + + if (getenv("XSPICE_DEFERRED_FPS") || qxl->options[OPTION_SPICE_DEFERRED_FPS].found) + qxl->deferred_fps = get_int_option(qxl->options, OPTION_SPICE_DEFERRED_FPS, "XSPICE_DEFERRED_FPS"); + + if (qxl->deferred_fps > 0) + xf86DrvMsg(scrnIndex, X_INFO, "Deferred FPS: %d\n", qxl->deferred_fps); + else + xf86DrvMsg(scrnIndex, X_INFO, "Deferred Frames: Disabled\n"); + #define DIV_ROUND_UP(n, a) (((n) + (a) - 1) / (a)) #define BYTES_TO_KB(bytes) DIV_ROUND_UP(bytes, 1024) #define PAGES_TO_KB(pages) ((pages) * getpagesize() / 1024) diff --git a/src/qxl_uxa.c b/src/qxl_uxa.c index 1810181..d84693e 100644 --- a/src/qxl_uxa.c +++ b/src/qxl_uxa.c @@ -490,11 +490,9 @@ qxl_uxa_init (qxl_screen_t *qxl, ScreenPtr screen) qxl->uxa->uxa_major = 1; qxl->uxa->uxa_minor = 0; -#ifdef XSPICE if (qxl->deferred_fps) dfps_set_uxa_functions(qxl, screen); else -#endif set_uxa_functions(qxl, screen); if (!uxa_driver_init (screen, qxl->uxa))
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel