[Spice-devel] [PATCH qxl-win v3] display/surface: add DEVICE_BITMAP_ALLOCATION_TYPE_RAM, cleanup surface alloc/free code paths
Yonit Halperin
yhalperi at redhat.com
Thu Jul 7 06:48:41 PDT 2011
On 07/07/2011 12:43 PM, Alon Levy wrote:
> This adds a third surface allocation type, allocation from guest memory using
> the windows ddk allocator.
>
> Not all code paths are used later - the creation is not done since
> copy-surfaces-to-ram allocates memory itself, and at the end we never allocate
> any surfaces when the device is disabled, we just punt the allocation to the
> gdi, but the code is still left in GetSurfaceMemory.
Isn't it better if we keep it symmetric: use
QXLGetSurface/QXLDelSurface, or do the alloc/free independently?
>
> Cc: Yonit Halperin<yhalperi at redhat.com>
> ---
> display/driver.c | 6 ++++++
> display/res.c | 40 +++++++++++++++++++++++-----------------
> display/surface.c | 17 +++++++++++++----
> display/surface.h | 1 +
> 4 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/display/driver.c b/display/driver.c
> index 11ba875..8375eff 100644
> --- a/display/driver.c
> +++ b/display/driver.c
> @@ -1292,9 +1292,15 @@ VOID APIENTRY DrvDeleteDeviceBitmap(DHSURF dhsurf)
> {
> UINT32 surface_id;
> SurfaceInfo *surface;
> + PDev *pdev;
>
> surface = (SurfaceInfo *)dhsurf;
> surface_id = GetSurfaceIdFromInfo(surface);
> + pdev = surface->u.pdev;
> +
> + DEBUG_PRINT((pdev, 3, "%s: %p: %d\n", __FUNCTION__, pdev, surface_id));
> +
> + ASSERT(pdev, surface_id< pdev->n_surfaces);
>
> DeleteDeviceBitmap(surface->u.pdev, surface_id, DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
> }
> diff --git a/display/res.c b/display/res.c
> index e6f0f81..16b9797 100644
> --- a/display/res.c
> +++ b/display/res.c
> @@ -793,13 +793,21 @@ _inline void GetSurfaceMemory(PDev *pdev, UINT32 x, UINT32 y, UINT32 depth, UINT
> *base_mem = AllocMem(pdev, MSPACE_TYPE_DEVRAM, (*stride) * y);
> *phys_mem = PA(pdev, *base_mem, pdev->main_mem_slot);
> break;
> - case DEVICE_BITMAP_ALLOCATION_TYPE_VRAM: {
> + case DEVICE_BITMAP_ALLOCATION_TYPE_VRAM:
> *stride = x * depth / 8;
> *stride = ALIGN(*stride, 4);
> *base_mem = __AllocMem(pdev, MSPACE_TYPE_VRAM, (*stride) * y, FALSE);
> *phys_mem = PA(pdev, (PVOID)((UINT64)*base_mem), pdev->vram_mem_slot);
> break;
> - }
> + case DEVICE_BITMAP_ALLOCATION_TYPE_RAM:
> + /* used only before suspend to sleep (DrvAssertMode(FALSE)) and then released
> + * and copied back to VRAM */
> + *stride = x * depth / 8;
> + *stride = ALIGN(*stride, 4);
> + *base_mem = EngAllocMem(0 /* don't zero memory, will be copied over in a bit */,
> + (*stride) * y, ALLOC_TAG);
> + *phys_mem = (QXLPHYSICAL)NULL; /* make sure no one uses it */
> + break;
> default:
> PANIC(pdev, "No allocation type");
> }
> @@ -813,10 +821,18 @@ void QXLGetSurface(PDev *pdev, QXLPHYSICAL *surface_phys, UINT32 x, UINT32 y, UI
>
> void QXLDelSurface(PDev *pdev, UINT8 *base_mem, UINT8 allocation_type)
> {
> - if (allocation_type == DEVICE_BITMAP_ALLOCATION_TYPE_DEVRAM) {
> + switch (allocation_type) {
> + case DEVICE_BITMAP_ALLOCATION_TYPE_DEVRAM:
> FreeMem(pdev, MSPACE_TYPE_DEVRAM, base_mem);
> - } else if (allocation_type == DEVICE_BITMAP_ALLOCATION_TYPE_VRAM) { // this wasn't there in the original code
> - FreeMem(pdev, MSPACE_TYPE_VRAM, base_mem);
> + break;
> + case DEVICE_BITMAP_ALLOCATION_TYPE_VRAM:
> + FreeMem(pdev, MSPACE_TYPE_VRAM, base_mem);
> + break;
> + case DEVICE_BITMAP_ALLOCATION_TYPE_RAM:
> + EngFreeMem(base_mem);
> + break;
> + default:
> + PANIC(pdev, "bad allocation type");
> }
> }
>
> @@ -832,18 +848,8 @@ static void FreeDelSurface(PDev *pdev, Resource *res)
> DEBUG_PRINT((pdev, 12, "%s\n", __FUNCTION__));
>
> internal = (InternalDelSurface *)res->res;
> - switch (internal->allocation_type) {
> - case DEVICE_BITMAP_ALLOCATION_TYPE_DEVRAM:
> - FreeMem(pdev, MSPACE_TYPE_DEVRAM,
> - GetSurfaceInfo(pdev, internal->surface_id)->draw_area.base_mem);
> - break;
> - case DEVICE_BITMAP_ALLOCATION_TYPE_VRAM:
> - FreeMem(pdev, MSPACE_TYPE_VRAM,
> - GetSurfaceInfo(pdev, internal->surface_id)->draw_area.base_mem);
> - break;
> - default:
> - PANIC(pdev, "bad allocation type");
> - }
> + QXLDelSurface(pdev, GetSurfaceInfo(pdev, internal->surface_id)->draw_area.base_mem,
> + internal->allocation_type);
> FreeSurfaceInfo(pdev, internal->surface_id);
> FreeMem(pdev, MSPACE_TYPE_DEVRAM, res);
>
> diff --git a/display/surface.c b/display/surface.c
> index d24e751..63e830c 100644
> --- a/display/surface.c
> +++ b/display/surface.c
> @@ -160,10 +160,19 @@ VOID DeleteDeviceBitmap(PDev *pdev, UINT32 surface_id, UINT8 allocation_type)
>
> if (allocation_type != DEVICE_BITMAP_ALLOCATION_TYPE_SURF0&&
> pdev->Res->surfaces_info[surface_id].draw_area.base_mem != NULL) {
> - QXLSurfaceCmd *surface;
>
> - surface = SurfaceCmd(pdev, QXL_SURFACE_CMD_DESTROY, surface_id);
> - QXLGetDelSurface(pdev, surface, surface_id, allocation_type);
> - PushSurfaceCmd(pdev, surface);
> + if (allocation_type == DEVICE_BITMAP_ALLOCATION_TYPE_RAM) {
> + /* server side this surface is already destroyed, just free it here */
> + ASSERT(pdev, pdev->Res->surfaces_info[surface_id].draw_area.base_mem ==
> + pdev->Res->surfaces_info[surface_id].copy);
> + QXLDelSurface(pdev, pdev->Res->surfaces_info[surface_id].draw_area.base_mem,
> + allocation_type);
> + FreeSurfaceInfo(pdev, surface_id);
> + } else {
> + QXLSurfaceCmd *surface_cmd;
> + surface_cmd = SurfaceCmd(pdev, QXL_SURFACE_CMD_DESTROY, surface_id);
> + QXLGetDelSurface(pdev, surface_cmd, surface_id, allocation_type);
> + PushSurfaceCmd(pdev, surface_cmd);
> + }
> }
> }
> diff --git a/display/surface.h b/display/surface.h
> index 0b7edb3..f723392 100644
> --- a/display/surface.h
> +++ b/display/surface.h
> @@ -98,6 +98,7 @@ enum {
> DEVICE_BITMAP_ALLOCATION_TYPE_SURF0,
> DEVICE_BITMAP_ALLOCATION_TYPE_DEVRAM,
> DEVICE_BITMAP_ALLOCATION_TYPE_VRAM,
> + DEVICE_BITMAP_ALLOCATION_TYPE_RAM,
> };
>
> HBITMAP CreateDeviceBitmap(PDev *pdev, SIZEL size, ULONG format, QXLPHYSICAL *phys_mem,
More information about the Spice-devel
mailing list