glthread: fix a perf regression due to draw_always_async flag, fix DrawIndirect

Performance regressed by 31% in one VP2020/Creo subtest because
the draw_always_async flag wasn't implemented correctly. Remove it
instead of fixing it.

While removing it, I noticed that our DrawIndirect async conditions
were incorrect. I fixed them.

Fixes: 3b897719e632a165f - glthread: add ctx->GLThread.draw_always_async to simplify draw checking

Acked-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21566>

diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index f1c4f4e..8973863 100644 (file)
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -1156,7 +1156,7 @@
     </function>
 
     <function name="Begin" deprecated="3.1" exec="beginend"
-              marshal_call_after="_mesa_glthread_Begin(ctx);">
+              marshal_call_after="ctx->GLThread.inside_begin_end = true;">
         <param name="mode" type="GLenum"/>
         <glx rop="4"/>
     </function>
@@ -1371,7 +1371,7 @@
     </function>
 
     <function name="End" deprecated="3.1" exec="beginend"
-              marshal_call_after="_mesa_glthread_End(ctx);">
+              marshal_call_after="ctx->GLThread.inside_begin_end = false;">
         <glx rop="23"/>
     </function>
 

diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
index ca0f638..a744d71 100644 (file)
--- a/src/mesa/main/glthread.c
+++ b/src/mesa/main/glthread.c
@@ -210,7 +210,6 @@ _mesa_glthread_init(struct gl_context *ctx)
    /* glthread takes over all L3 pinning */
    ctx->st->pin_thread_counter = ST_L3_PINNING_DISABLED;
 
-   _mesa_glthread_update_draw_always_async(ctx);
    _mesa_glthread_enable(ctx);
 
    /* Execute the thread initialization function in the thread. */

diff --git a/src/mesa/main/glthread.h b/src/mesa/main/glthread.h
index 55e57c2..65c61bd 100644 (file)
--- a/src/mesa/main/glthread.h
+++ b/src/mesa/main/glthread.h
@@ -185,7 +185,6 @@ struct glthread_state
    /** Whether GLThread is enabled. */
    bool enabled;
    bool inside_begin_end;
-   bool draw_always_async;
 
    /** Display lists. */
    GLenum16 ListMode; /**< Zero if not inside display list, else list mode. */

diff --git a/src/mesa/main/glthread_draw.c b/src/mesa/main/glthread_draw.c
index 6ea42d9..27084bd 100644 (file)
--- a/src/mesa/main/glthread_draw.c
+++ b/src/mesa/main/glthread_draw.c
@@ -444,7 +444,10 @@ draw_arrays(GLuint drawid, GLenum mode, GLint first, GLsizei count,
     * for possible GL errors.
     */
    if (!user_buffer_mask || count <= 0 || instance_count <= 0 ||
-       ctx->GLThread.draw_always_async) {
+       /* This will just generate GL_INVALID_OPERATION, as it should. */
+       ctx->GLThread.inside_begin_end ||
+       ctx->CurrentServerDispatch == ctx->ContextLost ||
+       ctx->GLThread.ListMode) {
       if (instance_count == 1 && baseinstance == 0 && drawid == 0) {
          int cmd_size = sizeof(struct marshal_cmd_DrawArrays);
          struct marshal_cmd_DrawArrays *cmd =
@@ -554,7 +557,8 @@ _mesa_marshal_MultiDrawArrays(GLenum mode, const GLint *first,
    struct glthread_attrib_binding buffers[VERT_ATTRIB_MAX];
    unsigned user_buffer_mask =
       ctx->API == API_OPENGL_CORE || draw_count <= 0 ||
-      ctx->GLThread.draw_always_async ? 0 : get_user_buffer_mask(ctx);
+      ctx->CurrentServerDispatch == ctx->ContextLost ||
+      ctx->GLThread.inside_begin_end ? 0 : get_user_buffer_mask(ctx);
 
    if (user_buffer_mask) {
       unsigned min_index = ~0;
@@ -856,9 +860,13 @@ draw_elements(GLuint drawid, GLenum mode, GLsizei count, GLenum type,
     * This is also an error path. Zero counts should still call the driver
     * for possible GL errors.
     */
-   if (ctx->GLThread.draw_always_async || count <= 0 || instance_count <= 0 ||
+   if (count <= 0 || instance_count <= 0 ||
        !is_index_type_valid(type) ||
-       (!user_buffer_mask && !has_user_indices)) {
+       (!user_buffer_mask && !has_user_indices) ||
+       ctx->CurrentServerDispatch == ctx->ContextLost ||
+       /* This will just generate GL_INVALID_OPERATION, as it should. */
+       ctx->GLThread.inside_begin_end ||
+       ctx->GLThread.ListMode) {
       if (instance_count == 1 && baseinstance == 0 && drawid == 0) {
          int cmd_size = sizeof(struct marshal_cmd_DrawElementsBaseVertex);
          struct marshal_cmd_DrawElementsBaseVertex *cmd =
@@ -1126,7 +1134,8 @@ _mesa_marshal_MultiDrawElementsBaseVertex(GLenum mode, const GLsizei *count,
     * a GL error, we don't upload anything.
     */
    if (draw_count > 0 && is_index_type_valid(type) &&
-       !ctx->GLThread.draw_always_async) {
+       ctx->CurrentServerDispatch != ctx->ContextLost &&
+       !ctx->GLThread.inside_begin_end) {
       user_buffer_mask = ctx->API == API_OPENGL_CORE ? 0 : get_user_buffer_mask(ctx);
       has_user_indices = vao->CurrentElementBufferName == 0;
    }
@@ -1361,6 +1370,19 @@ lower_draw_elements_indirect(struct gl_context *ctx, GLenum mode, GLenum type,
    unmap_draw_indirect_params(ctx);
 }
 
+static inline bool
+draw_indirect_async_allowed(struct gl_context *ctx, unsigned user_buffer_mask)
+{
+   return ctx->API != API_OPENGL_COMPAT ||
+          /* This will just generate GL_INVALID_OPERATION, as it should. */
+          ctx->GLThread.inside_begin_end ||
+          ctx->GLThread.ListMode ||
+          ctx->CurrentServerDispatch == ctx->ContextLost ||
+          /* If the DrawIndirect buffer is bound, it behaves like profile != compat
+           * if there are no user VBOs. */
+          (ctx->GLThread.CurrentDrawIndirectBufferName && !user_buffer_mask);
+}
+
 struct marshal_cmd_DrawArraysIndirect
 {
    struct marshal_cmd_base cmd_base;
@@ -1391,9 +1413,7 @@ _mesa_marshal_DrawArraysIndirect(GLenum mode, const GLvoid *indirect)
    unsigned user_buffer_mask =
       _mesa_is_gles31(ctx) ? 0 : vao->UserPointerMask & vao->BufferEnabled;
 
-   if (ctx->GLThread.draw_always_async ||
-       !ctx->GLThread.CurrentDrawIndirectBufferName ||
-       !user_buffer_mask) {
+   if (draw_indirect_async_allowed(ctx, user_buffer_mask)) {
       int cmd_size = sizeof(struct marshal_cmd_DrawArraysIndirect);
       struct marshal_cmd_DrawArraysIndirect *cmd;
 
@@ -1439,9 +1459,8 @@ _mesa_marshal_DrawElementsIndirect(GLenum mode, GLenum type, const GLvoid *indir
    unsigned user_buffer_mask =
       _mesa_is_gles31(ctx) ? 0 : vao->UserPointerMask & vao->BufferEnabled;
 
-   if (ctx->GLThread.draw_always_async || !is_index_type_valid(type) ||
-       !ctx->GLThread.CurrentDrawIndirectBufferName ||
-       !vao->CurrentElementBufferName || !user_buffer_mask) {
+   if (draw_indirect_async_allowed(ctx, user_buffer_mask) ||
+       !is_index_type_valid(type)) {
       int cmd_size = sizeof(struct marshal_cmd_DrawElementsIndirect);
       struct marshal_cmd_DrawElementsIndirect *cmd;
 
@@ -1492,9 +1511,8 @@ _mesa_marshal_MultiDrawArraysIndirect(GLenum mode, const GLvoid *indirect,
    unsigned user_buffer_mask =
       _mesa_is_gles31(ctx) ? 0 : vao->UserPointerMask & vao->BufferEnabled;
 
-   if (ctx->GLThread.draw_always_async ||
-       !ctx->GLThread.CurrentDrawIndirectBufferName ||
-       !user_buffer_mask) {
+   if (draw_indirect_async_allowed(ctx, user_buffer_mask) ||
+       primcount <= 0) {
       int cmd_size = sizeof(struct marshal_cmd_MultiDrawArraysIndirect);
       struct marshal_cmd_MultiDrawArraysIndirect *cmd;
 
@@ -1551,9 +1569,9 @@ _mesa_marshal_MultiDrawElementsIndirect(GLenum mode, GLenum type,
    unsigned user_buffer_mask =
       _mesa_is_gles31(ctx) ? 0 : vao->UserPointerMask & vao->BufferEnabled;
 
-   if (ctx->GLThread.draw_always_async || !is_index_type_valid(type) ||
-       !ctx->GLThread.CurrentDrawIndirectBufferName ||
-       !vao->CurrentElementBufferName || !user_buffer_mask) {
+   if (draw_indirect_async_allowed(ctx, user_buffer_mask) ||
+       primcount <= 0 ||
+       !is_index_type_valid(type)) {
       int cmd_size = sizeof(struct marshal_cmd_MultiDrawElementsIndirect);
       struct marshal_cmd_MultiDrawElementsIndirect *cmd;
 
@@ -1614,7 +1632,9 @@ _mesa_marshal_MultiDrawArraysIndirectCountARB(GLenum mode, GLintptr indirect,
    unsigned user_buffer_mask =
       _mesa_is_gles31(ctx) ? 0 : vao->UserPointerMask & vao->BufferEnabled;
 
-   if (ctx->GLThread.draw_always_async || !user_buffer_mask ||
+   if (draw_indirect_async_allowed(ctx, user_buffer_mask) ||
+       /* This will just generate GL_INVALID_OPERATION because Draw*IndirectCount
+        * functions forbid a user indirect buffer in the Compat profile. */
        !ctx->GLThread.CurrentDrawIndirectBufferName) {
       int cmd_size =
          sizeof(struct marshal_cmd_MultiDrawArraysIndirectCountARB);
@@ -1677,7 +1697,9 @@ _mesa_marshal_MultiDrawElementsIndirectCountARB(GLenum mode, GLenum type,
    unsigned user_buffer_mask =
       _mesa_is_gles31(ctx) ? 0 : vao->UserPointerMask & vao->BufferEnabled;
 
-   if (ctx->GLThread.draw_always_async || !user_buffer_mask ||
+   if (draw_indirect_async_allowed(ctx, user_buffer_mask) ||
+       /* This will just generate GL_INVALID_OPERATION because Draw*IndirectCount
+        * functions forbid a user indirect buffer in the Compat profile. */
        !ctx->GLThread.CurrentDrawIndirectBufferName ||
        !is_index_type_valid(type)) {
       int cmd_size = sizeof(struct marshal_cmd_MultiDrawElementsIndirectCountARB);

diff --git a/src/mesa/main/glthread_marshal.h b/src/mesa/main/glthread_marshal.h
index e070efd..3fbdd27 100644 (file)
--- a/src/mesa/main/glthread_marshal.h
+++ b/src/mesa/main/glthread_marshal.h
@@ -94,17 +94,6 @@ _mesa_glthread_has_no_unpack_buffer(const struct gl_context *ctx)
    return ctx->GLThread.CurrentPixelUnpackBufferName == 0;
 }
 
-static inline void
-_mesa_glthread_update_draw_always_async(struct gl_context *ctx)
-{
-   /* Executing erroneous cases will just generate GL_INVALID_OPERATION. */
-   ctx->GLThread.draw_always_async =
-      ctx->API == API_OPENGL_CORE ||
-      ctx->CurrentServerDispatch == ctx->ContextLost ||
-      ctx->GLThread.inside_begin_end ||
-      ctx->GLThread.ListMode;
-}
-
 static inline unsigned
 _mesa_buffer_enum_to_count(GLenum buffer)
 {
@@ -849,10 +838,8 @@ _mesa_glthread_CallLists(struct gl_context *ctx, GLsizei n, GLenum type,
 static inline void
 _mesa_glthread_NewList(struct gl_context *ctx, GLuint list, GLenum mode)
 {
-   if (!ctx->GLThread.ListMode) {
+   if (!ctx->GLThread.ListMode)
       ctx->GLThread.ListMode = MIN2(mode, 0xffff);
-      _mesa_glthread_update_draw_always_async(ctx);
-   }
 }
 
 static inline void
@@ -862,7 +849,6 @@ _mesa_glthread_EndList(struct gl_context *ctx)
       return;
 
    ctx->GLThread.ListMode = 0;
-   _mesa_glthread_update_draw_always_async(ctx);
 
    /* Track the last display list change. */
    p_atomic_set(&ctx->GLThread.LastDListChangeBatchIndex, ctx->GLThread.next);
@@ -911,18 +897,4 @@ _mesa_glthread_DeleteFramebuffers(struct gl_context *ctx, GLsizei n,
    }
 }
 
-static inline void
-_mesa_glthread_Begin(struct gl_context *ctx)
-{
-   ctx->GLThread.inside_begin_end = true;
-   _mesa_glthread_update_draw_always_async(ctx);
-}
-
-static inline void
-_mesa_glthread_End(struct gl_context *ctx)
-{
-   ctx->GLThread.inside_begin_end = false;
-   _mesa_glthread_update_draw_always_async(ctx);
-}
-
 #endif /* MARSHAL_H */

diff --git a/src/mesa/main/robustness.c b/src/mesa/main/robustness.c
index ffbd2fe..0ebee73 100644 (file)
--- a/src/mesa/main/robustness.c
+++ b/src/mesa/main/robustness.c
@@ -105,7 +105,6 @@ _mesa_set_context_lost_dispatch(struct gl_context *ctx)
 
    ctx->CurrentServerDispatch = ctx->ContextLost;
    _glapi_set_dispatch(ctx->CurrentServerDispatch);
-   _mesa_glthread_update_draw_always_async(ctx);
 }
 
 /**