svga: fix pre-mature flushing of the command buffer

When surface_invalidate is called to invalidate a newly created surface
in svga_validate_surface_view(), it is possible that the command
buffer is already full, and in this case, currently, the associated wddm
winsys function will flush the command buffer and resend the invalidate
surface command. However, this can pre-maturely flush the command buffer
if there is still pending image updates to be patched.

To fix the problem, this patch will add a return status to the
surface_invalidate interface and if it returns FALSE, the caller will
call svga_context_flush() to do the proper context flush.
Note, we don't call svga_context_flush() if surface_invalidate()
fails when flushing the screen surface cache though, because it is
already in the process of context flush, all the image updates are already
patched, calling svga_context_flush() can trigger a deadlock.
So in this case, we call the winsys context flush interface directly
to flush the command buffer.

Fixes driver errors and graphics corruption running Tropics. VMware bug 1891975.

Also tested with MTT glretrace, piglit and various OpenGL apps such as
Heaven, CinebenchR15, NobelClinicianViewer, Lightsmark, GoogleEarth.

cc: mesa-stable@lists.freedesktop.org
Reviewed-by: Brian Paul <brianp@vmware.com>

diff --git a/src/gallium/drivers/svga/svga_screen_cache.c b/src/gallium/drivers/svga/svga_screen_cache.c
index bf66fc60105ae2725a22cc8dfe98296392448337..0816ff68c8acce4608809503b5955fe0f33066b1 100644 (file)
--- a/src/gallium/drivers/svga/svga_screen_cache.c
+++ b/src/gallium/drivers/svga/svga_screen_cache.c
@@ -362,7 +362,21 @@ svga_screen_cache_flush(struct svga_screen *svgascreen,
          /* It is now safe to invalidate the surface content.
           * It will be done using the current context.
           */
-         svga->swc->surface_invalidate(svga->swc, entry->handle);
+         if (svga->swc->surface_invalidate(svga->swc, entry->handle) != PIPE_OK) {
+            enum pipe_error ret;
+
+            /* Even though surface invalidation here is done after the command
+             * buffer is flushed, it is still possible that it will
+             * fail because there might be just enough of this command that is
+             * filling up the command buffer, so in this case we will call
+             * the winsys flush directly to flush the buffer.
+             * Note, we don't want to call svga_context_flush() here because
+             * this function itself is called inside svga_context_flush().
+             */
+            svga->swc->flush(svga->swc, NULL);
+            ret = svga->swc->surface_invalidate(svga->swc, entry->handle);
+            assert(ret == PIPE_OK);
+         }
 
          /* add the entry to the invalidated list */
          LIST_ADD(&entry->head, &cache->invalidated);

diff --git a/src/gallium/drivers/svga/svga_surface.c b/src/gallium/drivers/svga/svga_surface.c
index 04173266ff0c279aea53844b43a64e853438237b..29d91fd32213fda769e8d0612c09f65fb3d9368e 100644 (file)
--- a/src/gallium/drivers/svga/svga_surface.c
+++ b/src/gallium/drivers/svga/svga_surface.c
@@ -531,7 +531,11 @@ svga_validate_surface_view(struct svga_context *svga, struct svga_surface *s)
           * need to update the host-side copy with the invalid
           * content when the associated mob is first bound to the surface.
           */
-         svga->swc->surface_invalidate(svga->swc, stex->handle);
+         if (svga->swc->surface_invalidate(svga->swc, stex->handle) != PIPE_OK) {
+            svga_context_flush(svga, NULL);
+            ret = svga->swc->surface_invalidate(svga->swc, stex->handle);
+            assert(ret == PIPE_OK);
+         }
          stex->validated = TRUE;
       }
 

diff --git a/src/gallium/drivers/svga/svga_winsys.h b/src/gallium/drivers/svga/svga_winsys.h
index 8b8b45b47f42c4564cab4b232e835623e98743fa..8823c115db580b0551291582a4888a1b5d34e8d0 100644 (file)
--- a/src/gallium/drivers/svga/svga_winsys.h
+++ b/src/gallium/drivers/svga/svga_winsys.h
@@ -407,7 +407,7 @@ struct svga_winsys_context
    /**
     * Invalidate the content of this surface
     */
-   void
+   enum pipe_error
    (*surface_invalidate)(struct svga_winsys_context *swc,
                          struct svga_winsys_surface *surface);
 

diff --git a/src/gallium/winsys/svga/drm/vmw_surface.c b/src/gallium/winsys/svga/drm/vmw_surface.c
index 80cc0914cc0e6e75498c39000fb447a947aa7925..04aa932784b944f76983476faa928f756ffcfa5e 100644 (file)
--- a/src/gallium/winsys/svga/drm/vmw_surface.c
+++ b/src/gallium/winsys/svga/drm/vmw_surface.c
@@ -176,7 +176,7 @@ vmw_svga_winsys_surface_unmap(struct svga_winsys_context *swc,
    mtx_unlock(&vsrf->mutex);
 }
 
-void
+enum pipe_error
 vmw_svga_winsys_surface_invalidate(struct svga_winsys_context *swc,
                                    struct svga_winsys_surface *surf)
 {
@@ -186,6 +186,7 @@ vmw_svga_winsys_surface_invalidate(struct svga_winsys_context *swc,
     * when guest-backed surface is enabled, that implies DMA is always enabled;
     * hence, surface invalidation is not needed.
     */
+   return PIPE_OK;
 }
 
 void

diff --git a/src/gallium/winsys/svga/drm/vmw_surface.h b/src/gallium/winsys/svga/drm/vmw_surface.h
index d6f2381220a7ca6a2f9e3f83bf6091f20cadbdef..0fdc8de1d56e5d46bc25a8e0f70ea1537c6b78a0 100644 (file)
--- a/src/gallium/winsys/svga/drm/vmw_surface.h
+++ b/src/gallium/winsys/svga/drm/vmw_surface.h
@@ -94,7 +94,7 @@ void
 vmw_svga_winsys_surface_unmap(struct svga_winsys_context *swc,
                               struct svga_winsys_surface *srf,
                               boolean *rebind);
-void
+enum pipe_error
 vmw_svga_winsys_surface_invalidate(struct svga_winsys_context *swc,
                                    struct svga_winsys_surface *srf);