Hi,
Generally, when parameters of a function are divided into several lines,
they should be aligned to the same column.
On 07/07/2011 12:43 PM, Alon Levy wrote:
Adds fields to SurfaceInfo to cache data previously only available
via SurfaceArea::draw_area.
Adds two functions to save and restore surfaces from ram:
MoveAllSurfacesToVideoRam
allocates and copies surfaces from vram to ram, and calls
EngModifySurface
with an empty hook list to make those surfaces completely managed by
gdi and not
us.
MoveAllSurfacesToRam
recreates surfaces on vram and calls EngModifySurface with
QXL_SURFACE_HOOKS, and
finally sends a QXL_SURFACE_CMD_CREATE with the valid data flag to
make the server
send a surface image message after the surface create message.
Cc: Yonit Halperin<yhalp...@redhat.com>
---
display/driver.c | 3 +-
display/qxldd.h | 8 ++
display/surface.c | 212
++++++++++++++++++++++++++++++++++++++++++++++++----
display/surface.h | 3 +
4 files changed, 208 insertions(+), 18 deletions(-)
diff --git a/display/driver.c b/display/driver.c
index 8375eff..bbad696 100644
--- a/display/driver.c
+++ b/display/driver.c
@@ -1302,7 +1302,8 @@ VOID APIENTRY DrvDeleteDeviceBitmap(DHSURF dhsurf)
ASSERT(pdev, surface_id< pdev->n_surfaces);
- DeleteDeviceBitmap(surface->u.pdev, surface_id,
DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
+ DeleteDeviceBitmap(surface->u.pdev, surface_id,
+ surface->copy ? DEVICE_BITMAP_ALLOCATION_TYPE_RAM :
DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
}
#ifdef CALL_TEST
diff --git a/display/qxldd.h b/display/qxldd.h
index af9cb3d..15be2fa 100644
--- a/display/qxldd.h
+++ b/display/qxldd.h
@@ -189,6 +189,14 @@ typedef struct DrawArea {
typedef struct SurfaceInfo SurfaceInfo;
struct SurfaceInfo {
DrawArea draw_area;
+ /* TODO: all these fields are dual to DrawArea. But we destroy
draw_area when
+ * switching to ram. Can we not destroy it but just disable it and
keep using it's
+ * fields opaquely? */
I guess you meant the SURFOBJ inside DrawArea and the size and
bitmap_format fields. I think it is o.k to save these fields inside
surface_info, and we should unlock the SURFOBJ, since the surface its
referring to no longer exists. From msdn:
The driver is responsible for unlocking the surface when it no longer
needs it. Surfaces should be locked only for very short periods of time.
+ QXLPHYSICAL phys_mem;
we don't use this field
+ HBITMAP hbitmap;
+ SIZEL size;
+ UINT8 *copy;
+ ULONG bitmap_format;
union {
PDev *pdev;
SurfaceInfo *next_free;
diff --git a/display/surface.c b/display/surface.c
index 63e830c..8f7aae1 100644
--- a/display/surface.c
+++ b/display/surface.c
@@ -83,48 +83,70 @@ static VOID FreeDrawArea(DrawArea *drawarea)
}
}
-HBITMAP CreateDeviceBitmap(PDev *pdev, SIZEL size, ULONG format,
QXLPHYSICAL *phys_mem,
- UINT8 **base_mem, UINT32 surface_id, UINT8 allocation_type)
+static void BitmapFormatToDepthAndSurfaceFormat(ULONG format, UINT32
*depth, UINT32 *surface_format)
{
- UINT32 surface_format, depth;
- HBITMAP surf;
- UINT32 stride;
-
switch (format) {
case BMF_16BPP:
- surface_format = SPICE_SURFACE_FMT_16_555;
- depth = 16;
+ *surface_format = SPICE_SURFACE_FMT_16_555;
+ *depth = 16;
break;
case BMF_24BPP:
case BMF_32BPP:
- surface_format = SPICE_SURFACE_FMT_32_xRGB;
- depth = 32;
+ *surface_format = SPICE_SURFACE_FMT_32_xRGB;
+ *depth = 32;
break;
- case BMF_8BPP:
default:
- return 0;
+ *depth = 0;
+ break;
};
+}
+
- if (!(surf = EngCreateDeviceBitmap((DHSURF)GetSurfaceInfo(pdev,
surface_id), size, format))) {
+HBITMAP CreateDeviceBitmap(PDev *pdev, SIZEL size, ULONG format,
QXLPHYSICAL *phys_mem,
+ UINT8 **base_mem, UINT32 surface_id, UINT8 allocation_type)
+{
+ UINT32 surface_format, depth;
+ HBITMAP hbitmap;
+ UINT32 stride;
+ SurfaceInfo *surface_info;
+
+ BitmapFormatToDepthAndSurfaceFormat(format,&depth,&surface_format);
+ if (!depth) {
+ return 0;
+ }
+
+ DEBUG_PRINT((pdev, 9, "%s: %p: %d, (%dx%d), %d\n", __FUNCTION__,
pdev, surface_id,
+ size.cx, size.cy, format));
+ if (!(hbitmap = EngCreateDeviceBitmap((DHSURF)GetSurfaceInfo(pdev,
surface_id), size, format))) {
DEBUG_PRINT((pdev, 0, "%s: EngCreateDeviceBitmap failed, pdev 0x%lx,
surface_id=%d\n",
__FUNCTION__, pdev, surface_id));
goto out_error1;
}
- if (!EngAssociateSurface((HSURF)surf, pdev->eng, QXL_SURFACE_HOOKS)) {
+ if (!EngAssociateSurface((HSURF)hbitmap, pdev->eng,
QXL_SURFACE_HOOKS)) {
DEBUG_PRINT((pdev, 0, "%s: EngAssociateSurface failed\n", __FUNCTION__));
goto out_error2;
}
- GetSurfaceInfo(pdev, surface_id)->u.pdev = pdev;
+ surface_info = GetSurfaceInfo(pdev, surface_id);
+ surface_info->phys_mem = 0;
+ surface_info->u.pdev = pdev;
+ surface_info->hbitmap = hbitmap;
+ surface_info->copy = NULL;
+ surface_info->size = size;
+ surface_info->bitmap_format = format;
QXLGetSurface(pdev, phys_mem, size.cx, size.cy, depth,
&stride, base_mem, allocation_type);
if (!*base_mem) {
+ DEBUG_PRINT((pdev, 0, "%s: %p: QXLGetSurface failed (%d)\n",
__FUNCTION__, pdev,
+ allocation_type));
goto out_error2;
}
+ surface_info->phys_mem = *phys_mem;
if (!CreateDrawArea(pdev, *base_mem, format, size.cx, size.cy, stride,
surface_id)) {
+ DEBUG_PRINT((pdev, 0, "%s: CreateDrawArea failed\n", __FUNCTION__));
goto out_error3;
}
@@ -140,12 +162,12 @@ HBITMAP CreateDeviceBitmap(PDev *pdev, SIZEL
size, ULONG format, QXLPHYSICAL *ph
PushSurfaceCmd(pdev, surface);
}
- return surf;
+ return hbitmap;
out_error3:
QXLDelSurface(pdev, *base_mem, allocation_type);
out_error2:
- EngDeleteSurface((HSURF)surf);
+ EngDeleteSurface((HSURF)hbitmap);
out_error1:
return 0;
}
@@ -176,3 +198,159 @@ VOID DeleteDeviceBitmap(PDev *pdev, UINT32
surface_id, UINT8 allocation_type)
}
}
}
+
+BOOL MoveSurfaceToVideoRam(PDev *pdev, UINT32 surface_id)
+{
I think the part of QxlGetSurface and CreateDrawArea can be shared with
CreateDeviceBitmap
+ QXLSurfaceCmd *surface;
+ UINT32 surface_format;
+ UINT32 depth;
+ int count_used = 0;
+ int size;
+ INT32 stride = 0;
+ UINT8 *base_mem;
+ QXLPHYSICAL phys_mem;
+ SurfaceInfo *surface_info = GetSurfaceInfo(pdev, surface_id);
+ UINT32 cx = surface_info->size.cx;
+ UINT32 cy = surface_info->size.cy;
+
+
BitmapFormatToDepthAndSurfaceFormat(surface_info->bitmap_format,&depth,&surface_format);
+ ASSERT(pdev, depth != 0);
+ QXLGetSurface(pdev,&phys_mem, cx, cy, depth,&stride,
+&base_mem, DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
+ DEBUG_PRINT((pdev, 3,
+ "%s: %d, pm %0lX, fmt %d, d %d, s (%d, %d) st %d\n",
+ __FUNCTION__, surface_id, (uint64_t)surface_info->phys_mem,
surface_format,
+ depth, cx, cy, stride));
+ size = abs(stride) * cy;
+ if (!base_mem) {
+ DEBUG_PRINT((pdev, 0, "%s: %p: %d: QXLGetSurface failed (%d bytes
alloc)\n",
+ __FUNCTION__, pdev, surface_id, size));
+ return FALSE;
+ }
+ if (!CreateDrawArea(pdev, base_mem, surface_info->bitmap_format, cx,
cy, stride, surface_id)) {
+ DEBUG_PRINT((pdev, 0, "%s: %p: %d: CreateDrawArea failed\n",
+ __FUNCTION__, pdev, surface_id, size));
+ goto error_create_draw_area;
+ }
+ if (!EngModifySurface((HSURF)surface_info->hbitmap, pdev->eng,
QXL_SURFACE_HOOKS,
+ MS_NOTSYSTEMMEMORY, (DHSURF)surface_info, NULL, 0, NULL)) {
+ DEBUG_PRINT((pdev, 0, "%s: %p: %d: EngModifySurface failed\n",
+ __FUNCTION__, pdev, surface_id, size));
+ goto error_eng_modify_surface;
+ }
+ DEBUG_PRINT((pdev, 3, "%s: stride = %d, phys_mem = %p, base_mem =
%p\n",
+ __FUNCTION__, -stride, surface_info->phys_mem,
+ DEBUG_PRINT((pdev, 3, "%s: copy %d bytes to %d\n", __FUNCTION__,
size, surface_id));
+ RtlCopyMemory(surface_info->draw_area.base_mem, surface_info->copy,
size);
+ EngFreeMem(surface_info->copy);
+ surface_info->copy = NULL;
+ // Everything ok - commit changes to surface_info and send create
command to device.
+ surface_info->phys_mem = phys_mem;
+ surface_info->draw_area.base_mem = base_mem;
+ surface = SurfaceCmd(pdev, QXL_SURFACE_CMD_CREATE, surface_id);
+ surface->u.surface_create.format = surface_format;
+ surface->u.surface_create.width = cx;
+ surface->u.surface_create.height = cy;
+ surface->u.surface_create.stride = -stride;
+ surface->u.surface_create.data = surface_info->phys_mem;
+ PushSurfaceCmd(pdev, surface);
+ return TRUE;
+
+error_eng_modify_surface:
+ FreeDrawArea(&surface_info->draw_area);
+error_create_draw_area:
+ // TODO: Why did it fail? nothing in the MSDN
+ QXLDelSurface(pdev, base_mem, DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
+ return FALSE;
+}
+
+/* when we return from S3 we need to resend all the surface creation
commands.
+ * Actually moving the memory vram<->guest is not strictly neccessary
since vram
+ * is not reset during the suspend, so contents are not lost */
+VOID MoveAllSurfacesToVideoRam(PDev *pdev)
+{
+ UINT32 surface_id;
+ SurfaceInfo *surface_info;
+
+ /* brute force implementation - alternative is to keep an updated
used_surfaces list */
+ DEBUG_PRINT((pdev, 3, "%s\n", __FUNCTION__));
+
+ for (surface_id = 1 ; surface_id< pdev->n_surfaces ; ++surface_id) {
+ surface_info = GetSurfaceInfo(pdev, surface_id);
+ if (!surface_info->draw_area.base_mem) {
+ continue;
+ }
+ if (surface_info->u.pdev != pdev) {
+ DEBUG_PRINT((pdev, 0, "%s: %p: not out pdev (%p)\n", __FUNCTION__,
pdev,
+ surface_info->u.pdev));
+ continue;
+ }
+ if (surface_info->draw_area.surf_obj) {
Isn't it a bug if it happens? A surface info doesn't hold surf_obj after
the surface is deleted (DrvDeleteDeviceBitmap), but not yet fully
destroyed since the worker still holds references to it. But when we
move surfaces to RAM, we make sure the worker doesn't hold references to
surfaces, so for suce surfaces (base_mem == NULL).
Maybe for clarity also better to use macros like
#define SURFACE_IS_FREE(surface_info) (surface_info->draw_area.base_mem
== NULL)
+ DEBUG_PRINT((pdev, 0, "%s: surface_id = %d, surf_obj not empty\n",
__FUNCTION__,
+ surface_id));
+ continue;
+ }
+ if (surface_info->copy == NULL) {
Isn't it a bug if it happens?
+ DEBUG_PRINT((pdev, 0, "%s: %p: %d: no copy buffer, ignored\n",
__FUNCTION__,
+ pdev, surface_id));
+ }
+ MoveSurfaceToVideoRam(pdev, surface_id);
+ }
+}
+
+BOOL MoveAllSurfacesToRam(PDev *pdev)
+{
+ UINT32 surface_id;
+ SurfaceInfo *surface_info;
+ SURFOBJ *surf_obj;
+ UINT8 *copy;
+ UINT8 *line0;
+ int size;
+
+ for (surface_id = 1 ; surface_id< pdev->n_surfaces ; ++surface_id) {
+ surface_info = GetSurfaceInfo(pdev, surface_id);
+ if (!surface_info->draw_area.base_mem) {
macro
+ continue;
+ }
+ surf_obj = surface_info->draw_area.surf_obj;
+ if (!surf_obj) {
shouldn't happen
+ DEBUG_PRINT((pdev, 3, "%s: %d: no surfobj, not copying\n",
__FUNCTION__, surface_id));
+ continue;
+#if 0
+ DEBUG_PRINT((pdev, 0, "%s: %d: no surfobj, finishing release of
surface\n",
+ __FUNCTION__, surface_id));
+ QXLDelSurface(pdev, surface_info->draw_area.base_mem,
+ DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
+ FreeSurfaceInfo(pdev, surface_id);
+ continue;
+#endif
+ }
+ size = surf_obj->sizlBitmap.cy * abs(surf_obj->lDelta);
+ copy = EngAllocMem(0, size, ALLOC_TAG);
+ DEBUG_PRINT((pdev, 3, "%s: %d: copying #%d to %p (%d)\n",
__FUNCTION__, surface_id, size,
+ copy, surf_obj->lDelta));
+ RtlCopyMemory(copy, surface_info->draw_area.base_mem, size);
+ surface_info->copy = copy;
+ line0 = surf_obj->lDelta> 0 ? copy : copy + abs(surf_obj->lDelta) *
+ (surf_obj->sizlBitmap.cy - 1);
+ if (!EngModifySurface((HSURF)surface_info->hbitmap,
+ pdev->eng,
+ 0, /* from the example: used to monitor memory HOOK_COPYBITS |
HOOK_BITBLT, */
+ 0, /* It's system-memory */
+ (DHSURF)surface_info,
+ line0,
+ surf_obj->lDelta,
+ NULL)) {
+ /* NOTE: in this case we have a halfbreed - some surfaces are GDI
managed, some are
+ * managed by us. */
+ EngFreeMem(surface_info->copy);
+ surface_info->copy = NULL;
+ return FALSE;
+ }
+ QXLDelSurface(pdev, surface_info->draw_area.base_mem,
DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
+ surface_info->draw_area.base_mem = copy;
+ FreeDrawArea(&surface_info->draw_area);
+ }
+ return TRUE;
+}
diff --git a/display/surface.h b/display/surface.h
index f723392..0ce4e6c 100644
--- a/display/surface.h
+++ b/display/surface.h
@@ -105,4 +105,7 @@ HBITMAP CreateDeviceBitmap(PDev *pdev, SIZEL size,
ULONG format, QXLPHYSICAL *ph
UINT8 **base_mem, UINT32 surface_id, UINT8 allocation_type);
VOID DeleteDeviceBitmap(PDev *pdev, UINT32 surface_id, UINT8
allocation_type);
+VOID MoveAllSurfacesToVideoRam(PDev *pdev);
+BOOL MoveAllSurfacesToRam(PDev *pdev);
+
#endif
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
