[Spice-devel] [PATCH 3/3] Add a deferred frames mode.
Alon Levy
alevy at redhat.com
Sat Sep 15 10:03:06 PDT 2012
> This renders all operations to a frame buffer,
> and sends updates periodically.
At a cursory read it looks good. I assume it compiles for non Xspice build. I wonder if it also makes sense for non Xspice, but of course it won't work there the way it is written now just because you use SpiceCoreInterface and SpiceTimer, but that's probably easy to fix (you can use X timers which I've already forgotten the API for).
I find it a bit funny that qemu recently got a damage tracking vga mode for qxl, and now you are introducing it here - but hey, if it works, great. And it does in spades :)
> ---
> src/Makefile.am | 1 +
> src/dfps.c | 290
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/dfps.h | 42 ++++++++
> src/qxl.h | 5 +
> src/qxl_driver.c | 69 +++++++------
> src/qxl_surface.c | 52 ++++++++++
> 6 files changed, 430 insertions(+), 29 deletions(-)
> create mode 100644 src/dfps.c
> create mode 100644 src/dfps.h
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 45ba8c9..2a91024 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -90,5 +90,6 @@ spiceqxl_drv_la_SOURCES = \
> murmurhash3.h \
> qxl_cursor.c \
> qxl_edid.c \
> + dfps.c \
> compat-api.h
> endif
> diff --git a/src/dfps.c b/src/dfps.c
> new file mode 100644
> index 0000000..fca51ff
> --- /dev/null
> +++ b/src/dfps.c
> @@ -0,0 +1,290 @@
> +/*
> + * Copyright (C) 2012 CodeWeavers, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person
> obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> the
> + * Software is furnished to do so, subject to the following
> conditions:
> + *
> + * The above copyright notice and this permission notice (including
> the next
> + * paragraph) shall be included in all copies or substantial
> portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS IN THE
> + * SOFTWARE.
> + *
> + * Authors:
> + * Jeremy White <jwhite at codeweavers.com>
> + */
> +
> +/*----------------------------------------------------------------------------
> + Deferred Frames Per Second (dfps) Support File
> +
> + By default, The xorg-video-qxl driver transmits all of the video
> + operation across the wire. While this has the greatest fidelity,
> and
> + lends itself well to a variety of optimizations and behavior
> tuning,
> + it does not always use bandwidth efficiently.
> +
> + This file implements a 'deferred frames' mode which instead
> renders
> + everything to a frame buffer, and then periodically sends only
> updated
> + portions of the screen.
> +
> + For some use cases, this proves to be far more efficient with
> network
> + resources.
> +----------------------------------------------------------------------------*/
> +
> +#include <xorg-server.h>
> +#include "qxl.h"
> +#include "dfps.h"
> +
> +struct dfps_info_t
> +{
> + RegionRec updated_region;
> +
> + PixmapPtr copy_src;
> + Pixel solid_pixel;
> +};
> +
> +void dfps_ticker(void *opaque)
> +{
> + qxl_screen_t *qxl = (qxl_screen_t *) opaque;
> + dfps_info_t *info = NULL;
> + PixmapPtr pixmap;
> +
> + pixmap =
> qxl->pScrn->pScreen->GetScreenPixmap(qxl->pScrn->pScreen);
> + if (pixmap)
> + info = dfps_get_info(pixmap);
> + if (info)
> + {
> + qxl_surface_upload_primary_regions(qxl, pixmap,
> &info->updated_region);
> + RegionUninit(&info->updated_region);
> + RegionInit(&info->updated_region, NULL, 0);
> + }
> + qxl->core->timer_start(qxl->frames_timer, 1000 /
> qxl->deferred_fps);
> +}
> +
> +
> +static Bool unaccel (void)
> +{
> + return FALSE;
> +}
> +
> +static Bool dfps_prepare_solid (PixmapPtr pixmap, int alu, Pixel
> planemask, Pixel fg)
> +{
> + dfps_info_t *info;
> +
> + if (!(info = dfps_get_info (pixmap)))
> + return FALSE;
> +
> + info->solid_pixel = fg;
> +
> + return TRUE;
> +}
> +
> +static void dfps_solid (PixmapPtr pixmap, int x_1, int y_1, int x_2,
> int y_2)
> +{
> + FbBits *bits;
> + int stride;
> + int bpp;
> + int xoff, yoff;
> + struct pixman_box16 box;
> + RegionPtr region;
> + Bool throwaway_bool;
> + dfps_info_t *info;
> +
> + if (!(info = dfps_get_info (pixmap)))
> + return;
> +
> + /* Draw to the frame buffer */
> + fbGetDrawable((DrawablePtr)pixmap, bits, stride, bpp, xoff,
> yoff);
> + pixman_fill((uint32_t *) bits, stride, bpp, x_1 + xoff, y_1 +
> yoff, x_2 - x_1, y_2 - y_1, info->solid_pixel);
> + fbValidateDrawable(pixmap);
> + fbFinishAccess(pixmap);
> +
> + /* Track the updated region */
> + box.x1 = x_1; box.x2 = x_2; box.y1 = y_1; box.y2 = y_2;
> + region = RegionCreate(&box, 1);
> + RegionAppend(&info->updated_region, region);
> + RegionValidate(&info->updated_region, &throwaway_bool);
> + RegionUninit(region);
> + return;
> +}
> +
> +static void dfps_done_solid (PixmapPtr pixmap)
> +{
> +}
> +
> +static Bool dfps_prepare_copy (PixmapPtr source, PixmapPtr dest,
> + int xdir, int ydir, int alu,
> + Pixel planemask)
> +{
> + dfps_info_t *info;
> +
> + if (!(info = dfps_get_info (dest)))
> + return FALSE;
> +
> + info->copy_src = source;
> + return TRUE;
> +}
> +
> +static void dfps_copy (PixmapPtr dest,
> + int src_x1, int src_y1,
> + int dest_x1, int dest_y1,
> + int width, int height)
> +{
> + struct pixman_box16 box;
> + RegionPtr region;
> + Bool throwaway_bool;
> + GCPtr pgc;
> +
> + dfps_info_t *info;
> +
> + if (!(info = dfps_get_info (dest)))
> + return;
> +
> + /* Render into to the frame buffer */
> + pgc = GetScratchGC(dest->drawable.depth,
> dest->drawable.pScreen);
> + ValidateGC(&dest->drawable, pgc);
> + fbCopyArea(&info->copy_src->drawable, &dest->drawable, pgc,
> src_x1, src_y1, width, height, dest_x1, dest_y1);
> + FreeScratchGC(pgc);
> +
> + /* Update the tracking region */
> + box.x1 = dest_x1; box.x2 = dest_x1 + width; box.y1 = dest_y1;
> box.y2 = dest_y1 + height;
> + region = RegionCreate(&box, 1);
> + RegionAppend(&info->updated_region, region);
> + RegionValidate(&info->updated_region, &throwaway_bool);
> + RegionUninit(region);
> +}
> +
> +static void dfps_done_copy (PixmapPtr dest)
> +{
> +}
> +
> +static Bool dfps_put_image (PixmapPtr dest, int x, int y, int w, int
> h,
> + char *src, int src_pitch)
> +{
> + struct pixman_box16 box;
> + RegionPtr region;
> + Bool throwaway_bool;
> + dfps_info_t *info;
> +
> + if (!(info = dfps_get_info (dest)))
> + return FALSE;
> +
> + box.x1 = x; box.x2 = x + w; box.y1 = y; box.y2 = y + h;
> + region = RegionCreate(&box, 1);
> + RegionAppend(&info->updated_region, region);
> + RegionValidate(&info->updated_region, &throwaway_bool);
> + RegionUninit(region);
> +
> + /* We can avoid doing the put image ourselves, as the uxa driver
> + will fall back and do it for us if we return false */
> + return FALSE;
> +}
> +
> +
> +static Bool dfps_prepare_access (PixmapPtr pixmap, RegionPtr region,
> uxa_access_t requested_access)
> +{
> + fbPrepareAccess(pixmap);
> + if (requested_access == UXA_ACCESS_RW)
> + {
> + dfps_info_t *info;
> + Bool throwaway_bool;
> +
> + if (!(info = dfps_get_info (pixmap)))
> + return FALSE;
> + RegionAppend(&info->updated_region, region);
> + RegionValidate(&info->updated_region, &throwaway_bool);
> + }
> + return TRUE;
> +}
> +
> +static void dfps_finish_access (PixmapPtr pixmap)
> +{
> + fbFinishAccess(pixmap);
> +}
> +
> +static Bool dfps_pixmap_is_offscreen (PixmapPtr pixmap)
> +{
> + return !!dfps_get_info(pixmap);
> +}
> +
> +static void dfps_set_screen_pixmap (PixmapPtr pixmap)
> +{
> + pixmap->drawable.pScreen->devPrivate = pixmap;
> +}
> +
> +static PixmapPtr dfps_create_pixmap (ScreenPtr screen, int w, int h,
> int depth, unsigned usage)
> +{
> + PixmapPtr pixmap;
> + dfps_info_t *info;
> +
> + info = calloc(1, sizeof(*info));
> + if (!info)
> + return FALSE;
> +
> + pixmap = fbCreatePixmap (screen, w, h, depth, usage);
> + if (pixmap)
> + dfps_set_info(pixmap, info);
> + else
> + free(info);
> +
> + return pixmap;
> +}
> +
> +static Bool dfps_destroy_pixmap (PixmapPtr pixmap)
> +{
> + if (pixmap->refcnt == 1)
> + {
> + dfps_info_t *info = dfps_get_info (pixmap);
> + if (info)
> + free(info);
> + dfps_set_info(pixmap, NULL);
> + }
> +
> + return fbDestroyPixmap (pixmap);
> +}
> +
> +void dfps_set_uxa_functions(qxl_screen_t *qxl, ScreenPtr screen)
> +{
> + /* Solid fill */
> + //qxl->uxa->check_solid = dfps_check_solid;
> + qxl->uxa->prepare_solid = dfps_prepare_solid;
> + qxl->uxa->solid = dfps_solid;
> + qxl->uxa->done_solid = dfps_done_solid;
> +
> + /* Copy */
> + //qxl->uxa->check_copy = qxl_check_copy;
> + qxl->uxa->prepare_copy = dfps_prepare_copy;
> + qxl->uxa->copy = dfps_copy;
> + qxl->uxa->done_copy = dfps_done_copy;
> +
> + /* Composite */
> + qxl->uxa->check_composite =
> (typeof(qxl->uxa->check_composite))unaccel;
> + qxl->uxa->check_composite_target =
> (typeof(qxl->uxa->check_composite_target))unaccel;
> + qxl->uxa->check_composite_texture =
> (typeof(qxl->uxa->check_composite_texture))unaccel;
> + qxl->uxa->prepare_composite =
> (typeof(qxl->uxa->prepare_composite))unaccel;
> + qxl->uxa->composite = (typeof(qxl->uxa->composite))unaccel;
> + qxl->uxa->done_composite =
> (typeof(qxl->uxa->done_composite))unaccel;
> +
> + /* PutImage */
> + qxl->uxa->put_image = dfps_put_image;
> +
> + /* Prepare access */
> + qxl->uxa->prepare_access = dfps_prepare_access;
> + qxl->uxa->finish_access = dfps_finish_access;
> +
> + /* General screen information */
> + qxl->uxa->pixmap_is_offscreen = dfps_pixmap_is_offscreen;
> +
> + screen->SetScreenPixmap = dfps_set_screen_pixmap;
> + screen->CreatePixmap = dfps_create_pixmap;
> + screen->DestroyPixmap = dfps_destroy_pixmap;
> +}
> diff --git a/src/dfps.h b/src/dfps.h
> new file mode 100644
> index 0000000..ea38a46
> --- /dev/null
> +++ b/src/dfps.h
> @@ -0,0 +1,42 @@
> +/*
> + * Copyright (C) 2012 CodeWeavers, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person
> obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> the
> + * Software is furnished to do so, subject to the following
> conditions:
> + *
> + * The above copyright notice and this permission notice (including
> the next
> + * paragraph) shall be included in all copies or substantial
> portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
> +
> +typedef struct dfps_info_t dfps_info_t;
> +
> +void dfps_ticker(void *opaque);
> +void dfps_set_uxa_functions(qxl_screen_t *qxl, ScreenPtr screen);
> +
> +static inline dfps_info_t *dfps_get_info (PixmapPtr pixmap)
> +{
> +#if HAS_DEVPRIVATEKEYREC
> + return dixGetPrivate(&pixmap->devPrivates, &uxa_pixmap_index);
> +#else
> + return dixLookupPrivate(&pixmap->devPrivates,
> &uxa_pixmap_index);
> +#endif
> +}
> +
> +static inline void dfps_set_info (PixmapPtr pixmap, dfps_info_t
> *info)
> +{
> + dixSetPrivate(&pixmap->devPrivates, &uxa_pixmap_index, info);
> +}
> diff --git a/src/qxl.h b/src/qxl.h
> index f26ddd0..ed02010 100644
> --- a/src/qxl.h
> +++ b/src/qxl.h
> @@ -238,6 +238,9 @@ struct _qxl_screen_t
> /* XSpice specific */
> struct QXLRom shadow_rom; /* Parameter RAM */
> SpiceServer * spice_server;
> + SpiceCoreInterface *core;
> + SpiceTimer * frames_timer;
> +
> QXLWorker * worker;
> int worker_running;
> QXLInstance display_sin;
> @@ -437,6 +440,8 @@ get_ram_header (qxl_screen_t *qxl)
> ((uint8_t *)qxl->ram + qxl->rom->ram_header_offset);
> }
>
> +void qxl_surface_upload_primary_regions(qxl_screen_t *qxl, PixmapPtr
> pixmap, RegionRec *r);
> +
> /*
> * Images
> */
> diff --git a/src/qxl_driver.c b/src/qxl_driver.c
> index c43cbde..fa93c48 100644
> --- a/src/qxl_driver.c
> +++ b/src/qxl_driver.c
> @@ -56,6 +56,7 @@
> #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);
> @@ -916,14 +917,10 @@ set_screen_pixmap_header (ScreenPtr pScreen)
>
> if (pPixmap)
> {
> - ErrorF ("new stride: %d (display width: %d, bpp: %d)\n",
> - qxl->pScrn->displayWidth * qxl->bytes_per_pixel,
> - qxl->pScrn->displayWidth, qxl->bytes_per_pixel);
> -
> pScreen->ModifyPixmapHeader (pPixmap,
> qxl->primary_mode.x_res,
> qxl->primary_mode.y_res,
> -1, -1,
> - qxl->pScrn->displayWidth *
> qxl->bytes_per_pixel,
> + qxl->primary_mode.x_res *
> qxl->bytes_per_pixel,
> qxl_surface_get_host_bits(qxl->primary));
> }
> else
> @@ -953,7 +950,7 @@ qxl_resize_primary_to_virtual (qxl_screen_t *qxl)
> {
> ScreenPtr pScreen;
> long new_surface0_size;
> -
> +
> if ((qxl->primary_mode.x_res == qxl->virtual_x &&
> qxl->primary_mode.y_res == qxl->virtual_y) &&
> qxl->device_primary == QXL_DEVICE_PRIMARY_CREATED)
> @@ -989,12 +986,16 @@ qxl_resize_primary_to_virtual (qxl_screen_t
> *qxl)
> if (pScreen)
> {
> PixmapPtr root = pScreen->GetScreenPixmap (pScreen);
> - qxl_surface_t *surf;
> -
> - if ((surf = get_surface (root)))
> - qxl_surface_kill (surf);
> -
> - set_surface (root, qxl->primary);
> +
> + if (qxl->deferred_fps <= 0)
> + {
> + qxl_surface_t *surf;
> +
> + if ((surf = get_surface (root)))
> + qxl_surface_kill (surf);
> +
> + set_surface (root, qxl->primary);
> + }
>
> set_screen_pixmap_header (pScreen);
> }
> @@ -1208,14 +1209,17 @@ qxl_create_screen_resources (ScreenPtr
> pScreen)
> return FALSE;
>
> pPixmap = pScreen->GetScreenPixmap (pScreen);
> -
> - set_screen_pixmap_header (pScreen);
> -
> - if ((surf = get_surface (pPixmap)))
> - qxl_surface_kill (surf);
> -
> - set_surface (pPixmap, qxl->primary);
> -
> +
> + if (qxl->deferred_fps <= 0)
> + {
> + set_screen_pixmap_header (pScreen);
> +
> + if ((surf = get_surface (pPixmap)))
> + qxl_surface_kill (surf);
> +
> + set_surface (pPixmap, qxl->primary);
> + }
> +
> /* HACK - I don't want to enable any crtcs other then the first
> at the beginning */
> for (i = 1; i < qxl->num_heads; ++i)
> {
> @@ -1675,7 +1679,10 @@ setup_uxa (qxl_screen_t *qxl, ScreenPtr
> screen)
> qxl->uxa->uxa_major = 1;
> qxl->uxa->uxa_minor = 0;
>
> - set_uxa_functions(qxl, screen);
> + if (qxl->deferred_fps)
> + dfps_set_uxa_functions(qxl, screen);
> + else
> + set_uxa_functions(qxl, screen);
>
> if (!uxa_driver_init (screen, qxl->uxa))
> {
> @@ -1702,18 +1709,21 @@ setup_uxa (qxl_screen_t *qxl, ScreenPtr
> screen)
> static void
> spiceqxl_screen_init (ScrnInfoPtr pScrn, qxl_screen_t *qxl)
> {
> - SpiceCoreInterface *core;
> -
> // Init spice
> if (!qxl->spice_server)
> {
> qxl->spice_server = xspice_get_spice_server ();
> xspice_set_spice_server_options (qxl->options);
> - core = basic_event_loop_init ();
> - spice_server_init (qxl->spice_server, core);
> + qxl->core = basic_event_loop_init ();
> + spice_server_init (qxl->spice_server, qxl->core);
> 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;
> }
> @@ -1920,11 +1930,12 @@ qxl_leave_vt (VT_FUNC_ARGS_DECL)
> qxl_screen_t *qxl = pScrn->driverPrivate;
>
> xf86_hide_cursors (pScrn);
> -
> +
> pScrn->EnableDisableFBAccess (XF86_SCRN_ARG (pScrn), FALSE);
> -
> - qxl->vt_surfaces = qxl_surface_cache_evacuate_all
> (qxl->surface_cache);
> -
> +
> + if (qxl->deferred_fps <= 0)
> + qxl->vt_surfaces = qxl_surface_cache_evacuate_all
> (qxl->surface_cache);
> +
> ioport_write (qxl, QXL_IO_RESET, 0);
>
> qxl_restore_state (pScrn);
> diff --git a/src/qxl_surface.c b/src/qxl_surface.c
> index 8c89eb6..3da9079 100644
> --- a/src/qxl_surface.c
> +++ b/src/qxl_surface.c
> @@ -438,6 +438,7 @@ qxl_surface_get_host_bits(qxl_surface_t *surface)
> {
> return (void *) pixman_image_get_data(surface->host_image);
> }
> +
> static struct QXLSurfaceCmd *
> make_surface_cmd (surface_cache_t *cache, uint32_t id,
> QXLSurfaceCmdType type)
> {
> @@ -1090,6 +1091,57 @@ upload_box (qxl_surface_t *surface, int x1,
> int y1, int x2, int y2)
> }
> }
>
> +static void
> +upload_one_primary_region(qxl_screen_t *qxl, PixmapPtr pixmap,
> BoxPtr b)
> +{
> + struct QXLRect rect;
> + struct QXLDrawable *drawable;
> + struct QXLImage *image;
> + FbBits *data;
> + int stride;
> + int bpp;
> +
> + rect.left = b->x1;
> + rect.right = b->x2;
> + rect.top = b->y1;
> + rect.bottom = b->y2;
> +
> + drawable = make_drawable (qxl, 0, QXL_DRAW_COPY, &rect);
> + drawable->u.copy.src_area = rect;
> + translate_rect (&drawable->u.copy.src_area);
> + drawable->u.copy.rop_descriptor = ROPD_OP_PUT;
> + drawable->u.copy.scale_mode = 0;
> + drawable->u.copy.mask.flags = 0;
> + drawable->u.copy.mask.pos.x = 0;
> + drawable->u.copy.mask.pos.y = 0;
> + drawable->u.copy.mask.bitmap = 0;
> +
> + fbGetPixmapBitsData(pixmap, data, stride, bpp);
> + image = qxl_image_create (
> + qxl, (const uint8_t *)data, b->x1, b->y1, b->x2 - b->x1, b->y2 -
> b->y1, stride * sizeof(*data),
> + bpp == 24 ? 4 : bpp / 8, TRUE);
> + drawable->u.copy.src_bitmap =
> + physical_address (qxl, image, qxl->main_mem_slot);
> +
> + push_drawable (qxl, drawable);
> +}
> +
> +void
> +qxl_surface_upload_primary_regions(qxl_screen_t *qxl, PixmapPtr
> pixmap, RegionRec *r)
> +{
> + int n_boxes;
> + BoxPtr boxes;
> +
> + n_boxes = RegionNumRects(r);
> + boxes = RegionRects(r);
> +
> + while (n_boxes--)
> + {
> + upload_one_primary_region(qxl, pixmap, boxes);
> + boxes++;
> + }
> +}
> +
> void
> qxl_surface_finish_access (qxl_surface_t *surface, PixmapPtr pixmap)
> {
> --
> 1.7.9.5
>
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
More information about the Spice-devel
mailing list