[Spice-devel] [PATCH] display/driver: reimplement DrvAssertMode for Suspend+Hibernate (S3+S4) support

[Alon Levy]

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 <yhalperi at 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 740069f..02ee8d0 100644
--- a/display/driver.c
+++ b/display/driver.c
@@ -937,7 +937,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) {
@@ -951,25 +953,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