[Spice-commits] 7 commits - display/driver.c display/qxldd.h display/res.c display/surface.c display/surface.h
Alexander Larsson
alexl at kemper.freedesktop.org
Tue Aug 24 05:02:49 PDT 2010
display/driver.c | 30 +++++---------
display/qxldd.h | 14 ++++--
display/res.c | 112 ++++++++++++++++++++++++++++++++----------------------
display/surface.c | 4 -
display/surface.h | 26 ++++++++----
5 files changed, 106 insertions(+), 80 deletions(-)
New commits:
commit 056b6da899b93f5a1038111cbe6a1e8598d4aa32
Author: Alexander Larsson <alexl at redhat.com>
Date: Mon Aug 23 15:28:53 2010 +0200
Remove unused surfaces_used
diff --git a/display/qxldd.h b/display/qxldd.h
index 2cb2db4..82886a9 100644
--- a/display/qxldd.h
+++ b/display/qxldd.h
@@ -198,7 +198,6 @@ typedef struct DevRes {
SurfaceInfo *surfaces_info;
SurfaceInfo *free_surfaces;
- UINT8 *surfaces_used;
HANDLE driver;
#ifdef DBG
diff --git a/display/res.c b/display/res.c
index e20dc96..d466ce7 100644
--- a/display/res.c
+++ b/display/res.c
@@ -377,10 +377,6 @@ void CleanGlobalRes()
EngFreeMem(global_res[i].dynamic);
global_res[i].dynamic = NULL;
}
- if (global_res[i].surfaces_used) {
- EngFreeMem(global_res[i].surfaces_used);
- global_res[i].surfaces_used = NULL;
- }
if (global_res[i].surfaces_info) {
EngFreeMem(global_res[i].surfaces_info);
global_res[i].surfaces_info = NULL;
@@ -425,13 +421,7 @@ static void InitRes(PDev *pdev)
PANIC(pdev, "Res dynamic allocation failed\n");
}
- pdev->Res.surfaces_used = EngAllocMem(FL_ZERO_MEMORY, sizeof(UINT8) * pdev->n_surfaces,
- ALLOC_TAG);
- if (!pdev->Res.surfaces_used) {
- PANIC(pdev, "Res surfaces_used allocation failed\n");
- }
-
- pdev->Res.surfaces_info = (SurfaceInfo *)EngAllocMem(FL_ZERO_MEMORY,
+ pdev->Res.surfaces_info = (SurfaceInfo *)EngAllocMem(FL_ZERO_MEMORY,
sizeof(SurfaceInfo) * pdev->n_surfaces,
ALLOC_TAG);
if (!pdev->Res.surfaces_info) {
commit ebbfce22c79945884b7851857338e0fa7f12979c
Author: Alexander Larsson <alexl at redhat.com>
Date: Mon Aug 23 15:21:10 2010 +0200
New list based surface allocator
We store a free list in the SurfaceInfos, using a field not otherwise
used for free surfaces. Also, treat base_mem == NULL as "surface in use"
for easy checking.
diff --git a/display/qxldd.h b/display/qxldd.h
index 7c5b6ec..2cb2db4 100644
--- a/display/qxldd.h
+++ b/display/qxldd.h
@@ -197,6 +197,7 @@ typedef struct DevRes {
UINT32 num_palettes;
SurfaceInfo *surfaces_info;
+ SurfaceInfo *free_surfaces;
UINT8 *surfaces_used;
HANDLE driver;
diff --git a/display/res.c b/display/res.c
index eef6a49..e20dc96 100644
--- a/display/res.c
+++ b/display/res.c
@@ -437,6 +437,10 @@ static void InitRes(PDev *pdev)
if (!pdev->Res.surfaces_info) {
PANIC(pdev, "Res surfaces_info allocation failed\n");
}
+ pdev->Res.free_surfaces = &pdev->Res.surfaces_info[0];
+ for (i = 0; i < pdev->n_surfaces - 1; i++) {
+ pdev->Res.surfaces_info[i].u.next_free = &pdev->Res.surfaces_info[i+1];
+ }
pdev->Res.free_outputs = 0;
pdev->Res.malloc_sem = EngCreateSemaphore();
diff --git a/display/surface.c b/display/surface.c
index 54e0d0c..bfffb7f 100644
--- a/display/surface.c
+++ b/display/surface.c
@@ -163,7 +163,7 @@ VOID DeleteDeviceBitmap(PDev *pdev, UINT32 surface_id, UINT8 allocation_type)
FreeDrawArea(drawarea);
if (allocation_type != DEVICE_BITMAP_ALLOCATION_TYPE_SURF0 &&
- pdev->Res.surfaces_used[surface_id]) {
+ pdev->Res.surfaces_info[surface_id].draw_area.base_mem != NULL) {
QXLSurfaceCmd *surface;
surface = SurfaceCmd(pdev, QXL_SURFACE_CMD_DESTROY, surface_id);
diff --git a/display/surface.h b/display/surface.h
index 5504793..27e8239 100644
--- a/display/surface.h
+++ b/display/surface.h
@@ -32,23 +32,31 @@ static _inline UINT32 GetSurfaceId(SURFOBJ *surf)
static _inline void FreeSurface(PDev *pdev, UINT32 surface_id)
{
- pdev->Res.surfaces_used[surface_id] = 0;
+ SurfaceInfo *surface;
+
+ if (surface_id == 0) {
+ return;
+ }
+ surface = &pdev->Res.surfaces_info[surface_id];
+ surface->draw_area.base_mem = NULL; /* Mark as not used */
+ surface->u.next_free = pdev->Res.free_surfaces;
+ pdev->Res.free_surfaces = surface;
}
static UINT32 GetFreeSurface(PDev *pdev)
{
UINT32 x;
+ SurfaceInfo *surface;
- //not effective, fix me
- for (x = 1; x < pdev->n_surfaces; ++x) {
- if (!pdev->Res.surfaces_used[x]) {
- pdev->Res.surfaces_used[x] = 1;
- return x;
- }
+ surface = pdev->Res.free_surfaces;
+ if (surface == NULL) {
+ return 0;
}
- return 0;
+ pdev->Res.free_surfaces = surface->u.next_free;
+
+ return surface - pdev->Res.surfaces_info;
}
enum {
commit da7f343123bca38a1f1beb2facd2be622fe1bdbc
Author: Alexander Larsson <alexl at redhat.com>
Date: Mon Aug 23 15:18:10 2010 +0200
Add union to SurfaceInfo to put in next_free pointer
diff --git a/display/driver.c b/display/driver.c
index 1c8e1f6..f088df5 100644
--- a/display/driver.c
+++ b/display/driver.c
@@ -1268,7 +1268,7 @@ VOID APIENTRY DrvDeleteDeviceBitmap(DHSURF dhsurf)
surface = (SurfaceInfo *)dhsurf;
surface_id = GetSurfaceIdFromInfo(surface);
- DeleteDeviceBitmap(surface->pdev, surface_id, DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
+ DeleteDeviceBitmap(surface->u.pdev, surface_id, DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
}
#ifdef CALL_TEST
diff --git a/display/qxldd.h b/display/qxldd.h
index 868a1a9..7c5b6ec 100644
--- a/display/qxldd.h
+++ b/display/qxldd.h
@@ -169,10 +169,14 @@ typedef struct DrawArea {
UINT8 *base_mem;
} DrawArea;
-typedef struct SurfaceInfo {
+typedef struct SurfaceInfo SurfaceInfo;
+struct SurfaceInfo {
DrawArea draw_area;
- PDev *pdev;
-} SurfaceInfo;
+ union {
+ PDev *pdev;
+ SurfaceInfo *next_free;
+ } u;
+};
typedef struct DevRes {
MspaceInfo mspaces[NUM_MSPACES];
diff --git a/display/surface.c b/display/surface.c
index d9fc6f4..54e0d0c 100644
--- a/display/surface.c
+++ b/display/surface.c
@@ -119,7 +119,7 @@ HBITMAP CreateDeviceBitmap(PDev *pdev, SIZEL size, ULONG format, QXLPHYSICAL *ph
goto out_error2;
}
- GetSurfaceInfo(pdev, surface_id)->pdev = pdev;
+ GetSurfaceInfo(pdev, surface_id)->u.pdev = pdev;
QXLGetSurface(pdev, phys_mem, size.cx, size.cy, depth,
&stride, base_mem, allocation_type);
diff --git a/display/surface.h b/display/surface.h
index dfda502..5504793 100644
--- a/display/surface.h
+++ b/display/surface.h
@@ -7,7 +7,7 @@ static _inline UINT32 GetSurfaceIdFromInfo(SurfaceInfo *info)
{
PDev *pdev;
- pdev = info->pdev;
+ pdev = info->u.pdev;
if (info == &pdev->surface0_info) {
return 0;
}
commit 84bc7af08210a97abc486a662a138a41cf664319
Author: Alexander Larsson <alexl at redhat.com>
Date: Mon Aug 23 14:56:53 2010 +0200
Free surface id if surface construction failed
diff --git a/display/driver.c b/display/driver.c
index 46df8f2..1c8e1f6 100644
--- a/display/driver.c
+++ b/display/driver.c
@@ -1248,14 +1248,16 @@ HBITMAP APIENTRY DrvCreateDeviceBitmap(DHPDEV dhpdev, SIZEL size, ULONG format)
hbitmap = CreateDeviceBitmap(pdev, size, pdev->bitmap_format, &phys_mem, &base_mem, surface_id,
DEVICE_BITMAP_ALLOCATION_TYPE_VRAM);
if (!hbitmap) {
- goto out_error;
+ goto out_error2;
}
return hbitmap;
// to optimize the failure case
+out_error2:
+ FreeSurface(pdev, surface_id);
out_error:
- return 0;
+ return 0;
}
VOID APIENTRY DrvDeleteDeviceBitmap(DHSURF dhsurf)
commit 594c584b4669521b85dbe354af7ccfa2bcf9b617
Author: Alexander Larsson <alexl at redhat.com>
Date: Mon Aug 23 14:38:51 2010 +0200
Release resources before allocating
Before trying to allocate memory we free a large chunk of
freeable resources. Leaving the resources unfreed before allocating
increases the risks of unnecessarily fragmenting the heap, which
is bad, especially for the vram which needs large free contiguous
allocations for surfaces.
We do somewhat limit the number of resources we free on each call
to ensure that we get an approximately even spread of the cost of
allocations.
diff --git a/display/res.c b/display/res.c
index 6fe375f..eef6a49 100644
--- a/display/res.c
+++ b/display/res.c
@@ -36,8 +36,6 @@
#define WAIT_FOR_EVENT(pdev, event, timeout) EngWaitForSingleObject(event, timeout)
#endif
-#define SURFACE_ALLOC_RELEASE_BUNCH_SIZE 3
-
static _inline QXLPHYSICAL PA(PDev *pdev, PVOID virt, UINT8 slot_id)
{
PMemSlot *p_slot = &pdev->mem_slots[slot_id];
@@ -285,10 +283,36 @@ static void WaitForReleaseRing(PDev* pdev)
DEBUG_PRINT((pdev, 16, "%s: 0x%lx, done\n", __FUNCTION__, pdev));
}
+static void FlushReleaseRing(PDev *pdev)
+{
+ UINT64 output;
+ int notify;
+ int num_to_release = 50;
+
+ output = pdev->Res.free_outputs;
+
+ while (1) {
+ while (output != 0) {
+ output = ReleaseOutput(pdev, output);
+ if (--num_to_release == 0) {
+ break;
+ }
+ }
+
+ if (SPICE_RING_IS_EMPTY(pdev->release_ring)) {
+ break;
+ }
+
+ output = *SPICE_RING_CONS_ITEM(pdev->release_ring);
+ SPICE_RING_POP(pdev->release_ring, notify);
+ }
+
+ pdev->Res.free_outputs = output;
+}
+
// todo: separate VRAM releases from DEVRAM releases
-#define AllocMem(pdev, mspace_type, size) __AllocMem(pdev, mspace_type, size, TRUE, 1)
-static void *__AllocMem(PDev* pdev, UINT32 mspace_type, size_t size,
- BOOL force, INT32 release_bunch)
+#define AllocMem(pdev, mspace_type, size) __AllocMem(pdev, mspace_type, size, TRUE)
+static void *__AllocMem(PDev* pdev, UINT32 mspace_type, size_t size, BOOL force)
{
UINT8 *ptr;
@@ -296,35 +320,32 @@ static void *__AllocMem(PDev* pdev, UINT32 mspace_type, size_t size,
DEBUG_PRINT((pdev, 12, "%s: 0x%lx size %u\n", __FUNCTION__, pdev, size));
EngAcquireSemaphore(pdev->Res.malloc_sem);
- while (!(ptr = mspace_malloc(pdev->Res.mspaces[mspace_type]._mspace, size))) {
- int notify;
- int num_to_release = release_bunch;
- while (pdev->Res.free_outputs && num_to_release) {
- pdev->Res.free_outputs = ReleaseOutput(pdev, pdev->Res.free_outputs);
- num_to_release--;
+ while (1) {
+ /* Release lots of queued resources, before allocating, as we
+ want to release early to minimize fragmentation risks. */
+ FlushReleaseRing(pdev);
+
+ ptr = mspace_malloc(pdev->Res.mspaces[mspace_type]._mspace, size);
+ if (ptr) {
+ break;
}
- if (!num_to_release) {
+ if (pdev->Res.free_outputs != 0 ||
+ !SPICE_RING_IS_EMPTY(pdev->release_ring)) {
+ /* We have more things to free, try that */
continue;
}
if (force) {
+ /* Ask spice to free some stuff */
WaitForReleaseRing(pdev);
} else {
- if (SPICE_RING_IS_EMPTY(pdev->release_ring)) {
- break;
- }
- }
-
- pdev->Res.free_outputs = *SPICE_RING_CONS_ITEM(pdev->release_ring);
- SPICE_RING_POP(pdev->release_ring, notify);
-
- while (pdev->Res.free_outputs && num_to_release) {
- pdev->Res.free_outputs = ReleaseOutput(pdev, pdev->Res.free_outputs);
- num_to_release--;
+ /* Fail */
+ break;
}
}
+
EngReleaseSemaphore(pdev->Res.malloc_sem);
ASSERT(pdev, (!ptr && !force) || (ptr >= pdev->Res.mspaces[mspace_type].mspace_start &&
ptr < pdev->Res.mspaces[mspace_type].mspace_end));
@@ -630,8 +651,7 @@ _inline void GetSurfaceMemory(PDev *pdev, UINT32 x, UINT32 y, UINT32 depth, UINT
case DEVICE_BITMAP_ALLOCATION_TYPE_VRAM: {
*stride = x * depth / 8;
*stride = ALIGN(*stride, 4);
- *base_mem = __AllocMem(pdev, MSPACE_TYPE_VRAM, (*stride) * y, FALSE,
- SURFACE_ALLOC_RELEASE_BUNCH_SIZE);
+ *base_mem = __AllocMem(pdev, MSPACE_TYPE_VRAM, (*stride) * y, FALSE);
*phys_mem = PA(pdev, (PVOID)((UINT64)*base_mem), pdev->vram_mem_slot);
break;
}
@@ -1339,14 +1359,11 @@ static CacheImage *AllocCacheImage(PDev* pdev)
{
RingItem *item;
while (!(item = RingGetTail(pdev, &pdev->Res.dynamic->cache_image_lru))) {
- int notify;
- if (pdev->Res.free_outputs) {
- pdev->Res.free_outputs = ReleaseOutput(pdev, pdev->Res.free_outputs);
- continue;
+ if (pdev->Res.free_outputs == 0 &&
+ SPICE_RING_IS_EMPTY(pdev->release_ring)) {
+ WaitForReleaseRing(pdev);
}
- WaitForReleaseRing(pdev);
- pdev->Res.free_outputs = *SPICE_RING_CONS_ITEM(pdev->release_ring);
- SPICE_RING_POP(pdev->release_ring, notify);
+ FlushReleaseRing(pdev);
}
RingRemove(pdev, item);
return CONTAINEROF(item, CacheImage, lru_link);
commit e10b5a283779d4d180ff398c172b392d9ced70cc
Author: Alexander Larsson <alexl at redhat.com>
Date: Mon Aug 23 12:45:07 2010 +0200
Grab malloc_sem in FreeMem
This is needed as much as in AllocMem to protect the mspaces data
from concurrent access.
diff --git a/display/res.c b/display/res.c
index 1f67dbf..6fe375f 100644
--- a/display/res.c
+++ b/display/res.c
@@ -337,7 +337,9 @@ static void FreeMem(PDev* pdev, UINT32 mspace_type, void *ptr)
ASSERT(pdev, pdev && pdev->Res.mspaces[mspace_type]._mspace);
ASSERT(pdev, (UINT8 *)ptr >= pdev->Res.mspaces[mspace_type].mspace_start &&
(UINT8 *)ptr < pdev->Res.mspaces[mspace_type].mspace_end);
+ EngAcquireSemaphore(pdev->Res.malloc_sem);
mspace_free(pdev->Res.mspaces[mspace_type]._mspace, ptr);
+ EngReleaseSemaphore(pdev->Res.malloc_sem);
}
DevRes *global_res = NULL;
commit 8b5f476e2087f4708a3bc744583bf42eb8ea9ab8
Author: Alexander Larsson <alexl at redhat.com>
Date: Mon Aug 23 12:43:11 2010 +0200
Make malloc_sem global
It protects shared data (mspaces info) so it needs to be shared.
diff --git a/display/driver.c b/display/driver.c
index 742348c..46df8f2 100644
--- a/display/driver.c
+++ b/display/driver.c
@@ -505,24 +505,19 @@ DHPDEV DrvEnablePDEV(DEVMODEW *dev_mode, PWSTR ignore1, ULONG ignore2, HSURF *ig
goto err1;
}
- if (!(pdev->malloc_sem = EngCreateSemaphore())) {
- DEBUG_PRINT((NULL, 0, "%s: create malloc sem failed\n", __FUNCTION__));
- goto err2;
- }
-
if (!(pdev->print_sem = EngCreateSemaphore())) {
- DEBUG_PRINT((NULL, 0, "%s: create malloc sem failed\n", __FUNCTION__));
- goto err3;
+ DEBUG_PRINT((NULL, 0, "%s: create print sem failed\n", __FUNCTION__));
+ goto err2;
}
if (!(pdev->cmd_sem = EngCreateSemaphore())) {
DEBUG_PRINT((NULL, 0, "%s: create cmd sem failed\n", __FUNCTION__));
- goto err4;
+ goto err3;
}
if (!ResInit(pdev)) {
DEBUG_PRINT((NULL, 0, "%s: init res failed\n", __FUNCTION__));
- goto err5;
+ goto err4;
}
RtlCopyMemory(dev_caps, &gdi_info, dev_caps_size);
@@ -531,14 +526,10 @@ DHPDEV DrvEnablePDEV(DEVMODEW *dev_mode, PWSTR ignore1, ULONG ignore2, HSURF *ig
DEBUG_PRINT((NULL, 1, "%s: 0x%lx\n", __FUNCTION__, pdev));
return(DHPDEV)pdev;
-err5:
- EngDeleteSemaphore(pdev->cmd_sem);
err4:
- EngDeleteSemaphore(pdev->print_sem);
-
+ EngDeleteSemaphore(pdev->cmd_sem);
err3:
- EngDeleteSemaphore(pdev->malloc_sem);
-
+ EngDeleteSemaphore(pdev->print_sem);
err2:
DestroyPalette(pdev);
@@ -556,7 +547,6 @@ VOID DrvDisablePDEV(DHPDEV in_pdev)
ResDestroy(pdev);
DestroyPalette(pdev);
EngDeleteSemaphore(pdev->cmd_sem);
- EngDeleteSemaphore(pdev->malloc_sem);
EngDeleteSemaphore(pdev->print_sem);
EngFreeMem(pdev);
DEBUG_PRINT((NULL, 1, "%s: 0x%lx exit\n", __FUNCTION__, pdev));
diff --git a/display/qxldd.h b/display/qxldd.h
index 7d22a0d..868a1a9 100644
--- a/display/qxldd.h
+++ b/display/qxldd.h
@@ -176,6 +176,7 @@ typedef struct SurfaceInfo {
typedef struct DevRes {
MspaceInfo mspaces[NUM_MSPACES];
+ HSEMAPHORE malloc_sem;
BOOL need_init;
UINT64 free_outputs;
@@ -258,7 +259,6 @@ typedef struct PDev {
UINT8 *log_buf;
UINT32 *log_level;
- HSEMAPHORE malloc_sem;
HSEMAPHORE print_sem;
HSEMAPHORE cmd_sem;
diff --git a/display/res.c b/display/res.c
index 15e14ae..1f67dbf 100644
--- a/display/res.c
+++ b/display/res.c
@@ -295,7 +295,7 @@ static void *__AllocMem(PDev* pdev, UINT32 mspace_type, size_t size,
ASSERT(pdev, pdev && pdev->Res.mspaces[mspace_type]._mspace);
DEBUG_PRINT((pdev, 12, "%s: 0x%lx size %u\n", __FUNCTION__, pdev, size));
- EngAcquireSemaphore(pdev->malloc_sem);
+ EngAcquireSemaphore(pdev->Res.malloc_sem);
while (!(ptr = mspace_malloc(pdev->Res.mspaces[mspace_type]._mspace, size))) {
int notify;
int num_to_release = release_bunch;
@@ -325,7 +325,7 @@ static void *__AllocMem(PDev* pdev, UINT32 mspace_type, size_t size,
num_to_release--;
}
}
- EngReleaseSemaphore(pdev->malloc_sem);
+ EngReleaseSemaphore(pdev->Res.malloc_sem);
ASSERT(pdev, (!ptr && !force) || (ptr >= pdev->Res.mspaces[mspace_type].mspace_start &&
ptr < pdev->Res.mspaces[mspace_type].mspace_end));
DEBUG_PRINT((pdev, 13, "%s: 0x%lx done 0x%x\n", __FUNCTION__, pdev, ptr));
@@ -362,6 +362,10 @@ void CleanGlobalRes()
EngFreeMem(global_res[i].surfaces_info);
global_res[i].surfaces_info = NULL;
}
+ if (global_res[i].malloc_sem) {
+ EngDeleteSemaphore(global_res[i].malloc_sem);
+ global_res[i].malloc_sem = NULL;
+ }
}
EngFreeMem(global_res);
global_res = NULL;
@@ -412,6 +416,11 @@ static void InitRes(PDev *pdev)
}
pdev->Res.free_outputs = 0;
+ pdev->Res.malloc_sem = EngCreateSemaphore();
+ if (!pdev->Res.malloc_sem) {
+ PANIC(pdev, "Res malloc sem creation failed\n");
+ }
+
InitMspace(&pdev->Res, MSPACE_TYPE_DEVRAM, pdev->io_pages_virt, pdev->num_io_pages * PAGE_SIZE);
InitMspace(&pdev->Res, MSPACE_TYPE_VRAM, pdev->fb, pdev->fb_size);
pdev->Res.update_id = *pdev->dev_update_id;
More information about the Spice-commits
mailing list