video: a hardware core's image is let go of before the core destroys it

Closing content with a Vulkan hardware core under threaded video could
freeze or crash the frontend inside the GPU driver.

A staged content close keeps the video driver up after the core is
gone and presents dupes of the last frame until the next session's
drivers are built. For a hardware core the last frame is the core's
own image, which the core destroys in context_destroy. Nothing told
the driver: video_driver_free_hw_context() called context_destroy and
left the driver holding the image, and every dupe after it sampled a
destroyed VkImageView. Seen on NVIDIA as a fault inside the driver,
called from the filter chain's final pass on the video thread, with
the main thread waiting on that frame in the ring drain.

Under the wrapper two more things made it certain rather than likely:

 - the wrapper had no invalidate_hw_render_cache, so the runloop's
   invalidate (core_reset) stopped at the wrapper and never reached
   the driver;
 - a core's set_image(NULL) - how a core withdraws its image before
   destroying it, and what LRPS2 does in context_destroy - only wrote
   the ring slot the core fills next. A dupe re-reads the slot last
   presented, so the withdrawal never took effect.

Now:

 - video_driver_free_hw_context() invalidates the driver's hardware
   render cache on both sides of context_destroy: before, so nothing
   is drawing from the image while the core tears it down, and after,
   because a core may present once more on its way out;
 - the wrapper implements invalidate_hw_render_cache: drain the frame
   ring, have the driver wait out its submissions and drop the image
   on the video thread, then clear the ring's slots and the slot a
   dupe would re-read;
 - set_image(NULL) under the ring does the same.

A dupe after that draws the driver's blank texture, as it already does
without the wrapper after a reset.

gfx/video_driver.c

