[Spice-devel] [PATCH] Unify memory management for the primary screen.
Alon Levy
alevy at redhat.com
Thu Sep 13 13:50:35 PDT 2012
> We never actually connected the host_image pixmap to
> the primary screen, because we never operate on the bits
> of the primary screen.
>
> This causes problems if we wish to use traditional X
> operations to modify the primary screen, as they will
> fail and corrupt memory as they go.
Looks good to me.
> ---
> src/qxl.h | 2 +-
> src/qxl_driver.c | 63
> +++++++++++++++++++++++++----------------------------
> src/qxl_surface.c | 13 +++++------
> 3 files changed, 37 insertions(+), 41 deletions(-)
>
> diff --git a/src/qxl.h b/src/qxl.h
> index 8494550..33b0fb7 100644
> --- a/src/qxl.h
> +++ b/src/qxl.h
> @@ -162,7 +162,6 @@ struct _qxl_screen_t
>
> int virtual_x;
> int virtual_y;
> - void * fb;
>
> /* not the same as the heads mode for #head > 1 or virtual !=
> head size */
> struct QXLMode primary_mode;
> @@ -340,6 +339,7 @@ int qxl_ring_cons (struct
> qxl_ring *ring);
> surface_cache_t * qxl_surface_cache_create (qxl_screen_t *qxl);
> qxl_surface_t * qxl_surface_cache_create_primary
> (surface_cache_t *qxl,
> struct QXLMode *mode);
> +void * qxl_surface_get_host_bits(qxl_surface_t
> *surface);
> qxl_surface_t * qxl_surface_create (surface_cache_t *qxl,
> int width,
> int height,
> diff --git a/src/qxl_driver.c b/src/qxl_driver.c
> index dbb8ceb..bfdb51a 100644
> --- a/src/qxl_driver.c
> +++ b/src/qxl_driver.c
> @@ -881,9 +881,6 @@ qxl_close_screen (CLOSE_SCREEN_ARGS_DECL)
> #else
> pScrn->EnableDisableFBAccess (pScrn, FALSE);
> #endif
> - ErrorF ("Freeing %p\n", qxl->fb);
> - free (qxl->fb);
> - qxl->fb = NULL;
>
> pScreen->CreateScreenResources = qxl->create_screen_resources;
> pScreen->CloseScreen = qxl->close_screen;
> @@ -925,7 +922,7 @@ set_screen_pixmap_header (ScreenPtr pScreen)
> qxl->primary_mode.x_res,
> qxl->primary_mode.y_res,
> -1, -1,
> qxl->pScrn->displayWidth *
> qxl->bytes_per_pixel,
> - NULL);
> +
> qxl_surface_get_host_bits(qxl->primary));
> }
> else
> {
> @@ -933,6 +930,22 @@ set_screen_pixmap_header (ScreenPtr pScreen)
> }
> }
>
> +static qxl_surface_t *
> +qxl_create_primary(qxl_screen_t *qxl)
> +{
> + struct QXLMode *pm = &qxl->primary_mode;
> + pm->id = 0x4242;
> + pm->x_res = qxl->virtual_x;
> + pm->y_res = qxl->virtual_y;
> + pm->bits = qxl->pScrn->bitsPerPixel;
> + pm->stride = qxl->virtual_x * pm->bits / 8;
> + pm->x_mili = 0; // TODO
> + pm->y_mili = 0; // TODO
> + pm->orientation = 0; // ? supported by us for single head usage?
> more TODO
> +
> + return qxl_surface_cache_create_primary (qxl->surface_cache,
> &qxl->primary_mode);
> +}
> +
> static Bool
> qxl_resize_primary_to_virtual (qxl_screen_t *qxl)
> {
> @@ -967,18 +980,7 @@ qxl_resize_primary_to_virtual (qxl_screen_t
> *qxl)
> qxl_io_destroy_primary (qxl);
> }
>
> - {
> - struct QXLMode *pm = &qxl->primary_mode;
> - pm->id = 0x4242;
> - pm->x_res = qxl->virtual_x;
> - pm->y_res = qxl->virtual_y;
> - pm->bits = qxl->pScrn->bitsPerPixel;
> - pm->stride = qxl->virtual_x * pm->bits / 8;
> - pm->x_mili = 0; // TODO
> - pm->y_mili = 0; // TODO
> - pm->orientation = 0; // ? supported by us for single head usage?
> more TODO
> - }
> - qxl->primary = qxl_surface_cache_create_primary
> (qxl->surface_cache, &qxl->primary_mode);
> + qxl->primary = qxl_create_primary(qxl);
> qxl->bytes_per_pixel = (qxl->pScrn->bitsPerPixel + 7) / 8;
>
> pScreen = qxl->pScrn->pScreen;
> @@ -991,6 +993,8 @@ qxl_resize_primary_to_virtual (qxl_screen_t *qxl)
> qxl_surface_kill (surf);
>
> set_surface (root, qxl->primary);
> +
> + set_screen_pixmap_header (pScreen);
> }
>
> ErrorF ("primary is %p\n", qxl->primary);
> @@ -1712,12 +1716,8 @@ static Bool
> qxl_fb_init (qxl_screen_t *qxl, ScreenPtr pScreen)
> {
> ScrnInfoPtr pScrn = qxl->pScrn;
> -
> -#if 0
> - ErrorF ("allocated %d x %d %p\n", pScrn->virtualX,
> pScrn->virtualY, qxl->fb);
> -#endif
> -
> - if (!fbScreenInit (pScreen, NULL,
> +
> + if (!fbScreenInit (pScreen,
> qxl_surface_get_host_bits(qxl->primary),
> pScrn->virtualX, pScrn->virtualY,
> pScrn->xDpi, pScrn->yDpi,
> pScrn->displayWidth,
> pScrn->bitsPerPixel))
> @@ -1761,16 +1761,19 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL)
> goto out;
> pScrn->displayWidth = pScrn->virtualX;
>
> - qxl->fb = calloc (pScrn->virtualY * pScrn->displayWidth, 4);
> - if (!qxl->fb)
> - goto out;
> -
> #if 0
> ErrorF ("allocated %d x %d %p\n", pScrn->virtualX,
> pScrn->virtualY, qxl->fb);
> #endif
> -
> +
> pScrn->virtualX = pScrn->currentMode->HDisplay;
> pScrn->virtualY = pScrn->currentMode->VDisplay;
> +
> + /* Set up resources */
> + qxl_reset_and_create_mem_slots (qxl);
> + ErrorF ("done reset\n");
> +
> + qxl->surface_cache = qxl_surface_cache_create (qxl);
> + qxl->primary = qxl_create_primary(qxl);
>
> if (!qxl_fb_init (qxl, pScreen))
> goto out;
> @@ -1791,10 +1794,6 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL)
>
> qxl->uxa = uxa_driver_alloc ();
>
> - /* Set up resources */
> - qxl_reset_and_create_mem_slots (qxl);
> - ErrorF ("done reset\n");
> -
> #ifndef XSPICE
> qxl->io_pages = (void *)((unsigned long)qxl->ram);
> qxl->io_pages_physical = (void *)((unsigned
> long)qxl->ram_physical);
> @@ -1810,8 +1809,6 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL)
> sizeof (uint64_t),
> QXL_RELEASE_RING_SIZE, 0,
> qxl);
>
> - qxl->surface_cache = qxl_surface_cache_create (qxl);
> -
> /* xf86DPMSInit (pScreen, xf86DPMSSet, 0); */
>
> pScreen->SaveScreen = qxl_blank_screen;
> diff --git a/src/qxl_surface.c b/src/qxl_surface.c
> index e88675f..8c89eb6 100644
> --- a/src/qxl_surface.c
> +++ b/src/qxl_surface.c
> @@ -404,15 +404,9 @@ qxl_surface_cache_create_primary
> (surface_cache_t *cache,
> dev_image = pixman_image_create_bits (format, mode->x_res,
> mode->y_res,
> (uint32_t *)dev_addr, -mode->stride);
>
> - if (qxl->fb != NULL)
> - free(qxl->fb);
> - qxl->fb = calloc (qxl->virtual_x * qxl->virtual_y, 4);
> - if (!qxl->fb)
> - return NULL;
> -
> host_image = pixman_image_create_bits (format,
> qxl->virtual_x, qxl->virtual_y,
> - qxl->fb, mode->stride);
> + NULL, mode->stride);
> #if 0
> xf86DrvMsg(cache->qxl->pScrn->scrnIndex, X_ERROR,
> "testing dev_image memory (%d x %d)\n",
> @@ -439,6 +433,11 @@ qxl_surface_cache_create_primary
> (surface_cache_t *cache,
> return surface;
> }
>
> +void *
> +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)
> {
> --
> 1.7.10.4
>
>
> _______________________________________________
> 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