[Spice-devel] Experimental patch to enable Deferred FPS mode for the qemu qxl driver.
Hans de Goede
hdegoede at redhat.com
Tue Jan 29 03:41:27 PST 2013
Hi,
On 01/28/2013 09:46 PM, Jeremy White wrote:
> When I decided to try to understand what was going on with Gnome
> fallback, I hacked the Deferred FPS mode into working with a qemu
> system.
>
> This is not quite ready for prime time, but should make for
> interesting testing.
>
> My rather biased sense is that it's an improvement over the normal mode.
>
> Cheers,
>
> Jeremy
>
>
> ---
> configure.ac | 15 ++++++---------
> src/Makefile.am | 2 ++
> src/dfps.c | 40 +++++++++++++++++++++++++++++++++++++++-
> src/dfps.h | 1 +
> src/qxl.h | 11 +++++------
> src/qxl_driver.c | 31 ++++++++++---------------------
> src/qxl_surface.c | 5 ++++-
> 7 files changed, 67 insertions(+), 38 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 48904a2..1aba1c1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -92,16 +92,13 @@ AC_ARG_ENABLE(xspice,
> 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)
> - ],
> +PKG_CHECK_MODULES([SPICE], [spice-server >= 0.6.3],
> +[
> + AC_SUBST(SPICE_CFLAGS)
> + AC_SUBST(SPICE_LIBS)
> +],
> )
> -else
> - enable_xspice=no
> -fi
> +
> AM_CONDITIONAL(BUILD_XSPICE, test "x$enable_xspice" = "xyes")
> AM_CONDITIONAL(BUILD_QXL, test "x$enable_qxl" = "xyes")
>
Hmm, this seems like some debugging / wip work left over ?
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 6950ba5..08674b2 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
> @@ -51,6 +52,7 @@ qxl_drv_la_SOURCES = \
> qxl_option_helpers.c \
> qxl_option_helpers.h \
> qxl_edid.c \
> + dfps.c \
> compat-api.h
> endif
>
> diff --git a/src/dfps.c b/src/dfps.c
> index e5a2273..5fd6964 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 8e2802a..c301e0a 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"
> @@ -104,6 +102,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,
> @@ -127,7 +126,6 @@ enum {
> OPTION_SPICE_TLS_CIPHERS,
> OPTION_SPICE_CACERT_FILE,
> OPTION_SPICE_DH_FILE,
> - OPTION_SPICE_DEFERRED_FPS,
> OPTION_SPICE_EXIT_ON_DISCONNECT,
> #endif
> OPTION_COUNT,
> @@ -237,12 +235,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;
> @@ -266,8 +265,8 @@ struct _qxl_screen_t
> uint8_t *data, *flipped;
> } guest_primary;
>
> - uint32_t deferred_fps;
> #endif /* XSPICE */
> + uint32_t deferred_fps;
> };
>
> typedef struct qxl_output_private {
> diff --git a/src/qxl_driver.c b/src/qxl_driver.c
> index e1893dc..e951ccc 100644
> --- a/src/qxl_driver.c
> +++ b/src/qxl_driver.c
> @@ -44,20 +44,19 @@
>
> #include "mspace.h"
>
> +#include <spice.h>
> #include "qxl.h"
> #include "assert.h"
> #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"
> #include "spiceqxl_io_port.h"
> -#include "spiceqxl_spice_server.h"
> #include "dfps.h"
> -#endif /* XSPICE */
>
> extern void compat_init_scrn (ScrnInfoPtr);
>
> @@ -79,6 +78,8 @@ const OptionInfoRec DefaultOptions[] =
> "EnableSurfaces", OPTV_BOOLEAN, { 0 }, TRUE },
> { OPTION_NUM_HEADS,
> "NumHeads", OPTV_INTEGER, { 4 }, FALSE },
> + { OPTION_SPICE_DEFERRED_FPS,
> + "SpiceDeferredFPS", OPTV_INTEGER, { 15 }, FALSE},
> #ifdef XSPICE
> { OPTION_SPICE_PORT,
> "SpicePort", OPTV_INTEGER, {5900}, FALSE },
> @@ -125,8 +126,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},
> #endif
> @@ -988,9 +987,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;
>
> @@ -1203,9 +1200,7 @@ qxl_create_screen_resources (ScreenPtr pScreen)
>
> pPixmap = pScreen->GetScreenPixmap (pScreen);
>
> -#ifdef XSPICE
> if (qxl->deferred_fps <= 0)
> -#endif
> {
> set_screen_pixmap_header (pScreen);
>
> @@ -1680,11 +1675,9 @@ setup_uxa (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))
> @@ -1722,11 +1715,6 @@ spiceqxl_screen_init (ScrnInfoPtr pScrn, qxl_screen_t *qxl)
> qxl_add_spice_display_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;
> }
> @@ -1757,7 +1745,7 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL)
> VisualPtr visual;
>
> CHECK_POINT ();
> -
> +
> assert (qxl->pScrn == pScrn);
>
> if (!qxl_map_memory (qxl, pScrn->scrnIndex))
> @@ -1888,6 +1876,11 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL)
> /* bounds" */
> xf86RandR12SetTransformSupport (pScreen, TRUE);
>
> + if (qxl->deferred_fps)
> + {
> + dfps_start_ticker(qxl);
> + }
> +
> return TRUE;
>
> out:
> @@ -1940,9 +1933,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);
> @@ -2431,13 +2422,11 @@ qxl_pre_init (ScrnInfoPtr pScrn, int flags)
> 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");
And the below chunk looks like it should be in a separate commit, with a commit
message explaining it in more detail.
> diff --git a/src/qxl_surface.c b/src/qxl_surface.c
> index 3da9079..f6de4d0 100644
> --- a/src/qxl_surface.c
> +++ b/src/qxl_surface.c
> @@ -1137,7 +1137,10 @@ qxl_surface_upload_primary_regions(qxl_screen_t *qxl, PixmapPtr pixmap, RegionRe
>
> while (n_boxes--)
> {
> - upload_one_primary_region(qxl, pixmap, boxes);
> + if (boxes->x2 > boxes->x1 && boxes->y2 > boxes->y1)
> + upload_one_primary_region(qxl, pixmap, boxes);
> + else
> + ErrorF("JPW skipping boxes %d, %d to %d, %d\n", boxes->x1, boxes->y1, boxes->x2, boxes->y2);
> boxes++;
> }
> }
>
Regards,
Hans
More information about the Spice-devel
mailing list