@@ -2522,8 +2522,19 @@ void video_driver_init_filter(enum retro_pixel_format colfmt_int,
 void video_driver_free_hw_context(void)
 {
    video_driver_state_t *video_st       = &video_driver_st;
+
+   /* The driver can outlive this: a staged content close keeps it up
+    * and presenting dupes of the last frame until the next session's
+    * drivers are built, and for a hardware core the last frame is an
+    * image the core destroys in context_destroy. So the driver lets go
+    * of it on both sides of the call - before, so nothing is drawing
+    * from the image while the core tears it down, and after, because
+    * a core may present once more on its way out (one that runs ahead
+    * of the frontend drains the frame it had queued). */
+   video_driver_invalidate_hw_render_cache();
    if (video_st->hw_render.context_destroy)
       video_st->hw_render.context_destroy();
+   video_driver_invalidate_hw_render_cache();
 
    memset(&video_st->hw_render, 0, sizeof(video_st->hw_render));
    /* After the memset: an acquire reader that sees NONE is

gfx/video_thread_hw.c

@@ -215,7 +215,17 @@ static void hw_set_image(void *handle,
       s->has_image = true;
    }
    else
+   {
       s->has_image = false;
+      /* A core that withdraws its image means every frame after this
+       * one, dupes included - that is what the call does without the
+       * wrapper, where it writes the driver's state directly. Here it
+       * only reached the slot the core fills next, which is not the
+       * one a dupe re-reads: the ring went on handing the driver the
+       * last presented image, and a core that withdraws it to destroy
+       * it had it drawn from afterwards. */
+      video_thread_invalidate_hw_render_cache(ring->thr);
+   }
    if (num_semaphores > s->cap_semaphores)
    {
       VkSemaphore *grown = (VkSemaphore*)realloc(s->semaphores,
@@ -942,6 +952,49 @@ void video_thread_hw_after_frame(thread_video_t *thr, int hw_slot)
    s->in_flight = true;
 }
 
+bool video_thread_hw_holds_frame(thread_video_t *thr)
+{
+#ifdef HAVE_VULKAN
+   hw_ring_t *ring = (hw_ring_t*)thr->frame.hw_ring;
+   unsigned i;
+   if (!ring || ring->api != HW_API_VULKAN)
+      return false;
+   if (retro_atomic_load_acquire_int(&ring->last_presented) >= 0)
+      return true;
+   for (i = 0; i < VIDEO_THREAD_HW_RING; i++)
+   {
+      if (ring->slot[i].has_image)
+         return true;
+   }
+#else
+   (void)thr;
+#endif
+   return false;
+}
+
+void video_thread_hw_forget(thread_video_t *thr)
+{
+#ifdef HAVE_VULKAN
+   hw_ring_t *ring = (hw_ring_t*)thr->frame.hw_ring;
+   unsigned i;
+   if (!ring || ring->api != HW_API_VULKAN)
+      return;
+   for (i = 0; i < VIDEO_THREAD_HW_RING; i++)
+   {
+      hw_slot_t *s      = &ring->slot[i];
+      /* The empty submission behind the slot's last frame. */
+      hw_wait_slot(ring, i);
+      s->has_image      = false;
+      s->num_semaphores = 0;
+      s->num_cmd        = 0;
+   }
+   /* A dupe has no slot to re-read until the core presents again. */
+   retro_atomic_store_release_int(&ring->last_presented, -1);
+#else
+   (void)thr;
+#endif
+}
+
 static void hw_context_free_cb(void *data)
 {
    thread_video_t *thr = (thread_video_t*)data;
@@ -1055,6 +1108,8 @@ void video_thread_hw_free(thread_video_t *thr) { (void)thr; }
 bool video_thread_hw_allowed(void) { return false; }
 bool video_thread_hw_bind_core_context(void *data) { (void)data; return false; }
 uintptr_t video_thread_hw_get_current_framebuffer(void *data) { (void)data; return 0; }
+bool video_thread_hw_holds_frame(thread_video_t *thr) { (void)thr; return false; }
+void video_thread_hw_forget(thread_video_t *thr) { (void)thr; }
 
 #endif /* VIDEO_THREAD_HW_ANY */
 

gfx/video_thread_hw.h

@@ -89,6 +89,16 @@ bool video_thread_hw_bind_core_context(void *data);
 /* thread_poke.get_current_framebuffer for OpenGL cores. Main thread. */
 uintptr_t video_thread_hw_get_current_framebuffer(void *data);
 
+/* Main thread: whether the ring holds anything of the core's - an image
+ * in a slot, or a presented slot a dupe would read again. Vulkan only; the other APIs' slots hold a reference or
+ * are the driver's own. */
+bool video_thread_hw_holds_frame(struct thread_video *thr);
+
+/* Main thread, with no frame queued or being drawn and the driver's own
+ * hold on the core's image dropped: the ring forgets it too, so nothing
+ * installs it again. See video_thread_invalidate_hw_render_cache(). */
+void video_thread_hw_forget(struct thread_video *thr);
+
 /* Video thread, before the driver is freed: releases the ring and its
  * fences while the device is still up. */
 void video_thread_hw_free(struct thread_video *thr);

gfx/video_thread_wrapper.c

@@ -4167,8 +4167,12 @@ static const video_driver_t video_thread = {
    NULL, /* shader_load_begin */
    NULL, /* shader_load_step */
 #ifdef HAVE_GFX_WIDGETS
-   video_thread_wrapper_gfx_widgets_enabled
+   video_thread_wrapper_gfx_widgets_enabled,
 #endif
+   /* Without this the runloop's invalidate stopped at the wrapper: the
+    * driver kept the core's image across a reset, and the ring kept
+    * handing it back across a close. */
+   video_thread_invalidate_hw_render_cache
 };
 
 static void video_thread_set_callbacks(thread_video_t *thr,
@@ -4476,6 +4480,34 @@ void video_thread_status_text(const char *s)
 }
 #endif
 
+static uintptr_t video_thread_invalidate_hw_cb(void *data)
+{
+   thread_video_t *thr = (thread_video_t*)data;
+   if (thr->driver && thr->driver->invalidate_hw_render_cache)
+      thr->driver->invalidate_hw_render_cache(thr->driver_data);
+   return 0;
+}
+
+void video_thread_invalidate_hw_render_cache(void *data)
+{
+   thread_video_t *thr = (thread_video_t*)data;
+
+   if (!thr || !thr->thread)
+      return;
+   /* From the video thread the frame in progress is the caller's own. */
+   if (sthread_get_thread_id(thr->thread) == sthread_get_current_thread_id())
+      return;
+   if (!video_thread_hw_holds_frame(thr))
+      return;
+
+   /* In this order: nothing queued or being drawn, then the driver
+    * waits out what it submitted and lets go of the image on the
+    * thread that owns its state, then the ring stops offering it. */
+   video_thread_ring_drain(thr);
+   video_thread_run_blocking(video_thread_invalidate_hw_cb, thr);
+   video_thread_hw_forget(thr);
+}
+
 void video_thread_wait_idle(void)
 {
    video_driver_state_t *video_st = video_state_get_ptr();

gfx/video_thread_wrapper.h

@@ -953,6 +953,14 @@ void video_thread_call_on_waiter(void (*fn)(void *data), void *data);
 
 void video_thread_wait_idle(void);
 
+/* The wrapper's invalidate_hw_render_cache, and what a hardware core's
+ * set_image(NULL) means under the ring. Main thread. On return no
+ * frame that reads the core's image is queued, being drawn or still on
+ * the GPU, and neither the driver nor the ring will hand it to a later
+ * one: a dupe draws the driver's blank texture until the core presents
+ * again. The core's image is then the core's to destroy. */
+void video_thread_invalidate_hw_render_cache(void *data);
+
 /* GPU recording under the wrapper, main thread. The video thread reads
  * back each frame it draws through a dedicated recording reader; this
  * takes the newest since the last call without waiting for one.