our old code did a very minimal flow good for resolution change: DrvAssertMode False (disable) destroy primary surface delete vram memslot DrvAssertMode True (enable) create primary surface (destroyed on disable) create vram memslot
Aside: Importantly the flow for resolution change involves two pdevs, so actually the enable call is not called, only: DrvAssertMode(PDEV#1, FALSE) DrvEnableSurface(PDEV#2) EnableSurface creates a primary surface, so the call to AssertMode must destroy it. This fails on suspend for many reasons, one of them being that we don't disable operations to any driver managed off screen surfaces, and the other is that after the qxl reset done via acpi S3 request by windows, we don't reinitialize the primary memslot (this is fixed by a previous commit). The correct (per example drivers from WinDDK, and per msdn) thing to do in DrvAssertMode is to not do any further interaction with the device. The simplest way to achieve that is to fail any operation. The GDI is designed such that it can work completely without any driver, so for any operation there is a fallback in case the driver returns a failure error code. A simplification is to use EngModifySurface to move a surface to GDI control and so not to get any further callback on it (except for the deletion callback when it is deleted). This is also done in the 3dlabs example driver. There is zero synchronization between the miniport, which knows about the power state, and the displayport, which knows about the pci device structure, command rings, resources. As a result the easiest and also consistent with the above requirements implementation for suspend to ram and to disk is to reset any server side state during DrvAssertMode(FALSE), copying all volatile (surface contents only atm) memory to guest ram (from there windows will copy it to disk for us if we are hibernating). So the new flow for DriveAssertMode is then: AssertModeDisable: 1. set pdev->enable to False (all further operations will be punted) 2. tell server to prepare for sleep, via new QXL_IO_UPDATE_MEM(QXL_UPDATE_MEM_RENDER_ALL): server updates all surfaces server destroys all surfaces. Since we never sent it a destroy command this doesn't trigger any guest side surface destruction, only a release of the creation command resources. 3. release anything in the release ring. 4. tell server to write it's current releasable items list (qxl->last_release) to the release_ring (we just made sure the release_ring is empty, and since we are not sending anything new to the worker and we already had it render anything outstanding it will not fill it) release the last resources (at this point there is nothing allocated on devram and vram - we verify that in debug mode with DUMP_MSPACE_VRAM and DUMP_MSPACE_DEVRAM). 5. Destroy primary surface 6. Delete vram memslot AssertModeEnable: 1. Create primary surface 2. create vram memslot 3. copy surfaces from ram to vram, sending create commands which cause both create and surface image commands to be sent to the client. In suspend there is a single PDEV involved which does exactly those two calls, disable before sleep and enable after. For resolution change this work is excessive but correct. Since miniport:SetPowerState is called after DrvAssertMode during suspend (actually there is no defined order in the documentation), there is no way to distinguish between the two anyway. Cc: Yonit Halperin <yhalp...@redhat.com> --- display/driver.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++------ display/surface.c | 1 + 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/display/driver.c b/display/driver.c index a231bb4..fff462d 100644 --- a/display/driver.c +++ b/display/driver.c @@ -939,7 +939,9 @@ VOID DrvDisableSurface(DHPDEV in_pdev) DEBUG_PRINT((pdev, 1, "%s: 0x%lx\n", __FUNCTION__, pdev)); - DisableQXLPrimarySurface(pdev, 1 /* hide mouse */); + // Don't destroy the primary - it's destroyed by destroy_all_surfaces + // at AssertModeDisable + pdev->surf_enable = FALSE; UnmapFB(pdev); if (pdev->surf) { @@ -953,25 +955,76 @@ VOID DrvDisableSurface(DHPDEV in_pdev) pdev->mem_slots = NULL; } - DEBUG_PRINT((NULL, 1, "%s: 0x%lx exit\n", __FUNCTION__, pdev)); + DEBUG_PRINT((pdev, 1, "%s: 0x%lx exit\n", __FUNCTION__, pdev)); +} + +static BOOL AssertModeDisable(PDev *pdev) +{ + DEBUG_PRINT((pdev, 3, "%s entry\n", __FUNCTION__)); + /* flush command ring and update all surfaces */ + WRITE_PORT_UCHAR(pdev->notify_cmd_port, 0); + WRITE_PORT_UCHAR(pdev->notify_cursor_port, 0); + WRITE_PORT_UCHAR(pdev->flush_surfaces_port, 0); + WRITE_PORT_UCHAR(pdev->destroy_all_surfaces_port, 0); + /* move all surfaces from device to system memory */ + if (!MoveAllSurfacesToRam(pdev)) { + return FALSE; + } + /* Free release ring contents */ + ReleaseCacheDeviceMemoryResources(pdev); + EmptyReleaseRing(pdev); + /* Get the last free list onto the release ring */ + WRITE_PORT_UCHAR(pdev->flush_release_port, 0); + DEBUG_PRINT((pdev, 4, "%s after FLUSH_RELEASE\n", __FUNCTION__)); + /* And release that. mspace allocators should be clean after. */ + EmptyReleaseRing(pdev); + RemoveVRamSlot(pdev); + DebugCountAliveSurfaces(pdev); + DEBUG_PRINT((pdev, 4, "%s: [%d,%d] [%d,%d] [%d,%d] %lx\n", __FUNCTION__, + pdev->cmd_ring->prod, pdev->cmd_ring->cons, + pdev->cursor_ring->prod, pdev->cursor_ring->cons, + pdev->release_ring->prod, pdev->release_ring->cons, + pdev->Res->free_outputs)); + DEBUG_PRINT((pdev, 4, "%s exit\n", __FUNCTION__)); + return TRUE; +} + +static void AssertModeEnable(PDev *pdev) +{ + InitResources(pdev); + InitDeviceMemoryResources(pdev); + DEBUG_PRINT((pdev, 3, "%s: [%d,%d] [%d,%d] [%d,%d] %lx\n", __FUNCTION__, + pdev->cmd_ring->prod, pdev->cmd_ring->cons, + pdev->cursor_ring->prod, pdev->cursor_ring->cons, + pdev->release_ring->prod, pdev->release_ring->cons, + pdev->Res->free_outputs)); + EnableQXLPrimarySurface(pdev); + CreateVRamSlot(pdev); + DebugCountAliveSurfaces(pdev); + MoveAllSurfacesToVideoRam(pdev); + DebugCountAliveSurfaces(pdev); } BOOL DrvAssertMode(DHPDEV in_pdev, BOOL enable) { PDev* pdev = (PDev*)in_pdev; + BOOL ret = TRUE; DEBUG_PRINT((pdev, 1, "%s: 0x%lx %d\n", __FUNCTION__, pdev, enable)); + if (pdev->enabled == enable) { + DEBUG_PRINT((pdev, 1, "%s: called twice with same argument (%d)\n", __FUNCTION__, + enable)); + return TRUE; + } + pdev->enabled = enable; if (enable) { - InitResources(pdev); - EnableQXLPrimarySurface(pdev); - CreateVRamSlot(pdev); + AssertModeEnable(pdev); } else { - DisableQXLPrimarySurface(pdev, 0); - RemoveVRamSlot(pdev); + ret = AssertModeDisable(pdev); } DEBUG_PRINT((pdev, 1, "%s: 0x%lx exit %d\n", __FUNCTION__, pdev, enable)); - return TRUE; + return ret; } ULONG DrvGetModes(HANDLE driver, ULONG dev_modes_size, DEVMODEW *dev_modes) diff --git a/display/surface.c b/display/surface.c index 8f7aae1..b8628fe 100644 --- a/display/surface.c +++ b/display/surface.c @@ -249,6 +249,7 @@ BOOL MoveSurfaceToVideoRam(PDev *pdev, UINT32 surface_id) surface_info->phys_mem = phys_mem; surface_info->draw_area.base_mem = base_mem; surface = SurfaceCmd(pdev, QXL_SURFACE_CMD_CREATE, surface_id); + surface->flags |= QXL_SURF_FLAG_KEEP_DATA; surface->u.surface_create.format = surface_format; surface->u.surface_create.width = cx; surface->u.surface_create.height = cy; -- 1.7.5.4 _______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel