[PATCH 17/26] hw/display/apple-gfx: Asynchronous rendering and graphics update

[Phil Dennis-Jordan]

This change avoids doing expensive rendering while holding the BQL.
Rendering with the lock held is not only inefficient, it can also
cause deadlocks when the PV Graphics framework’s encode... method
causes a (synchronous) call to a callback, which in turn tries to
acquire the BQL.

Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
---
 hw/display/apple-gfx.h |  15 +++-
 hw/display/apple-gfx.m | 193 +++++++++++++++++++++++++++--------------
 2 files changed, 138 insertions(+), 70 deletions(-)

diff --git a/hw/display/apple-gfx.h b/hw/display/apple-gfx.h
index fa7fea6368..9d6d40795e 100644
--- a/hw/display/apple-gfx.h
+++ b/hw/display/apple-gfx.h
@@ -15,12 +15,14 @@ void apple_gfx_common_init(Object *obj, AppleGFXState *s, 
const char* obj_name);
 #include "qemu/osdep.h"
 #include "exec/memory.h"
 #include "ui/surface.h"
+#include <dispatch/dispatch.h>
 
 @class PGDeviceDescriptor;
 @protocol PGDevice;
 @protocol PGDisplay;
 @protocol MTLDevice;
 @protocol MTLTexture;
+@protocol MTLCommandQueue;
 
 typedef QTAILQ_HEAD(, PGTask_s) AppleGFXTaskList;
 
@@ -30,14 +32,21 @@ struct AppleGFXState {
     id<PGDisplay> pgdisp;
     AppleGFXTaskList tasks;
     QemuConsole *con;
-    void *vram;
     id<MTLDevice> mtl;
-    id<MTLTexture> texture;
+    id<MTLCommandQueue> mtl_queue;
     bool handles_frames;
     bool new_frame;
     bool cursor_show;
-    DisplaySurface *surface;
     QEMUCursor *cursor;
+
+    dispatch_queue_t render_queue;
+    /* The following fields should only be accessed from render_queue: */
+    bool gfx_update_requested;
+    bool new_frame_ready;
+    int32_t pending_frames;
+    void *vram;
+    DisplaySurface *surface;
+    id<MTLTexture> texture;
 };
 
 void apple_gfx_common_realize(AppleGFXState *s, PGDeviceDescriptor *desc);
diff --git a/hw/display/apple-gfx.m b/hw/display/apple-gfx.m
index 48463e5a1f..5855d1d7f5 100644
--- a/hw/display/apple-gfx.m
+++ b/hw/display/apple-gfx.m
@@ -35,6 +35,9 @@
 
 static Error *apple_gfx_mig_blocker;
 
+static void apple_gfx_render_frame_completed(AppleGFXState *s, void *vram,
+                                             id<MTLTexture> texture);
+
 static AppleGFXTask *apple_gfx_new_task(AppleGFXState *s, uint64_t len)
 {
     mach_vm_address_t task_mem;
@@ -105,41 +108,63 @@ static void apple_gfx_write(void *opaque, hwaddr offset, 
uint64_t val,
     },
 };
 
-static void apple_gfx_fb_update_display(void *opaque)
+static void apple_gfx_render_new_frame(AppleGFXState *s)
 {
-    AppleGFXState *s = opaque;
-
-    if (!s->new_frame || !s->handles_frames) {
+    BOOL r;
+    void *vram = s->vram;
+    uint32_t width = surface_width(s->surface);
+    uint32_t height = surface_height(s->surface);
+    MTLRegion region = MTLRegionMake2D(0, 0, width, height);
+    id<MTLCommandBuffer> command_buffer = [s->mtl_queue commandBuffer];
+    id<MTLTexture> texture = s->texture;
+    r = [s->pgdisp encodeCurrentFrameToCommandBuffer:command_buffer
+                                             texture:texture
+                                              region:region];
+    if (!r) {
         return;
     }
+    [texture retain];
+
+    [command_buffer retain];
+    [command_buffer addCompletedHandler:
+        ^(id<MTLCommandBuffer> cb)
+        {
+            dispatch_async(s->render_queue, ^{
+                apple_gfx_render_frame_completed(s, vram, texture);
+                [texture release];
+            });
+            [command_buffer release];
+        }];
+    [command_buffer commit];
+}
 
-    @autoreleasepool {
-        s->new_frame = false;
-
-        BOOL r;
-        uint32_t width = surface_width(s->surface);
-        uint32_t height = surface_height(s->surface);
-        MTLRegion region = MTLRegionMake2D(0, 0, width, height);
-        id<MTLCommandQueue> commandQueue = [s->mtl newCommandQueue];
-        id<MTLCommandBuffer> mipmapCommandBuffer = [commandQueue 
commandBuffer];
-
-        r = [s->pgdisp encodeCurrentFrameToCommandBuffer:mipmapCommandBuffer
-                                                 texture:s->texture
-                                                  region:region];
+static void copy_mtl_texture_to_surface_mem(id<MTLTexture> texture, void *vram)
+{
+    /* TODO: Skip this entirely on a pure Metal or headless/guest-only
+     * rendering path, else use a blit command encoder? Needs careful
+     * (double?) buffering design. */
+    size_t width = texture.width, height = texture.height;
+    MTLRegion region = MTLRegionMake2D(0, 0, width, height);
+    [texture getBytes:vram
+          bytesPerRow:(width * 4)
+        bytesPerImage:(width * height * 4)
+           fromRegion:region
+          mipmapLevel:0
+                slice:0];
+}
 
-        if (r != YES) {
-            return;
-        }
+static void apple_gfx_render_frame_completed(AppleGFXState *s, void *vram,
+                                             id<MTLTexture> texture)
+{
+    --s->pending_frames;
+    assert(s->pending_frames >= 0);
 
-        id<MTLBlitCommandEncoder> blitCommandEncoder = [mipmapCommandBuffer 
blitCommandEncoder];
-        [blitCommandEncoder endEncoding];
-        [mipmapCommandBuffer commit];
-        [mipmapCommandBuffer waitUntilCompleted];
-        [s->texture getBytes:s->vram bytesPerRow:(width * 4)
-                                     bytesPerImage: (width * height * 4)
-                                     fromRegion: region
-                                     mipmapLevel: 0
-                                     slice: 0];
+    if (vram != s->vram) {
+        /* Display mode has changed, drop this old frame. */
+        assert(texture != s->texture);
+        g_free(vram);
+    } else {
+        copy_mtl_texture_to_surface_mem(texture, vram);
 
         /* Need to render cursor manually if not supported by backend */
         if (!dpy_cursor_define_supported(s->con) && s->cursor && 
s->cursor_show) {
@@ -159,14 +184,40 @@ static void apple_gfx_fb_update_display(void *opaque)
             pixman_image_unref(image);
         }
 
-        dpy_gfx_update_full(s->con);
-
-        [commandQueue release];
+        if (s->gfx_update_requested) {
+            s->gfx_update_requested = false;
+            dpy_gfx_update_full(s->con);
+            graphic_hw_update_done(s->con);
+            s->new_frame_ready = false;
+        } else {
+            s->new_frame_ready = true;
+        }
+    }
+    if (s->pending_frames > 0) {
+        apple_gfx_render_new_frame(s);
     }
 }
 
+static void apple_gfx_fb_update_display(void *opaque)
+{
+    AppleGFXState *s = opaque;
+
+    dispatch_async(s->render_queue, ^{
+        if (s->pending_frames > 0) {
+            s->gfx_update_requested = true;
+        } else {
+            if (s->new_frame_ready) {
+                dpy_gfx_update_full(s->con);
+                s->new_frame_ready = false;
+            }
+            graphic_hw_update_done(s->con);
+        }
+    });
+}
+
 static const GraphicHwOps apple_gfx_fb_ops = {
     .gfx_update = apple_gfx_fb_update_display,
+    .gfx_update_async = true,
 };
 
 static void update_cursor(AppleGFXState *s)
@@ -182,28 +233,23 @@ static void update_cursor(AppleGFXState *s)
 static void set_mode(AppleGFXState *s, uint32_t width, uint32_t height)
 {
     void *vram = NULL;
-    void *old_vram = s->vram;
     DisplaySurface *surface;
     MTLTextureDescriptor *textureDescriptor;
-    id<MTLTexture> old_texture = nil;
     id<MTLTexture> texture = nil;
-    bool locking_required = false;
-
-    locking_required = !bql_locked();
-    if (locking_required) {
-        bql_lock();
-    }
-    if (s->surface &&
-        width == surface_width(s->surface) &&
-        height == surface_height(s->surface)) {
-        if (locking_required) {
-            bql_unlock();
-        }
+    __block bool no_change = false;
+
+    dispatch_sync(s->render_queue,
+        ^{
+            if (s->surface &&
+                width == surface_width(s->surface) &&
+                height == surface_height(s->surface)) {
+                no_change = true;
+            }
+        });
+
+    if (no_change) {
         return;
     }
-    if (locking_required) {
-        bql_unlock();
-    }
 
     vram = g_malloc0(width * height * 4);
     surface = qemu_create_displaysurface_from(width, height, 
PIXMAN_LE_a8r8g8b8,
@@ -220,21 +266,23 @@ static void set_mode(AppleGFXState *s, uint32_t width, 
uint32_t height)
         texture = [s->mtl newTextureWithDescriptor:textureDescriptor];
     }
 
-    if (locking_required) {
-        bql_lock();
-    }
-    old_vram = s->vram;
-    s->vram = vram;
-    s->surface = surface;
-    dpy_gfx_replace_surface(s->con, surface);
-    old_texture = s->texture;
-    s->texture = texture;
-    if (locking_required) {
-        bql_unlock();
-    }
+    dispatch_sync(s->render_queue,
+        ^{
+            id<MTLTexture> old_texture = nil;
+            void *old_vram = s->vram;
+            s->vram = vram;
+            s->surface = surface;
 
-    g_free(old_vram);
-    [old_texture release];
+            dpy_gfx_replace_surface(s->con, surface);
+
+            old_texture = s->texture;
+            s->texture = texture;
+            [old_texture release];
+
+            if (s->pending_frames == 0) {
+                g_free(old_vram);
+            }
+        });
 }
 
 static void create_fb(AppleGFXState *s)
@@ -354,10 +402,18 @@ static void 
apple_gfx_register_task_mapping_handlers(AppleGFXState *s,
     disp_desc.queue = dispatch_get_main_queue();
     disp_desc.newFrameEventHandler = ^(void) {
         trace_apple_gfx_new_frame();
-
-        /* Tell QEMU gfx stack that a new frame arrived */
-        s->handles_frames = true;
-        s->new_frame = true;
+        dispatch_async(s->render_queue, ^{
+            /* Drop frames if we get too far ahead. */
+            if (s->pending_frames >= 2)
+                return;
+            ++s->pending_frames;
+            if (s->pending_frames > 1) {
+                return;
+            }
+            @autoreleasepool {
+                apple_gfx_render_new_frame(s);
+            }
+        });
     };
     disp_desc.modeChangeHandler = ^(PGDisplayCoord_t sizeInPixels,
                                     OSType pixelFormat) {
@@ -422,7 +478,10 @@ void apple_gfx_common_realize(AppleGFXState *s, 
PGDeviceDescriptor *desc)
     PGDisplayDescriptor *disp_desc = nil;
 
     QTAILQ_INIT(&s->tasks);
+    s->render_queue = dispatch_queue_create("apple-gfx.render",
+                                            DISPATCH_QUEUE_SERIAL);
     s->mtl = MTLCreateSystemDefaultDevice();
+    s->mtl_queue = [s->mtl newCommandQueue];
 
     desc.device = s->mtl;
 
-- 
2.39.3 (Apple Git-146)